* [PATCH 0/5] Add support in dwmac-sun8i for accessing EMAC clock @ 2018-04-11 14:16 Icenowy Zheng [not found] ` <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:16 ` [PATCH 5/5] arm64: allwinner: a64: add SRAM controller device tree node Icenowy Zheng 0 siblings, 2 replies; 22+ messages in thread From: Icenowy Zheng @ 2018-04-11 14:16 UTC (permalink / raw) To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng On some Allwinner SoCs, the EMAC clock register is in another device's emory space, e.g. on A64 it's in the memory space of SRAM controller. This patchset adds the possibility for the device to export the EMAC clock register as a single-register regmap. PATCH 1 adds the device tree binding for dwmac-sun8i to use another device's regmap. PATCH 2 and 3 are dwmac-sun8i refactors. PATCH 4 exports the EMAC clock regmap in the SRAM controller driver. PATCH 5 enable SRAM controller in the A64 device tree (replaces syscon), and bind dwmac-sun8i to it. Chen-Yu Tsai (2): net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device Icenowy Zheng (3): dt-bindings: allow dwmac-sun8i to use other devices' exported regmap drivers: soc: sunxi: export a regmap for EMAC clock reg on A64 arm64: allwinner: a64: add SRAM controller device tree node .../devicetree/bindings/net/dwmac-sun8i.txt | 5 +- arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 +++++- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 85 +++++++++++++++++++--- drivers/soc/sunxi/sunxi_sram.c | 48 +++++++++++- 4 files changed, 141 insertions(+), 20 deletions(-) -- 2.15.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org>]
* [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap [not found] ` <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org> @ 2018-04-11 14:16 ` Icenowy Zheng [not found] ` <20180411141641.14675-2-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:16 ` [PATCH 2/5] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access Icenowy Zheng ` (2 subsequent siblings) 3 siblings, 1 reply; 22+ messages in thread From: Icenowy Zheng @ 2018-04-11 14:16 UTC (permalink / raw) To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng On some Allwinner SoCs the EMAC clock register needed by dwmac-sun8i is in another device's memory space. In this situation dwmac-sun8i can use a regmap exported by the other device with only the EMAC clock register. Document this situation in the dwmac-sun8i device tree binding documentation. Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> --- Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt index 3d6d5fa0c4d5..0c5f63a80617 100644 --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt @@ -20,8 +20,9 @@ Required properties: - phy-handle: See ethernet.txt - #address-cells: shall be 1 - #size-cells: shall be 0 -- syscon: A phandle to the syscon of the SoC with one of the following - compatible string: +- syscon: A phandle to a device which exports the EMAC clock register as a + regmap or to the syscon of the SoC with one of the following compatible + string: - allwinner,sun8i-h3-system-controller - allwinner,sun8i-v3s-system-controller - allwinner,sun50i-a64-system-controller -- 2.15.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
[parent not found: <20180411141641.14675-2-icenowy-h8G6r0blFSE@public.gmane.org>]
* Re: [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap [not found] ` <20180411141641.14675-2-icenowy-h8G6r0blFSE@public.gmane.org> @ 2018-04-11 14:36 ` Brüns, Stefan 2018-04-16 18:47 ` Rob Herring 1 sibling, 0 replies; 22+ messages in thread From: Brüns, Stefan @ 2018-04-11 14:36 UTC (permalink / raw) To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, icenowy-h8G6r0blFSE Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe, netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Mittwoch, 11. April 2018 16:16:37 CEST Icenowy Zheng wrote: > On some Allwinner SoCs the EMAC clock register needed by dwmac-sun8i is > in another device's memory space. In this situation dwmac-sun8i can use > a regmap exported by the other device with only the EMAC clock register. > > Document this situation in the dwmac-sun8i device tree binding > documentation. > > Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> > --- > Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt index > 3d6d5fa0c4d5..0c5f63a80617 100644 > --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > @@ -20,8 +20,9 @@ Required properties: > - phy-handle: See ethernet.txt > - #address-cells: shall be 1 > - #size-cells: shall be 0 > -- syscon: A phandle to the syscon of the SoC with one of the following > - compatible string: > +- syscon: A phandle to a device which exports the EMAC clock register as a > + regmap or to the syscon of the SoC with one of the following compatible ... regmap, or ... > + string: string_s_: > - allwinner,sun8i-h3-system-controller > - allwinner,sun8i-v3s-system-controller > - allwinner,sun50i-a64-system-controller Kind regards, Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap [not found] ` <20180411141641.14675-2-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:36 ` Brüns, Stefan @ 2018-04-16 18:47 ` Rob Herring 2018-04-16 23:17 ` Icenowy Zheng 1 sibling, 1 reply; 22+ messages in thread From: Rob Herring @ 2018-04-16 18:47 UTC (permalink / raw) To: Icenowy Zheng Cc: Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe, netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw On Wed, Apr 11, 2018 at 10:16:37PM +0800, Icenowy Zheng wrote: > On some Allwinner SoCs the EMAC clock register needed by dwmac-sun8i is > in another device's memory space. In this situation dwmac-sun8i can use > a regmap exported by the other device with only the EMAC clock register. If this is a clock, then why not use the clock binding? > > Document this situation in the dwmac-sun8i device tree binding > documentation. > > Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> > --- > Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > index 3d6d5fa0c4d5..0c5f63a80617 100644 > --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt > @@ -20,8 +20,9 @@ Required properties: > - phy-handle: See ethernet.txt > - #address-cells: shall be 1 > - #size-cells: shall be 0 > -- syscon: A phandle to the syscon of the SoC with one of the following > - compatible string: > +- syscon: A phandle to a device which exports the EMAC clock register as a > + regmap or to the syscon of the SoC with one of the following compatible > + string: > - allwinner,sun8i-h3-system-controller > - allwinner,sun8i-v3s-system-controller > - allwinner,sun50i-a64-system-controller > -- > 2.15.1 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap 2018-04-16 18:47 ` Rob Herring @ 2018-04-16 23:17 ` Icenowy Zheng [not found] ` <04C9F795-2680-4220-A39A-7B7D5FD74C4A-h8G6r0blFSE@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Icenowy Zheng @ 2018-04-16 23:17 UTC (permalink / raw) To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, netdev-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Corentin Labbe, Giuseppe Cavallaro 于 2018年4月17日 GMT+08:00 上午2:47:45, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 写到: >On Wed, Apr 11, 2018 at 10:16:37PM +0800, Icenowy Zheng wrote: >> On some Allwinner SoCs the EMAC clock register needed by dwmac-sun8i >is >> in another device's memory space. In this situation dwmac-sun8i can >use >> a regmap exported by the other device with only the EMAC clock >register. > >If this is a clock, then why not use the clock binding? EMAC clock register is only the datasheet name. It contains MII mode selection and delay chain configuration. > >> >> Document this situation in the dwmac-sun8i device tree binding >> documentation. >> >> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >> --- >> Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >> index 3d6d5fa0c4d5..0c5f63a80617 100644 >> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >> @@ -20,8 +20,9 @@ Required properties: >> - phy-handle: See ethernet.txt >> - #address-cells: shall be 1 >> - #size-cells: shall be 0 >> -- syscon: A phandle to the syscon of the SoC with one of the >following >> - compatible string: >> +- syscon: A phandle to a device which exports the EMAC clock >register as a >> + regmap or to the syscon of the SoC with one of the following >compatible >> + string: >> - allwinner,sun8i-h3-system-controller >> - allwinner,sun8i-v3s-system-controller >> - allwinner,sun50i-a64-system-controller >> -- >> 2.15.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe devicetree" >in >> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <04C9F795-2680-4220-A39A-7B7D5FD74C4A-h8G6r0blFSE@public.gmane.org>]
* Re: Re: [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap [not found] ` <04C9F795-2680-4220-A39A-7B7D5FD74C4A-h8G6r0blFSE@public.gmane.org> @ 2018-04-28 13:42 ` Chen-Yu Tsai 0 siblings, 0 replies; 22+ messages in thread From: Chen-Yu Tsai @ 2018-04-28 13:42 UTC (permalink / raw) To: Rob Herring Cc: linux-arm-kernel, devicetree, Maxime Ripard, netdev, linux-sunxi, linux-kernel, Corentin Labbe, Giuseppe Cavallaro, Icenowy Zheng Hi Rob, On Tue, Apr 17, 2018 at 7:17 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote: > > > 于 2018年4月17日 GMT+08:00 上午2:47:45, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 写到: >>On Wed, Apr 11, 2018 at 10:16:37PM +0800, Icenowy Zheng wrote: >>> On some Allwinner SoCs the EMAC clock register needed by dwmac-sun8i >>is >>> in another device's memory space. In this situation dwmac-sun8i can >>use >>> a regmap exported by the other device with only the EMAC clock >>register. >> >>If this is a clock, then why not use the clock binding? > > EMAC clock register is only the datasheet name. It contains > MII mode selection and delay chain configuration. As Icenowy already mentioned, this is likely a misnomer. The register contains controls on how to route the TX and RX clock lines, and also what interface mode to use. The former includes things like the delays mentioned in the device tree binding, and also whether to invert the signals or not. The latter influences whether the TXC line is an input or an output (or maybe what decoding module to send all the signals to). On the H3/H5, it even contains controls for the embedded PHY. The settings only make sense to the MAC. To expose it as a generic clock line would not be a good fit. You can look at what we did for sun7i-a20-gmac, which is not pretty. All other DWMAC platforms that were introduced after sun7i-a20-gmac also use a syscon, instead of clocks, even though they probably cover the same set of RXC/TXC controls. ChenYu >> >>> >>> Document this situation in the dwmac-sun8i device tree binding >>> documentation. >>> >>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >>> --- >>> Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >>b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >>> index 3d6d5fa0c4d5..0c5f63a80617 100644 >>> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >>> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt >>> @@ -20,8 +20,9 @@ Required properties: >>> - phy-handle: See ethernet.txt >>> - #address-cells: shall be 1 >>> - #size-cells: shall be 0 >>> -- syscon: A phandle to the syscon of the SoC with one of the >>following >>> - compatible string: >>> +- syscon: A phandle to a device which exports the EMAC clock >>register as a >>> + regmap or to the syscon of the SoC with one of the following >>compatible >>> + string: >>> - allwinner,sun8i-h3-system-controller >>> - allwinner,sun8i-v3s-system-controller >>> - allwinner,sun50i-a64-system-controller >>> -- >>> 2.15.1 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe devicetree" >>in >>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >>_______________________________________________ >>linux-arm-kernel mailing list >>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org >>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access [not found] ` <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:16 ` [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap Icenowy Zheng @ 2018-04-11 14:16 ` Icenowy Zheng 2018-04-11 14:16 ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device Icenowy Zheng 2018-04-11 14:16 ` [PATCH 4/5] drivers: soc: sunxi: export a regmap for EMAC clock reg on A64 Icenowy Zheng 3 siblings, 0 replies; 22+ messages in thread From: Icenowy Zheng @ 2018-04-11 14:16 UTC (permalink / raw) To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> In several SoCs the EMAC register is in the range of another device, either the SRAM controller (e.g. A64) or the clock controlling unit (e.g. R40). In this situation we're going to let the device to export a regmap which contains only the EMAC register, for the dwmac-sun8i driver to manipulation this register. This patch converts the use of regmap to regmap_field for mapping and accessing the EMAC register, so we can have the register address based on the regmap class, and not in the actual register manipulation code. This patch only converts regmap_read() and regmap_write() calls to regmap_field_read() and regmap_field_write() calls. There are some places where it might make sense to switch to regmap_field_update_bits(), but this is not done here to keep the patch simple. Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> [Icenowy: decide reg_field based on regmap type, change commit message] Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 41 ++++++++++++++++------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index a3fa65b1ca8e..1037f6c78bca 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -61,9 +61,10 @@ struct emac_variant { * @regulator: reference to the optional regulator * @rst_ephy: reference to the optional EPHY reset for the internal PHY * @variant: reference to the current board variant - * @regmap: regmap for using the syscon + * @regmap_field: regmap field for the gmac register * @internal_phy_powered: Does the internal PHY is enabled * @mux_handle: Internal pointer used by mdio-mux lib + * @emac_reg_field: reg_field for the regmap's gmac register */ struct sunxi_priv_data { struct clk *tx_clk; @@ -71,9 +72,17 @@ struct sunxi_priv_data { struct regulator *regulator; struct reset_control *rst_ephy; const struct emac_variant *variant; - struct regmap *regmap; + struct regmap_field *regmap_field; bool internal_phy_powered; void *mux_handle; + const struct reg_field *emac_reg_field; +}; + +/* EMAC clock register @ 0x30 in the "system control" address range */ +const struct reg_field old_syscon_reg_field = { + .reg = 0x30, + .lsb = 0, + .msb = 31, }; static const struct emac_variant emac_variant_h3 = { @@ -216,7 +225,6 @@ static const struct emac_variant emac_variant_a64 = { #define SYSCON_ETCS_MII 0x0 #define SYSCON_ETCS_EXT_GMII 0x1 #define SYSCON_ETCS_INT_GMII 0x2 -#define SYSCON_EMAC_REG 0x30 /* sun8i_dwmac_dma_reset() - reset the EMAC * Called from stmmac via stmmac_dma_ops->reset @@ -745,7 +753,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child, bool need_power_ephy = false; if (current_child ^ desired_child) { - regmap_read(gmac->regmap, SYSCON_EMAC_REG, ®); + regmap_field_read(gmac->regmap_field, ®); switch (desired_child) { case DWMAC_SUN8I_MDIO_MUX_INTERNAL_ID: dev_info(priv->device, "Switch mux to internal PHY"); @@ -763,7 +771,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child, desired_child); return -EINVAL; } - regmap_write(gmac->regmap, SYSCON_EMAC_REG, val); + regmap_field_write(gmac->regmap_field, val); if (need_power_ephy) { ret = sun8i_dwmac_power_internal_phy(priv); if (ret) @@ -801,7 +809,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv) int ret; u32 reg, val; - regmap_read(gmac->regmap, SYSCON_EMAC_REG, &val); + regmap_field_read(gmac->regmap_field, &val); reg = gmac->variant->default_syscon_value; if (reg != val) dev_warn(priv->device, @@ -883,7 +891,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv) return -EINVAL; } - regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg); + regmap_field_write(gmac->regmap_field, reg); return 0; } @@ -892,7 +900,7 @@ static void sun8i_dwmac_unset_syscon(struct sunxi_priv_data *gmac) { u32 reg = gmac->variant->default_syscon_value; - regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg); + regmap_field_write(gmac->regmap_field, reg); } static void sun8i_dwmac_exit(struct platform_device *pdev, void *priv) @@ -980,6 +988,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) int ret; struct stmmac_priv *priv; struct net_device *ndev; + struct regmap *regmap; ret = stmmac_get_platform_resources(pdev, &stmmac_res); if (ret) @@ -1014,13 +1023,21 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) gmac->regulator = NULL; } - gmac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, - "syscon"); - if (IS_ERR(gmac->regmap)) { - ret = PTR_ERR(gmac->regmap); + regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon"); + if (IS_ERR(regmap)) { + ret = PTR_ERR(regmap); dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret); return ret; } + gmac->emac_reg_field = old_syscon_reg_field; + + gmac->regmap_field = devm_regmap_field_alloc(dev, regmap, + *gmac->emac_reg_field); + if (IS_ERR(gmac->regmap_field)) { + ret = PTR_ERR(gmac->regmap_field); + dev_err(dev, "Unable to map syscon register: %d\n", ret); + return ret; + } plat_dat->interface = of_get_phy_mode(dev->of_node); -- 2.15.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device [not found] ` <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:16 ` [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap Icenowy Zheng 2018-04-11 14:16 ` [PATCH 2/5] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access Icenowy Zheng @ 2018-04-11 14:16 ` Icenowy Zheng [not found] ` <20180411141641.14675-4-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:16 ` [PATCH 4/5] drivers: soc: sunxi: export a regmap for EMAC clock reg on A64 Icenowy Zheng 3 siblings, 1 reply; 22+ messages in thread From: Icenowy Zheng @ 2018-04-11 14:16 UTC (permalink / raw) To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU address space; on the A64 SoC this register is in the SRAM controller address space, and with a different offset. To access the register from another device and hide the internal difference between the device, let it register a regmap named "emac-clock". We can then get the device from the phandle, and retrieve the regmap with dev_get_regmap(); in this situation the regmap_field will be set up to access the only register in the regmap. Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> [Icenowy: change to use regmaps with single register, change commit message] Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 ++++++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 1037f6c78bca..b61210c0d415 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { .msb = 31, }; +/* Specially exported regmap which contains only EMAC register */ +const struct reg_field single_reg_field = { + .reg = 0, + .lsb = 0, + .msb = 31, +}; + static const struct emac_variant emac_variant_h3 = { .default_syscon_value = 0x58000, .soc_has_internal_phy = true, @@ -979,6 +986,44 @@ static struct mac_device_info *sun8i_dwmac_setup(void *ppriv) return mac; } +static struct regmap *sun8i_dwmac_get_emac_regmap(struct sunxi_priv_data *emac, + struct device_node *node) +{ + struct device_node *syscon_node; + struct platform_device *syscon_pdev = NULL; + struct regmap *regmap = NULL; + + syscon_node = of_parse_phandle(node, "syscon", 0); + if (!syscon_node) + return ERR_PTR(-ENODEV); + + if (of_device_is_compatible(syscon_node, "syscon")) { + regmap = syscon_regmap_lookup_by_phandle(node, "syscon"); + + emac->emac_reg_field = &old_syscon_reg_field; + } else { + syscon_pdev = of_find_device_by_node(syscon_node); + if (!syscon_pdev) { + /* platform device might not be probed yet */ + regmap = ERR_PTR(-EPROBE_DEFER); + goto out_put_node; + } + + /* If no regmap is found then trap into error */ + regmap = dev_get_regmap(&syscon_pdev->dev, "emac-clock"); + if (!regmap) + regmap = ERR_PTR(-EINVAL); + + emac->emac_reg_field = &single_reg_field; + } + + if (syscon_pdev) + platform_device_put(syscon_pdev); +out_put_node: + of_node_put(syscon_node); + return regmap; +} + static int sun8i_dwmac_probe(struct platform_device *pdev) { struct plat_stmmacenet_data *plat_dat; @@ -1023,13 +1068,12 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) gmac->regulator = NULL; } - regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon"); + regmap = sun8i_dwmac_get_emac_regmap(gmac, pdev->dev.of_node); if (IS_ERR(regmap)) { ret = PTR_ERR(regmap); dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret); return ret; } - gmac->emac_reg_field = old_syscon_reg_field; gmac->regmap_field = devm_regmap_field_alloc(dev, regmap, *gmac->emac_reg_field); -- 2.15.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
[parent not found: <20180411141641.14675-4-icenowy-h8G6r0blFSE@public.gmane.org>]
* Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device [not found] ` <20180411141641.14675-4-icenowy-h8G6r0blFSE@public.gmane.org> @ 2018-04-12 14:56 ` Maxime Ripard 2018-04-12 15:11 ` Icenowy Zheng 2018-04-13 6:03 ` [RFC PATCH] net: stmmac: dwmac-sun8i: single_reg_field can be static kbuild test robot 2018-04-13 6:03 ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device kbuild test robot 2 siblings, 1 reply; 22+ messages in thread From: Maxime Ripard @ 2018-04-12 14:56 UTC (permalink / raw) To: Icenowy Zheng Cc: Rob Herring, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe, netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw [-- Attachment #1: Type: text/plain, Size: 2021 bytes --] On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: > From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > > On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU > address space; on the A64 SoC this register is in the SRAM controller > address space, and with a different offset. > > To access the register from another device and hide the internal > difference between the device, let it register a regmap named > "emac-clock". We can then get the device from the phandle, and > retrieve the regmap with dev_get_regmap(); in this situation the > regmap_field will be set up to access the only register in the regmap. > > Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > [Icenowy: change to use regmaps with single register, change commit > message] > Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 ++++++++++++++++++++++- > 1 file changed, 46 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > index 1037f6c78bca..b61210c0d415 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { > .msb = 31, > }; > > +/* Specially exported regmap which contains only EMAC register */ > +const struct reg_field single_reg_field = { > + .reg = 0, > + .lsb = 0, > + .msb = 31, > +}; > + I'm not sure this would be wise. If we ever need some other register exported through the regmap, will have to change all the calling sites everywhere in the kernel, which will be a pain and will break bisectability. Chen-Yu's (or was it yours?) initial solution with a custom writeable hook only allowing a single register seemed like a better one. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device 2018-04-12 14:56 ` Maxime Ripard @ 2018-04-12 15:11 ` Icenowy Zheng [not found] ` <A027702A-F3F7-4896-B2B8-E024DA521661-h8G6r0blFSE@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Icenowy Zheng @ 2018-04-12 15:11 UTC (permalink / raw) To: Maxime Ripard Cc: Rob Herring, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe, netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: >On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: >> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> >> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU >> address space; on the A64 SoC this register is in the SRAM controller >> address space, and with a different offset. >> >> To access the register from another device and hide the internal >> difference between the device, let it register a regmap named >> "emac-clock". We can then get the device from the phandle, and >> retrieve the regmap with dev_get_regmap(); in this situation the >> regmap_field will be set up to access the only register in the >regmap. >> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> [Icenowy: change to use regmaps with single register, change commit >> message] >> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >> --- >> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 >++++++++++++++++++++++- >> 1 file changed, 46 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> index 1037f6c78bca..b61210c0d415 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { >> .msb = 31, >> }; >> >> +/* Specially exported regmap which contains only EMAC register */ >> +const struct reg_field single_reg_field = { >> + .reg = 0, >> + .lsb = 0, >> + .msb = 31, >> +}; >> + > >I'm not sure this would be wise. If we ever need some other register >exported through the regmap, will have to change all the calling sites >everywhere in the kernel, which will be a pain and will break >bisectability. In this situation the register can be exported as another regmap. Currently the code will access a regmap with name "emac-clock" for this register. > >Chen-Yu's (or was it yours?) initial solution with a custom writeable >hook only allowing a single register seemed like a better one. But I remember you mentioned that you want it to hide the difference inside the device. > >Maxime -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <A027702A-F3F7-4896-B2B8-E024DA521661-h8G6r0blFSE@public.gmane.org>]
* Re: Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device [not found] ` <A027702A-F3F7-4896-B2B8-E024DA521661-h8G6r0blFSE@public.gmane.org> @ 2018-04-12 15:23 ` Chen-Yu Tsai [not found] ` <CAGb2v66Qi2D1qgF3-iVJFAiznMUzzen0nHM5vsPe7Cyi6QTYpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Chen-Yu Tsai @ 2018-04-12 15:23 UTC (permalink / raw) To: Icenowy Zheng Cc: Maxime Ripard, Rob Herring, Giuseppe Cavallaro, Corentin Labbe, netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote: > > > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: >>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >>> >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU >>> address space; on the A64 SoC this register is in the SRAM controller >>> address space, and with a different offset. >>> >>> To access the register from another device and hide the internal >>> difference between the device, let it register a regmap named >>> "emac-clock". We can then get the device from the phandle, and >>> retrieve the regmap with dev_get_regmap(); in this situation the >>> regmap_field will be set up to access the only register in the >>regmap. >>> >>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >>> [Icenowy: change to use regmaps with single register, change commit >>> message] >>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >>> --- >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 >>++++++++++++++++++++++- >>> 1 file changed, 46 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>> index 1037f6c78bca..b61210c0d415 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { >>> .msb = 31, >>> }; >>> >>> +/* Specially exported regmap which contains only EMAC register */ >>> +const struct reg_field single_reg_field = { >>> + .reg = 0, >>> + .lsb = 0, >>> + .msb = 31, >>> +}; >>> + >> >>I'm not sure this would be wise. If we ever need some other register >>exported through the regmap, will have to change all the calling sites >>everywhere in the kernel, which will be a pain and will break >>bisectability. > > In this situation the register can be exported as another > regmap. Currently the code will access a regmap with name > "emac-clock" for this register. > >> >>Chen-Yu's (or was it yours?) initial solution with a custom writeable >>hook only allowing a single register seemed like a better one. > > But I remember you mentioned that you want it to hide the > difference inside the device. Hi, The idea is that a device can export multiple regmaps. This one, the one named "gmac" (in my soon to come v2) or "emac-clock" here, is but one of many possible regmaps, and it only exports the register needed by the GMAC/EMAC. IMHO it is highly unlikely the same piece of hardware would need a second register from the same device. A more likely situation would be it needs another register from a different device, like on the H6 where the "internal PHY" is not so internal from a system bus point of view. ChenYu -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAGb2v66Qi2D1qgF3-iVJFAiznMUzzen0nHM5vsPe7Cyi6QTYpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device [not found] ` <CAGb2v66Qi2D1qgF3-iVJFAiznMUzzen0nHM5vsPe7Cyi6QTYpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-04-16 14:31 ` Maxime Ripard 2018-04-16 14:34 ` Icenowy Zheng 2018-04-16 14:51 ` Chen-Yu Tsai 0 siblings, 2 replies; 22+ messages in thread From: Maxime Ripard @ 2018-04-16 14:31 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Icenowy Zheng, Rob Herring, Giuseppe Cavallaro, Corentin Labbe, netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 3899 bytes --] On Thu, Apr 12, 2018 at 11:23:30PM +0800, Chen-Yu Tsai wrote: > On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote: > > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: > >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: > >>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > >>> > >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU > >>> address space; on the A64 SoC this register is in the SRAM controller > >>> address space, and with a different offset. > >>> > >>> To access the register from another device and hide the internal > >>> difference between the device, let it register a regmap named > >>> "emac-clock". We can then get the device from the phandle, and > >>> retrieve the regmap with dev_get_regmap(); in this situation the > >>> regmap_field will be set up to access the only register in the > >>regmap. > >>> > >>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > >>> [Icenowy: change to use regmaps with single register, change commit > >>> message] > >>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> > >>> --- > >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 > >>++++++++++++++++++++++- > >>> 1 file changed, 46 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >>> index 1037f6c78bca..b61210c0d415 100644 > >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { > >>> .msb = 31, > >>> }; > >>> > >>> +/* Specially exported regmap which contains only EMAC register */ > >>> +const struct reg_field single_reg_field = { > >>> + .reg = 0, > >>> + .lsb = 0, > >>> + .msb = 31, > >>> +}; > >>> + > >> > >>I'm not sure this would be wise. If we ever need some other register > >>exported through the regmap, will have to change all the calling sites > >>everywhere in the kernel, which will be a pain and will break > >>bisectability. > > > > In this situation the register can be exported as another > > regmap. Currently the code will access a regmap with name > > "emac-clock" for this register. > > > >> > >>Chen-Yu's (or was it yours?) initial solution with a custom writeable > >>hook only allowing a single register seemed like a better one. > > > > But I remember you mentioned that you want it to hide the > > difference inside the device. > > The idea is that a device can export multiple regmaps. This one, > the one named "gmac" (in my soon to come v2) or "emac-clock" here, > is but one of many possible regmaps, and it only exports the register > needed by the GMAC/EMAC. I'm not sure this would be wise either. There's a single register map, and as far as I know we don't have a binding to express this in the DT. This means that the customer and provider would have to use the same name, but without anything actually enforcing it aside from "someone in the community knows it". This is not a really good design, and I was actually preferring your first option. We shouldn't rely on any undocumented rule. This will be easy to break and hard to maintain. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device 2018-04-16 14:31 ` Maxime Ripard @ 2018-04-16 14:34 ` Icenowy Zheng 2018-04-16 14:51 ` Chen-Yu Tsai 1 sibling, 0 replies; 22+ messages in thread From: Icenowy Zheng @ 2018-04-16 14:34 UTC (permalink / raw) To: Maxime Ripard, Chen-Yu Tsai Cc: Rob Herring, Giuseppe Cavallaro, Corentin Labbe, netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi 于 2018年4月16日 GMT+08:00 下午10:31:30, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: >On Thu, Apr 12, 2018 at 11:23:30PM +0800, Chen-Yu Tsai wrote: >> On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >wrote: >> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard ><maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: >> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: >> >>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> >>> >> >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU >> >>> address space; on the A64 SoC this register is in the SRAM >controller >> >>> address space, and with a different offset. >> >>> >> >>> To access the register from another device and hide the internal >> >>> difference between the device, let it register a regmap named >> >>> "emac-clock". We can then get the device from the phandle, and >> >>> retrieve the regmap with dev_get_regmap(); in this situation the >> >>> regmap_field will be set up to access the only register in the >> >>regmap. >> >>> >> >>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> >>> [Icenowy: change to use regmaps with single register, change >commit >> >>> message] >> >>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >> >>> --- >> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 >> >>++++++++++++++++++++++- >> >>> 1 file changed, 46 insertions(+), 2 deletions(-) >> >>> >> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>> index 1037f6c78bca..b61210c0d415 100644 >> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = >{ >> >>> .msb = 31, >> >>> }; >> >>> >> >>> +/* Specially exported regmap which contains only EMAC register >*/ >> >>> +const struct reg_field single_reg_field = { >> >>> + .reg = 0, >> >>> + .lsb = 0, >> >>> + .msb = 31, >> >>> +}; >> >>> + >> >> >> >>I'm not sure this would be wise. If we ever need some other >register >> >>exported through the regmap, will have to change all the calling >sites >> >>everywhere in the kernel, which will be a pain and will break >> >>bisectability. >> > >> > In this situation the register can be exported as another >> > regmap. Currently the code will access a regmap with name >> > "emac-clock" for this register. >> > >> >> >> >>Chen-Yu's (or was it yours?) initial solution with a custom >writeable >> >>hook only allowing a single register seemed like a better one. >> > >> > But I remember you mentioned that you want it to hide the >> > difference inside the device. >> >> The idea is that a device can export multiple regmaps. This one, >> the one named "gmac" (in my soon to come v2) or "emac-clock" here, >> is but one of many possible regmaps, and it only exports the register >> needed by the GMAC/EMAC. > >I'm not sure this would be wise either. There's a single register map, >and as far as I know we don't have a binding to express this in the >DT. This means that the customer and provider would have to use the >same name, but without anything actually enforcing it aside from >"someone in the community knows it". > >This is not a really good design, and I was actually preferring your >first option. We shouldn't rely on any undocumented rule. This will be >easy to break and hard to maintain. Okay. Then I will revert back to the original solution in the next version. > >Maxime -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device 2018-04-16 14:31 ` Maxime Ripard 2018-04-16 14:34 ` Icenowy Zheng @ 2018-04-16 14:51 ` Chen-Yu Tsai [not found] ` <CAGb2v65VLYc1X2bN5v0_1xokZCEkKFYNLzecnE3WeP8goc2KmQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 22+ messages in thread From: Chen-Yu Tsai @ 2018-04-16 14:51 UTC (permalink / raw) To: Maxime Ripard Cc: Icenowy Zheng, Rob Herring, Giuseppe Cavallaro, Corentin Labbe, netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Mon, Apr 16, 2018 at 10:31 PM, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > On Thu, Apr 12, 2018 at 11:23:30PM +0800, Chen-Yu Tsai wrote: >> On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote: >> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: >> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: >> >>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> >>> >> >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU >> >>> address space; on the A64 SoC this register is in the SRAM controller >> >>> address space, and with a different offset. >> >>> >> >>> To access the register from another device and hide the internal >> >>> difference between the device, let it register a regmap named >> >>> "emac-clock". We can then get the device from the phandle, and >> >>> retrieve the regmap with dev_get_regmap(); in this situation the >> >>> regmap_field will be set up to access the only register in the >> >>regmap. >> >>> >> >>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >> >>> [Icenowy: change to use regmaps with single register, change commit >> >>> message] >> >>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >> >>> --- >> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 >> >>++++++++++++++++++++++- >> >>> 1 file changed, 46 insertions(+), 2 deletions(-) >> >>> >> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>> index 1037f6c78bca..b61210c0d415 100644 >> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { >> >>> .msb = 31, >> >>> }; >> >>> >> >>> +/* Specially exported regmap which contains only EMAC register */ >> >>> +const struct reg_field single_reg_field = { >> >>> + .reg = 0, >> >>> + .lsb = 0, >> >>> + .msb = 31, >> >>> +}; >> >>> + >> >> >> >>I'm not sure this would be wise. If we ever need some other register >> >>exported through the regmap, will have to change all the calling sites >> >>everywhere in the kernel, which will be a pain and will break >> >>bisectability. >> > >> > In this situation the register can be exported as another >> > regmap. Currently the code will access a regmap with name >> > "emac-clock" for this register. >> > >> >> >> >>Chen-Yu's (or was it yours?) initial solution with a custom writeable >> >>hook only allowing a single register seemed like a better one. >> > >> > But I remember you mentioned that you want it to hide the >> > difference inside the device. >> >> The idea is that a device can export multiple regmaps. This one, >> the one named "gmac" (in my soon to come v2) or "emac-clock" here, >> is but one of many possible regmaps, and it only exports the register >> needed by the GMAC/EMAC. > > I'm not sure this would be wise either. There's a single register map, > and as far as I know we don't have a binding to express this in the > DT. This means that the customer and provider would have to use the > same name, but without anything actually enforcing it aside from > "someone in the community knows it". > > This is not a really good design, and I was actually preferring your > first option. We shouldn't rely on any undocumented rule. This will be > easy to break and hard to maintain. So, one regmap per device covering the whole register range, and the consumer knows which register to poke by looking at its own compatible. That sound right? ChenYu -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAGb2v65VLYc1X2bN5v0_1xokZCEkKFYNLzecnE3WeP8goc2KmQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device [not found] ` <CAGb2v65VLYc1X2bN5v0_1xokZCEkKFYNLzecnE3WeP8goc2KmQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-04-17 11:52 ` Maxime Ripard 2018-04-17 11:59 ` [linux-sunxi] " Chen-Yu Tsai 0 siblings, 1 reply; 22+ messages in thread From: Maxime Ripard @ 2018-04-17 11:52 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Icenowy Zheng, Rob Herring, Giuseppe Cavallaro, Corentin Labbe, netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 4600 bytes --] On Mon, Apr 16, 2018 at 10:51:55PM +0800, Chen-Yu Tsai wrote: > On Mon, Apr 16, 2018 at 10:31 PM, Maxime Ripard > <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: > > On Thu, Apr 12, 2018 at 11:23:30PM +0800, Chen-Yu Tsai wrote: > >> On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote: > >> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: > >> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: > >> >>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > >> >>> > >> >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU > >> >>> address space; on the A64 SoC this register is in the SRAM controller > >> >>> address space, and with a different offset. > >> >>> > >> >>> To access the register from another device and hide the internal > >> >>> difference between the device, let it register a regmap named > >> >>> "emac-clock". We can then get the device from the phandle, and > >> >>> retrieve the regmap with dev_get_regmap(); in this situation the > >> >>> regmap_field will be set up to access the only register in the > >> >>regmap. > >> >>> > >> >>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> > >> >>> [Icenowy: change to use regmaps with single register, change commit > >> >>> message] > >> >>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> > >> >>> --- > >> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 > >> >>++++++++++++++++++++++- > >> >>> 1 file changed, 46 insertions(+), 2 deletions(-) > >> >>> > >> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >> >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >> >>> index 1037f6c78bca..b61210c0d415 100644 > >> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > >> >>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { > >> >>> .msb = 31, > >> >>> }; > >> >>> > >> >>> +/* Specially exported regmap which contains only EMAC register */ > >> >>> +const struct reg_field single_reg_field = { > >> >>> + .reg = 0, > >> >>> + .lsb = 0, > >> >>> + .msb = 31, > >> >>> +}; > >> >>> + > >> >> > >> >>I'm not sure this would be wise. If we ever need some other register > >> >>exported through the regmap, will have to change all the calling sites > >> >>everywhere in the kernel, which will be a pain and will break > >> >>bisectability. > >> > > >> > In this situation the register can be exported as another > >> > regmap. Currently the code will access a regmap with name > >> > "emac-clock" for this register. > >> > > >> >> > >> >>Chen-Yu's (or was it yours?) initial solution with a custom writeable > >> >>hook only allowing a single register seemed like a better one. > >> > > >> > But I remember you mentioned that you want it to hide the > >> > difference inside the device. > >> > >> The idea is that a device can export multiple regmaps. This one, > >> the one named "gmac" (in my soon to come v2) or "emac-clock" here, > >> is but one of many possible regmaps, and it only exports the register > >> needed by the GMAC/EMAC. > > > > I'm not sure this would be wise either. There's a single register map, > > and as far as I know we don't have a binding to express this in the > > DT. This means that the customer and provider would have to use the > > same name, but without anything actually enforcing it aside from > > "someone in the community knows it". > > > > This is not a really good design, and I was actually preferring your > > first option. We shouldn't rely on any undocumented rule. This will be > > easy to break and hard to maintain. > > So, one regmap per device covering the whole register range, and the > consumer knows which register to poke by looking at its own compatible. > > That sound right? Yep. And ideally, sending a single serie for both the A64 and the R40 cases, in order to provide the big picture. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [linux-sunxi] Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device 2018-04-17 11:52 ` Maxime Ripard @ 2018-04-17 11:59 ` Chen-Yu Tsai [not found] ` <CAGb2v66e+tWsYqzpOnRaJtjq4OneUOruszYML0FnvAGbXi5qsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Chen-Yu Tsai @ 2018-04-17 11:59 UTC (permalink / raw) To: Maxime Ripard, Icenowy Zheng Cc: Rob Herring, Giuseppe Cavallaro, Corentin Labbe, netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Tue, Apr 17, 2018 at 7:52 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Mon, Apr 16, 2018 at 10:51:55PM +0800, Chen-Yu Tsai wrote: >> On Mon, Apr 16, 2018 at 10:31 PM, Maxime Ripard >> <maxime.ripard@bootlin.com> wrote: >> > On Thu, Apr 12, 2018 at 11:23:30PM +0800, Chen-Yu Tsai wrote: >> >> On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy@aosc.io> wrote: >> >> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >> >> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: >> >> >>> From: Chen-Yu Tsai <wens@csie.org> >> >> >>> >> >> >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU >> >> >>> address space; on the A64 SoC this register is in the SRAM controller >> >> >>> address space, and with a different offset. >> >> >>> >> >> >>> To access the register from another device and hide the internal >> >> >>> difference between the device, let it register a regmap named >> >> >>> "emac-clock". We can then get the device from the phandle, and >> >> >>> retrieve the regmap with dev_get_regmap(); in this situation the >> >> >>> regmap_field will be set up to access the only register in the >> >> >>regmap. >> >> >>> >> >> >>> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >> >>> [Icenowy: change to use regmaps with single register, change commit >> >> >>> message] >> >> >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >> >> >>> --- >> >> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 >> >> >>++++++++++++++++++++++- >> >> >>> 1 file changed, 46 insertions(+), 2 deletions(-) >> >> >>> >> >> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>> index 1037f6c78bca..b61210c0d415 100644 >> >> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> >> >>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = { >> >> >>> .msb = 31, >> >> >>> }; >> >> >>> >> >> >>> +/* Specially exported regmap which contains only EMAC register */ >> >> >>> +const struct reg_field single_reg_field = { >> >> >>> + .reg = 0, >> >> >>> + .lsb = 0, >> >> >>> + .msb = 31, >> >> >>> +}; >> >> >>> + >> >> >> >> >> >>I'm not sure this would be wise. If we ever need some other register >> >> >>exported through the regmap, will have to change all the calling sites >> >> >>everywhere in the kernel, which will be a pain and will break >> >> >>bisectability. >> >> > >> >> > In this situation the register can be exported as another >> >> > regmap. Currently the code will access a regmap with name >> >> > "emac-clock" for this register. >> >> > >> >> >> >> >> >>Chen-Yu's (or was it yours?) initial solution with a custom writeable >> >> >>hook only allowing a single register seemed like a better one. >> >> > >> >> > But I remember you mentioned that you want it to hide the >> >> > difference inside the device. >> >> >> >> The idea is that a device can export multiple regmaps. This one, >> >> the one named "gmac" (in my soon to come v2) or "emac-clock" here, >> >> is but one of many possible regmaps, and it only exports the register >> >> needed by the GMAC/EMAC. >> > >> > I'm not sure this would be wise either. There's a single register map, >> > and as far as I know we don't have a binding to express this in the >> > DT. This means that the customer and provider would have to use the >> > same name, but without anything actually enforcing it aside from >> > "someone in the community knows it". >> > >> > This is not a really good design, and I was actually preferring your >> > first option. We shouldn't rely on any undocumented rule. This will be >> > easy to break and hard to maintain. >> >> So, one regmap per device covering the whole register range, and the >> consumer knows which register to poke by looking at its own compatible. >> >> That sound right? > > Yep. And ideally, sending a single serie for both the A64 and the R40 > cases, in order to provide the big picture. OK. I'll incorporate Icenowy's stuff into my series. ChenYu ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAGb2v66e+tWsYqzpOnRaJtjq4OneUOruszYML0FnvAGbXi5qsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device [not found] ` <CAGb2v66e+tWsYqzpOnRaJtjq4OneUOruszYML0FnvAGbXi5qsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-04-17 12:06 ` Icenowy Zheng 0 siblings, 0 replies; 22+ messages in thread From: Icenowy Zheng @ 2018-04-17 12:06 UTC (permalink / raw) To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chen-Yu Tsai, Maxime Ripard Cc: devicetree, netdev, linux-kernel, linux-sunxi, Rob Herring, Corentin Labbe, Giuseppe Cavallaro, linux-arm-kernel 于 2018年4月17日 GMT+08:00 下午7:59:38, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> 写到: >On Tue, Apr 17, 2018 at 7:52 PM, Maxime Ripard ><maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: >> On Mon, Apr 16, 2018 at 10:51:55PM +0800, Chen-Yu Tsai wrote: >>> On Mon, Apr 16, 2018 at 10:31 PM, Maxime Ripard >>> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote: >>> > On Thu, Apr 12, 2018 at 11:23:30PM +0800, Chen-Yu Tsai wrote: >>> >> On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >wrote: >>> >> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard ><maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到: >>> >> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote: >>> >> >>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >>> >> >>> >>> >> >>> On the Allwinner R40 SoC, the "GMAC clock" register is in the >CCU >>> >> >>> address space; on the A64 SoC this register is in the SRAM >controller >>> >> >>> address space, and with a different offset. >>> >> >>> >>> >> >>> To access the register from another device and hide the >internal >>> >> >>> difference between the device, let it register a regmap named >>> >> >>> "emac-clock". We can then get the device from the phandle, >and >>> >> >>> retrieve the regmap with dev_get_regmap(); in this situation >the >>> >> >>> regmap_field will be set up to access the only register in >the >>> >> >>regmap. >>> >> >>> >>> >> >>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> >>> >> >>> [Icenowy: change to use regmaps with single register, change >commit >>> >> >>> message] >>> >> >>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> >>> >> >>> --- >>> >> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 >>> >> >>++++++++++++++++++++++- >>> >> >>> 1 file changed, 46 insertions(+), 2 deletions(-) >>> >> >>> >>> >> >>> diff --git >a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>> >> >>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>> >> >>> index 1037f6c78bca..b61210c0d415 100644 >>> >> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>> >> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >>> >> >>> @@ -85,6 +85,13 @@ const struct reg_field >old_syscon_reg_field = { >>> >> >>> .msb = 31, >>> >> >>> }; >>> >> >>> >>> >> >>> +/* Specially exported regmap which contains only EMAC >register */ >>> >> >>> +const struct reg_field single_reg_field = { >>> >> >>> + .reg = 0, >>> >> >>> + .lsb = 0, >>> >> >>> + .msb = 31, >>> >> >>> +}; >>> >> >>> + >>> >> >> >>> >> >>I'm not sure this would be wise. If we ever need some other >register >>> >> >>exported through the regmap, will have to change all the >calling sites >>> >> >>everywhere in the kernel, which will be a pain and will break >>> >> >>bisectability. >>> >> > >>> >> > In this situation the register can be exported as another >>> >> > regmap. Currently the code will access a regmap with name >>> >> > "emac-clock" for this register. >>> >> > >>> >> >> >>> >> >>Chen-Yu's (or was it yours?) initial solution with a custom >writeable >>> >> >>hook only allowing a single register seemed like a better one. >>> >> > >>> >> > But I remember you mentioned that you want it to hide the >>> >> > difference inside the device. >>> >> >>> >> The idea is that a device can export multiple regmaps. This one, >>> >> the one named "gmac" (in my soon to come v2) or "emac-clock" >here, >>> >> is but one of many possible regmaps, and it only exports the >register >>> >> needed by the GMAC/EMAC. >>> > >>> > I'm not sure this would be wise either. There's a single register >map, >>> > and as far as I know we don't have a binding to express this in >the >>> > DT. This means that the customer and provider would have to use >the >>> > same name, but without anything actually enforcing it aside from >>> > "someone in the community knows it". >>> > >>> > This is not a really good design, and I was actually preferring >your >>> > first option. We shouldn't rely on any undocumented rule. This >will be >>> > easy to break and hard to maintain. >>> >>> So, one regmap per device covering the whole register range, and the >>> consumer knows which register to poke by looking at its own >compatible. >>> >>> That sound right? >> >> Yep. And ideally, sending a single serie for both the A64 and the R40 >> cases, in order to provide the big picture. > >OK. I'll incorporate Icenowy's stuff into my series. In this situation maybe I should send newer revision of A64 drivers to you? > >ChenYu > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH] net: stmmac: dwmac-sun8i: single_reg_field can be static [not found] ` <20180411141641.14675-4-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-12 14:56 ` Maxime Ripard @ 2018-04-13 6:03 ` kbuild test robot 2018-04-13 6:03 ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device kbuild test robot 2 siblings, 0 replies; 22+ messages in thread From: kbuild test robot @ 2018-04-13 6:03 UTC (permalink / raw) To: Icenowy Zheng Cc: kbuild-all-JC7UmRfGjtg, Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe, netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng Fixes: 529123418105 ("net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device") Signed-off-by: Fengguang Wu <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- dwmac-sun8i.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index b61210c..842315a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -86,7 +86,7 @@ const struct reg_field old_syscon_reg_field = { }; /* Specially exported regmap which contains only EMAC register */ -const struct reg_field single_reg_field = { +static const struct reg_field single_reg_field = { .reg = 0, .lsb = 0, .msb = 31, ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device [not found] ` <20180411141641.14675-4-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-12 14:56 ` Maxime Ripard 2018-04-13 6:03 ` [RFC PATCH] net: stmmac: dwmac-sun8i: single_reg_field can be static kbuild test robot @ 2018-04-13 6:03 ` kbuild test robot 2 siblings, 0 replies; 22+ messages in thread From: kbuild test robot @ 2018-04-13 6:03 UTC (permalink / raw) To: Icenowy Zheng Cc: kbuild-all-JC7UmRfGjtg, Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe, netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng Hi Chen-Yu, I love your patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on v4.16 next-20180412] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Icenowy-Zheng/Add-support-in-dwmac-sun8i-for-accessing-EMAC-clock/20180413-004816 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:82:24: sparse: symbol 'old_syscon_reg_field' was not declared. Should it be static? >> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:89:24: sparse: symbol 'single_reg_field' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] drivers: soc: sunxi: export a regmap for EMAC clock reg on A64 [not found] ` <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org> ` (2 preceding siblings ...) 2018-04-11 14:16 ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device Icenowy Zheng @ 2018-04-11 14:16 ` Icenowy Zheng 3 siblings, 0 replies; 22+ messages in thread From: Icenowy Zheng @ 2018-04-11 14:16 UTC (permalink / raw) To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng The A64 SRAM controller memory zone has a EMAC clock register, which is needed by the Ethernet MAC driver (dwmac-sun8i). Export a regmap for this register on A64. Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> --- drivers/soc/sunxi/sunxi_sram.c | 48 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c index 882be5ed7e84..35ab5f334bb1 100644 --- a/drivers/soc/sunxi/sunxi_sram.c +++ b/drivers/soc/sunxi/sunxi_sram.c @@ -17,6 +17,7 @@ #include <linux/of_address.h> #include <linux/of_device.h> #include <linux/platform_device.h> +#include <linux/regmap.h> #include <linux/soc/sunxi/sunxi_sram.h> @@ -281,13 +282,41 @@ int sunxi_sram_release(struct device *dev) } EXPORT_SYMBOL(sunxi_sram_release); +struct sunxi_sramc_variant { + bool has_emac_clock; +}; + +static const struct sunxi_sramc_variant sun4i_a10_sramc_variant = { + /* Nothing special */ +}; + +static const struct sunxi_sramc_variant sun50i_a64_sramc_variant = { + .has_emac_clock = true, +}; + +static struct regmap_config sunxi_sram_emac_clock_regmap = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, + .max_register = 0x0, /* only single register */ + .name = "emac-clock", +}; + +#define SUNXI_SRAM_EMAC_CLOCK_REG 0x30 + static int sunxi_sram_probe(struct platform_device *pdev) { struct resource *res; struct dentry *d; + struct regmap *emac_clock; + const struct sunxi_sramc_variant *variant; sram_dev = &pdev->dev; + variant = of_device_get_match_data(&pdev->dev); + if (!variant) + return -EINVAL; + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); base = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(base)) @@ -300,12 +329,27 @@ static int sunxi_sram_probe(struct platform_device *pdev) if (!d) return -ENOMEM; + if (variant->has_emac_clock) { + emac_clock = devm_regmap_init_mmio(&pdev->dev, + base + SUNXI_SRAM_EMAC_CLOCK_REG, + &sunxi_sram_emac_clock_regmap); + + if (IS_ERR(emac_clock)) + return PTR_ERR(emac_clock); + } + return 0; } static const struct of_device_id sunxi_sram_dt_match[] = { - { .compatible = "allwinner,sun4i-a10-sram-controller" }, - { .compatible = "allwinner,sun50i-a64-sram-controller" }, + { + .compatible = "allwinner,sun4i-a10-sram-controller", + .data = &sun4i_a10_sramc_variant, + }, + { + .compatible = "allwinner,sun50i-a64-sram-controller", + .data = &sun50i_a64_sramc_variant, + }, { }, }; MODULE_DEVICE_TABLE(of, sunxi_sram_dt_match); -- 2.15.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/5] arm64: allwinner: a64: add SRAM controller device tree node 2018-04-11 14:16 [PATCH 0/5] Add support in dwmac-sun8i for accessing EMAC clock Icenowy Zheng [not found] ` <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org> @ 2018-04-11 14:16 ` Icenowy Zheng [not found] ` <20180411141641.14675-6-icenowy-h8G6r0blFSE@public.gmane.org> 1 sibling, 1 reply; 22+ messages in thread From: Icenowy Zheng @ 2018-04-11 14:16 UTC (permalink / raw) To: Rob Herring, Maxime Ripard, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe Cc: netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi, Icenowy Zheng 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> --- 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"; 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 { + compatible = "allwinner,sun50i-a64-sram-c"; + reg = <0x0000 0x28000>; + }; + }; }; dma: dma-controller@1c02000 { @@ -599,7 +614,7 @@ emac: ethernet@1c30000 { compatible = "allwinner,sun50i-a64-emac"; - syscon = <&syscon>; + syscon = <&sram_controller>; reg = <0x01c30000 0x10000>; interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = "macirq"; -- 2.15.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
[parent not found: <20180411141641.14675-6-icenowy-h8G6r0blFSE@public.gmane.org>]
* Re: [PATCH 5/5] arm64: allwinner: a64: add SRAM controller device tree node [not found] ` <20180411141641.14675-6-icenowy-h8G6r0blFSE@public.gmane.org> @ 2018-04-12 14:58 ` Maxime Ripard 0 siblings, 0 replies; 22+ messages in thread From: Maxime Ripard @ 2018-04-12 14:58 UTC (permalink / raw) To: Icenowy Zheng Cc: Rob Herring, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe, netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw [-- Attachment #1: Type: text/plain, Size: 1919 bytes --] Hi, On Wed, Apr 11, 2018 at 10:16:41PM +0800, Icenowy Zheng wrote: > 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-h8G6r0blFSE@public.gmane.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"; > 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 { > + compatible = "allwinner,sun50i-a64-sram-c"; > + reg = <0x0000 0x28000>; > + }; > + }; That doesn't look related at all to what's being discussed here, so you'd rather add it as part of your DE2-enablement serie (or amend your commit log to say why this is important to do it in this patch). Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-04-28 13:42 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-11 14:16 [PATCH 0/5] Add support in dwmac-sun8i for accessing EMAC clock Icenowy Zheng [not found] ` <20180411141641.14675-1-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:16 ` [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap Icenowy Zheng [not found] ` <20180411141641.14675-2-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-11 14:36 ` Brüns, Stefan 2018-04-16 18:47 ` Rob Herring 2018-04-16 23:17 ` Icenowy Zheng [not found] ` <04C9F795-2680-4220-A39A-7B7D5FD74C4A-h8G6r0blFSE@public.gmane.org> 2018-04-28 13:42 ` Chen-Yu Tsai 2018-04-11 14:16 ` [PATCH 2/5] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access Icenowy Zheng 2018-04-11 14:16 ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device Icenowy Zheng [not found] ` <20180411141641.14675-4-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-12 14:56 ` Maxime Ripard 2018-04-12 15:11 ` Icenowy Zheng [not found] ` <A027702A-F3F7-4896-B2B8-E024DA521661-h8G6r0blFSE@public.gmane.org> 2018-04-12 15:23 ` Chen-Yu Tsai [not found] ` <CAGb2v66Qi2D1qgF3-iVJFAiznMUzzen0nHM5vsPe7Cyi6QTYpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-04-16 14:31 ` Maxime Ripard 2018-04-16 14:34 ` Icenowy Zheng 2018-04-16 14:51 ` Chen-Yu Tsai [not found] ` <CAGb2v65VLYc1X2bN5v0_1xokZCEkKFYNLzecnE3WeP8goc2KmQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-04-17 11:52 ` Maxime Ripard 2018-04-17 11:59 ` [linux-sunxi] " Chen-Yu Tsai [not found] ` <CAGb2v66e+tWsYqzpOnRaJtjq4OneUOruszYML0FnvAGbXi5qsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-04-17 12:06 ` Icenowy Zheng 2018-04-13 6:03 ` [RFC PATCH] net: stmmac: dwmac-sun8i: single_reg_field can be static kbuild test robot 2018-04-13 6:03 ` [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device kbuild test robot 2018-04-11 14:16 ` [PATCH 4/5] drivers: soc: sunxi: export a regmap for EMAC clock reg on A64 Icenowy Zheng 2018-04-11 14:16 ` [PATCH 5/5] arm64: allwinner: a64: add SRAM controller device tree node Icenowy Zheng [not found] ` <20180411141641.14675-6-icenowy-h8G6r0blFSE@public.gmane.org> 2018-04-12 14:58 ` Maxime Ripard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).