Re: [PATCH v2 08/15] clk: sunxi-ng: a523: add system mod clocks

[Andre Przywara <andre.przywara@arm.com>]

On Tue, 18 Feb 2025 20:34:27 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

Hi,

> Dne petek, 14. februar 2025 ob 13:53:52 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> > Add the clocks driving some core system related subsystems of the SoC:
> > the "CE" crypto engine, the high speed timers, the DRAM and the associated
> > MBUS clock, and the PCIe clock.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 119 +++++++++++++++++++++++++
> >  1 file changed, 119 insertions(+)
> > 
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > index 0ef1fd71a1ca5..b68c44bce825f 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > @@ -423,6 +423,18 @@ static SUNXI_CCU_M_HW_WITH_MUX_GATE(gpu_clk, "gpu", gpu_parents, 0x670,
> >  				    BIT(31),	/* gate */
> >  				    0);
> >  
> > +static const struct clk_parent_data ce_parents[] = {
> > +	{ .fw_name = "hosc" },
> > +	{ .hw = &pll_periph0_480M_clk.common.hw },
> > +	{ .hw = &pll_periph0_400M_clk.hw },
> > +	{ .hw = &pll_periph0_300M_clk.hw },
> > +};
> > +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(ce_clk, "ce", ce_parents, 0x680,
> > +				       0, 5,	/* M */
> > +				       24, 3,	/* mux */
> > +				       BIT(31),	/* gate */
> > +				       0);
> > +
> >  static const struct clk_hw *ve_parents[] = {
> >  	&pll_ve_clk.common.hw,
> >  	&pll_periph0_480M_clk.common.hw,
> > @@ -435,6 +447,65 @@ static SUNXI_CCU_M_HW_WITH_MUX_GATE(ve_clk, "ve", ve_parents, 0x690,
> >  				    BIT(31),	/* gate */
> >  				    CLK_SET_RATE_PARENT);
> >  
> > +static const struct clk_parent_data hstimer_parents[] = {
> > +	{ .fw_name = "hosc" },
> > +	{ .fw_name = "iosc" },
> > +	{ .fw_name = "losc" },
> > +	{ .hw = &pll_periph0_200M_clk.hw },
> > +};
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(hstimer0_clk, "hstimer0",
> > +				       hstimer_parents, 0x730,
> > +				       0, 0,	/* M */
> > +				       0, 3,	/* P */
> > +				       24, 3,	/* mux */
> > +				       BIT(31),	/* gate */
> > +				       CLK_SET_RATE_PARENT);  
> 
> CLK_SET_RATE_PARENT doesn't make much sense for fixed clocks.

That's a very good point ;-)
Removed.

> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(hstimer1_clk, "hstimer1",
> > +				       hstimer_parents,
> > +				       0x734,
> > +				       0, 0,	/* M */
> > +				       0, 3,	/* P */
> > +				       24, 3,	/* mux */
> > +				       BIT(31),	/* gate */
> > +				       CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(hstimer2_clk, "hstimer2",
> > +				       hstimer_parents,
> > +				       0x738,
> > +				       0, 0,	/* M */
> > +				       0, 3,	/* P */
> > +				       24, 3,	/* mux */
> > +				       BIT(31),	/* gate */
> > +				       CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(hstimer3_clk, "hstimer3",
> > +				       hstimer_parents,
> > +				       0x73c,
> > +				       0, 0,	/* M */
> > +				       0, 3,	/* P */
> > +				       24, 3,	/* mux */
> > +				       BIT(31),	/* gate */
> > +				       CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(hstimer4_clk, "hstimer4",
> > +				       hstimer_parents,
> > +				       0x740,
> > +				       0, 0,	/* M */
> > +				       0, 3,	/* P */
> > +				       24, 3,	/* mux */
> > +				       BIT(31),	/* gate */
> > +				       CLK_SET_RATE_PARENT);
> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(hstimer5_clk, "hstimer5",
> > +				       hstimer_parents,
> > +				       0x744,
> > +				       0, 0,	/* M */
> > +				       0, 3,	/* P */
> > +				       24, 3,	/* mux */
> > +				       BIT(31),	/* gate */
> > +				       CLK_SET_RATE_PARENT);
> > +
> >  static const struct clk_parent_data iommu_parents[] = {
> >  	{ .hw = &pll_periph0_600M_clk.hw },
> >  	{ .hw = &pll_ddr0_clk.common.hw },
> > @@ -450,6 +521,34 @@ static SUNXI_CCU_M_DATA_WITH_MUX_GATE(iommu_clk, "iommu", iommu_parents, 0x7b0,
> >  				      BIT(31),	/* gate */
> >  				      CLK_SET_RATE_PARENT);
> >  
> > +static const struct clk_hw *dram_parents[] = {
> > +	&pll_ddr0_clk.common.hw,
> > +	&pll_periph0_600M_clk.hw,
> > +	&pll_periph0_480M_clk.common.hw,
> > +	&pll_periph0_400M_clk.hw,
> > +	&pll_periph0_150M_clk.hw,
> > +};
> > +static SUNXI_CCU_M_HW_WITH_MUX_GATE(dram_clk, "dram", dram_parents, 0x800,
> > +				    0, 5,	/* M */
> > +				    24, 3,	/* mux */
> > +				    BIT(31),	/* gate */
> > +				    CLK_IS_CRITICAL);  
> 
> Same comment as for IOMMU clock. Update bit is needed to actually apply configuration.

Fixed now.

Thanks!
Andre

> 
> Best regards,
> Jernej
> 
> > +
> > +static CLK_FIXED_FACTOR_HW(mbus_clk, "mbus",
> > +			   &dram_clk.common.hw, 4, 1, 0);
> > +
> > +static const struct clk_parent_data losc_hosc_parents[] = {
> > +	{ .fw_name = "hosc" },
> > +	{ .fw_name = "losc" },
> > +};
> > +
> > +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(pcie_aux_clk, "pcie-aux",
> > +				      losc_hosc_parents, 0xaa0,
> > +				      0, 5,	/* M */
> > +				      24, 1,	/* mux */
> > +				      BIT(31),	/* gate */
> > +				      0);
> > +
> >  static SUNXI_CCU_GATE_DATA(hdmi_24M_clk, "hdmi-24M", osc24M, 0xb04, BIT(31), 0);
> >  
> >  /* TODO: add mux between 32kOSC and PERIPH0/18750 */
> > @@ -584,8 +683,17 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
> >  	&di_clk.common,
> >  	&g2d_clk.common,
> >  	&gpu_clk.common,
> > +	&ce_clk.common,
> >  	&ve_clk.common,
> > +	&hstimer0_clk.common,
> > +	&hstimer1_clk.common,
> > +	&hstimer2_clk.common,
> > +	&hstimer3_clk.common,
> > +	&hstimer4_clk.common,
> > +	&hstimer5_clk.common,
> >  	&iommu_clk.common,
> > +	&dram_clk.common,
> > +	&pcie_aux_clk.common,
> >  	&hdmi_24M_clk.common,
> >  	&hdmi_cec_32k_clk.common,
> >  	&hdmi_cec_clk.common,
> > @@ -644,11 +752,22 @@ static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> >  		[CLK_AHB]		= &ahb_clk.common.hw,
> >  		[CLK_APB0]		= &apb0_clk.common.hw,
> >  		[CLK_APB1]		= &apb1_clk.common.hw,
> > +		[CLK_MBUS]		= &mbus_clk.hw,
> >  		[CLK_DE]		= &de_clk.common.hw,
> >  		[CLK_DI]		= &di_clk.common.hw,
> >  		[CLK_G2D]		= &g2d_clk.common.hw,
> >  		[CLK_GPU]		= &gpu_clk.common.hw,
> > +		[CLK_CE]		= &ce_clk.common.hw,
> >  		[CLK_VE]		= &ve_clk.common.hw,
> > +		[CLK_HSTIMER0]		= &hstimer0_clk.common.hw,
> > +		[CLK_HSTIMER1]		= &hstimer1_clk.common.hw,
> > +		[CLK_HSTIMER2]		= &hstimer2_clk.common.hw,
> > +		[CLK_HSTIMER3]		= &hstimer3_clk.common.hw,
> > +		[CLK_HSTIMER4]		= &hstimer4_clk.common.hw,
> > +		[CLK_HSTIMER5]		= &hstimer5_clk.common.hw,
> > +		[CLK_IOMMU]		= &iommu_clk.common.hw,
> > +		[CLK_DRAM]		= &dram_clk.common.hw,
> > +		[CLK_PCIE_AUX]		= &pcie_aux_clk.common.hw,
> >  		[CLK_HDMI_24M]		= &hdmi_24M_clk.common.hw,
> >  		[CLK_HDMI_CEC_32K]	= &hdmi_cec_32k_clk.common.hw,
> >  		[CLK_HDMI_CEC]		= &hdmi_cec_clk.common.hw,
> >   
> 
> 
> 
>