| Message ID | 20180614153548.9644-1-wens@csie.org |
|---|---|
| Headers | show |
| Series | allwinner: a64: add SRAM controller / system control | expand |
On Thu, Jun 14, 2018 at 9:05 PM, Chen-Yu Tsai <wens@csie.org> wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner A64 has a SRAM controller, and in the device tree currently > we have a syscon node to enable EMAC driver to access the EMAC clock > register. As SRAM controller driver can now export regmap for this > register, replace the syscon node to the SRAM controller device node, > and let EMAC driver to acquire its EMAC clock regmap. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > [wens@csie.org: Updated compatible string] > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > index 1b2ef28c42bd..87968dafe1dc 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > @@ -169,9 +169,24 @@ > ranges; > > syscon: syscon@1c00000 { > - compatible = "allwinner,sun50i-a64-system-controller", > - "syscon"; > + compatible = "allwinner,sun50i-a64-system-control"; > reg = <0x01c00000 0x1000>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + sram_c: sram@18000 { > + compatible = "mmio-sram"; > + reg = <0x00018000 0x28000>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0x00018000 0x28000>; > + > + de2_sram: sram-section@0 { So, this can attach to display-engine node through allwinner,sram and add support to claim the sram on sun4i/sun4i_drv.c, correct? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne četrtek, 14. junij 2018 ob 19:09:56 CEST je Jagan Teki napisal(a): > On Thu, Jun 14, 2018 at 9:05 PM, Chen-Yu Tsai <wens@csie.org> wrote: > > From: Icenowy Zheng <icenowy@aosc.io> > > > > Allwinner A64 has a SRAM controller, and in the device tree currently > > we have a syscon node to enable EMAC driver to access the EMAC clock > > register. As SRAM controller driver can now export regmap for this > > register, replace the syscon node to the SRAM controller device node, > > and let EMAC driver to acquire its EMAC clock regmap. > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > [wens@csie.org: Updated compatible string] > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > --- > > > > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index > > 1b2ef28c42bd..87968dafe1dc 100644 > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > > @@ -169,9 +169,24 @@ > > > > ranges; > > > > syscon: syscon@1c00000 { > > > > - compatible = > > "allwinner,sun50i-a64-system-controller", - > > "syscon"; > > + compatible = > > "allwinner,sun50i-a64-system-control"; > > > > reg = <0x01c00000 0x1000>; > > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + sram_c: sram@18000 { > > + compatible = "mmio-sram"; > > + reg = <0x00018000 0x28000>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges = <0 0x00018000 0x28000>; > > + > > + de2_sram: sram-section@0 { > > So, this can attach to display-engine node through allwinner,sram and > add support to claim the sram on sun4i/sun4i_drv.c, correct? Actually, it has to be added to display_clocks node and claimed in drivers/ clk/sunxi-ng/ccu-sun8i-de2.c Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
于 2018年6月15日 GMT+08:00 上午1:27:21, "Jernej Škrabec" <jernej.skrabec@gmail.com> 写到: >Dne četrtek, 14. junij 2018 ob 19:09:56 CEST je Jagan Teki napisal(a): >> On Thu, Jun 14, 2018 at 9:05 PM, Chen-Yu Tsai <wens@csie.org> wrote: >> > From: Icenowy Zheng <icenowy@aosc.io> >> > >> > Allwinner A64 has a SRAM controller, and in the device tree >currently >> > we have a syscon node to enable EMAC driver to access the EMAC >clock >> > register. As SRAM controller driver can now export regmap for this >> > register, replace the syscon node to the SRAM controller device >node, >> > and let EMAC driver to acquire its EMAC clock regmap. >> > >> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >> > [wens@csie.org: Updated compatible string] >> > Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> > --- >> > >> > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 >+++++++++++++++++-- >> > 1 file changed, 17 insertions(+), 2 deletions(-) >> > >> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index >> > 1b2ef28c42bd..87968dafe1dc 100644 >> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> > @@ -169,9 +169,24 @@ >> > >> > ranges; >> > >> > syscon: syscon@1c00000 { >> > >> > - compatible = >> > "allwinner,sun50i-a64-system-controller", - > >> > "syscon"; >> > + compatible = >> > "allwinner,sun50i-a64-system-control"; >> > >> > reg = <0x01c00000 0x1000>; >> > >> > + #address-cells = <1>; >> > + #size-cells = <1>; >> > + ranges; >> > + >> > + sram_c: sram@18000 { >> > + compatible = "mmio-sram"; >> > + reg = <0x00018000 0x28000>; >> > + #address-cells = <1>; >> > + #size-cells = <1>; >> > + ranges = <0 0x00018000 0x28000>; >> > + >> > + de2_sram: sram-section@0 { >> >> So, this can attach to display-engine node through allwinner,sram and >> add support to claim the sram on sun4i/sun4i_drv.c, correct? > >Actually, it has to be added to display_clocks node and claimed in >drivers/ >clk/sunxi-ng/ccu-sun8i-de2.c Sorry it's old practice. My new way is to add a bus driver which claims the SRAM. > >Best regards, >Jernej > > > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 14, 2018 at 11:35:44PM +0800, Chen-Yu Tsai wrote: > Hi, > > This series is the remaining A64 syscon changes from the R40 DWMAC > series. The series aligns how the A64 system control exports a regmap > for the sun8i DWMAC driver to access with what we've done for the R40. > > Originally the A64 used the generic syscon for this bit of hardware. > But this block also contains mapping bits for the onboard SRAM, used > by various peripherals, and other vendor specific bits we may use in > the future. It is by no means generic. And we already have a device > tree binding and driver for the SRAM part. > > The first patch make the SRAM control device export a regmap, exposing > a single EMAC control register, for the DWMAC driver to consume. > > The second and third patches rename the A64 compatible string to read > "system control", which is what the block is named in the user manual. > > The last patch fixes up the device node, and also adds the lone mappable > SRAM block, which is needed by the Display Engine. For the serie: Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> Maxime
On Fri, Jun 15, 2018 at 5:14 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Thu, Jun 14, 2018 at 11:35:44PM +0800, Chen-Yu Tsai wrote: >> Hi, >> >> This series is the remaining A64 syscon changes from the R40 DWMAC >> series. The series aligns how the A64 system control exports a regmap >> for the sun8i DWMAC driver to access with what we've done for the R40. >> >> Originally the A64 used the generic syscon for this bit of hardware. >> But this block also contains mapping bits for the onboard SRAM, used >> by various peripherals, and other vendor specific bits we may use in >> the future. It is by no means generic. And we already have a device >> tree binding and driver for the SRAM part. >> >> The first patch make the SRAM control device export a regmap, exposing >> a single EMAC control register, for the DWMAC driver to consume. >> >> The second and third patches rename the A64 compatible string to read >> "system control", which is what the block is named in the user manual. >> >> The last patch fixes up the device node, and also adds the lone mappable >> SRAM block, which is needed by the Display Engine. > > For the serie: > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> Cool. I'll apply them once you've rebased and pushed out any patches you queued up during the merge window. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 18, 2018 at 10:11:20AM +0800, Chen-Yu Tsai wrote: > On Fri, Jun 15, 2018 at 5:14 PM, Maxime Ripard > <maxime.ripard@bootlin.com> wrote: > > On Thu, Jun 14, 2018 at 11:35:44PM +0800, Chen-Yu Tsai wrote: > >> Hi, > >> > >> This series is the remaining A64 syscon changes from the R40 DWMAC > >> series. The series aligns how the A64 system control exports a regmap > >> for the sun8i DWMAC driver to access with what we've done for the R40. > >> > >> Originally the A64 used the generic syscon for this bit of hardware. > >> But this block also contains mapping bits for the onboard SRAM, used > >> by various peripherals, and other vendor specific bits we may use in > >> the future. It is by no means generic. And we already have a device > >> tree binding and driver for the SRAM part. > >> > >> The first patch make the SRAM control device export a regmap, exposing > >> a single EMAC control register, for the DWMAC driver to consume. > >> > >> The second and third patches rename the A64 compatible string to read > >> "system control", which is what the block is named in the user manual. > >> > >> The last patch fixes up the device node, and also adds the lone mappable > >> SRAM block, which is needed by the Display Engine. > > > > For the serie: > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > Cool. I'll apply them once you've rebased and pushed out any patches you > queued up during the merge window. All should be pushed now, feel free to apply them :) Maxime
On Fri, Jun 15, 2018 at 4:39 AM, Icenowy Zheng <icenowy@aosc.io> wrote: > > > 于 2018年6月15日 GMT+08:00 上午1:27:21, "Jernej Škrabec" <jernej.skrabec@gmail.com> 写到: >>Dne četrtek, 14. junij 2018 ob 19:09:56 CEST je Jagan Teki napisal(a): >>> On Thu, Jun 14, 2018 at 9:05 PM, Chen-Yu Tsai <wens@csie.org> wrote: >>> > From: Icenowy Zheng <icenowy@aosc.io> >>> > >>> > Allwinner A64 has a SRAM controller, and in the device tree >>currently >>> > we have a syscon node to enable EMAC driver to access the EMAC >>clock >>> > register. As SRAM controller driver can now export regmap for this >>> > register, replace the syscon node to the SRAM controller device >>node, >>> > and let EMAC driver to acquire its EMAC clock regmap. >>> > >>> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >>> > [wens@csie.org: Updated compatible string] >>> > Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>> > --- >>> > >>> > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 >>+++++++++++++++++-- >>> > 1 file changed, 17 insertions(+), 2 deletions(-) >>> > >>> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>> > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index >>> > 1b2ef28c42bd..87968dafe1dc 100644 >>> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>> > @@ -169,9 +169,24 @@ >>> > >>> > ranges; >>> > >>> > syscon: syscon@1c00000 { >>> > >>> > - compatible = >>> > "allwinner,sun50i-a64-system-controller", - >> >>> > "syscon"; >>> > + compatible = >>> > "allwinner,sun50i-a64-system-control"; >>> > >>> > reg = <0x01c00000 0x1000>; >>> > >>> > + #address-cells = <1>; >>> > + #size-cells = <1>; >>> > + ranges; >>> > + >>> > + sram_c: sram@18000 { >>> > + compatible = "mmio-sram"; >>> > + reg = <0x00018000 0x28000>; >>> > + #address-cells = <1>; >>> > + #size-cells = <1>; >>> > + ranges = <0 0x00018000 0x28000>; >>> > + >>> > + de2_sram: sram-section@0 { >>> >>> So, this can attach to display-engine node through allwinner,sram and >>> add support to claim the sram on sun4i/sun4i_drv.c, correct? >> >>Actually, it has to be added to display_clocks node and claimed in >>drivers/ >>clk/sunxi-ng/ccu-sun8i-de2.c > > Sorry it's old practice. > > My new way is to add a bus driver which claims the SRAM. Are you pointing this driver[1] ? if yes do we have recent version patches for these changes so-that I can import. [1] https://patchwork.kernel.org/patch/10290399/ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
于 2018年6月19日 GMT+08:00 下午3:36:19, Jagan Teki <jagan@amarulasolutions.com> 写到: >On Fri, Jun 15, 2018 at 4:39 AM, Icenowy Zheng <icenowy@aosc.io> wrote: >> >> >> 于 2018年6月15日 GMT+08:00 上午1:27:21, "Jernej Škrabec" ><jernej.skrabec@gmail.com> 写到: >>>Dne četrtek, 14. junij 2018 ob 19:09:56 CEST je Jagan Teki >napisal(a): >>>> On Thu, Jun 14, 2018 at 9:05 PM, Chen-Yu Tsai <wens@csie.org> >wrote: >>>> > From: Icenowy Zheng <icenowy@aosc.io> >>>> > >>>> > Allwinner A64 has a SRAM controller, and in the device tree >>>currently >>>> > we have a syscon node to enable EMAC driver to access the EMAC >>>clock >>>> > register. As SRAM controller driver can now export regmap for >this >>>> > register, replace the syscon node to the SRAM controller device >>>node, >>>> > and let EMAC driver to acquire its EMAC clock regmap. >>>> > >>>> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >>>> > [wens@csie.org: Updated compatible string] >>>> > Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>>> > --- >>>> > >>>> > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 >>>+++++++++++++++++-- >>>> > 1 file changed, 17 insertions(+), 2 deletions(-) >>>> > >>>> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>>> > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index >>>> > 1b2ef28c42bd..87968dafe1dc 100644 >>>> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>>> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>>> > @@ -169,9 +169,24 @@ >>>> > >>>> > ranges; >>>> > >>>> > syscon: syscon@1c00000 { >>>> > >>>> > - compatible = >>>> > "allwinner,sun50i-a64-system-controller", - >>> >>>> > "syscon"; >>>> > + compatible = >>>> > "allwinner,sun50i-a64-system-control"; >>>> > >>>> > reg = <0x01c00000 0x1000>; >>>> > >>>> > + #address-cells = <1>; >>>> > + #size-cells = <1>; >>>> > + ranges; >>>> > + >>>> > + sram_c: sram@18000 { >>>> > + compatible = "mmio-sram"; >>>> > + reg = <0x00018000 0x28000>; >>>> > + #address-cells = <1>; >>>> > + #size-cells = <1>; >>>> > + ranges = <0 0x00018000 0x28000>; >>>> > + >>>> > + de2_sram: sram-section@0 { >>>> >>>> So, this can attach to display-engine node through allwinner,sram >and >>>> add support to claim the sram on sun4i/sun4i_drv.c, correct? >>> >>>Actually, it has to be added to display_clocks node and claimed in >>>drivers/ >>>clk/sunxi-ng/ccu-sun8i-de2.c >> >> Sorry it's old practice. >> >> My new way is to add a bus driver which claims the SRAM. > >Are you pointing this driver[1] ? if yes do we have recent version >patches for these changes so-that I can import. I'll resend it when this patchset is ready. > >[1] https://patchwork.kernel.org/patch/10290399/ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 18, 2018 at 3:34 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Mon, Jun 18, 2018 at 10:11:20AM +0800, Chen-Yu Tsai wrote: >> On Fri, Jun 15, 2018 at 5:14 PM, Maxime Ripard >> <maxime.ripard@bootlin.com> wrote: >> > On Thu, Jun 14, 2018 at 11:35:44PM +0800, Chen-Yu Tsai wrote: >> >> Hi, >> >> >> >> This series is the remaining A64 syscon changes from the R40 DWMAC >> >> series. The series aligns how the A64 system control exports a regmap >> >> for the sun8i DWMAC driver to access with what we've done for the R40. >> >> >> >> Originally the A64 used the generic syscon for this bit of hardware. >> >> But this block also contains mapping bits for the onboard SRAM, used >> >> by various peripherals, and other vendor specific bits we may use in >> >> the future. It is by no means generic. And we already have a device >> >> tree binding and driver for the SRAM part. >> >> >> >> The first patch make the SRAM control device export a regmap, exposing >> >> a single EMAC control register, for the DWMAC driver to consume. >> >> >> >> The second and third patches rename the A64 compatible string to read >> >> "system control", which is what the block is named in the user manual. >> >> >> >> The last patch fixes up the device node, and also adds the lone mappable >> >> SRAM block, which is needed by the Display Engine. >> > >> > For the serie: >> > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> >> >> Cool. I'll apply them once you've rebased and pushed out any patches you >> queued up during the merge window. > > All should be pushed now, feel free to apply them :) Applied. Thanks. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Chen-Yu, On Thu, 14 Jun 2018 23:35:44 +0800 Chen-Yu Tsai <wens@csie.org> wrote: > Hi, > > This series is the remaining A64 syscon changes from the R40 DWMAC > series. The series aligns how the A64 system control exports a regmap > for the sun8i DWMAC driver to access with what we've done for the R40. > > Originally the A64 used the generic syscon for this bit of hardware. > But this block also contains mapping bits for the onboard SRAM, used > by various peripherals, and other vendor specific bits we may use in > the future. It is by no means generic. And we already have a device > tree binding and driver for the SRAM part. > > The first patch make the SRAM control device export a regmap, exposing > a single EMAC control register, for the DWMAC driver to consume. > > The second and third patches rename the A64 compatible string to read > "system control", which is what the block is named in the user manual. > > The last patch fixes up the device node, and also adds the lone mappable > SRAM block, which is needed by the Display Engine. > > Changes since v2: > > - changed the compatible string from "*-sram-controller" to > "*-system-control" > > > ChenYu > > Chen-Yu Tsai (2): > dt-bindings: sram: Rename A64 SRAM controller compatible > soc: sunxi: sram: Add updated compatible string for A64 system control > > Icenowy Zheng (2): > soc: sunxi: export a regmap for EMAC clock reg on A64 > arm64: dts: allwinner: a64: add SRAM controller device tree node > > .../devicetree/bindings/sram/sunxi-sram.txt | 3 +- > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++- > drivers/soc/sunxi/sunxi_sram.c | 61 ++++++++++++++++++- > 3 files changed, 78 insertions(+), 5 deletions(-) > > -- > 2.17.1 I wish to have seen this serie before as it have some inconsistencies. In patch 2 you renamed allwinner,sun50i-a64-sram-controller to allwinner,sun50i-a64-system-control but the former was never used in the DTS, the compatible used was allwinner,sun50i-a64-system-controller. You also say that you've never seen use of it. How can you make that claim ? There is a lot of downstream users of DTS now (FreeBSD, NetBSD, OpenBSD and even RiscOS and Haiku iirc), it's not just Linux. Also this compatible is currently the one used in the u-boot dts, which mean that users of the embedded DTB use or can use it (which is the default for EFI users of U-Boot). Cheers,
On Mon, Aug 20, 2018 at 3:42 PM, Emmanuel Vadot <manu@bidouilliste.com> wrote: > > Hello Chen-Yu, > > On Thu, 14 Jun 2018 23:35:44 +0800 > Chen-Yu Tsai <wens@csie.org> wrote: > >> Hi, >> >> This series is the remaining A64 syscon changes from the R40 DWMAC >> series. The series aligns how the A64 system control exports a regmap >> for the sun8i DWMAC driver to access with what we've done for the R40. >> >> Originally the A64 used the generic syscon for this bit of hardware. >> But this block also contains mapping bits for the onboard SRAM, used >> by various peripherals, and other vendor specific bits we may use in >> the future. It is by no means generic. And we already have a device >> tree binding and driver for the SRAM part. >> >> The first patch make the SRAM control device export a regmap, exposing >> a single EMAC control register, for the DWMAC driver to consume. >> >> The second and third patches rename the A64 compatible string to read >> "system control", which is what the block is named in the user manual. >> >> The last patch fixes up the device node, and also adds the lone mappable >> SRAM block, which is needed by the Display Engine. >> >> Changes since v2: >> >> - changed the compatible string from "*-sram-controller" to >> "*-system-control" >> >> >> ChenYu >> >> Chen-Yu Tsai (2): >> dt-bindings: sram: Rename A64 SRAM controller compatible >> soc: sunxi: sram: Add updated compatible string for A64 system control >> >> Icenowy Zheng (2): >> soc: sunxi: export a regmap for EMAC clock reg on A64 >> arm64: dts: allwinner: a64: add SRAM controller device tree node >> >> .../devicetree/bindings/sram/sunxi-sram.txt | 3 +- >> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++- >> drivers/soc/sunxi/sunxi_sram.c | 61 ++++++++++++++++++- >> 3 files changed, 78 insertions(+), 5 deletions(-) >> >> -- >> 2.17.1 > > I wish to have seen this serie before as it have some inconsistencies. > > In patch 2 you renamed allwinner,sun50i-a64-sram-controller to > allwinner,sun50i-a64-system-control but the former was never used in > the DTS, the compatible used was allwinner,sun50i-a64-system-controller. > You also say that you've never seen use of it. How can you make that > claim ? There is a lot of downstream users of DTS now (FreeBSD, NetBSD, > OpenBSD and even RiscOS and Haiku iirc), it's not just Linux. The original "a64-sram-controller" was never used in any upstream device tree, be it Linux or U-boot. Since the projects you listed all derive their device trees from U-boot, with maybe some extra devices on top, they would either have had the "system-controller" version, or the even earlier version in U-boot where the emac node just lists the syscon address range as its own. U-boot has the latter "system-controller" because it was copied from Linux. The original use-case was a SoC-specific compatible string followed by the very generic "syscon". The latter "syscon" binding is what we actually support in Linux and U-boot. Same goes for NetBSD and OpenBSD. IMHO FreeBSD could also move to a generic syscon API, instead of "system-controller" for sunxi and "grf" for rockchip. You can continue to support old device trees and the unlisted compatible through the generic "syscon" fallback. For the updated device tree you will have to support the new compatible, along with supporting the SRAM mappings. The SRAM mappings are why the "syscon" compatible was removed. It just doesn't fit the semantics described by the "syscon" binding. > Also this compatible is currently the one used in the u-boot dts, > which mean that users of the embedded DTB use or can use it (which is > the default for EFI users of U-Boot). As mentioned above, the old device tree uses the syscon binding, which you can continue to support. The new binding is add support for the SRAM mapping system. In addition, you probably don't want two device drivers supporting the same compatible string. On Linux it's even worse, because the "syscon" driver sidesteps the device model and isn't an actual device driver. Regards ChenYu
On Mon, 20 Aug 2018 16:41:09 +0800 Chen-Yu Tsai <wens@csie.org> wrote: > On Mon, Aug 20, 2018 at 3:42 PM, Emmanuel Vadot <manu@bidouilliste.com> wrote: > > > > Hello Chen-Yu, > > > > On Thu, 14 Jun 2018 23:35:44 +0800 > > Chen-Yu Tsai <wens@csie.org> wrote: > > > >> Hi, > >> > >> This series is the remaining A64 syscon changes from the R40 DWMAC > >> series. The series aligns how the A64 system control exports a regmap > >> for the sun8i DWMAC driver to access with what we've done for the R40. > >> > >> Originally the A64 used the generic syscon for this bit of hardware. > >> But this block also contains mapping bits for the onboard SRAM, used > >> by various peripherals, and other vendor specific bits we may use in > >> the future. It is by no means generic. And we already have a device > >> tree binding and driver for the SRAM part. > >> > >> The first patch make the SRAM control device export a regmap, exposing > >> a single EMAC control register, for the DWMAC driver to consume. > >> > >> The second and third patches rename the A64 compatible string to read > >> "system control", which is what the block is named in the user manual. > >> > >> The last patch fixes up the device node, and also adds the lone mappable > >> SRAM block, which is needed by the Display Engine. > >> > >> Changes since v2: > >> > >> - changed the compatible string from "*-sram-controller" to > >> "*-system-control" > >> > >> > >> ChenYu > >> > >> Chen-Yu Tsai (2): > >> dt-bindings: sram: Rename A64 SRAM controller compatible > >> soc: sunxi: sram: Add updated compatible string for A64 system control > >> > >> Icenowy Zheng (2): > >> soc: sunxi: export a regmap for EMAC clock reg on A64 > >> arm64: dts: allwinner: a64: add SRAM controller device tree node > >> > >> .../devicetree/bindings/sram/sunxi-sram.txt | 3 +- > >> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++- > >> drivers/soc/sunxi/sunxi_sram.c | 61 ++++++++++++++++++- > >> 3 files changed, 78 insertions(+), 5 deletions(-) > >> > >> -- > >> 2.17.1 > > > > I wish to have seen this serie before as it have some inconsistencies. > > > > In patch 2 you renamed allwinner,sun50i-a64-sram-controller to > > allwinner,sun50i-a64-system-control but the former was never used in > > the DTS, the compatible used was allwinner,sun50i-a64-system-controller. > > You also say that you've never seen use of it. How can you make that > > claim ? There is a lot of downstream users of DTS now (FreeBSD, NetBSD, > > OpenBSD and even RiscOS and Haiku iirc), it's not just Linux. > > The original "a64-sram-controller" was never used in any upstream device > tree, be it Linux or U-boot. Since the projects you listed all derive > their device trees from U-boot, with maybe some extra devices on top, > they would either have had the "system-controller" version, or the even > earlier version in U-boot where the emac node just lists the syscon > address range as its own. > > U-boot has the latter "system-controller" because it was copied from Linux. > The original use-case was a SoC-specific compatible string followed by the > very generic "syscon". The latter "syscon" binding is what we actually > support in Linux and U-boot. Same goes for NetBSD and OpenBSD. > > IMHO FreeBSD could also move to a generic syscon API, instead of > "system-controller" for sunxi and "grf" for rockchip. We decided to always implement a specific driver if the node have another compatible than syscon (I don't see the point of having a syscon compatible plus another one) > You can continue to support old device trees and the unlisted compatible > through the generic "syscon" fallback. For the updated device tree you will > have to support the new compatible, along with supporting the SRAM mappings. > The SRAM mappings are why the "syscon" compatible was removed. It just > doesn't fit the semantics described by the "syscon" binding. I understand the removal of syscon since the sram addition, I just don't understand the strict renaming and why adding a new one couldn't work. > > Also this compatible is currently the one used in the u-boot dts, > > which mean that users of the embedded DTB use or can use it (which is > > the default for EFI users of U-Boot). > > As mentioned above, the old device tree uses the syscon binding, which > you can continue to support. The new binding is add support for the SRAM > mapping system. In addition, you probably don't want two device drivers > supporting the same compatible string. On Linux it's even worse, because > the "syscon" driver sidesteps the device model and isn't an actual device > driver. > > Regards > ChenYu > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Aug 20, 2018 at 10:01 PM, Emmanuel Vadot <manu@bidouilliste.com> wrote: > On Mon, 20 Aug 2018 16:41:09 +0800 > Chen-Yu Tsai <wens@csie.org> wrote: > >> On Mon, Aug 20, 2018 at 3:42 PM, Emmanuel Vadot <manu@bidouilliste.com> wrote: >> > >> > Hello Chen-Yu, >> > >> > On Thu, 14 Jun 2018 23:35:44 +0800 >> > Chen-Yu Tsai <wens@csie.org> wrote: >> > >> >> Hi, >> >> >> >> This series is the remaining A64 syscon changes from the R40 DWMAC >> >> series. The series aligns how the A64 system control exports a regmap >> >> for the sun8i DWMAC driver to access with what we've done for the R40. >> >> >> >> Originally the A64 used the generic syscon for this bit of hardware. >> >> But this block also contains mapping bits for the onboard SRAM, used >> >> by various peripherals, and other vendor specific bits we may use in >> >> the future. It is by no means generic. And we already have a device >> >> tree binding and driver for the SRAM part. >> >> >> >> The first patch make the SRAM control device export a regmap, exposing >> >> a single EMAC control register, for the DWMAC driver to consume. >> >> >> >> The second and third patches rename the A64 compatible string to read >> >> "system control", which is what the block is named in the user manual. >> >> >> >> The last patch fixes up the device node, and also adds the lone mappable >> >> SRAM block, which is needed by the Display Engine. >> >> >> >> Changes since v2: >> >> >> >> - changed the compatible string from "*-sram-controller" to >> >> "*-system-control" >> >> >> >> >> >> ChenYu >> >> >> >> Chen-Yu Tsai (2): >> >> dt-bindings: sram: Rename A64 SRAM controller compatible >> >> soc: sunxi: sram: Add updated compatible string for A64 system control >> >> >> >> Icenowy Zheng (2): >> >> soc: sunxi: export a regmap for EMAC clock reg on A64 >> >> arm64: dts: allwinner: a64: add SRAM controller device tree node >> >> >> >> .../devicetree/bindings/sram/sunxi-sram.txt | 3 +- >> >> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++- >> >> drivers/soc/sunxi/sunxi_sram.c | 61 ++++++++++++++++++- >> >> 3 files changed, 78 insertions(+), 5 deletions(-) >> >> >> >> -- >> >> 2.17.1 >> > >> > I wish to have seen this serie before as it have some inconsistencies. >> > >> > In patch 2 you renamed allwinner,sun50i-a64-sram-controller to >> > allwinner,sun50i-a64-system-control but the former was never used in >> > the DTS, the compatible used was allwinner,sun50i-a64-system-controller. >> > You also say that you've never seen use of it. How can you make that >> > claim ? There is a lot of downstream users of DTS now (FreeBSD, NetBSD, >> > OpenBSD and even RiscOS and Haiku iirc), it's not just Linux. >> >> The original "a64-sram-controller" was never used in any upstream device >> tree, be it Linux or U-boot. Since the projects you listed all derive >> their device trees from U-boot, with maybe some extra devices on top, >> they would either have had the "system-controller" version, or the even >> earlier version in U-boot where the emac node just lists the syscon >> address range as its own. >> >> U-boot has the latter "system-controller" because it was copied from Linux. >> The original use-case was a SoC-specific compatible string followed by the >> very generic "syscon". The latter "syscon" binding is what we actually >> support in Linux and U-boot. Same goes for NetBSD and OpenBSD. >> >> IMHO FreeBSD could also move to a generic syscon API, instead of >> "system-controller" for sunxi and "grf" for rockchip. > > We decided to always implement a specific driver if the node have > another compatible than syscon (I don't see the point of having a > syscon compatible plus another one) In most cases, the fallback is there in situations when support for the older or more generic device type can work with the new device, albeit at lower efficiency or without full functionality. Using a specific one with the "syscon" doesn't quite make sense to me either, but some platforms, like Rockchip, do use it. I suppose it would help them identify the type of syscon, but I think that is really tied to the consumer. >> You can continue to support old device trees and the unlisted compatible >> through the generic "syscon" fallback. For the updated device tree you will >> have to support the new compatible, along with supporting the SRAM mappings. >> The SRAM mappings are why the "syscon" compatible was removed. It just >> doesn't fit the semantics described by the "syscon" binding. > > I understand the removal of syscon since the sram addition, I just > don't understand the strict renaming and why adding a new one couldn't > work. You mean having something like the following? compatible = "allwinner,sun50i-a64-system-control", "allwinner,sun50i-a64-system-controller"; Well, the "system-controller" compatible was never listed in any binding, and it shouldn't have been there in the first place. And I think having the compatibles match up with the datasheet is better. Last, it is not an actual "controller". It is just a set of control registers. ChenYu >> > Also this compatible is currently the one used in the u-boot dts, >> > which mean that users of the embedded DTB use or can use it (which is >> > the default for EFI users of U-Boot). >> >> As mentioned above, the old device tree uses the syscon binding, which >> you can continue to support. The new binding is add support for the SRAM >> mapping system. In addition, you probably don't want two device drivers >> supporting the same compatible string. On Linux it's even worse, because >> the "syscon" driver sidesteps the device model and isn't an actual device >> driver. >> >> Regards >> ChenYu >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > -- > Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
On Mon, 20 Aug 2018 22:25:16 +0800 Chen-Yu Tsai <wens@csie.org> wrote: > On Mon, Aug 20, 2018 at 10:01 PM, Emmanuel Vadot <manu@bidouilliste.com> wrote: > > On Mon, 20 Aug 2018 16:41:09 +0800 > > Chen-Yu Tsai <wens@csie.org> wrote: > > > >> On Mon, Aug 20, 2018 at 3:42 PM, Emmanuel Vadot <manu@bidouilliste.com> wrote: > >> > > >> > Hello Chen-Yu, > >> > > >> > On Thu, 14 Jun 2018 23:35:44 +0800 > >> > Chen-Yu Tsai <wens@csie.org> wrote: > >> > > >> >> Hi, > >> >> > >> >> This series is the remaining A64 syscon changes from the R40 DWMAC > >> >> series. The series aligns how the A64 system control exports a regmap > >> >> for the sun8i DWMAC driver to access with what we've done for the R40. > >> >> > >> >> Originally the A64 used the generic syscon for this bit of hardware. > >> >> But this block also contains mapping bits for the onboard SRAM, used > >> >> by various peripherals, and other vendor specific bits we may use in > >> >> the future. It is by no means generic. And we already have a device > >> >> tree binding and driver for the SRAM part. > >> >> > >> >> The first patch make the SRAM control device export a regmap, exposing > >> >> a single EMAC control register, for the DWMAC driver to consume. > >> >> > >> >> The second and third patches rename the A64 compatible string to read > >> >> "system control", which is what the block is named in the user manual. > >> >> > >> >> The last patch fixes up the device node, and also adds the lone mappable > >> >> SRAM block, which is needed by the Display Engine. > >> >> > >> >> Changes since v2: > >> >> > >> >> - changed the compatible string from "*-sram-controller" to > >> >> "*-system-control" > >> >> > >> >> > >> >> ChenYu > >> >> > >> >> Chen-Yu Tsai (2): > >> >> dt-bindings: sram: Rename A64 SRAM controller compatible > >> >> soc: sunxi: sram: Add updated compatible string for A64 system control > >> >> > >> >> Icenowy Zheng (2): > >> >> soc: sunxi: export a regmap for EMAC clock reg on A64 > >> >> arm64: dts: allwinner: a64: add SRAM controller device tree node > >> >> > >> >> .../devicetree/bindings/sram/sunxi-sram.txt | 3 +- > >> >> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++- > >> >> drivers/soc/sunxi/sunxi_sram.c | 61 ++++++++++++++++++- > >> >> 3 files changed, 78 insertions(+), 5 deletions(-) > >> >> > >> >> -- > >> >> 2.17.1 > >> > > >> > I wish to have seen this serie before as it have some inconsistencies. > >> > > >> > In patch 2 you renamed allwinner,sun50i-a64-sram-controller to > >> > allwinner,sun50i-a64-system-control but the former was never used in > >> > the DTS, the compatible used was allwinner,sun50i-a64-system-controller. > >> > You also say that you've never seen use of it. How can you make that > >> > claim ? There is a lot of downstream users of DTS now (FreeBSD, NetBSD, > >> > OpenBSD and even RiscOS and Haiku iirc), it's not just Linux. > >> > >> The original "a64-sram-controller" was never used in any upstream device > >> tree, be it Linux or U-boot. Since the projects you listed all derive > >> their device trees from U-boot, with maybe some extra devices on top, > >> they would either have had the "system-controller" version, or the even > >> earlier version in U-boot where the emac node just lists the syscon > >> address range as its own. > >> > >> U-boot has the latter "system-controller" because it was copied from Linux. > >> The original use-case was a SoC-specific compatible string followed by the > >> very generic "syscon". The latter "syscon" binding is what we actually > >> support in Linux and U-boot. Same goes for NetBSD and OpenBSD. > >> > >> IMHO FreeBSD could also move to a generic syscon API, instead of > >> "system-controller" for sunxi and "grf" for rockchip. > > > > We decided to always implement a specific driver if the node have > > another compatible than syscon (I don't see the point of having a > > syscon compatible plus another one) > > In most cases, the fallback is there in situations when support for the > older or more generic device type can work with the new device, albeit > at lower efficiency or without full functionality. > > Using a specific one with the "syscon" doesn't quite make sense to me > either, but some platforms, like Rockchip, do use it. I suppose it would > help them identify the type of syscon, but I think that is really tied > to the consumer. Maybe we should revised our decision then. > >> You can continue to support old device trees and the unlisted compatible > >> through the generic "syscon" fallback. For the updated device tree you will > >> have to support the new compatible, along with supporting the SRAM mappings. > >> The SRAM mappings are why the "syscon" compatible was removed. It just > >> doesn't fit the semantics described by the "syscon" binding. > > > > I understand the removal of syscon since the sram addition, I just > > don't understand the strict renaming and why adding a new one couldn't > > work. > > You mean having something like the following? > > compatible = "allwinner,sun50i-a64-system-control", > "allwinner,sun50i-a64-system-controller"; Yes. > Well, the "system-controller" compatible was never listed in any binding, > and it shouldn't have been there in the first place. And I think having > the compatibles match up with the datasheet is better. Last, it is not > an actual "controller". It is just a set of control registers. Yes I think the main problem was that some non-documented binding was used. > ChenYu > > >> > Also this compatible is currently the one used in the u-boot dts, > >> > which mean that users of the embedded DTB use or can use it (which is > >> > the default for EFI users of U-Boot). > >> > >> As mentioned above, the old device tree uses the syscon binding, which > >> you can continue to support. The new binding is add support for the SRAM > >> mapping system. In addition, you probably don't want two device drivers > >> supporting the same compatible string. On Linux it's even worse, because > >> the "syscon" driver sidesteps the device model and isn't an actual device > >> driver. > >> > >> Regards > >> ChenYu > >> > >> _______________________________________________ > >> linux-arm-kernel mailing list > >> linux-arm-kernel@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > > > -- > > Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
Hi, This series is the remaining A64 syscon changes from the R40 DWMAC series. The series aligns how the A64 system control exports a regmap for the sun8i DWMAC driver to access with what we've done for the R40. Originally the A64 used the generic syscon for this bit of hardware. But this block also contains mapping bits for the onboard SRAM, used by various peripherals, and other vendor specific bits we may use in the future. It is by no means generic. And we already have a device tree binding and driver for the SRAM part. The first patch make the SRAM control device export a regmap, exposing a single EMAC control register, for the DWMAC driver to consume. The second and third patches rename the A64 compatible string to read "system control", which is what the block is named in the user manual. The last patch fixes up the device node, and also adds the lone mappable SRAM block, which is needed by the Display Engine. Changes since v2: - changed the compatible string from "*-sram-controller" to "*-system-control" ChenYu Chen-Yu Tsai (2): dt-bindings: sram: Rename A64 SRAM controller compatible soc: sunxi: sram: Add updated compatible string for A64 system control Icenowy Zheng (2): soc: sunxi: export a regmap for EMAC clock reg on A64 arm64: dts: allwinner: a64: add SRAM controller device tree node .../devicetree/bindings/sram/sunxi-sram.txt | 3 +- arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++- drivers/soc/sunxi/sunxi_sram.c | 61 ++++++++++++++++++- 3 files changed, 78 insertions(+), 5 deletions(-)