Re: [PATCH v2 06/10] clk: sunxi: Add H3 clocks support

Maxime Ripard
2015-05-17T19:00:48Z

[-- Attachment #1: Type: text/plain, Size: 6657 bytes --]

On Fri, May 15, 2015 at 06:38:56PM +0200, Jens Kuske wrote:
> The H3 clock control unit is similar to the those of other sun8i family
> members like the A23.
>
> It makes use of the new multiple parents option for the bus gates.
> Some of the gates use the new AHB2 clock as parent, whose clock source
> is muxable between AHB1 and PLL6/2. The documentation isn't totally clear
> about which devices belong to AHB2 now, especially USB EHIC/OHIC, so it
> is mostly based on Allwinner kernel source code.
>
> Signed-off-by: Jens Kuske <jenskuske@gmail.com>
> ---
> Documentation/devicetree/bindings/clock/sunxi.txt | 6 +++
> drivers/clk/sunxi/clk-sunxi.c | 50 ++++++++++++++++++++++-
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index 4fa11af..e367963 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -14,6 +14,7 @@ Required properties:
> "allwinner,sun4i-a10-pll5-clk" - for the PLL5 clock
> "allwinner,sun4i-a10-pll6-clk" - for the PLL6 clock
> "allwinner,sun6i-a31-pll6-clk" - for the PLL6 clock on A31
> + "allwinner,sun8i-h3-pll6-clk" - for the PLL6 clock on H3
> "allwinner,sun9i-a80-gt-clk" - for the GT bus clock on A80
> "allwinner,sun4i-a10-cpu-clk" - for the CPU multiplexer clock
> "allwinner,sun4i-a10-axi-clk" - for the AXI clock
> @@ -28,6 +29,7 @@ Required properties:
> "allwinner,sun7i-a20-ahb-gates-clk" - for the AHB gates on A20
> "allwinner,sun6i-a31-ar100-clk" - for the AR100 on A31
> "allwinner,sun6i-a31-ahb1-clk" - for the AHB1 clock on A31
> + "allwinner,sun8i-h3-ahb2-clk" - for the AHB2 clock on H3
> "allwinner,sun6i-a31-ahb1-gates-clk" - for the AHB1 gates on A31
> "allwinner,sun8i-a23-ahb1-gates-clk" - for the AHB1 gates on A23
> "allwinner,sun9i-a80-ahb0-gates-clk" - for the AHB0 gates on A80
> @@ -55,6 +57,7 @@ Required properties:
> "allwinner,sun9i-a80-apb1-gates-clk" - for the APB1 gates on A80
> "allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
> "allwinner,sun8i-a23-apb2-gates-clk" - for the APB2 gates on A23
> + "allwinner,sun8i-h3-bus-gates-clk" - for the bus gates on H3
> "allwinner,sun5i-a13-mbus-clk" - for the MBUS clock on A13
> "allwinner,sun4i-a10-mmc-clk" - for the MMC clock
> "allwinner,sun9i-a80-mmc-clk" - for mmc module clocks on A80
> @@ -95,6 +98,9 @@ The "allwinner,sun9i-a80-mmc-config-clk" clock also requires:
> For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate
> dummy clocks at 25 MHz and 125 MHz, respectively. See example.
>
> +For "allwinner,sun8i-h3-bus-gates-clk", the parent clocks shall be
> +AHB1, AHB2, APB1 and APB2, in that order.
> +
> Clock consumers should specify the desired clocks they use with a
> "clocks" phandle cell. Consumers that are using a gated clock should
> provide an additional ID in their clock property. This ID is the
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index afe560c..79364be 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -771,6 +771,10 @@ static const struct mux_data sun6i_a31_ahb1_mux_data __initconst = {
> .shift = 12,
> };
>
> +static const struct mux_data sun8i_h3_ahb2_mux_data __initconst = {
> + .shift = 0,
> +};
> +
> static void __init sunxi_mux_clk_setup(struct device_node *node,
> struct mux_data *data)
> {
> @@ -886,7 +890,7 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
> * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
> */
>
> -#define SUNXI_GATES_MAX_SIZE 64
> +#define SUNXI_GATES_MAX_SIZE 160
>
> struct gates_data {
> DECLARE_BITMAP(mask, SUNXI_GATES_MAX_SIZE);
> @@ -990,6 +994,36 @@ static const struct gates_data sun8i_a23_apb2_gates_data __initconst = {
> .mask = {0x1F0007},
> };
>
> +#define BUS_GATE_PARENT_AHB1 0
> +#define BUS_GATE_PARENT_AHB2 1
> +#define BUS_GATE_PARENT_APB1 2
> +#define BUS_GATE_PARENT_APB2 3
> +
> +static const u8 sun8i_h3_bus_gates_parents[] __initconst = {
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1,

> BUS_GATE_PARENT_AHB2,
> BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,

> + BUS_GATE_PARENT_AHB2, BUS_GATE_PARENT_AHB2, BUS_GATE_PARENT_AHB2,

> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1, BUS_GATE_PARENT_AHB1,

> BUS_GATE_PARENT_APB1,
> + BUS_GATE_PARENT_APB1, BUS_GATE_PARENT_APB1, BUS_GATE_PARENT_APB1,
> + BUS_GATE_PARENT_APB1, BUS_GATE_PARENT_APB1, BUS_GATE_PARENT_APB1,

> + BUS_GATE_PARENT_APB2, BUS_GATE_PARENT_APB2, BUS_GATE_PARENT_APB2,
> + BUS_GATE_PARENT_APB2, BUS_GATE_PARENT_APB2, BUS_GATE_PARENT_APB2,
> + BUS_GATE_PARENT_APB2, BUS_GATE_PARENT_APB2,

> BUS_GATE_PARENT_AHB1,
> + BUS_GATE_PARENT_AHB1
> +}

I think something like this:

if (index < .. || index > ..)
clk_parent = "ahb1"
if (index == .. || index < ...)
clk_parent = "ahb2"
etc...

Would be both easier to maintain and to read.

> +
> +static const struct gates_data sun8i_h3_bus_gates_data __initconst = {
> + .mask = {0xffbe6760, 0x00701b39, 0x00007123, 0x001f0007, 0x00000081},
> + .parents = sun8i_h3_bus_gates_parents,
> +};
> +
> static void __init sunxi_gates_clk_setup(struct device_node *node,
> struct gates_data *data)
> {
> @@ -1110,6 +1144,16 @@ static const struct divs_data sun6i_a31_pll6_divs_data __initconst = {
> }
> };
>
> +static const struct divs_data sun8i_h3_pll6_divs_data __initconst = {
> + .factors = &sun6i_a31_pll6_data,
> + .ndivs = 3,
> + .div = {
> + { .fixed = 2 }, /* normal output, pll6 */
> + { .self = 1 }, /* base factor clock, pll6 x2 */
> + { .fixed = 4 }, /* divided output, pll6 /2 */
> + }
> +};

This is exactly the same clock as A31's PLL6, it shouldn't be declared
as a different one.

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]