Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1609370 > unrolled thread

[PATCH v2 0/6] ARM: sunxi: Convert sun4i/sun7i series SoCs to sunxi-ng

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/6] ARM: sunxi: Convert sun4i/sun7i series SoCs to sunxi-ng Priit Laes <plaes@plaes.org> - 2017-03-26 19:40 +0200
    [PATCH v2 4/6] dt-bindings: List devicetree binding for the CCU of Allwinner A20 Priit Laes <plaes@plaes.org> - 2017-03-26 20:00 +0200
    [PATCH v2 6/6] clk: sunxi-ng: Display index when clock registration fails Priit Laes <plaes@plaes.org> - 2017-03-26 20:10 +0200
    [PATCH v2 5/6] dt-bindings: List devicetree binding for the CCU of Allwinner A10 Priit Laes <plaes@plaes.org> - 2017-03-26 20:40 +0200
    Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-03-27 11:00 +0200

#1609370 — [PATCH v2 0/6] ARM: sunxi: Convert sun4i/sun7i series SoCs to sunxi-ng

Hi,

This serie brings A10 (sun4i) and A20 (sun7i) SoCs into the
sunxi-ng world.

As mentioned in sun5i conversion, this is pretty much standard
stuff as all the required clocks were already implemented in
the sunxi-ng framework.

Unfortunately there's an issue with LVDS reset control that
causes issues with LVDS displays unless 'clk_ignore_unused'
option is used.

Need help with that :(

Changes from v1:
 - Drop useless comments
 - Add support for A10 / sun4i.
 - Rename driver to sunxi-a10-a20.
 - Add previously unimplemented clocks.
 - Document the audio pll hardcoded post-divider
 - Add Acked-by: Rob Herring <robh at kernel.org> on patch 4

Priit Laes (6):
  clk: sunxi-ng: Add sun4i/sun7i CCU driver
  ARM: sun7i: Convert to CCU
  ARM: sun4i: Convert to CCU
  dt-bindings: List devicetree binding for the CCU of Allwinner A20
  dt-bindings: List devicetree binding for the CCU of Allwinner A10
  clk: sunxi-ng: Display index when clock registration fails

 Documentation/devicetree/bindings/clock/sunxi-ccu.txt |    2 +-
 arch/arm/boot/dts/sun4i-a10.dtsi                      |  636 +----
 arch/arm/boot/dts/sun7i-a20.dtsi                      |  717 +-----
 drivers/clk/sunxi-ng/Kconfig                          |   13 +-
 drivers/clk/sunxi-ng/Makefile                         |    1 +-
 drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c              | 1532 ++++++++++-
 drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h              |   59 +-
 drivers/clk/sunxi-ng/ccu_common.c                     |    4 +-
 include/dt-bindings/clock/sunxi-a10-a20-ccu.h         |  208 +-
 include/dt-bindings/reset/sunxi-a10-a20-ccu.h         |   66 +-
 10 files changed, 2050 insertions(+), 1188 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c
 create mode 100644 drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h
 create mode 100644 include/dt-bindings/clock/sunxi-a10-a20-ccu.h
 create mode 100644 include/dt-bindings/reset/sunxi-a10-a20-ccu.h

base-commit: 8e19fb843be8934d48b31fafbb32a4176f7feb65
-- 
git-series 0.9.1

[toc] | [next] | [standalone]

#1609372 — [PATCH v2 4/6] dt-bindings: List devicetree binding for the CCU of Allwinner A20

Allwinner A20 is now driven by sunxi-ng CCU driver.

Add devicetree binding for it.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Priit Laes <plaes@plaes.org>
---
 Documentation/devicetree/bindings/clock/sunxi-ccu.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
index 68512aa..de90988 100644
--- a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
@@ -4,6 +4,7 @@ Allwinner Clock Control Unit Binding
 Required properties :
 - compatible: must contain one of the following compatibles:
 		- "allwinner,sun6i-a31-ccu"
+		- "allwinner,sun7i-a20-ccu"
 		- "allwinner,sun8i-a23-ccu"
 		- "allwinner,sun8i-a33-ccu"
 		- "allwinner,sun8i-h3-ccu"
-- 
git-series 0.9.1

[toc] | [prev] | [next] | [standalone]

#1609374 — [PATCH v2 6/6] clk: sunxi-ng: Display index when clock registration fails

Add clock index to clock registration failure message.

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 drivers/clk/sunxi-ng/ccu_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c
index 8a47baf..188fa50 100644
--- a/drivers/clk/sunxi-ng/ccu_common.c
+++ b/drivers/clk/sunxi-ng/ccu_common.c
@@ -63,8 +63,8 @@ int sunxi_ccu_probe(struct device_node *node, void __iomem *reg,
 
 		ret = clk_hw_register(NULL, hw);
 		if (ret) {
-			pr_err("Couldn't register clock %s\n",
-			       clk_hw_get_name(hw));
+			pr_err("Couldn't register clock %d - %s\n",
+			       i, clk_hw_get_name(hw));
 			goto err_clk_unreg;
 		}
 	}
-- 
git-series 0.9.1

[toc] | [prev] | [next] | [standalone]

#1609380 — [PATCH v2 5/6] dt-bindings: List devicetree binding for the CCU of Allwinner A10

Allwinner A10 is now driven by sunxi-ng CCU driver.

Add devicetree binding for it.

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 Documentation/devicetree/bindings/clock/sunxi-ccu.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
index de90988..db49cbc 100644
--- a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
@@ -3,6 +3,7 @@ Allwinner Clock Control Unit Binding
 
 Required properties :
 - compatible: must contain one of the following compatibles:
+		- "allwinner,sun4i-a10-ccu"
 		- "allwinner,sun6i-a31-ccu"
 		- "allwinner,sun7i-a20-ccu"
 		- "allwinner,sun8i-a23-ccu"
-- 
git-series 0.9.1

[toc] | [prev] | [next] | [standalone]

#1609588 — Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver

[Multipart message — attachments visible in raw view] — view raw

Hi,

Thanks a lot for working on this.

On Sun, Mar 26, 2017 at 08:20:16PM +0300, Priit Laes wrote:
> Introduce a clock controller driver for sun4i A10 and sun7i A20
> series SoCs.
> 
> Signed-off-by: Priit Laes <plaes@plaes.org>
> ---
>  drivers/clk/sunxi-ng/Kconfig                  |   13 +-
>  drivers/clk/sunxi-ng/Makefile                 |    1 +-
>  drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c      | 1532 ++++++++++++++++++-
>  drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h      |   59 +-
>  include/dt-bindings/clock/sunxi-a10-a20-ccu.h |  208 ++-
>  include/dt-bindings/reset/sunxi-a10-a20-ccu.h |   66 +-

I'm not too fond of those sunxi-<all the SoCs supported>. We're not
doing that for any other driver, I don't really know why this has
became a trend lately.

You can call them ccu-sun4i-a10.h, and it will work just fine.

> +/* Not documented on A10 */
> +static SUNXI_CCU_GATE(pll_periph_sata_clk, "pll-periph-sata", "pll-periph",
> +		      0x028, BIT(14), 0);

The rate doesn't come from pll-periph directly, does it?

> +#define SUN4I_AHB_REG		0x054
> +static struct ccu_mux cpu_clk = {
> +	.mux		= {
> +		.shift		= 16,
> +		.width		= 2,
> +		.fixed_predivs	= cpu_predivs,
> +		.n_predivs	= ARRAY_SIZE(cpu_predivs),
> +	},
> +	.common		= {
> +		.reg		= 0x054,

Why did you define this one, even though you don't seem to be using it
anywhere?

> +static const char *const ahb_parents[] = { "axi", "pll-periph",
> +					   "pll-periph-2x" };
> +static const struct ccu_mux_fixed_prediv ahb_predivs[] = {
> +	{ .index = 2, .div = 2, },
> +};

This seems to be only true for the A20, and not the A10.

Are you sure here? The pll-periph-2x seem to be only used in the MBUS
clock in our current code.

And then, using pll-periph-2x, and then dividing it by 2 just gives us
pll-periph, which is also our previous parent :)

> +/* Undocumented on A10 */
> +static SUNXI_CCU_PHASE(mmc0_output_clk, "mmc0_output", "mmc0",
> +		       0x088, 8, 3, 0);
> +/* Undocumented on A10 */
> +static SUNXI_CCU_PHASE(mmc0_sample_clk, "mmc0_sample", "mmc0",
> +		       0x088, 20, 3, 0);

The A10 doesn't have them.

> +/* TODO: Check whether A10 actually supports osc32k as 4th parent? */
> +static const char *const ir_parents_sun4i[] = { "hosc", "pll-periph",
> +						"pll-ddr-other" };

What does the BSP say about this?

> +/* Undocumented on A10 */
> +static SUNXI_CCU_MUX_WITH_GATE(spdif_clk, "spdif", audio_parents,
> +			       0x0c0, 16, 2, BIT(31), CLK_SET_RATE_PARENT);

This doesn't seem to exist at all on the A10

> +/*
> + * TODO: SATA clock also supports external clock as parent via BIT(24)
> + * The external clock is probably an optional crystal or oscillator
> + * that can be connected to the SATA-CLKM / SATA-CLKP pins.
> + */
> +static SUNXI_CCU_GATE(sata_clk, "sata", "pll-periph-sata",
> +		      0x0c8, BIT(31), 0);

The rate won't be good here either. This is supposed to be 100MHz.

> +static const char *const csi_isp_parents[] = { "pll-video0", "pll-ve",
> +					       "pll-ddr-other", "pll-sata" };
> +
> +static SUNXI_CCU_M_WITH_MUX_GATE(csi_isp_clk, "csi-isp",
> +				 csi_isp_parents,
> +				 0x120, 0, 4, 24, 2, BIT(31), 0);

We've been calling it sclk in the other SoC iirc. Any particular
reason to call it differently?

> +static const char *const out_parents[] = { "hosc", "osc32k", "hosc" };
> +static SUNXI_CCU_MP_WITH_MUX_GATE(out_a_clk, "out-a", out_parents,
> +				  0x1f0, 8, 5, 20, 2, 24, 2, BIT(31), 0);
> +static SUNXI_CCU_MP_WITH_MUX_GATE(out_b_clk, "out-b", out_parents,
> +				  0x1f4, 8, 5, 20, 2, 24, 2, BIT(31), 0);

There's a fixed pre-divider on the first hosc of 750.

> +static void init_clocks(void __iomem *reg)
> +{
> +	u32 val;
> +
> +	/* Force the PLL-Audio-1x divider to 4 */
> +	val = readl(reg + SUN4I_PLL_AUDIO_REG);
> +	val &= ~GENMASK(19, 16);
> +	writel(val | (3 << 16), reg + SUN4I_PLL_AUDIO_REG);
> +
> +	/* Use PLL6 as parent for AHB */
> +	val = readl(reg + SUN4I_AHB_REG);
> +	val &= ~GENMASK(7, 6);
> +	writel(val | (2 << 6), reg + SUN4I_AHB_REG);

Keeping some kind of comment similar to what was in the DT would be
great, otherwise we lose *why* we need to do so.

> +}
> +
> +static void __init sun4i_a10_ccu_setup(struct device_node *node)
> +{
> +	void __iomem *reg;
> +
> +	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> +	if (IS_ERR(reg)) {
> +		pr_err("%s: Could not map the clock registers\n",
> +		       of_node_full_name(node));
> +		return;
> +	}
> +
> +	init_clocks(reg);
> +
> +	sunxi_ccu_probe(node, reg, &sun4i_a10_ccu_desc);

Can't you move the request_and_map / probe in the common function?

> +#ifndef _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> +#define _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> +
> +#define CLK_HOSC		1
> +#define CLK_PLL_PERIPH_SATA	16

That one looks suspicious. I don't see why we would need the PLL,
while we have a perfectly functional SATA clock below. Have you tried
gating the bit31 of the register 0xc8 to see if it has any impact?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web