RE: [PATCH v3 3/7] dt-bindings: clock: Add R9A07G043 CPG Clock and Reset Definitions

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

 



> Subject: RE: [PATCH v3 3/7] dt-bindings: clock: Add R9A07G043 CPG Clock
> and Reset Definitions
> 
> Hi Geert,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH v3 3/7] dt-bindings: clock: Add R9A07G043 CPG
> > Clock and Reset Definitions
> >
> > Hi Biju,
> >
> > On Tue, Mar 15, 2022 at 3:26 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > wrote:
> > > Define RZ/G2UL (R9A07G043U) Clock Pulse Generator Core Clock and
> > > module clock outputs, as listed in Table 7.1.4.2 ("Clock List
> > > r0.51") and also add Reset definitions referring to registers
> > > CPG_RST_* in Section 7.2.3 ("Register configuration") of the RZ/G2UL
> > > Hardware User's
> > Manual (Rev.
> > > 0.51, Nov. 2021).
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > ---
> > > v2->v3:
> > >  * Removed leading u/U from r9a07g043
> > >  * renamed the file r9a07g043u-cpg.h->r9a07g043-cpg.h
> > >  * Prepared Common Module Clock/Reset indices for RZ/G2UL and
> > > RZ/Five
> > >  * Prepared RZ/G2UL specific Module Clock/Reset indices.
> >
> > Thanks for the update!
> >
> > > --- /dev/null
> > > +++ b/include/dt-bindings/clock/r9a07g043-cpg.h
> > > @@ -0,0 +1,190 @@
> > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > + *
> > > + * Copyright (C) 2022 Renesas Electronics Corp.
> > > + */
> > > +#ifndef __DT_BINDINGS_CLOCK_R9A07G043_CPG_H__
> > > +#define __DT_BINDINGS_CLOCK_R9A07G043_CPG_H__
> > > +
> > > +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> > > +
> > > +/* R9A07G043 CPG Core Clocks */
> > > +#define R9A07G043_CLK_I                        0
> > > +#define R9A07G043_CLK_I2               1
> > > +#define R9A07G043_CLK_S0               2
> > > +#define R9A07G043_CLK_SPI0             3
> > > +#define R9A07G043_CLK_SPI1             4
> > > +#define R9A07G043_CLK_SD0              5
> > > +#define R9A07G043_CLK_SD1              6
> > > +#define R9A07G043_CLK_M0               7
> > > +#define R9A07G043_CLK_M2               8
> > > +#define R9A07G043_CLK_M3               9
> > > +#define R9A07G043_CLK_HP               10
> > > +#define R9A07G043_CLK_TSU              11
> > > +#define R9A07G043_CLK_ZT               12
> > > +#define R9A07G043_CLK_P0               13
> > > +#define R9A07G043_CLK_P1               14
> > > +#define R9A07G043_CLK_P2               15
> > > +#define R9A07G043_CLK_AT               16
> > > +#define R9A07G043_OSCCLK               17
> > > +#define R9A07G043_CLK_P0_DIV2          18
> > > +
> > > +/* R9A07G043 Common Module Clocks */
> > > +#define R9A07G043_IA55_CLK             0
> > > +#define R9A07G043_IA55_PCLK            1
> >
> > I think IA55 does not exist on RZ/Five?
> >
> > > +#define R9A07G043_DMAC_ACLK            2
> > > +#define R9A07G043_DMAC_PCLK            3
> > > +#define R9A07G043_OSTM0_PCLK           4
> > > +#define R9A07G043_OSTM1_PCLK           5
> > > +#define R9A07G043_OSTM2_PCLK           6
> > > +#define R9A07G043_MTU_X_MCK_MTU3       7
> > > +#define R9A07G043_POE3_CLKM_POE                8
> > > +#define R9A07G043_WDT0_PCLK            9
> > > +#define R9A07G043_WDT0_CLK             10
> > > +#define R9A07G043_SPI_CLK2             11
> > > +#define R9A07G043_SPI_CLK              12
> > > +#define R9A07G043_SDHI0_IMCLK          13
> > > +#define R9A07G043_SDHI0_IMCLK2         14
> > > +#define R9A07G043_SDHI0_CLK_HS         15
> > > +#define R9A07G043_SDHI0_ACLK           16
> > > +#define R9A07G043_SDHI1_IMCLK          17
> > > +#define R9A07G043_SDHI1_IMCLK2         18
> > > +#define R9A07G043_SDHI1_CLK_HS         19
> > > +#define R9A07G043_SDHI1_ACLK           20
> > > +#define R9A07G043_SSI0_PCLK2           21
> > > +#define R9A07G043_SSI0_PCLK_SFR                22
> > > +#define R9A07G043_SSI1_PCLK2           23
> > > +#define R9A07G043_SSI1_PCLK_SFR                24
> > > +#define R9A07G043_SSI2_PCLK2           25
> > > +#define R9A07G043_SSI2_PCLK_SFR                26
> > > +#define R9A07G043_SSI3_PCLK2           27
> > > +#define R9A07G043_SSI3_PCLK_SFR                28
> > > +#define R9A07G043_SRC_CLKP             29
> > > +#define R9A07G043_USB_U2H0_HCLK                30
> > > +#define R9A07G043_USB_U2H1_HCLK                31
> > > +#define R9A07G043_USB_U2P_EXR_CPUCLK   32
> > > +#define R9A07G043_USB_PCLK             33
> > > +#define R9A07G043_ETH0_CLK_AXI         34
> > > +#define R9A07G043_ETH0_CLK_CHI         35
> > > +#define R9A07G043_ETH1_CLK_AXI         36
> > > +#define R9A07G043_ETH1_CLK_CHI         37
> > > +#define R9A07G043_I2C0_PCLK            38
> > > +#define R9A07G043_I2C1_PCLK            39
> > > +#define R9A07G043_I2C2_PCLK            40
> > > +#define R9A07G043_I2C3_PCLK            41
> > > +#define R9A07G043_SCIF0_CLK_PCK                42
> > > +#define R9A07G043_SCIF1_CLK_PCK                43
> > > +#define R9A07G043_SCIF2_CLK_PCK                44
> > > +#define R9A07G043_SCIF3_CLK_PCK                45
> > > +#define R9A07G043_SCIF4_CLK_PCK                46
> > > +#define R9A07G043_SCI0_CLKP            47
> > > +#define R9A07G043_SCI1_CLKP            48
> > > +#define R9A07G043_IRDA_CLKP            49
> > > +#define R9A07G043_RSPI0_CLKB           50
> > > +#define R9A07G043_RSPI1_CLKB           51
> > > +#define R9A07G043_RSPI2_CLKB           52
> > > +#define R9A07G043_CANFD_PCLK           53
> > > +#define R9A07G043_GPIO_HCLK            54
> > > +#define R9A07G043_ADC_ADCLK            55
> > > +#define R9A07G043_ADC_PCLK             56
> > > +#define R9A07G043_TSU_PCLK             57
> > > +#define R9A07G043_LAST_COMMON_CLK      (R9A07G043_TSU_PCLK)
> >
> > Does R9A07G043_LAST_COMMON_CLK need to be part of the bindings?
> 
> I thought we can reuse same header file for both RZ/Five and RZ/G2UL.
> The same can be achieved as per your suggestion below.
> 
> > Do you actually have a use case for this definition, besides the use
> > below?  If not, I would get rid of the definition, and just hardcode
> > the numeric values below.
> >
> > Perhaps you planned to start enumerating RZ/Five-specific clocks from
> > R9A07G043_LAST_COMMON_CLK + 1, too?  I don't think that's a good idea,
> > as it would complicate validation of indices in the driver.
> 
> OK, Agreed.
> 
> >
> > > +
> > > +/* RZ/G2UL Specific */
> > > +#define R9A07G043_CA55_SCLK            (R9A07G043_LAST_COMMON_CLK +
> 1)
> > > +#define R9A07G043_CA55_PCLK            (R9A07G043_LAST_COMMON_CLK +
> 2)
> > > +#define R9A07G043_CA55_ATCLK           (R9A07G043_LAST_COMMON_CLK +
> 3)
> > > +#define R9A07G043_CA55_GICCLK          (R9A07G043_LAST_COMMON_CLK +
> 4)
> > > +#define R9A07G043_CA55_PERICLK         (R9A07G043_LAST_COMMON_CLK +
> 5)
> > > +#define R9A07G043_CA55_ACLK            (R9A07G043_LAST_COMMON_CLK +
> 6)
> > > +#define R9A07G043_CA55_TSCLK           (R9A07G043_LAST_COMMON_CLK +
> 7)
> > > +#define R9A07G043_GIC600_GICCLK
> > (R9A07G043_LAST_COMMON_CLK + 8)
> > > +#define R9A07G043_MHU_PCLK             (R9A07G043_LAST_COMMON_CLK +
> 9)
> > > +#define R9A07G043_SYC_CNT_CLK          (R9A07G043_LAST_COMMON_CLK +
> 10)
> >
> > I think SYC_CNT does exist on RZ/Five?
> 
> Basically SYC (System Counter) for RZ/G2UL and MT (Machine Timer) for
> RZ/Five. See page 26 of RZ/Five HW manual

Since RZ/Five Clock list Document mention it as SYC_CNT, I guess we can
use SYS_CNT for RZ/Five. Any way I will check this with HW team.

Regards,
Biju




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux