Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1479955 > unrolled thread

[PATCH v3 0/9] net-next: ethernet: add sun8i-emac driver

Started byCorentin Labbe <clabbe.montjoie@gmail.com>
First post2016-09-09 14:50 +0200
Last post2016-09-10 06:00 +0200
Articles 20 on this page of 27 — 6 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 0/9] net-next: ethernet: add sun8i-emac driver Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
    [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
      Re: [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-12 09:30 +0200
        Re: [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver LABBE Corentin <clabbe.montjoie@gmail.com> - 2016-09-14 11:00 +0200
    [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
      Re: [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon  control module Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-12 09:30 +0200
        Re: [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon  control module LABBE Corentin <clabbe.montjoie@gmail.com> - 2016-09-13 15:40 +0200
    [PATCH v3 6/9] ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
    [PATCH v3 2/9] MAINTAINERS: Add myself as maintainer of sun8i-emac Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
    [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
      Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-12 22:50 +0200
        Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support LABBE Corentin <clabbe.montjoie@gmail.com> - 2016-09-14 16:10 +0200
          Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-17 15:50 +0200
          Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support Florian Fainelli <f.fainelli@gmail.com> - 2016-09-17 19:00 +0200
    [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
      Re: [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on  sunxi_defconfig Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-12 09:40 +0200
        Re: [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on  sunxi_defconfig LABBE Corentin <clabbe.montjoie@gmail.com> - 2016-09-13 15:50 +0200
    [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-09-09 14:50 +0200
      Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for  Allwinner sun8i-emac Andrew Lunn <andrew@lunn.ch> - 2016-09-09 16:10 +0200
        Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for  Allwinner sun8i-emac LABBE Corentin <clabbe.montjoie@gmail.com> - 2016-09-12 17:10 +0200
          Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for  Allwinner sun8i-emac Andrew Lunn <andrew@lunn.ch> - 2016-09-12 17:20 +0200
      Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for  Allwinner sun8i-emac Andrew Lunn <andrew@lunn.ch> - 2016-09-09 16:20 +0200
        Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for  Allwinner sun8i-emac LABBE Corentin <clabbe.montjoie@gmail.com> - 2016-09-13 15:40 +0200
          Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for  Allwinner sun8i-emac Andrew Lunn <andrew@lunn.ch> - 2016-09-13 16:20 +0200
    Re: [PATCH v3 1/9] ethernet: add sun8i-emac driver Andrew Lunn <andrew@lunn.ch> - 2016-09-09 16:20 +0200
      Re: [PATCH v3 1/9] ethernet: add sun8i-emac driver LABBE Corentin <clabbe.montjoie@gmail.com> - 2016-09-13 15:40 +0200
    Re: [PATCH v3 0/9] net-next: ethernet: add sun8i-emac driver David Miller <davem@davemloft.net> - 2016-09-10 06:00 +0200

Page 1 of 2  [1] 2  Next page →


#1479955 — [PATCH v3 0/9] net-next: ethernet: add sun8i-emac driver

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[PATCH v3 0/9] net-next: ethernet: add sun8i-emac driver
Message-ID<sftpM-2S8-9@gated-at.bofh.it>
Hello

This patch series add the driver for sun8i-emac which handle the Ethernet MAC
present on 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.

This patch series enable the driver only for the H3 SoC since A83T and A64
doesn't have the necessary clocks present in mainline.

This patch series enable the driver only for OrangePiPC and OrangePIOne boards
since other board with H3 use external PHY which need optional regulators
that will be supported later.

The driver have been tested on the following boards:
- H3 Orange PI PC, Orange PI Plus, BananaPI-M2+
- A64 Pine64
- A83T BananaPI-M3

I would like to thanks Chen-Yu Tsai for his help on developing this driver.

Regards

Changes since v2
- Added patch to support Orange PI One
- Added a patch to enable SUN8I_EMAC in sunxi_defconfig
- Added a patch to enable pm_runtime
- The Emac clock is now used standard syscon/regmap
- Added netdev_sent_queue/netdev_completed_queue
- Added lots of documentation on working (locks, memory barrier, etc...)
- Cleaned DT bindings documentation
- Added ethernet0 aliases as suggested by Hans de Goede
- Change prefix of all DEFINE to EMAC_
- The driver is now endian safe
- Reworked the internal PHY mechanism
- Removed dma_set_mask_and_coherent(32) since it is the default for of_platform_driver

Changes since v1
- Implement NAPI
- Sorted and reworded all define
- Reworked ethtools stats strings
- Removed all unneeded __packked and __aligned
- Added tuning of RX/TX ring size via ethtool
- Corrected use of sk/skb naming
- Added some wmb when needed
- Moved irq claim/free to emac_open/close
- Lots of code refactoring

Corentin Labbe (8):
  ethernet: sun8i-emac: add pm_runtime support
  ethernet: add sun8i-emac driver
  MAINTAINERS: Add myself as maintainer of sun8i-emac
  ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
  ARM: dts: sun8i-h3: Add dt node for the syscon control module
  ARM: dts: sun8i-h3: add sun8i-emac ethernet driver
  ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC
  ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig

Hans de Goede (1):
  ARM: dts: sun8i: Enable sun8i-emac on the Orange PI One

 .../bindings/net/allwinner,sun8i-emac.txt          |   64 +
 MAINTAINERS                                        |    6 +
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts        |   10 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts         |   10 +
 arch/arm/boot/dts/sun8i-h3.dtsi                    |   24 +
 arch/arm/configs/sunxi_defconfig                   |    1 +
 drivers/net/ethernet/allwinner/Kconfig             |   13 +
 drivers/net/ethernet/allwinner/Makefile            |    1 +
 drivers/net/ethernet/allwinner/sun8i-emac.c        | 2313 ++++++++++++++++++++
 9 files changed, 2442 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
 create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c

-- 
2.7.3

[toc] | [next] | [standalone]


#1479957 — [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver
Message-ID<sftpN-2S8-39@gated-at.bofh.it>
In reply to#1479955
The sun8i-emac is an ethernet MAC hardware that support 10/100/1000
speed.

This patch enable the sun8i-emac on the Allwinner H3 SoC Device-tree.
The SoC H3 have an internal PHY, so optionals syscon and ephy are set.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index a39da6f..a3ac476 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -50,6 +50,10 @@
 / {
 	interrupt-parent = <&gic>;
 
+	aliases {
+		ethernet0 = &emac;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -446,6 +450,21 @@
 			status = "disabled";
 		};
 
+		emac: ethernet@1c30000 {
+			compatible = "allwinner,sun8i-h3-emac";
+			syscon = <&syscon>;
+			reg = <0x01c30000 0x104>;
+			reg-names = "emac";
+			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&ccu RST_BUS_EMAC>, <&ccu RST_BUS_EPHY>;
+			reset-names = "ahb", "ephy";
+			clocks = <&ccu CLK_BUS_EMAC>, <&ccu CLK_BUS_EPHY>;
+			clock-names = "ahb", "ephy";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		gic: interrupt-controller@01c81000 {
 			compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
 			reg = <0x01c81000 0x1000>,
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1480968 — Re: [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-09-12 09:30 +0200
SubjectRe: [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver
Message-ID<sgtQJ-ct-7@gated-at.bofh.it>
In reply to#1479957

[Multipart message — attachments visible in raw view] — view raw

On Fri, Sep 09, 2016 at 02:45:13PM +0200, Corentin Labbe wrote:
> The sun8i-emac is an ethernet MAC hardware that support 10/100/1000
> speed.
> 
> This patch enable the sun8i-emac on the Allwinner H3 SoC Device-tree.
> The SoC H3 have an internal PHY, so optionals syscon and ephy are set.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index a39da6f..a3ac476 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -50,6 +50,10 @@
>  / {
>  	interrupt-parent = <&gic>;
>  
> +	aliases {
> +		ethernet0 = &emac;
> +	};
> +

This needs to be done at the board level.

>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> @@ -446,6 +450,21 @@
>  			status = "disabled";
>  		};
>  
> +		emac: ethernet@1c30000 {
> +			compatible = "allwinner,sun8i-h3-emac";
> +			syscon = <&syscon>;
> +			reg = <0x01c30000 0x104>;
> +			reg-names = "emac";

You don't need reg-names anymore.

> +			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> +			resets = <&ccu RST_BUS_EMAC>, <&ccu RST_BUS_EPHY>;
> +			reset-names = "ahb", "ephy";
> +			clocks = <&ccu CLK_BUS_EMAC>, <&ccu CLK_BUS_EPHY>;
> +			clock-names = "ahb", "ephy";

I still believe that having the same node for both the PHY and the MAC
is wrong.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1483110 — Re: [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver

FromLABBE Corentin <clabbe.montjoie@gmail.com>
Date2016-09-14 11:00 +0200
SubjectRe: [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver
Message-ID<shecV-76W-5@gated-at.bofh.it>
In reply to#1480968
On Mon, Sep 12, 2016 at 09:29:33AM +0200, Maxime Ripard wrote:
> On Fri, Sep 09, 2016 at 02:45:13PM +0200, Corentin Labbe wrote:
> > The sun8i-emac is an ethernet MAC hardware that support 10/100/1000
> > speed.
> > 
> > This patch enable the sun8i-emac on the Allwinner H3 SoC Device-tree.
> > The SoC H3 have an internal PHY, so optionals syscon and ephy are set.
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  arch/arm/boot/dts/sun8i-h3.dtsi | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > index a39da6f..a3ac476 100644
> > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > @@ -50,6 +50,10 @@
> >  / {
> >  	interrupt-parent = <&gic>;
> >  
> > +	aliases {
> > +		ethernet0 = &emac;
> > +	};
> > +
> 
> This needs to be done at the board level.
> 

ok

> >  	cpus {
> >  		#address-cells = <1>;
> >  		#size-cells = <0>;
> > @@ -446,6 +450,21 @@
> >  			status = "disabled";
> >  		};
> >  
> > +		emac: ethernet@1c30000 {
> > +			compatible = "allwinner,sun8i-h3-emac";
> > +			syscon = <&syscon>;
> > +			reg = <0x01c30000 0x104>;
> > +			reg-names = "emac";
> 
> You don't need reg-names anymore.
> 

ok

> > +			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> > +			resets = <&ccu RST_BUS_EMAC>, <&ccu RST_BUS_EPHY>;
> > +			reset-names = "ahb", "ephy";
> > +			clocks = <&ccu CLK_BUS_EMAC>, <&ccu CLK_BUS_EPHY>;
> > +			clock-names = "ahb", "ephy";
> 
> I still believe that having the same node for both the PHY and the MAC
> is wrong.
> 

Ok I have moved clock/reset of ephy in its node.

Thanks

Regards

Corentin Labbe

[toc] | [prev] | [next] | [standalone]


#1479958 — [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module
Message-ID<sftpM-2S8-23@gated-at.bofh.it>
In reply to#1479955
This patch add the dt node for the syscon register present on the
Allwinner H3.

Only two register are present in this syscon and the only one useful is
the one dedicated to EMAC clock.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index fdf9fdb..a39da6f 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -140,6 +140,11 @@
 		#size-cells = <1>;
 		ranges;
 
+		syscon: syscon@01c00000 {
+			compatible = "syscon";
+			reg = <0x01c00000 0x34>;
+		};
+
 		dma: dma-controller@01c02000 {
 			compatible = "allwinner,sun8i-h3-dma";
 			reg = <0x01c02000 0x1000>;
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1480966 — Re: [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-09-12 09:30 +0200
SubjectRe: [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module
Message-ID<sgtQJ-ct-3@gated-at.bofh.it>
In reply to#1479958

[Multipart message — attachments visible in raw view] — view raw

On Fri, Sep 09, 2016 at 02:45:12PM +0200, Corentin Labbe wrote:
> This patch add the dt node for the syscon register present on the
> Allwinner H3.
> 
> Only two register are present in this syscon and the only one useful is
> the one dedicated to EMAC clock.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index fdf9fdb..a39da6f 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -140,6 +140,11 @@
>  		#size-cells = <1>;
>  		ranges;
>  
> +		syscon: syscon@01c00000 {
> +			compatible = "syscon";

Having our compatible would be nice here. syscon doesn't mean anything
by itself.

> +			reg = <0x01c00000 0x34>;

And the size of our system controller is 0x1000

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1482487 — Re: [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module

FromLABBE Corentin <clabbe.montjoie@gmail.com>
Date2016-09-13 15:40 +0200
SubjectRe: [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module
Message-ID<sgW6m-2yc-15@gated-at.bofh.it>
In reply to#1480966
On Mon, Sep 12, 2016 at 09:28:12AM +0200, Maxime Ripard wrote:
> On Fri, Sep 09, 2016 at 02:45:12PM +0200, Corentin Labbe wrote:
> > This patch add the dt node for the syscon register present on the
> > Allwinner H3.
> > 
> > Only two register are present in this syscon and the only one useful is
> > the one dedicated to EMAC clock.
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > index fdf9fdb..a39da6f 100644
> > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > @@ -140,6 +140,11 @@
> >  		#size-cells = <1>;
> >  		ranges;
> >  
> > +		syscon: syscon@01c00000 {
> > +			compatible = "syscon";
> 
> Having our compatible would be nice here. syscon doesn't mean anything
> by itself.
> 

Since no driver handle it, I follow what I saw in other DT.
At your choice, I can add a sun8i-syscon, but it will be unused.

> > +			reg = <0x01c00000 0x34>;
> 
> And the size of our system controller is 0x1000
> 

I put the real size used, but I can put what datasheet said.

Regards

Corentin Labbe

[toc] | [prev] | [next] | [standalone]


#1479961 — [PATCH v3 6/9] ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[PATCH v3 6/9] ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC
Message-ID<sftpM-2S8-29@gated-at.bofh.it>
In reply to#1479955
The sun8i-emac hardware is present on the Orange PI PC.
It uses the internal PHY.

This patch create the needed emac and phy nodes.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index daf50b9a6..24f8e97 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -165,3 +165,13 @@
 	/* USB VBUS is always on */
 	status = "okay";
 };
+
+&emac {
+	phy = <&phy1>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+	phy1: ethernet-phy@1 {
+		reg = <1>;
+	};
+};
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1479963 — [PATCH v3 2/9] MAINTAINERS: Add myself as maintainer of sun8i-emac

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[PATCH v3 2/9] MAINTAINERS: Add myself as maintainer of sun8i-emac
Message-ID<sftpM-2S8-35@gated-at.bofh.it>
In reply to#1479955
This patch add myself as maintainer of the sun8i-emac driver.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6781a3f..43f5be3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -582,6 +582,12 @@ S:	Maintained
 F:	Documentation/i2c/busses/i2c-ali1563
 F:	drivers/i2c/busses/i2c-ali1563.c
 
+ALLWINNER SUN8I-EMAC ETHERNET DRIVER
+M:	Corentin Labbe <clabbe.montjoie@gmail.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/net/ethernet/allwinner/sun8i-emac.c
+
 ALLWINNER SECURITY SYSTEM
 M:	Corentin Labbe <clabbe.montjoie@gmail.com>
 L:	linux-crypto@vger.kernel.org
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1479964 — [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support
Message-ID<sftpN-2S8-43@gated-at.bofh.it>
In reply to#1479955
This patch add pm_runtime support to sun8i-emac.
For the moment, only basic support is added, (the device is marked as
used when net/open)

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/ethernet/allwinner/sun8i-emac.c | 62 ++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c b/drivers/net/ethernet/allwinner/sun8i-emac.c
index 1c4bc80..cce886e 100644
--- a/drivers/net/ethernet/allwinner/sun8i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
@@ -9,7 +9,6 @@
  * - MAC filtering
  * - Jumbo frame
  * - features rx-all (NETIF_F_RXALL_BIT)
- * - PM runtime
  */
 #include <linux/bitops.h>
 #include <linux/clk.h>
@@ -27,6 +26,7 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/scatterlist.h>
 #include <linux/skbuff.h>
@@ -1301,11 +1301,18 @@ static int sun8i_emac_open(struct net_device *ndev)
 	int err;
 	u32 v;
 
+	err = pm_runtime_get_sync(priv->dev);
+	if (err) {
+		pm_runtime_put_noidle(priv->dev);
+		dev_err(priv->dev, "pm_runtime error: %d\n", err);
+		return err;
+	}
+
 	err = request_irq(priv->irq, sun8i_emac_dma_interrupt, 0,
 			  dev_name(priv->dev), ndev);
 	if (err) {
 		dev_err(priv->dev, "Cannot request IRQ: %d\n", err);
-		return err;
+		goto err_runtime;
 	}
 
 	/* Set interface mode (and configure internal PHY on H3) */
@@ -1395,6 +1402,8 @@ err_syscon:
 	sun8i_emac_unset_syscon(ndev);
 err_irq:
 	free_irq(priv->irq, ndev);
+err_runtime:
+	pm_runtime_put(priv->dev);
 	return err;
 }
 
@@ -1483,6 +1492,8 @@ static int sun8i_emac_stop(struct net_device *ndev)
 	dma_free_coherent(priv->dev, priv->nbdesc_tx * sizeof(struct dma_desc),
 			  priv->dd_tx, priv->dd_tx_phy);
 
+	pm_runtime_put(priv->dev);
+
 	return 0;
 }
 
@@ -2210,6 +2221,8 @@ static int sun8i_emac_probe(struct platform_device *pdev)
 		goto probe_err;
 	}
 
+	pm_runtime_enable(priv->dev);
+
 	return 0;
 
 probe_err:
@@ -2221,6 +2234,8 @@ static int sun8i_emac_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 
+	pm_runtime_disable(&pdev->dev);
+
 	unregister_netdev(ndev);
 	platform_set_drvdata(pdev, NULL);
 	free_netdev(ndev);
@@ -2228,6 +2243,47 @@ static int sun8i_emac_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused sun8i_emac_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct sun8i_emac_priv *priv = netdev_priv(ndev);
+
+	napi_disable(&priv->napi);
+
+	if (netif_running(ndev))
+		netif_device_detach(ndev);
+
+	sun8i_emac_stop_tx(ndev);
+	sun8i_emac_stop_rx(ndev);
+
+	sun8i_emac_rx_clean(ndev);
+	sun8i_emac_tx_clean(ndev);
+
+	phy_stop(ndev->phydev);
+
+	return 0;
+}
+
+static int __maybe_unused sun8i_emac_resume(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct sun8i_emac_priv *priv = netdev_priv(ndev);
+
+	phy_start(ndev->phydev);
+
+	sun8i_emac_start_tx(ndev);
+	sun8i_emac_start_rx(ndev);
+
+	if (netif_running(ndev))
+		netif_device_attach(ndev);
+
+	netif_start_queue(ndev);
+
+	napi_enable(&priv->napi);
+
+	return 0;
+}
+
 static const struct of_device_id sun8i_emac_of_match_table[] = {
 	{ .compatible = "allwinner,sun8i-a83t-emac",
 	  .data = &emac_variant_a83t },
@@ -2246,6 +2302,8 @@ static struct platform_driver sun8i_emac_driver = {
 		.name           = "sun8i-emac",
 		.of_match_table	= sun8i_emac_of_match_table,
 	},
+	.suspend	= sun8i_emac_suspend,
+	.resume		= sun8i_emac_resume,
 };
 
 module_platform_driver(sun8i_emac_driver);
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1481952 — Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-09-12 22:50 +0200
SubjectRe: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support
Message-ID<sgGkV-8vM-9@gated-at.bofh.it>
In reply to#1479964

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Fri, Sep 09, 2016 at 02:45:17PM +0200, Corentin Labbe wrote:
> This patch add pm_runtime support to sun8i-emac.
> For the moment, only basic support is added, (the device is marked as
> used when net/open)
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  drivers/net/ethernet/allwinner/sun8i-emac.c | 62 ++++++++++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c b/drivers/net/ethernet/allwinner/sun8i-emac.c
> index 1c4bc80..cce886e 100644
> --- a/drivers/net/ethernet/allwinner/sun8i-emac.c
> +++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
> @@ -9,7 +9,6 @@
>   * - MAC filtering
>   * - Jumbo frame
>   * - features rx-all (NETIF_F_RXALL_BIT)
> - * - PM runtime
>   */
>  #include <linux/bitops.h>
>  #include <linux/clk.h>
> @@ -27,6 +26,7 @@
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/pinctrl/pinctrl.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  #include <linux/scatterlist.h>
>  #include <linux/skbuff.h>
> @@ -1301,11 +1301,18 @@ static int sun8i_emac_open(struct net_device *ndev)
>  	int err;
>  	u32 v;
>  
> +	err = pm_runtime_get_sync(priv->dev);
> +	if (err) {
> +		pm_runtime_put_noidle(priv->dev);
> +		dev_err(priv->dev, "pm_runtime error: %d\n", err);
> +		return err;
> +	}
> +
>  	err = request_irq(priv->irq, sun8i_emac_dma_interrupt, 0,
>  			  dev_name(priv->dev), ndev);
>  	if (err) {
>  		dev_err(priv->dev, "Cannot request IRQ: %d\n", err);
> -		return err;
> +		goto err_runtime;
>  	}
>  
>  	/* Set interface mode (and configure internal PHY on H3) */
> @@ -1395,6 +1402,8 @@ err_syscon:
>  	sun8i_emac_unset_syscon(ndev);
>  err_irq:
>  	free_irq(priv->irq, ndev);
> +err_runtime:
> +	pm_runtime_put(priv->dev);
>  	return err;
>  }
>  
> @@ -1483,6 +1492,8 @@ static int sun8i_emac_stop(struct net_device *ndev)
>  	dma_free_coherent(priv->dev, priv->nbdesc_tx * sizeof(struct dma_desc),
>  			  priv->dd_tx, priv->dd_tx_phy);
>  
> +	pm_runtime_put(priv->dev);
> +
>  	return 0;
>  }
>  
> @@ -2210,6 +2221,8 @@ static int sun8i_emac_probe(struct platform_device *pdev)
>  		goto probe_err;
>  	}
>  
> +	pm_runtime_enable(priv->dev);
> +
>  	return 0;
>  
>  probe_err:
> @@ -2221,6 +2234,8 @@ static int sun8i_emac_remove(struct platform_device *pdev)
>  {
>  	struct net_device *ndev = platform_get_drvdata(pdev);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	unregister_netdev(ndev);
>  	platform_set_drvdata(pdev, NULL);
>  	free_netdev(ndev);
> @@ -2228,6 +2243,47 @@ static int sun8i_emac_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static int __maybe_unused sun8i_emac_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
> +
> +	napi_disable(&priv->napi);
> +
> +	if (netif_running(ndev))
> +		netif_device_detach(ndev);
> +
> +	sun8i_emac_stop_tx(ndev);
> +	sun8i_emac_stop_rx(ndev);
> +
> +	sun8i_emac_rx_clean(ndev);
> +	sun8i_emac_tx_clean(ndev);
> +
> +	phy_stop(ndev->phydev);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused sun8i_emac_resume(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
> +
> +	phy_start(ndev->phydev);
> +
> +	sun8i_emac_start_tx(ndev);
> +	sun8i_emac_start_rx(ndev);
> +
> +	if (netif_running(ndev))
> +		netif_device_attach(ndev);
> +
> +	netif_start_queue(ndev);
> +
> +	napi_enable(&priv->napi);
> +
> +	return 0;
> +}

The main idea behind the runtime PM hooks is that they bring the
device to a working state and shuts it down when it's not needed
anymore.

However, they shouldn't be called when the device is still in used, so
all the mangling with NAPI, the phy and so on is irrelevant here, but
the clocks, resets, for example, are.

>  static const struct of_device_id sun8i_emac_of_match_table[] = {
>  	{ .compatible = "allwinner,sun8i-a83t-emac",
>  	  .data = &emac_variant_a83t },
> @@ -2246,6 +2302,8 @@ static struct platform_driver sun8i_emac_driver = {
>  		.name           = "sun8i-emac",
>  		.of_match_table	= sun8i_emac_of_match_table,
>  	},
> +	.suspend	= sun8i_emac_suspend,
> +	.resume		= sun8i_emac_resume,

These are not the runtime PM hooks. How did you test that?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1483344 — Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support

FromLABBE Corentin <clabbe.montjoie@gmail.com>
Date2016-09-14 16:10 +0200
SubjectRe: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support
Message-ID<shj2X-1R7-41@gated-at.bofh.it>
In reply to#1481952
On Mon, Sep 12, 2016 at 10:44:51PM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Sep 09, 2016 at 02:45:17PM +0200, Corentin Labbe wrote:
> > This patch add pm_runtime support to sun8i-emac.
> > For the moment, only basic support is added, (the device is marked as
> > used when net/open)
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  drivers/net/ethernet/allwinner/sun8i-emac.c | 62 ++++++++++++++++++++++++++++-
> >  1 file changed, 60 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > index 1c4bc80..cce886e 100644
> > --- a/drivers/net/ethernet/allwinner/sun8i-emac.c
> > +++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > @@ -9,7 +9,6 @@
> >   * - MAC filtering
> >   * - Jumbo frame
> >   * - features rx-all (NETIF_F_RXALL_BIT)
> > - * - PM runtime
> >   */
> >  #include <linux/bitops.h>
> >  #include <linux/clk.h>
> > @@ -27,6 +26,7 @@
> >  #include <linux/pinctrl/consumer.h>
> >  #include <linux/pinctrl/pinctrl.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/reset.h>
> >  #include <linux/scatterlist.h>
> >  #include <linux/skbuff.h>
> > @@ -1301,11 +1301,18 @@ static int sun8i_emac_open(struct net_device *ndev)
> >  	int err;
> >  	u32 v;
> >  
> > +	err = pm_runtime_get_sync(priv->dev);
> > +	if (err) {
> > +		pm_runtime_put_noidle(priv->dev);
> > +		dev_err(priv->dev, "pm_runtime error: %d\n", err);
> > +		return err;
> > +	}
> > +
> >  	err = request_irq(priv->irq, sun8i_emac_dma_interrupt, 0,
> >  			  dev_name(priv->dev), ndev);
> >  	if (err) {
> >  		dev_err(priv->dev, "Cannot request IRQ: %d\n", err);
> > -		return err;
> > +		goto err_runtime;
> >  	}
> >  
> >  	/* Set interface mode (and configure internal PHY on H3) */
> > @@ -1395,6 +1402,8 @@ err_syscon:
> >  	sun8i_emac_unset_syscon(ndev);
> >  err_irq:
> >  	free_irq(priv->irq, ndev);
> > +err_runtime:
> > +	pm_runtime_put(priv->dev);
> >  	return err;
> >  }
> >  
> > @@ -1483,6 +1492,8 @@ static int sun8i_emac_stop(struct net_device *ndev)
> >  	dma_free_coherent(priv->dev, priv->nbdesc_tx * sizeof(struct dma_desc),
> >  			  priv->dd_tx, priv->dd_tx_phy);
> >  
> > +	pm_runtime_put(priv->dev);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -2210,6 +2221,8 @@ static int sun8i_emac_probe(struct platform_device *pdev)
> >  		goto probe_err;
> >  	}
> >  
> > +	pm_runtime_enable(priv->dev);
> > +
> >  	return 0;
> >  
> >  probe_err:
> > @@ -2221,6 +2234,8 @@ static int sun8i_emac_remove(struct platform_device *pdev)
> >  {
> >  	struct net_device *ndev = platform_get_drvdata(pdev);
> >  
> > +	pm_runtime_disable(&pdev->dev);
> > +
> >  	unregister_netdev(ndev);
> >  	platform_set_drvdata(pdev, NULL);
> >  	free_netdev(ndev);
> > @@ -2228,6 +2243,47 @@ static int sun8i_emac_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > +static int __maybe_unused sun8i_emac_suspend(struct platform_device *pdev, pm_message_t state)
> > +{
> > +	struct net_device *ndev = platform_get_drvdata(pdev);
> > +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +
> > +	napi_disable(&priv->napi);
> > +
> > +	if (netif_running(ndev))
> > +		netif_device_detach(ndev);
> > +
> > +	sun8i_emac_stop_tx(ndev);
> > +	sun8i_emac_stop_rx(ndev);
> > +
> > +	sun8i_emac_rx_clean(ndev);
> > +	sun8i_emac_tx_clean(ndev);
> > +
> > +	phy_stop(ndev->phydev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused sun8i_emac_resume(struct platform_device *pdev)
> > +{
> > +	struct net_device *ndev = platform_get_drvdata(pdev);
> > +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +
> > +	phy_start(ndev->phydev);
> > +
> > +	sun8i_emac_start_tx(ndev);
> > +	sun8i_emac_start_rx(ndev);
> > +
> > +	if (netif_running(ndev))
> > +		netif_device_attach(ndev);
> > +
> > +	netif_start_queue(ndev);
> > +
> > +	napi_enable(&priv->napi);
> > +
> > +	return 0;
> > +}
> 
> The main idea behind the runtime PM hooks is that they bring the
> device to a working state and shuts it down when it's not needed
> anymore.
> 

I expect that the first part (all pm_runtime_xxx) of the patch bring that.
When the interface is not opened:
cat /sys/devices/platform/soc/1c30000.ethernet/power/runtime_status 
suspended

> However, they shouldn't be called when the device is still in used, so
> all the mangling with NAPI, the phy and so on is irrelevant here, but
> the clocks, resets, for example, are.
> 

I do the same as other ethernet driver for suspend/resume.

> >  static const struct of_device_id sun8i_emac_of_match_table[] = {
> >  	{ .compatible = "allwinner,sun8i-a83t-emac",
> >  	  .data = &emac_variant_a83t },
> > @@ -2246,6 +2302,8 @@ static struct platform_driver sun8i_emac_driver = {
> >  		.name           = "sun8i-emac",
> >  		.of_match_table	= sun8i_emac_of_match_table,
> >  	},
> > +	.suspend	= sun8i_emac_suspend,
> > +	.resume		= sun8i_emac_resume,
> 
> These are not the runtime PM hooks. How did you test that?
> 

Anyway I didnt test suspend/resume so I will remove it until I successfully found how to hibernate my board.

Thanks

Regards

Corentin Labbe

[toc] | [prev] | [next] | [standalone]


#1485533 — Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-09-17 15:50 +0200
SubjectRe: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support
Message-ID<sioae-3aQ-17@gated-at.bofh.it>
In reply to#1483344

[Multipart message — attachments visible in raw view] — view raw

On Wed, Sep 14, 2016 at 04:03:04PM +0200, LABBE Corentin wrote:
> > > +static int __maybe_unused sun8i_emac_suspend(struct platform_device *pdev, pm_message_t state)
> > > +{
> > > +	struct net_device *ndev = platform_get_drvdata(pdev);
> > > +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > > +
> > > +	napi_disable(&priv->napi);
> > > +
> > > +	if (netif_running(ndev))
> > > +		netif_device_detach(ndev);
> > > +
> > > +	sun8i_emac_stop_tx(ndev);
> > > +	sun8i_emac_stop_rx(ndev);
> > > +
> > > +	sun8i_emac_rx_clean(ndev);
> > > +	sun8i_emac_tx_clean(ndev);
> > > +
> > > +	phy_stop(ndev->phydev);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int __maybe_unused sun8i_emac_resume(struct platform_device *pdev)
> > > +{
> > > +	struct net_device *ndev = platform_get_drvdata(pdev);
> > > +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > > +
> > > +	phy_start(ndev->phydev);
> > > +
> > > +	sun8i_emac_start_tx(ndev);
> > > +	sun8i_emac_start_rx(ndev);
> > > +
> > > +	if (netif_running(ndev))
> > > +		netif_device_attach(ndev);
> > > +
> > > +	netif_start_queue(ndev);
> > > +
> > > +	napi_enable(&priv->napi);
> > > +
> > > +	return 0;
> > > +}
> > 
> > The main idea behind the runtime PM hooks is that they bring the
> > device to a working state and shuts it down when it's not needed
> > anymore.

Indeed.

> I expect that the first part (all pm_runtime_xxx) of the patch bring that.
> When the interface is not opened:
> cat /sys/devices/platform/soc/1c30000.ethernet/power/runtime_status 
> suspended
> 
> > However, they shouldn't be called when the device is still in used, so
> > all the mangling with NAPI, the phy and so on is irrelevant here, but
> > the clocks, resets, for example, are.
> > 
> 
> I do the same as other ethernet driver for suspend/resume.

suspend / resume are used when you put the whole system into suspend,
and bring it back.

runtime_pm is only when the device is not used anymore. It makes sense
when you suspend to do whatever you're doing here. It doesn't make any
when the system is not suspended, but the device is.

> > >  static const struct of_device_id sun8i_emac_of_match_table[] = {
> > >  	{ .compatible = "allwinner,sun8i-a83t-emac",
> > >  	  .data = &emac_variant_a83t },
> > > @@ -2246,6 +2302,8 @@ static struct platform_driver sun8i_emac_driver = {
> > >  		.name           = "sun8i-emac",
> > >  		.of_match_table	= sun8i_emac_of_match_table,
> > >  	},
> > > +	.suspend	= sun8i_emac_suspend,
> > > +	.resume		= sun8i_emac_resume,
> > 
> > These are not the runtime PM hooks. How did you test that?
> > 
> 
> Anyway I didnt test suspend/resume so I will remove it until I
> successfully found how to hibernate my board.

So you submit code you never tested? That's usually a recipe for
disaster.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1485578 — Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support

FromFlorian Fainelli <f.fainelli@gmail.com>
Date2016-09-17 19:00 +0200
SubjectRe: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support
Message-ID<sir85-50A-19@gated-at.bofh.it>
In reply to#1483344
On 09/14/2016 07:03 AM, LABBE Corentin wrote:
> On Mon, Sep 12, 2016 at 10:44:51PM +0200, Maxime Ripard wrote:
>>> +static int __maybe_unused sun8i_emac_resume(struct platform_device *pdev)
>>> +{
>>> +	struct net_device *ndev = platform_get_drvdata(pdev);
>>> +	struct sun8i_emac_priv *priv = netdev_priv(ndev);
>>> +
>>> +	phy_start(ndev->phydev);
>>> +
>>> +	sun8i_emac_start_tx(ndev);
>>> +	sun8i_emac_start_rx(ndev);
>>> +
>>> +	if (netif_running(ndev))
>>> +		netif_device_attach(ndev);
>>> +
>>> +	netif_start_queue(ndev);
>>> +
>>> +	napi_enable(&priv->napi);
>>> +
>>> +	return 0;
>>> +}
>>
>> The main idea behind the runtime PM hooks is that they bring the
>> device to a working state and shuts it down when it's not needed
>> anymore.
>>
> 
> I expect that the first part (all pm_runtime_xxx) of the patch bring that.
> When the interface is not opened:
> cat /sys/devices/platform/soc/1c30000.ethernet/power/runtime_status 
> suspended

If your interface is not open, it should be in a low power state, only
when it gets open (which means it is used) should you make it
functional, that's pretty much the same thing as the runtime PM
reference count usage here.

I don't see a lot of value for using runtime_pm_* hooks here except
calling into the existing suspend/resume functions that you have defined
already, but then again, the code should be modular enough already in
the driver.

Runtime PM for network devices cannot be used as efficiently as you
would with any kind of host-initiated bus/controller because your device
needs to be able to receive packets without the host's ability to wake
up the device to receive packets, so, with the exception of MDIO (which
is host initiated), everything else besides except packet transmission
(then again, I would not want to wait N ms to bring the interface in a
state where it can now transmit packets, that's terrible for latency) is
pretty much impossible to fully suspend due to its asynchronous nature.
-- 
Florian

[toc] | [prev] | [next] | [standalone]


#1479968 — [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig
Message-ID<sftpN-2S8-47@gated-at.bofh.it>
In reply to#1479955
Enable the sun8i-emac driver in the sunxi default configuration

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/configs/sunxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 714da33..153707a 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -40,6 +40,7 @@ CONFIG_ATA=y
 CONFIG_AHCI_SUNXI=y
 CONFIG_NETDEVICES=y
 CONFIG_SUN4I_EMAC=y
+CONFIG_SUN8I_EMAC=y
 # CONFIG_NET_VENDOR_ARC is not set
 # CONFIG_NET_CADENCE is not set
 # CONFIG_NET_VENDOR_BROADCOM is not set
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1480979 — Re: [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-09-12 09:40 +0200
SubjectRe: [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig
Message-ID<sgu0p-fL-23@gated-at.bofh.it>
In reply to#1479968

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Fri, Sep 09, 2016 at 02:45:16PM +0200, Corentin Labbe wrote:
> Enable the sun8i-emac driver in the sunxi default configuration
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Could you make the same patch for multi_v7 ?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1482507 — Re: [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig

FromLABBE Corentin <clabbe.montjoie@gmail.com>
Date2016-09-13 15:50 +0200
SubjectRe: [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig
Message-ID<sgWg2-2C0-27@gated-at.bofh.it>
In reply to#1480979
On Mon, Sep 12, 2016 at 09:30:08AM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Sep 09, 2016 at 02:45:16PM +0200, Corentin Labbe wrote:
> > Enable the sun8i-emac driver in the sunxi default configuration
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> 
> Could you make the same patch for multi_v7 ?
> 

I will

Thanks

[toc] | [prev] | [next] | [standalone]


#1479969 — [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-09-09 14:50 +0200
Subject[PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
Message-ID<sftpN-2S8-51@gated-at.bofh.it>
In reply to#1479955
This patch adds documentation for Device-Tree bindings for the
Allwinner sun8i-emac driver.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 .../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 0000000..4968ee3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
@@ -0,0 +1,64 @@
+* Allwinner sun8i EMAC ethernet controller
+
+Required properties:
+- compatible: should be one of the following string:
+		"allwinner,sun8i-a83t-emac"
+		"allwinner,sun8i-h3-emac"
+		"allwinner,sun50i-a64-emac"
+- reg: address and length of the register for the device.
+- reg-names: should be "emac"
+- syscon: A phandle to the syscon of the SoC
+- interrupts: interrupt for the device
+- clocks: A phandle to the reference clock for this device
+- clock-names: should be "ahb"
+- resets: A phandle to the reset control for this device
+- reset-names: should be "ahb"
+- phy-mode: See ethernet.txt
+- phy or phy-handle: See ethernet.txt
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+
+"allwinner,sun8i-h3-emac" also requires:
+- clocks: an extra phandle to the reference clock for the EPHY
+- clock-names: an extra "ephy" entry matching the clocks property
+- resets: an extra phandle to the reset control for the EPHY
+- resets-names: an extra "ephy" entry matching the resets property
+
+See ethernet.txt in the same directory for generic bindings for ethernet
+controllers.
+
+The device node referenced by "phy" or "phy-handle" should be a child node
+of this node. See phy.txt for the generic PHY bindings.
+
+Optional properties:
+- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. Default is 0)
+- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. Default is 0)
+
+The TX/RX clock delay chain settings are board specific.
+
+Optional properties for "allwinner,sun8i-h3-emac":
+- allwinner,leds-active-low: EPHY LEDs are active low
+
+Example:
+
+emac: ethernet@01c0b000 {
+	compatible = "allwinner,sun8i-h3-emac";
+	syscon = <&syscon>;
+	reg = <0x01c0b000 0x104>;
+	reg-names = "emac";
+	interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+	resets = <&ccu RST_BUS_EMAC>, <<&ccu RST_BUS_EPHY>;
+	reset-names = "ahb", "ephy";
+	clocks = <&ccu CLK_BUS_EMAC>, <&ccu CLK_BUS_EPHY>;
+	clock-names = "ahb", "ephy";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	phy = <&phy1>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+
+	phy1: ethernet-phy@1 {
+		reg = <1>;
+	};
+};
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1480030 — Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

FromAndrew Lunn <andrew@lunn.ch>
Date2016-09-09 16:10 +0200
SubjectRe: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
Message-ID<sfuFd-3MV-43@gated-at.bofh.it>
In reply to#1479969
> +The device node referenced by "phy" or "phy-handle" should be a child node
> +of this node. See phy.txt for the generic PHY bindings.

I've not looked at the code yet, but is this really true? Generally
there is not this limitation. You can point to any Ethernet phy
anyway, so long as it is on am MDIO bus.

> +
> +Optional properties:
> +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. Default is 0)
> +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. Default is 0)
> +
> +The TX/RX clock delay chain settings are board specific.
> +
> +Optional properties for "allwinner,sun8i-h3-emac":
> +- allwinner,leds-active-low: EPHY LEDs are active low
> +
> +Example:
> +
> +emac: ethernet@01c0b000 {
> +	compatible = "allwinner,sun8i-h3-emac";
> +	syscon = <&syscon>;
> +	reg = <0x01c0b000 0x104>;
> +	reg-names = "emac";
> +	interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> +	resets = <&ccu RST_BUS_EMAC>, <<&ccu RST_BUS_EPHY>;
> +	reset-names = "ahb", "ephy";
> +	clocks = <&ccu CLK_BUS_EMAC>, <&ccu CLK_BUS_EPHY>;
> +	clock-names = "ahb", "ephy";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	phy = <&phy1>;

ethernet.txt say:

- phy: the same as "phy-handle" property, not recommended for new bindings.

This is a new binding, please don't support it.

> +	phy-mode = "mii";
> +	allwinner,leds-active-low;
> +
> +	phy1: ethernet-phy@1 {
> +		reg = <1>;
> +	};

It is normal to place these phy nodes inside an container node called
mdio.

	Andrew

[toc] | [prev] | [next] | [standalone]


#1481407 — Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

FromLABBE Corentin <clabbe.montjoie@gmail.com>
Date2016-09-12 17:10 +0200
SubjectRe: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
Message-ID<sgB1U-4WE-25@gated-at.bofh.it>
In reply to#1480030
On Fri, Sep 09, 2016 at 04:04:13PM +0200, Andrew Lunn wrote:
> > +The device node referenced by "phy" or "phy-handle" should be a child node
> > +of this node. See phy.txt for the generic PHY bindings.
> 
> I've not looked at the code yet, but is this really true? Generally
> there is not this limitation. You can point to any Ethernet phy
> anyway, so long as it is on am MDIO bus.
> 
> > +
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. Default is 0)
> > +
> > +The TX/RX clock delay chain settings are board specific.
> > +
> > +Optional properties for "allwinner,sun8i-h3-emac":
> > +- allwinner,leds-active-low: EPHY LEDs are active low
> > +
> > +Example:
> > +
> > +emac: ethernet@01c0b000 {
> > +	compatible = "allwinner,sun8i-h3-emac";
> > +	syscon = <&syscon>;
> > +	reg = <0x01c0b000 0x104>;
> > +	reg-names = "emac";
> > +	interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> > +	resets = <&ccu RST_BUS_EMAC>, <<&ccu RST_BUS_EPHY>;
> > +	reset-names = "ahb", "ephy";
> > +	clocks = <&ccu CLK_BUS_EMAC>, <&ccu CLK_BUS_EPHY>;
> > +	clock-names = "ahb", "ephy";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	phy = <&phy1>;
> 
> ethernet.txt say:
> 
> - phy: the same as "phy-handle" property, not recommended for new bindings.
> 
> This is a new binding, please don't support it.
> 
> > +	phy-mode = "mii";
> > +	allwinner,leds-active-low;
> > +
> > +	phy1: ethernet-phy@1 {
> > +		reg = <1>;
> > +	};
> 
> It is normal to place these phy nodes inside an container node called
> mdio.
> 

Hello

Since the MDIO bus is a part of the sun8i-emac, does I really need to create such a mdio node ?
All example I found are mdio bus with separate driver. (others driver have the phy directly in [eg]mac node.

Anyway I try the following patch to solve your comments, but it breaks the PHY finding(Could not attach to PHY).

Regards

-->8--
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -166,14 +166,18 @@
        status = "okay";
 };
 
+&mdio {
+       reg = <1>;
+       phy1: ethernet-phy@1 {
+               reg = <1>;
+       };
+};
+
 &emac {
-       phy = <&phy1>;
+       phy-handle = <&phy1>;
        phy-mode = "mii";
        allwinner,leds-active-low;
        status = "okay";
-       phy1: ethernet-phy@1 {
-               reg = <1>;
-       };
 };/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -474,6 +474,11 @@
                        #address-cells = <1>;
                        #size-cells = <0>;
                        status = "disabled";
+
+                       mdio: mdio@0 {
+                               #address-cells = <1>;
+                               #size-cells = <0>;
+                       };
                };
 
                crypto: crypto@1c15000 {
--- a/drivers/net/ethernet/allwinner/sun8i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
@@ -2122,7 +2122,7 @@ static int sun8i_emac_probe(struct platform_device *pdev)
                return -EINVAL;
        }
 
-       priv->phy_node = of_parse_phandle(node, "phy", 0);
+       priv->phy_node = of_parse_phandle(node, "phy-handle", 0);
        if (!priv->phy_node) {
                netdev_err(ndev, "No associated PHY\n");
                return -ENODEV;

 
 &crypto {

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web