Hi Paul,

> -----Original Message-----
> From: Paul Barker <paul.barker...@bp.renesas.com>
> Sent: 05 March 2025 11:37
> Subject: Re: [PATCH 1/5] reset: rzg2l-usbphy-ctrl: Add new driver
> 
> 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.

We can avoid unnecessary public file. That is the only reason.

Cheers,
Biju

Reply via email to