On 04/03/2025 17:07, Biju Das wrote: > Hi Paul, > >> -----Original Message----- >> From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Paul Barker >> Subject: [PATCH 1/5] reset: rzg2l-usbphy-ctrl: Add new driver
>> diff --git a/include/renesas/rzg2l-usbphy.h b/include/renesas/rzg2l-usbphy.h
>> new file mode 100644
>> index 000000000000..1a46b585f170
>> --- /dev/null
>> +++ b/include/renesas/rzg2l-usbphy.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * RZ/G2L USB PHY common definitions
>> + *
>> + * Copyright (C) 2021-2023 Renesas Electronics Corp.
>> + */
>> +
>> +#ifndef RENESAS_RZG2L_USBPHY_H
>> +#define RENESAS_RZG2L_USBPHY_H
>> +
>> +#include <fdtdec.h>
>> +
>> +struct rzg2l_usbphy_ctrl_priv {
>> + fdt_addr_t regs;
>
> This header file may not be required. As it is a single variable in
> struct. If you plan to add more variables in this struct, then keep it.
>
> Otherwise, You can directly use driver data as regs in .c file like below??
>
> struct fdt_addr_t *regs = dev_get_priv(dev);
Hi Biju,
I prefer to have a struct defined in a header file in case we need to
change things in the future. This ensures that both
rzg2l-usbphy-regulator.c and reset-rzg2l-usbphy-ctrl.c use the same type
definition. Without a structure and a common header, it would be far too
easy for someone to accidentally change the type in the reset driver and
not notice the associated regulator driver.
There is a small cost of allocating space for an instance of struct
rzg2l_usbphy_ctrl_priv, but I think this is ok.
Thanks,
--
Paul Barker
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
