RE: [PATCH v2 03/13] dt-bindings: clock: Add r9a09g011 CPG Clock Definitions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

On 22 April 2022 16:02 Geert Uytterhoeven wrote:
> On Fri, Apr 22, 2022 at 1:29 PM Phil Edworthy wrote:
> > On 20 April 2022 22:13 Geert Uytterhoeven wrote:
> > > On Wed, Mar 30, 2022 at 5:41 PM Phil Edworthy wrote:
> > > > Define RZ/V2M (R9A09G011) Clock Pulse Generator core clocks, module
> > > clock
> > >
> > > The definitions contain no core clocks, only module clocks and resets?
> > > Perhaps you will need a core clock for the Ethernet reference clock,
> > > like on RZ/G2L?
> > It looks like rz/v2m has a gate for every clock, so no need for any core
> > clocks.
> 
> OK. Then please remove "core clock," from the patch description.
Will do.

> > > > outputs (CPG_CLK_ON* registers), and reset definitions (CPG_RST_*
> > > > registers) in Section 48.5 ("Register Description") of the RZ/V2M
> > > Hardware
> > > > User's Manual (Rev. 1.10, Sep. 2021).
> > > >
> > > > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> 
> > > > --- /dev/null
> > > > +++ b/include/dt-bindings/clock/r9a09g011-cpg.h
> 
> > > > +#define R9A09G011_IIC01_PCLK           79
> > >
> > > IIC0_PCLK?
> > There are four IIC peripherals, this pclk if for iic0 and iic1.
> 
> I know.
> 
> > Would IIC0_1_PCLK be a better name for this?
> >
> > > > +#define R9A09G011_IIC23_PCLK           89
> > > IIC1_PCLK?
> > and IIC2_3_PCLK?
> 
> Well, IIC0_PCLK andIIC1_PCLK match the Hardware Manual.
> 
> BTW, for resets, they avoided the confusion by naming the groups
> A and B, instead of 0 and 1.
Since the HW manual describes these as IIC_PCLK[0] and IIC_PCLK[1],
and I've changed the names of clks so the module index comes after
the module name to match other Renesas SoCs, how about IIC_PCLK0
and IIC_PCLK1 to differentiate them?


<added this back in for discussion>
> > +#define R9A09G011_PMC_RESET_N          10
> > +
> > +#define R9A09G011_CST_NTRST            11
> > +#define R9A09G011_CST_NPOTRST          12

> Missing (shared) CST_NTRST?
Actually, CST_NTRST is already defined on the line before it.
However, the HW manual says "CST_NTRST" in both bit 0 and bit 1 of
CPG_RST2. I'll ask the HW people what the difference is.

Thanks
Phil






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux