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

[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 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 <wens@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 000000000000..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 = <&bus_gates 128>;
+	resets = <&ahb_rst 66>;
+	allwinner,ephy-addr = <0x1>;
+	#clock-cells = <0>;
+	clock-output-names = "mac_tx";
+};
-- 
2.7.0

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

[Rob Herring]

On Tue, Apr 05, 2016 at 12:22:30AM +0800, 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 <wens@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

Acked-by: Rob Herring <robh@kernel.org>

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

[Florian Fainelli]

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 <wens@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 000000000000..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.

> +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.

> +- 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.
-- 
Florian

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.fainelli@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 <wens@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 000000000000..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

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 <wens@csie.org> wrote:
> On Tue, Apr 12, 2016 at 3:23 AM, Florian Fainelli <f.fainelli@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 <wens@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 000000000000..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 1/5] net: phy: sun8i-h3-ephy: Add bindings for Allwinner H3 Ethernet PHY

[Hans de Goede]

Hi,

On 07-05-16 07:30, Chen-Yu Tsai wrote:
> Hi,
>
> On Tue, Apr 12, 2016 at 9:38 AM, Chen-Yu Tsai <wens@csie.org> wrote:
>> On Tue, Apr 12, 2016 at 3:23 AM, Florian Fainelli <f.fainelli@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 <wens@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 000000000000..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.

I don't know the emac stuff well enough to really have an opinion,
but with that said I've no objections against just folding this into
the driver. If you believe that is best, go for it.

Regards,

Hans

[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 <wens@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 000000000000..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 <wens@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 <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#define REG_PHY_ADDR_SHIFT	20
+#define REG_PHY_ADDR_MASK	GENMASK(4, 0)
+#define REG_LED_POL		BIT(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	25000000
+#define INT_TX_CLK_RATE		125000000
+#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", &clk_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), GFP_KERNEL);
+	if (!gate)
+		return -ENOMEM;
+
+	/* Register fixed clocks to use as parents */
+	phy->mac_clks[0] = clk_register_fixed_rate(dev, MII_PHY_CLK_NAME,
+						   NULL, 0, MII_PHY_CLK_RATE);
+	if (IS_ERR(phy->mac_clks[0]))
+		return PTR_ERR(phy->mac_clks[0]);
+
+	phy->mac_clks[1] = clk_register_fixed_rate(dev, INT_TX_CLK_NAME,
+						   NULL, 0, INT_TX_CLK_RATE);
+	if (IS_ERR(phy->mac_clks[1])) {
+		ret = PTR_ERR(phy->mac_clks[1]);
+		goto err_int_tx_clk;
+	}
+
+	/* set up gate and mux properties */
+	gate->reg = phy->reg;
+	gate->bit_idx = REG_GPIT;
+	gate->lock = &sun8i_h3_ephy_lock;
+	mux->reg = phy->reg;
+	mux->mask = REG_ETCS_MASK;
+	mux->table = sun8i_h3_emac_mux_table;
+	mux->lock = &sun8i_h3_ephy_lock;
+
+	phy->mac_clks[2] = clk_register_composite(dev, clk_name,
+						  sun8i_h3_emac_mux_parents,
+						  SUN8I_H3_EMAC_PARENTS,
+						  &mux->hw, &clk_mux_ops,
+						  NULL, NULL,
+						  &gate->hw, &clk_gate_ops,
+						  0);
+	if (IS_ERR(phy->mac_clks[2])) {
+		ret = PTR_ERR(phy->mac_clks[2]);
+		goto err_tx_clk;
+	}
+
+	ret = of_clk_add_provider(dev->of_node, of_clk_src_simple_get,
+				  phy->mac_clks[2]);
+	if (ret)
+		goto err_of_clk;
+
+	return 0;
+
+err_of_clk:
+	/* TODO: switch to clk_unregister_composite when it's available */
+	clk_unregister(phy->mac_clks[2]);
+err_tx_clk:
+	clk_unregister_fixed_rate(phy->mac_clks[1]);
+err_int_tx_clk:
+	clk_unregister_fixed_rate(phy->mac_clks[0]);
+
+	return ret;
+}
+
+static void sun8i_h3_ephy_init_phy(struct sun8i_h3_ephy *phy, u32 addr)
+{
+	u32 val = readl(phy->reg);
+
+	/* set default values, but don't touch clock settings */
+	val = (val & ~REG_DEFAULT_MASK) | REG_DEFAULT_VALUE;
+
+	if (addr < REG_PHY_ADDR_MASK) {
+		dev_info(phy->dev, "Using interal PHY at 0x%x\n", addr);
+
+		val |= addr << REG_PHY_ADDR_SHIFT;
+		val &= ~REG_SHUTDOWN;
+		val |= REG_PHY_SELECT;
+
+		if (of_property_read_bool(phy->dev->of_node,
+					  "allwinner,leds-active-low"))
+			val |= REG_LED_POL;
+	} else {
+		dev_info(phy->dev, "Using external PHY\n");
+	}
+
+	writel(val, phy->reg);
+}
+
+static void sun8i_h3_ephy_reset_phy(struct sun8i_h3_ephy *phy)
+{
+	u32 val;
+
+	val = readl(phy->reg);
+	val = (val & ~REG_DEFAULT_MASK) | REG_DEFAULT_VALUE;
+	writel(val, phy->reg);
+}
+
+int sun8i_h3_ephy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct sun8i_h3_ephy *phy;
+	struct resource *res;
+	u32 addr = REG_PHY_ADDR_MASK;
+	int ret;
+
+	ret = of_property_read_u32(np, "allwinner,ephy-addr", &addr);
+	if (!ret && addr > REG_PHY_ADDR_MASK) {
+		dev_err(dev, "invalid PHY address: 0x%x\n", addr);
+		return -EINVAL;
+	} else if (ret && ret != -EINVAL) {
+		dev_err(dev, "could not get PHY address: %d\n", ret);
+		return ret;
+	}
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->dev = dev;
+	platform_set_drvdata(pdev, phy);
+
+	phy->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(phy->clk)) {
+		dev_err(dev, "failed to get clock\n");
+		return PTR_ERR(phy->clk);
+	}
+
+	phy->reset = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(phy->reset)) {
+		dev_err(dev, "failed to get reset control\n");
+		return PTR_ERR(phy->reset);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	phy->reg = devm_ioremap_resource(dev, res);
+	if (IS_ERR(phy->reg)) {
+		dev_err(dev, "failed to map registers\n");
+		return PTR_ERR(phy->reg);
+	}
+
+	ret = clk_prepare_enable(phy->clk);
+	if (ret) {
+		dev_err(dev, "failed to enable clock\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(phy->reset);
+	if (ret) {
+		dev_err(dev, "failed to deassert reset control\n");
+		goto err_reset;
+	}
+
+	sun8i_h3_ephy_init_phy(phy, addr);
+
+	ret = sun8i_h3_ephy_register_clocks(phy);
+	if (ret)
+		goto err_tx_clk;
+
+	return 0;
+
+err_tx_clk:
+	sun8i_h3_ephy_reset_phy(phy);
+	reset_control_assert(phy->reset);
+err_reset:
+	clk_disable_unprepare(phy->clk);
+	return ret;
+}
+
+static const struct of_device_id sun8i_h3_ephy_of_match[] = {
+	{ .compatible = "allwinner,sun8i-h3-ephy", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun8i_h3_ephy_of_match);
+
+static struct platform_driver sun8i_h3_ephy_driver = {
+	.probe	= sun8i_h3_ephy_probe,
+	.driver = {
+		.of_match_table	= sun8i_h3_ephy_of_match,
+		.name  = "sun8i-h3-ephy",
+	}
+};
+module_platform_driver(sun8i_h3_ephy_driver);
+
+MODULE_DESCRIPTION("Allwinner sun8i H3 Ethernet PHY driver");
+MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.7.0

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

[Florian Fainelli]

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?

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.
-- 
Florian

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.fainelli@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

[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 <wens@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 = <&bus_gates 128>;
+			resets = <&ahb_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 4/5] ARM: dts: sun8i-h3: Add Ethernet controller device node to sun8i-h3.dtsi

[Chen-Yu Tsai]

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

The Allwinner H3 SoC has a gigabit Ethernet controller.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
[wens@csie.org: drop pinmux; update clocks/resets]
Signed-off-by: Chen-Yu Tsai <wens@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@1c30000 {
+			compatible = "allwinner,sun8i-h3-emac";
+			reg = <0x01c30000 0x1000>;
+			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&ahb_rst 17>;
+			reset-names = "ahb";
+			clocks = <&bus_gates 17>, <&ephy>;
+			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 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 <wens@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";
 };
 
+&ephy {
+	allwinner,ephy-addr = <0x1>;
+};
+
+&emac {
+	phy = <&phy1>;
+	phy-mode = "mii";
+	status = "okay";
+
+	phy1: ethernet-phy@1 {
+		reg = <1>;
+	};
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ir_pins_a>;
-- 
2.7.0

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

[Florian Fainelli]

On 04/04/16 09:22, Chen-Yu Tsai wrote:
> 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 <wens@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";
>  };
>  
> +&ephy {
> +	allwinner,ephy-addr = <0x1>;
> +};
> +
> +&emac {
> +	phy = <&phy1>;
> +	phy-mode = "mii";
> +	status = "okay";
> +
> +	phy1: ethernet-phy@1 {
> +		reg = <1>;
> +	};
> +};

As commented in patch 1, the fact that you have to put the Ethernet PHY
address twice here is not really a good thing, because they cannot be
dissociated from eath other, I would rather have a standard Ethernet PHY
DT node represent the desired PHY address, and have your glue/SHIM
SUN8I_H3_EMAC driver scan the Device Tree to know what address to
program for the Ethernet PHY.
-- 
Florian