Hi Geert-san, Thank you for your comments! > From: Geert Uytterhoeven, Sent: Monday, December 6, 2021 9:43 PM > > Hi Shimoda-san, > > On Wed, Dec 1, 2021 at 8:33 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > > According to the official website [1], the R-Car V3U SoC is based > > on the R-Car Gen4 architecture. So, introduce R-Car Gen4 CPG > > driver. > > > > [1] > > > https://www.renesas.com/us/en/products/automotive-products/automotive-system-chips-socs/r-car-v3u-best-class-r-car-v > 3u-asil-d-system-chip-automated-driving > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > Thanks for your patch! > > > --- /dev/null > > +++ b/drivers/clk/renesas/rcar-gen4-cpg.c > > > +/* > > + * RPC Clocks > > + */ > > +#define CPG_RPCCKCR 0x874 > > This is also defined in rcar-gen4-cpg.h, so I will drop it while applying. Thanks! > > + > > > --- /dev/null > > +++ b/drivers/clk/renesas/rcar-gen4-cpg.h > > @@ -0,0 +1,76 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * R-Car Gen4 Clock Pulse Generator > > + * > > + * Copyright (C) 2021 Renesas Electronics Corp. > > + * > > + */ > > + > > +#ifndef __CLK_RENESAS_RCAR_GEN4_CPG_H__ > > +#define __CLK_RENESAS_RCAR_GEN4_CPG_H__ > > + > > +enum rcar_gen4_clk_types { > > + CLK_TYPE_GEN4_MAIN = CLK_TYPE_CUSTOM, > > + CLK_TYPE_GEN4_PLL1, > > + CLK_TYPE_GEN4_PLL2, > > + CLK_TYPE_GEN4_PLL2X_3X, /* r8a779a0 only */ > > + CLK_TYPE_GEN4_PLL3, > > + CLK_TYPE_GEN4_PLL5, > > + CLK_TYPE_GEN4_PLL6, > > + CLK_TYPE_GEN4_SDSRC, > > + CLK_TYPE_GEN4_SDH, > > + CLK_TYPE_GEN4_SD, > > + CLK_TYPE_GEN4_MDSEL, /* Select parent/divider using mode pin */ > > + CLK_TYPE_GEN4_Z, > > + CLK_TYPE_GEN4_OSC, /* OSC EXTAL predivider and fixed divider */ > > + CLK_TYPE_GEN4_RPCSRC, > > + CLK_TYPE_GEN4_RPC, > > + CLK_TYPE_GEN4_RPCD2, > > + > > + /* SoC specific definitions start here */ > > + CLK_TYPE_GEN4_SOC_BASE, > > +}; > > + > > +#define DEF_GEN4_SDH(_name, _id, _parent, _offset) \ > > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_SDH, _parent, .offset = _offset) > > + > > +#define DEF_GEN4_SD(_name, _id, _parent, _offset) \ > > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_SD, _parent, .offset = _offset) > > + > > +#define DEF_GEN4_MDSEL(_name, _id, _md, _parent0, _div0, _parent1, _div1) \ > > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_MDSEL, \ > > + (_parent0) << 16 | (_parent1), \ > > + .div = (_div0) << 16 | (_div1), .offset = _md) > > + > > +#define DEF_GEN4_OSC(_name, _id, _parent, _div) \ > > + DEF_BASE(_name, _id, CLK_TYPE_GEN4_OSC, _parent, .div = _div) > > + > > +#define DEF_GEN4_Z(_name, _id, _type, _parent, _div, _offset) \ > > + DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = _offset) > > Is there any specific reason _type is not fixed to CLK_TYPE_GEN4_Z, > like before? Perhaps you have a future use-case in mind? This is a similar definition with DEF_GEN3_Z. And, there is not upstream use-case though, if we support ZG clock, we have to use _type for ZG like Gen3 BSP. In Gen4, we will support ZG clock on other SoCs in the future, the _type is not fixed. > > + > > +struct rcar_gen4_cpg_pll_config { > > + u8 extal_div; > > + u8 pll1_mult; > > + u8 pll1_div; > > + u8 pll2_mult; > > + u8 pll2_div; > > + u8 pll3_mult; > > + u8 pll3_div; > > + u8 pll5_mult; > > + u8 pll5_div; > > + u8 pll6_mult; > > + u8 pll6_div; > > + u8 osc_prediv; > > +}; > > + > > +#define CPG_RPCCKCR 0x874 > > +#define SD0CKCR1 0x8a4 > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > i.e. will queue in renesas-clk-for-v5.17 when the above has been sorted > out. Thank you very much for your review! Best regards, Yoshihiro Shimoda