[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250228135058.47f03a66@donnerap.manchester.arm.com>
Date: Fri, 28 Feb 2025 13:50:58 +0000
From: Andre Przywara <andre.przywara@....com>
To: Jernej Škrabec <jernej.skrabec@...il.com>
Cc: Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Chen-Yu Tsai
<wens@...e.org>, Samuel Holland <samuel@...lland.org>, Philipp Zabel
<p.zabel@...gutronix.de>, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 11/15] clk: sunxi-ng: a523: remaining mod clocks
On Tue, 18 Feb 2025 20:55:34 +0100
Jernej Škrabec <jernej.skrabec@...il.com> wrote:
Hi,
> Dne petek, 14. februar 2025 ob 13:53:55 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> > Add the remaining mod clocks, driving various parts of the SoC: the "LEDC"
> > LED controller, the "CSI" camera interface, the "ISP" image processor,
> > the DSP clock, and the "fanout" clocks, which allow to put clock signals
> > on external pins.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@....com>
> > ---
> > drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 190 +++++++++++++++++++++++++
> > 1 file changed, 190 insertions(+)
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > index 59cc31f89ae77..6a4340f1fd041 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > @@ -354,6 +354,7 @@ static SUNXI_CCU_M_DATA_WITH_MUX(apb0_clk, "apb0", ahb_apb0_parents, 0x520,
> > 0, 5, /* M */
> > 24, 2, /* mux */
> > 0);
> > +static const struct clk_hw *apb0_hws[] = { &apb0_clk.common.hw };
> >
> > static const struct clk_parent_data apb1_parents[] = {
> > { .fw_name = "hosc" },
> > @@ -832,6 +833,153 @@ static SUNXI_CCU_M_HW_WITH_MUX_GATE(edp_clk, "edp", edp_parents, 0xbb0,
> > BIT(31), /* gate */
> > 0);
> >
> > +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(ledc_clk, "ledc", ir_tx_ledc_parents,
> > + 0xbf0,
> > + 0, 4, /* M */
> > + 24, 1, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static const struct clk_hw *csi_top_parents[] = {
> > + &pll_periph0_300M_clk.hw,
> > + &pll_periph0_400M_clk.hw,
> > + &pll_periph0_480M_clk.common.hw,
> > + &pll_video3_4x_clk.common.hw,
> > + &pll_video3_3x_clk.hw,
> > +};
> > +static SUNXI_CCU_M_HW_WITH_MUX_GATE(csi_top_clk, "csi-top", csi_top_parents,
> > + 0xc04,
> > + 0, 5, /* M */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static const struct clk_parent_data csi_mclk_parents[] = {
> > + { .fw_name = "hosc" },
> > + { .hw = &pll_video3_4x_clk.common.hw },
> > + { .hw = &pll_video0_4x_clk.common.hw },
> > + { .hw = &pll_video1_4x_clk.common.hw },
> > + { .hw = &pll_video2_4x_clk.common.hw },
> > +};
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(csi_mclk0_clk, "csi-mclk0",
> > + csi_mclk_parents, 0xc08,
> > + 0, 5, /* M */
> > + 8, 5, /* P */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + 0);
>
> Missing dual div flag for csi-mclk clocks.
Nice one, another case for the new macro ...
> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(csi_mclk1_clk, "csi-mclk1",
> > + csi_mclk_parents, 0xc0c,
> > + 0, 5, /* M */
> > + 8, 5, /* P */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(csi_mclk2_clk, "csi-mclk2",
> > + csi_mclk_parents, 0xc10,
> > + 0, 5, /* M */
> > + 8, 5, /* P */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(csi_mclk3_clk, "csi-mclk3",
> > + csi_mclk_parents, 0xc14,
> > + 0, 5, /* M */
> > + 8, 5, /* P */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static const struct clk_hw *isp_parents[] = {
> > + &pll_periph0_300M_clk.hw,
> > + &pll_periph0_400M_clk.hw,
> > + &pll_video2_4x_clk.common.hw,
> > + &pll_video3_4x_clk.common.hw,
> > +};
> > +static SUNXI_CCU_M_HW_WITH_MUX_GATE(isp_clk, "isp", isp_parents, 0xc20,
> > + 0, 5, /* M */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static const struct clk_parent_data dsp_parents[] = {
> > + { .fw_name = "hosc" },
> > + { .fw_name = "losc" },
> > + { .fw_name = "iosc" },
> > + { .hw = &pll_periph0_2x_clk.common.hw },
> > + { .hw = &pll_periph0_400M_clk.hw, },
>
> Last one should be pll_periph0_480M_clk.
Right, spotting the difference between an 8 and a 0 earns you some extra
points! Fixed.
>
> > +};
> > +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(dsp_clk, "dsp", dsp_parents, 0xc70,
> > + 0, 5, /* M */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static SUNXI_CCU_GATE_DATA(fanout_24M_clk, "fanout-24M", osc24M,
> > + 0xf30, BIT(0), 0);
> > +static SUNXI_CCU_GATE_DATA_WITH_PREDIV(fanout_12M_clk, "fanout-12M", osc24M,
> > + 0xf30, BIT(1), 2, 0);
> > +static SUNXI_CCU_GATE_HWS_WITH_PREDIV(fanout_16M_clk, "fanout-16M",
> > + pll_periph0_480M_hws,
> > + 0xf30, BIT(2), 30, 0);
> > +static SUNXI_CCU_GATE_HWS_WITH_PREDIV(fanout_25M_clk, "fanout-25M",
> > + pll_periph0_2x_hws,
> > + 0xf30, BIT(3), 48, 0);
> > +static SUNXI_CCU_GATE_HWS_WITH_PREDIV(fanout_50M_clk, "fanout-50M",
> > + pll_periph0_2x_hws,
> > + 0xf30, BIT(4), 24, 0);
> > +
> > +/* These clocks have a second divider that is not modelled and forced to 0. */
>
> Any specific reason for that?
Laziness? Being annoyed about Allwinner doing those random changes without
good reason? I sighed audibly, then added a new macro that uses the dual
divider feature. This allows us to get rid of the initialisation of those
registers below.
Thanks for poking into this ;-)
Cheers,
Andre
> Best regards,
> Jernej
>
> > +#define SUN55I_A523_FANOUT_27M_REG 0xf34
> > +static const struct clk_hw *fanout_27M_parents[] = {
> > + &pll_video0_4x_clk.common.hw,
> > + &pll_video1_4x_clk.common.hw,
> > + &pll_video2_4x_clk.common.hw,
> > + &pll_video3_4x_clk.common.hw,
> > +};
> > +static SUNXI_CCU_M_HW_WITH_MUX_GATE(fanout_27M_clk, "fanout-27M",
> > + fanout_27M_parents, 0xf34,
> > + 0, 5, /* M */
> > + 24, 2, /* mux */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +#define SUN55I_A523_FANOUT_PCLK_REG 0xf38
> > +static SUNXI_CCU_M_HWS_WITH_GATE(fanout_pclk_clk, "fanout-pclk", apb0_hws,
> > + 0xf38,
> > + 0, 5, /* M */
> > + BIT(31), /* gate */
> > + 0);
> > +
> > +static const struct clk_parent_data fanout_parents[] = {
> > + { .fw_name = "osc32k-out" },
> > + { .hw = &fanout_12M_clk.common.hw, },
> > + { .hw = &fanout_16M_clk.common.hw, },
> > + { .hw = &fanout_24M_clk.common.hw, },
> > + { .hw = &fanout_25M_clk.common.hw, },
> > + { .hw = &fanout_27M_clk.common.hw, },
> > + { .hw = &fanout_pclk_clk.common.hw, },
> > + { .hw = &fanout_50M_clk.common.hw, },
> > +};
> > +static SUNXI_CCU_MUX_DATA_WITH_GATE(fanout0_clk, "fanout0", fanout_parents,
> > + 0xf3c,
> > + 0, 3, /* mux */
> > + BIT(21), /* gate */
> > + 0);
> > +static SUNXI_CCU_MUX_DATA_WITH_GATE(fanout1_clk, "fanout1", fanout_parents,
> > + 0xf3c,
> > + 3, 3, /* mux */
> > + BIT(22), /* gate */
> > + 0);
> > +static SUNXI_CCU_MUX_DATA_WITH_GATE(fanout2_clk, "fanout2", fanout_parents,
> > + 0xf3c,
> > + 6, 3, /* mux */
> > + BIT(23), /* gate */
> > + 0);
> > +
> > /*
> > * Contains all clocks that are controlled by a hardware register. They
> > * have a (sunxi) .common member, which needs to be initialised by the common
> > @@ -904,6 +1052,23 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
> > &tcon_tv0_clk.common,
> > &tcon_tv1_clk.common,
> > &edp_clk.common,
> > + &ledc_clk.common,
> > + &csi_top_clk.common,
> > + &csi_mclk0_clk.common,
> > + &csi_mclk1_clk.common,
> > + &csi_mclk2_clk.common,
> > + &csi_mclk3_clk.common,
> > + &isp_clk.common,
> > + &dsp_clk.common,
> > + &fanout_24M_clk.common,
> > + &fanout_12M_clk.common,
> > + &fanout_16M_clk.common,
> > + &fanout_25M_clk.common,
> > + &fanout_27M_clk.common,
> > + &fanout_pclk_clk.common,
> > + &fanout0_clk.common,
> > + &fanout1_clk.common,
> > + &fanout2_clk.common,
> > };
> >
> > static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> > @@ -997,6 +1162,23 @@ static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> > [CLK_TCON_TV0] = &tcon_tv0_clk.common.hw,
> > [CLK_TCON_TV1] = &tcon_tv1_clk.common.hw,
> > [CLK_EDP] = &edp_clk.common.hw,
> > + [CLK_LEDC] = &ledc_clk.common.hw,
> > + [CLK_CSI_TOP] = &csi_top_clk.common.hw,
> > + [CLK_CSI_MCLK0] = &csi_mclk0_clk.common.hw,
> > + [CLK_CSI_MCLK1] = &csi_mclk1_clk.common.hw,
> > + [CLK_CSI_MCLK2] = &csi_mclk2_clk.common.hw,
> > + [CLK_CSI_MCLK3] = &csi_mclk3_clk.common.hw,
> > + [CLK_ISP] = &isp_clk.common.hw,
> > + [CLK_DSP] = &dsp_clk.common.hw,
> > + [CLK_FANOUT_24M] = &fanout_24M_clk.common.hw,
> > + [CLK_FANOUT_12M] = &fanout_12M_clk.common.hw,
> > + [CLK_FANOUT_16M] = &fanout_16M_clk.common.hw,
> > + [CLK_FANOUT_25M] = &fanout_25M_clk.common.hw,
> > + [CLK_FANOUT_27M] = &fanout_27M_clk.common.hw,
> > + [CLK_FANOUT_PCLK] = &fanout_pclk_clk.common.hw,
> > + [CLK_FANOUT0] = &fanout0_clk.common.hw,
> > + [CLK_FANOUT1] = &fanout1_clk.common.hw,
> > + [CLK_FANOUT2] = &fanout2_clk.common.hw,
> > },
> > };
> >
> > @@ -1048,6 +1230,14 @@ static int sun55i_a523_ccu_probe(struct platform_device *pdev)
> > val &= ~(BIT(1) | BIT(0));
> > writel(val, reg + SUN55I_A523_PLL_AUDIO0_REG);
> >
> > + /* Force fanout factors N to 0. */
> > + val = readl(reg + SUN55I_A523_FANOUT_27M_REG);
> > + val &= ~GENMASK(12, 8);
> > + writel(val, reg + SUN55I_A523_FANOUT_27M_REG);
> > + val = readl(reg + SUN55I_A523_FANOUT_PCLK_REG);
> > + val &= ~GENMASK(9, 5);
> > + writel(val, reg + SUN55I_A523_FANOUT_PCLK_REG);
> > +
> > ret = devm_sunxi_ccu_probe(&pdev->dev, reg, &sun55i_a523_ccu_desc);
> > if (ret)
> > return ret;
> >
>
>
>
>
Powered by blists - more mailing lists