Re: [PATCH net-next v2 15/15] arm64: dts: allwinner: a64: add SRAM controller device tree node

[Chen-Yu Tsai]

On Mon, May 14, 2018 at 1:03 AM, Maxime Ripard
<maxime.rip...@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.rip...@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 <w...@csie.org> 写到:
>> >> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard
>> >> ><maxime.rip...@bootlin.com> wrote:
>> >> >> Hi,
>> >> >>
>> >> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai wrote:
>> >> >>> From: Icenowy Zheng <icen...@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 <icen...@aosc.io>
>> >> >>> Signed-off-by: Chen-Yu Tsai <w...@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@1c0 {
>> >> >>> - compatible =
>> >> >"allwinner,sun50i-a64-system-controller",
>> >> >>> - "syscon";
>> >> >>> + sram_controller: sram-controller@1c0 {
>> >> >>> + 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

Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions

[Chen-Yu Tsai]

On Sun, May 13, 2018 at 1:29 PM, Andrew Lunn <and...@lunn.ch> wrote:
> On Sun, May 13, 2018 at 01:11:08PM -0700, Chen-Yu Tsai wrote:
>> On Sun, May 13, 2018 at 1:05 PM, Andrew Lunn <and...@lunn.ch> wrote:
>> >> > Hi Chen-Yu
>> >> >
>> >> > Are these delays the MAC applies? Not the PHY. It would be good to
>> >> > make it clear here these are MAC imposed delays.
>> >>
>> >> Yes these are applied on the MAC side. Being described in the device
>> >> tree bindings for the MAC, I thought this was implied to be the case?
>> >> Are there known exceptions?
>> >
>> > There is frequent confusion with this. Most of the time, the PHY does
>> > the delay, not the MAC, based on the phy-mode. So the MAC doing it is
>> > an exception in itself.
>> >
>> > Do you actually need these delays for the board you adding support
>> > for? Does the PHY not support adding the needed delays? If you don't
>> > need the delays, i would not even implement them.
>>
>> Yes this is already used on the Bananapi M3. This patch merely reformats
>> the description and adds a note saying this only applies to RGMII mode.
>
> Yes, the current code is needed for the Bananapi M3. But you have
> another patch which extends the code to support a smaller range. Do
> you have a board which actually needs this? If not, i would not add
> that new code.

IIRC the delay on the PHY side is either 2ns or none. The delay on the
MAC side here is an order smaller, likely fine tuning to cope with board
design deficiencies.

Currently no other board requires this, but this is already part of the
binding. The new stuff limits the range for a specific SoC, simply because
that is the range supported by the control register. Not implementing, i.e.
supporting the whole range from the property, which might get truncated,
doesn't make much sense to me.

Regards
ChenYu

Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions

[Chen-Yu Tsai]

On Sun, May 13, 2018 at 1:05 PM, Andrew Lunn  wrote:
>> > Hi Chen-Yu
>> >
>> > Are these delays the MAC applies? Not the PHY. It would be good to
>> > make it clear here these are MAC imposed delays.
>>
>> Yes these are applied on the MAC side. Being described in the device
>> tree bindings for the MAC, I thought this was implied to be the case?
>> Are there known exceptions?
>
> There is frequent confusion with this. Most of the time, the PHY does
> the delay, not the MAC, based on the phy-mode. So the MAC doing it is
> an exception in itself.
>
> Do you actually need these delays for the board you adding support
> for? Does the PHY not support adding the needed delays? If you don't
> need the delays, i would not even implement them.

Yes this is already used on the Bananapi M3. This patch merely reformats
the description and adds a note saying this only applies to RGMII mode.

ChenYu

Re: [PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions

[Chen-Yu Tsai]

On Sun, May 13, 2018 at 12:49 PM, Andrew Lunn <and...@lunn.ch> wrote:
> On Mon, May 14, 2018 at 03:14:18AM +0800, Chen-Yu Tsai wrote:
>> The clock delay chains found in the glue layer for dwmac-sun8i are only
>> used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
>> MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
>> range of possible values for the delay chain.
>>
>> This patch reformats the delay chain section of the device tree binding
>> to make it clear that the delay chains only apply to RGMII PHYs, and
>> make it easier to add the R40-specific bits later.
>>
>> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>> Reviewed-by: Rob Herring <r...@kernel.org>
>> Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
>> ---
>>  Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
>> b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> index 3d6d5fa0c4d5..e04ce75e24a3 100644
>> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> @@ -28,10 +28,13 @@ Required properties:
>>- allwinner,sun8i-a83t-system-controller
>>
>>  Optional properties:
>> -- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 
>> 0-700. Default is 0)
>> -- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 
>> 0-3100. Default is 0)
>> -Both delay properties need to be a multiple of 100. They control the delay 
>> for
>> -external PHY.
>> +- allwinner,tx-delay-ps: TX clock delay chain value in ps.
>> +  Range is 0-700. Default is 0.
>> +- allwinner,rx-delay-ps: RX clock delay chain value in ps.
>> +  Range is 0-3100. Default is 0.
>> +Both delay properties need to be a multiple of 100. They control the
>> +clock delay for external RGMII PHY. They do not apply to the internal
>> +PHY or external non-RGMII PHYs.
>
> Hi Chen-Yu
>
> Are these delays the MAC applies? Not the PHY. It would be good to
> make it clear here these are MAC imposed delays.

Yes these are applied on the MAC side. Being described in the device
tree bindings for the MAC, I thought this was implied to be the case?
Are there known exceptions?

Thanks
ChenYu

Re: [PATCH net-next v2 15/15] arm64: dts: allwinner: a64: add SRAM controller device tree node

[Chen-Yu Tsai]

On Wed, May 2, 2018 at 4:54 AM, Maxime Ripard <maxime.rip...@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 <w...@csie.org> 写到:
>> >On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard
>> ><maxime.rip...@bootlin.com> wrote:
>> >> Hi,
>> >>
>> >> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai wrote:
>> >>> From: Icenowy Zheng <icen...@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 <icen...@aosc.io>
>> >>> Signed-off-by: Chen-Yu Tsai <w...@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@1c0 {
>> >>> - compatible =
>> >"allwinner,sun50i-a64-system-controller",
>> >>> - "syscon";
>> >>> + sram_controller: sram-controller@1c0 {
>> >>> + 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

[PATCH RESEND net-next v2 1/8] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions

[Chen-Yu Tsai]

The clock delay chains found in the glue layer for dwmac-sun8i are only
used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
range of possible values for the delay chain.

This patch reformats the delay chain section of the device tree binding
to make it clear that the delay chains only apply to RGMII PHYs, and
make it easier to add the R40-specific bits later.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Reviewed-by: Rob Herring <r...@kernel.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 3d6d5fa0c4d5..e04ce75e24a3 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -28,10 +28,13 @@ Required properties:
   - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
-- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 
0-700. Default is 0)
-- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 
0-3100. Default is 0)
-Both delay properties need to be a multiple of 100. They control the delay for
-external PHY.
+- allwinner,tx-delay-ps: TX clock delay chain value in ps.
+Range is 0-700. Default is 0.
+- allwinner,rx-delay-ps: RX clock delay chain value in ps.
+Range is 0-3100. Default is 0.
+Both delay properties need to be a multiple of 100. They control the
+clock delay for external RGMII PHY. They do not apply to the internal
+PHY or external non-RGMII PHYs.
 
 Optional properties for the following compatibles:
   - "allwinner,sun8i-h3-emac",
-- 
2.17.0

[PATCH RESEND net-next v2 2/8] dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order

[Chen-Yu Tsai]

The A83T syscon compatible was appended to the syscon compatibles list,
instead of inserted in to preserve the ordering.

Move it to the proper place to keep the list sorted.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Reviewed-by: Rob Herring <r...@kernel.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index e04ce75e24a3..1b8e33e71651 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -22,10 +22,10 @@ Required properties:
 - #size-cells: shall be 0
 - syscon: A phandle to the syscon of the SoC with one of the following
  compatible string:
+  - allwinner,sun8i-a83t-system-controller
   - allwinner,sun8i-h3-system-controller
   - allwinner,sun8i-v3s-system-controller
   - allwinner,sun50i-a64-system-controller
-  - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

[PATCH RESEND net-next v2 5/8] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access

[Chen-Yu Tsai]

On the Allwinner R40, the "GMAC clock" register is located in the CCU
block, at a different register address than the other SoCs that have
it in the "system control" block.

This patch converts the use of regmap to regmap_field for mapping and
accessing the syscon register, so we can have the register address in
the variants data, 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 <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 42 ++-
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a3fa65b1ca8e..bbc051474806 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -42,6 +42,7 @@
  * This value is used for disabling properly EMAC
  * and used as a good starting value in case of the
  * boot process(uboot) leave some stuff.
+ * @syscon_field   reg_field for the syscon's gmac register
  * @soc_has_internal_phy:  Does the MAC embed an internal PHY
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
@@ -49,6 +50,7 @@
  */
 struct emac_variant {
u32 default_syscon_value;
+   const struct reg_field *syscon_field;
bool soc_has_internal_phy;
bool support_mii;
bool support_rmii;
@@ -71,13 +73,21 @@ 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;
 };
 
+/* EMAC clock register @ 0x30 in the "system control" address range */
+static const struct reg_field sun8i_syscon_reg_field = {
+   .reg = 0x30,
+   .lsb = 0,
+   .msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
@@ -86,12 +96,14 @@ static const struct emac_variant emac_variant_h3 = {
 
 static const struct emac_variant emac_variant_v3s = {
.default_syscon_value = 0x38000,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = true,
.support_mii = true
 };
 
 static const struct emac_variant emac_variant_a83t = {
.default_syscon_value = 0,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
.support_rgmii = true
@@ -99,6 +111,7 @@ static const struct emac_variant emac_variant_a83t = {
 
 static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
@@ -216,7 +229,6 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_ETCS_MII0x0
 #define SYSCON_ETCS_EXT_GMII   0x1
 #define SYSCON_ETCS_INT_GMII   0x2
-#define SYSCON_EMAC_REG0x30
 
 /* sun8i_dwmac_dma_reset() - reset the EMAC
  * Called from stmmac via stmmac_dma_ops->reset
@@ -745,7 +757,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 +775,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 +813,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
int ret;
u32 reg, val;
 
-   regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
+   regmap_field_read(gmac->regmap_field, 

[PATCH RESEND net-next v2 0/8] net: stmmac: dwmac-sun8i: Support R40

[Chen-Yu Tsai]

This is a resend of the patches for net-next split out from my R40
Ethernet support v2 series, as requested by David Miller. The arm-soc
bits will follow, once I rework the A64 system controller compatible.

Patches 1, 2, and 3 clean up the dwmac-sun8i binding.

Patch 4 adds device tree binding for Allwinner R40's Ethernet
controller.

Patch 5 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 6 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 7 adds support for different or absent TX/RX delay chain ranges
to the dwmac-sun8i driver.

Patch 8 adds support for the R40's ethernet controller.


Excerpt from original cover letter:

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.


Please have a look.

Regards
ChenYu

Chen-Yu Tsai (8):
  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

 .../devicetree/bindings/net/dwmac-sun8i.txt   |  21 +--
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 139 +++---
 2 files changed, 130 insertions(+), 30 deletions(-)

-- 
2.17.0

[PATCH RESEND net-next v2 6/8] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external device

[Chen-Yu Tsai]

On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU
address space. Using a standard syscon to access it provides no
coordination with the CCU driver for register access. Neither does
it prevent this and other drivers from accessing other, maybe critical,
clock control registers. On other SoCs, the register is in the "system
control" address space, which might also contain controls for mapping
SRAM to devices or the CPU. This hardware has the same issues.

Instead, for these types of setups, we let the device containing the
control register create a regmap tied to it. We can then get the device
from the existing syscon phandle, and retrieve the regmap with
dev_get_regmap().

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 50 ++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index bbc051474806..79e104a20e20 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -983,6 +983,34 @@ static struct mac_device_info *sun8i_dwmac_setup(void 
*ppriv)
return mac;
 }
 
+static struct regmap *sun8i_dwmac_get_syscon_from_dev(struct device_node *node)
+{
+   struct device_node *syscon_node;
+   struct platform_device *syscon_pdev;
+   struct regmap *regmap = NULL;
+
+   syscon_node = of_parse_phandle(node, "syscon", 0);
+   if (!syscon_node)
+   return ERR_PTR(-ENODEV);
+
+   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 the other device driver is at fault */
+   regmap = dev_get_regmap(_pdev->dev, NULL);
+   if (!regmap)
+   regmap = ERR_PTR(-EINVAL);
+
+   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;
@@ -1027,7 +1055,27 @@ static int sun8i_dwmac_probe(struct platform_device 
*pdev)
gmac->regulator = NULL;
}
 
-   regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon");
+   /* The "GMAC clock control" register might be located in the
+* CCU address range (on the R40), or the system control address
+* range (on most other sun8i and later SoCs).
+*
+* The former controls most if not all clocks in the SoC. The
+* latter has an SoC identification register, and on some SoCs,
+* controls to map device specific SRAM to either the intended
+* peripheral, or the CPU address space.
+*
+* In either case, there should be a coordinated and restricted
+* method of accessing the register needed here. This is done by
+* having the device export a custom regmap, instead of a generic
+* syscon, which grants all access to all registers.
+*
+* To support old device trees, we fall back to using the syscon
+* interface if possible.
+*/
+   regmap = sun8i_dwmac_get_syscon_from_dev(pdev->dev.of_node);
+   if (IS_ERR(regmap))
+   regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+"syscon");
if (IS_ERR(regmap)) {
ret = PTR_ERR(regmap);
dev_err(>dev, "Unable to map syscon: %d\n", ret);
-- 
2.17.0

[PATCH RESEND net-next v2 8/8] net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC

[Chen-Yu Tsai]

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds support for it using the framework laid out by previous
patches to map the differences.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c   | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4f5612a3c855..2f7f0915f071 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -93,6 +93,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
.msb = 31,
 };
 
+/* EMAC clock register @ 0x164 in the CCU address range */
+static const struct reg_field sun8i_ccu_reg_field = {
+   .reg = 0x164,
+   .lsb = 0,
+   .msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
.syscon_field = _syscon_reg_field,
@@ -121,6 +128,14 @@ static const struct emac_variant emac_variant_a83t = {
.tx_delay_max = 7,
 };
 
+static const struct emac_variant emac_variant_r40 = {
+   .default_syscon_value = 0,
+   .syscon_field = _ccu_reg_field,
+   .support_mii = true,
+   .support_rgmii = true,
+   .rx_delay_max = 7,
+};
+
 static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
.syscon_field = _syscon_reg_field,
@@ -1160,6 +1175,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
.data = _variant_v3s },
{ .compatible = "allwinner,sun8i-a83t-emac",
.data = _variant_a83t },
+   { .compatible = "allwinner,sun8i-r40-gmac",
+   .data = _variant_r40 },
{ .compatible = "allwinner,sun50i-a64-emac",
.data = _variant_a64 },
{ }
-- 
2.17.0

[PATCH RESEND net-next v2 4/8] dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40 SoC

[Chen-Yu Tsai]

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds the R40 specific bits to the dwmac-sun8i binding.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Reviewed-by: Rob Herring <r...@kernel.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1c0906a5c02b..cfe724398a12 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -7,6 +7,7 @@ Required properties:
 - compatible: must be one of the following string:
"allwinner,sun8i-a83t-emac"
"allwinner,sun8i-h3-emac"
+   "allwinner,sun8i-r40-gmac"
"allwinner,sun8i-v3s-emac"
"allwinner,sun50i-a64-emac"
 - reg: address and length of the register for the device.
@@ -25,8 +26,10 @@ Required properties:
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
 Range is 0-700. Default is 0.
+Unavailable for allwinner,sun8i-r40-gmac
 - allwinner,rx-delay-ps: RX clock delay chain value in ps.
 Range is 0-3100. Default is 0.
+Range is 0-700 for allwinner,sun8i-r40-gmac
 Both delay properties need to be a multiple of 100. They control the
 clock delay for external RGMII PHY. They do not apply to the internal
 PHY or external non-RGMII PHYs.
-- 
2.17.0

[PATCH RESEND net-next v2 3/8] dt-bindings: net: dwmac-sun8i: simplify description of syscon property

[Chen-Yu Tsai]

The syscon property is used to point to the device that holds the glue
layer control register known as the "EMAC (or GMAC) clock register".

We do not need to explicitly list what compatible strings are needed, as
this information is readily available in the user manuals. Also the
"syscon" device type is more of an implementation detail. There are many
ways to access a register not in a device's address range, the syscon
interface being the most generic and unrestricted one.

Simplify the description so that it says what it is supposed to
describe.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Reviewed-by: Rob Herring <r...@kernel.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1b8e33e71651..1c0906a5c02b 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -20,12 +20,7 @@ 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:
-  - allwinner,sun8i-a83t-system-controller
-  - allwinner,sun8i-h3-system-controller
-  - allwinner,sun8i-v3s-system-controller
-  - allwinner,sun50i-a64-system-controller
+- syscon: A phandle to the device containing the EMAC or GMAC clock register
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

[PATCH RESEND net-next v2 7/8] net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay chains

[Chen-Yu Tsai]

On the R40 SoC, the RX delay chain only has a range of 0~7 (hundred
picoseconds), instead of 0~31. Also the TX delay chain is completely
absent.

This patch adds support for different ranges by adding per-compatible
maximum values in the variant data. A maximum of 0 indicates that the
delay chain is not supported or absent.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 32 +--
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 79e104a20e20..4f5612a3c855 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -47,6 +47,12 @@
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
  * @support_rgmii: Does the MAC handle RGMII
+ *
+ * @rx_delay_max:  Maximum raw value for RX delay chain
+ * @tx_delay_max:  Maximum raw value for TX delay chain
+ * These two also indicate the bitmask for
+ * the RX and TX delay chain registers. A
+ * value of zero indicates this is not supported.
  */
 struct emac_variant {
u32 default_syscon_value;
@@ -55,6 +61,8 @@ struct emac_variant {
bool support_mii;
bool support_rmii;
bool support_rgmii;
+   u8 rx_delay_max;
+   u8 tx_delay_max;
 };
 
 /* struct sunxi_priv_data - hold all sunxi private data
@@ -91,7 +99,9 @@ static const struct emac_variant emac_variant_h3 = {
.soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_v3s = {
@@ -106,7 +116,9 @@ static const struct emac_variant emac_variant_a83t = {
.syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_a64 = {
@@ -115,7 +127,9 @@ static const struct emac_variant emac_variant_a64 = {
.soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 #define EMAC_BASIC_CTL0 0x00
@@ -219,9 +233,7 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides EPIT) */
 
 /* Generic system control EMAC_CLK bits */
-#define SYSCON_ETXDC_MASK  GENMASK(2, 0)
 #define SYSCON_ETXDC_SHIFT 10
-#define SYSCON_ERXDC_MASK  GENMASK(4, 0)
 #define SYSCON_ERXDC_SHIFT 5
 /* EMAC PHY Interface Type */
 #define SYSCON_EPITBIT(2) /* 1: RGMII, 0: MII */
@@ -847,8 +859,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
}
val /= 100;
dev_dbg(priv->device, "set tx-delay to %x\n", val);
-   if (val <= SYSCON_ETXDC_MASK) {
-   reg &= ~(SYSCON_ETXDC_MASK << SYSCON_ETXDC_SHIFT);
+   if (val <= gmac->variant->tx_delay_max) {
+   reg &= ~(gmac->variant->tx_delay_max <<
+SYSCON_ETXDC_SHIFT);
reg |= (val << SYSCON_ETXDC_SHIFT);
} else {
dev_err(priv->device, "Invalid TX clock delay: %d\n",
@@ -864,8 +877,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
}
val /= 100;
dev_dbg(priv->device, "set rx-delay to %x\n", val);
-   if (val <= SYSCON_ERXDC_MASK) {
-   reg &= ~(SYSCON_ERXDC_MASK << SYSCON_ERXDC_SHIFT);
+   if (val <= gmac->variant->rx_delay_max) {
+   reg &= ~(gmac->variant->rx_delay_max <<
+SYSCON_ERXDC_SHIFT);
reg |= (val << SYSCON_ERXDC_SHIFT);
} else {
dev_err(priv->device, "Invalid RX clock delay: %d\n",
-- 
2.17.0

Re: [PATCH net-next v2 15/15] arm64: dts: allwinner: a64: add SRAM controller device tree node

[Chen-Yu Tsai]

On Wed, May 2, 2018 at 5:51 PM, Maxime Ripard <maxime.rip...@bootlin.com> wrote:
> Hi,
>
> On Wed, May 02, 2018 at 12:12:27AM +0800, Chen-Yu Tsai wrote:
>> From: Icenowy Zheng <icen...@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 <icen...@aosc.io>
>> Signed-off-by: Chen-Yu Tsai <w...@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@1c0 {
>> - compatible = "allwinner,sun50i-a64-system-controller",
>> - "syscon";
>> + sram_controller: sram-controller@1c0 {
>> + 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.rip...@bootlin.com>
>
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH net-next v2 00/15] ARM: sun8i: r40: Add Ethernet support

[Chen-Yu Tsai]

On Wed, May 2, 2018 at 12:12 AM, Chen-Yu Tsai <w...@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
>

[PATCH net-next v2 11/15] ARM: dts: sun8i: r40: bananapi-m2-ultra: Sort device node dereferences

[Chen-Yu Tsai]

The device nodes dereference () usages should be sorted by the label
names, barring any parsing order issues such as the #include statement
for the PMIC's .dtsi file that must come after the PMIC.

Move the mmc and ohci blocks in front of the PMIC's regulator blocks.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 69 ++-
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts 
b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
index 27d9ccd0ef2f..c6da21e43572 100644
--- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
@@ -114,6 +114,41 @@
 
 #include "axp22x.dtsi"
 
+ {
+   vmmc-supply = <_dcdc1>;
+   bus-width = <4>;
+   cd-gpios = < 7 13 GPIO_ACTIVE_HIGH>; /* PH13 */
+   cd-inverted;
+   status = "okay";
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pg_pins>;
+   vmmc-supply = <_dldo2>;
+   vqmmc-supply = <_dldo1>;
+   mmc-pwrseq = <_pwrseq>;
+   bus-width = <4>;
+   non-removable;
+   status = "okay";
+};
+
+ {
+   vmmc-supply = <_dcdc1>;
+   vqmmc-supply = <_dcdc1>;
+   bus-width = <8>;
+   non-removable;
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
 _aldo3 {
regulator-always-on;
regulator-min-microvolt = <270>;
@@ -161,40 +196,6 @@
regulator-name = "vcc-wifi";
 };
 
- {
-   vmmc-supply = <_dcdc1>;
-   bus-width = <4>;
-   cd-gpios = < 7 13 GPIO_ACTIVE_LOW>; /* PH13 */
-   status = "okay";
-};
-
- {
-   pinctrl-names = "default";
-   pinctrl-0 = <_pg_pins>;
-   vmmc-supply = <_dldo2>;
-   vqmmc-supply = <_dldo1>;
-   mmc-pwrseq = <_pwrseq>;
-   bus-width = <4>;
-   non-removable;
-   status = "okay";
-};
-
- {
-   vmmc-supply = <_dcdc1>;
-   vqmmc-supply = <_dcdc1>;
-   bus-width = <8>;
-   non-removable;
-   status = "okay";
-};
-
- {
-   status = "okay";
-};
-
- {
-   status = "okay";
-};
-
  {
pinctrl-names = "default";
pinctrl-0 = <_pb_pins>;
-- 
2.17.0

[PATCH net-next v2 12/15] ARM: dts: sun8i: r40: Add device node and RGMII pinmux node for GMAC

[Chen-Yu Tsai]

The R40 SoC has a GMAC (gigabit capable Ethernet controller). Add a
device node for it. The only publicly available board for this SoC
uses an RGMII PHY. Add a pinmux node for it as well.

Since this SoC also has an old 10/100 Mbps EMAC, which also has an
MDIO bus controller, the MDIO bus for the GMAC is labeled "gmac_mdio".

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 arch/arm/boot/dts/sun8i-r40.dtsi | 34 
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 173dcc1652d2..bd97ca3dc2fa 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -265,6 +265,19 @@
#interrupt-cells = <3>;
#gpio-cells = <3>;
 
+   gmac_rgmii_pins: gmac-rgmii-pins {
+   pins = "PA0", "PA1", "PA2", "PA3",
+  "PA4", "PA5", "PA6", "PA7",
+  "PA8", "PA10", "PA11", "PA12",
+  "PA13", "PA15", "PA16";
+   function = "gmac";
+   /*
+* data lines in RGMII mode use DDR mode
+* and need a higher signal drive strength
+*/
+   drive-strength = <40>;
+   };
+
i2c0_pins: i2c0-pins {
pins = "PB0", "PB1";
function = "i2c0";
@@ -451,6 +464,27 @@
#size-cells = <0>;
};
 
+   gmac: ethernet@1c5 {
+   compatible = "allwinner,sun8i-r40-gmac";
+   syscon = <>;
+   reg = <0x01c5 0x1>;
+   interrupts = ;
+   interrupt-names = "macirq";
+   resets = < RST_BUS_GMAC>;
+   reset-names = "stmmaceth";
+   clocks = < CLK_BUS_GMAC>;
+   clock-names = "stmmaceth";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";
+
+   gmac_mdio: mdio {
+   compatible = "snps,dwmac-mdio";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+   };
+
gic: interrupt-controller@1c81000 {
compatible = "arm,gic-400";
reg = <0x01c81000 0x1000>,
-- 
2.17.0

[PATCH net-next v2 01/15] clk: sunxi-ng: r40: rewrite init code to a platform driver

[Chen-Yu Tsai]

From: Icenowy Zheng <icen...@aosc.io>

As we need to register a regmap on the R40 CCU, there needs to be a
device structure bound to the CCU device node.

Rewrite the R40 CCU driver initial code to make it a proper platform
driver, thus we will have a platform device bound to it.

Signed-off-by: Icenowy Zheng <icen...@aosc.io>
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 39 
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c 
b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
index 933f2e68f42a..c3aa839a453d 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
@@ -12,7 +12,8 @@
  */
 
 #include 
-#include 
+#include 
+#include 
 
 #include "ccu_common.h"
 #include "ccu_reset.h"
@@ -1250,17 +1251,17 @@ static struct ccu_mux_nb sun8i_r40_cpu_nb = {
.bypass_index   = 1, /* index of 24 MHz oscillator */
 };
 
-static void __init sun8i_r40_ccu_setup(struct device_node *node)
+static int sun8i_r40_ccu_probe(struct platform_device *pdev)
 {
+   struct resource *res;
void __iomem *reg;
u32 val;
+   int ret;
 
-   reg = of_io_request_and_map(node, 0, of_node_full_name(node));
-   if (IS_ERR(reg)) {
-   pr_err("%s: Could not map the clock registers\n",
-  of_node_full_name(node));
-   return;
-   }
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   reg = devm_ioremap_resource(>dev, res);
+   if (IS_ERR(reg))
+   return PTR_ERR(reg);
 
/* Force the PLL-Audio-1x divider to 4 */
val = readl(reg + SUN8I_R40_PLL_AUDIO_REG);
@@ -1277,7 +1278,9 @@ static void __init sun8i_r40_ccu_setup(struct device_node 
*node)
val &= ~GENMASK(25, 20);
writel(val, reg + SUN8I_R40_USB_CLK_REG);
 
-   sunxi_ccu_probe(node, reg, _r40_ccu_desc);
+   ret = sunxi_ccu_probe(pdev->dev.of_node, reg, _r40_ccu_desc);
+   if (ret)
+   return ret;
 
/* Gate then ungate PLL CPU after any rate changes */
ccu_pll_notifier_register(_r40_pll_cpu_nb);
@@ -1285,6 +1288,20 @@ static void __init sun8i_r40_ccu_setup(struct 
device_node *node)
/* Reparent CPU during PLL CPU rate changes */
ccu_mux_notifier_register(pll_cpu_clk.common.hw.clk,
  _r40_cpu_nb);
+
+   return 0;
 }
-CLK_OF_DECLARE(sun8i_r40_ccu, "allwinner,sun8i-r40-ccu",
-  sun8i_r40_ccu_setup);
+
+static const struct of_device_id sun8i_r40_ccu_ids[] = {
+   { .compatible = "allwinner,sun8i-r40-ccu" },
+   { }
+};
+
+static struct platform_driver sun8i_r40_ccu_driver = {
+   .probe  = sun8i_r40_ccu_probe,
+   .driver = {
+   .name   = "sun8i-r40-ccu",
+   .of_match_table = sun8i_r40_ccu_ids,
+   },
+};
+builtin_platform_driver(sun8i_r40_ccu_driver);
-- 
2.17.0

[PATCH net-next v2 04/15] dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order

[Chen-Yu Tsai]

The A83T syscon compatible was appended to the syscon compatibles list,
instead of inserted in to preserve the ordering.

Move it to the proper place to keep the list sorted.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
Reviewed-by: Rob Herring <r...@kernel.org>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index e04ce75e24a3..1b8e33e71651 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -22,10 +22,10 @@ Required properties:
 - #size-cells: shall be 0
 - syscon: A phandle to the syscon of the SoC with one of the following
  compatible string:
+  - allwinner,sun8i-a83t-system-controller
   - allwinner,sun8i-h3-system-controller
   - allwinner,sun8i-v3s-system-controller
   - allwinner,sun50i-a64-system-controller
-  - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

[PATCH net-next v2 10/15] net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC

[Chen-Yu Tsai]

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds support for it using the framework laid out by previous
patches to map the differences.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c   | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4f5612a3c855..2f7f0915f071 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -93,6 +93,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
.msb = 31,
 };
 
+/* EMAC clock register @ 0x164 in the CCU address range */
+static const struct reg_field sun8i_ccu_reg_field = {
+   .reg = 0x164,
+   .lsb = 0,
+   .msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
.syscon_field = _syscon_reg_field,
@@ -121,6 +128,14 @@ static const struct emac_variant emac_variant_a83t = {
.tx_delay_max = 7,
 };
 
+static const struct emac_variant emac_variant_r40 = {
+   .default_syscon_value = 0,
+   .syscon_field = _ccu_reg_field,
+   .support_mii = true,
+   .support_rgmii = true,
+   .rx_delay_max = 7,
+};
+
 static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
.syscon_field = _syscon_reg_field,
@@ -1160,6 +1175,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
.data = _variant_v3s },
{ .compatible = "allwinner,sun8i-a83t-emac",
.data = _variant_a83t },
+   { .compatible = "allwinner,sun8i-r40-gmac",
+   .data = _variant_r40 },
{ .compatible = "allwinner,sun50i-a64-emac",
.data = _variant_a64 },
{ }
-- 
2.17.0

[PATCH net-next v2 09/15] net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay chains

[Chen-Yu Tsai]

On the R40 SoC, the RX delay chain only has a range of 0~7 (hundred
picoseconds), instead of 0~31. Also the TX delay chain is completely
absent.

This patch adds support for different ranges by adding per-compatible
maximum values in the variant data. A maximum of 0 indicates that the
delay chain is not supported or absent.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 32 +--
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 79e104a20e20..4f5612a3c855 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -47,6 +47,12 @@
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
  * @support_rgmii: Does the MAC handle RGMII
+ *
+ * @rx_delay_max:  Maximum raw value for RX delay chain
+ * @tx_delay_max:  Maximum raw value for TX delay chain
+ * These two also indicate the bitmask for
+ * the RX and TX delay chain registers. A
+ * value of zero indicates this is not supported.
  */
 struct emac_variant {
u32 default_syscon_value;
@@ -55,6 +61,8 @@ struct emac_variant {
bool support_mii;
bool support_rmii;
bool support_rgmii;
+   u8 rx_delay_max;
+   u8 tx_delay_max;
 };
 
 /* struct sunxi_priv_data - hold all sunxi private data
@@ -91,7 +99,9 @@ static const struct emac_variant emac_variant_h3 = {
.soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_v3s = {
@@ -106,7 +116,9 @@ static const struct emac_variant emac_variant_a83t = {
.syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_a64 = {
@@ -115,7 +127,9 @@ static const struct emac_variant emac_variant_a64 = {
.soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 #define EMAC_BASIC_CTL0 0x00
@@ -219,9 +233,7 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides EPIT) */
 
 /* Generic system control EMAC_CLK bits */
-#define SYSCON_ETXDC_MASK  GENMASK(2, 0)
 #define SYSCON_ETXDC_SHIFT 10
-#define SYSCON_ERXDC_MASK  GENMASK(4, 0)
 #define SYSCON_ERXDC_SHIFT 5
 /* EMAC PHY Interface Type */
 #define SYSCON_EPITBIT(2) /* 1: RGMII, 0: MII */
@@ -847,8 +859,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
}
val /= 100;
dev_dbg(priv->device, "set tx-delay to %x\n", val);
-   if (val <= SYSCON_ETXDC_MASK) {
-   reg &= ~(SYSCON_ETXDC_MASK << SYSCON_ETXDC_SHIFT);
+   if (val <= gmac->variant->tx_delay_max) {
+   reg &= ~(gmac->variant->tx_delay_max <<
+SYSCON_ETXDC_SHIFT);
reg |= (val << SYSCON_ETXDC_SHIFT);
} else {
dev_err(priv->device, "Invalid TX clock delay: %d\n",
@@ -864,8 +877,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
}
val /= 100;
dev_dbg(priv->device, "set rx-delay to %x\n", val);
-   if (val <= SYSCON_ERXDC_MASK) {
-   reg &= ~(SYSCON_ERXDC_MASK << SYSCON_ERXDC_SHIFT);
+   if (val <= gmac->variant->rx_delay_max) {
+   reg &= ~(gmac->variant->rx_delay_max <<
+SYSCON_ERXDC_SHIFT);
reg |= (val << SYSCON_ERXDC_SHIFT);
} else {
dev_err(priv->device, "Invalid RX clock delay: %d\n",
-- 
2.17.0

[PATCH net-next v2 06/15] dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40 SoC

[Chen-Yu Tsai]

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds the R40 specific bits to the dwmac-sun8i binding.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1c0906a5c02b..cfe724398a12 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -7,6 +7,7 @@ Required properties:
 - compatible: must be one of the following string:
"allwinner,sun8i-a83t-emac"
"allwinner,sun8i-h3-emac"
+   "allwinner,sun8i-r40-gmac"
"allwinner,sun8i-v3s-emac"
"allwinner,sun50i-a64-emac"
 - reg: address and length of the register for the device.
@@ -25,8 +26,10 @@ Required properties:
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
 Range is 0-700. Default is 0.
+Unavailable for allwinner,sun8i-r40-gmac
 - allwinner,rx-delay-ps: RX clock delay chain value in ps.
 Range is 0-3100. Default is 0.
+Range is 0-700 for allwinner,sun8i-r40-gmac
 Both delay properties need to be a multiple of 100. They control the
 clock delay for external RGMII PHY. They do not apply to the internal
 PHY or external non-RGMII PHYs.
-- 
2.17.0

[PATCH net-next v2 00/15] ARM: sun8i: r40: Add Ethernet support

[Chen-Yu Tsai]

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(-)

-- 
2.17.0

[PATCH net-next v2 15/15] arm64: dts: allwinner: a64: add SRAM controller device tree node

[Chen-Yu Tsai]

From: Icenowy Zheng <icen...@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 <icen...@aosc.io>
Signed-off-by: Chen-Yu Tsai <w...@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@1c0 {
-   compatible = "allwinner,sun50i-a64-system-controller",
-   "syscon";
+   sram_controller: sram-controller@1c0 {
+   compatible = "allwinner,sun50i-a64-sram-controller";
reg = <0x01c0 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 = <0x 0x28000>;
+   };
+   };
};
 
dma: dma-controller@1c02000 {
@@ -599,7 +614,7 @@
 
emac: ethernet@1c3 {
compatible = "allwinner,sun50i-a64-emac";
-   syscon = <>;
+   syscon = <_controller>;
reg = <0x01c3 0x1>;
interrupts = ;
interrupt-names = "macirq";
-- 
2.17.0

[PATCH net-next v2 02/15] clk: sunxi-ng: r40: export a regmap to access the GMAC register

[Chen-Yu Tsai]

From: Icenowy Zheng <icen...@aosc.io>

There's a GMAC configuration register, which exists on A64/A83T/H3/H5 in
the syscon part, in the CCU of R40 SoC.

Export a regmap of the CCU.

Read access is not restricted to all registers, but only the GMAC
register is allowed to be written.

Signed-off-by: Icenowy Zheng <icen...@aosc.io>
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 33 
 1 file changed, 33 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c 
b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
index c3aa839a453d..65ba6455feb7 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
@@ -1251,9 +1251,37 @@ static struct ccu_mux_nb sun8i_r40_cpu_nb = {
.bypass_index   = 1, /* index of 24 MHz oscillator */
 };
 
+/*
+ * Add a regmap for the GMAC driver (dwmac-sun8i) to access the
+ * GMAC configuration register.
+ * Only this register is allowed to be written, in order to
+ * prevent overriding critical clock configuration.
+ */
+
+#define SUN8I_R40_GMAC_CFG_REG 0x164
+static bool sun8i_r40_ccu_regmap_accessible_reg(struct device *dev,
+   unsigned int reg)
+{
+   if (reg == SUN8I_R40_GMAC_CFG_REG)
+   return true;
+   return false;
+}
+
+static struct regmap_config sun8i_r40_ccu_regmap_config = {
+   .reg_bits   = 32,
+   .val_bits   = 32,
+   .reg_stride = 4,
+   .max_register   = 0x320, /* PLL_LOCK_CTRL_REG */
+
+   /* other devices have no business accessing other registers */
+   .readable_reg   = sun8i_r40_ccu_regmap_accessible_reg,
+   .writeable_reg  = sun8i_r40_ccu_regmap_accessible_reg,
+};
+
 static int sun8i_r40_ccu_probe(struct platform_device *pdev)
 {
struct resource *res;
+   struct regmap *regmap;
void __iomem *reg;
u32 val;
int ret;
@@ -1278,6 +1306,11 @@ static int sun8i_r40_ccu_probe(struct platform_device 
*pdev)
val &= ~GENMASK(25, 20);
writel(val, reg + SUN8I_R40_USB_CLK_REG);
 
+   regmap = devm_regmap_init_mmio(>dev, reg,
+  _r40_ccu_regmap_config);
+   if (IS_ERR(regmap))
+   return PTR_ERR(regmap);
+
ret = sunxi_ccu_probe(pdev->dev.of_node, reg, _r40_ccu_desc);
if (ret)
return ret;
-- 
2.17.0

[PATCH net-next v2 03/15] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions

[Chen-Yu Tsai]

The clock delay chains found in the glue layer for dwmac-sun8i are only
used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
range of possible values for the delay chain.

This patch reformats the delay chain section of the device tree binding
to make it clear that the delay chains only apply to RGMII PHYs, and
make it easier to add the R40-specific bits later.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 3d6d5fa0c4d5..e04ce75e24a3 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -28,10 +28,13 @@ Required properties:
   - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
-- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 
0-700. Default is 0)
-- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 
0-3100. Default is 0)
-Both delay properties need to be a multiple of 100. They control the delay for
-external PHY.
+- allwinner,tx-delay-ps: TX clock delay chain value in ps.
+Range is 0-700. Default is 0.
+- allwinner,rx-delay-ps: RX clock delay chain value in ps.
+Range is 0-3100. Default is 0.
+Both delay properties need to be a multiple of 100. They control the
+clock delay for external RGMII PHY. They do not apply to the internal
+PHY or external non-RGMII PHYs.
 
 Optional properties for the following compatibles:
   - "allwinner,sun8i-h3-emac",
-- 
2.17.0

[PATCH net-next v2 08/15] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from external device

[Chen-Yu Tsai]

On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU
address space. Using a standard syscon to access it provides no
coordination with the CCU driver for register access. Neither does
it prevent this and other drivers from accessing other, maybe critical,
clock control registers. On other SoCs, the register is in the "system
control" address space, which might also contain controls for mapping
SRAM to devices or the CPU. This hardware has the same issues.

Instead, for these types of setups, we let the device containing the
control register create a regmap tied to it. We can then get the device
from the existing syscon phandle, and retrieve the regmap with
dev_get_regmap().

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 50 ++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index bbc051474806..79e104a20e20 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -983,6 +983,34 @@ static struct mac_device_info *sun8i_dwmac_setup(void 
*ppriv)
return mac;
 }
 
+static struct regmap *sun8i_dwmac_get_syscon_from_dev(struct device_node *node)
+{
+   struct device_node *syscon_node;
+   struct platform_device *syscon_pdev;
+   struct regmap *regmap = NULL;
+
+   syscon_node = of_parse_phandle(node, "syscon", 0);
+   if (!syscon_node)
+   return ERR_PTR(-ENODEV);
+
+   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 the other device driver is at fault */
+   regmap = dev_get_regmap(_pdev->dev, NULL);
+   if (!regmap)
+   regmap = ERR_PTR(-EINVAL);
+
+   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;
@@ -1027,7 +1055,27 @@ static int sun8i_dwmac_probe(struct platform_device 
*pdev)
gmac->regulator = NULL;
}
 
-   regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon");
+   /* The "GMAC clock control" register might be located in the
+* CCU address range (on the R40), or the system control address
+* range (on most other sun8i and later SoCs).
+*
+* The former controls most if not all clocks in the SoC. The
+* latter has an SoC identification register, and on some SoCs,
+* controls to map device specific SRAM to either the intended
+* peripheral, or the CPU address space.
+*
+* In either case, there should be a coordinated and restricted
+* method of accessing the register needed here. This is done by
+* having the device export a custom regmap, instead of a generic
+* syscon, which grants all access to all registers.
+*
+* To support old device trees, we fall back to using the syscon
+* interface if possible.
+*/
+   regmap = sun8i_dwmac_get_syscon_from_dev(pdev->dev.of_node);
+   if (IS_ERR(regmap))
+   regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+"syscon");
if (IS_ERR(regmap)) {
ret = PTR_ERR(regmap);
dev_err(>dev, "Unable to map syscon: %d\n", ret);
-- 
2.17.0

[PATCH net-next v2 05/15] dt-bindings: net: dwmac-sun8i: simplify description of syscon property

[Chen-Yu Tsai]

The syscon property is used to point to the device that holds the glue
layer control register known as the "EMAC (or GMAC) clock register".

We do not need to explicitly list what compatible strings are needed, as
this information is readily available in the user manuals. Also the
"syscon" device type is more of an implementation detail. There are many
ways to access a register not in a device's address range, the syscon
interface being the most generic and unrestricted one.

Simplify the description so that it says what it is supposed to
describe.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 1b8e33e71651..1c0906a5c02b 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -20,12 +20,7 @@ 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:
-  - allwinner,sun8i-a83t-system-controller
-  - allwinner,sun8i-h3-system-controller
-  - allwinner,sun8i-v3s-system-controller
-  - allwinner,sun50i-a64-system-controller
+- syscon: A phandle to the device containing the EMAC or GMAC clock register
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.17.0

[PATCH net-next v2 13/15] ARM: dts: sun8i: r40: bananapi-m2-ultra: Enable GMAC ethernet controller

[Chen-Yu Tsai]

The Bananapi M2 Ultra has a Realtek RTL8211E RGMII PHY tied to the GMAC.
The PMIC's DC1SW output provides power for the PHY, while the ALDO2
output provides I/O voltages on both sides.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 30 +++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts 
b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
index c6da21e43572..25fb048c7df2 100644
--- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
@@ -51,6 +51,7 @@
compatible = "sinovoip,bpi-m2-ultra", "allwinner,sun8i-r40";
 
aliases {
+   ethernet0 = 
serial0 = 
};
 
@@ -101,6 +102,22 @@
status = "okay";
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_rgmii_pins>;
+   phy-handle = <>;
+   phy-mode = "rgmii";
+   phy-supply = <_dc1sw>;
+   status = "okay";
+};
+
+_mdio {
+   phy1: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <1>;
+   };
+};
+
  {
status = "okay";
 
@@ -149,6 +166,13 @@
status = "okay";
 };
 
+_aldo2 {
+   regulator-always-on;
+   regulator-min-microvolt = <250>;
+   regulator-max-microvolt = <250>;
+   regulator-name = "vcc-pa";
+};
+
 _aldo3 {
regulator-always-on;
regulator-min-microvolt = <270>;
@@ -156,6 +180,12 @@
regulator-name = "avcc";
 };
 
+_dc1sw {
+   regulator-min-microvolt = <300>;
+   regulator-max-microvolt = <300>;
+   regulator-name = "vcc-gmac-phy";
+};
+
 _dcdc1 {
regulator-always-on;
regulator-min-microvolt = <300>;
-- 
2.17.0

[PATCH net-next v2 07/15] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access

[Chen-Yu Tsai]

On the Allwinner R40, the "GMAC clock" register is located in the CCU
block, at a different register address than the other SoCs that have
it in the "system control" block.

This patch converts the use of regmap to regmap_field for mapping and
accessing the syscon register, so we can have the register address in
the variants data, 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 <w...@csie.org>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 42 ++-
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a3fa65b1ca8e..bbc051474806 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -42,6 +42,7 @@
  * This value is used for disabling properly EMAC
  * and used as a good starting value in case of the
  * boot process(uboot) leave some stuff.
+ * @syscon_field   reg_field for the syscon's gmac register
  * @soc_has_internal_phy:  Does the MAC embed an internal PHY
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
@@ -49,6 +50,7 @@
  */
 struct emac_variant {
u32 default_syscon_value;
+   const struct reg_field *syscon_field;
bool soc_has_internal_phy;
bool support_mii;
bool support_rmii;
@@ -71,13 +73,21 @@ 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;
 };
 
+/* EMAC clock register @ 0x30 in the "system control" address range */
+static const struct reg_field sun8i_syscon_reg_field = {
+   .reg = 0x30,
+   .lsb = 0,
+   .msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
@@ -86,12 +96,14 @@ static const struct emac_variant emac_variant_h3 = {
 
 static const struct emac_variant emac_variant_v3s = {
.default_syscon_value = 0x38000,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = true,
.support_mii = true
 };
 
 static const struct emac_variant emac_variant_a83t = {
.default_syscon_value = 0,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
.support_rgmii = true
@@ -99,6 +111,7 @@ static const struct emac_variant emac_variant_a83t = {
 
 static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
@@ -216,7 +229,6 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_ETCS_MII0x0
 #define SYSCON_ETCS_EXT_GMII   0x1
 #define SYSCON_ETCS_INT_GMII   0x2
-#define SYSCON_EMAC_REG0x30
 
 /* sun8i_dwmac_dma_reset() - reset the EMAC
  * Called from stmmac via stmmac_dma_ops->reset
@@ -745,7 +757,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 +775,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 +813,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
int ret;
u32 reg, val;
 
-   regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
+   regmap_field_read(gmac->regmap_field, );
reg = gmac->variant->default_syscon_value;
   

[PATCH net-next v2 14/15] soc: sunxi: export a regmap for EMAC clock reg on A64

[Chen-Yu Tsai]

From: Icenowy Zheng <icen...@aosc.io>

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 <icen...@aosc.io>
[w...@csie.org: export whole address range with only EMAC register
accessible and drop regmap name]
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/soc/sunxi/sunxi_sram.c | 57 --
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 882be5ed7e84..eec7fc6e9f66 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -281,13 +282,51 @@ 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,
+};
+
+#define SUNXI_SRAM_EMAC_CLOCK_REG  0x30
+static bool sunxi_sram_regmap_accessible_reg(struct device *dev,
+unsigned int reg)
+{
+   if (reg == SUNXI_SRAM_EMAC_CLOCK_REG)
+   return true;
+   return false;
+}
+
+static struct regmap_config sunxi_sram_emac_clock_regmap = {
+   .reg_bits   = 32,
+   .val_bits   = 32,
+   .reg_stride = 4,
+   /* last defined register */
+   .max_register   = SUNXI_SRAM_EMAC_CLOCK_REG,
+   /* other devices have no business accessing other registers */
+   .readable_reg   = sunxi_sram_regmap_accessible_reg,
+   .writeable_reg  = sunxi_sram_regmap_accessible_reg,
+};
+
 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 = >dev;
 
+   variant = of_device_get_match_data(>dev);
+   if (!variant)
+   return -EINVAL;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(>dev, res);
if (IS_ERR(base))
@@ -300,12 +339,26 @@ static int sunxi_sram_probe(struct platform_device *pdev)
if (!d)
return -ENOMEM;
 
+   if (variant->has_emac_clock) {
+   emac_clock = devm_regmap_init_mmio(>dev, base,
+  
_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 = _a10_sramc_variant,
+   },
+   {
+   .compatible = "allwinner,sun50i-a64-sram-controller",
+   .data = _a64_sramc_variant,
+   },
{ },
 };
 MODULE_DEVICE_TABLE(of, sunxi_sram_dt_match);
-- 
2.17.0

Re: [linux-sunxi] Re: [PATCH 1/5] dt-bindings: allow dwmac-sun8i to use other devices' exported regmap

[Chen-Yu Tsai]

Hi Rob,

On Tue, Apr 17, 2018 at 7:17 AM, Icenowy Zheng  wrote:
>
>
> 于 2018年4月17日 GMT+08:00 上午2:47:45, Rob Herring  写到:
>>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 
>>> ---
>>>  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 majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>___
>>linux-arm-kernel mailing list
>>linux-arm-ker...@lists.infradead.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+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device

[Chen-Yu Tsai]

On Tue, Apr 17, 2018 at 7:52 PM, Maxime Ripard
<maxime.rip...@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.rip...@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 <icen...@aosc.io> wrote:
>> >> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard 
>> >> > <maxime.rip...@bootlin.com> 写到:
>> >> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote:
>> >> >>> From: Chen-Yu Tsai <w...@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 <w...@csie.org>
>> >> >>> [Icenowy: change to use regmaps with single register, change commit
>> >> >>>  message]
>> >> >>> Signed-off-by: Icenowy Zheng <icen...@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

Re: [linux-sunxi] Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device

[Chen-Yu Tsai]

On Mon, Apr 16, 2018 at 10:31 PM, Maxime Ripard
<maxime.rip...@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 <icen...@aosc.io> wrote:
>> > 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard 
>> > <maxime.rip...@bootlin.com> 写到:
>> >>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote:
>> >>> From: Chen-Yu Tsai <w...@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 <w...@csie.org>
>> >>> [Icenowy: change to use regmaps with single register, change commit
>> >>>  message]
>> >>> Signed-off-by: Icenowy Zheng <icen...@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?

ChenYu

Re: [linux-sunxi] Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device

[Chen-Yu Tsai]

On Thu, Apr 12, 2018 at 11:11 PM, Icenowy Zheng <icen...@aosc.io> wrote:
>
>
> 于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.rip...@bootlin.com> 
> 写到:
>>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote:
>>> From: Chen-Yu Tsai <w...@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 <w...@csie.org>
>>> [Icenowy: change to use regmaps with single register, change commit
>>>  message]
>>> Signed-off-by: Icenowy Zheng <icen...@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.

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

Re: [PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to access the GMAC register

[Chen-Yu Tsai]

On Wed, Apr 4, 2018 at 2:45 PM, Icenowy Zheng <icen...@aosc.io> wrote:
> 在 2018-04-03二的 11:50 +0200，Maxime Ripard写道：
>> On Tue, Apr 03, 2018 at 11:48:45AM +0200, Maxime Ripard wrote:
>> > On Tue, Mar 20, 2018 at 03:15:02PM +0800, Chen-Yu Tsai wrote:
>> > > On Mon, Mar 19, 2018 at 5:31 AM, Maxime Ripard
>> > > <maxime.rip...@bootlin.com> wrote:
>> > > > On Sat, Mar 17, 2018 at 05:28:47PM +0800, Chen-Yu Tsai wrote:
>> > > > > From: Icenowy Zheng <icen...@aosc.io>
>> > > > >
>> > > > > There's a GMAC configuration register, which exists on
>> > > > > A64/A83T/H3/H5 in
>> > > > > the syscon part, in the CCU of R40 SoC.
>> > > > >
>> > > > > Export a regmap of the CCU.
>> > > > >
>> > > > > Read access is not restricted to all registers, but only the
>> > > > > GMAC
>> > > > > register is allowed to be written.
>> > > > >
>> > > > > Signed-off-by: Icenowy Zheng <icen...@aosc.io>
>> > > > > Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>> > > >
>> > > > Gah, this is crazy. I'm really starting to regret letting that
>> > > > syscon
>> > > > in in the first place...
>> > >
>> > > IMHO syscon is really a better fit. It's part of the glue layer
>> > > and
>> > > most other dwmac user platforms treat it as such and use a
>> > > syscon.
>> > > Plus the controls encompass delays (phase), inverters (polarity),
>> > > and even signal routing. It's not really just a group of clock
>> > > controls,
>> > > like what we poorly modeled for A20/A31. I think that was really
>> > > a
>> > > mistake.
>> > >
>> > > As I mentioned in the cover letter, a slightly saner approach
>> > > would
>> > > be to let drivers add custom syscon entries, which would then
>> > > require
>> > > less custom plumbing.
>> >
>> > A syscon is convenient, sure, but it also bypasses any abstraction
>> > layer we have everywhere else, which means that we'll have to
>> > maintain
>> > the register layout in each and every driver that uses it.
>> >
>> > So far, it's only be the GMAC, but it can also be others (the SRAM
>> > controller comes to my mind), and then, if there's any difference
>> > in
>> > the design in a future SoC, we'll have to maintain that in the GMAC
>> > driver as well.
>>
>> I guess I forgot to say something, I'm fine with using a syscon we
>> already have.
>
> As we finally need the SRAM controller on these new SoCs (for VE on all
> SoCs targeted by dwmac-sun8i, and for DE on A64), should we deprecate
> the syscon and all switch to device regmap (and let sunxi-dram driver
> to export a regmap)? (As in the A64 DE2 thread discussed, two device
> nodes shouldn't cover the same MMIO region.)

Sounds like a plan.

> In addition, there might be multiple registers in the syscon/sram zone
> that are needed (for example, if a version driver is written, it will
> need the "0x24 Version Register"; and GMAC needs "0x30 EMAC Clock
> Register"). How to deal with this if we export the syscon/sram zone as
> a generic device regmap?

You can have multiple regmaps for a device. And the API allows you to
request them up by name.

ChenYu

>>
>> I'm just questionning if merging any other driver using one is the
>> right move.
>>
>> Maxime
>>
>> ___
>> linux-arm-kernel mailing list
>> linux-arm-ker...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to access the GMAC register

[Chen-Yu Tsai]

On Tue, Apr 3, 2018 at 5:54 PM, Icenowy Zheng <icen...@aosc.io> wrote:
>
>
> 于 2018年4月3日 GMT+08:00 下午5:53:08, Chen-Yu Tsai <w...@csie.org> 写到:
>>On Tue, Apr 3, 2018 at 5:50 PM, Maxime Ripard
>><maxime.rip...@bootlin.com> wrote:
>>> On Tue, Apr 03, 2018 at 11:48:45AM +0200, Maxime Ripard wrote:
>>>> On Tue, Mar 20, 2018 at 03:15:02PM +0800, Chen-Yu Tsai wrote:
>>>> > On Mon, Mar 19, 2018 at 5:31 AM, Maxime Ripard
>>>> > <maxime.rip...@bootlin.com> wrote:
>>>> > > On Sat, Mar 17, 2018 at 05:28:47PM +0800, Chen-Yu Tsai wrote:
>>>> > >> From: Icenowy Zheng <icen...@aosc.io>
>>>> > >>
>>>> > >> There's a GMAC configuration register, which exists on
>>A64/A83T/H3/H5 in
>>>> > >> the syscon part, in the CCU of R40 SoC.
>>>> > >>
>>>> > >> Export a regmap of the CCU.
>>>> > >>
>>>> > >> Read access is not restricted to all registers, but only the
>>GMAC
>>>> > >> register is allowed to be written.
>>>> > >>
>>>> > >> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
>>>> > >> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>>>> > >
>>>> > > Gah, this is crazy. I'm really starting to regret letting that
>>syscon
>>>> > > in in the first place...
>>>> >
>>>> > IMHO syscon is really a better fit. It's part of the glue layer
>>and
>>>> > most other dwmac user platforms treat it as such and use a syscon.
>>>> > Plus the controls encompass delays (phase), inverters (polarity),
>>>> > and even signal routing. It's not really just a group of clock
>>controls,
>>>> > like what we poorly modeled for A20/A31. I think that was really a
>>>> > mistake.
>>>> >
>>>> > As I mentioned in the cover letter, a slightly saner approach
>>would
>>>> > be to let drivers add custom syscon entries, which would then
>>require
>>>> > less custom plumbing.
>>>>
>>>> A syscon is convenient, sure, but it also bypasses any abstraction
>>>> layer we have everywhere else, which means that we'll have to
>>maintain
>>>> the register layout in each and every driver that uses it.
>>>>
>>>> So far, it's only be the GMAC, but it can also be others (the SRAM
>>>> controller comes to my mind), and then, if there's any difference in
>>>> the design in a future SoC, we'll have to maintain that in the GMAC
>>>> driver as well.
>>>
>>> I guess I forgot to say something, I'm fine with using a syscon we
>>> already have.
>>>
>>> I'm just questionning if merging any other driver using one is the
>>> right move.
>>
>>Right. So in this case, we are not actually going through the syscon
>>API. Rather we are exporting a regmap whose properties we actually
>>define. If it makes you more acceptable to it, we could map just
>>the GMAC register in the new regmap, and also have it named. This
>>is all plumbing within the kernel so the device tree stays the same.
>
> I think my driver has already restricted the write permission
> only to GMAC register.

Correct, but it still maps the entire region out, which means the
consumer needs to know which offset to use. Maxime is saying this
is something that is troublesome to maintain. So my proposal was
to create a regmap with a base at the GMAC register offset. That
way, the consumer doesn't need to use an offset to access it.

ChenYu

Re: [PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to access the GMAC register

[Chen-Yu Tsai]

On Tue, Apr 3, 2018 at 5:50 PM, Maxime Ripard <maxime.rip...@bootlin.com> wrote:
> On Tue, Apr 03, 2018 at 11:48:45AM +0200, Maxime Ripard wrote:
>> On Tue, Mar 20, 2018 at 03:15:02PM +0800, Chen-Yu Tsai wrote:
>> > On Mon, Mar 19, 2018 at 5:31 AM, Maxime Ripard
>> > <maxime.rip...@bootlin.com> wrote:
>> > > On Sat, Mar 17, 2018 at 05:28:47PM +0800, Chen-Yu Tsai wrote:
>> > >> From: Icenowy Zheng <icen...@aosc.io>
>> > >>
>> > >> There's a GMAC configuration register, which exists on A64/A83T/H3/H5 in
>> > >> the syscon part, in the CCU of R40 SoC.
>> > >>
>> > >> Export a regmap of the CCU.
>> > >>
>> > >> Read access is not restricted to all registers, but only the GMAC
>> > >> register is allowed to be written.
>> > >>
>> > >> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
>> > >> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>> > >
>> > > Gah, this is crazy. I'm really starting to regret letting that syscon
>> > > in in the first place...
>> >
>> > IMHO syscon is really a better fit. It's part of the glue layer and
>> > most other dwmac user platforms treat it as such and use a syscon.
>> > Plus the controls encompass delays (phase), inverters (polarity),
>> > and even signal routing. It's not really just a group of clock controls,
>> > like what we poorly modeled for A20/A31. I think that was really a
>> > mistake.
>> >
>> > As I mentioned in the cover letter, a slightly saner approach would
>> > be to let drivers add custom syscon entries, which would then require
>> > less custom plumbing.
>>
>> A syscon is convenient, sure, but it also bypasses any abstraction
>> layer we have everywhere else, which means that we'll have to maintain
>> the register layout in each and every driver that uses it.
>>
>> So far, it's only be the GMAC, but it can also be others (the SRAM
>> controller comes to my mind), and then, if there's any difference in
>> the design in a future SoC, we'll have to maintain that in the GMAC
>> driver as well.
>
> I guess I forgot to say something, I'm fine with using a syscon we
> already have.
>
> I'm just questionning if merging any other driver using one is the
> right move.

Right. So in this case, we are not actually going through the syscon
API. Rather we are exporting a regmap whose properties we actually
define. If it makes you more acceptable to it, we could map just
the GMAC register in the new regmap, and also have it named. This
is all plumbing within the kernel so the device tree stays the same.

ChenYu

Re: [PATCH net-next 03/12] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions

[Chen-Yu Tsai]

On Sun, Mar 18, 2018 at 5:21 PM, Sergei Shtylyov
<sergei.shtyl...@cogentembedded.com> wrote:
> Hello!
>
>
> On 3/17/2018 12:28 PM, Chen-Yu Tsai wrote:
>
>> The clock delay chains found in the glue layer for dwmac-sun8i are only
>> used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
>> MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
>> range of possible values for the delay chain.
>>
>> This patch reformats the delay chain section of the device tree binding
>> to make it clear that the delay chains only apply to RGMII PHYs, and
>> make it easier to add the R40-specific bits later.
>>
>> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>> ---
>>   Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> index 3d6d5fa0c4d5..b8a3028d6c30 100644
>> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>> @@ -28,10 +28,13 @@ Required properties:
>> - allwinner,sun8i-a83t-system-controller
>> Optional properties:
>> -- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is
>> 0-700. Default is 0)
>> -- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is
>> 0-3100. Default is 0)
>> -Both delay properties need to be a multiple of 100. They control the
>> delay for
>> -external PHY.
>> +- allwinner,tx-delay-ps: TX clock delay chain value in ps.
>> +Range is 0-700. Default is 0.
>> +- allwinner,rx-delay-ps: RX clock delay chain value in ps.
>> +Range is 0-3100. Default is 0.
>> +Both delay properties need to be a multiple of 100. They control the
>> +clock delay for external RGMII PHY. They are do apply to the internal
>
>
>   s/are do/do not/?

Fixed. Thanks!

ChenYu

Re: [PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to access the GMAC register

[Chen-Yu Tsai]

On Mon, Mar 19, 2018 at 5:31 AM, Maxime Ripard
<maxime.rip...@bootlin.com> wrote:
> On Sat, Mar 17, 2018 at 05:28:47PM +0800, Chen-Yu Tsai wrote:
>> From: Icenowy Zheng <icen...@aosc.io>
>>
>> There's a GMAC configuration register, which exists on A64/A83T/H3/H5 in
>> the syscon part, in the CCU of R40 SoC.
>>
>> Export a regmap of the CCU.
>>
>> Read access is not restricted to all registers, but only the GMAC
>> register is allowed to be written.
>>
>> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
>> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>
> Gah, this is crazy. I'm really starting to regret letting that syscon
> in in the first place...

IMHO syscon is really a better fit. It's part of the glue layer and
most other dwmac user platforms treat it as such and use a syscon.
Plus the controls encompass delays (phase), inverters (polarity),
and even signal routing. It's not really just a group of clock controls,
like what we poorly modeled for A20/A31. I think that was really a
mistake.

As I mentioned in the cover letter, a slightly saner approach would
be to let drivers add custom syscon entries, which would then require
less custom plumbing.

> And I'm not really looking forward the time where SCPI et al. will be
> mature and we'll have the clock controller completely outside of our
> control.

I don't think it's going to happen for any of the older SoCs. The R40
only stands out because the GMAC controls are in the clock controller
address space, presumably to be like the A20.


ChenYu

[PATCH net-next 01/12] clk: sunxi-ng: r40: rewrite init code to a platform driver

[Chen-Yu Tsai]

From: Icenowy Zheng <icen...@aosc.io>

As we need to register a regmap on the R40 CCU, there needs to be a
device structure bound to the CCU device node.

Rewrite the R40 CCU driver initial code to make it a proper platform
driver, thus we will have a platform device bound to it.

Signed-off-by: Icenowy Zheng <icen...@aosc.io>
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 39 ++--
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c 
b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
index 933f2e68f42a..c3aa839a453d 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
@@ -12,7 +12,8 @@
  */
 
 #include 
-#include 
+#include 
+#include 
 
 #include "ccu_common.h"
 #include "ccu_reset.h"
@@ -1250,17 +1251,17 @@ static struct ccu_mux_nb sun8i_r40_cpu_nb = {
.bypass_index   = 1, /* index of 24 MHz oscillator */
 };
 
-static void __init sun8i_r40_ccu_setup(struct device_node *node)
+static int sun8i_r40_ccu_probe(struct platform_device *pdev)
 {
+   struct resource *res;
void __iomem *reg;
u32 val;
+   int ret;
 
-   reg = of_io_request_and_map(node, 0, of_node_full_name(node));
-   if (IS_ERR(reg)) {
-   pr_err("%s: Could not map the clock registers\n",
-  of_node_full_name(node));
-   return;
-   }
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   reg = devm_ioremap_resource(>dev, res);
+   if (IS_ERR(reg))
+   return PTR_ERR(reg);
 
/* Force the PLL-Audio-1x divider to 4 */
val = readl(reg + SUN8I_R40_PLL_AUDIO_REG);
@@ -1277,7 +1278,9 @@ static void __init sun8i_r40_ccu_setup(struct device_node 
*node)
val &= ~GENMASK(25, 20);
writel(val, reg + SUN8I_R40_USB_CLK_REG);
 
-   sunxi_ccu_probe(node, reg, _r40_ccu_desc);
+   ret = sunxi_ccu_probe(pdev->dev.of_node, reg, _r40_ccu_desc);
+   if (ret)
+   return ret;
 
/* Gate then ungate PLL CPU after any rate changes */
ccu_pll_notifier_register(_r40_pll_cpu_nb);
@@ -1285,6 +1288,20 @@ static void __init sun8i_r40_ccu_setup(struct 
device_node *node)
/* Reparent CPU during PLL CPU rate changes */
ccu_mux_notifier_register(pll_cpu_clk.common.hw.clk,
  _r40_cpu_nb);
+
+   return 0;
 }
-CLK_OF_DECLARE(sun8i_r40_ccu, "allwinner,sun8i-r40-ccu",
-  sun8i_r40_ccu_setup);
+
+static const struct of_device_id sun8i_r40_ccu_ids[] = {
+   { .compatible = "allwinner,sun8i-r40-ccu" },
+   { }
+};
+
+static struct platform_driver sun8i_r40_ccu_driver = {
+   .probe  = sun8i_r40_ccu_probe,
+   .driver = {
+   .name   = "sun8i-r40-ccu",
+   .of_match_table = sun8i_r40_ccu_ids,
+   },
+};
+builtin_platform_driver(sun8i_r40_ccu_driver);
-- 
2.16.2

[PATCH net-next 10/12] ARM: dts: sun8i: r40: bananapi-m2-ultra: Sort device node dereferences

[Chen-Yu Tsai]

The device nodes dereference () usages should be sorted by the label
names, barring any parsing order issues such as the #include statement
for the PMIC's .dtsi file that must come after the PMIC.

Move the mmc and ohci blocks in front of the PMIC's regulator blocks.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 69 ---
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts 
b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
index 27d9ccd0ef2f..c6da21e43572 100644
--- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
@@ -114,6 +114,41 @@
 
 #include "axp22x.dtsi"
 
+ {
+   vmmc-supply = <_dcdc1>;
+   bus-width = <4>;
+   cd-gpios = < 7 13 GPIO_ACTIVE_HIGH>; /* PH13 */
+   cd-inverted;
+   status = "okay";
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pg_pins>;
+   vmmc-supply = <_dldo2>;
+   vqmmc-supply = <_dldo1>;
+   mmc-pwrseq = <_pwrseq>;
+   bus-width = <4>;
+   non-removable;
+   status = "okay";
+};
+
+ {
+   vmmc-supply = <_dcdc1>;
+   vqmmc-supply = <_dcdc1>;
+   bus-width = <8>;
+   non-removable;
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
 _aldo3 {
regulator-always-on;
regulator-min-microvolt = <270>;
@@ -161,40 +196,6 @@
regulator-name = "vcc-wifi";
 };
 
- {
-   vmmc-supply = <_dcdc1>;
-   bus-width = <4>;
-   cd-gpios = < 7 13 GPIO_ACTIVE_LOW>; /* PH13 */
-   status = "okay";
-};
-
- {
-   pinctrl-names = "default";
-   pinctrl-0 = <_pg_pins>;
-   vmmc-supply = <_dldo2>;
-   vqmmc-supply = <_dldo1>;
-   mmc-pwrseq = <_pwrseq>;
-   bus-width = <4>;
-   non-removable;
-   status = "okay";
-};
-
- {
-   vmmc-supply = <_dcdc1>;
-   vqmmc-supply = <_dcdc1>;
-   bus-width = <8>;
-   non-removable;
-   status = "okay";
-};
-
- {
-   status = "okay";
-};
-
- {
-   status = "okay";
-};
-
  {
pinctrl-names = "default";
pinctrl-0 = <_pb_pins>;
-- 
2.16.2

[PATCH net-next 09/12] net: stmmac: dwmac-sun8i: Add support for GMAC on Allwinner R40 SoC

[Chen-Yu Tsai]

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds support for it using the framework laid out by previous
patches to map the differences.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 08d263567a52..be6705e89e95 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -97,6 +97,13 @@ const struct reg_field sun8i_syscon_reg_field = {
.msb = 31,
 };
 
+/* EMAC clock register @ 0x164 in the CCU address range */
+const struct reg_field sun8i_ccu_reg_field = {
+   .reg = 0x164,
+   .lsb = 0,
+   .msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
.syscon_field = _syscon_reg_field,
@@ -125,6 +132,15 @@ static const struct emac_variant emac_variant_a83t = {
.tx_delay_max = 7,
 };
 
+static const struct emac_variant emac_variant_r40 = {
+   .default_syscon_value = 0,
+   .syscon_field = _ccu_reg_field,
+   .syscon_from_dev = true,
+   .support_mii = true,
+   .support_rgmii = true,
+   .rx_delay_max = 7,
+};
+
 static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
.syscon_field = _syscon_reg_field,
@@ -1148,6 +1164,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
.data = _variant_v3s },
{ .compatible = "allwinner,sun8i-a83t-emac",
.data = _variant_a83t },
+   { .compatible = "allwinner,sun8i-r40-gmac",
+   .data = _variant_r40 },
{ .compatible = "allwinner,sun50i-a64-emac",
.data = _variant_a64 },
{ }
-- 
2.16.2

[PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to access the GMAC register

[Chen-Yu Tsai]

From: Icenowy Zheng <icen...@aosc.io>

There's a GMAC configuration register, which exists on A64/A83T/H3/H5 in
the syscon part, in the CCU of R40 SoC.

Export a regmap of the CCU.

Read access is not restricted to all registers, but only the GMAC
register is allowed to be written.

Signed-off-by: Icenowy Zheng <icen...@aosc.io>
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c 
b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
index c3aa839a453d..54c7a6106206 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c
@@ -1251,9 +1251,35 @@ static struct ccu_mux_nb sun8i_r40_cpu_nb = {
.bypass_index   = 1, /* index of 24 MHz oscillator */
 };
 
+/*
+ * Add a regmap for the GMAC driver (dwmac-sun8i) to access the
+ * GMAC configuration register.
+ * Only this register is allowed to be written, in order to
+ * prevent overriding critical clock configuration.
+ */
+
+#define SUN8I_R40_GMAC_CFG_REG 0x164
+static bool sun8i_r40_ccu_regmap_writeable_reg(struct device *dev,
+  unsigned int reg)
+{
+   if (reg == SUN8I_R40_GMAC_CFG_REG)
+   return true;
+   return false;
+}
+
+static struct regmap_config sun8i_r40_ccu_regmap_config = {
+   .reg_bits   = 32,
+   .val_bits   = 32,
+   .reg_stride = 4,
+   .max_register   = 0x320, /* PLL_LOCK_CTRL_REG */
+
+   .writeable_reg  = sun8i_r40_ccu_regmap_writeable_reg,
+};
+
 static int sun8i_r40_ccu_probe(struct platform_device *pdev)
 {
struct resource *res;
+   struct regmap *regmap;
void __iomem *reg;
u32 val;
int ret;
@@ -1278,6 +1304,11 @@ static int sun8i_r40_ccu_probe(struct platform_device 
*pdev)
val &= ~GENMASK(25, 20);
writel(val, reg + SUN8I_R40_USB_CLK_REG);
 
+   regmap = devm_regmap_init_mmio(>dev, reg,
+  _r40_ccu_regmap_config);
+   if (IS_ERR(regmap))
+   return PTR_ERR(regmap);
+
ret = sunxi_ccu_probe(pdev->dev.of_node, reg, _r40_ccu_desc);
if (ret)
return ret;
-- 
2.16.2

[PATCH net-next 04/12] dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order

[Chen-Yu Tsai]

The A83T syscon compatible was appended to the syscon compatibles list,
instead of inserted in to preserve the ordering.

Move it to the proper place to keep the list sorted.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index b8a3028d6c30..74b3ef79e57a 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -22,10 +22,10 @@ Required properties:
 - #size-cells: shall be 0
 - syscon: A phandle to the syscon of the SoC with one of the following
  compatible string:
+  - allwinner,sun8i-a83t-system-controller
   - allwinner,sun8i-h3-system-controller
   - allwinner,sun8i-v3s-system-controller
   - allwinner,sun50i-a64-system-controller
-  - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
-- 
2.16.2

[PATCH net-next 08/12] net: stmmac: dwmac-sun8i: Support different ranges for TX/RX delay chains

[Chen-Yu Tsai]

On the R40 SoC, the RX delay chain only has a range of 0~7 (hundred
picoseconds), instead of 0~31. Also the TX delay chain is completely
absent.

This patch adds support for different ranges by adding per-compatible
maximum values in the variant data. A maximum of 0 indicates that the
delay chain is not supported or absent.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 32 ---
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a51175bcfd11..08d263567a52 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -50,6 +50,12 @@
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
  * @support_rgmii: Does the MAC handle RGMII
+ *
+ * @rx_delay_max:  Maximum raw value for RX delay chain
+ * @tx_delay_max:  Maximum raw value for TX delay chain
+ * These two also indicate the bitmask for
+ * the RX and TX delay chain registers. A
+ * value of zero indicates this is not supported.
  */
 struct emac_variant {
u32 default_syscon_value;
@@ -59,6 +65,8 @@ struct emac_variant {
bool support_mii;
bool support_rmii;
bool support_rgmii;
+   u8 rx_delay_max;
+   u8 tx_delay_max;
 };
 
 /* struct sunxi_priv_data - hold all sunxi private data
@@ -95,7 +103,9 @@ static const struct emac_variant emac_variant_h3 = {
.soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_v3s = {
@@ -110,7 +120,9 @@ static const struct emac_variant emac_variant_a83t = {
.syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 static const struct emac_variant emac_variant_a64 = {
@@ -119,7 +131,9 @@ static const struct emac_variant emac_variant_a64 = {
.soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
-   .support_rgmii = true
+   .support_rgmii = true,
+   .rx_delay_max = 31,
+   .tx_delay_max = 7,
 };
 
 #define EMAC_BASIC_CTL0 0x00
@@ -223,9 +237,7 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides EPIT) */
 
 /* Generic system control EMAC_CLK bits */
-#define SYSCON_ETXDC_MASK  GENMASK(2, 0)
 #define SYSCON_ETXDC_SHIFT 10
-#define SYSCON_ERXDC_MASK  GENMASK(4, 0)
 #define SYSCON_ERXDC_SHIFT 5
 /* EMAC PHY Interface Type */
 #define SYSCON_EPITBIT(2) /* 1: RGMII, 0: MII */
@@ -851,8 +863,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
}
val /= 100;
dev_dbg(priv->device, "set tx-delay to %x\n", val);
-   if (val <= SYSCON_ETXDC_MASK) {
-   reg &= ~(SYSCON_ETXDC_MASK << SYSCON_ETXDC_SHIFT);
+   if (val <= gmac->variant->tx_delay_max) {
+   reg &= ~(gmac->variant->tx_delay_max <<
+SYSCON_ETXDC_SHIFT);
reg |= (val << SYSCON_ETXDC_SHIFT);
} else {
dev_err(priv->device, "Invalid TX clock delay: %d\n",
@@ -868,8 +881,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
}
val /= 100;
dev_dbg(priv->device, "set rx-delay to %x\n", val);
-   if (val <= SYSCON_ERXDC_MASK) {
-   reg &= ~(SYSCON_ERXDC_MASK << SYSCON_ERXDC_SHIFT);
+   if (val <= gmac->variant->rx_delay_max) {
+   reg &= ~(gmac->variant->rx_delay_max <<
+SYSCON_ERXDC_SHIFT);
reg |= (val << SYSCON_ERXDC_SHIFT);
} else {
dev_err(priv->device, "Invalid RX clock delay: %d\n",
-- 
2.16.2

[PATCH net-next 06/12] net: stmmac: dwmac-sun8i: Use regmap_field for syscon register access

[Chen-Yu Tsai]

On the Allwinner R40, the "GMAC clock" register is located in the CCU
block, at a different register address than the other SoCs that have
it in the "system control" block.

This patch converts the use of regmap to regmap_field for mapping and
accessing the syscon register, so we can have the register address in
the variants data, 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 <w...@csie.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 42 +--
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a3fa65b1ca8e..de93f0faf58d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -42,6 +42,7 @@
  * This value is used for disabling properly EMAC
  * and used as a good starting value in case of the
  * boot process(uboot) leave some stuff.
+ * @syscon_field   reg_field for the syscon's gmac register
  * @soc_has_internal_phy:  Does the MAC embed an internal PHY
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
@@ -49,6 +50,7 @@
  */
 struct emac_variant {
u32 default_syscon_value;
+   const struct reg_field *syscon_field;
bool soc_has_internal_phy;
bool support_mii;
bool support_rmii;
@@ -71,13 +73,21 @@ 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;
 };
 
+/* EMAC clock register @ 0x30 in the "system control" address range */
+const struct reg_field sun8i_syscon_reg_field = {
+   .reg = 0x30,
+   .lsb = 0,
+   .msb = 31,
+};
+
 static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
@@ -86,12 +96,14 @@ static const struct emac_variant emac_variant_h3 = {
 
 static const struct emac_variant emac_variant_v3s = {
.default_syscon_value = 0x38000,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = true,
.support_mii = true
 };
 
 static const struct emac_variant emac_variant_a83t = {
.default_syscon_value = 0,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
.support_rgmii = true
@@ -99,6 +111,7 @@ static const struct emac_variant emac_variant_a83t = {
 
 static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
+   .syscon_field = _syscon_reg_field,
.soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
@@ -216,7 +229,6 @@ static const struct emac_variant emac_variant_a64 = {
 #define SYSCON_ETCS_MII0x0
 #define SYSCON_ETCS_EXT_GMII   0x1
 #define SYSCON_ETCS_INT_GMII   0x2
-#define SYSCON_EMAC_REG0x30
 
 /* sun8i_dwmac_dma_reset() - reset the EMAC
  * Called from stmmac via stmmac_dma_ops->reset
@@ -745,7 +757,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 +775,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 +813,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
int ret;
u32 reg, val;
 
-   regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
+   regmap_field_read(gmac->regmap_field, );
reg = gmac->variant->default_syscon_value;
   

[PATCH net-next 05/12] dt-bindings: net: dwmac-sun8i: Add binding for GMAC on Allwinner R40 SoC

[Chen-Yu Tsai]

The Allwinner R40 SoC has the EMAC controller supported by dwmac-sun8i.
It is named "GMAC", while EMAC refers to the 10/100 Mbps Ethernet
controller supported by sun4i-emac. The controller is the same, but
the R40 has the glue layer controls in the clock control unit (CCU),
with a reduced RX delay chain, and no TX delay chain.

This patch adds the R40 specific bits to the dwmac-sun8i binding.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 74b3ef79e57a..fe4a48a1eb50 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -7,6 +7,7 @@ Required properties:
 - compatible: must be one of the following string:
"allwinner,sun8i-a83t-emac"
"allwinner,sun8i-h3-emac"
+   "allwinner,sun8i-r40-gmac"
"allwinner,sun8i-v3s-emac"
"allwinner,sun50i-a64-emac"
 - reg: address and length of the register for the device.
@@ -24,14 +25,17 @@ Required properties:
  compatible string:
   - allwinner,sun8i-a83t-system-controller
   - allwinner,sun8i-h3-system-controller
+  - allwinner,sun8i-r40-ccu
   - allwinner,sun8i-v3s-system-controller
   - allwinner,sun50i-a64-system-controller
 
 Optional properties:
 - allwinner,tx-delay-ps: TX clock delay chain value in ps.
 Range is 0-700. Default is 0.
+Unavailable for allwinner,sun8i-r40-gmac
 - allwinner,rx-delay-ps: RX clock delay chain value in ps.
 Range is 0-3100. Default is 0.
+Range is 0-700 for allwinner,sun8i-r40-gmac
 Both delay properties need to be a multiple of 100. They control the
 clock delay for external RGMII PHY. They are do apply to the internal
 PHY or external non-RGMII PHYs.
-- 
2.16.2

[PATCH net-next 07/12] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from CCU device

[Chen-Yu Tsai]

On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU
address space. Using a standard syscon to access it provides no
coordination with the CCU driver for register access. Neither does
it prevent this and other drivers from accessing other, maybe critical,
clock control registers.

Instead, for these types of setups, we let the CCU register a proper
device and a regmap tied to it. We can then get the device from the
phandle, and retrieve the regmap with dev_get_regmap().

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 38 ++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index de93f0faf58d..a51175bcfd11 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -43,6 +43,9 @@
  * and used as a good starting value in case of the
  * boot process(uboot) leave some stuff.
  * @syscon_field   reg_field for the syscon's gmac register
+ * @syscon_from_devsyscon regmap is in ccu address space and
+ * needs to be retrieved using dev_get_regmap()
+ * instead of syscon_regmap_lookup_by_phandle()
  * @soc_has_internal_phy:  Does the MAC embed an internal PHY
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
@@ -51,6 +54,7 @@
 struct emac_variant {
u32 default_syscon_value;
const struct reg_field *syscon_field;
+   bool syscon_from_dev;
bool soc_has_internal_phy;
bool support_mii;
bool support_rmii;
@@ -983,6 +987,34 @@ static struct mac_device_info *sun8i_dwmac_setup(void 
*ppriv)
return mac;
 }
 
+static struct regmap *sun8i_dwmac_get_syscon_from_dev(struct device_node *node)
+{
+   struct device_node *syscon_node;
+   struct platform_device *syscon_pdev;
+   struct regmap *regmap = NULL;
+
+   syscon_node = of_parse_phandle(node, "syscon", 0);
+   if (!syscon_node)
+   return ERR_PTR(-ENODEV);
+
+   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 the other device driver is at fault */
+   regmap = dev_get_regmap(_pdev->dev, NULL);
+   if (!regmap)
+   regmap = ERR_PTR(-EINVAL);
+
+   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;
@@ -1027,7 +1059,11 @@ static int sun8i_dwmac_probe(struct platform_device 
*pdev)
gmac->regulator = NULL;
}
 
-   regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "syscon");
+   if (gmac->variant->syscon_from_dev)
+   regmap = sun8i_dwmac_get_syscon_from_dev(pdev->dev.of_node);
+   else
+   regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+"syscon");
if (IS_ERR(regmap)) {
ret = PTR_ERR(regmap);
dev_err(>dev, "Unable to map syscon: %d\n", ret);
-- 
2.16.2

[PATCH net-next 03/12] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions

[Chen-Yu Tsai]

The clock delay chains found in the glue layer for dwmac-sun8i are only
used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
range of possible values for the delay chain.

This patch reformats the delay chain section of the device tree binding
to make it clear that the delay chains only apply to RGMII PHYs, and
make it easier to add the R40-specific bits later.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 3d6d5fa0c4d5..b8a3028d6c30 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -28,10 +28,13 @@ Required properties:
   - allwinner,sun8i-a83t-system-controller
 
 Optional properties:
-- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 
0-700. Default is 0)
-- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 
0-3100. Default is 0)
-Both delay properties need to be a multiple of 100. They control the delay for
-external PHY.
+- allwinner,tx-delay-ps: TX clock delay chain value in ps.
+Range is 0-700. Default is 0.
+- allwinner,rx-delay-ps: RX clock delay chain value in ps.
+Range is 0-3100. Default is 0.
+Both delay properties need to be a multiple of 100. They control the
+clock delay for external RGMII PHY. They are do apply to the internal
+PHY or external non-RGMII PHYs.
 
 Optional properties for the following compatibles:
   - "allwinner,sun8i-h3-emac",
-- 
2.16.2

[PATCH net-next 11/12] ARM: dts: sun8i: r40: Add device node and RGMII pinmux node for GMAC

[Chen-Yu Tsai]

The R40 SoC has a GMAC (gigabit capable Ethernet controller). Add a
device node for it. The only publicly available board for this SoC
uses an RGMII PHY. Add a pinmux node for it as well.

Since this SoC also has an old 10/100 Mbps EMAC, which also has an
MDIO bus controller, the MDIO bus for the GMAC is labeled "gmac_mdio".

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 arch/arm/boot/dts/sun8i-r40.dtsi | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 173dcc1652d2..bd97ca3dc2fa 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -265,6 +265,19 @@
#interrupt-cells = <3>;
#gpio-cells = <3>;
 
+   gmac_rgmii_pins: gmac-rgmii-pins {
+   pins = "PA0", "PA1", "PA2", "PA3",
+  "PA4", "PA5", "PA6", "PA7",
+  "PA8", "PA10", "PA11", "PA12",
+  "PA13", "PA15", "PA16";
+   function = "gmac";
+   /*
+* data lines in RGMII mode use DDR mode
+* and need a higher signal drive strength
+*/
+   drive-strength = <40>;
+   };
+
i2c0_pins: i2c0-pins {
pins = "PB0", "PB1";
function = "i2c0";
@@ -451,6 +464,27 @@
#size-cells = <0>;
};
 
+   gmac: ethernet@1c5 {
+   compatible = "allwinner,sun8i-r40-gmac";
+   syscon = <>;
+   reg = <0x01c5 0x1>;
+   interrupts = ;
+   interrupt-names = "macirq";
+   resets = < RST_BUS_GMAC>;
+   reset-names = "stmmaceth";
+   clocks = < CLK_BUS_GMAC>;
+   clock-names = "stmmaceth";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";
+
+   gmac_mdio: mdio {
+   compatible = "snps,dwmac-mdio";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+   };
+
gic: interrupt-controller@1c81000 {
compatible = "arm,gic-400";
reg = <0x01c81000 0x1000>,
-- 
2.16.2

[PATCH net-next 12/12] ARM: dts: sun8i: r40: bananapi-m2-ultra: Enable GMAC ethernet controller

[Chen-Yu Tsai]

The Bananapi M2 Ultra has a Realtek RTL8211E RGMII PHY tied to the GMAC.
The PMIC's DC1SW output provides power for the PHY, while the ALDO2
output provides I/O voltages on both sides.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 30 +++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts 
b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
index c6da21e43572..25fb048c7df2 100644
--- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
@@ -51,6 +51,7 @@
compatible = "sinovoip,bpi-m2-ultra", "allwinner,sun8i-r40";
 
aliases {
+   ethernet0 = 
serial0 = 
};
 
@@ -101,6 +102,22 @@
status = "okay";
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_rgmii_pins>;
+   phy-handle = <>;
+   phy-mode = "rgmii";
+   phy-supply = <_dc1sw>;
+   status = "okay";
+};
+
+_mdio {
+   phy1: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <1>;
+   };
+};
+
  {
status = "okay";
 
@@ -149,6 +166,13 @@
status = "okay";
 };
 
+_aldo2 {
+   regulator-always-on;
+   regulator-min-microvolt = <250>;
+   regulator-max-microvolt = <250>;
+   regulator-name = "vcc-pa";
+};
+
 _aldo3 {
regulator-always-on;
regulator-min-microvolt = <270>;
@@ -156,6 +180,12 @@
regulator-name = "avcc";
 };
 
+_dc1sw {
+   regulator-min-microvolt = <300>;
+   regulator-max-microvolt = <300>;
+   regulator-name = "vcc-gmac-phy";
+};
+
 _dcdc1 {
regulator-always-on;
regulator-min-microvolt = <300>;
-- 
2.16.2

[PATCH net-next 00/12] ARM: sun8i: r40: Add Ethernet support

[Chen-Yu Tsai]

Hi everyone,

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.

Mark (Brown), could you take a look at the regmap bits in patches 2 and
7? 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.

Patch 3 and 4 clean up the dwmac-sun8i binding.

Patch 5 adds device tree binding for Allwinner R40's Ethernet
controller.

Patch 6 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 7 introduces custom plumbing in the dwmac-sun8i driver to fetch
a regmap from the clock module, by looking up a device via a phandle,
then getting the regmap associated with that device.

Patch 8 adds support for different or absent TX/RX delay chain ranges
to the dwmac-sun8i driver.

Patch 9 adds support for the R40's ethernet controller.

Patch 10 cleans up the Bananapi M2 Ultra device tree file.

Patch 11 adds a GMAC device node and RGMII mode pinmux setting for the
R40.

Patch 12 enables Ethernet on the Bananapi M2 Ultra.


Please have a look.

Regards
ChenYu

Chen-Yu Tsai (10):
  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: 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 CCU 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 (2):
  clk: sunxi-ng: r40: rewrite init code to a platform driver
  clk: sunxi-ng: r40: export a regmap to access the GMAC register

 .../devicetree/bindings/net/dwmac-sun8i.txt|  17 ++-
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts  |  99 ++--
 arch/arm/boot/dts/sun8i-r40.dtsi   |  34 ++
 drivers/clk/sunxi-ng/ccu-sun8i-r40.c   |  70 +--
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 128 +
 5 files changed, 278 insertions(+), 70 deletions(-)

-- 
2.16.2

Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling

[Chen-Yu Tsai]

On Wed, Nov 29, 2017 at 11:46 PM, Andrew Lunn  wrote:
> Hi ChenYu
>
>> It worked at one point. During some previous iteration, they lit up as
>> they were supposed to.
>
> For a released version of the kernel? Or during development work?  If
> they did work, but broken, it would be good to know which commit broke
> it. We can then add a fixes: tag to the patch as proposed.

During development work. The bindings / driver was never released.

ChenYu

Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling

[Chen-Yu Tsai]

On Wed, Nov 29, 2017 at 11:37 PM, Andrew Lunn  wrote:
> On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
>> On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
>> > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
>> > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
>> > > the binding doc expect it to be in MAC node.
>> > >
>> > > Since all board DT use it also in MAC node, the driver need to search
>> > > allwinner,leds-active-low in MAC node.
>> >
>> > Hi Corentin
>> >
>> > I'm having trouble working out how this worked before. This is code
>> > you moved around, when adding external/internal MDIOs. But the very
>> > first version of this driver code used priv->plat->phy_node. Did that
>> > somehow point to the MAC node when the internal PHY is used? Or has it
>> > been broken all the time?
>> >
>>
>> Hello
>>
>
>> Since this feature control only when the activity LED need to blink,
>> nobody see that it was broken.
>
> Hi Corentin
>
> So it never worked?
>
> If it never worked, moving the DT properties into the PHY node, where
> they belong, won't introduce a regression :-)

It worked at one point. During some previous iteration, they lit up as
they were supposed to.

ChenYu

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Chen-Yu Tsai]

On Fri, Sep 8, 2017 at 3:36 PM, Corentin Labbe
 wrote:
> On Fri, Sep 08, 2017 at 09:19:54AM +0200, Maxime Ripard wrote:
>> On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
>> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts 
>> > b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > index 1c2387bd5df6..968908761194 100644
>> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > @@ -50,6 +50,7 @@
>> > compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
>> >
>> > aliases {
>> > +   ethernet0 = 
>> > serial0 = 
>> > };
>> >
>> > @@ -108,6 +109,22 @@
>> > status = "okay";
>> >  };
>> >
>> > + {
>> > +   pinctrl-names = "default";
>> > +   pinctrl-0 = <_rgmii_pins>;
>> > +   phy-supply = <_gmac_3v3>;
>> > +   phy-handle = <_rgmii_phy>;
>> > +   phy-mode = "rgmii";
>> > +   status = "okay";
>> > +};
>> > +
>> > + {
>> > +   ext_rgmii_phy: ethernet-phy@7 {
>> > +   compatible = "ethernet-phy-ieee802.3-c22";
>> > +   reg = <7>;
>> > +   };
>> > +};
>> > +
>>
>> This won't compile, you don't have that node in the H5 DTSI.
>>
>
> Since H5 DTSI include arm/sunxi-h3-h5.dtsi it compiles.
> Furthermore, I restested just now and confirm, it compiles fine.

The order of your patches are wrong. No individual patch should
introduce build failures, not just the whole patch series.

ChenYu

Re: [PATCH 0/4] net: stmmac: revert the EMAC bindings

[Chen-Yu Tsai]

On Sat, Aug 26, 2017 at 3:12 AM, Maxime Ripard
<maxime.rip...@free-electrons.com> wrote:
> Hi,
>
> The bindings of the stmmac glue for the new Allwinner EMAC controller
> are still controversial and being discussed, even though they've been
> merged in 4.13.
>
> In order not to introduce any binding we do not really want to commit
> to in a stable release, especially since that would mean we would have
> to support both the right and old bindings, let's revert them.
>
> We will reintroduce them in due time, once the discussion has settled
> down.
>
> The first three patches should go through the arm-soc tree, the last
> one through the net tree. All of them must be treated as fixes.
>
> Thanks!
> Maxime
>
> Maxime Ripard (4):
>   dt-bindings: net: Revert sun8i dwmac binding
>   arm64: dts: allwinner: Revert EMAC changes
>   arm: dts: sunxi: Revert EMAC changes
>   net: stmmac: sun8i: Remove the compatibles
>
>  .../devicetree/bindings/net/dwmac-sun8i.txt| 84 
> --
>  arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts  |  9 ---
>  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 19 -
>  arch/arm/boot/dts/sun8i-h3-beelink-x2.dts  |  8 ---

I think this particular change is in -next, not v4.13-rc.

Otherwise, whole series is

Acked-by: Chen-Yu Tsai <w...@csie.org>

>  arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts  |  7 --
>  arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  |  8 ---
>  arch/arm/boot/dts/sun8i-h3-orangepi-one.dts|  8 ---
>  arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts|  5 --
>  arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |  8 ---
>  arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts   | 22 --
>  arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts | 16 -
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 26 ---
>  .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts | 17 -
>  .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts  | 15 
>  .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 18 -
>  .../dts/allwinner/sun50i-a64-sopine-baseboard.dts  | 17 -
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi  | 20 --
>  .../boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts   | 17 -
>  .../boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts  | 17 -
>  .../dts/allwinner/sun50i-h5-orangepi-prime.dts | 17 -
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  |  8 ---
>  21 files changed, 366 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/dwmac-sun8i.txt
>
> --
> 2.13.5
>

Re: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac

[Chen-Yu Tsai]

On Fri, Aug 25, 2017 at 11:05 AM, Florian Fainelli <f.faine...@gmail.com> wrote:
>
>
> On 08/24/2017 07:54 PM, Chen-Yu Tsai wrote:
>> On Fri, Aug 25, 2017 at 3:59 AM, Florian Fainelli <f.faine...@gmail.com> 
>> wrote:
>>> On 08/24/2017 01:21 AM, Corentin Labbe wrote:
>>>> On Wed, Aug 23, 2017 at 09:31:53AM -0700, Florian Fainelli wrote:
>>>>> On 08/23/2017 12:49 AM, Maxime Ripard wrote:
>>>>>> Hi Florian,
>>>>>>
>>>>>> On Tue, Aug 22, 2017 at 11:35:01AM -0700, Florian Fainelli wrote:
>>>>>>>>>> So I think what you are saying is either impossible or 
>>>>>>>>>> engineering-wise
>>>>>>>>>> a very stupid design, like using an external MAC with a discrete PHY
>>>>>>>>>> connected to the internal MAC's MDIO bus, while using the internal 
>>>>>>>>>> MAC
>>>>>>>>>> with the internal PHY.
>>>>>>>>>>
>>>>>>>>>> Now can we please decide on something? We're a week and a half from
>>>>>>>>>> the 4.13 release. If mdio-mux is wrong, then we could have two mdio
>>>>>>>>>> nodes (internal-mdio & external-mdio).
>>>>>>>>>
>>>>>>>>> I really don't see a need for a mdio-mux in the first place, just have
>>>>>>>>> one MDIO controller (current state) sub-node which describes the
>>>>>>>>> built-in STMMAC MDIO controller and declare the internal PHY as a 
>>>>>>>>> child
>>>>>>>>> node (along with 'phy-is-integrated'). If a different configuration is
>>>>>>>>> used, then just put the external PHY as a child node there.
>>>>>>>>>
>>>>>>>>> If fixed-link is required, the mdio node becomes unused anyway.
>>>>>>>>>
>>>>>>>>> Works for everyone?
>>>>>>>>
>>>>>>>> If we put an external PHY with reg=1 as a child of internal MDIO,
>>>>>>>> il will be merged with internal PHY node and get
>>>>>>>> phy-is-integrated.
>>>>>>>
>>>>>>> Then have the .dtsi file contain just the mdio node, but no internal or
>>>>>>> external PHY and push all the internal and external PHY node definition
>>>>>>> (in its entirety) to the per-board DTS file, does not that work?
>>>>>>
>>>>>> If possible, I'd really like to have the internal PHY in the
>>>>>> DTSI. It's always there in hardware anyway, and duplicating the PHY,
>>>>>> with its clock, reset line, and whatever info we might need in the
>>>>>> future in each and every board DTS that uses it will be very error
>>>>>> prone and we will have the usual bunch of issues that come up with
>>>>>> duplication.
>>>>>
>>>>> OK, then what if you put the internal PHY in the DTSI, mark it with a
>>>>> status = "disabled" property, and have the per-board DTS put a status =
>>>>> "okay" property along with a "phy-is-integrated" boolean property? Would
>>>>> that work?
>>>>
>>>> No, I tested and for example with sun8i-h3-orangepi-plus.dts, the external 
>>>> PHY (ethernet-phy@1) is still merged.
>>>
>>> Is not there is a mistake in the unit address not matching the "reg"
>>> property, or am I not looking at the right tree?
>>>
>>>  {
>>> ext_rgmii_phy: ethernet-phy@1 {
>>> compatible = "ethernet-phy-ieee802.3-c22";
>>> reg = <0>;
>>> };
>>> };
>>>
>>> If the PHY is really at MDIO address 0, then it should be
>>> ethernet-phy@0, and not ethernet-phy@1, and then no problem with the
>>> merging?
>>
>> That is wrong. The board described in the example likely has a Realtek
>> RTL8211E @ address 0x1. Address 0 for this PHY is a broadcast address,
>> so it still works, but is the wrong representation.
>>
>>>
>>>
>>>> So that adding a 'status = "disabled"' does not bring anything.
>>>>
>>>>>
>>>>> What I really don't think is necessary is:
>>

Re: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac

[Chen-Yu Tsai]

On Fri, Aug 25, 2017 at 3:59 AM, Florian Fainelli  wrote:
> On 08/24/2017 01:21 AM, Corentin Labbe wrote:
>> On Wed, Aug 23, 2017 at 09:31:53AM -0700, Florian Fainelli wrote:
>>> On 08/23/2017 12:49 AM, Maxime Ripard wrote:
 Hi Florian,

 On Tue, Aug 22, 2017 at 11:35:01AM -0700, Florian Fainelli wrote:
 So I think what you are saying is either impossible or engineering-wise
 a very stupid design, like using an external MAC with a discrete PHY
 connected to the internal MAC's MDIO bus, while using the internal MAC
 with the internal PHY.

 Now can we please decide on something? We're a week and a half from
 the 4.13 release. If mdio-mux is wrong, then we could have two mdio
 nodes (internal-mdio & external-mdio).
>>>
>>> I really don't see a need for a mdio-mux in the first place, just have
>>> one MDIO controller (current state) sub-node which describes the
>>> built-in STMMAC MDIO controller and declare the internal PHY as a child
>>> node (along with 'phy-is-integrated'). If a different configuration is
>>> used, then just put the external PHY as a child node there.
>>>
>>> If fixed-link is required, the mdio node becomes unused anyway.
>>>
>>> Works for everyone?
>>
>> If we put an external PHY with reg=1 as a child of internal MDIO,
>> il will be merged with internal PHY node and get
>> phy-is-integrated.
>
> Then have the .dtsi file contain just the mdio node, but no internal or
> external PHY and push all the internal and external PHY node definition
> (in its entirety) to the per-board DTS file, does not that work?

 If possible, I'd really like to have the internal PHY in the
 DTSI. It's always there in hardware anyway, and duplicating the PHY,
 with its clock, reset line, and whatever info we might need in the
 future in each and every board DTS that uses it will be very error
 prone and we will have the usual bunch of issues that come up with
 duplication.
>>>
>>> OK, then what if you put the internal PHY in the DTSI, mark it with a
>>> status = "disabled" property, and have the per-board DTS put a status =
>>> "okay" property along with a "phy-is-integrated" boolean property? Would
>>> that work?
>>
>> No, I tested and for example with sun8i-h3-orangepi-plus.dts, the external 
>> PHY (ethernet-phy@1) is still merged.
>
> Is not there is a mistake in the unit address not matching the "reg"
> property, or am I not looking at the right tree?
>
>  {
> ext_rgmii_phy: ethernet-phy@1 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <0>;
> };
> };
>
> If the PHY is really at MDIO address 0, then it should be
> ethernet-phy@0, and not ethernet-phy@1, and then no problem with the
> merging?

That is wrong. The board described in the example likely has a Realtek
RTL8211E @ address 0x1. Address 0 for this PHY is a broadcast address,
so it still works, but is the wrong representation.

>
>
>> So that adding a 'status = "disabled"' does not bring anything.
>>
>>>
>>> What I really don't think is necessary is:
>>>
>>> - duplicating the "mdio" controller node for external vs. internal PHY,
>>> because this is not accurate, there is just one MDIO controller, but
>>> there may be different kinds of MDIO/PHY devices attached
>>
>> For me, if we want to represent the reality, we need two MDIO:
>> - since two PHY at the same address could co-exists
>> - since they are isolated so not on the same MDIO bus
>
> Is that really true? It might be, but from experience with e.g:
> bcmgenet, the integrated PHY and the external PHYs are on the same MDIO
> bus, which is convenient, except when you have an address conflict.

There's a mux in the hardware: either the internal MDIO+MII lines
from the internal PHY are connected to the MAC, or the external
MDIO+MII lines from the pin controller are connected. I believe
this was already mentioned?

>
>>
>>> - having the STMMAC driver MDIO probing code having to deal with a
>>> "mdio" sub-node or an "internal-mdio" sub-node because this is confusing
>>> and requiring more driver-level changes that are error prone
>>
>> My patch for stmmac is really small, only the name of my variable 
>> ("need_mdio_mux_ids")
>> have to be changed to something like "register_parent_mdio"
>>
>>
>> So I agree with Maxime, we need to avoid merging PHY nodes, and we can avoid 
>> it only by having two separate MDIO nodes.
>> Furthermore, with only one MDIO, we will face with lots of small patch for 
>> adding phy-is-integrated, with two we do not need to change any board DT, 
>> all is simply clean.
>> Really having two MDIO seems cleaner.
>
> The only valid thing that you have provided so far is this merging
> problem. Anything else ranging from "we will face with lots of small
> patch for adding phy-is-integrated" to "Really having two MDIO seems
> 

Re: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac

[Chen-Yu Tsai]

On Mon, Aug 21, 2017 at 10:23 PM, Andrew Lunn  wrote:
>> All muxes are mostly always represented the same way afaik, or do you
>> want to simply introduce a new compatible / property?
>
> + mdio-mux {
> +   compatible = "allwinner,sun8i-h3-mdio-switch";
> +   mdio-parent-bus = <_parent>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   internal_mdio: mdio@1 {
> reg = <1>;
> -   clocks = < CLK_BUS_EPHY>;
> -   resets = < RST_BUS_EPHY>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   int_mii_phy: ethernet-phy@1 {
> +   compatible = "ethernet-phy-ieee802.3-c22";
> +   reg = <1>;
> +   clocks = < CLK_BUS_EPHY>;
> +   resets = < RST_BUS_EPHY>;
> +   phy-is-integrated;
> +   };
> +   };
> +   mdio: mdio@0 {
> +   reg = <0>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> };
>
> Hi Maxim
>
> Anybody who knows the MDIO-mux code/binding, knows that it is a run
> time mux. You swap the mux per MDIO transaction. You can access all
> the PHY and switches on the mux'ed MDIO bus.
>
> However here, it is effectively a boot-time MUX. You cannot change it
> on the fly. What happens when somebody has a phandle to a PHY on the
> internal and a phandle to a phy on the external? Does the driver at
> least return -EINVAL, or -EBUSY? Is there a representation which
> eliminates this possibility?

There is only one controller. Either you use the internal PHY, which
is then directly coupled (no magnetics needed) to the RJ45 port, or
you use an external PHY over MII/RMII/RGMII. You could supposedly
have both on a board, and let the user choose one. But why bother
with the extra complexity and cost? Either you use the internal PHY
at 100M, or an external RGMII PHY for gigabit speeds.

So I think what you are saying is either impossible or engineering-wise
a very stupid design, like using an external MAC with a discrete PHY
connected to the internal MAC's MDIO bus, while using the internal MAC
with the internal PHY.

Now can we please decide on something? We're a week and a half from
the 4.13 release. If mdio-mux is wrong, then we could have two mdio
nodes (internal-mdio & external-mdio).

Regards
ChenYu

Re: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac

[Chen-Yu Tsai]

On Sun, Aug 20, 2017 at 10:25 PM, Andrew Lunn  wrote:
>> I think we cannot use mdio-mux-mmioreg since the register for doing
>> the switch is in middle of the "System Control" and shared with
>> other functions.  This is why we use a sycon/regmap for selecting
>> the MDIO.
>
> You could add a mdio-mux-regmap.c.
>
> However, it probably need restructuring of the stmmac mdio code, to
> make the mdio bus usable as a separate driver. You need stmmac mdio
> to probe first, then mdio-mux-remap should probe, and then lastly
> stmmmac mac driver.
>
> With stmmac mdio and stmmac mac being in one driver, there is no time
> in the middle to allow the mux driver to probe.
>
> It is some effort, but a nice cleanup and generalization.

I'm not sure mdio-mux is the right thing here.

Looking at mdio-mux, it seems to provide a way to access multiplexed
MDIO buses. It says nothing about the MAC<->PHY connection.

With our hardware, and likely Rockchip's as well, the muxed connections
include the MDIO and MII connections, which are muxed at the same time.
It's likely to cause some confusion or even bugs if we implement it as
a separate driver.

ChenYu

Re: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac

[Chen-Yu Tsai]

On Fri, Aug 18, 2017 at 8:21 PM, Corentin Labbe
 wrote:
> In case of a MDIO switch, the registered MDIO node should be
> the parent of the PHY. Otherwise of_phy_connect will fail.
>
> Signed-off-by: Corentin Labbe 
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c 
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index a366b3747eeb..ca3cc99d8960 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -312,10 +312,12 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data 
> *plat,
> static const struct of_device_id need_mdio_ids[] = {
> { .compatible = "snps,dwc-qos-ethernet-4.10" },
> { .compatible = "allwinner,sun8i-a83t-emac" },
> -   { .compatible = "allwinner,sun8i-h3-emac" },
> { .compatible = "allwinner,sun8i-v3s-emac" },
> { .compatible = "allwinner,sun50i-a64-emac" },
> };
> +   static const struct of_device_id need_mdio_mux_ids[] = {
> +   { .compatible = "allwinner,sun8i-h3-emac" },
> +   };
>
> /* If phy-handle property is passed from DT, use it as the PHY */
> plat->phy_node = of_parse_phandle(np, "phy-handle", 0);
> @@ -332,7 +334,13 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data 
> *plat,
> mdio = false;
> }
>
> -   if (of_match_node(need_mdio_ids, np)) {
> +   /*
> +* In case of a MDIO switch/mux, the registered MDIO node should be
> +* the parent of the PHY. Otherwise of_phy_connect will fail.
> +*/
> +   if (of_match_node(need_mdio_mux_ids, np)) {
> +plat->mdio_node =  of_get_parent(plat->phy_node);

Extra space before of_get_parent.

Also this is going to fail horribly if a fixed link is used.

ChenYu

> +   } else if (of_match_node(need_mdio_ids, np)) {
> plat->mdio_node = of_get_child_by_name(np, "mdio");
> } else {
> /**
> --
> 2.13.0
>

Re: [PATCH v3 2/4] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated

[Chen-Yu Tsai]

On Fri, Aug 18, 2017 at 8:21 PM, Corentin Labbe
<clabbe.montj...@gmail.com> wrote:
> The current way to find if the phy is internal is to compare DT phy-mode
> and emac_variant/internal_phy.
> But it will negate a possible future SoC where an external PHY use the
> same phy mode than the internal one.
>
> This patch adds a new way to find if the PHY is internal, via
> the phy-is-integrated property.
>
> Since the internal_phy variable does not need anymore to contain the xMII mode
> used by the internal PHY, it is still used for knowing the presence of an
> internal PHY, so it is modified to a boolean soc_has_internal_phy.
>
> Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>

Acked-by: Chen-Yu Tsai <w...@csie.org>

Re: [PATCH v3 4/4] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Chen-Yu Tsai]

On Fri, Aug 18, 2017 at 8:21 PM, Corentin Labbe
 wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
>
> Signed-off-by: Corentin Labbe 
> ---
>  .../devicetree/bindings/net/dwmac-sun8i.txt| 112 
> +++--
>  1 file changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
> b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 725f3b187886..631084122532 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
>  - allwinner,leds-active-low: EPHY LEDs are active low
>
>  Required child node of emac:
> -- mdio bus node: should be named mdio
> +- mdio bus node: should be named mdio (or mdio_parent in case of H3)

The node would still be named "mdio". You are confusing the node name
with the label. But you don't really need the mdio node for the H3.
You could say here that it is not needed for the H3. See below.

>
>  Required properties of the mdio node:
>  - #address-cells: shall be 1
> @@ -48,14 +48,21 @@ Required properties of the mdio node:
>  The device node referenced by "phy" or "phy-handle" should be a child node
>  of the mdio node. See phy.txt for the generic PHY bindings.
>
> -Required properties of the phy node with the following compatibles:
> +Required mdio-mux nodes for the following compatibles:

The following compatibles require an mdio-mux node:

> +  - "allwinner,sun8i-h3-emac",

Drop the comma. It's already a list.

> +- a mdio-mux node with compatible = "allwinner,sun8i-h3-mdio-switch"

Required properties for the mdio-mux node:
  - compatible: "allwinner,sun8i-h3-mdio-switch"

> +  - mdio-parent-bus: The parent bus is "mdio_parent"

This is optional in the mdio-mux binding. Since you have the mdio-mux under
the EMAC node, it should be obvious that it is connected to the EMAC. The
parent phandle is more useful for muxes that are separate from the controller,
such as an external GPIO-controlled mux. So you could just drop it.

> +  - two child mdio, one for the integrated mdio, one for the external mdio
> +
> +Required properties of the integrated phy node with the following 
> compatibles:

The following compatibles require a PHY node representing the integrated
PHY, under the integrated MDIO bus node if an mdio-mux node is used:

>- "allwinner,sun8i-h3-emac",
>- "allwinner,sun8i-v3s-emac":

Required properties of the integrated PHY node:

>  - clocks: a phandle to the reference clock for the EPHY
>  - resets: a phandle to the reset control for the EPHY
> +- phy-is-integrated
> +- Should be a child of the integrated mdio
>
> -Example:
> -
> +Example with integrated PHY:
>  emac: ethernet@1c0b000 {
> compatible = "allwinner,sun8i-h3-emac";
> syscon = <>;
> @@ -75,10 +82,101 @@ emac: ethernet@1c0b000 {
> mdio: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> -   int_mii_phy: ethernet-phy@1 {
> +   };
> +   mdio-mux {

Drop the mdio node as mentioned above. And also have spaces between
blocks to be consistent in styling. It's also easier to read.

> +   compatible = "allwinner,sun8i-h3-mdio-switch";
> +   mdio-parent-bus = <>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   int_mdio: mdio@1 {
> +   reg = <1>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   int_mii_phy: ethernet-phy@1 {
> +   reg = <1>;
> +   clocks = < CLK_BUS_EPHY>;
> +   resets = < RST_BUS_EPHY>;
> +   phy-is-integrated
> +   };
> +   };
> +   ext_mdio: mdio@0 {
> +   reg = <0>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   };
> +};
> +
> +Example with external PHY:
> +emac: ethernet@1c0b000 {
> +   compatible = "allwinner,sun8i-h3-emac";
> +   syscon = <>;
> +   reg = <0x01c0b000 0x104>;
> +   interrupts = ;
> +   interrupt-names = "macirq";
> +   resets = < RST_BUS_EMAC>;
> +   reset-names = "stmmaceth";
> +   clocks = < CLK_BUS_EMAC>;
> +   clock-names = "stmmaceth";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   phy-handle = <_rgmii_phy>;
> +   phy-mode = "rgmii";
> +   allwinner,leds-active-low;

You don't need this when using the external PHY.

Regards
ChenYu

> +   mdio: mdio {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   };
> +   mdio-mux {
> +   compatible = 

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Chen-Yu Tsai]

On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
<clabbe.montj...@gmail.com> wrote:
> On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
>> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
>> <clabbe.montj...@gmail.com> wrote:
>> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
>> >> Hi,
>> >>
>> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
>> >> <clabbe.montj...@gmail.com> wrote:
>> >> > This patch add the new phy-is-integrated property to the internal PHY
>> >> > node.
>> >> >
>> >> > Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>
>> >> > ---
>> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>> >> >  1 file changed, 1 insertion(+)
>> >> >
>> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
>> >> > b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> > index 4b599b5d26f6..54fc24e4c569 100644
>> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> > @@ -425,6 +425,7 @@
>> >> > reg = <1>;
>> >> > clocks = < CLK_BUS_EPHY>;
>> >> > resets = < RST_BUS_EPHY>;
>> >> > +   phy-is-integrated;
>> >>
>> >> You also need to "delete" this property at the board level for
>> >> any board that has the external PHY at address <1>. Otherwise
>> >> they will stop working. This is due to the internal and external
>> >> PHYs having the same path and node name in the device tree, so
>> >> they are effectively the same node.
>> >>
>> >> ChenYu
>> >>
>> >
>> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
>>
>> That is just the label. The label plays no part in device tree merging. The 
>> path
>>
>> /soc/ethernet@1c3/mdio/ethernet-phy@1
>>
>> is the same. You can look under
>>
>> /proc/device-tree/soc/ethernet@1c3/mdio
>>
>> on the OrangePI Plus 2E or any other H3 board that uses an
>> external PHY at address 1.
>>
>> ChenYu
>
> Since we get the phy node by phy-handle and not by path, I think all should 
> be good.

You are not getting me. The fact that the two seemingly separate
nodes are merged together means, whatever properties you put in
the internal PHY node, also affect the external PHY node. Once
compiled, they are the SAME node.

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Chen-Yu Tsai]

On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
<clabbe.montj...@gmail.com> wrote:
> On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
>> <clabbe.montj...@gmail.com> wrote:
>> > This patch add the new phy-is-integrated property to the internal PHY
>> > node.
>> >
>> > Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>
>> > ---
>> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
>> > b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> > index 4b599b5d26f6..54fc24e4c569 100644
>> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> > @@ -425,6 +425,7 @@
>> > reg = <1>;
>> > clocks = < CLK_BUS_EPHY>;
>> > resets = < RST_BUS_EPHY>;
>> > +   phy-is-integrated;
>>
>> You also need to "delete" this property at the board level for
>> any board that has the external PHY at address <1>. Otherwise
>> they will stop working. This is due to the internal and external
>> PHYs having the same path and node name in the device tree, so
>> they are effectively the same node.
>>
>> ChenYu
>>
>
> They have not the same name, ext_rgmii_phy vs int_mii_phy.

That is just the label. The label plays no part in device tree merging. The path

/soc/ethernet@1c3/mdio/ethernet-phy@1

is the same. You can look under

/proc/device-tree/soc/ethernet@1c3/mdio

on the OrangePI Plus 2E or any other H3 board that uses an
external PHY at address 1.

ChenYu

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Chen-Yu Tsai]

Hi,

On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
 wrote:
> This patch add the new phy-is-integrated property to the internal PHY
> node.
>
> Signed-off-by: Corentin Labbe 
> ---
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
> b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 4b599b5d26f6..54fc24e4c569 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -425,6 +425,7 @@
> reg = <1>;
> clocks = < CLK_BUS_EPHY>;
> resets = < RST_BUS_EPHY>;
> +   phy-is-integrated;

You also need to "delete" this property at the board level for
any board that has the external PHY at address <1>. Otherwise
they will stop working. This is due to the internal and external
PHYs having the same path and node name in the device tree, so
they are effectively the same node.

ChenYu

> };
> };
> };
> --
> 2.13.0
>

Re: [PATCH v4 05/12] Documentation: net: phy: Add phy-is-internal binding

[Chen-Yu Tsai]

On Thu, Aug 10, 2017 at 8:20 AM, Andrew Lunn  wrote:
> On Wed, Aug 09, 2017 at 03:47:34PM -0700, Florian Fainelli wrote:
>> On August 9, 2017 5:10:30 AM PDT, David Wu  wrote:
>> >Add the documentation for internal phy. A boolean property
>> >indicates that a internal phy will be used.
>> >
>> >Signed-off-by: David Wu 
>> >---
>> > Documentation/devicetree/bindings/net/phy.txt | 3 +++
>> > 1 file changed, 3 insertions(+)
>> >
>> >diff --git a/Documentation/devicetree/bindings/net/phy.txt
>> >b/Documentation/devicetree/bindings/net/phy.txt
>> >index b558576..942c892 100644
>> >--- a/Documentation/devicetree/bindings/net/phy.txt
>> >+++ b/Documentation/devicetree/bindings/net/phy.txt
>> >@@ -52,6 +52,9 @@ Optional Properties:
>> >   Mark the corresponding energy efficient ethernet mode as broken and
>> >   request the ethernet to stop advertising it.
>> >
>> >+- phy-is-internal: If set, indicates that phy will connect to the MAC
>> >as a
>> >+  internal phy.
>>
>> Something along the lines of:
>>
>> If set, indicates that the PHY is integrated into the same physical package 
>> as the Ethernet MAC.
>
> Hi Florian, David.
>
> I'm happy with the property name. But i think the text needs more
> description. We deal with Ethernet switches with integrated PHYs. Yet
> for us, this property is unneeded.
>
> Seeing this property means some bit of software needs to ensure the
> internal PHY should be used, when given the choice between an internal
> and external PHY. So i would say something like:
>
> If set, indicates that the PHY is integrated into the same
> physical package as the Ethernet MAC. If needed, muxers should be
> configured to ensure the internal PHY is used. The absence of this
> property indicates the muxers should be configured so that the
> external PHY is used.
>
> This last part is important. If the bootloader has set the internal
> PHY to be used, you need to reset it. Otherwise we are going to get
> into a mess sometime later and need to add a phy-is-external property.

Ack.

One other thing. We need to fix our (sunxi) binding which is already
in 4.13-rc1. We'd like to see this new property in netdev, i.e. merged
for 4.13, so we can use it.

Thanks
ChenYu

Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support

[Chen-Yu Tsai]

Hi David,

On Wed, Aug 9, 2017 at 5:38 PM, David.Wu <david...@rock-chips.com> wrote:
> Hello Corentin, Chen-Yu
>
>
> 在 2017/8/9 16:45, Corentin Labbe 写道:
>>
>> On Thu, Aug 03, 2017 at 07:06:33PM +0800, Chen-Yu Tsai wrote:
>>>
>>> On Thu, Aug 3, 2017 at 1:38 AM, Florian Fainelli <f.faine...@gmail.com>
>>> wrote:
>>>>
>>>> On 08/01/2017 11:21 PM, David Wu wrote:
>>>>>
>>>>> To make internal phy work, need to configure the phy_clock,
>>>>> phy cru_reset and related registers.
>>>>>
>>>>> Signed-off-by: David Wu <david...@rock-chips.com>
>>>>> ---
>>>>>   .../devicetree/bindings/net/rockchip-dwmac.txt |  6 +-
>>>>>   drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 81
>>>>> ++
>>>>>   2 files changed, 86 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> index 8f42755..ec39b31 100644
>>>>> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>>>>> @@ -25,7 +25,8 @@ Required properties:
>>>>>- clock-names: One name for each entry in the clocks property.
>>>>>- phy-mode: See ethernet.txt file in the same directory.
>>>>>- pinctrl-names: Names corresponding to the numbered pinctrl states.
>>>>> - - pinctrl-0: pin-control mode. can be <_pins> or <_pins>.
>>>>> + - pinctrl-0: pin-control mode. can be <_pins>, <_pins> or
>>>>> led pins
>>>>> +   for internal phy mode.
>>>>>- clock_in_out: For RGMII, it must be "input", means main
>>>>> clock(125MHz)
>>>>>  is not sourced from SoC's PLL, but input from PHY; For RMII,
>>>>> "input" means
>>>>>  PHY provides the reference clock(50MHz), "output" means GMAC
>>>>> provides the
>>>>> @@ -40,6 +41,9 @@ Optional properties:
>>>>>- tx_delay: Delay value for TXD timing. Range value is 0~0x7F, 0x30
>>>>> as default.
>>>>>- rx_delay: Delay value for RXD timing. Range value is 0~0x7F, 0x10
>>>>> as default.
>>>>>- phy-supply: phandle to a regulator if the PHY needs one
>>>>> + - clocks: < MAC_PHY>: Clock selector for internal macphy
>>>>> + - phy-is-internal: A boolean property allows us to know that MAC will
>>>>> connect to
>>>>> +   internal phy.
>>>>
>>>>
>>>> This is incorrect in two ways:
>>>>
>>>> - this is a property of the PHY, so it should be documented as such in
>>>> Documentation/devicetree/bindings/net/phy.txt so other bindings can
>>>> re-use it
>>>>
>>>> - if it was specific to your MAC you would expect a vendor prefix to
>>>> this property, which is not there
>>>>
>>>> An alternative way to implement the external/internal logic selection
>>>> would be mandate that your Ethernet PHY node have a compatible string
>>>> like this:
>>>>
>>>> phy@0 {
>>>>  compatible = "ethernet-phy-id1234.d400",
>>>> "ethernet-phy-802.3-c22";
>>>> };
>>>>
>>>> Then you don't need this phy-is-internal property, because you can
>>>> locate the PHY node by the phy-handle (see more about that in a reply to
>>>> patch 10) and you can determine ahead of time whether this PHY is
>>>> internal or not based on its compatible string.
>>>
>>>
>>> This doesn't really work for us (sunxi). The "internal" PHY of the H3
>>> is also available in the X-Powers AC200 combo chip, which would be
>>> external. Same ID. So if someone were to be stupid and put the two
>>> together, you wouldn't know which one it actually is. Generically
>>> it doesn't make sense to match against the ID either. The PHY is
>>> only integrated or inlined into the SoC, meaning it could be moved
>>> elsewhere or even be a discreet part.
>>>
>>> The way I see it is more like a reversed pinmux. The system should
>>> select either the internal set or external set of xMII pins to use.
>>>
>

Re: [PATCH v3 05/11] net: stmmac: dwmac-rk: Add internal phy support

[Chen-Yu Tsai]

On Thu, Aug 3, 2017 at 1:38 AM, Florian Fainelli  wrote:
> On 08/01/2017 11:21 PM, David Wu wrote:
>> To make internal phy work, need to configure the phy_clock,
>> phy cru_reset and related registers.
>>
>> Signed-off-by: David Wu 
>> ---
>>  .../devicetree/bindings/net/rockchip-dwmac.txt |  6 +-
>>  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 81 
>> ++
>>  2 files changed, 86 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt 
>> b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>> index 8f42755..ec39b31 100644
>> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
>> @@ -25,7 +25,8 @@ Required properties:
>>   - clock-names: One name for each entry in the clocks property.
>>   - phy-mode: See ethernet.txt file in the same directory.
>>   - pinctrl-names: Names corresponding to the numbered pinctrl states.
>> - - pinctrl-0: pin-control mode. can be <_pins> or <_pins>.
>> + - pinctrl-0: pin-control mode. can be <_pins>, <_pins> or led 
>> pins
>> +   for internal phy mode.
>>   - clock_in_out: For RGMII, it must be "input", means main clock(125MHz)
>> is not sourced from SoC's PLL, but input from PHY; For RMII, "input" 
>> means
>> PHY provides the reference clock(50MHz), "output" means GMAC provides the
>> @@ -40,6 +41,9 @@ Optional properties:
>>   - tx_delay: Delay value for TXD timing. Range value is 0~0x7F, 0x30 as 
>> default.
>>   - rx_delay: Delay value for RXD timing. Range value is 0~0x7F, 0x10 as 
>> default.
>>   - phy-supply: phandle to a regulator if the PHY needs one
>> + - clocks: < MAC_PHY>: Clock selector for internal macphy
>> + - phy-is-internal: A boolean property allows us to know that MAC will 
>> connect to
>> +   internal phy.
>
> This is incorrect in two ways:
>
> - this is a property of the PHY, so it should be documented as such in
> Documentation/devicetree/bindings/net/phy.txt so other bindings can
> re-use it
>
> - if it was specific to your MAC you would expect a vendor prefix to
> this property, which is not there
>
> An alternative way to implement the external/internal logic selection
> would be mandate that your Ethernet PHY node have a compatible string
> like this:
>
> phy@0 {
> compatible = "ethernet-phy-id1234.d400", "ethernet-phy-802.3-c22";
> };
>
> Then you don't need this phy-is-internal property, because you can
> locate the PHY node by the phy-handle (see more about that in a reply to
> patch 10) and you can determine ahead of time whether this PHY is
> internal or not based on its compatible string.

This doesn't really work for us (sunxi). The "internal" PHY of the H3
is also available in the X-Powers AC200 combo chip, which would be
external. Same ID. So if someone were to be stupid and put the two
together, you wouldn't know which one it actually is. Generically
it doesn't make sense to match against the ID either. The PHY is
only integrated or inlined into the SoC, meaning it could be moved
elsewhere or even be a discreet part.

The way I see it is more like a reversed pinmux. The system should
select either the internal set or external set of xMII pins to use.

A phy-is-internal property in the PHY node would work for us. We
already need a PHY node to describe its clocks and resets.

A side note to this solution is that, since the internal PHY is defined
at the .dtsi level, any external PHYs at the same address would need to
make sure to delete the property, which is kind of counterintuitive, but
it is how device tree works. One would want to put the internal PHY's
address, assuming it is configurable, on something that is rarely used.


Regards
ChenYu

Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

[Chen-Yu Tsai]

Hi David, Florian, Andrew

(resent in plain text)

On Fri, Jul 28, 2017 at 2:56 PM, David.Wu  wrote:
>
> Hi Florian,
>
> 在 2017/7/28 0:54, Florian Fainelli 写道:
>>
>> - if you need knowledge about this PHY connection type prior to binding
>> the PHY device and its driver (that is, before of_phy_connect()) we
>> could add a boolean property e.g: "phy-is-internal" that allows us to
>> know that, or we can have a new phy-mode value, e.g: "internal-rmii"
>> which describes that, either way would probably be fine, but the former
>> scales better
>>
>
> Using "phy-is-internal" is very helpful, but it is easy to confuse with
> the real internal PHY, may we use the other words like phy is inlined.

If "internal" is confusing, would "phy-is-integrated" in the MAC node work?

Either way we would like to have a definitive solution to this. Our
dwmac-sun8i driver is already in v4.13-rc1, with a somewhat flaky
method of knowing whether the internal PHY is used (phy-mode = "mii").

We really want a fix for this release, otherwise we would be force
to revert either the internal PHY part or the whole driver.

ChenYu

>
>> Then again, using phy-mode = "internal" even though this is Reduced MII
>> is not big of a deal IMHO as long as there is no loss of information and
>> that internal de-facto means internal reduced MII for instance.
>> --
>
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] net-next: stmmac: support future possible different internal phy mode

[Chen-Yu Tsai]

On Sat, Jul 29, 2017 at 2:48 PM, Corentin Labbe
 wrote:
> On Fri, Jul 28, 2017 at 10:54:30AM -0700, Florian Fainelli wrote:
>> On 07/28/2017 07:44 AM, Corentin Labbe wrote:
>> > On Fri, Jul 28, 2017 at 04:36:00PM +0200, Andrew Lunn wrote:
>>  I've probably asked this before: Does the internal PHY use a different
>>  PHY ID in registers 2 and 3?
>> 
>> >>>
>> >>> yes
>> >>>
>> >>> reg2: 0x0044
>> >>> reg3: 0X1500
>> >
>> > Copy/paste error, its 1400
>> >
>> >>
>> >> So this is not about loading the correct PHY driver. You can already
>> >> do this based on the PHY IDs...
>> >>
>> >> This is about selecting which PHY to use. Internal or External?
>> >>
>> >>  Andrew
>> >
>> > It is too late when we know the PHY ID.
>>
>> > We need to set a syscon for choosing external/internal PHY.
>> > So we can rely only on DT.
>>
>> Since the Device Tree needs to be correct to identify which PHY to use
>> (internal or external), if you use the standard compatible string for
>> the PHY that contains its OUI, e.g:
>>
>> compatible = "ethernet-phy-id0044.1400", "ethernet-phy-ieee802.3-c22"
>>
>> then you can have your Ethernet MAC identify whether this is an internal
>> PHY by having a list of compatible strings to match against.
>
> So basicly, I replace sun8i-h3-ephy by ethernet-phy-id0044.1400 and it is 
> good ?

IIRC you mentioned some time ago the PHY in the AC200 also has this ID?
Do you remember if this is true?

If someone were crazy enough to hook that up to the H3, then we still
wouldn't be able to tell if it's the internal or external one.

ChenYu

>
>>
>> Corentin, can you make sure you copy netdev, Andrew and myself on the
>> next submissions so we don't have to keep track of seemingly identical
>> threads (this one + the rockchip dwmac changes) and we can work towards
>> one solution?
>
> Ok
>
> Thanks
> Corentin Labbe

Re: [PATCH 0/3] net-next: stmmac: support future possible different internal phy mode

[Chen-Yu Tsai]

On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
 wrote:
> Hello
>
> The current way to find if the phy is internal is to compare DT phy-mode
> and emac_variant/internal_phy.
> But it will negate a possible future SoC where an external PHY use the
> same phy mode than the internal one.
>
> This patchs series adds a new way to find if the PHY is internal, via its
> compatible.

You've already joined in on the discussion for the patch "net: stmmac:
dwmac-rk: Add internal phy support". It is pretty much the same issue.
Please wait for it to come to a proper conclusion. At least then we can
agree on something so different platforms with the same problem don't
have diverging solutions.

ChenYu

>
> Corentin Labbe (3):
>   dt-bindings: net: add compatible for internal sun8i-h3/sun8i-v3s PHYs
>   ARM: sunxi: h3/h5: Add sun8i-h3-ephy compatible
>   net-next: stmmac: dwmac-sun8i: choose internal PHY via compatible
>
>  Documentation/devicetree/bindings/net/dwmac-sun8i.txt |  4 ++--
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi|  3 ++-
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++--
>  3 files changed, 14 insertions(+), 9 deletions(-)
>
> --
> 2.13.0
>

Re: [PATCH 3/3] net-next: stmmac: dwmac-sun8i: choose internal PHY via compatible

[Chen-Yu Tsai]

On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
 wrote:
> The current way to find if the phy is internal is to compare DT phy-mode
> and emac_variant/internal_phy.
> But it will negate a possible future SoC where an external PHY use the
> same phy mode than the internal one.
>
> This patch adds a new way to find if the PHY is internal, via its
> compatible.
>
> Since the phy_mode of the internal PHY does need to be know, the
> variant internal_phy member is converted to a boolean.
>
> Signed-off-by: Corentin Labbe 
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index fffd6d5fc907..04f7ddd802b0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -48,7 +48,7 @@
>   */
>  struct emac_variant {
> u32 default_syscon_value;
> -   int internal_phy;
> +   bool internal_phy;

Nit: please add a verb to the name, like "has_internal_phy",
or even "soc_has_internal_phy". This makes it clear what this
property represents.

ChenYu

Re: [PATCH 2/3] ARM: sunxi: h3/h5: Add sun8i-h3-ephy compatible

[Chen-Yu Tsai]

On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
 wrote:
> This patch adds the sun8i-h3-ephy compatible to the internal PHY.
>
> Signed-off-by: Corentin Labbe 
> ---
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 3 ++-

To avoid repeating the past, this patch, if approved, will be merged
through the sunxi tree, not netdev nor net-next.

>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
> b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 4b599b5d26f6..7aaa837c2388 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -421,7 +421,8 @@
> #address-cells = <1>;
> #size-cells = <0>;
> int_mii_phy: ethernet-phy@1 {
> -   compatible = 
> "ethernet-phy-ieee802.3-c22";
> +   compatible = 
> "allwinner,sun8i-h3-ephy",
> +   
> "ethernet-phy-ieee802.3-c22";

Are you expecting people to override this properly?

As it currently is, any external phy at address 1 will simply
reuse the same device node. And if they don't override the
property correctly, the driver will end up trying to use
the internal phy, while the user is expecting the external
one to be used.

Maybe you could move this to some other address, maybe the last
valid one, or second last valid one?

ChenYu

> reg = <1>;
> clocks = < CLK_BUS_EPHY>;
> resets = < RST_BUS_EPHY>;
> --
> 2.13.0
>

Re: [linux-sunxi] Re: [PATCH 6/6] net: stmmac: revert "support future possible different internal phy mode"

[Chen-Yu Tsai]

Hi Corentin,

On Thu, Jul 6, 2017 at 5:45 PM, David Miller  wrote:
> From: Corentin Labbe 
> Date: Thu, 6 Jul 2017 10:51:47 +0200
>
>> On Sun, Jul 02, 2017 at 02:31:59PM +0200, Corentin Labbe wrote:
>>> Since internal phy-mode is reserved for non-xMII protocol we cannot use
>>> it with dwmac-sun8i
>>> This reverts commit 1c2fa5f84683 ("net: stmmac: support future possible 
>>> different internal phy mode")
>>>
>>> Signed-off-by: Corentin Labbe 
>>> ---
>>
>> Hello
>>
>> This patch was left not applied but all other patch from this serie was.
>> Could you apply it, or perhaps you prefer I resend it with some "Fixes:"
>
> You never need to ask questions like this.
>
> If it's not active in my patchwork queue, you must resend.
>
> Thank you.

Your series was not applied at all. See the status on patchwork:

http://patchwork.ozlabs.org/patch/783179/

Maxime's patch to revert all device tree changes in net-next was
applied first.

Please rebase and resend this patch for netdev (not net-next).

ChenYu

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 6:15 PM, Andre Przywara <andre.przyw...@arm.com> wrote:
> Hi,
>
> On 27/06/17 10:41, Maxime Ripard wrote:
>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> (CC:ing some people from that Rockchip dmwac series)
>>>
>>> On 27/06/17 09:21, Corentin Labbe wrote:
>>>> On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>>>>> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>>>>> <clabbe.montj...@gmail.com> wrote:
>>>>>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>>>>>>> On 31/05/17 08:18, Corentin Labbe wrote:
>>>>>>>> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>>>>>>>> allwinner.
>>>>>>>> In fact the only common part is the descriptor management and the first
>>>>>>>> register function.
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I know I am a bit late with this, but while adapting the U-Boot driver
>>>>>>> to the new binding I was wondering about the internal PHY detection:
>>>>>>>
>>>>>>>
>>>>>>> So here you seem to deduce the usage of the internal PHY by the PHY
>>>>>>> interface specified in the DT (MII = internal, RGMII = external).
>>>>>>> I think I raised this question before, but isn't it perfectly legal for
>>>>>>> a board to use MII with an external PHY even on those SoCs that feature
>>>>>>> an internal PHY?
>>>>>>> On the first glance that does not make too much sense, but apart from
>>>>>>> not being the correct binding to describe all of the SoCs features I see
>>>>>>> two scenarios:
>>>>>>> 1) A board vendor might choose to not use the internal PHY because it
>>>>>>> has bugs, lacks features (configurability) or has other issues. For
>>>>>>> instance I have heard reports that the internal PHY makes the SoC go
>>>>>>> rather hot, possibly limiting the CPU frequency. By using an external
>>>>>>> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>>>>>>> 2) A PHY does not necessarily need to be directly connected to
>>>>>>> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>>>>>>> IC or some other network circuitry, for instance fibre connectors.
>>>>>>>
>>>>>>> So I was wondering if we would need an explicit:
>>>>>>>   allwinner,use-internal-phy;
>>>>>>> boolean DT property to signal the usage of the internal PHY?
>>>>>>> Alternatively we could go with the negative version:
>>>>>>>   allwinner,disable-internal-phy;
>>>>>>>
>>>>>>> Or what about introducing a new "allwinner,internal-mii-phy" compatible
>>>>>>> string for the *PHY* node and use that?
>>>>>>>
>>>>>>> I just want to avoid that we introduce a binding that causes us
>>>>>>> headaches later. I think we can still fix this with a followup patch
>>>>>>> before the driver and its binding hit a release kernel.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Andre.
>>>>>>>
>>>>>>
>>>>>> I just see some patch, where "phy-mode = internal" is valid.
>>>>>> I will try to find a way to use it
>>>>>
>>>>> Can you provide a link?
>>>>
>>>> https://lkml.org/lkml/2017/6/23/479
>>>>
>>>>>
>>>>> I'm not a fan of using phy-mode for this. There's no guarantee what
>>>>> mode the internal PHY uses. That's what phy-mode is for.
>>>
>>> I can understand Chen-Yu's concerns, but ...
>>>
>>>> For each soc the internal PHY mode is know and setted in 
>>>> emac_variant/internal_phy
>>>> So its not a problem.
>>>
>>> that is true as well, at least for now.
>>>
>>> So while I agree that having a separate property to indicate the usage
>>> of the internal PHY would be nice, I am bit tempted to use this easier
>>> approach and piggy back on the existing phy-mode propert

Re: [linux-sunxi] Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 6:17 PM, Icenowy Zheng <icen...@aosc.io> wrote:
>
>
> 于 2017年6月27日 GMT+08:00 下午6:11:47, Chen-Yu Tsai <w...@csie.org> 写到:
>>On Tue, Jun 27, 2017 at 5:41 PM, Maxime Ripard
>><maxime.rip...@free-electrons.com> wrote:
>>> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>>>> Hi,
>>>>
>>>> (CC:ing some people from that Rockchip dmwac series)
>>>>
>>>> On 27/06/17 09:21, Corentin Labbe wrote:
>>>> > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>>>> >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>>>> >> <clabbe.montj...@gmail.com> wrote:
>>>> >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>>>> >>>> On 31/05/17 08:18, Corentin Labbe wrote:
>>>> >>>>> The dwmac-sun8i is a heavy hacked version of stmmac hardware
>>by
>>>> >>>>> allwinner.
>>>> >>>>> In fact the only common part is the descriptor management and
>>the first
>>>> >>>>> register function.
>>>> >>>>
>>>> >>>> Hi,
>>>> >>>>
>>>> >>>> I know I am a bit late with this, but while adapting the U-Boot
>>driver
>>>> >>>> to the new binding I was wondering about the internal PHY
>>detection:
>>>> >>>>
>>>> >>>>
>>>> >>>> So here you seem to deduce the usage of the internal PHY by the
>>PHY
>>>> >>>> interface specified in the DT (MII = internal, RGMII =
>>external).
>>>> >>>> I think I raised this question before, but isn't it perfectly
>>legal for
>>>> >>>> a board to use MII with an external PHY even on those SoCs that
>>feature
>>>> >>>> an internal PHY?
>>>> >>>> On the first glance that does not make too much sense, but
>>apart from
>>>> >>>> not being the correct binding to describe all of the SoCs
>>features I see
>>>> >>>> two scenarios:
>>>> >>>> 1) A board vendor might choose to not use the internal PHY
>>because it
>>>> >>>> has bugs, lacks features (configurability) or has other issues.
>>For
>>>> >>>> instance I have heard reports that the internal PHY makes the
>>SoC go
>>>> >>>> rather hot, possibly limiting the CPU frequency. By using an
>>external
>>>> >>>> MII PHY (which are still cheaper than RGMII PHYs) this can be
>>avoided.
>>>> >>>> 2) A PHY does not necessarily need to be directly connected to
>>>> >>>> magnetics. Indeed quite some boards use (RG)MII to connect to a
>>switch
>>>> >>>> IC or some other network circuitry, for instance fibre
>>connectors.
>>>> >>>>
>>>> >>>> So I was wondering if we would need an explicit:
>>>> >>>>   allwinner,use-internal-phy;
>>>> >>>> boolean DT property to signal the usage of the internal PHY?
>>>> >>>> Alternatively we could go with the negative version:
>>>> >>>>   allwinner,disable-internal-phy;
>>>> >>>>
>>>> >>>> Or what about introducing a new "allwinner,internal-mii-phy"
>>compatible
>>>> >>>> string for the *PHY* node and use that?
>>>> >>>>
>>>> >>>> I just want to avoid that we introduce a binding that causes us
>>>> >>>> headaches later. I think we can still fix this with a followup
>>patch
>>>> >>>> before the driver and its binding hit a release kernel.
>>>> >>>>
>>>> >>>> Cheers,
>>>> >>>> Andre.
>>>> >>>>
>>>> >>>
>>>> >>> I just see some patch, where "phy-mode = internal" is valid.
>>>> >>> I will try to find a way to use it
>>>> >>
>>>> >> Can you provide a link?
>>>> >
>>>> > https://lkml.org/lkml/2017/6/23/479
>>>> >
>>>> >>
>>>> >> I'm not a fan of using phy-mode for this. There's no guarantee
>>what
>>>> >> m

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 5:41 PM, Maxime Ripard
<maxime.rip...@free-electrons.com> wrote:
> On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:
>> Hi,
>>
>> (CC:ing some people from that Rockchip dmwac series)
>>
>> On 27/06/17 09:21, Corentin Labbe wrote:
>> > On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:
>> >> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
>> >> <clabbe.montj...@gmail.com> wrote:
>> >>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>> >>>> On 31/05/17 08:18, Corentin Labbe wrote:
>> >>>>> The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>> >>>>> allwinner.
>> >>>>> In fact the only common part is the descriptor management and the first
>> >>>>> register function.
>> >>>>
>> >>>> Hi,
>> >>>>
>> >>>> I know I am a bit late with this, but while adapting the U-Boot driver
>> >>>> to the new binding I was wondering about the internal PHY detection:
>> >>>>
>> >>>>
>> >>>> So here you seem to deduce the usage of the internal PHY by the PHY
>> >>>> interface specified in the DT (MII = internal, RGMII = external).
>> >>>> I think I raised this question before, but isn't it perfectly legal for
>> >>>> a board to use MII with an external PHY even on those SoCs that feature
>> >>>> an internal PHY?
>> >>>> On the first glance that does not make too much sense, but apart from
>> >>>> not being the correct binding to describe all of the SoCs features I see
>> >>>> two scenarios:
>> >>>> 1) A board vendor might choose to not use the internal PHY because it
>> >>>> has bugs, lacks features (configurability) or has other issues. For
>> >>>> instance I have heard reports that the internal PHY makes the SoC go
>> >>>> rather hot, possibly limiting the CPU frequency. By using an external
>> >>>> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>> >>>> 2) A PHY does not necessarily need to be directly connected to
>> >>>> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>> >>>> IC or some other network circuitry, for instance fibre connectors.
>> >>>>
>> >>>> So I was wondering if we would need an explicit:
>> >>>>   allwinner,use-internal-phy;
>> >>>> boolean DT property to signal the usage of the internal PHY?
>> >>>> Alternatively we could go with the negative version:
>> >>>>   allwinner,disable-internal-phy;
>> >>>>
>> >>>> Or what about introducing a new "allwinner,internal-mii-phy" compatible
>> >>>> string for the *PHY* node and use that?
>> >>>>
>> >>>> I just want to avoid that we introduce a binding that causes us
>> >>>> headaches later. I think we can still fix this with a followup patch
>> >>>> before the driver and its binding hit a release kernel.
>> >>>>
>> >>>> Cheers,
>> >>>> Andre.
>> >>>>
>> >>>
>> >>> I just see some patch, where "phy-mode = internal" is valid.
>> >>> I will try to find a way to use it
>> >>
>> >> Can you provide a link?
>> >
>> > https://lkml.org/lkml/2017/6/23/479
>> >
>> >>
>> >> I'm not a fan of using phy-mode for this. There's no guarantee what
>> >> mode the internal PHY uses. That's what phy-mode is for.
>>
>> I can understand Chen-Yu's concerns, but ...
>>
>> > For each soc the internal PHY mode is know and setted in 
>> > emac_variant/internal_phy
>> > So its not a problem.
>>
>> that is true as well, at least for now.
>>
>> So while I agree that having a separate property to indicate the usage
>> of the internal PHY would be nice, I am bit tempted to use this easier
>> approach and piggy back on the existing phy-mode property.
>
> We're trying to fix an issue that works for now too.
>
> If we want to consider future weird cases, then we must consider all
> of them. And the phy mode changing is definitely not really far
> fetched.

I guess the issue is whether it's likely that the vendor puts 2 intern

Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

[Chen-Yu Tsai]

On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
 wrote:
> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:
>> On 31/05/17 08:18, Corentin Labbe wrote:
>> > The dwmac-sun8i is a heavy hacked version of stmmac hardware by
>> > allwinner.
>> > In fact the only common part is the descriptor management and the first
>> > register function.
>>
>> Hi,
>>
>> I know I am a bit late with this, but while adapting the U-Boot driver
>> to the new binding I was wondering about the internal PHY detection:
>>
>>
>> So here you seem to deduce the usage of the internal PHY by the PHY
>> interface specified in the DT (MII = internal, RGMII = external).
>> I think I raised this question before, but isn't it perfectly legal for
>> a board to use MII with an external PHY even on those SoCs that feature
>> an internal PHY?
>> On the first glance that does not make too much sense, but apart from
>> not being the correct binding to describe all of the SoCs features I see
>> two scenarios:
>> 1) A board vendor might choose to not use the internal PHY because it
>> has bugs, lacks features (configurability) or has other issues. For
>> instance I have heard reports that the internal PHY makes the SoC go
>> rather hot, possibly limiting the CPU frequency. By using an external
>> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided.
>> 2) A PHY does not necessarily need to be directly connected to
>> magnetics. Indeed quite some boards use (RG)MII to connect to a switch
>> IC or some other network circuitry, for instance fibre connectors.
>>
>> So I was wondering if we would need an explicit:
>>   allwinner,use-internal-phy;
>> boolean DT property to signal the usage of the internal PHY?
>> Alternatively we could go with the negative version:
>>   allwinner,disable-internal-phy;
>>
>> Or what about introducing a new "allwinner,internal-mii-phy" compatible
>> string for the *PHY* node and use that?
>>
>> I just want to avoid that we introduce a binding that causes us
>> headaches later. I think we can still fix this with a followup patch
>> before the driver and its binding hit a release kernel.
>>
>> Cheers,
>> Andre.
>>
>
> I just see some patch, where "phy-mode = internal" is valid.
> I will try to find a way to use it

Can you provide a link?

I'm not a fan of using phy-mode for this. There's no guarantee what
mode the internal PHY uses. That's what phy-mode is for.

In any case, we should fix this before 4.13 is released.

ChenYu

Re: [PATCH v5 00/20] net-next: stmmac: add dwmac-sun8i ethernet driver

[Chen-Yu Tsai]

On Mon, May 1, 2017 at 10:01 PM, Andrew Lunn  wrote:
> On Mon, May 01, 2017 at 02:45:00PM +0200, Corentin Labbe wrote:
>> Hello
>>
>> This patch series add the driver for dwmac-sun8i which handle the Ethernet 
>> MAC
>> present on Allwinner H3/H5/A83T/A64 SoCs.
>>
>> This driver is the continuation of the sun8i-emac driver.
>> During the development, it appeared that in fact the hardware was a modified
>> version of some dwmac.
>> So the driver is now written as a glue driver for stmmac.
>>
>> It supports 10/100/1000 Mbit/s speed with half/full duplex.
>> It can use an internal PHY (MII 10/100) or an external PHY
>> via RGMII/RMII.
>
> Hi Corentin
>
> Sorry if this has been asked before
>
> Does the internal PHY have a phy driver? It seems like
> tx-delay-ps/rx-delay-ps are properties of this internal PHY, and so
> should be in the phy driver, if it has one.

Nope. These affect the delay lines for the external PHY interface.
These have existed since the A20, when the GMAC hardware block and
glue layer controls were introduced.

ChenYu

Re: [PATCH v4 13/18] arm64: allwinner: sun50i-a64: add dwmac-sun8i Ethernet driver

[Chen-Yu Tsai]

On Mon, Apr 24, 2017 at 8:24 PM, Corentin Labbe
 wrote:
> On Wed, Apr 12, 2017 at 02:41:53PM +0200, Maxime Ripard wrote:
>> On Wed, Apr 12, 2017 at 01:13:55PM +0200, Corentin Labbe wrote:
>> > The dwmac-sun8i is an Ethernet MAC that supports 10/100/1000 Mbit
>> > connections. It is very similar to the device found in the Allwinner
>> > H3, but lacks the internal 100 Mbit PHY and its associated control
>> > bits.
>> > This adds the necessary bits to the Allwinner A64 SoC .dtsi, but keeps
>> > it disabled at this level.
>> >
>> > Signed-off-by: Corentin Labbe 
>> > ---
>> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 37 
>> > +++
>> >  1 file changed, 37 insertions(+)
>> >
>> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
>> > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > index 0b0f4ab..2569827 100644
>> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > @@ -287,6 +287,23 @@
>> > bias-pull-up;
>> > };
>> >
>> > +   rmii_pins: rmii_pins {
>> > +   pins = "PD10", "PD11", "PD13", "PD14",
>> > +   "PD17", "PD18", "PD19", "PD20",
>> > +   "PD22", "PD23";
>>
>> Please align the wrapped lines on the first pin.
>>
>
> OK
>
>> > +   function = "emac";
>> > +   drive-strength = <40>;
>>
>> Do you actually need that for all the boards, or only a few of them?
>
> I have tried to use lower value without success on some boards. (opipc/pine64 
> in my memory)

FYI we need them for all the boards that use RGMII.
The signals at gigabit speed run at 125 MHz DDR.

For RMII we probably don't need it. Even at 100 Mbps,
it's only 50 MHz SDR. drive-strength = <30> should be
enough.

ChenYu

Re: Linux kernel commit breaks IPMI on iface downing

[Chen-Yu Tsai]

Hi,

On Wed, Sep 9, 2015 at 9:33 AM, Harish Patil  wrote:
>
>>On Fri, 2015-09-04 at 09:55 +0200, Sébastien Bocahu wrote:
>>> Hi,
>>>
>>> Any chance this behaviour gets fixed, with either a new firmware or a
>>> workaround in the kernel ?
>>>
>>
>>As I said earlier, when we call bnx2_shutdown_chip(), we inform the
>>firmware that the driver is shutting down.  The firmware should know
>>that there is IPMI firmware and the link needs to stay up.
>>
>>In the older driver, we would also call bnx2_set_power_state() which
>>would send some additional messages to the firmware before putting the
>>device in D3hot.  May be these messages are required by the firmware to
>>keep the link up.  Harish, please check with your firmware team.
>>Thanks.
>>
>>
>>
>>
>
> Hi Michael/Sebastien,
> ACK. Sure, I will look into it and get back.

Sorry to resurrect such an old thread. I'm still having problems with this.

We have a whole bunch of Dell PowerEdge R210 II servers at our datacenter.
These have the NetXtreme II BCM 5716 controller. Most still have the
factory firmware (6.2.12) while a few have been updated to the latest
firmware released by Dell (7.12.19). With both versions, IPMI over LAN
becomes unusable as soon as Ubuntu's installer reconfigures the network.
It only returns after installation is complete and the system is reboot,
after the firmware is reloaded.

This means if anything were to fail during an automated installation,
the system would no longer be controllable remotely. For us this is a
huge inconvenience, as we have hundreds of them. If any go down due
to bad installation scripts or entering a wrong command, someone has
to literally go there and reboot the machine.

I also tested with Ubuntu 16.04.1, running Ubuntu's 4.4.0-31-generic
kernel. IPMI over LAN is unusable as soon as "ip link set XXX down" is
run, and recovers in 1 second after "ip link set XXX up" is run, or when
the system is restarted.

So in addition to IPMI not working on iface down, there might be a
problem with how the installer works, resulting in the prolonged
outage.


Regards
ChenYu

Re: [PATCH v2 1/5] ethernet: add sun8i-emac driver

[Chen-Yu Tsai]

On Sat, Jul 30, 2016 at 1:25 AM, Maxime Ripard
 wrote:
> On Thu, Jul 28, 2016 at 04:57:34PM +0200, LABBE Corentin wrote:
>> > > +static int sun8i_mdio_write(struct mii_bus *bus, int phy_addr, int 
>> > > phy_reg,
>> > > + u16 data)
>> > > +{
>> > > + struct net_device *ndev = bus->priv;
>> > > + struct sun8i_emac_priv *priv = netdev_priv(ndev);
>> > > + u32 reg;
>> > > + int err;
>> > > +
>> > > + err = readl_poll_timeout(priv->base + SUN8I_EMAC_MDIO_CMD, reg,
>> > > +  !(reg & MDIO_CMD_MII_BUSY), 100, 1);
>> > > + if (err) {
>> > > + dev_err(priv->dev, "%s timeout %x\n", __func__, reg);
>> > > + return err;
>> > > + }
>> >
>> > Why the poll_timeout variant?
>> >
>> Because, in case of bad clock/reset/regulator setting, the value
>> expected to come could never be set.
>
> Ah, I missed that it was for a busy bit, my bad. However, you seem to
> be using that on several occasions, maybe you could turn that into a
> function?
>
>> > > +static void sun8i_emac_unset_syscon(struct net_device *ndev)
>> > > +{
>> > > + struct sun8i_emac_priv *priv = netdev_priv(ndev);
>> > > + u32 reg = 0;
>> > > +
>> > > + if (priv->variant == H3_EMAC)
>> > > + reg = H3_EPHY_DEFAULT_VALUE;
>> >
>> > Why do you need that?
>> >
>> For resetting the syscon to the factory default.
>
> Yes, but does it matter? Does it have any side effect? Is that
> register shared with another device?
>
> Otherwise, either it won't be used anymore, and you don't care, or you
> will reload the driver later, and the driver should work whatever
> state is programmed in there. In both cases, you don't need to reset
> that value.

The "default" setting also disables and powers down the internal PHY.
I think that's a good thing? The naming could be better.

>> > > +static irqreturn_t sun8i_emac_dma_interrupt(int irq, void *dev_id)
>> > > +{
>> > > + struct net_device *ndev = dev_id;
>> > > + struct sun8i_emac_priv *priv = netdev_priv(ndev);
>> > > + u32 v, u;
>> > > +
>> > > + v = readl(priv->base + SUN8I_EMAC_INT_STA);
>> > > +
>> > > + /* When this bit is asserted, a frame transmission is completed. */
>> > > + if (v & BIT(0)) {
>> > > + priv->estats.tx_int++;
>> > > + writel(0, priv->base + SUN8I_EMAC_INT_EN);
>> > > + napi_schedule(>napi);
>> > > + }
>> > > +
>> > > + /* When this bit is asserted, the TX DMA FSM is stopped. */
>> > > + if (v & BIT(1))
>> > > + priv->estats.tx_dma_stop++;
>> > > +
>> > > + /* When this asserted, the TX DMA can not acquire next TX descriptor
>> > > +  * and TX DMA FSM is suspended.
>> > > + */
>> > > + if (v & BIT(2))
>> > > + priv->estats.tx_dma_ua++;
>> > > +
>> > > + if (v & BIT(3))
>> > > + netif_dbg(priv, intr, ndev, "Unhandled interrupt TX 
>> > > TIMEOUT\n");
>> >
>> > Why do you enable that interrupt if you can't handle it?
>>
>> Some interrupt fire even when not enabled (like RX_BUF_UA_INT/TX_BUF_UA_INT)
>
> So the bits 9 and 2, respectively, in the interrupt enable register
> are useless?

Does it actually fire, i.e. pull the interrupt line on the GIC? Or is it just
the interrupt state showing an event? IIRC some other hardware blocks have this
behavior, such as the timer.

ChenYu

>> > And printing in the interrupt handler is a very bad idea.
>>
>> There are printed only when DEBUG is set, so not a problem ?
>
> It's always a problem, this adds a very significant latency and will
> fill the kernel log buffer at an insane rate, flushing out actual
> important messages, for no particular reason.
>> > > +
>> > > + return IRQ_HANDLED;
>> >
>> > The lack of spinlocks in there is quite worrying.
>> >
>>
>> The interrupt handler just do nothing harmfull if it race with itself.
>> Just stats, enabling NAPI etc..
>> Anyway, It miss a comment for that non-locking strategy
>
> The interrupt handler cannot race with itself. The interrupts will be
> masked on the local CPU and the interrupt can only be delivered to a
> single CPU (so, the one that the handler is currently running from).
>
>> > > +}
>> > > +
>> > > +static int sun8i_emac_probe(struct platform_device *pdev)
>> > > +{
>> > > + struct device_node *node = pdev->dev.of_node;
>> > > + struct sun8i_emac_priv *priv;
>> > > + struct net_device *ndev;
>> > > + struct resource *res;
>> > > + int ret;
>> > > +
>> > > + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
>> > > + if (ret) {
>> > > + dev_err(>dev, "No suitable DMA available\n");
>> > > + return ret;
>> > > + }
>> >
>> > Isn't that the default?
>> >
>> No, it is necessary on arm64 as apritzel requested.
>
> http://lxr.free-electrons.com/source/drivers/of/device.c#L93
>
> It seems to be shared between the two.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH 3/5] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[Chen-Yu Tsai]

Hi Rob,

On Thu, Jun 9, 2016 at 3:11 AM, Rob Herring  wrote:
> On Mon, Jun 06, 2016 at 08:10:54PM +0200, Corentin LABBE wrote:
>> Le 06/06/2016 16:14, Rob Herring a écrit :
>> > On Fri, Jun 03, 2016 at 11:56:28AM +0200, LABBE Corentin wrote:
>> >> This patch adds documentation for Device-Tree bindings for the
>> >> Allwinner sun8i-emac driver.
>> >>
>> >> Signed-off-by: LABBE Corentin 
>> >> ---
>> >>  .../bindings/net/allwinner,sun8i-emac.txt  | 64 
>> >> ++
>> >>  1 file changed, 64 insertions(+)
>> >>  create mode 100644 
>> >> Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
>> >>
>> >> diff --git 
>> >> a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
>> >> b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
>> >> new file mode 100644
>> >> index 000..cf71a71
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
>> >> @@ -0,0 +1,64 @@
>> >> +* Allwinner sun8i EMAC ethernet controller
>> >> +
>> >> +Required properties:
>> >> +- compatible: "allwinner,sun8i-a83t-emac", "allwinner,sun8i-h3-emac",
>> >> +  or "allwinner,sun50i-a64-emac"
>> >> +- reg: address and length of the register sets for the device.
>> >> +- reg-names: should be "emac" and "syscon", matching the register sets
>> >
>> > Is syscon shared with other devices? Your example only has 1 reg
>> > address.
>> >
>>
>> The example is bad, emac and syscon are two distinct regspaces.
>> I will correct the example.
>
> And the syscon registers are not shared with anything else? Typically,
> syscon registers would be a separate node not part of this blocks
> registers. The main thing is make sure you are not creating overlapping
> register addresses.

These syscon registers are in a separate address range, like a glue layer
over the underlying EMAC hardware. There are only 2 registers defined in
that address range, and this particular one is used only for EMAC related
controls.

The other register is not used anywhere in the kernel. It's a readonly
register that gives the SoC revision. IIRC there was some work to add
per-platform functions to support this, but we haven't the need to use
it yet.

Hope this clears things up.

Regards
ChenYu

Re: [linux-sunxi] [PATCH 1/5] ethernet: add sun8i-emac driver

[Chen-Yu Tsai]

On Mon, Jun 6, 2016 at 6:32 AM, André Przywara  wrote:
> On 03/06/16 10:56, LABBE Corentin wrote:
>
> Hi,
>
> first: thanks for posting this and the time and work that you spent on
> it. With the respective DT nodes this works for me on the Pine64 and
> turns this board eventually into something useful.
>
> Some comments below:
>
>> This patch add support for sun8i-emac ethernet MAC hardware.
>> It could be found in Allwinner H3/A83T/A64 SoCs.
>>
>> It supports 10/100/1000 Mbit/s speed with half/full duplex.
>> It can use an internal PHY (MII 10/100) or an external PHY
>> via RGMII/RMII.
>>
>> Signed-off-by: LABBE Corentin 
>> ---
>>  drivers/net/ethernet/allwinner/Kconfig  |   13 +
>>  drivers/net/ethernet/allwinner/Makefile |1 +
>>  drivers/net/ethernet/allwinner/sun8i-emac.c | 1943 
>> +++
>>  3 files changed, 1957 insertions(+)
>>  create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c
>>
>> diff --git a/drivers/net/ethernet/allwinner/Kconfig 
>> b/drivers/net/ethernet/allwinner/Kconfig
>> index 47da7e7..226499d 100644
>> --- a/drivers/net/ethernet/allwinner/Kconfig
>> +++ b/drivers/net/ethernet/allwinner/Kconfig
>> @@ -33,4 +33,17 @@ config SUN4I_EMAC
>>To compile this driver as a module, choose M here.  The module
>>will be called sun4i-emac.
>>
>> +config SUN8I_EMAC
>> +tristate "Allwinner sun8i EMAC support"
>
> nit: w/s error
>
>> + depends on ARCH_SUNXI || COMPILE_TEST
>> + depends on OF
>> + select MII
>> + select PHYLIB
>> +---help---
>> +   This driver support the sun8i EMAC ethernet driver present on
>> +   H3/A83T/A64 Allwinner SoCs.
>> +
>> +  To compile this driver as a module, choose M here.  The module
>> +  will be called sun8i-emac.
>> +
>>  endif # NET_VENDOR_ALLWINNER
>> diff --git a/drivers/net/ethernet/allwinner/Makefile 
>> b/drivers/net/ethernet/allwinner/Makefile
>> index 03129f7..8bd1693c 100644
>> --- a/drivers/net/ethernet/allwinner/Makefile
>> +++ b/drivers/net/ethernet/allwinner/Makefile
>> @@ -3,3 +3,4 @@
>>  #
>>
>>  obj-$(CONFIG_SUN4I_EMAC) += sun4i-emac.o
>> +obj-$(CONFIG_SUN8I_EMAC) += sun8i-emac.o
>> diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c 
>> b/drivers/net/ethernet/allwinner/sun8i-emac.c
>> new file mode 100644
>> index 000..179aa61
>> --- /dev/null
>> +++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
>> @@ -0,0 +1,1943 @@
>> +/*
>> + * sun8i-emac driver
>> + *
>> + * Copyright (C) 2015-2016 Corentin LABBE 
>> + *
>> + * This is the driver for Allwinner Ethernet MAC found in H3/A83T/A64 SoC
>> + *
>> + * TODO:
>> + * - NAPI
>> + * - MAC filtering
>> + * - Jumbo frame
>> + * - features rx-all (NETIF_F_RXALL_BIT)
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define SUN8I_EMAC_BASIC_CTL0 0x00
>> +#define SUN8I_EMAC_BASIC_CTL1 0x04
>> +
>> +#define SUN8I_EMAC_MDIO_CMD 0x48
>> +#define SUN8I_EMAC_MDIO_DATA 0x4C
>
> Can you align all these register offsets with tabs, so that the actual
> offset values are below each other?
> Also ordering them by address seems useful to me.
>
>> +
>> +#define SUN8I_EMAC_RX_CTL0 0x24
>> +#define SUN8I_EMAC_RX_CTL1 0x28
>> +
>> +#define SUN8I_EMAC_TX_CTL0 0x10
>> +#define SUN8I_EMAC_TX_CTL1 0x14
>> +
>> +#define SUN8I_EMAC_TX_FLOW_CTL 0x1C
>> +
>> +#define SUN8I_EMAC_RX_FRM_FLT 0x38
>> +
>> +#define SUN8I_EMAC_INT_STA 0x08
>> +#define SUN8I_EMAC_INT_EN 0x0C
>> +#define SUN8I_EMAC_RGMII_STA 0xD0
>
>
>> +
>> +#define SUN8I_EMAC_TX_DMA_STA 0xB0
>> +#define SUN8I_EMAC_TX_CUR_DESC 0xB4
>> +#define SUN8I_EMAC_TX_CUR_BUF 0xB8
>> +#define SUN8I_EMAC_RX_DMA_STA 0xC0
>> +
>> +#define MDIO_CMD_MII_BUSYBIT(0)
>> +#define MDIO_CMD_MII_WRITE   BIT(1)
>> +#define MDIO_CMD_MII_PHY_REG_ADDR_MASK   GENMASK(8, 4)
>> +#define MDIO_CMD_MII_PHY_REG_ADDR_SHIFT  4
>> +#define MDIO_CMD_MII_PHY_ADDR_MASK   GENMASK(16, 12)
>> +#define MDIO_CMD_MII_PHY_ADDR_SHIFT  12
>> +
>> +#define SUN8I_EMAC_MACADDR_HI0x50
>> +#define SUN8I_EMAC_MACADDR_LO0x54
>> +
>> +#define SUN8I_EMAC_RX_DESC_LIST 0x34
>> +#define SUN8I_EMAC_TX_DESC_LIST 0x20
>> +
>> +#define SUN8I_EMAC_TX_DO_CRC (BIT(27) | BIT(28))
>> +#define SUN8I_EMAC_RX_DO_CRC BIT(27)
>> +#define SUN8I_EMAC_RX_STRIP_FCS BIT(28)
>> +
>> +#define SUN8I_COULD_BE_USED_BY_DMA BIT(31)
>> +
>> +#define FLOW_RX 1
>> +#define FLOW_TX 2
>> +
>> +/* describe how data from skb are DMA mapped */
>> +#define MAP_SINGLE 1
>> +#define MAP_PAGE 2
>> +
>> +enum emac_variant {
>> + A83T_EMAC,
>> + H3_EMAC,
>> + A64_EMAC,
>> +};
>> +
>> +struct ethtool_str {
>> + char name[ETH_GSTRING_LEN];
>> +};
>> +
>> +static const struct 

Re: [RFC] Reset pins of phys and their representation in a device tree

[Chen-Yu Tsai]

Hi,

On Thu, May 12, 2016 at 9:40 PM, Javier Martinez Canillas
 wrote:
> [adding Krzysztof as cc]
>
> On Thu, May 12, 2016 at 8:25 AM, Sergei Shtylyov
>  wrote:
>> Hello.
>>
>>
>> On 5/12/2016 10:15 AM, Uwe Kleine-König wrote:
>>
>>> I have a machine here where the reset pin of the phy is connected to a
>>> GPIO.
>>>
>>> There are different possibilities available today to handle this
>>> situation, here are the ones I'm aware of:
>>>
>>>  - Use a gpio-hog to set the reset gpio to non-active
>>>This might result in dependency problems (and that's what I am
>>>currently faced with) because there is no connection in the device
>>>tree between the hog and the phy.
>>>
>>>  - [Documentation/devicetree/bindings/net/fsl-fec.txt]
>>>The fec node supports properties
>>>
>>> phy-reset-gpios = < 14 0>;
>>> phy-reset-duration = <200> /* milliseconds */;
>>>
>>>Something similar exists in TI's vendor kernel
>>>
>>> (http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/17d192b999ee904ced223c16cef76111a51c461b)
>>>with different (and IMHO bader) naming.
>>>This is the wrong place to specify the gpios; they shouldn't be in the
>>>mac's node, but in a phy node instead.
>>>
>>> So what I actually want is to put the gpio specification in the right
>>> place and let it look as follows:
>>>
>>> mymdiobus {
>>> [...]
>>> myfirstphy: ethernet-phy@0 {
>>> compatible = "ethernet-phy-ieee802.3-c22";
>>> reg = <0>;
>>>
>>> reset-gpios = < 14 0>;
>>> reset-duration-ms = <200>;
>>> };
>>>
>>> mysecondphy: ethernet-phy@2 {
>>> compatible = "ethernet-phy-ieee802.3-c22";
>>> reg = <2>;
>>>
>>> reset-gpios = < 10 0>;
>>> reset-duration-ms = <200>;
>>> };
>>> };
>>>
>>> And with this we could defer probe of  if  isn't
>>> available yet.
>>>
>>> Does this sound sensible? Does something like that already exist which I
>>> missed? Any further ideas/comments?
>>
>>
>> http://patchwork.ozlabs.org/patch/616495/
>>
>>I'll post a new version RSN.
>>
>
> This seems to be similar to what's needed by MMC/SD/SDIO devices (i.e:
> a WiFi chip) that needs some power sequence for reset and be
> enumerable.
>
> Krzysztof has been working  to make the MMC pwrseq framework more
> generic [0] since he wants to use it also for built-in USB devices.
>
> From a quick look at the patches mentioned in this thread, it seems
> that the same framework can be used to reset the PHYs unless I'm
> missing something. Have you considered using this? It would be good if
> there is a consistent way to define the power sequence for devices
> across the different subsystems.
>
> [0]: https://lkml.org/lkml/2016/5/5/230

You probably missed the part that Rob nacked the bindings in that
series?

Putting the reset gpios under the PHY nodes and handling it from
phylib is probably the way to go. I'd like to ask for regulator
and clock support as well, if possible.

Regards
ChenYu

Re: [PATCH RFC 1/5] net: phy: sun8i-h3-ephy: Add bindings for Allwinner H3 Ethernet PHY

[Chen-Yu Tsai]

Hi,

On Tue, Apr 12, 2016 at 9:38 AM, Chen-Yu Tsai <w...@csie.org> wrote:
> On Tue, Apr 12, 2016 at 3:23 AM, Florian Fainelli <f.faine...@gmail.com> 
> wrote:
>> On 04/04/16 09:22, Chen-Yu Tsai wrote:
>>> The Allwinner H3 SoC incorporates an Ethernet PHY. This is enabled and
>>> configured through a memory mapped hardware register.
>>>
>>> This same register also configures the MAC interface mode and TX clock
>>> source. Also covered by the register, but not supported in these bindings,
>>> are TX/RX clock delay chains and inverters, and an RMII module.
>>>
>>> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>>> ---
>>>  .../bindings/net/allwinner,sun8i-h3-ephy.txt   | 44 
>>> ++
>>>  1 file changed, 44 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt 
>>> b/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
>>> new file mode 100644
>>> index ..146f227e6d88
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
>>> @@ -0,0 +1,44 @@
>>> +* Allwinner H3 E(thernet) PHY
>>> +
>>> +The Allwinner H3 integrates an MII ethernet PHY. As with external PHYs,
>>> +before it can be configured over the MDIO bus and used, certain hardware
>>> +features must be configured, such as the PHY address and LED polarity.
>>
>> Is the internal PHY address really configurable? Not that there is
>> anything wrong with it, this is good.
>
> It is. Things that are configured or provided to a discrete PHY are routed
> to registers in the SoC, things such as PHY address, clocks, resets.
>
>>> +The PHY must also be powered on and brought out of reset.
>>> +
>>> +This is accomplished with regulators and pull-up/downs for external PHYs.
>>> +For this internal PHY, a hardware register is programmed.
>>> +
>>> +The same hardware register also contains clock and interface controls
>>> +for the MAC. This is also present in earlier SoCs, and is covered by
>>> +"allwinner,sun7i-a20-gmac-clk". The controls in the H3 are slightly
>>> +different due to the inclusion of what appears to be an RMII-MII
>>> +bridge.
>>> +
>>> +Required properties:
>>> +- compatible: should be "allwinner,sun8i-h3-ephy"
>>> +- reg: address and length of the register set for the device
>>> +- clocks: A phandle to the reference clock for this device
>>> +- resets: A phandle to the reset control for this device
>>> +
>>> +Ethernet PHY related properties:
>>> +- allwinner,ephy-addr: the MDIO bus address the PHY should respond to.
>>> +If this is not set, the external PHY is used, and
>>> +everything else in this section is ignored.
>>
>> So we are going to put the same value here, and in the actual Ethernet
>> PHY device tree node that should be hanging off the EMAC/MDIO bus
>> controller, this is confusing and error prone.
>
> Yes, that would be an issue when writing the DTS.
>>
>>> +- allwinner,leds-active-low: LEDs are active low. Without this, LEDs are
>>> +  active high.
>>> +
>>> +Ethernet MAC clock related properties:
>>> +- #clock-cells: should be 0
>>> +- clock-output-names: "mac_tx"
>>> +
>>> +Example:
>>> +
>>> +ethernet-phy@01c00030 {
>>> + compatible = "allwinner,sun8i-h3-ephy";
>>> + reg = <0x01c00030 0x4>;
>>
>> Looking at this register space this looks more like an internal PHY SHIM
>> that is needed to be configured before the internal PHY can be access,
>> not a proper Ethernet PHY per-se, see replies in aptch 2.
>>
>> Should not this block be a second cell associated with the Ethernet MAC
>> block? One or the other are not going to be very useful without
>> knowledge of each other.
>
> True. However the lower half of the same register also controls the
> MAC interface mode and TX clock source and delays. This we had a clock
> driver that was used in conjuction with stmmac on earlier SoCs. I was
> hoping to keep that model with the newer EMAC. At the time it was
> argued that what seemed like a clock should be handled by a clock
> driver, instead of just a "syscon". If this is reaching too far to
> handle this new use case, I will happily just provide patches to merge
> this into the MAC.

Maxime, Hans, any thoughts?

It seems like it'd be easier to just fold this into the EMAC driver.
The register is not part of the clock controller in these new SoCs,
so it's nicer than what we had in A20/A31. It's also not just a clock
control, but a bunch of various controls.

ChenYu

> I would like to know how to deal with things like a PHY requiring
> some sort of shim driver, be it an internal one, or an external mfd
> chip that happens to have an Ethernet PHY included? How do we tie
> this into the PHY node under the MDIO bus?

Re: [PATCH RFC 2/5] net: phy: sun8i-h3-ephy: Add driver for Allwinner H3 Ethernet PHY

[Chen-Yu Tsai]

On Tue, Apr 12, 2016 at 3:23 AM, Florian Fainelli <f.faine...@gmail.com> wrote:
> On 04/04/16 09:22, Chen-Yu Tsai wrote:
>> The Allwinner H3 SoC incorporates an Ethernet PHY. This is enabled and
>> configured through a memory mapped hardware register.
>>
>> This same register also configures the MAC interface mode and TX clock
>> source. Also covered by the register, but not supported in this driver,
>> are TX/RX clock delay chains and inverters, and an RMII module.
>
> This is not really a PHY driver, more a driver for a special piece of
> hardware responsible for properly configuring a more standard integrated
> PHY which is then driven via standard MDIO accesses, right?
>
> The intention to make this driver re-usable is fine, but still makes me
> wonder if it should not be put in a file which is linked into the
> Ethernet MAC driver, and utilized by this one in a way that may be more
> "ad-hoc" than what you are proposing here.
>
> One thing that is not obvious here, is how is the device parenting done?
> Are we able to associate a phy_device to this SUN8I_H3_EPHY platform
> device here?

AFAIK you can't tie a platform device to an MDIO bus and phy_device.
I looked around for any examples and found none.

We're going to run into a similar issue with an MFD device later on.
The X-Powers AC200 is a I2C based combo chip which includes an Ethernet
PHY.

> Another thing is that the Ethernet MAC driver is fully aware of when
> putting an Ethernet PHY into suspend, shutdown, or fully functional
> power state should occur, if you have a separate platform driver here
> which does not listen for such kinds of events (hint: none are generated
> right now), then you cannot implement a working power state interface
> between the MAC, SHIM and PHY here, even though you would want that.

For this particular piece of hardware it would be possible to move the
code into the MAC driver itself. For the AC200, not so much. Using the
generic PHY framework to do power up / down would be a possibility.


Regards
ChenYu

Re: [PATCH RFC 1/5] net: phy: sun8i-h3-ephy: Add bindings for Allwinner H3 Ethernet PHY

[Chen-Yu Tsai]

On Tue, Apr 12, 2016 at 3:23 AM, Florian Fainelli <f.faine...@gmail.com> wrote:
> On 04/04/16 09:22, Chen-Yu Tsai wrote:
>> The Allwinner H3 SoC incorporates an Ethernet PHY. This is enabled and
>> configured through a memory mapped hardware register.
>>
>> This same register also configures the MAC interface mode and TX clock
>> source. Also covered by the register, but not supported in these bindings,
>> are TX/RX clock delay chains and inverters, and an RMII module.
>>
>> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>> ---
>>  .../bindings/net/allwinner,sun8i-h3-ephy.txt   | 44 
>> ++
>>  1 file changed, 44 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt 
>> b/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
>> new file mode 100644
>> index ..146f227e6d88
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
>> @@ -0,0 +1,44 @@
>> +* Allwinner H3 E(thernet) PHY
>> +
>> +The Allwinner H3 integrates an MII ethernet PHY. As with external PHYs,
>> +before it can be configured over the MDIO bus and used, certain hardware
>> +features must be configured, such as the PHY address and LED polarity.
>
> Is the internal PHY address really configurable? Not that there is
> anything wrong with it, this is good.

It is. Things that are configured or provided to a discrete PHY are routed
to registers in the SoC, things such as PHY address, clocks, resets.

>> +The PHY must also be powered on and brought out of reset.
>> +
>> +This is accomplished with regulators and pull-up/downs for external PHYs.
>> +For this internal PHY, a hardware register is programmed.
>> +
>> +The same hardware register also contains clock and interface controls
>> +for the MAC. This is also present in earlier SoCs, and is covered by
>> +"allwinner,sun7i-a20-gmac-clk". The controls in the H3 are slightly
>> +different due to the inclusion of what appears to be an RMII-MII
>> +bridge.
>> +
>> +Required properties:
>> +- compatible: should be "allwinner,sun8i-h3-ephy"
>> +- reg: address and length of the register set for the device
>> +- clocks: A phandle to the reference clock for this device
>> +- resets: A phandle to the reset control for this device
>> +
>> +Ethernet PHY related properties:
>> +- allwinner,ephy-addr: the MDIO bus address the PHY should respond to.
>> +If this is not set, the external PHY is used, and
>> +everything else in this section is ignored.
>
> So we are going to put the same value here, and in the actual Ethernet
> PHY device tree node that should be hanging off the EMAC/MDIO bus
> controller, this is confusing and error prone.

Yes, that would be an issue when writing the DTS.
>
>> +- allwinner,leds-active-low: LEDs are active low. Without this, LEDs are
>> +  active high.
>> +
>> +Ethernet MAC clock related properties:
>> +- #clock-cells: should be 0
>> +- clock-output-names: "mac_tx"
>> +
>> +Example:
>> +
>> +ethernet-phy@01c00030 {
>> + compatible = "allwinner,sun8i-h3-ephy";
>> + reg = <0x01c00030 0x4>;
>
> Looking at this register space this looks more like an internal PHY SHIM
> that is needed to be configured before the internal PHY can be access,
> not a proper Ethernet PHY per-se, see replies in aptch 2.
>
> Should not this block be a second cell associated with the Ethernet MAC
> block? One or the other are not going to be very useful without
> knowledge of each other.

True. However the lower half of the same register also controls the
MAC interface mode and TX clock source and delays. This we had a clock
driver that was used in conjuction with stmmac on earlier SoCs. I was
hoping to keep that model with the newer EMAC. At the time it was
argued that what seemed like a clock should be handled by a clock
driver, instead of just a "syscon". If this is reaching too far to
handle this new use case, I will happily just provide patches to merge
this into the MAC.

I would like to know how to deal with things like a PHY requiring
some sort of shim driver, be it an internal one, or an external mfd
chip that happens to have an Ethernet PHY included? How do we tie
this into the PHY node under the MDIO bus?


Regards
ChenYu

[PATCH RFC 0/5] net: phy: sun8i-h3-ephy: Add Allwinner H3 Ethernet PHY driver

[Chen-Yu Tsai]

Hi everyone,

This is an attempt to support the Ethernet PHY incorporated in Allwinner's
H3 SoC. It is a MII PHY, supporting 10/100 Mbps Ethernet.

Before the PHY can be accessed via MDIO and used, it needs to be powered
on and configured. This is done via a system control register in the CPU
address space. The same register also controls PHY interface mode and
MAC TX clock sources.

This driver brings up the Ethernet PHY (if it is used), and exports a
clock control for the MAC TX clock.

Patch 1 adds the bindings for this piece of hardware.

Patch 2 adds the driver for it.

Patch 3 adds a device node for the PHY.

Patch 4 & 5 are not to be merged. They are provided here as a solid
example of how this driver is used. The sun8i-emac bindings have not
been submitted and are not stable yet.

Comments welcome.

Regards
ChenYu


Chen-Yu Tsai (4):
  net: phy: sun8i-h3-ephy: Add bindings for Allwinner H3 Ethernet PHY
  net: phy: sun8i-h3-ephy: Add driver for Allwinner H3 Ethernet PHY
  ARM: dts: sun8i-h3: Add H3 Ethernet PHY device node to sun8i-h3.dtsi
  ARM: dts: sun8i: Enable Ethernet controller on the Orange PI PC

LABBE Corentin (1):
  ARM: dts: sun8i-h3: Add Ethernet controller device node to
sun8i-h3.dtsi

 .../bindings/net/allwinner,sun8i-h3-ephy.txt   |  44 
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |  14 ++
 arch/arm/boot/dts/sun8i-h3.dtsi|  21 ++
 drivers/net/phy/Kconfig|   8 +
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/sun8i-h3-ephy.c| 266 +
 6 files changed, 354 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
 create mode 100644 drivers/net/phy/sun8i-h3-ephy.c

-- 
2.7.0

[PATCH RFC 3/5] ARM: dts: sun8i-h3: Add H3 Ethernet PHY device node to sun8i-h3.dtsi

[Chen-Yu Tsai]

The Allwinner H3 SoC incorporates an Ethernet PHY, whose controls are
mapped to a system control register.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 4a4926b0b0ed..9a28aeba9bc6 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -323,6 +323,15 @@
#size-cells = <1>;
ranges;
 
+   ephy: ethernet-phy@01c00030 {
+   compatible = "allwinner,sun8i-h3-ephy";
+   reg = <0x01c00030 0x4>;
+   clocks = <_gates 128>;
+   resets = <_rst 66>;
+   #clock-cells = <0>;
+   clock-output-names = "emac_tx";
+   };
+
dma: dma-controller@01c02000 {
compatible = "allwinner,sun8i-h3-dma";
reg = <0x01c02000 0x1000>;
-- 
2.7.0

[PATCH RFC 5/5] ARM: dts: sun8i: Enable Ethernet controller on the Orange PI PC

[Chen-Yu Tsai]

The Orange PI PC uses the H3's internal Ethernet PHY with the EMAC
Ethernet controller.

Set a proper address for the PHY and enable the EMAC.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---

This patch depends on "ARM: dts: sun8i-h3: Add Ethernet controller device",
which uses an binding still in development.

Do not merge.

---
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index daf50b9a6657..f01e10df812a 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -102,6 +102,20 @@
status = "okay";
 };
 
+ {
+   allwinner,ephy-addr = <0x1>;
+};
+
+ {
+   phy = <>;
+   phy-mode = "mii";
+   status = "okay";
+
+   phy1: ethernet-phy@1 {
+   reg = <1>;
+   };
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
-- 
2.7.0

[PATCH RFC 4/5] ARM: dts: sun8i-h3: Add Ethernet controller device node to sun8i-h3.dtsi

[Chen-Yu Tsai]

From: LABBE Corentin <clabbe.montj...@gmail.com>

The Allwinner H3 SoC has a gigabit Ethernet controller.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
[w...@csie.org: drop pinmux; update clocks/resets]
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---

This is not a stable binding. Do not merge.

---
 arch/arm/boot/dts/sun8i-h3.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 9a28aeba9bc6..7749af6354bb 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -635,6 +635,18 @@
status = "disabled";
};
 
+   emac: ethernet@1c3 {
+   compatible = "allwinner,sun8i-h3-emac";
+   reg = <0x01c3 0x1000>;
+   interrupts = ;
+   resets = <_rst 17>;
+   reset-names = "ahb";
+   clocks = <_gates 17>, <>;
+   clock-names = "ahb", "tx";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
gic: interrupt-controller@01c81000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c81000 0x1000>,
-- 
2.7.0

[PATCH RFC 2/5] net: phy: sun8i-h3-ephy: Add driver for Allwinner H3 Ethernet PHY

[Chen-Yu Tsai]

The Allwinner H3 SoC incorporates an Ethernet PHY. This is enabled and
configured through a memory mapped hardware register.

This same register also configures the MAC interface mode and TX clock
source. Also covered by the register, but not supported in this driver,
are TX/RX clock delay chains and inverters, and an RMII module.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/net/phy/Kconfig |   8 ++
 drivers/net/phy/Makefile|   1 +
 drivers/net/phy/sun8i-h3-ephy.c | 266 
 3 files changed, 275 insertions(+)
 create mode 100644 drivers/net/phy/sun8i-h3-ephy.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 6dad9a9c356c..f4b9eb6e9114 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -276,3 +276,11 @@ endif # PHYLIB
 config MICREL_KS8995MA
tristate "Micrel KS8995MA 5-ports 10/100 managed Ethernet switch"
depends on SPI
+
+config SUN8I_H3_EPHY
+   tristate "Allwinner sun8i H3 Ethernet PHY"
+   depends on OF && HAS_IOMEM && COMMON_CLK && RESET_CONTROLLER
+   depends on MACH_SUN8I || COMPILE_TEST
+   help
+ This modules provides a driver for the internal Ethernet PHY
+ and Ethernet MAC controls found in the Allwinner H3 SoC.
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index fcdbb9299fab..7cec3aceb518 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o
 obj-$(CONFIG_MDIO_BCM_UNIMAC)  += mdio-bcm-unimac.o
 obj-$(CONFIG_MICROCHIP_PHY)+= microchip.o
 obj-$(CONFIG_MDIO_BCM_IPROC)   += mdio-bcm-iproc.o
+obj-$(CONFIG_SUN8I_H3_EPHY)+= sun8i-h3-ephy.o
diff --git a/drivers/net/phy/sun8i-h3-ephy.c b/drivers/net/phy/sun8i-h3-ephy.c
new file mode 100644
index ..bf2703bee024
--- /dev/null
+++ b/drivers/net/phy/sun8i-h3-ephy.c
@@ -0,0 +1,266 @@
+/*
+ * Allwinner sun8i H3 E(thernet) PHY driver
+ *
+ * Copyright (C) 2016 Chen-Yu Tsai <w...@csie.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define REG_PHY_ADDR_SHIFT 20
+#define REG_PHY_ADDR_MASK  GENMASK(4, 0)
+#define REG_LED_POLBIT(17) /* 1: active low, 0: active high */
+#define REG_SHUTDOWN   BIT(16) /* 1: shutdown, 0: power up */
+#define REG_PHY_SELECT BIT(15) /* 1: internal PHY, 0: external PHY */
+
+#define REG_DEFAULT_VALUE  0x58000
+#define REG_DEFAULT_MASK   GENMASK(31, 15)
+
+#define REG_GPIT   2
+#define REG_ETCS_MASK  0x3
+#define SUN8I_H3_EMAC_PARENTS  2
+#define MII_PHY_CLK_RATE   2500
+#define INT_TX_CLK_RATE12500
+#define MII_PHY_CLK_NAME   "mii_phy_tx"
+#define INT_TX_CLK_NAME"int_tx"
+
+struct sun8i_h3_ephy {
+   struct device *dev;
+   void __iomem *reg;
+   struct clk *clk;
+   struct reset_control *reset;
+   struct clk *mac_clks[SUN8I_H3_EMAC_PARENTS + 1];
+};
+
+static DEFINE_SPINLOCK(sun8i_h3_ephy_lock);
+
+/* The EMAC clock/interface controls are much like those found on the A20,
+ * which is supported by sun7i-a20-gmac-clk. The H3 adds an RMII module,
+ * enabled by bit 13. Unfortunately vendor code and documentation don't
+ * explain  how it should work with the other bits, and no hardware
+ * actually uses it.
+ */
+static u32 sun8i_h3_emac_mux_table[SUN8I_H3_EMAC_PARENTS] = {
+   0x0,/* Select TX clock from MII PHY */
+   0x2,/* Select internal TX clock (for RGMII) */
+};
+
+static const char *sun8i_h3_emac_mux_parents[SUN8I_H3_EMAC_PARENTS] = {
+   MII_PHY_CLK_NAME,
+   INT_TX_CLK_NAME,
+};
+
+int sun8i_h3_ephy_register_clocks(struct sun8i_h3_ephy *phy)
+{
+   struct device *dev = phy->dev;
+   struct device_node *np = dev->of_node;
+   const char *clk_name;
+   struct clk_mux *mux;
+   struct clk_gate *gate;
+   int ret;
+
+   if (of_property_read_string(np, "clock-output-names", _name)) {
+   dev_err(phy->dev, "No clock output name given\n");
+   return -EINVAL;
+   }
+
+   /* allocate mux and gate clock structs */
+   mux = devm_kzalloc(dev, sizeof(struct clk_mux), GFP_KERNEL);
+   if (!mux)
+   return -ENOMEM;
+
+   gate = devm_kzalloc(dev, sizeof(struct clk_gate), 

[PATCH RFC 1/5] net: phy: sun8i-h3-ephy: Add bindings for Allwinner H3 Ethernet PHY

[Chen-Yu Tsai]

The Allwinner H3 SoC incorporates an Ethernet PHY. This is enabled and
configured through a memory mapped hardware register.

This same register also configures the MAC interface mode and TX clock
source. Also covered by the register, but not supported in these bindings,
are TX/RX clock delay chains and inverters, and an RMII module.

Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 .../bindings/net/allwinner,sun8i-h3-ephy.txt   | 44 ++
 1 file changed, 44 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt 
b/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
new file mode 100644
index ..146f227e6d88
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-h3-ephy.txt
@@ -0,0 +1,44 @@
+* Allwinner H3 E(thernet) PHY
+
+The Allwinner H3 integrates an MII ethernet PHY. As with external PHYs,
+before it can be configured over the MDIO bus and used, certain hardware
+features must be configured, such as the PHY address and LED polarity.
+The PHY must also be powered on and brought out of reset.
+
+This is accomplished with regulators and pull-up/downs for external PHYs.
+For this internal PHY, a hardware register is programmed.
+
+The same hardware register also contains clock and interface controls
+for the MAC. This is also present in earlier SoCs, and is covered by
+"allwinner,sun7i-a20-gmac-clk". The controls in the H3 are slightly
+different due to the inclusion of what appears to be an RMII-MII
+bridge.
+
+Required properties:
+- compatible: should be "allwinner,sun8i-h3-ephy"
+- reg: address and length of the register set for the device
+- clocks: A phandle to the reference clock for this device
+- resets: A phandle to the reset control for this device
+
+Ethernet PHY related properties:
+- allwinner,ephy-addr: the MDIO bus address the PHY should respond to.
+  If this is not set, the external PHY is used, and
+  everything else in this section is ignored.
+- allwinner,leds-active-low: LEDs are active low. Without this, LEDs are
+active high.
+
+Ethernet MAC clock related properties:
+- #clock-cells: should be 0
+- clock-output-names: "mac_tx"
+
+Example:
+
+ethernet-phy@01c00030 {
+   compatible = "allwinner,sun8i-h3-ephy";
+   reg = <0x01c00030 0x4>;
+   clocks = <_gates 128>;
+   resets = <_rst 66>;
+   allwinner,ephy-addr = <0x1>;
+   #clock-cells = <0>;
+   clock-output-names = "mac_tx";
+};
-- 
2.7.0

Re: [PATCH v5 2/4] Documentation: Bindings: Add STM32 DWMAC glue

[Chen-Yu Tsai]

On Mon, Mar 21, 2016 at 10:02 PM, Giuseppe CAVALLARO
<peppe.cavall...@st.com> wrote:
> On 3/21/2016 11:45 AM, Alexandre Torgue wrote:
>>
>> Hi,
>>
>> 2016-03-18 17:00 GMT+01:00 Chen-Yu Tsai <w...@csie.org>:
>>>
>>> Hi,
>>>
>>> On Fri, Mar 18, 2016 at 11:37 PM, Alexandre TORGUE
>>> <alexandre.tor...@gmail.com> wrote:
>>>>
>>>> Signed-off-by: Alexandre TORGUE <alexandre.tor...@gmail.com>
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>>> b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>>> new file mode 100644
>>>> index 000..ada2aa4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>>> @@ -0,0 +1,32 @@
>>>> +STMicroelectronics STM32 / MCU DWMAC glue layer controller
>>>> +
>>>> +This file documents platform glue layer for stmmac.
>>>> +Please see stmmac.txt for the other unchanged properties.
>>>> +
>>>> +The device node has following properties.
>>>> +
>>>> +Required properties:
>>>> +- compatible:  Should be "st,stm32-dwmac" to select glue, and
>>>> +  "snps,dwmac-3.50a" to select IP vesrion.
>
>
> Almost all the synp gmac chips have the HW capability register that is
> used for setting all the parameters at probe time.
> This will override fields passed from DT. In theory, it is not
> necessary to pass: "snps,dwmac-3.50a" from device tree at least there
> is either no HW cap reg or the glue has some w/a for a specific chip
> revision.
> To be honest, I like to see the "snps,dwmac-3.50a" as compatibility
> to also have a better readability (that's my personal view ;-) ).

I agree having the versioned strings is good for informational purposes,
and to signal hardware capability. It is not so good for directly
binding drivers in the implementation though.

Unfortunately, as Joachim pointed out, exynos5440 uses it so we cannot
change it.

ChenYu

> Peppe
>
>
>>>
>>> If you need have sort of hardware glue, then it is not compatible.
>>>
>>
>> We could have the case where the glue is set by a bootloader.
>> In this case, we will select IP version in compatible and we will use
>> generic dwmac glue to probe stmmac driver.
>>
>> Regards
>>
>> Alex.
>>
>>> ChenYu
>>>
>>>> +- clocks: Must contain a phandle for each entry in clock-names.
>>>> +- clock-names: Should be "stmmaceth" for the host clock.
>>>> +  Should be "tx-clk" for the MAC TX clock.
>>>> +  Should be "rx-clk" for the MAC RX clock.
>>>> +- st,syscon : Should be phandle/offset pair. The phandle to the syscon
>>>> node which
>>>> + encompases the glue register, and the offset of the
>>>> control register.
>>>> +Example:
>>>> +
>>>> +   ethernet0: dwmac@40028000 {
>>>> +   compatible = "st,stm32-dwmac",
>>>> "snps,dwmac-3.50a";
>>>> +   status = "disabled";
>>>> +   reg = <0x40028000 0x8000>;
>>>> +   reg-names = "stmmaceth";
>>>> +   interrupts = <0 61 0>, <0 62 0>;
>>>> +   interrupt-names = "macirq", "eth_wake_irq";
>>>> +   clock-names = "stmmaceth", "tx-clk", "rx-clk";
>>>> +   clocks = < 0 25>, < 0 26>, < 0 27>;
>>>> +   st,syscon = < 0x4>;
>>>> +   snps,pbl = <8>;
>>>> +   snps,mixed-burst;
>>>> +   dma-ranges;
>>>> +   };
>>>> --
>>>> 1.9.1
>>>>
>>>>
>>>> ___
>>>> linux-arm-kernel mailing list
>>>> linux-arm-ker...@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>>
>

Re: [PATCH v5 2/4] Documentation: Bindings: Add STM32 DWMAC glue

[Chen-Yu Tsai]

On Mon, Mar 21, 2016 at 6:45 PM, Alexandre Torgue
<alexandre.tor...@gmail.com> wrote:
> Hi,
>
> 2016-03-18 17:00 GMT+01:00 Chen-Yu Tsai <w...@csie.org>:
>> Hi,
>>
>> On Fri, Mar 18, 2016 at 11:37 PM, Alexandre TORGUE
>> <alexandre.tor...@gmail.com> wrote:
>>> Signed-off-by: Alexandre TORGUE <alexandre.tor...@gmail.com>
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt 
>>> b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>> new file mode 100644
>>> index 000..ada2aa4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>> @@ -0,0 +1,32 @@
>>> +STMicroelectronics STM32 / MCU DWMAC glue layer controller
>>> +
>>> +This file documents platform glue layer for stmmac.
>>> +Please see stmmac.txt for the other unchanged properties.
>>> +
>>> +The device node has following properties.
>>> +
>>> +Required properties:
>>> +- compatible:  Should be "st,stm32-dwmac" to select glue, and
>>> +  "snps,dwmac-3.50a" to select IP vesrion.
>>
>> If you need have sort of hardware glue, then it is not compatible.
>>
>
> We could have the case where the glue is set by a bootloader.
> In this case, we will select IP version in compatible and we will use
> generic dwmac glue to probe stmmac driver.

It seems most platforms using DWMAC follow this design set by
the original stmmac bindings. I'm arguing that the requirement
of setting up the glue makes them incompatible.

What happens when the bootloader didn't setup the glue? And one
forgets to build the STM32 driver, only the generic one? The
generic driver even matches to some, but not all, version
strings.

Maybe it would've been better if the versioned strings were
only used to indicate functionality, and not used to bind
the drivers. But the bindings were set some time ago.


Regards
ChenYu


> Regards
>
> Alex.
>
>> ChenYu
>>
>>> +- clocks: Must contain a phandle for each entry in clock-names.
>>> +- clock-names: Should be "stmmaceth" for the host clock.
>>> +  Should be "tx-clk" for the MAC TX clock.
>>> +  Should be "rx-clk" for the MAC RX clock.
>>> +- st,syscon : Should be phandle/offset pair. The phandle to the syscon 
>>> node which
>>> + encompases the glue register, and the offset of the control 
>>> register.
>>> +Example:
>>> +
>>> +   ethernet0: dwmac@40028000 {
>>> +   compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
>>> +   status = "disabled";
>>> +   reg = <0x40028000 0x8000>;
>>> +   reg-names = "stmmaceth";
>>> +   interrupts = <0 61 0>, <0 62 0>;
>>> +   interrupt-names = "macirq", "eth_wake_irq";
>>> +   clock-names = "stmmaceth", "tx-clk", "rx-clk";
>>> +   clocks = < 0 25>, < 0 26>, < 0 27>;
>>> +   st,syscon = < 0x4>;
>>> +   snps,pbl = <8>;
>>> +   snps,mixed-burst;
>>> +   dma-ranges;
>>> +   };
>>> --
>>> 1.9.1
>>>
>>>
>>> ___
>>> linux-arm-kernel mailing list
>>> linux-arm-ker...@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [patch] mdio-sun4i: oops in error handling in probe

[Chen-Yu Tsai]

On Mon, Mar 21, 2016 at 5:02 PM, Dan Carpenter <dan.carpen...@oracle.com> wrote:
> We could end up dereferencing an error pointer when we call
> regulator_disable().
>
> Fixes: 4bdcb1dd9feb ('net: Add MDIO bus driver for the Allwinner EMAC')
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>

Acked-by: Chen-Yu Tsai <w...@csie.org>