RE: [PATCH] dt-bindings: clk: renesas: Update {ETH,SDHI,USB} CPG Clock Definitions

Hi Geert,

Thanks for the feedback.

Subject: Re: [PATCH] dt-bindings: clk: renesas: Update {ETH,SDHI,USB} CPG
Clock Definitions

Hi Biju,

On Wed, Jun 16, 2021 at 1:18 PM Biju Das [off-list ref]
wrote:

quoted

Update {ETH, SDHI, USB} clock definitions, as they need special
handling.

ETH has 2 clocks controlled by single bit.
USB has 4 clocks pclock is shared by USB Ch0 and USB Ch1.
SDHI has 4 clocks.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

quoted

--- a/include/dt-bindings/clock/r9a07g044-cpg.h
+++ b/include/dt-bindings/clock/r9a07g044-cpg.h

@@ -39,51 +39,61 @@
 #define R9A07G044_CLK_SYSC             4
 #define R9A07G044_CLK_MTU              5
 #define R9A07G044_CLK_GPT              6
-#define R9A07G044_CLK_ETH0             7
-#define R9A07G044_CLK_ETH1             8
-#define R9A07G044_CLK_I2C0             9
-#define R9A07G044_CLK_I2C1             10
-#define R9A07G044_CLK_I2C2             11
-#define R9A07G044_CLK_I2C3             12
-#define R9A07G044_CLK_SCIF0            13
-#define R9A07G044_CLK_SCIF1            14
-#define R9A07G044_CLK_SCIF2            15
-#define R9A07G044_CLK_SCIF3            16
-#define R9A07G044_CLK_SCIF4            17
-#define R9A07G044_CLK_SCI0             18
-#define R9A07G044_CLK_SCI1             19
-#define R9A07G044_CLK_GPIO             20
-#define R9A07G044_CLK_SDHI0            21
-#define R9A07G044_CLK_SDHI1            22
-#define R9A07G044_CLK_USB0             23
-#define R9A07G044_CLK_USB1             24
-#define R9A07G044_CLK_CANFD            25
-#define R9A07G044_CLK_SSI0             26
-#define R9A07G044_CLK_SSI1             27
-#define R9A07G044_CLK_SSI2             28
-#define R9A07G044_CLK_SSI3             29
-#define R9A07G044_CLK_MHU              30
-#define R9A07G044_CLK_OSTM0            31
-#define R9A07G044_CLK_OSTM1            32
-#define R9A07G044_CLK_OSTM2            33
-#define R9A07G044_CLK_WDT0             34
-#define R9A07G044_CLK_WDT1             35
-#define R9A07G044_CLK_WDT2             36
-#define R9A07G044_CLK_WDT_PON          37
-#define R9A07G044_CLK_GPU              38
-#define R9A07G044_CLK_ISU              39
-#define R9A07G044_CLK_H264             40
-#define R9A07G044_CLK_CRU              41
-#define R9A07G044_CLK_MIPI_DSI         42
-#define R9A07G044_CLK_LCDC             43
-#define R9A07G044_CLK_SRC              44
-#define R9A07G044_CLK_RSPI0            45
-#define R9A07G044_CLK_RSPI1            46
-#define R9A07G044_CLK_RSPI2            47
-#define R9A07G044_CLK_ADC              48
-#define R9A07G044_CLK_TSU_PCLK         49
-#define R9A07G044_CLK_SPI              50
-#define R9A07G044_CLK_MIPI_DSI_V       51
-#define R9A07G044_CLK_MIPI_DSI_PIN     52
+#define ETH0_CLK_AXI                   7
+#define ETH0_CLK_CHI                   8
+#define ETH1_CLK_AXI                   9
+#define ETH1_CLK_CHI                   10

R9A07G044_ETH0_CLK_AXI etc.?

OK. Will fix that.

quoted

+#define R9A07G044_CLK_I2C0             11
+#define R9A07G044_CLK_I2C1             12
+#define R9A07G044_CLK_I2C2             13
+#define R9A07G044_CLK_I2C3             14
+#define R9A07G044_CLK_SCIF0            15
+#define R9A07G044_CLK_SCIF1            16
+#define R9A07G044_CLK_SCIF2            17
+#define R9A07G044_CLK_SCIF3            18
+#define R9A07G044_CLK_SCIF4            19
+#define R9A07G044_CLK_SCI0             20
+#define R9A07G044_CLK_SCI1             21
+#define R9A07G044_CLK_GPIO             22
+#define R9A07G044_CLK_SDHI0_IMCLK      23
+#define R9A07G044_CLK_SDHI0_IMCLK2     24
+#define R9A07G044_CLK_SDHI0_CLK_HS     25
+#define R9A07G044_CLK_SDHI0_ACLK       26
+#define R9A07G044_CLK_SDHI1_IMCLK      27
+#define R9A07G044_CLK_SDHI1_IMCLK2     28
+#define R9A07G044_CLK_SDHI1_CLK_HS     29
+#define R9A07G044_CLK_SDHI1_ACLK       30
+#define R9A07G044_CLK_USB_U2H0_HCLK    31
+#define R9A07G044_CLK_USB_U2H1_HCLK    32
+#define R9A07G044_CLK_HSUSB            33
+#define R9A07G044_CLK_USB_PCLK         34

I think we should use the opportunity to
  1. Split the remaining module clocks like R9A07G044_CLK_IA55 and
     R9A07G044_CLK_DMAC,
  2. Rename the definitions to match the "Clock Name" column in the
     RZG2L clock list, with an "R9A07G044_" prefix,

OK I will create patches fixing 1 and 2. 

  3. Add all missing clocks, as listed in the RZG2L clock list.
This will prevent similar issues from popping up later, when the DT
bindings clock list is part of a released kernel version, and becomes cast
in stone and append-only (so yes, step 3 could be postponed).

Do you agree?

Yes I agree

Regards,
Biju