| Message ID | 20180501161227.2110-1-wens@csie.org |
|---|---|
| Headers | show |
| Series | ARM: sun8i: r40: Add Ethernet support | expand |
On Wed, May 2, 2018 at 12:12 AM, Chen-Yu Tsai <wens@csie.org> wrote: > Hi everyone, > > This is v2 of my R40 Ethernet support series. > > Changes since v1: > > - Default to fetching regmap from device pointed to by syscon phandle, > and falling back to syscon API if that fails. > > - Dropped .syscon_from_dev field in device data as a result of the > previous change. > > - Added a large comment block explaining the first change. > > - Simplified description of syscon property in sun8i-dwmac binding. > > - Regmap now only exposes the EMAC/GMAC register, but retains the > offset within its address space. > > - Added patches for A64, which reuse the same sun8i-dwmac changes. > > This series adds support for the DWMAC based Ethernet controller found > on the Allwinner R40 SoC. The controller is either a DWMAC clone or > DWMAC core with its registers rearranged. This is already supported by > the dwmac-sun8i driver. The glue layer control registers, unlike other > sun8i family SoCs, is not in the system controller region, but in the > clock control unit, like with the older A20 and A31 SoCs. > > While we reuse the bindings for dwmac-sun8i using a syscon phandle > reference, we need some custom plumbing for the clock driver to export > a regmap that only allows access to the GMAC register to the dwmac-sun8i > driver. An alternative would be to allow drivers to register custom > syscon devices with their own regmap and locking. > > Patch 1 converts the CLK_OF_DECLARE style clock driver to a platform > one, so the regmap introduced later has a struct device to tie to. > > Patch 2 adds a regmap that is exported by the clock driver for the > dwmac-sun8i driver to use. > > Patches 3, 4, and 5 clean up the dwmac-sun8i binding. > > Patch 6 adds device tree binding for Allwinner R40's Ethernet > controller. > > Patch 7 converts regmap access of the syscon region in the dwmac-sun8i > driver to regmap_field, in anticipation of different field widths on > the R40. > > Patch 8 introduces custom plumbing in the dwmac-sun8i driver to fetch > a regmap from another device, by looking up said device via a phandle, > then getting the regmap associated with that device. > > Patch 9 adds support for different or absent TX/RX delay chain ranges > to the dwmac-sun8i driver. > > Patch 10 adds support for the R40's ethernet controller. I should've mentioned that patches 3 ~ 10, and only these, should go through net-next. sunxi will handle the remaining clk, device tree, and soc driver patches. Thanks ChenYu > Patch 11 cleans up the Bananapi M2 Ultra device tree file. > > Patch 12 adds a GMAC device node and RGMII mode pinmux setting for the > R40. > > Patch 13 enables Ethernet on the Bananapi M2 Ultra. > > Patches 14 and 15 are for the A64. They convert the existing syscon > device to an SRAM controller device that exports a regmap. The needed > driver changes are in patch 14, and the device tree changes are in > patch 15. > > > Please have a look. > > Regards > ChenYu > > Chen-Yu Tsai (11): > dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions > dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical > order > dt-bindings: net: dwmac-sun8i: simplify description of syscon property > dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40 > SoC > net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access > net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external > device > net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay > chains > net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC > ARM: dts: sun8i: r40: bananapi-m2-ultra: Sort device node dereferences > ARM: dts: sun8i: r40: Add device node and RGMII pinmux node for GMAC > ARM: dts: sun8i: r40: bananapi-m2-ultra: Enable GMAC ethernet > controller > > Icenowy Zheng (4): > clk: sunxi-ng: r40: rewrite init code to a platform driver > clk: sunxi-ng: r40: export a regmap to access the GMAC register > soc: sunxi: export a regmap for EMAC clock reg on A64 > arm64: dts: allwinner: a64: add SRAM controller device tree node > > .../devicetree/bindings/net/dwmac-sun8i.txt | 21 +-- > .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 99 ++++++++----- > arch/arm/boot/dts/sun8i-r40.dtsi | 34 +++++ > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 ++- > drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 72 +++++++-- > .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 139 +++++++++++++++--- > drivers/soc/sunxi/sunxi_sram.c | 57 ++++++- > 7 files changed, 364 insertions(+), 81 deletions(-) > > -- > 2.17.0 > -- 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
Hi, On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 +++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > index 1b2ef28c42bd..1c37659d9d41 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > @@ -168,10 +168,25 @@ > #size-cells = <1>; > ranges; > > - syscon: syscon@1c00000 { > - compatible = "allwinner,sun50i-a64-system-controller", > - "syscon"; > + sram_controller: sram-controller@1c00000 { > + compatible = "allwinner,sun50i-a64-sram-controller"; I don't think there's anything preventing us from keeping the -system-controller compatible. It's what was in the DT before, and it's how it's called in the datasheet. Otherwise, the whole serie looks good to me: Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> Maxime
On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > Hi, > > On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 +++++++++++++++---- >> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> index 1b2ef28c42bd..1c37659d9d41 100644 >> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> @@ -168,10 +168,25 @@ >> #size-cells = <1>; >> ranges; >> >> - syscon: syscon@1c00000 { >> - compatible = "allwinner,sun50i-a64-system-controller", >> - "syscon"; >> + sram_controller: sram-controller@1c00000 { >> + compatible = "allwinner,sun50i-a64-sram-controller"; > > I don't think there's anything preventing us from keeping the > -system-controller compatible. It's what was in the DT before, and > it's how it's called in the datasheet. I actually meant to ask you about this. The -system-controller compatible matches the datasheet better. Maybe we should just switch to that one? ChenYu > Otherwise, the whole serie looks good to me: > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > Maxime > > -- > Maxime Ripard, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com -- 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年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到: >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard ><maxime.ripard@bootlin.com> wrote: >> Hi, >> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >>> --- >>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 >+++++++++++++++---- >>> 1 file changed, 19 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>> index 1b2ef28c42bd..1c37659d9d41 100644 >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >>> @@ -168,10 +168,25 @@ >>> #size-cells = <1>; >>> ranges; >>> >>> - syscon: syscon@1c00000 { >>> - compatible = >"allwinner,sun50i-a64-system-controller", >>> - "syscon"; >>> + sram_controller: sram-controller@1c00000 { >>> + compatible = >"allwinner,sun50i-a64-sram-controller"; >> >> I don't think there's anything preventing us from keeping the >> -system-controller compatible. It's what was in the DT before, and >> it's how it's called in the datasheet. > >I actually meant to ask you about this. The -system-controller >compatible >matches the datasheet better. Maybe we should just switch to that one? No, if we do the switch the system-controller compatible, the device will be probed on the same memory region with a syscon on old DTs. > >ChenYu > >> Otherwise, the whole serie looks good to me: >> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> >> >> Maxime >> >> -- >> Maxime Ripard, Bootlin (formerly Free Electrons) >> Embedded Linux and Kernel engineering >> https://bootlin.com > >_______________________________________________ >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 Wed, May 02, 2018 at 06:19:51PM +0800, Icenowy Zheng wrote: > > > 于 2018年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到: > >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard > ><maxime.ripard@bootlin.com> wrote: > >> Hi, > >> > >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> > >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >>> --- > >>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 > >+++++++++++++++---- > >>> 1 file changed, 19 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >>> index 1b2ef28c42bd..1c37659d9d41 100644 > >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >>> @@ -168,10 +168,25 @@ > >>> #size-cells = <1>; > >>> ranges; > >>> > >>> - syscon: syscon@1c00000 { > >>> - compatible = > >"allwinner,sun50i-a64-system-controller", > >>> - "syscon"; > >>> + sram_controller: sram-controller@1c00000 { > >>> + compatible = > >"allwinner,sun50i-a64-sram-controller"; > >> > >> I don't think there's anything preventing us from keeping the > >> -system-controller compatible. It's what was in the DT before, and > >> it's how it's called in the datasheet. > > > >I actually meant to ask you about this. The -system-controller > >compatible matches the datasheet better. Maybe we should just > >switch to that one? > > No, if we do the switch the system-controller compatible, > the device will be probed on the same memory region with > a syscon on old DTs. The device hasn't magically changed either. Maybe we just need to add a check to make sure we don't have the syscon compatible in the SRAM driver probe so that the double driver issue doesn't happen? Maxime
From: Chen-Yu Tsai <wens@csie.org> Date: Wed, 2 May 2018 00:12:12 +0800 > This is v2 of my R40 Ethernet support series. There are a lot of non-networking changes in here, in particular DTS changes and modifications to clock drivers. Are all parties involved OK with the entire series going into net-next? Thanks. -- 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
From: Chen-Yu Tsai <wens@csie.org> Date: Wed, 2 May 2018 00:33:45 +0800 > I should've mentioned that patches 3 ~ 10, and only these, should go > through net-next. sunxi will handle the remaining clk, device tree, and > soc driver patches. Ok, I just noticed this. Why don't you just post those patches separately as a series on their own then, in order to avoid confusion? Then you can adjust the patch series header posting to explain the non-net-next changes, where they got merged, and what they provide in order to faciliate the net-next changes. Thanks. -- 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
Hi Dave, On Wed, May 02, 2018 at 11:06:17AM -0400, David Miller wrote: > From: Chen-Yu Tsai <wens@csie.org> > Date: Wed, 2 May 2018 00:33:45 +0800 > > > I should've mentioned that patches 3 ~ 10, and only these, should go > > through net-next. sunxi will handle the remaining clk, device tree, and > > soc driver patches. > > Ok, I just noticed this. > > Why don't you just post those patches separately as a series on their > own then, in order to avoid confusion? > > Then you can adjust the patch series header posting to explain the > non-net-next changes, where they got merged, and what they provide > in order to faciliate the net-next changes. I now that we usually have some feedback from non-net maintainers that they actually prefer seeing the full picture (and I also tend to prefer that as well) and having all the patches relevant to enable a particular feature, even if it means getting multiple maintainers involved. Just to make sure we understood you fully, do you want Chen-Yu to resend his serie following your comments, or was that just a general remark for next time? Thanks! Maxime
From: Maxime Ripard <maxime.ripard@bootlin.com> Date: Thu, 3 May 2018 15:12:57 +0200 > Hi Dave, > > On Wed, May 02, 2018 at 11:06:17AM -0400, David Miller wrote: >> From: Chen-Yu Tsai <wens@csie.org> >> Date: Wed, 2 May 2018 00:33:45 +0800 >> >> > I should've mentioned that patches 3 ~ 10, and only these, should go >> > through net-next. sunxi will handle the remaining clk, device tree, and >> > soc driver patches. >> >> Ok, I just noticed this. >> >> Why don't you just post those patches separately as a series on their >> own then, in order to avoid confusion? >> >> Then you can adjust the patch series header posting to explain the >> non-net-next changes, where they got merged, and what they provide >> in order to faciliate the net-next changes. > > I now that we usually have some feedback from non-net maintainers that > they actually prefer seeing the full picture (and I also tend to > prefer that as well) and having all the patches relevant to enable a > particular feature, even if it means getting multiple maintainers > involved. > > Just to make sure we understood you fully, do you want Chen-Yu to > resend his serie following your comments, or was that just a general > remark for next time? Yeah, good questions. I think it can be argued either way. For review having the complete context is important. But from a maintainer's standpoint, when there is any ambiguity whatsoever about what patches go into this tree or that, it is really frowned upon and is quite error prone. Also, that header posting is _SO_ important. It explains the series. But for these 'partial apply' situations the header posting refers to patches not in the series. This looks terrible in the logs, when, as I do, the header posting text is added to a marge commit for the series. People will read it and say "where are all of these other changes mentioned in the text? was this series misapplied?" That's why, maybe after the review is successful, I want the actual patch series standalone with appropriately updated header posting text. -- 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, May 03, 2018 at 02:40:42PM -0400, David Miller wrote: > From: Maxime Ripard <maxime.ripard@bootlin.com> > Date: Thu, 3 May 2018 15:12:57 +0200 > > > Hi Dave, > > > > On Wed, May 02, 2018 at 11:06:17AM -0400, David Miller wrote: > >> From: Chen-Yu Tsai <wens@csie.org> > >> Date: Wed, 2 May 2018 00:33:45 +0800 > >> > >> > I should've mentioned that patches 3 ~ 10, and only these, should go > >> > through net-next. sunxi will handle the remaining clk, device tree, and > >> > soc driver patches. > >> > >> Ok, I just noticed this. > >> > >> Why don't you just post those patches separately as a series on their > >> own then, in order to avoid confusion? > >> > >> Then you can adjust the patch series header posting to explain the > >> non-net-next changes, where they got merged, and what they provide > >> in order to faciliate the net-next changes. > > > > I now that we usually have some feedback from non-net maintainers that > > they actually prefer seeing the full picture (and I also tend to > > prefer that as well) and having all the patches relevant to enable a > > particular feature, even if it means getting multiple maintainers > > involved. > > > > Just to make sure we understood you fully, do you want Chen-Yu to > > resend his serie following your comments, or was that just a general > > remark for next time? > > Yeah, good questions. > > I think it can be argued either way. For review having the complete > context is important. > > But from a maintainer's standpoint, when there is any ambiguity > whatsoever about what patches go into this tree or that, it is really > frowned upon and is quite error prone. > > Also, that header posting is _SO_ important. It explains the series. > But for these 'partial apply' situations the header posting refers > to patches not in the series. > > This looks terrible in the logs, when, as I do, the header posting > text is added to a marge commit for the series. People will read it > and say "where are all of these other changes mentioned in the text? > was this series misapplied?" > > That's why, maybe after the review is successful, I want the actual > patch series standalone with appropriately updated header posting > text. Ok, thanks for the explanation, that makes sense :) Maxime
On Wed, May 2, 2018 at 4:54 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Wed, May 02, 2018 at 06:19:51PM +0800, Icenowy Zheng wrote: >> >> >> 于 2018年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到: >> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard >> ><maxime.ripard@bootlin.com> wrote: >> >> Hi, >> >> >> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> >> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >>> --- >> >>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 >> >+++++++++++++++---- >> >>> 1 file changed, 19 insertions(+), 4 deletions(-) >> >>> >> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >>> index 1b2ef28c42bd..1c37659d9d41 100644 >> >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >>> @@ -168,10 +168,25 @@ >> >>> #size-cells = <1>; >> >>> ranges; >> >>> >> >>> - syscon: syscon@1c00000 { >> >>> - compatible = >> >"allwinner,sun50i-a64-system-controller", >> >>> - "syscon"; >> >>> + sram_controller: sram-controller@1c00000 { >> >>> + compatible = >> >"allwinner,sun50i-a64-sram-controller"; >> >> >> >> I don't think there's anything preventing us from keeping the >> >> -system-controller compatible. It's what was in the DT before, and >> >> it's how it's called in the datasheet. >> > >> >I actually meant to ask you about this. The -system-controller >> >compatible matches the datasheet better. Maybe we should just >> >switch to that one? >> >> No, if we do the switch the system-controller compatible, >> the device will be probed on the same memory region with >> a syscon on old DTs. > > The device hasn't magically changed either. Maybe we just need to add > a check to make sure we don't have the syscon compatible in the SRAM > driver probe so that the double driver issue doesn't happen? The syscon interface (which is not even a full blown device driver) only looks at the "syscon" compatible. Either way we're removing that part from the device tree so things should be ok for new device trees. As Maxime mentioned we can do a check for the syscon compatible and either give a warning to the user asking them to update their device tree, or not register our custom regmap, or not probe the SRAM driver. Personally I prefer the first option. The system controller block is probed before any syscon users, so we should be fine, given the dwmac driver goes the custom regmap path first. BTW, I still might end up changing the compatible. The manual uses "system control", not "system controller", which I think makes sense, since it is just a bunch of register files, kind of like the GRF (General Register Files) block found in Rockchip SoCs [1], and not an actual "controller". ChenYu [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/soc/rockchip/grf.txt -- 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
1;5201;0c On Sun, May 13, 2018 at 12:37:49PM -0700, Chen-Yu Tsai wrote: > On Wed, May 2, 2018 at 4:54 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Wed, May 02, 2018 at 06:19:51PM +0800, Icenowy Zheng wrote: > >> > >> > >> 于 2018年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到: > >> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard > >> ><maxime.ripard@bootlin.com> wrote: > >> >> Hi, > >> >> > >> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> > >> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> >>> --- > >> >>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 > >> >+++++++++++++++---- > >> >>> 1 file changed, 19 insertions(+), 4 deletions(-) > >> >>> > >> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >>> index 1b2ef28c42bd..1c37659d9d41 100644 > >> >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >>> @@ -168,10 +168,25 @@ > >> >>> #size-cells = <1>; > >> >>> ranges; > >> >>> > >> >>> - syscon: syscon@1c00000 { > >> >>> - compatible = > >> >"allwinner,sun50i-a64-system-controller", > >> >>> - "syscon"; > >> >>> + sram_controller: sram-controller@1c00000 { > >> >>> + compatible = > >> >"allwinner,sun50i-a64-sram-controller"; > >> >> > >> >> I don't think there's anything preventing us from keeping the > >> >> -system-controller compatible. It's what was in the DT before, and > >> >> it's how it's called in the datasheet. > >> > > >> >I actually meant to ask you about this. The -system-controller > >> >compatible matches the datasheet better. Maybe we should just > >> >switch to that one? > >> > >> No, if we do the switch the system-controller compatible, > >> the device will be probed on the same memory region with > >> a syscon on old DTs. > > > > The device hasn't magically changed either. Maybe we just need to add > > a check to make sure we don't have the syscon compatible in the SRAM > > driver probe so that the double driver issue doesn't happen? > > The syscon interface (which is not even a full blown device driver) > only looks at the "syscon" compatible. Either way we're removing that > part from the device tree so things should be ok for new device trees. > As Maxime mentioned we can do a check for the syscon compatible and > either give a warning to the user asking them to update their device > tree, or not register our custom regmap, or not probe the SRAM driver. > Personally I prefer the first option. The system controller block is > probed before any syscon users, so we should be fine, given the dwmac > driver goes the custom regmap path first. > > BTW, I still might end up changing the compatible. The manual uses > "system control", not "system controller", which I think makes sense, > since it is just a bunch of register files, kind of like the GRF > (General Register Files) block found in Rockchip SoCs [1], and not an > actual "controller". I'm not really fond of that, but we should at least make it consistent on the other patches Paul sent then. Maxime
On Mon, May 14, 2018 at 1:03 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > 1;5201;0c > On Sun, May 13, 2018 at 12:37:49PM -0700, Chen-Yu Tsai wrote: >> On Wed, May 2, 2018 at 4:54 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: >> > On Wed, May 02, 2018 at 06:19:51PM +0800, Icenowy Zheng wrote: >> >> >> >> >> >> 于 2018年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到: >> >> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard >> >> ><maxime.ripard@bootlin.com> wrote: >> >> >> Hi, >> >> >> >> >> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> >> >> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >> >>> --- >> >> >>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 >> >> >+++++++++++++++---- >> >> >>> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> >>> >> >> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >> >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >> >>> index 1b2ef28c42bd..1c37659d9d41 100644 >> >> >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >> >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi >> >> >>> @@ -168,10 +168,25 @@ >> >> >>> #size-cells = <1>; >> >> >>> ranges; >> >> >>> >> >> >>> - syscon: syscon@1c00000 { >> >> >>> - compatible = >> >> >"allwinner,sun50i-a64-system-controller", >> >> >>> - "syscon"; >> >> >>> + sram_controller: sram-controller@1c00000 { >> >> >>> + compatible = >> >> >"allwinner,sun50i-a64-sram-controller"; >> >> >> >> >> >> I don't think there's anything preventing us from keeping the >> >> >> -system-controller compatible. It's what was in the DT before, and >> >> >> it's how it's called in the datasheet. >> >> > >> >> >I actually meant to ask you about this. The -system-controller >> >> >compatible matches the datasheet better. Maybe we should just >> >> >switch to that one? >> >> >> >> No, if we do the switch the system-controller compatible, >> >> the device will be probed on the same memory region with >> >> a syscon on old DTs. >> > >> > The device hasn't magically changed either. Maybe we just need to add >> > a check to make sure we don't have the syscon compatible in the SRAM >> > driver probe so that the double driver issue doesn't happen? >> >> The syscon interface (which is not even a full blown device driver) >> only looks at the "syscon" compatible. Either way we're removing that >> part from the device tree so things should be ok for new device trees. >> As Maxime mentioned we can do a check for the syscon compatible and >> either give a warning to the user asking them to update their device >> tree, or not register our custom regmap, or not probe the SRAM driver. >> Personally I prefer the first option. The system controller block is >> probed before any syscon users, so we should be fine, given the dwmac >> driver goes the custom regmap path first. >> >> BTW, I still might end up changing the compatible. The manual uses >> "system control", not "system controller", which I think makes sense, >> since it is just a bunch of register files, kind of like the GRF >> (General Register Files) block found in Rockchip SoCs [1], and not an >> actual "controller". > > I'm not really fond of that, but we should at least make it consistent > on the other patches Paul sent then. For the A10s / A13 right? I think my naming is slightly better, but it's just a minor detail. While we're still debating this, can I merge the R40 stuff first? The driver bits are already in. 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
Hi, On Tue, May 15, 2018 at 11:47:16PM -0700, Chen-Yu Tsai wrote: > On Mon, May 14, 2018 at 1:03 AM, Maxime Ripard > <maxime.ripard@bootlin.com> wrote: > > 1;5201;0c > > On Sun, May 13, 2018 at 12:37:49PM -0700, Chen-Yu Tsai wrote: > >> On Wed, May 2, 2018 at 4:54 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > >> > On Wed, May 02, 2018 at 06:19:51PM +0800, Icenowy Zheng wrote: > >> >> > >> >> > >> >> 于 2018年5月2日 GMT+08:00 下午5:53:21, Chen-Yu Tsai <wens@csie.org> 写到: > >> >> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard > >> >> ><maxime.ripard@bootlin.com> wrote: > >> >> >> Hi, > >> >> >> > >> >> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai 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> > >> >> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> >> >>> --- > >> >> >>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 > >> >> >+++++++++++++++---- > >> >> >>> 1 file changed, 19 insertions(+), 4 deletions(-) > >> >> >>> > >> >> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >> >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >> >>> index 1b2ef28c42bd..1c37659d9d41 100644 > >> >> >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >> >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > >> >> >>> @@ -168,10 +168,25 @@ > >> >> >>> #size-cells = <1>; > >> >> >>> ranges; > >> >> >>> > >> >> >>> - syscon: syscon@1c00000 { > >> >> >>> - compatible = > >> >> >"allwinner,sun50i-a64-system-controller", > >> >> >>> - "syscon"; > >> >> >>> + sram_controller: sram-controller@1c00000 { > >> >> >>> + compatible = > >> >> >"allwinner,sun50i-a64-sram-controller"; > >> >> >> > >> >> >> I don't think there's anything preventing us from keeping the > >> >> >> -system-controller compatible. It's what was in the DT before, and > >> >> >> it's how it's called in the datasheet. > >> >> > > >> >> >I actually meant to ask you about this. The -system-controller > >> >> >compatible matches the datasheet better. Maybe we should just > >> >> >switch to that one? > >> >> > >> >> No, if we do the switch the system-controller compatible, > >> >> the device will be probed on the same memory region with > >> >> a syscon on old DTs. > >> > > >> > The device hasn't magically changed either. Maybe we just need to add > >> > a check to make sure we don't have the syscon compatible in the SRAM > >> > driver probe so that the double driver issue doesn't happen? > >> > >> The syscon interface (which is not even a full blown device driver) > >> only looks at the "syscon" compatible. Either way we're removing that > >> part from the device tree so things should be ok for new device trees. > >> As Maxime mentioned we can do a check for the syscon compatible and > >> either give a warning to the user asking them to update their device > >> tree, or not register our custom regmap, or not probe the SRAM driver. > >> Personally I prefer the first option. The system controller block is > >> probed before any syscon users, so we should be fine, given the dwmac > >> driver goes the custom regmap path first. > >> > >> BTW, I still might end up changing the compatible. The manual uses > >> "system control", not "system controller", which I think makes sense, > >> since it is just a bunch of register files, kind of like the GRF > >> (General Register Files) block found in Rockchip SoCs [1], and not an > >> actual "controller". > > > > I'm not really fond of that, but we should at least make it consistent > > on the other patches Paul sent then. > > For the A10s / A13 right? And A33, yep. > I think my naming is slightly better, but it's just a minor detail. Let's do this then. > While we're still debating this, can I merge the R40 stuff first? > The driver bits are already in. Yep, go ahead. Maxime
Hi everyone, This is v2 of my R40 Ethernet support series. Changes since v1: - Default to fetching regmap from device pointed to by syscon phandle, and falling back to syscon API if that fails. - Dropped .syscon_from_dev field in device data as a result of the previous change. - Added a large comment block explaining the first change. - Simplified description of syscon property in sun8i-dwmac binding. - Regmap now only exposes the EMAC/GMAC register, but retains the offset within its address space. - Added patches for A64, which reuse the same sun8i-dwmac changes. This series adds support for the DWMAC based Ethernet controller found on the Allwinner R40 SoC. The controller is either a DWMAC clone or DWMAC core with its registers rearranged. This is already supported by the dwmac-sun8i driver. The glue layer control registers, unlike other sun8i family SoCs, is not in the system controller region, but in the clock control unit, like with the older A20 and A31 SoCs. While we reuse the bindings for dwmac-sun8i using a syscon phandle reference, we need some custom plumbing for the clock driver to export a regmap that only allows access to the GMAC register to the dwmac-sun8i driver. An alternative would be to allow drivers to register custom syscon devices with their own regmap and locking. Patch 1 converts the CLK_OF_DECLARE style clock driver to a platform one, so the regmap introduced later has a struct device to tie to. Patch 2 adds a regmap that is exported by the clock driver for the dwmac-sun8i driver to use. Patches 3, 4, and 5 clean up the dwmac-sun8i binding. Patch 6 adds device tree binding for Allwinner R40's Ethernet controller. Patch 7 converts regmap access of the syscon region in the dwmac-sun8i driver to regmap_field, in anticipation of different field widths on the R40. Patch 8 introduces custom plumbing in the dwmac-sun8i driver to fetch a regmap from another device, by looking up said device via a phandle, then getting the regmap associated with that device. Patch 9 adds support for different or absent TX/RX delay chain ranges to the dwmac-sun8i driver. Patch 10 adds support for the R40's ethernet controller. Patch 11 cleans up the Bananapi M2 Ultra device tree file. Patch 12 adds a GMAC device node and RGMII mode pinmux setting for the R40. Patch 13 enables Ethernet on the Bananapi M2 Ultra. Patches 14 and 15 are for the A64. They convert the existing syscon device to an SRAM controller device that exports a regmap. The needed driver changes are in patch 14, and the device tree changes are in patch 15. Please have a look. Regards ChenYu Chen-Yu Tsai (11): dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order dt-bindings: net: dwmac-sun8i: simplify description of syscon property dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40 SoC net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external device net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay chains net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC ARM: dts: sun8i: r40: bananapi-m2-ultra: Sort device node dereferences ARM: dts: sun8i: r40: Add device node and RGMII pinmux node for GMAC ARM: dts: sun8i: r40: bananapi-m2-ultra: Enable GMAC ethernet controller Icenowy Zheng (4): clk: sunxi-ng: r40: rewrite init code to a platform driver clk: sunxi-ng: r40: export a regmap to access the GMAC register soc: sunxi: export a regmap for EMAC clock reg on A64 arm64: dts: allwinner: a64: add SRAM controller device tree node .../devicetree/bindings/net/dwmac-sun8i.txt | 21 +-- .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 99 ++++++++----- arch/arm/boot/dts/sun8i-r40.dtsi | 34 +++++ arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 ++- drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 72 +++++++-- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 139 +++++++++++++++--- drivers/soc/sunxi/sunxi_sram.c | 57 ++++++- 7 files changed, 364 insertions(+), 81 deletions(-)