Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751303AbbEILaQ (ORCPT ); Sat, 9 May 2015 07:30:16 -0400 Received: from down.free-electrons.com ([37.187.137.238]:45824 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750745AbbEILaE (ORCPT ); Sat, 9 May 2015 07:30:04 -0400 Date: Sat, 9 May 2015 13:27:18 +0200 From: Maxime Ripard To: Jens Kuske Cc: Emilio =?iso-8859-1?Q?L=F3pez?= , Mike Turquette , Linus Walleij , Vinod Koul , Rob Herring , Chen-Yu Tsai , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com Subject: Re: [PATCH 2/6] clk: sunxi: Add H3 clocks support Message-ID: <20150509112718.GT11057@lukather> References: <1430904693-1404-1-git-send-email-jenskuske@gmail.com> <1430904693-1404-3-git-send-email-jenskuske@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="yypaS3FvPkEUiGyo" Content-Disposition: inline In-Reply-To: <1430904693-1404-3-git-send-email-jenskuske@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7241 Lines: 182 --yypaS3FvPkEUiGyo Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, May 06, 2015 at 11:31:29AM +0200, Jens Kuske wrote: > The H3 clock control unit is similar to the those of other sun8i family > members like the A23. >=20 > The AHB1 gates got split up into AHB1 and AHB2, with AHB2 clock source > being muxable between AHB1 and PLL6/2, but still sharing gate registers. > 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. >=20 > Signed-off-by: Jens Kuske > --- > Documentation/devicetree/bindings/clock/sunxi.txt | 7 ++++ > drivers/clk/sunxi/clk-sunxi.c | 46 +++++++++++++++++= +++++- > 2 files changed, 52 insertions(+), 1 deletion(-) >=20 > diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Document= ation/devicetree/bindings/clock/sunxi.txt > index 4fa11af..4eeb893 100644 > --- a/Documentation/devicetree/bindings/clock/sunxi.txt > +++ b/Documentation/devicetree/bindings/clock/sunxi.txt > @@ -14,6 +14,8 @@ 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,sun8i-h3-pll8-clk" - for the PLL8 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,8 +30,11 @@ 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,sun8i-h3-ahb1-gates-clk" - for the AHB1 gates on H3 > + "allwinner,sun8i-h3-ahb2-gates-clk" - for the AHB2 gates on H3 > "allwinner,sun9i-a80-ahb0-gates-clk" - for the AHB0 gates on A80 > "allwinner,sun9i-a80-ahb1-gates-clk" - for the AHB1 gates on A80 > "allwinner,sun9i-a80-ahb2-gates-clk" - for the AHB2 gates on A80 > @@ -52,8 +57,10 @@ Required properties: > "allwinner,sun6i-a31-apb1-gates-clk" - for the APB1 gates on A31 > "allwinner,sun7i-a20-apb1-gates-clk" - for the APB1 gates on A20 > "allwinner,sun8i-a23-apb1-gates-clk" - for the APB1 gates on A23 > + "allwinner,sun8i-h3-apb1-gates-clk" - for the APB1 gates on H3 > "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-h3-apb2-gates-clk" - for the APB2 gates on H3 > "allwinner,sun8i-a23-apb2-gates-clk" - for the APB2 gates on A23 > "allwinner,sun5i-a13-mbus-clk" - for the MBUS clock on A13 > "allwinner,sun4i-a10-mmc-clk" - for the MMC clock > diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c > index 7e1e2bd..152a1f7 100644 > --- a/drivers/clk/sunxi/clk-sunxi.c > +++ b/drivers/clk/sunxi/clk-sunxi.c > @@ -727,6 +727,12 @@ static const struct factors_data sun5i_a13_ahb_data = __initconst =3D { > .getter =3D sun5i_a13_get_ahb_factors, > }; > =20 > +static const struct factors_data sun8i_h3_pll8_data __initconst =3D { > + .enable =3D 31, > + .table =3D &sun6i_a31_pll6_config, > + .getter =3D sun6i_a31_get_pll6_factors, > +}; This looks like it's just another instance of the A31 pll6. In such a case, we don't need to declare a new driver, just reuse the same compatible. > static const struct factors_data sun4i_apb1_data __initconst =3D { > .mux =3D 24, > .muxmask =3D BIT(1) | BIT(0), > @@ -777,6 +783,10 @@ static const struct mux_data sun6i_a31_ahb1_mux_data= __initconst =3D { > .shift =3D 12, > }; > =20 > +static const struct mux_data sun8i_h3_ahb2_mux_data __initconst =3D { > + .shift =3D 0, > +}; > + > static void __init sunxi_mux_clk_setup(struct device_node *node, > struct mux_data *data) > { > @@ -892,7 +902,7 @@ static void __init sunxi_divider_clk_setup(struct dev= ice_node *node, > * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks > */ > =20 > -#define SUNXI_GATES_MAX_SIZE 64 > +#define SUNXI_GATES_MAX_SIZE 160 > =20 > struct gates_data { > DECLARE_BITMAP(mask, SUNXI_GATES_MAX_SIZE); > @@ -926,6 +936,10 @@ static const struct gates_data sun8i_a23_ahb1_gates_= data __initconst =3D { > .mask =3D {0x25386742, 0x2505111}, > }; > =20 > +static const struct gates_data sun8i_h3_ahb1_gates_data __initconst =3D { > + .mask =3D {0x1fbc6760, 0x00701b39, 0x00000000, 0x00000000, 0x00000081}, > +}; > + Judging from the user manual, there's a few gates in those 0 registers, is this normal that you don't support them? > static const struct gates_data sun9i_a80_ahb0_gates_data __initconst =3D= { > .mask =3D {0xF5F12B}, > }; > @@ -938,6 +952,10 @@ static const struct gates_data sun9i_a80_ahb2_gates_= data __initconst =3D { > .mask =3D {0x9B7}, > }; > =20 > +static const struct gates_data sun8i_h3_ahb2_gates_data __initconst =3D { > + .mask =3D {0xe0020000}, > +}; > + I don't think we should split the ahb1 and ahb2 gates here. It really looks like it's the same controller. The way I'm seeing it would be to have a single clock driver that would handle both your ahb1 and ahb2 gates. It would take two parents, ahb1 and ahb2, obviously, and would take register depending on the gate w'ere registering either the ahb1 or the ahb2 parent. It seems like there's only a handful of devices in ahb2 anyway, so that wouldn't make a very long list of devices to declare as childs of ahb2. Thanks! Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com --yypaS3FvPkEUiGyo Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVTe8WAAoJEBx+YmzsjxAghQUP/3tB6qA2o0gGHqQNzEvrM3Wo 6/OOF6x54rj1d14LyoajiWzQuV0ipQcoRWj4wpWZCSTus0HzpNmHq6LsISI9WJUr fPTp396JHDyG0wdIXAlnevaiacVf4uQH8wGsVdsvMYSa7LKT+rgEte69wo3aEUwG TMcoF9ilj4QqGCrYYXYhAdXCMKlpeeHbX4HiC+CkpteqyMfZBtpSTthiIoTdojEg D2r/WeQIOpFx4JDN4KMbjbzJm+3TPkUZlSRrsfWCyTF5gjsokCb1f2UOJauLvtN0 ceBjSkzxbzUMb4nK3NA6m9y8TlpLfrb9z8Fc39UQ3BoJQ6x1Mo//xGiYFUYAb76+ gRUm3xVzOP12v0VUL03mxtf/Wqinjg2MF62InxuFSung0aNXsgzRDaSt+Ia/Ezu4 am3tb3GCUi/ne577sVSkZhOADeRquTb0401HbOu80fVqaC7GZqoilyd3GCBP7Ecn xkZ2oulJo5fNDVOhlQYboEaheDl40yfIlxCToNyHeCesKYjWj36kfqHcnxPzV2NU uzfdS2sMVrr+c85afJlBXNPUGu2F5bO0Oqfw8bjdZS+iYMHZyNIMwfbYrKBuLVgM Wa7je8J/ohkbQNWVPL6x2uACSUdnPoeuRno/mTYMFUCF2TbvTLDltAcE5QxRxof4 ukrRm7cqKktgz7fN6ajR =oU09 -----END PGP SIGNATURE----- --yypaS3FvPkEUiGyo-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/