| Message ID | 2e083461752c9d52fdb251ad9071f6111f13c3c5.1704726960.git.geert+renesas@glider.be |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: renesas: Add R-Car V4M and Gray Hawk Single support | expand |
Hi Geert, Thanks for your patch. On 2024-01-08 16:33:41 +0100, Geert Uytterhoeven wrote: > From: Duy Nguyen <duy.nguyen.rh@renesas.com> > > Add all Clock Pulse Generator Core Clock Outputs for the Renesas R-Car > V4M (R8A779H0) SoC. > > Signed-off-by: Duy Nguyen <duy.nguyen.rh@renesas.com> > Signed-off-by: Hai Pham <hai.pham.ud@renesas.com> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> I have no opinion on Krzysztof comment about renaming the file, for the file content, Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > Changes compared to the BSP: > - Drop POST* clocks, as they are internal clocks. > --- > include/dt-bindings/clock/r8a779h0-cpg-mssr.h | 96 +++++++++++++++++++ > 1 file changed, 96 insertions(+) > create mode 100644 include/dt-bindings/clock/r8a779h0-cpg-mssr.h > > diff --git a/include/dt-bindings/clock/r8a779h0-cpg-mssr.h b/include/dt-bindings/clock/r8a779h0-cpg-mssr.h > new file mode 100644 > index 0000000000000000..baf41231c215acb3 > --- /dev/null > +++ b/include/dt-bindings/clock/r8a779h0-cpg-mssr.h > @@ -0,0 +1,96 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > +/* > + * Copyright (C) 2023 Renesas Electronics Corp. > + */ > +#ifndef __DT_BINDINGS_CLOCK_R8A779H0_CPG_MSSR_H__ > +#define __DT_BINDINGS_CLOCK_R8A779H0_CPG_MSSR_H__ > + > +#include <dt-bindings/clock/renesas-cpg-mssr.h> > + > +/* r8a779h0 CPG Core Clocks */ > + > +#define R8A779H0_CLK_ZX 0 > +#define R8A779H0_CLK_ZD 1 > +#define R8A779H0_CLK_ZS 2 > +#define R8A779H0_CLK_ZT 3 > +#define R8A779H0_CLK_ZTR 4 > +#define R8A779H0_CLK_S0D2 5 > +#define R8A779H0_CLK_S0D3 6 > +#define R8A779H0_CLK_S0D4 7 > +#define R8A779H0_CLK_S0D1_VIO 8 > +#define R8A779H0_CLK_S0D2_VIO 9 > +#define R8A779H0_CLK_S0D4_VIO 10 > +#define R8A779H0_CLK_S0D8_VIO 11 > +#define R8A779H0_CLK_VIOBUSD1 12 > +#define R8A779H0_CLK_VIOBUSD2 13 > +#define R8A779H0_CLK_S0D1_VC 14 > +#define R8A779H0_CLK_S0D2_VC 15 > +#define R8A779H0_CLK_S0D4_VC 16 > +#define R8A779H0_CLK_VCBUSD1 17 > +#define R8A779H0_CLK_VCBUSD2 18 > +#define R8A779H0_CLK_S0D2_MM 19 > +#define R8A779H0_CLK_S0D4_MM 20 > +#define R8A779H0_CLK_S0D2_U3DG 21 > +#define R8A779H0_CLK_S0D4_U3DG 22 > +#define R8A779H0_CLK_S0D2_RT 23 > +#define R8A779H0_CLK_S0D3_RT 24 > +#define R8A779H0_CLK_S0D4_RT 25 > +#define R8A779H0_CLK_S0D6_RT 26 > +#define R8A779H0_CLK_S0D2_PER 27 > +#define R8A779H0_CLK_S0D3_PER 28 > +#define R8A779H0_CLK_S0D4_PER 29 > +#define R8A779H0_CLK_S0D6_PER 30 > +#define R8A779H0_CLK_S0D12_PER 31 > +#define R8A779H0_CLK_S0D24_PER 32 > +#define R8A779H0_CLK_S0D1_HSC 33 > +#define R8A779H0_CLK_S0D2_HSC 34 > +#define R8A779H0_CLK_S0D4_HSC 35 > +#define R8A779H0_CLK_S0D8_HSC 36 > +#define R8A779H0_CLK_SVD1_IR 37 > +#define R8A779H0_CLK_SVD2_IR 38 > +#define R8A779H0_CLK_IMPAD1 39 > +#define R8A779H0_CLK_IMPAD4 40 > +#define R8A779H0_CLK_IMPB 41 > +#define R8A779H0_CLK_SVD1_VIP 42 > +#define R8A779H0_CLK_SVD2_VIP 43 > +#define R8A779H0_CLK_CL 44 > +#define R8A779H0_CLK_CL16M 45 > +#define R8A779H0_CLK_CL16M_MM 46 > +#define R8A779H0_CLK_CL16M_RT 47 > +#define R8A779H0_CLK_CL16M_PER 48 > +#define R8A779H0_CLK_CL16M_HSC 49 > +#define R8A779H0_CLK_ZC0 50 > +#define R8A779H0_CLK_ZC1 51 > +#define R8A779H0_CLK_ZC2 52 > +#define R8A779H0_CLK_ZC3 53 > +#define R8A779H0_CLK_ZB3 54 > +#define R8A779H0_CLK_ZB3D2 55 > +#define R8A779H0_CLK_ZB3D4 56 > +#define R8A779H0_CLK_ZG 57 > +#define R8A779H0_CLK_SD0H 58 > +#define R8A779H0_CLK_SD0 59 > +#define R8A779H0_CLK_RPC 60 > +#define R8A779H0_CLK_RPCD2 61 > +#define R8A779H0_CLK_MSO 62 > +#define R8A779H0_CLK_CANFD 63 > +#define R8A779H0_CLK_CSI 64 > +#define R8A779H0_CLK_FRAY 65 > +#define R8A779H0_CLK_IPC 66 > +#define R8A779H0_CLK_SASYNCRT 67 > +#define R8A779H0_CLK_SASYNCPERD1 68 > +#define R8A779H0_CLK_SASYNCPERD2 69 > +#define R8A779H0_CLK_SASYNCPERD4 70 > +#define R8A779H0_CLK_DSIEXT 71 > +#define R8A779H0_CLK_DSIREF 72 > +#define R8A779H0_CLK_ADGH 73 > +#define R8A779H0_CLK_OSC 74 > +#define R8A779H0_CLK_ZR0 75 > +#define R8A779H0_CLK_ZR1 76 > +#define R8A779H0_CLK_ZR2 77 > +#define R8A779H0_CLK_RGMII 78 > +#define R8A779H0_CLK_CPEX 79 > +#define R8A779H0_CLK_CP 80 > +#define R8A779H0_CLK_CBFUSA 81 > +#define R8A779H0_CLK_R 82 > + > +#endif /* __DT_BINDINGS_CLOCK_R8A779H0_CPG_MSSR_H__ */ > -- > 2.34.1 > >
Hi Krzysztof, On Tue, Jan 9, 2024 at 8:21 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 08/01/2024 16:33, Geert Uytterhoeven wrote: > > From: Duy Nguyen <duy.nguyen.rh@renesas.com> > > > > Add all Clock Pulse Generator Core Clock Outputs for the Renesas R-Car > > V4M (R8A779H0) SoC. > > > > Signed-off-by: Duy Nguyen <duy.nguyen.rh@renesas.com> > > Signed-off-by: Hai Pham <hai.pham.ud@renesas.com> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > Changes compared to the BSP: > > - Drop POST* clocks, as they are internal clocks. > > --- > > include/dt-bindings/clock/r8a779h0-cpg-mssr.h | 96 +++++++++++++++++++ > > Mediatek was able to switch to vendor,device naming scheme, so Renesas > should follow as well. For new bindings, or also for existing ones? Doing the former means there are inconsistencies among different SoCs in the same family. Doing the latter requires keeping the existing header files as wrappers including the new header files, because the binding definitions are part of the stable DT API. Thanks! Gr{oetje,eeting}s, Geert
On 15/01/2024 10:27, Geert Uytterhoeven wrote: > Hi Krzysztof, > > On Tue, Jan 9, 2024 at 8:21 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 08/01/2024 16:33, Geert Uytterhoeven wrote: >>> From: Duy Nguyen <duy.nguyen.rh@renesas.com> >>> >>> Add all Clock Pulse Generator Core Clock Outputs for the Renesas R-Car >>> V4M (R8A779H0) SoC. >>> >>> Signed-off-by: Duy Nguyen <duy.nguyen.rh@renesas.com> >>> Signed-off-by: Hai Pham <hai.pham.ud@renesas.com> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>> --- >>> Changes compared to the BSP: >>> - Drop POST* clocks, as they are internal clocks. >>> --- >>> include/dt-bindings/clock/r8a779h0-cpg-mssr.h | 96 +++++++++++++++++++ >> >> Mediatek was able to switch to vendor,device naming scheme, so Renesas >> should follow as well. > > For new bindings, or also for existing ones? > Doing the former means there are inconsistencies among different SoCs > in the same family. For the new ones. It's just naming inconsistency which does not cause any issues. Otherwise you never move to vendor,device.h format... which is not that critical, but in the long term brings uniformity. > Doing the latter requires keeping the existing header files as wrappers > including the new header files, because the binding definitions are > part of the stable DT API. This could also work if you want all the filenames to be consistent. I would go only with new ones, though. Best regards, Krzysztof
> > For new bindings, or also for existing ones? > > Doing the former means there are inconsistencies among different SoCs > > in the same family. > > For the new ones. It's just naming inconsistency which does not cause > any issues. Otherwise you never move to vendor,device.h format... which > is not that critical, but in the long term brings uniformity. I haven't reviewed the patch itself, but for the naming scheme I'd suggest to follow Krzysztof's suggestion. If he is OK with this level of uniformity, then all is good, I'd say. Converting existing ones sounds more messy to me.
diff --git a/include/dt-bindings/clock/r8a779h0-cpg-mssr.h b/include/dt-bindings/clock/r8a779h0-cpg-mssr.h new file mode 100644 index 0000000000000000..baf41231c215acb3 --- /dev/null +++ b/include/dt-bindings/clock/r8a779h0-cpg-mssr.h @@ -0,0 +1,96 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ +/* + * Copyright (C) 2023 Renesas Electronics Corp. + */ +#ifndef __DT_BINDINGS_CLOCK_R8A779H0_CPG_MSSR_H__ +#define __DT_BINDINGS_CLOCK_R8A779H0_CPG_MSSR_H__ + +#include <dt-bindings/clock/renesas-cpg-mssr.h> + +/* r8a779h0 CPG Core Clocks */ + +#define R8A779H0_CLK_ZX 0 +#define R8A779H0_CLK_ZD 1 +#define R8A779H0_CLK_ZS 2 +#define R8A779H0_CLK_ZT 3 +#define R8A779H0_CLK_ZTR 4 +#define R8A779H0_CLK_S0D2 5 +#define R8A779H0_CLK_S0D3 6 +#define R8A779H0_CLK_S0D4 7 +#define R8A779H0_CLK_S0D1_VIO 8 +#define R8A779H0_CLK_S0D2_VIO 9 +#define R8A779H0_CLK_S0D4_VIO 10 +#define R8A779H0_CLK_S0D8_VIO 11 +#define R8A779H0_CLK_VIOBUSD1 12 +#define R8A779H0_CLK_VIOBUSD2 13 +#define R8A779H0_CLK_S0D1_VC 14 +#define R8A779H0_CLK_S0D2_VC 15 +#define R8A779H0_CLK_S0D4_VC 16 +#define R8A779H0_CLK_VCBUSD1 17 +#define R8A779H0_CLK_VCBUSD2 18 +#define R8A779H0_CLK_S0D2_MM 19 +#define R8A779H0_CLK_S0D4_MM 20 +#define R8A779H0_CLK_S0D2_U3DG 21 +#define R8A779H0_CLK_S0D4_U3DG 22 +#define R8A779H0_CLK_S0D2_RT 23 +#define R8A779H0_CLK_S0D3_RT 24 +#define R8A779H0_CLK_S0D4_RT 25 +#define R8A779H0_CLK_S0D6_RT 26 +#define R8A779H0_CLK_S0D2_PER 27 +#define R8A779H0_CLK_S0D3_PER 28 +#define R8A779H0_CLK_S0D4_PER 29 +#define R8A779H0_CLK_S0D6_PER 30 +#define R8A779H0_CLK_S0D12_PER 31 +#define R8A779H0_CLK_S0D24_PER 32 +#define R8A779H0_CLK_S0D1_HSC 33 +#define R8A779H0_CLK_S0D2_HSC 34 +#define R8A779H0_CLK_S0D4_HSC 35 +#define R8A779H0_CLK_S0D8_HSC 36 +#define R8A779H0_CLK_SVD1_IR 37 +#define R8A779H0_CLK_SVD2_IR 38 +#define R8A779H0_CLK_IMPAD1 39 +#define R8A779H0_CLK_IMPAD4 40 +#define R8A779H0_CLK_IMPB 41 +#define R8A779H0_CLK_SVD1_VIP 42 +#define R8A779H0_CLK_SVD2_VIP 43 +#define R8A779H0_CLK_CL 44 +#define R8A779H0_CLK_CL16M 45 +#define R8A779H0_CLK_CL16M_MM 46 +#define R8A779H0_CLK_CL16M_RT 47 +#define R8A779H0_CLK_CL16M_PER 48 +#define R8A779H0_CLK_CL16M_HSC 49 +#define R8A779H0_CLK_ZC0 50 +#define R8A779H0_CLK_ZC1 51 +#define R8A779H0_CLK_ZC2 52 +#define R8A779H0_CLK_ZC3 53 +#define R8A779H0_CLK_ZB3 54 +#define R8A779H0_CLK_ZB3D2 55 +#define R8A779H0_CLK_ZB3D4 56 +#define R8A779H0_CLK_ZG 57 +#define R8A779H0_CLK_SD0H 58 +#define R8A779H0_CLK_SD0 59 +#define R8A779H0_CLK_RPC 60 +#define R8A779H0_CLK_RPCD2 61 +#define R8A779H0_CLK_MSO 62 +#define R8A779H0_CLK_CANFD 63 +#define R8A779H0_CLK_CSI 64 +#define R8A779H0_CLK_FRAY 65 +#define R8A779H0_CLK_IPC 66 +#define R8A779H0_CLK_SASYNCRT 67 +#define R8A779H0_CLK_SASYNCPERD1 68 +#define R8A779H0_CLK_SASYNCPERD2 69 +#define R8A779H0_CLK_SASYNCPERD4 70 +#define R8A779H0_CLK_DSIEXT 71 +#define R8A779H0_CLK_DSIREF 72 +#define R8A779H0_CLK_ADGH 73 +#define R8A779H0_CLK_OSC 74 +#define R8A779H0_CLK_ZR0 75 +#define R8A779H0_CLK_ZR1 76 +#define R8A779H0_CLK_ZR2 77 +#define R8A779H0_CLK_RGMII 78 +#define R8A779H0_CLK_CPEX 79 +#define R8A779H0_CLK_CP 80 +#define R8A779H0_CLK_CBFUSA 81 +#define R8A779H0_CLK_R 82 + +#endif /* __DT_BINDINGS_CLOCK_R8A779H0_CPG_MSSR_H__ */