Re: [PATCH] net: ethernet: ave: fix ave_start_xmit()'s return type

[Kunihiko Hayashi]

Hi,

On Tue, 24 Apr 2018 15:17:25 +0200
Luc Van Oostenryck <luc.vanoostenr...@gmail.com> wrote:

> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, but the implementation in this
> driver returns an 'int'.
> 
> Fix this by returning 'netdev_tx_t' in this driver too.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenr...@gmail.com>

Thanks for fixing.
I think it's preferable to add 'Fixes'.

Fixes: 4c270b55a5af ("net: ethernet: socionext: add AVE ethernet driver")
Acked-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>

Thank you,

> ---
>  drivers/net/ethernet/socionext/sni_ave.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
> b/drivers/net/ethernet/socionext/sni_ave.c
> index 0b3b7a460..95625bca1 100644
> --- a/drivers/net/ethernet/socionext/sni_ave.c
> +++ b/drivers/net/ethernet/socionext/sni_ave.c
> @@ -1360,7 +1360,7 @@ static int ave_stop(struct net_device *ndev)
>   return 0;
>  }
>  
> -static int ave_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +static netdev_tx_t ave_start_xmit(struct sk_buff *skb, struct net_device 
> *ndev)
>  {
>   struct ave_private *priv = netdev_priv(ndev);
>   u32 proc_idx, done_idx, ndesc, cmdsts;
> -- 
> 2.17.0

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v2 2/3] dt-bindings: net: ave: add syscon-phy-mode property to configure phy-mode setting

[Kunihiko Hayashi]

Add "socionext,syscon-phy-mode" property to specify system controller that
configures the settings about phy-mode.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Reviewed-by: Rob Herring <r...@kernel.org>
---
 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
index 85e0c49..fc8f017 100644
--- a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -13,7 +13,8 @@ Required properties:
  - reg: Address where registers are mapped and size of region.
  - interrupts: Should contain the MAC interrupt.
  - phy-mode: See ethernet.txt in the same directory. Allow to choose
-   "rgmii", "rmii", or "mii" according to the PHY.
+   "rgmii", "rmii", "mii", or "internal" according to the PHY.
+   The acceptable mode is SoC-dependent.
  - phy-handle: Should point to the external phy device.
See ethernet.txt file in the same directory.
  - clocks: A phandle to the clock for the MAC.
@@ -27,6 +28,8 @@ Required properties:
  - reset-names: Should contain
- "ether", "gio" for Pro4 SoC
- "ether" for others
+ - socionext,syscon-phy-mode: A phandle to syscon with one argument
+   that configures phy mode. The argument is the ID of MAC instance.
 
 Optional properties:
  - local-mac-address: See ethernet.txt in the same directory.
@@ -47,6 +50,7 @@ Example:
clocks = <_clk 6>;
reset-names = "ether";
resets = <_rst 6>;
+   socionext,syscon-phy-mode = <_glue 0>;
local-mac-address = [00 00 00 00 00 00];
 
mdio {
-- 
2.7.4

[PATCH net-next v2 1/3] net: ethernet: ave: add multiple clocks and resets support as required property

[Kunihiko Hayashi]

When the link is becoming up for Pro4 SoC, the kernel is stalled
due to some missing clocks and resets.

The AVE block for Pro4 is connected to the GIO bus in the SoC.
Without its clock/reset, the access to the AVE register makes the
system stall.

In the same way, another MAC clock for Giga-bit Connection and
the PHY clock are also required for Pro4 to activate the Giga-bit feature
and to recognize the PHY.

To satisfy these requirements, this patch adds support for multiple clocks
and resets, and adds the clock-names and reset-names to the binding because
we need to distinguish clock/reset for the AVE main block and the others.

Also, make the resets a required property. Currently, "reset is
optional" relies on that the bootloader or firmware has deasserted
the reset before booting the kernel.  Drivers should work without
such expectation.

Fixes: 4c270b55a5af ("net: ethernet: socionext: add AVE ethernet driver")
Suggested-by: Masahiro Yamada <yamada.masah...@socionext.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Reviewed-by: Rob Herring <r...@kernel.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   |  13 ++-
 drivers/net/ethernet/socionext/sni_ave.c   | 108 -
 2 files changed, 96 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
index 96398cc..85e0c49 100644
--- a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -17,9 +17,18 @@ Required properties:
  - phy-handle: Should point to the external phy device.
See ethernet.txt file in the same directory.
  - clocks: A phandle to the clock for the MAC.
+   For Pro4 SoC, that is "socionext,uniphier-pro4-ave4",
+   another MAC clock, GIO bus clock and PHY clock are also required.
+ - clock-names: Should contain
+   - "ether", "ether-gb", "gio", "ether-phy" for Pro4 SoC
+   - "ether" for others
+ - resets: A phandle to the reset control for the MAC. For Pro4 SoC,
+   GIO bus reset is also required.
+ - reset-names: Should contain
+   - "ether", "gio" for Pro4 SoC
+   - "ether" for others
 
 Optional properties:
- - resets: A phandle to the reset control for the MAC.
  - local-mac-address: See ethernet.txt in the same directory.
 
 Required subnode:
@@ -34,7 +43,9 @@ Example:
interrupts = <0 66 4>;
phy-mode = "rgmii";
phy-handle = <>;
+   clock-names = "ether";
clocks = <_clk 6>;
+   reset-names = "ether";
resets = <_rst 6>;
local-mac-address = [00 00 00 00 00 00];
 
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
index 0b3b7a4..52940bd 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -199,6 +199,9 @@
 
 #define IS_DESC_64BIT(p)   ((p)->data->is_desc_64bit)
 
+#define AVE_MAX_CLKS   4
+#define AVE_MAX_RSTS   2
+
 enum desc_id {
AVE_DESCID_RX,
AVE_DESCID_TX,
@@ -227,6 +230,8 @@ struct ave_desc_info {
 
 struct ave_soc_data {
boolis_desc_64bit;
+   const char  *clock_names[AVE_MAX_CLKS];
+   const char  *reset_names[AVE_MAX_RSTS];
 };
 
 struct ave_stats {
@@ -245,8 +250,10 @@ struct ave_private {
int phy_id;
unsigned intdesc_size;
u32 msg_enable;
-   struct clk  *clk;
-   struct reset_control*rst;
+   int nclks;
+   struct clk  *clk[AVE_MAX_CLKS];
+   int nrsts;
+   struct reset_control*rst[AVE_MAX_RSTS];
phy_interface_t phy_mode;
struct phy_device   *phydev;
struct mii_bus  *mdio;
@@ -1153,18 +1160,23 @@ static int ave_init(struct net_device *ndev)
struct device_node *np = dev->of_node;
struct device_node *mdio_np;
struct phy_device *phydev;
-   int ret;
+   int nc, nr, ret;
 
/* enable clk because of hw access until ndo_open */
-   ret = clk_prepare_enable(priv->clk);
-   if (ret) {
-   dev_err(dev, "can't enable clock\n");
-   return ret;
+   for (nc = 0; nc < priv->nclks; nc++) {
+   ret = clk_prepare_enable(priv->clk[nc]);
+   if (ret) {
+   dev_err(dev, "can't enable clock\n");
+   goto out_clk_disable;
+   }
}
-   ret = reset_control_deassert(priv->

[PATCH net-next v2 0/3] ave: fix the activation issues for some UniPhier SoCs

[Kunihiko Hayashi]

This add the following stuffs to fix the activation issues and satisfy
requirements for AVE ethernet driver implemented on some UniPhier SoCs.

- Add support for additional necessary clocks and resets, because the kernel
  is stalled on Pro4 due to lack of them.

- Check whether the SoC supports the specified phy-mode

- Add DT property support indicating system controller that has the feature
  for configurating phy-mode including built-in phy on LD11.

v1: https://www.spinics.net/lists/netdev/msg494904.html

Changes since v1:
- Add 'Reviewed-by' lines

Kunihiko Hayashi (3):
  net: ethernet: ave: add multiple clocks and resets support as required
property
  dt-bindings: net: ave: add syscon-phy-mode property to configure
phy-mode setting
  net: ethernet: ave: add support for phy-mode setting of system
controller

 .../bindings/net/socionext,uniphier-ave4.txt   |  19 +-
 drivers/net/ethernet/socionext/Kconfig |   2 +
 drivers/net/ethernet/socionext/sni_ave.c   | 252 ++---
 3 files changed, 238 insertions(+), 35 deletions(-)

-- 
2.7.4

[PATCH net-next v2 3/3] net: ethernet: ave: add support for phy-mode setting of system controller

[Kunihiko Hayashi]

This patch adds support for specifying system controller that configures
phy-mode setting.

According to the DT property "phy-mode", it's necessary to configure the
controller, which is used to choose the settings of the MAC suitable,
for example, mdio pin connections, internal clocks, and so on.

Supported phy-modes are SoC-dependent. The driver allows phy-mode to set
"internal" if the SoC has a built-in PHY, and {"mii", "rmii", "rgmii"}
if the SoC supports each mode. So we have to check whether the phy-mode
is valid or not.

This adds the following features for each SoC:
- check whether the SoC supports the specified phy-mode
- configure the controller accroding to phy-mode

The DT property accepts one argument to distinguish them for multiple MAC
instances.

ethernet@6500 {
...
socionext,syscon-phy-mode = <_glue 0>;
};

ethernet@6520 {
...
socionext,syscon-phy-mode = <_glue 1>;
};

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---
 drivers/net/ethernet/socionext/Kconfig   |   2 +
 drivers/net/ethernet/socionext/sni_ave.c | 150 ---
 2 files changed, 140 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
index 6bcfe27..b80048c 100644
--- a/drivers/net/ethernet/socionext/Kconfig
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -14,6 +14,8 @@ if NET_VENDOR_SOCIONEXT
 config SNI_AVE
tristate "Socionext AVE ethernet support"
depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   depends on HAS_IOMEM
+   select MFD_SYSCON
select PHYLIB
---help---
  Driver for gigabit ethernet MACs, called AVE, in the
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
index 52940bd..f7eccee 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -18,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -197,6 +199,11 @@
 #define AVE_INTM_COUNT 20
 #define AVE_FORCE_TXINTCNT 1
 
+/* SG */
+#define SG_ETPINMODE   0x540
+#define SG_ETPINMODE_EXTPHYBIT(1)  /* for LD11 */
+#define SG_ETPINMODE_RMII(ins) BIT(ins)
+
 #define IS_DESC_64BIT(p)   ((p)->data->is_desc_64bit)
 
 #define AVE_MAX_CLKS   4
@@ -228,12 +235,6 @@ struct ave_desc_info {
struct ave_desc *desc;  /* skb info related descriptor */
 };
 
-struct ave_soc_data {
-   boolis_desc_64bit;
-   const char  *clock_names[AVE_MAX_CLKS];
-   const char  *reset_names[AVE_MAX_RSTS];
-};
-
 struct ave_stats {
struct  u64_stats_sync  syncp;
u64 packets;
@@ -257,6 +258,9 @@ struct ave_private {
phy_interface_t phy_mode;
struct phy_device   *phydev;
struct mii_bus  *mdio;
+   struct regmap   *regmap;
+   unsigned intpinmode_mask;
+   unsigned intpinmode_val;
 
/* stats */
struct ave_statsstats_rx;
@@ -279,6 +283,14 @@ struct ave_private {
const struct ave_soc_data *data;
 };
 
+struct ave_soc_data {
+   boolis_desc_64bit;
+   const char  *clock_names[AVE_MAX_CLKS];
+   const char  *reset_names[AVE_MAX_RSTS];
+   int (*get_pinmode)(struct ave_private *priv,
+  phy_interface_t phy_mode, u32 arg);
+};
+
 static u32 ave_desc_read(struct net_device *ndev, enum desc_id id, int entry,
 int offset)
 {
@@ -1179,6 +1191,11 @@ static int ave_init(struct net_device *ndev)
}
}
 
+   ret = regmap_update_bits(priv->regmap, SG_ETPINMODE,
+priv->pinmode_mask, priv->pinmode_val);
+   if (ret)
+   return ret;
+
ave_global_reset(ndev);
 
mdio_np = of_get_child_by_name(np, "mdio");
@@ -1537,6 +1554,7 @@ static int ave_probe(struct platform_device *pdev)
const struct ave_soc_data *data;
struct device *dev = >dev;
char buf[ETHTOOL_FWVERS_LEN];
+   struct of_phandle_args args;
phy_interface_t phy_mode;
struct ave_private *priv;
struct net_device *ndev;
@@ -1559,12 +1577,6 @@ static int ave_probe(struct platform_device *pdev)
dev_err(dev, "phy-mode not found\n");
return -EINVAL;
}
-   if ((!phy_interface_mode_is_rgmii(phy_mode)) &&
-   phy_mode != PHY_INTERFACE_MODE_RMII &&
-   phy_mode != PHY_INTERFACE_MODE_MII) {
-   dev_err(dev, "phy-mode is invalid\n");
- 

[PATCH net-next 2/3] dt-bindings: net: ave: add syscon-phy-mode property to configure phy-mode setting

[Kunihiko Hayashi]

Add "socionext,syscon-phy-mode" property to specify system controller that
configures the settings about phy-mode.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
---
 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
index 85e0c49..fc8f017 100644
--- a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -13,7 +13,8 @@ Required properties:
  - reg: Address where registers are mapped and size of region.
  - interrupts: Should contain the MAC interrupt.
  - phy-mode: See ethernet.txt in the same directory. Allow to choose
-   "rgmii", "rmii", or "mii" according to the PHY.
+   "rgmii", "rmii", "mii", or "internal" according to the PHY.
+   The acceptable mode is SoC-dependent.
  - phy-handle: Should point to the external phy device.
See ethernet.txt file in the same directory.
  - clocks: A phandle to the clock for the MAC.
@@ -27,6 +28,8 @@ Required properties:
  - reset-names: Should contain
- "ether", "gio" for Pro4 SoC
- "ether" for others
+ - socionext,syscon-phy-mode: A phandle to syscon with one argument
+   that configures phy mode. The argument is the ID of MAC instance.
 
 Optional properties:
  - local-mac-address: See ethernet.txt in the same directory.
@@ -47,6 +50,7 @@ Example:
clocks = <_clk 6>;
reset-names = "ether";
resets = <_rst 6>;
+   socionext,syscon-phy-mode = <_glue 0>;
local-mac-address = [00 00 00 00 00 00];
 
mdio {
-- 
2.7.4

[PATCH net-next 0/3] ave: fix the activation issues for some UniPhier SoCs

[Kunihiko Hayashi]

This add the following stuffs to fix the activation issues and satisfy
requirements for AVE ethernet driver implemented on some UniPhier SoCs.

- Add support for additional necessary clocks and resets, because the kernel
  is stalled on Pro4 due to lack of them.

- Check whether the SoC supports the specified phy-mode

- Add DT property support indicating system controller that has the feature
  for configurating phy-mode including built-in phy on LD11.

Kunihiko Hayashi (3):
  net: ethernet: ave: add multiple clocks and resets support as required
property
  dt-bindings: net: ave: add syscon-phy-mode property to configure
phy-mode setting
  net: ethernet: ave: add support for phy-mode setting of system
controller

 .../bindings/net/socionext,uniphier-ave4.txt   |  19 +-
 drivers/net/ethernet/socionext/Kconfig |   2 +
 drivers/net/ethernet/socionext/sni_ave.c   | 252 ++---
 3 files changed, 238 insertions(+), 35 deletions(-)

-- 
2.7.4

[PATCH net-next 3/3] net: ethernet: ave: add support for phy-mode setting of system controller

[Kunihiko Hayashi]

This patch adds support for specifying system controller that configures
phy-mode setting.

According to the DT property "phy-mode", it's necessary to configure the
controller, which is used to choose the settings of the MAC suitable,
for example, mdio pin connections, internal clocks, and so on.

Supported phy-modes are SoC-dependent. The driver allows phy-mode to set
"internal" if the SoC has a built-in PHY, and {"mii", "rmii", "rgmii"}
if the SoC supports each mode. So we have to check whether the phy-mode
is valid or not.

This adds the following features for each SoC:
- check whether the SoC supports the specified phy-mode
- configure the controller accroding to phy-mode

The DT property accepts one argument to distinguish them for multiple MAC
instances.

ethernet@6500 {
...
socionext,syscon-phy-mode = <_glue 0>;
};

ethernet@6520 {
...
socionext,syscon-phy-mode = <_glue 1>;
};

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---
 drivers/net/ethernet/socionext/Kconfig   |   2 +
 drivers/net/ethernet/socionext/sni_ave.c | 150 ---
 2 files changed, 140 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
index 6bcfe27..b80048c 100644
--- a/drivers/net/ethernet/socionext/Kconfig
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -14,6 +14,8 @@ if NET_VENDOR_SOCIONEXT
 config SNI_AVE
tristate "Socionext AVE ethernet support"
depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   depends on HAS_IOMEM
+   select MFD_SYSCON
select PHYLIB
---help---
  Driver for gigabit ethernet MACs, called AVE, in the
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
index 52940bd..f7eccee 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -18,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -197,6 +199,11 @@
 #define AVE_INTM_COUNT 20
 #define AVE_FORCE_TXINTCNT 1
 
+/* SG */
+#define SG_ETPINMODE   0x540
+#define SG_ETPINMODE_EXTPHYBIT(1)  /* for LD11 */
+#define SG_ETPINMODE_RMII(ins) BIT(ins)
+
 #define IS_DESC_64BIT(p)   ((p)->data->is_desc_64bit)
 
 #define AVE_MAX_CLKS   4
@@ -228,12 +235,6 @@ struct ave_desc_info {
struct ave_desc *desc;  /* skb info related descriptor */
 };
 
-struct ave_soc_data {
-   boolis_desc_64bit;
-   const char  *clock_names[AVE_MAX_CLKS];
-   const char  *reset_names[AVE_MAX_RSTS];
-};
-
 struct ave_stats {
struct  u64_stats_sync  syncp;
u64 packets;
@@ -257,6 +258,9 @@ struct ave_private {
phy_interface_t phy_mode;
struct phy_device   *phydev;
struct mii_bus  *mdio;
+   struct regmap   *regmap;
+   unsigned intpinmode_mask;
+   unsigned intpinmode_val;
 
/* stats */
struct ave_statsstats_rx;
@@ -279,6 +283,14 @@ struct ave_private {
const struct ave_soc_data *data;
 };
 
+struct ave_soc_data {
+   boolis_desc_64bit;
+   const char  *clock_names[AVE_MAX_CLKS];
+   const char  *reset_names[AVE_MAX_RSTS];
+   int (*get_pinmode)(struct ave_private *priv,
+  phy_interface_t phy_mode, u32 arg);
+};
+
 static u32 ave_desc_read(struct net_device *ndev, enum desc_id id, int entry,
 int offset)
 {
@@ -1179,6 +1191,11 @@ static int ave_init(struct net_device *ndev)
}
}
 
+   ret = regmap_update_bits(priv->regmap, SG_ETPINMODE,
+priv->pinmode_mask, priv->pinmode_val);
+   if (ret)
+   return ret;
+
ave_global_reset(ndev);
 
mdio_np = of_get_child_by_name(np, "mdio");
@@ -1537,6 +1554,7 @@ static int ave_probe(struct platform_device *pdev)
const struct ave_soc_data *data;
struct device *dev = >dev;
char buf[ETHTOOL_FWVERS_LEN];
+   struct of_phandle_args args;
phy_interface_t phy_mode;
struct ave_private *priv;
struct net_device *ndev;
@@ -1559,12 +1577,6 @@ static int ave_probe(struct platform_device *pdev)
dev_err(dev, "phy-mode not found\n");
return -EINVAL;
}
-   if ((!phy_interface_mode_is_rgmii(phy_mode)) &&
-   phy_mode != PHY_INTERFACE_MODE_RMII &&
-   phy_mode != PHY_INTERFACE_MODE_MII) {
-   dev_err(dev, "phy-mode is invalid\n");
- 

[PATCH net-next 1/3] net: ethernet: ave: add multiple clocks and resets support as required property

[Kunihiko Hayashi]

When the link is becoming up for Pro4 SoC, the kernel is stalled
due to some missing clocks and resets.

The AVE block for Pro4 is connected to the GIO bus in the SoC.
Without its clock/reset, the access to the AVE register makes the
system stall.

In the same way, another MAC clock for Giga-bit Connection and
the PHY clock are also required for Pro4 to activate the Giga-bit feature
and to recognize the PHY.

To satisfy these requirements, this patch adds support for multiple clocks
and resets, and adds the clock-names and reset-names to the binding because
we need to distinguish clock/reset for the AVE main block and the others.

Also, make the resets a required property. Currently, "reset is
optional" relies on that the bootloader or firmware has deasserted
the reset before booting the kernel.  Drivers should work without
such expectation.

Fixes: 4c270b55a5af ("net: ethernet: socionext: add AVE ethernet driver")
Suggested-by: Masahiro Yamada <yamada.masah...@socionext.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
---
 .../bindings/net/socionext,uniphier-ave4.txt   |  13 ++-
 drivers/net/ethernet/socionext/sni_ave.c   | 108 -
 2 files changed, 96 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
index 96398cc..85e0c49 100644
--- a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -17,9 +17,18 @@ Required properties:
  - phy-handle: Should point to the external phy device.
See ethernet.txt file in the same directory.
  - clocks: A phandle to the clock for the MAC.
+   For Pro4 SoC, that is "socionext,uniphier-pro4-ave4",
+   another MAC clock, GIO bus clock and PHY clock are also required.
+ - clock-names: Should contain
+   - "ether", "ether-gb", "gio", "ether-phy" for Pro4 SoC
+   - "ether" for others
+ - resets: A phandle to the reset control for the MAC. For Pro4 SoC,
+   GIO bus reset is also required.
+ - reset-names: Should contain
+   - "ether", "gio" for Pro4 SoC
+   - "ether" for others
 
 Optional properties:
- - resets: A phandle to the reset control for the MAC.
  - local-mac-address: See ethernet.txt in the same directory.
 
 Required subnode:
@@ -34,7 +43,9 @@ Example:
interrupts = <0 66 4>;
phy-mode = "rgmii";
phy-handle = <>;
+   clock-names = "ether";
clocks = <_clk 6>;
+   reset-names = "ether";
resets = <_rst 6>;
local-mac-address = [00 00 00 00 00 00];
 
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
index 0b3b7a4..52940bd 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -199,6 +199,9 @@
 
 #define IS_DESC_64BIT(p)   ((p)->data->is_desc_64bit)
 
+#define AVE_MAX_CLKS   4
+#define AVE_MAX_RSTS   2
+
 enum desc_id {
AVE_DESCID_RX,
AVE_DESCID_TX,
@@ -227,6 +230,8 @@ struct ave_desc_info {
 
 struct ave_soc_data {
boolis_desc_64bit;
+   const char  *clock_names[AVE_MAX_CLKS];
+   const char  *reset_names[AVE_MAX_RSTS];
 };
 
 struct ave_stats {
@@ -245,8 +250,10 @@ struct ave_private {
int phy_id;
unsigned intdesc_size;
u32 msg_enable;
-   struct clk  *clk;
-   struct reset_control*rst;
+   int nclks;
+   struct clk  *clk[AVE_MAX_CLKS];
+   int nrsts;
+   struct reset_control*rst[AVE_MAX_RSTS];
phy_interface_t phy_mode;
struct phy_device   *phydev;
struct mii_bus  *mdio;
@@ -1153,18 +1160,23 @@ static int ave_init(struct net_device *ndev)
struct device_node *np = dev->of_node;
struct device_node *mdio_np;
struct phy_device *phydev;
-   int ret;
+   int nc, nr, ret;
 
/* enable clk because of hw access until ndo_open */
-   ret = clk_prepare_enable(priv->clk);
-   if (ret) {
-   dev_err(dev, "can't enable clock\n");
-   return ret;
+   for (nc = 0; nc < priv->nclks; nc++) {
+   ret = clk_prepare_enable(priv->clk[nc]);
+   if (ret) {
+   dev_err(dev, "can't enable clock\n");
+   goto out_clk_disable;
+   }
}
-   ret = reset_control_deassert(priv->rst);
-   if (ret) {
-  

[PATCH 2/2] net: ethernet: ave: add UniPhier PXs3 support

[Kunihiko Hayashi]

Add a compatible string and SoC data for ethernet controller on
UniPhier PXs3 SoC.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
---
 drivers/net/ethernet/socionext/sni_ave.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
index f5c5984..0b3b7a4 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -1701,6 +1701,10 @@ static const struct ave_soc_data ave_ld20_data = {
.is_desc_64bit = true,
 };
 
+static const struct ave_soc_data ave_pxs3_data = {
+   .is_desc_64bit = false,
+};
+
 static const struct of_device_id of_ave_match[] = {
{
.compatible = "socionext,uniphier-pro4-ave4",
@@ -1718,6 +1722,10 @@ static const struct of_device_id of_ave_match[] = {
.compatible = "socionext,uniphier-ld20-ave4",
.data = _ld20_data,
},
+   {
+   .compatible = "socionext,uniphier-pxs3-ave4",
+   .data = _pxs3_data,
+   },
{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, of_ave_match);
-- 
2.7.4

[PATCH 0/2] net: ethernet: ave: add UniPhier PXs3 support

[Kunihiko Hayashi]

Add ethernet controller support on UniPhier PXs3 SoC.

Kunihiko Hayashi (2):
  dt-bindings: net: ave: add PXs3 support
  net: ethernet: ave: add UniPhier PXs3 support

 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt | 1 +
 drivers/net/ethernet/socionext/sni_ave.c  | 8 
 2 files changed, 9 insertions(+)

-- 
2.7.4

[PATCH 1/2] dt-bindings: net: ave: add PXs3 support

[Kunihiko Hayashi]

Add a compatible string for ethernet controller on UniPhier PXs3 SoC.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
---
 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
index 270ea4e..96398cc 100644
--- a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -9,6 +9,7 @@ Required properties:
- "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
- "socionext,uniphier-ld11-ave4" : for LD11 SoC
- "socionext,uniphier-ld20-ave4" : for LD20 SoC
+   - "socionext,uniphier-pxs3-ave4" : for PXs3 SoC
  - reg: Address where registers are mapped and size of region.
  - interrupts: Should contain the MAC interrupt.
  - phy-mode: See ethernet.txt in the same directory. Allow to choose
-- 
2.7.4

[PATCH] net: ethernet: ave: enable Rx drop interrupt

[Kunihiko Hayashi]

This enables AVE_GI_RXDROP interrupt factor. This factor indicates
depletion of Rx descriptors and the handler counts the number
of dropped packets.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
---
 drivers/net/ethernet/socionext/sni_ave.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
index b247707..043eb55 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -1349,7 +1349,7 @@ static int ave_open(struct net_device *ndev)
val |= AVE_IIRQC_EN0 | (AVE_INTM_COUNT << 16);
writel(val, priv->base + AVE_IIRQC);
 
-   val = AVE_GI_RXIINT | AVE_GI_RXOVF | AVE_GI_TX;
+   val = AVE_GI_RXIINT | AVE_GI_RXOVF | AVE_GI_TX | AVE_GI_RXDROP;
ave_irq_restore(ndev, val);
 
napi_enable(>napi_rx);
-- 
2.7.4

Re: net: phy: question about phy_is_internal for generic-phy

[Kunihiko Hayashi]

Hi Andrew,
Thank you for your advice.

On Fri, 9 Feb 2018 15:59:54 +0100 <and...@lunn.ch> wrote:

> > There are some SoCs that have a built-in phy, and sometimes
> > these SoCs can choose to use built-in phy or external phy.
> 
> O.K. This is the same use case we had at the end of last year.
> 
> How are the MDIO busses arranged? Is there an internal MDIO bus and an
> external MDIO bus? How do you change between the internal and the
> external? Or is it one bus, with the two PHYs having different
> addresses?
> 
> For the hardware in question last year, an MDIO MUX was implemented.
> The MAC has a phy-handle pointing to either the internal PHY on the
> internal MDIO bus, or the external PHY on the external MDIO bus. The
> MDIO MUX layer would then set the register to select between the two.
> 
> https://patchwork.kernel.org/patch/10025279/

I've checked our hardware configuration.
There are internal and external MDIO buses, and the register selects
between them. I'll consider to apply MDIO MUX layer to our MAC driver
rather than phy_is_internal of generic-phy.

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: net: phy: question about phy_is_internal for generic-phy

[Kunihiko Hayashi]

Hi Andrew,

On Thu, 8 Feb 2018 13:51:44 +0100 <and...@lunn.ch> wrote:

> On Thu, Feb 08, 2018 at 07:09:25PM +0900, Kunihiko Hayashi wrote:
> > Hello,
> > 
> > Is there a way to specify "phy is internal" to generic phy driver,
> > that is, to make phy_is_internal() function available?
> > 
> > I found "phy-is-integrated" DT property in
> > Documentation/devicetree/bindings/net/phy.txt, however, it seems
> > that the property is no effect for generic phy. And I think that
> > the meaning of "integrated" is slightly different from "internal".
> 
> Hi Kunihiko
> 
> Could you explain the bigger picture. Why do you need this?


There are some SoCs that have a built-in phy, and sometimes
these SoCs can choose to use built-in phy or external phy.

In our case, MAC driver needs to set a register to choose whether
to use them. This choice depends on the board implemented on the SoC.
And this built-in phy can be driven with generic phy driver.

If 'struct phy_driver' of a phy driver has the flag with
PHY_IS_INTERNAL, phy_is_internal() returns true,
and the MAC driver can decide a value of the register using it.

drivers/net/phy/phy_device.c:

if (phydrv->flags & PHY_IS_INTERNAL)
phydev->is_internal = true;

include/linux/phy.h:

static inline bool phy_is_internal(struct phy_device *phydev)
{
return phydev->is_internal;
}

Although I can write a new driver with PHY_IS_INTERNAL, or
add a new property to the MAC driver,
I'd like to use generic phy driver if possible.

However, it seems that the generic phy driver doesn't have the way
to express built-in phy with/without even DT.

How can I handle such built-in phy?

Thank you,

---
Best Regards,
Kunihiko Hayashi

net: phy: question about phy_is_internal for generic-phy

[Kunihiko Hayashi]

Hello,

Is there a way to specify "phy is internal" to generic phy driver,
that is, to make phy_is_internal() function available?

I found "phy-is-integrated" DT property in
Documentation/devicetree/bindings/net/phy.txt, however, it seems
that the property is no effect for generic phy. And I think that
the meaning of "integrated" is slightly different from "internal".

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v9 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v8: https://www.spinics.net/lists/netdev/msg474374.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v8:
- move operators at the beginning of the line to the end of the previous line
- dt-bindings: add blank lines before mdio and phy subnodes

Changes since v7:
- dt-bindings: fix mdio subnode description

Changes since v6:
- sort the order of local variables from longest to shortest line
- fix ave_probe() which calls register_netdev() at the end of initialization
- dt-bindings: remove phy node descriptions in mdio node

Changes since v5:
- replace license boilerplate with SPDX Identifier
- remove inline directives and an unused function

Changes since v4:
- fix larger integer warning on AVE_PFMBYTE_MASK0

Changes since v3:
- remove checking dma address and use dma_set_mask() to restirct address
- replace ave_mdio_busywait() with read_poll_timeout()
- replace functions to access to registers with readl/writel() directly
- replace a function to access to macaddr with ave_hw_write_macaddr()
- change return value of ave_dma_map() to error value
- move mdiobus_unregister() from ave_remove() to ave_uninit()
- eliminate else block at the end of ave_dma_map()
- add mask definitions for packet filter
- sort bitmap definitions in descending order
- add error check to some functions
- rename and sort functions to clear sub-categories
- fix error value consistency
- remove unneeded initializers
- change type of constant arrays

Changes since v2:
- replace clk_get() with devm_clk_get()
- replace reset_control_get() with devm_reset_control_get_optional_shared()
- add error return when the error occurs on the above *_get functions
- sort soc data and compatible strings
- remove clearly obvious comments
- modify dt-bindings document consistent with these modifications

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   48 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|5 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1736 
 6 files changed, 1813 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next v9 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Acked-by: Rob Herring <r...@kernel.org>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..270ea4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,48 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - clocks: A phandle to the clock for the MAC.
+
+Optional properties:
+ - resets: A phandle to the reset control for the MAC.
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Required subnode:
+ - mdio: A container for child nodes representing phy nodes.
+ See phy.txt in the same directory.
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   clocks = <_clk 6>;
+   resets = <_rst 6>;
+   local-mac-address = [00 00 00 00 00 00];
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

[PATCH net-next v9 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Reviewed-by: Andrew Lunn <and...@lunn.ch>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |5 +
 drivers/net/ethernet/socionext/sni_ave.c | 1736 ++
 5 files changed, 1765 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 39f62733..6cf5ade 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..ab83df6
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..111e7ca
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1736 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x000   /* ID */
+#define AVE_VR 0x004   /* Version */
+#define AVE_GRR0x008   /* Global Reset */
+#define AVE_CFGR   0x00c   /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control */
+#define AVE_MDIOAR 0x218   /* MDIO Address */
+#define AVE_MDIOWDR0x21c   /* MDIO Data */
+#define AVE_MDIOSR 0x220   /* MDIO Status */
+#define AVE_MDIORDR0x224   /* MDIO Rd Data */
+
+/* Descriptor Co

[PATCH net-next v8 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Acked-by: Rob Herring <r...@kernel.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 47 ++
 1 file changed, 47 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..8b03668
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,47 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - clocks: A phandle to the clock for the MAC.
+
+Optional properties:
+ - resets: A phandle to the reset control for the MAC.
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Required subnode:
+ - mdio: A container for child nodes representing phy nodes.
+ See phy.txt in the same directory.
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   clocks = <_clk 6>;
+   resets = <_rst 6>;
+   local-mac-address = [00 00 00 00 00 00];
+
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

[PATCH net-next v8 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Reviewed-by: Andrew Lunn <and...@lunn.ch>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |5 +
 drivers/net/ethernet/socionext/sni_ave.c | 1736 ++
 5 files changed, 1765 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 39f62733..6cf5ade 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..ab83df6
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..0925675
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1736 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x000   /* ID */
+#define AVE_VR 0x004   /* Version */
+#define AVE_GRR0x008   /* Global Reset */
+#define AVE_CFGR   0x00c   /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control */
+#define AVE_MDIOAR 0x218   /* MDIO Address */
+#define AVE_MDIOWDR0x21c   /* MDIO Data */
+#define AVE_MDIOSR 0x220   /* MDIO Status */
+#define AVE_MDIORDR0x224   /* MDIO Rd Data */
+
+/* Descriptor Co

[PATCH net-next v8 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v7: https://www.spinics.net/lists/netdev/msg473896.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v7:
- dt-bindings: fix mdio subnode description

Changes since v6:
- sort the order of local variables from longest to shortest line
- fix ave_probe() which calls register_netdev() at the end of initialization
- dt-bindings: remove phy node descriptions in mdio node

Changes since v5:
- replace license boilerplate with SPDX Identifier
- remove inline directives and an unused function

Changes since v4:
- fix larger integer warning on AVE_PFMBYTE_MASK0

Changes since v3:
- remove checking dma address and use dma_set_mask() to restirct address
- replace ave_mdio_busywait() with read_poll_timeout()
- replace functions to access to registers with readl/writel() directly
- replace a function to access to macaddr with ave_hw_write_macaddr()
- change return value of ave_dma_map() to error value
- move mdiobus_unregister() from ave_remove() to ave_uninit()
- eliminate else block at the end of ave_dma_map()
- add mask definitions for packet filter
- sort bitmap definitions in descending order
- add error check to some functions
- rename and sort functions to clear sub-categories
- fix error value consistency
- remove unneeded initializers
- change type of constant arrays

Changes since v2:
- replace clk_get() with devm_clk_get()
- replace reset_control_get() with devm_reset_control_get_optional_shared()
- add error return when the error occurs on the above *_get functions
- sort soc data and compatible strings
- remove clearly obvious comments
- modify dt-bindings document consistent with these modifications

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   47 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|5 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1736 
 6 files changed, 1812 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

Re: [PATCH net-next v7 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

Hello Andrew,

On Thu, 21 Dec 2017 12:32:54 +0100 Andrew Lunn <and...@lunn.ch> wrote:

> > +Optional properties:
> > + - resets: A phandle to the reset control for the MAC
> > + - local-mac-address: See ethernet.txt in the same directory.
> > +
> > +Required subnode:
> > + - mdio: Device tree subnode with the following required properties:
> > +
> > +Example:
> 
> It sounds like there should be some properties before the Example.

Indeed, this is my carelessness.

> 
>Andrew
> 
> > +
> > +   ether: ethernet@6500 {
> > +   compatible = "socionext,uniphier-ld20-ave4";
> > +   reg = <0x6500 0x8500>;
> > +   interrupts = <0 66 4>;
> > +   phy-mode = "rgmii";
> > +   phy-handle = <>;
> > +   clocks = <_clk 6>;
> > +   resets = <_rst 6>;
> > +   local-mac-address = [00 00 00 00 00 00];
> 
> Typically you would put a blank line here, before the mdio node.

Okay, I'll put it.

> > +   mdio {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   ethphy: ethphy@1 {
> > +   reg = <1>;
> > +   };
> > +   };
> > +   };
> 
>   Andrew

Thank you,

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v7 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v6: https://www.spinics.net/lists/netdev/msg472133.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v6:
- sort the order of local variables from longest to shortest line
- fix ave_probe() which calls register_netdev() at the end of initialization
- dt-bindings: remove phy node descriptions in mdio node

Changes since v5:
- replace license boilerplate with SPDX Identifier
- remove inline directives and an unused function

Changes since v4:
- fix larger integer warning on AVE_PFMBYTE_MASK0

Changes since v3:
- remove checking dma address and use dma_set_mask() to restirct address
- replace ave_mdio_busywait() with read_poll_timeout()
- replace functions to access to registers with readl/writel() directly
- replace a function to access to macaddr with ave_hw_write_macaddr()
- change return value of ave_dma_map() to error value
- move mdiobus_unregister() from ave_remove() to ave_uninit()
- eliminate else block at the end of ave_dma_map()
- add mask definitions for packet filter
- sort bitmap definitions in descending order
- add error check to some functions
- rename and sort functions to clear sub-categories
- fix error value consistency
- remove unneeded initializers
- change type of constant arrays

Changes since v2:
- replace clk_get() with devm_clk_get()
- replace reset_control_get() with devm_reset_control_get_optional_shared()
- add error return when the error occurs on the above *_get functions
- sort soc data and compatible strings
- remove clearly obvious comments
- modify dt-bindings document consistent with these modifications

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   45 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|5 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1736 
 6 files changed, 1810 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next v7 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Acked-by: Rob Herring <r...@kernel.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 45 ++
 1 file changed, 45 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..c73a6f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,45 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - clocks: A phandle to the clock for the MAC.
+
+Optional properties:
+ - resets: A phandle to the reset control for the MAC
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Required subnode:
+ - mdio: Device tree subnode with the following required properties:
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   clocks = <_clk 6>;
+   resets = <_rst 6>;
+   local-mac-address = [00 00 00 00 00 00];
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

[PATCH net-next v7 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Reviewed-by: Andrew Lunn <and...@lunn.ch>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |5 +
 drivers/net/ethernet/socionext/sni_ave.c | 1736 ++
 5 files changed, 1765 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 39f62733..6cf5ade 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..ab83df6
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..0925675
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1736 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x000   /* ID */
+#define AVE_VR 0x004   /* Version */
+#define AVE_GRR0x008   /* Global Reset */
+#define AVE_CFGR   0x00c   /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control */
+#define AVE_MDIOAR 0x218   /* MDIO Address */
+#define AVE_MDIOWDR0x21c   /* MDIO Data */
+#define AVE_MDIOSR 0x220   /* MDIO Status */
+#define AVE_MDIORDR0x224   /* MDIO Rd Data */
+
+/* Descriptor Co

Re: [PATCH net-next v6 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

Hello David,

On Fri, 15 Dec 2017 12:57:49 -0500 David Miller <da...@davemloft.net> wrote:

> From: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> Date: Thu, 14 Dec 2017 19:05:10 +0900
> 
> > +static void ave_desc_write(struct net_device *ndev, enum desc_id id,
> > +  int entry, int offset, u32 val)
> > +{
> > +   struct ave_private *priv = netdev_priv(ndev);
> > +   u32 addr = (id == AVE_DESCID_TX) ? priv->tx.daddr : priv->rx.daddr;
> 
> Please always order local variables from longest to shortest line.
> 
> Audit your entire submission for this issue, thank you.

I see. I'll fix the order of local variables.

> > +   ret = register_netdev(ndev);
> > +   if (ret) {
> > +   dev_err(dev, "failed to register netdevice\n");
> > +   goto out_del_napi;
> > +   }
> > +
> > +   platform_set_drvdata(pdev, ndev);
> 
> You must make all software state settings before reigster_netdev() is
> invoked.
> 
> At the exact moment you call register_netdev(), your device can be
> brought up, interrupts processed, PHY state changes made, etc.
> 
> So you must put this platform_set_drvdata() before the
> register_netdev() call.
> 
> Generally speaking, register_netdev() must always be the last state
> modification done by your probe routine.

Indeed. It's not good to invoke register_netdev() with all software
initialization not completed. I'll move register_netdev() after
all initialization.

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next v6 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

Hello Florian,

On Thu, 14 Dec 2017 15:24:17 -0800 <f.faine...@gmail.com> wrote:
> 
> 
> On 12/14/2017 02:05 AM, Kunihiko Hayashi wrote:
> > DT bindings for the AVE ethernet controller found on Socionext's
> > UniPhier platforms.
> > 
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> > Acked-by: Rob Herring <r...@kernel.org>
> > ---
> >  .../bindings/net/socionext,uniphier-ave4.txt   | 48 
> > ++
> >  1 file changed, 48 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
> > b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > new file mode 100644
> > index 000..4700377
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > @@ -0,0 +1,48 @@
> > +* Socionext AVE ethernet controller
> > +
> > +This describes the devicetree bindings for AVE ethernet controller
> > +implemented on Socionext UniPhier SoCs.
> > +
> > +Required properties:
> > + - compatible: Should be
> > +   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> > +   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> > +   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
> > +   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> > + - reg: Address where registers are mapped and size of region.
> > + - interrupts: Should contain the MAC interrupt.
> > + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> > +   "rgmii", "rmii", or "mii" according to the PHY.
> > + - phy-handle: Should point to the external phy device.
> > +   See ethernet.txt file in the same directory.
> > + - clocks: A phandle to the clock for the MAC.
> > +
> > +Optional properties:
> > + - resets: A phandle to the reset control for the MAC
> > + - local-mac-address: See ethernet.txt in the same directory.
> > +
> > +Required subnode:
> > + - mdio: Device tree subnode with the following required properties:
> > +   - #address-cells: Must be <1>.
> > +   - #size-cells: Must be <0>.
> > +   - reg: phy ID number, usually a small integer.
> 
> Almost, the "reg" property applies to the MDIO child nodes: the Ethernet
> PHYs/MDIO devices. For the MDIO controller itself, the "reg" property,
> if specified, would be relative to the Ethernet controller's base
> register address.
> 
> Please drop this property's description here.

Thank you for pointing out.
Surely this MDIO node doesn't have own "reg" property, and this description
shows "reg" property of PHY device connected to the MDIO bus.

I'll remove the description.

Thank you,

> > +
> > +Example:
> > +
> > +   ether: ethernet@6500 {
> > +   compatible = "socionext,uniphier-ld20-ave4";
> > +   reg = <0x6500 0x8500>;
> > +   interrupts = <0 66 4>;
> > +   phy-mode = "rgmii";
> > +   phy-handle = <>;
> > +   clocks = <_clk 6>;
> > +   resets = <_rst 6>;
> > +   local-mac-address = [00 00 00 00 00 00];
> > +   mdio {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   ethphy: ethphy@1 {
> > +   reg = <1>;
> > +   };
> > +   };
> > +   };
> > 
> 
> -- 
> Florian

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v6 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Acked-by: Rob Herring <r...@kernel.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..4700377
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,48 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - clocks: A phandle to the clock for the MAC.
+
+Optional properties:
+ - resets: A phandle to the reset control for the MAC
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Required subnode:
+ - mdio: Device tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
+   - reg: phy ID number, usually a small integer.
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   clocks = <_clk 6>;
+   resets = <_rst 6>;
+   local-mac-address = [00 00 00 00 00 00];
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

[PATCH net-next v6 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v5: https://www.spinics.net/lists/netdev/msg471330.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v5:
- replace license boilerplate with SPDX Identifier
- remove inline directives and an unused function

Changes since v4:
- fix larger integer warning on AVE_PFMBYTE_MASK0

Changes since v3:
- remove checking dma address and use dma_set_mask() to restirct address
- replace ave_mdio_busywait() with read_poll_timeout()
- replace functions to access to registers with readl/writel() directly
- replace a function to access to macaddr with ave_hw_write_macaddr()
- change return value of ave_dma_map() to error value
- move mdiobus_unregister() from ave_remove() to ave_uninit()
- eliminate else block at the end of ave_dma_map()
- add mask definitions for packet filter
- sort bitmap definitions in descending order
- add error check to some functions
- rename and sort functions to clear sub-categories
- fix error value consistency
- remove unneeded initializers
- change type of constant arrays

Changes since v2:
- replace clk_get() with devm_clk_get()
- replace reset_control_get() with devm_reset_control_get_optional_shared()
- add error return when the error occurs on the above *_get functions
- sort soc data and compatible strings
- remove clearly obvious comments
- modify dt-bindings document consistent with these modifications

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   48 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|5 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1727 
 6 files changed, 1804 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next v6 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Reviewed-by: Andrew Lunn <and...@lunn.ch>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |5 +
 drivers/net/ethernet/socionext/sni_ave.c | 1727 ++
 5 files changed, 1756 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 39f62733..6cf5ade 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..ab83df6
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..15f975e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1727 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x000   /* ID */
+#define AVE_VR 0x004   /* Version */
+#define AVE_GRR0x008   /* Global Reset */
+#define AVE_CFGR   0x00c   /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control */
+#define AVE_MDIOAR 0x218   /* MDIO Address */
+#define AVE_MDIOWDR0x21c   /* MDIO Data */
+#define AVE_MDIOSR 0x220   /* MDIO Status */
+#define AVE_MDIORDR0x224   /* MDIO Rd Data */
+
+/* Descriptor Co

Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

On Mon, 11 Dec 2017 10:19:15 +0100 Phlippe Ombredanne <pombreda...@nexb.com> 
wrote:

> Dear Kunihiko-san,
> 
> On Mon, Dec 11, 2017 at 8:57 AM, Kunihiko Hayashi
> <hayashi.kunih...@socionext.com> wrote:
> > The UniPhier platform from Socionext provides the AVE ethernet
> > controller that includes MAC and MDIO bus supporting RGMII/RMII
> > modes. The controller is named AVE.
> >
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> > Reviewed-by: Andrew Lunn <and...@lunn.ch>
> [...]
> > --- /dev/null
> > +++ b/drivers/net/ethernet/socionext/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> You are correctly using SPDX ids here
> 
> > +obj-$(CONFIG_SNI_AVE) += sni_ave.o
> > diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
> > b/drivers/net/ethernet/socionext/sni_ave.c
> > new file mode 100644
> > index 000..7b293c2
> > --- /dev/null
> > +++ b/drivers/net/ethernet/socionext/sni_ave.c
> > @@ -0,0 +1,1744 @@
> > +/**
> > + * sni_ave.c - Socionext UniPhier AVE ethernet driver
> > + *
> > + * Copyright 2014 Panasonic Corporation
> > + * Copyright 2015-2017 Socionext Inc.
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2  of
> > + * the License as published by the Free Software Foundation.
> > + *
> > + * 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.
> > + */
> 
> ... then I guess you could also use them here, replacing at least 7
> lines of boilerplate by a single id line?
> 
> > +// SDPX-License-Identifier: GPL-2.0

Thank you for your kindly comment.

It seems there are few ethernet drivers applying SPDX-License-Identifier
to C source, then it has been difficult for me to decide whether to replace
boilerplate.

If it's no problem to apply SPDX to sources of ethernet drivers,
I'll replace it with the single line.

> And if you go C++ style all the way, this could be even more compact:
> 
> > +// SDPX-License-Identifier: GPL-2.0
> > +// sni_ave.c - Socionext UniPhier AVE ethernet driver
> > +// Copyright 2014 Panasonic Corporation
> > +// Copyright 2015-2017 Socionext Inc.

It's much simpler.
If it's reasonable to apply this style here, I can replace it, too.

Thank you,

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v5 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v4: https://www.spinics.net/lists/netdev/msg469294.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v4:
- fix larger integer warning on AVE_PFMBYTE_MASK0

Changes since v3:
- remove checking dma address and use dma_set_mask() to restirct address
- replace ave_mdio_busywait() with read_poll_timeout()
- replace functions to access to registers with readl/writel() directly
- replace a function to access to macaddr with ave_hw_write_macaddr()
- change return value of ave_dma_map() to error value
- move mdiobus_unregister() from ave_remove() to ave_uninit()
- eliminate else block at the end of ave_dma_map()
- add mask definitions for packet filter
- sort bitmap definitions in descending order
- add error check to some functions
- rename and sort functions to clear sub-categories
- fix error value consistency
- remove unneeded initializers
- change type of constant arrays

Changes since v2:
- replace clk_get() with devm_clk_get()
- replace reset_control_get() with devm_reset_control_get_optional_shared()
- add error return when the error occurs on the above *_get functions
- sort soc data and compatible strings
- remove clearly obvious comments
- modify dt-bindings document consistent with these modifications

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   48 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|5 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1744 
 6 files changed, 1821 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Reviewed-by: Andrew Lunn <and...@lunn.ch>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |5 +
 drivers/net/ethernet/socionext/sni_ave.c | 1744 ++
 5 files changed, 1773 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 39f62733..6cf5ade 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..ab83df6
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..7b293c2
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1744 @@
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ *
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x000   /* ID */
+#define AVE_VR 0x004   /* Version */
+#define AVE_GRR0x008   /* Global Reset */
+#define AVE_CFGR   0x00c   /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_R

[PATCH net-next v5 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Acked-by: Rob Herring <r...@kernel.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..4700377
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,48 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - clocks: A phandle to the clock for the MAC.
+
+Optional properties:
+ - resets: A phandle to the reset control for the MAC
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Required subnode:
+ - mdio: Device tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
+   - reg: phy ID number, usually a small integer.
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   clocks = <_clk 6>;
+   resets = <_rst 6>;
+   local-mac-address = [00 00 00 00 00 00];
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

Re: [PATCH net-next v4 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

Hi Andrew,

On Fri, 1 Dec 2017 14:49:00 +0100 Andrew Lunn <and...@lunn.ch> wrote:

> On Fri, Dec 01, 2017 at 10:03:50AM +0900, Kunihiko Hayashi wrote:
> > The UniPhier platform from Socionext provides the AVE ethernet
> > controller that includes MAC and MDIO bus supporting RGMII/RMII
> > modes. The controller is named AVE.
> > 
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> 
> Hi Kunihiko
> 
> I reviewed the PHY and MDIO code. It all looks good. I cannot say much
> about the rest of it though.
> 
> Reviewed-by: Andrew Lunn <and...@lunn.ch>

Thanks! I'll remove warning from kbuild test, and repost the series for
reviewing other part.

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v4 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |5 +
 drivers/net/ethernet/socionext/sni_ave.c | 1744 ++
 5 files changed, 1773 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 39f62733..6cf5ade 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..ab83df6
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..6f42f12
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1744 @@
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ *
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x000   /* ID */
+#define AVE_VR 0x004   /* Version */
+#define AVE_GRR0x008   /* Global Reset */
+#define AVE_CFGR   0x00c   /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (low

[PATCH net-next v4 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Acked-by: Rob Herring <r...@kernel.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..4700377
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,48 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - clocks: A phandle to the clock for the MAC.
+
+Optional properties:
+ - resets: A phandle to the reset control for the MAC
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Required subnode:
+ - mdio: Device tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
+   - reg: phy ID number, usually a small integer.
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   clocks = <_clk 6>;
+   resets = <_rst 6>;
+   local-mac-address = [00 00 00 00 00 00];
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

[PATCH net-next v4 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v3: http://www.spinics.net/lists/netdev/msg462550.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v3:
- remove checking dma address and use dma_set_mask() to restirct address
- replace ave_mdio_busywait() with read_poll_timeout()
- replace functions to access to registers with readl/writel() directly
- replace a function to access to macaddr with ave_hw_write_macaddr()
- change return value of ave_dma_map() to error value
- move mdiobus_unregister() from ave_remove() to ave_uninit()
- eliminate else block at the end of ave_dma_map()
- add mask definitions for packet filter
- sort bitmap definitions in descending order
- add error check to some functions
- rename and sort functions to clear sub-categories
- fix error value consistency
- remove unneeded initializers
- change type of constant arrays

Changes since v2:
- replace clk_get() with devm_clk_get()
- replace reset_control_get() with devm_reset_control_get_optional_shared()
- add error return when the error occurs on the above *_get functions
- sort soc data and compatible strings
- remove clearly obvious comments
- modify dt-bindings document consistent with these modifications

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   48 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|5 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1744 
 6 files changed, 1821 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next v3 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..4b7eca8
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,48 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - clocks: A phandle to the clock for the MAC.
+
+Optional properties:
+ - resets: A phandle to the reset control for the MAC.
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Required subnode:
+ - mdio: Device tree subnode with the following required properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
+   - reg: phy ID number, usually a small integer.
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   clocks = <_clk 6>;
+   resets = <_rst 6>;
+   local-mac-address = [00 00 00 00 00 00];
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

[PATCH net-next v3 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v1: http://www.spinics.net/lists/netdev/msg454292.html
v2: http://www.spinics.net/lists/netdev/msg460109.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v2:
- replace clk_get() with devm_clk_get()
- replace reset_control_get() with devm_reset_control_get_optional_shared()
- add error return when the error occurs on the above *_get functions
- sort soc data and compatible strings
- remove clearly obvious comments
- modify dt-bindings document consistent with these modifications

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   48 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|4 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1746 
 6 files changed, 1822 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next v3 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |4 +
 drivers/net/ethernet/socionext/sni_ave.c | 1746 ++
 5 files changed, 1774 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index a0a03d4..9f55b36 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..0356341
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..ed0f4a3
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1746 @@
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ *
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x0 /* ID */
+#define AVE_VR 0x4 /* Version */
+#define AVE_GRR0x8 /* Global Reset */
+#define AVE_CFGR   0xc /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control *

Re: [PATCH net-next v2 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

On Thu, 19 Oct 2017 09:29:03 +0900 <yamada.masah...@socionext.com> wrote:

> 2017-10-18 19:23 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> > On Mon, 16 Oct 2017 00:08:21 +0900 <yamada.masah...@socionext.com> wrote:
> 
> >> priv->rst = devm_reset_control_get_optional_shared(dev, NULL);
> >> if (IS_ERR(priv->rst))
> >>   return PTR_ERR(priv->rst);
> >
> > The clk and reset are optional in the driver.
> > Referring to your suggested method, I'll fix the part of clk and reset.
> >
> 
> 
> Why is clk optional?

Indeed. My check point was wrong.
This clk property should be treated as "required".

In v2, the clock enabling code moved to ndo_init().
Although probing the driver succeeds without clk,
enabling the driver fails clearly.

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next v2 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

On Sat, 14 Oct 2017 01:41:12 +0900 <yamada.masah...@socionext.com> wrote:

> 2017-10-13 9:35 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> > DT bindings for the AVE ethernet controller found on Socionext's
> > UniPhier platforms.
> >
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> > ---
> >  .../bindings/net/socionext,uniphier-ave4.txt   | 53 
> > ++
> >  1 file changed, 53 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
> > b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > new file mode 100644
> > index 000..25f4d92
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > @@ -0,0 +1,53 @@
> > +* Socionext AVE ethernet controller
> > +
> > +This describes the devicetree bindings for AVE ethernet controller
> > +implemented on Socionext UniPhier SoCs.
> > +
> > +Required properties:
> > + - compatible: Should be
> > +   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> > +   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> > +   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> > +   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
> 
> Nit.  LD11, LD20, in this order please.

I'll fix it. And also the SoC-dependent structures of the driver.

> > + - reg: Address where registers are mapped and size of region.
> > + - interrupts: Should contain the MAC interrupt.
> > + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> > +   "rgmii", "rmii", or "mii" according to the PHY.
> > + - pinctrl-names: List of assigned state names, see pinctrl
> > +   binding documentation.
> > + - pinctrl-0: List of phandles to configure the GPIO pin,
> 
> 
> configure the GPIO pin?
> 
> git-grep found this phrase.
> 
> 
> $ git grep "List of phandles to configure the GPIO"
> Documentation/devicetree/bindings/net/microchip,enc28j60.txt:-
> pinctrl-0: List of phandles to configure the GPIO pin used as
> interrupt line,
> 
> 
> 
> 
> 
> > +   see pinctrl binding documentation. Choose this appropriately
> > +   according to phy-mode.
> > +   - <_ether_rgmii> if phy-mode is "rgmii".
> > +   - <_ether_rmii> if phy-mode is "rmii".
> > +   - <_ether_mii> if phy-mode is "mii".
> 
> pinctrl_ether_rgmii is just a label
> you just happened to write in your DT file.
> 
> This information is totally unrelated to hardware specification.
> It is not stable, either.

Surely this driver is independent from the pinctrl.
I'll remove the descriptions about the pinctrl.

> > + - phy-handle: Should point to the external phy device.
> > +   See ethernet.txt file in the same directory.
> > + - mdio subnode: Should be device tree subnode with the following required
> > +   properties:
> > +   - #address-cells: Must be <1>.
> > +   - #size-cells: Must be <0>.
> > +   - reg: phy ID number, usually a small integer.
> 
> 
> Are you talking about "Required subnode" in the "Required properties"?

It might be confused description. The subnode is separated from the properties.

> > +Optional properties:
> > + - local-mac-address: See ethernet.txt in the same directory.

In addition, I'll add "clocks" and "resets" according to another mail.

> > +
> > +Example:
> > +
> > +   ether: ethernet@6500 {
> > +   compatible = "socionext,uniphier-ld20-ave4";
> > +   reg = <0x6500 0x8500>;
> > +   interrupts = <0 66 4>;
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_ether_rgmii>;
> > +   phy-mode = "rgmii";
> > +   phy-handle = <>;
> > +   local-mac-address = [00 00 00 00 00 00];
> > +   mdio {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   ethphy: ethphy@1 {
> > +   reg = <1>;
> > +   };
> > +   };
> > +   };
> > --
> > 2.7.4
> >
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next v2 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

On Mon, 16 Oct 2017 00:08:21 +0900 <yamada.masah...@socionext.com> wrote:

> 2017-10-13 9:35 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> > +static int ave_probe(struct platform_device *pdev)
> > +{
> > +   struct device *dev = >dev;
> > +   struct device_node *np = dev->of_node;
> > +   u32 ave_id;
> > +   struct ave_private *priv;
> > +   const struct ave_soc_data *data;
> > +   phy_interface_t phy_mode;
> > +   struct net_device *ndev;
> > +   struct resource *res;
> > +   void __iomem *base;
> > +   int irq, ret = 0;
> > +   char buf[ETHTOOL_FWVERS_LEN];
> > +   const void *mac_addr;
> > +
> > +   data = of_device_get_match_data(dev);
> > +   if (WARN_ON(!data))
> > +   return -EINVAL;
> > +
> > +   phy_mode = of_get_phy_mode(np);
> > +   if (phy_mode < 0) {
> > +   dev_err(dev, "phy-mode not found\n");
> > +   return -EINVAL;
> > +   }
> > +   if ((!phy_interface_mode_is_rgmii(phy_mode)) &&
> > +   phy_mode != PHY_INTERFACE_MODE_RMII &&
> > +   phy_mode != PHY_INTERFACE_MODE_MII) {
> > +   dev_err(dev, "phy-mode is invalid\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   irq = platform_get_irq(pdev, 0);
> > +   if (irq < 0) {
> > +   dev_err(dev, "IRQ not found\n");
> > +   return irq;
> > +   }
> > +
> > +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   base = devm_ioremap_resource(dev, res);
> > +   if (IS_ERR(base))
> > +   return PTR_ERR(base);
> > +
> > +   /* alloc netdevice */
> > +   ndev = alloc_etherdev(sizeof(struct ave_private));
> > +   if (!ndev) {
> > +   dev_err(dev, "can't allocate ethernet device\n");
> > +   return -ENOMEM;
> > +   }
> > +
> > +   ndev->netdev_ops = _netdev_ops;
> > +   ndev->ethtool_ops = _ethtool_ops;
> > +   SET_NETDEV_DEV(ndev, dev);
> > +
> > +   /* support hardware checksum */
> > +   ndev->features|= (NETIF_F_IP_CSUM | NETIF_F_RXCSUM);
> > +   ndev->hw_features |= (NETIF_F_IP_CSUM | NETIF_F_RXCSUM);
> > +
> > +   ndev->max_mtu = AVE_MAX_ETHFRAME - (ETH_HLEN + ETH_FCS_LEN);
> > +
> > +   /* get mac address */
> > +   mac_addr = of_get_mac_address(np);
> > +   if (mac_addr)
> > +   ether_addr_copy(ndev->dev_addr, mac_addr);
> > +
> > +   /* if the mac address is invalid, use random mac address */
> > +   if (!is_valid_ether_addr(ndev->dev_addr)) {
> > +   eth_hw_addr_random(ndev);
> > +   dev_warn(dev, "Using random MAC address: %pM\n",
> > +ndev->dev_addr);
> > +   }
> > +
> > +   priv = netdev_priv(ndev);
> > +   priv->base = base;
> > +   priv->irq = irq;
> > +   priv->ndev = ndev;
> > +   priv->msg_enable = netif_msg_init(-1, AVE_DEFAULT_MSG_ENABLE);
> > +   priv->phy_mode = phy_mode;
> > +   priv->data = data;
> > +
> > +   if (IS_DESC_64BIT(priv)) {
> > +   priv->desc_size = AVE_DESC_SIZE_64;
> > +   priv->tx.daddr  = AVE_TXDM_64;
> > +   priv->rx.daddr  = AVE_RXDM_64;
> > +   } else {
> > +   priv->desc_size = AVE_DESC_SIZE_32;
> > +   priv->tx.daddr  = AVE_TXDM_32;
> > +   priv->rx.daddr  = AVE_RXDM_32;
> > +   }
> > +   priv->tx.ndesc = AVE_NR_TXDESC;
> > +   priv->rx.ndesc = AVE_NR_RXDESC;
> > +
> > +   u64_stats_init(>stats_rx.syncp);
> > +   u64_stats_init(>stats_tx.syncp);
> > +
> > +   /* get clock */
> 
> Please remove this super-obvious comment.

I'll check comments out and remove them unnecessary.

> > +   priv->clk = clk_get(dev, NULL);
> 
> Missing clk_put() in the failure path.
> 
> Why don't you use devm?
> 
> 
> 
> > +   if (IS_ERR(priv->clk))
> > +   priv->clk = NULL;
> 
> So, clk is optional, but
> you need to check EPROBE_DEFER.
> 
> 
> 
> 
> > +   /* get reset */
> 
> Remove.
> 
> 
> > +   priv->rst = reset_control_get(dev, NULL);
> > +   if (IS_ERR(priv->rst))
> > +   priv->rst = NULL;
> 
> 
> reset_control_get() is deprecated.  Do not use it in a new driver.
> 
> Again, missing reset_control_put().   devm?
> 
> 
> The reset seems optional (again, ignoring EPROBE_DEFER)
> but you did not use reset_control_get_optional, why?
> 
> From your code, this reset is used as shared.
> 
> 
> priv->rst = devm_reset_control_get_optional_shared(dev, NULL);
> if (IS_ERR(priv->rst))
>   return PTR_ERR(priv->rst);

The clk and reset are optional in the driver.
Referring to your suggested method, I'll fix the part of clk and reset.

> 
> 
> -- 
> Best Regards
> Masahiro Yamada

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v2 0/2] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

v1: http://www.spinics.net/lists/netdev/msg454292.html

The PHY patch included in v1 has already separated in:
http://www.spinics.net/lists/netdev/msg454595.html

Changes since v1:
- add/remove devicetree properties and sub-node
  - remove "internal-phy-interrupt" and "desc-bits" property
  - add SoC data structures based on compatible strings
  - add node operation to apply "mdio" sub-node
- add support for features
  - add support for {get,set}_pauseparam and pause frame operations
  - add support for ndo_get_stats64 instead of ndo_get_stats
- replace with desiable functions
  - replace check for valid phy_mode with phy_interface{_mode}_is_rgmii()
  - replace phy attach message with phy_attached_info()
  - replace 32bit operation with {upper,lower}_32_bits() on ave_wdesc_addr()
  - replace nway_reset and get_link with generic functions
- move operations to proper functions
  - move phy_start_aneg() to ndo_open,
and remove unnecessary PHY interrupt operations
See http://www.spinics.net/lists/netdev/msg454590.html
  - move irq initialization and descriptor memory allocation to ndo_open
  - move initialization of reset and clock and mdiobus to ndo_init
- fix skbuffer operations
  - fix skb alignment operations and add Rx buffer adjustment for descriptor
See http://www.spinics.net/lists/netdev/msg456014.html
  - add error returns when dma_map_single() failed 
- clean up code structures
  - clean up wait-loop and wake-queue conditions
  - add ave_wdesc_addr() and offset definitions
  - add ave_macaddr_init() to clean up mac-address operation
  - fix checking whether Tx entry is not enough
  - fix supported features of phydev
  - add necessary free/disable operations
  - add phydev check on ave_{get,set}_wol()
  - remove netif_carrier functions, phydev initializer, and Tx budget check
- change obsolate codes
  - replace ndev->{base_addr,irq} with the members of ave_private
- rename goto labels and mask definitions, and remove unused codes

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   53 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|4 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1773 
 6 files changed, 1854 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next v2 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 53 ++
 1 file changed, 53 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..25f4d92
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,53 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be
+   - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
+   - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
+   - "socionext,uniphier-ld20-ave4" : for LD20 SoC
+   - "socionext,uniphier-ld11-ave4" : for LD11 SoC
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: Should contain the MAC interrupt.
+ - phy-mode: See ethernet.txt in the same directory. Allow to choose
+   "rgmii", "rmii", or "mii" according to the PHY.
+ - pinctrl-names: List of assigned state names, see pinctrl
+   binding documentation.
+ - pinctrl-0: List of phandles to configure the GPIO pin,
+   see pinctrl binding documentation. Choose this appropriately
+   according to phy-mode.
+   - <_ether_rgmii> if phy-mode is "rgmii".
+   - <_ether_rmii> if phy-mode is "rmii".
+   - <_ether_mii> if phy-mode is "mii".
+ - phy-handle: Should point to the external phy device.
+   See ethernet.txt file in the same directory.
+ - mdio subnode: Should be device tree subnode with the following required
+   properties:
+   - #address-cells: Must be <1>.
+   - #size-cells: Must be <0>.
+   - reg: phy ID number, usually a small integer.
+
+Optional properties:
+ - local-mac-address: See ethernet.txt in the same directory.
+
+Example:
+
+   ether: ethernet@6500 {
+   compatible = "socionext,uniphier-ld20-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_ether_rgmii>;
+   phy-mode = "rgmii";
+   phy-handle = <>;
+   local-mac-address = [00 00 00 00 00 00];
+   mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
+   };
-- 
2.7.4

[PATCH net-next v2 2/2] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |4 +
 drivers/net/ethernet/socionext/sni_ave.c | 1773 ++
 5 files changed, 1801 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index a0a03d4..9f55b36 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..3a1829e
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Socionext devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..0356341
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..7e399fc
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1773 @@
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ *
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x0 /* ID */
+#define AVE_VR 0x4 /* Version */
+#define AVE_GRR0x8 /* Global Reset */
+#define AVE_CFGR   0xc /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control *

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

On Mon, 11 Sep 2017 15:55:56 +0900 <hayashi.kunih...@socionext.com> wrote:

> > > +static int ave_set_rxdesc(struct net_device *ndev, int entry)
> > > +{
> > > + struct ave_private *priv = netdev_priv(ndev);
> > > + struct sk_buff *skb;
> > > + unsigned long align;
> > > + dma_addr_t paddr;
> > > + void *buffptr;
> > > + int ret = 0;
> > > +
> > > + skb = priv->rx.desc[entry].skbs;
> > > + if (!skb) {
> > > + skb = netdev_alloc_skb_ip_align(ndev,
> > > + AVE_MAX_ETHFRAME + NET_SKB_PAD);
> > > + if (!skb) {
> > > + netdev_err(ndev, "can't allocate skb for Rx\n");
> > > + return -ENOMEM;
> > > + }
> > > + }
> > > +
> > > + /* set disable to cmdsts */
> > > + ave_wdesc(ndev, AVE_DESCID_RX, entry, 0, AVE_STS_INTR | AVE_STS_OWN);
> > > +
> > > + /* align skb data for cache size */
> > > + align = (unsigned long)skb_tail_pointer(skb) & (NET_SKB_PAD - 1);
> > > + align = NET_SKB_PAD - align;
> > > + skb_reserve(skb, align);
> > > + buffptr = (void *)skb_tail_pointer(skb);
> > 
> > Are you positive you need this? Because by default, the networking stack
> > will align to the maximum between your L1 cache line size and 64 bytes,
> > which should be a pretty good alignment guarantee.
> 
> Now if L1 cache line size is 128,
> the skb buffer is also aligned to 128, isn't it?
> So this code doesn't make sense.

Although the above cache-alignment operation isn't necessary,
we should add the address adjustment because of the restriction of the hardware
specification.

The netdev_alloc_skb_ip_align() allocates the cache-aligned buffer
and add 2 byte to skb->data by skb_reserve(skb, NET_IP_ALIGN).
Then skb->data points to "aligned address + 2 byte".

When we call dma_map_single() with skb->data, it might return the aligned 
address
and there might not be 2 byte space.

On the other hand, according to the hardware specification,
the Rx buffer address set to the descriptor is assumed that:
 - the Rx address is 4 byte aligned,
 - the Rx address begins with 2 byte headroom, data will be put from (buffer+2).

Therefore, to make headroom in front of returned address from ave_dma_map(),
I think that the buffer address should be adjusted like that:

skb = netdev_alloc_skb_ip_align(ndev, AVE_MAX_ETHFRAME);

paddr = ave_dma_map(ndev, >rx.desc[entry],
skb->data - NET_IP_ALIGN,
AVE_MAX_ETHFRAME + NET_IP_ALIGN, DMA_FROM_DEVICE);

ave_wdesc_addr(ndev, AVE_DESCID_RX, entry, 4, paddr);

I'll apply the code to next patch.

BTW, since the Tx buffer address doesn't have any restrictions, the adjustment
like this isn't necessary.


> > > +
> > > + /* enable clock */
> > > + priv->clk = devm_clk_get(dev, NULL);
> > > + if (IS_ERR(priv->clk))
> > > + priv->clk = NULL;
> > > + clk_prepare_enable(priv->clk);
> > 
> > Same here with the clock, the block is clocked, so it can consume some
> > amount of power, just do the necessary HW initialization with the clock
> > enabled, then defer until ndo_open() before turning it back on.

There are a number of the functions that needs clock enabled and "block reset"
operations, like mdiobus_register(), phy_connect(), and so on.

I tried to move such functions to ndo_open() to defer clock enabled until 
ndo_open().
However, the driver didn't work for some reasons of hardware restriction.
I think it's hard to change this sequence.

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next v2 1/2] net: phy: realtek: rename RTL8211F_PAGE_SELECT to RTL821x_PAGE_SELECT

[Kunihiko Hayashi]

This renames the definition of page select register from
RTL8211F_PAGE_SELECT to RTL821x_PAGE_SELECT to use it across models.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
---
Changes since v1:
 - new patch in this series
---
 drivers/net/phy/realtek.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 9cbe645..99c3297 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -22,11 +22,11 @@
 #define RTL821x_INER   0x12
 #define RTL821x_INER_INIT  0x6400
 #define RTL821x_INSR   0x13
+#define RTL821x_PAGE_SELECT0x1f
 #define RTL8211E_INER_LINK_STATUS 0x400
 
 #define RTL8211F_INER_LINK_STATUS 0x0010
 #define RTL8211F_INSR  0x1d
-#define RTL8211F_PAGE_SELECT   0x1f
 #define RTL8211F_TX_DELAY  0x100
 
 MODULE_DESCRIPTION("Realtek PHY driver");
@@ -46,10 +46,10 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
 {
int err;
 
-   phy_write(phydev, RTL8211F_PAGE_SELECT, 0xa43);
+   phy_write(phydev, RTL821x_PAGE_SELECT, 0xa43);
err = phy_read(phydev, RTL8211F_INSR);
/* restore to default page 0 */
-   phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
+   phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
 
return (err < 0) ? err : 0;
 }
@@ -102,7 +102,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
if (ret < 0)
return ret;
 
-   phy_write(phydev, RTL8211F_PAGE_SELECT, 0xd08);
+   phy_write(phydev, RTL821x_PAGE_SELECT, 0xd08);
reg = phy_read(phydev, 0x11);
 
/* enable TX-delay for rgmii-id and rgmii-txid, otherwise disable it */
@@ -114,7 +114,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
 
phy_write(phydev, 0x11, reg);
/* restore to default page 0 */
-   phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
+   phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
 
return 0;
 }
-- 
2.7.4

[PATCH net-next v2 2/2] net: phy: realtek: add RTL8201F phy-id and functions

[Kunihiko Hayashi]

From: Jassi Brar <jaswinder.si...@linaro.org>

Add RTL8201F phy-id and the related functions to the driver.

The original patch is as follows:
https://patchwork.kernel.org/patch/2538341/

Signed-off-by: Jongsung Kim <neidhard@lge.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Reviewed-by: Andrew Lunn <and...@lunn.ch>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
---
Changes since v1:
 - use RTL821x_PAGE_SELECT instead of defining RTL8201F_PAGE_SELECT
---
 drivers/net/phy/realtek.c | 44 
 1 file changed, 44 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 99c3297..d4670ec 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -29,10 +29,22 @@
 #define RTL8211F_INSR  0x1d
 #define RTL8211F_TX_DELAY  0x100
 
+#define RTL8201F_ISR   0x1e
+#define RTL8201F_IER   0x13
+
 MODULE_DESCRIPTION("Realtek PHY driver");
 MODULE_AUTHOR("Johnson Leung");
 MODULE_LICENSE("GPL");
 
+static int rtl8201_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_ISR);
+
+   return (err < 0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -54,6 +66,25 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
 }
 
+static int rtl8201_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   /* switch to page 7 */
+   phy_write(phydev, RTL821x_PAGE_SELECT, 0x7);
+
+   if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_IER,
+   BIT(13) | BIT(12) | BIT(11));
+   else
+   err = phy_write(phydev, RTL8201F_IER, 0);
+
+   /* restore to default page 0 */
+   phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -129,6 +160,18 @@ static struct phy_driver realtek_drvs[] = {
.config_aneg= _config_aneg,
.read_status= _read_status,
}, {
+   .phy_id = 0x001cc816,
+   .name   = "RTL8201F 10/100Mbps Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   }, {
.phy_id = 0x001cc912,
.name   = "RTL8211B Gigabit Ethernet",
.phy_id_mask= 0x001f,
@@ -181,6 +224,7 @@ static struct phy_driver realtek_drvs[] = {
 module_phy_driver(realtek_drvs);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc914, 0x001f },
{ 0x001cc915, 0x001f },
-- 
2.7.4

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

Hi Andrew,

On Mon, 11 Sep 2017 14:00:09 +0200 Andrew Lunn <and...@lunn.ch> wrote:

> > > > +static irqreturn_t ave_interrupt(int irq, void *netdev)
> > > > +{
> > > > +   struct net_device *ndev = (struct net_device *)netdev;
> > > > +   struct ave_private *priv = netdev_priv(ndev);
> > > > +   u32 gimr_val, gisr_val;
> > > > +
> > > > +   gimr_val = ave_irq_disable_all(ndev);
> > > > +
> > > > +   /* get interrupt status */
> > > > +   gisr_val = ave_r32(ndev, AVE_GISR);
> > > > +
> > > > +   /* PHY */
> > > > +   if (gisr_val & AVE_GI_PHY) {
> > > > +   ave_w32(ndev, AVE_GISR, AVE_GI_PHY);
> > > > +   if (priv->internal_phy_interrupt)
> > > > +   phy_mac_interrupt(ndev->phydev, 
> > > > ndev->phydev->link);
> > > 
> > > Humm. I don't think this is correct. You are supposed to give it the
> > > new link state, not the old.
> > > 
> > > What does a PHY interrupt mean here? 
> > 
> > In the general case, I think PHY events like changing link state are 
> > transmitted
> > to CPU as interrupt via interrupt controller, then PHY driver itself can 
> > handle
> > the interrupt.
> > 
> > And in this case, PHY events are transmitted to MAC as one of its interrupt 
> > factor,
> > then I thought that MAC driver had to tell the events to PHY.
> 
> Could this be in-band SGMI signalling from the PHY to the MAC? Does
> the documentation give examples of when this interrupt will happen?
> 
> Andrew

Unfortunately this MAC doesn't support SGMII.
And there aren't any examples of when this interrupt will happen.
This interrupt happens after ave_open() is called and link is established.

However, I found that auto negotiation didn't start when this interrupt wasn't 
handled.

Although ave_init() calls phy_start_aneg(), it doesn't make sense
because phy driver doesn't start yet.

And according to Florian's comment in ave_init(),

> + phy_start_interrupts(phydev);
> +
> + phy_start_aneg(phydev);
>
> No, no, phy_start() would take care of all of that correctly for you,
> you don't have have to do it just there because ave_open() eventually
> calls phy_start() for you, so just drop these two calls.

When moving phy_start_aneg() to ave_open(), the handler doesn't need to call
phy_mac_interrupt() and I can remove it from the handler.

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions

[Kunihiko Hayashi]

Hi Andrew,
Thank your for reviewing.

On Sat, 9 Sep 2017 17:55:17 +0200 <and...@lunn.ch> wrote:

> On Sat, Sep 09, 2017 at 09:03:05AM +0530, Jassi Brar wrote:
> > On 9 September 2017 at 00:21, Florian Fainelli <f.faine...@gmail.com> wrote:
> > > On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> > >> From: Jassi Brar <jaswinder.si...@linaro.org>
> > >>
> > >> Add RTL8201F phy-id and the related functions to the driver.
> > >>
> > >> The original patch is as follows:
> > >> https://patchwork.kernel.org/patch/2538341/
> > >>
> > >> Signed-off-by: Jongsung Kim <neidhard@lge.com>
> > >> Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> > >> Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > >> ---
> > >>  drivers/net/phy/realtek.c | 45 
> > >> +
> > >>  1 file changed, 45 insertions(+)
> > >>
> > >> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> > >> index 9cbe645..d9974ce 100644
> > >> --- a/drivers/net/phy/realtek.c
> > >> +++ b/drivers/net/phy/realtek.c
> > >> @@ -29,10 +29,23 @@
> > >>  #define RTL8211F_PAGE_SELECT 0x1f
> > >>  #define RTL8211F_TX_DELAY0x100
> > >>
> > >> +#define RTL8201F_ISR 0x1e
> > >> +#define RTL8201F_PAGE_SELECT 0x1f
> > >
> > > We have a page select register define for the RTL8211F right above, so
> > > surely we can make that a common definition?
> > >
> > That is just for the sake of consistency.
> > I mean RTL8211 wouldn't look neat among everything else RTL8201.
> > 
> > Also the page-select offsets just _happen_ to be same value...
> 
> If you look at all the other supported PHYs, they all consistently use
> the same page register across models. Marvell is always 22, mscc is
> always 31, vitesse is always 31.
> 
> I would say it is a safe bet that all realtek PHYs will use 0x1f for
> page select. So please add a patch which renames RTL8211F_PAGE_SELECT
> to RTL821x_PAGE_SELECT.
> 
> It is best to do this now. I spent a while cleaning up the mess the
> Marvell driver had got into with its page select code. Lots of
> duplicate code and defines doing the same thing.
> 
> Andrew

I see. In case of renaming to RTL821x_PAGE_SELECT, 
I think that I'll make a patch series as realtek PHY series including this
patch independent from the series of MAC driver.

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next 1/3] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

Hi Florian,

On Fri, 8 Sep 2017 11:54:31 -0700 <f.faine...@gmail.com> wrote:

> On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> > DT bindings for the AVE ethernet controller found on Socionext's
> > UniPhier platforms.
> > 
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> > ---
> >  .../bindings/net/socionext,uniphier-ave4.txt   | 44 
> > ++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
> > b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > new file mode 100644
> > index 000..57ae96d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> > @@ -0,0 +1,44 @@
> > +* Socionext AVE ethernet controller
> > +
> > +This describes the devicetree bindings for AVE ethernet controller
> > +implemented on Socionext UniPhier SoCs.
> > +
> > +Required properties:
> > + - compatible:  Should be "socionext,uniphier-ave4"
> > + - reg: Address where registers are mapped and size of region.
> > + - interrupts: IRQ common for mac and phy interrupts.
> > + - phy-mode: See ethernet.txt in the same directory.
> > + - #address-cells: Must be <1>.
> > + - #size-cells: Must be <0>.
> > + - Node, with 'reg' property, for each PHY on the MDIO bus.
> > +
> > +Optional properties:
> > + - socionext,desc-bits: 32/64 descriptor size. Default 32.
> 
> 32 bits, this has an unit. Why is this configurable through Device Tree
> and if this is describing the HW, as it should, why is not that inferred
> from the compatible string?

I've thought the common compatible string was reasonable for all UniPhier
SoCs. However, since there are some different features among SoCs
like "desc-bits" etc. and such feature would increase from now on,
it's convenient to add compatible strings for each SoCs.

> > + - local-mac-address: See ethernet.txt in the same directory.
> > + - pinctrl-names: List of assigned state names, see pinctrl
> > +   binding documentation.
> > + - pinctrl-0: List of phandles to configure the GPIO pin used
> > +   as interrupt line, see also generic and your platform
> > +   specific pinctrl binding documentation.
> > + - socionext,internal-phy-interrupt: Boolean to denote if the
> > +   PHY interrupt is internally handled by the MAC.
> 
> Interesting, why do you need to declare this as opposed to also
> determining this entirely from the compatible string of the Ethernet MAC?

This property was added after determining to use common compatible string.
I'll re-consider to use compatible strings for each SoCs.

> 
> > +
> > +
> > +Example:
> > +
> > +   eth: ethernet@6500 {
> > +   compatible = "socionext,uniphier-ave4";
> > +   reg = <0x6500 0x8500>;
> > +   interrupts = <0 66 4>;
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_ether_rgmii>;
> > +   phy-mode = "rgmii";
> > +   socionext,desc-bits = <64>;
> > +   local-mac-address = [00 00 00 00 00 00];
> > +
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> 
> There is no phy-handle property, yet there clearly seems to be one
> attached, and I think Andrew now also responded to that in the driver
> patch, this needs fixing.

Surely phy-handle is missing in this. I'll fix it with mdio node.

> > +   ethphy: ethphy@1 {
> > +   reg = <1>;
> > +   };
> > +   };
> > 
> 
> 
> -- 
> Florian

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next 1/3] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

Hi Andrew,

On Fri, 8 Sep 2017 16:03:22 +0200 <and...@lunn.ch> wrote:

> > +   eth: ethernet@6500 {
> > +   compatible = "socionext,uniphier-ave4";
> > +   reg = <0x6500 0x8500>;
> > +   interrupts = <0 66 4>;
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_ether_rgmii>;
> > +   phy-mode = "rgmii";
> > +   socionext,desc-bits = <64>;
> > +   local-mac-address = [00 00 00 00 00 00];
> > +
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> 
> > +   ethphy: ethphy@1 {
> > +   reg = <1>;
> > +   };
> 
> 
> So you normally have an mdio node, and the phy as a children of that
> node.
> 
>mdio {
> ethphy: ethernet-phy@6 {
> reg = <6>;
> };
> };
> 
>   Andrew

I see. I'll rewrite the binding example using mdio node.

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

Hi Florian,

On Sat, 9 Sep 2017 09:30:58 -0700 <f.faine...@gmail.com> wrote:

> 
> 
> On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> > The UniPhier platform from Socionext provides the AVE ethernet
> > controller that includes MAC and MDIO bus supporting RGMII/RMII
> > modes. The controller is named AVE.
> > 
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> > ---
> 
> [snip]
> 
> > +static int ave_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> > +{
> > +   struct ave_private *priv = netdev_priv(ndev);
> > +   u32 proc_idx, done_idx, ndesc, cmdsts;
> > +   int freepkt;
> > +   unsigned char *buffptr = NULL; /* buffptr for descriptor */
> > +   unsigned int len;
> > +   dma_addr_t paddr;
> > +
> > +   proc_idx = priv->tx.proc_idx;
> > +   done_idx = priv->tx.done_idx;
> > +   ndesc = priv->tx.ndesc;
> > +   freepkt = ((done_idx + ndesc - 1) - proc_idx) % ndesc;
> > +
> > +   /* not enough entry, then we stop queue */
> > +   if (unlikely(freepkt < 2)) {
> > +   netif_stop_queue(ndev);
> > +   if (unlikely(freepkt < 1))
> > +   return NETDEV_TX_BUSY;
> 
> This looks wrong, why are you checking first for less than 2
> descriptors, and if there is none, NETDEV_TX_BUSY? If you need 2 slots
> to complete a transmision, stop the transmit queue and return
> NETDEV_TX_BUSY.

This code is misleading and I have to fix this.
The device needs a slot to complete a transmission.

> > +   }
> > +
> > +   priv->tx.desc[proc_idx].skbs = skb;
> > +
> > +   /* add padding for short packet */
> > +   if (skb_padto(skb, ETH_ZLEN)) {
> > +   dev_kfree_skb_any(skb);
> > +   return NETDEV_TX_OK;
> > +   }
> 
> skb_padto() frees the SKB in case of error, that would lead to a double
> free here.

Ah, it occurs double free. I'll fix it.

> > +
> > +   buffptr = skb->data - NET_IP_ALIGN;
> > +   len = max_t(unsigned int, ETH_ZLEN, skb->len);
> 
> If you use skb_put_padto() if padding was necessary skb->len will be at
> least ETH_ZLEN, so you can remove this.

I see. It's reasonable.

> > +
> > +   paddr = ave_dma_map(ndev, >tx.desc[proc_idx], buffptr,
> > +   len + NET_IP_ALIGN, DMA_TO_DEVICE);
> 
> As mentioned before you can't assume this will never fail.

Okay, I'll rewrite it in consideration of error case.

> > +   paddr += NET_IP_ALIGN;
> > +
> > +   /* set buffer address to descriptor */
> > +   ave_wdesc_addr(ndev, AVE_DESCID_TX, proc_idx, 4, paddr);
> 
> Also mentioned in the other email, make this 4 a constant so we know
> it's an offset and not a length.

I see.

> > +
> > +   /* set flag and length to send */
> > +   cmdsts = AVE_STS_OWN | AVE_STS_1ST | AVE_STS_LAST
> > +   | (len & AVE_STS_PKTLEN_TX);
> 
> AVE_STS_PKTLEN_TX would be better named with a _MASK suffix.

Yes.

> > +
> > +   /* set interrupt per AVE_FORCE_TXINTCNT or when queue is stopped */
> > +   if (!(proc_idx % AVE_FORCE_TXINTCNT) || netif_queue_stopped(ndev))
> > +   cmdsts |= AVE_STS_INTR;
> > +
> > +   /* disable checksum calculation when skb doesn't calurate checksum */
> > +   if (skb->ip_summed == CHECKSUM_NONE ||
> > +   skb->ip_summed == CHECKSUM_UNNECESSARY)
> > +   cmdsts |= AVE_STS_NOCSUM;
> > +
> > +   /* set cmdsts */
> > +   ave_wdesc(ndev, AVE_DESCID_TX, proc_idx, 0, cmdsts);
> > +
> > +   priv->tx.proc_idx = (proc_idx + 1) % ndesc;
> 
> You should also check the ring space after transmission and assert flow
> control on the transmit queue if needed.

Okay, I'll add this.

> > +
> > +   return NETDEV_TX_OK;
> > +}
> 
> [snip]
> 
> > +static struct net_device_stats *ave_stats(struct net_device *ndev)
> > +{
> > +   struct ave_private *priv = netdev_priv(ndev);
> > +   u32 drop_num = 0;
> > +
> > +   priv->stats.rx_errors = ave_r32(ndev, AVE_BFCR);
> > +
> > +   drop_num += ave_r32(ndev, AVE_RX0OVFFC);
> > +   drop_num += ave_r32(ndev, AVE_SN5FC);
> > +   drop_num += ave_r32(ndev, AVE_SN6FC);
> > +   drop_num += ave_r32(ndev, AVE_SN7FC);
> > +   priv->stats.rx_dropped = drop_num;
> > +
> 
> You should consider switching to 64-bit statistics, this requires a
> little bit more work for 32-bit hosts (see
> include/linux/u64_stats_sync.h) but this allows you to keep statistics
> around above 4GB.

I see.
I'll refer to this header and its examples, and rewrite it to be suitable
for 32-bit and 64-bit hosts.

> 
> > +   return >stats;
> > +}
> > +-- 
> Florian

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

Hi Florian,
Thank you for your comments,

On Fri, 8 Sep 2017 12:31:18 -0700 <f.faine...@gmail.com> wrote:

> On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> > The UniPhier platform from Socionext provides the AVE ethernet
> > controller that includes MAC and MDIO bus supporting RGMII/RMII
> > modes. The controller is named AVE.
> > 
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
> > ---

..snip..

> > +static inline u32 ave_r32(struct net_device *ndev, u32 offset)
> > +{
> > +   void __iomem *addr = (void __iomem *)ndev->base_addr;
> > +
> > +   return readl_relaxed(addr + offset);
> > +}
> 
> Don't do this, ndev->base_addr is convenient, but is now deprecated
> (unlike ISA cards, you can't change this anymore) instead, just pass a
> reference to an ave_private structure and store the base register
> pointer there.

Oh, I didn't notice that ndev->base_addr was deprecated.
I'll rewrite it using an ave_private structure.

> > +
> > +static inline void ave_w32(struct net_device *ndev, u32 offset, u32 value)
> > +{
> > +   void __iomem *addr = (void __iomem *)ndev->base_addr;
> > +
> > +   writel_relaxed(value, addr + offset);
> > +}
> 
> Same here.

Ditto.

> > +
> > +static inline u32 ave_rdesc(struct net_device *ndev, enum desc_id id,
> > +   int entry, int offset)
> > +{
> > +   struct ave_private *priv = netdev_priv(ndev);
> > +   u32 addr = (id == AVE_DESCID_TX) ? priv->tx.daddr : priv->rx.daddr;
> > +
> > +   addr += entry * priv->desc_size + offset;
> > +
> > +   return ave_r32(ndev, addr);
> > +}
> > +
> > +static inline void ave_wdesc(struct net_device *ndev, enum desc_id id,
> > +int entry, int offset, u32 val)
> > +{
> > +   struct ave_private *priv = netdev_priv(ndev);
> > +   u32 addr = (id == AVE_DESCID_TX) ? priv->tx.daddr : priv->rx.daddr;
> > +
> > +   addr += entry * priv->desc_size + offset;
> > +
> > +   ave_w32(ndev, addr, val);
> > +}
> > +
> > +static void ave_wdesc_addr(struct net_device *ndev, enum desc_id id,
> > +  int entry, int offset, dma_addr_t paddr)
> > +{
> > +   struct ave_private *priv = netdev_priv(ndev);
> > +
> > +   ave_wdesc(ndev, id, entry, offset, (u32)((u64)paddr & 0xULL));
> 
> lower_32_bits()

It's convenient.

> > +   if (IS_DESC_64BIT(priv))
> > +   ave_wdesc(ndev, id,
> > + entry, offset + 4, (u32)((u64)paddr >> 32));
> 
> upper_32_bits()

Ditto.

> > +   else if ((u64)paddr > (u64)U32_MAX)
> > +   netdev_warn(ndev, "DMA address exceeds the address space\n");
> > +}
> > +
> > +static void ave_get_fwversion(struct net_device *ndev, char *buf, int len)
> > +{
> > +   u32 major, minor;
> > +
> > +   major = (ave_r32(ndev, AVE_VR) & GENMASK(15, 8)) >> 8;
> > +   minor = (ave_r32(ndev, AVE_VR) & GENMASK(7, 0));
> > +   snprintf(buf, len, "v%u.%u", major, minor);
> > +}
> > +
> > +static void ave_get_drvinfo(struct net_device *ndev,
> > +   struct ethtool_drvinfo *info)
> > +{
> > +   struct device *dev = ndev->dev.parent;
> > +
> > +   strlcpy(info->driver, dev->driver->name, sizeof(info->driver));
> > +   strlcpy(info->bus_info, dev_name(dev), sizeof(info->bus_info));
> 
> bus_info would likely be platform, since this is a memory mapped
> peripheral, right?

Yes, this is a memory mapped peripheral.
Now ethtool says "bus-info: 6500.ethernet" in our case.
Is it reasonable for bus-info? or is null desirable?

> > +   ave_get_fwversion(ndev, info->fw_version, sizeof(info->fw_version));
> > +}
> > +
> > +static int ave_nway_reset(struct net_device *ndev)
> > +{
> > +   return genphy_restart_aneg(ndev->phydev);
> > +}
> 
> You can just set your ethtool_ops::nway_reset to be
> phy_ethtool_nway_reset() which does additional checks.

I see. I'll use it.

> > +
> > +static u32 ave_get_link(struct net_device *ndev)
> > +{
> > +   int err;
> > +
> > +   err = genphy_update_link(ndev->phydev);
> > +   if (err)
> > +   return ethtool_op_get_link(ndev);
> 
> No, calling genphy_update_link() is bogus see:
> 
> e69e46261063a25c3907bed16a2e9d18b115d1fd ("net: dsa: Do not clobber PHY
> link outside of state machine")

You mean that

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

Hi Yamada-san,
Thank you for your comments,

On Fri, 8 Sep 2017 23:44:13 +0900 <yamada.masah...@socionext.com> wrote:

> 2017-09-08 22:02 GMT+09:00 Kunihiko Hayashi <hayashi.kunih...@socionext.com>:
> 
> > diff --git a/drivers/net/ethernet/socionext/Kconfig 
> > b/drivers/net/ethernet/socionext/Kconfig
> > new file mode 100644
> > index 000..788f26f
> > --- /dev/null
> > +++ b/drivers/net/ethernet/socionext/Kconfig
> > @@ -0,0 +1,22 @@
> > +config NET_VENDOR_SOCIONEXT
> > +   bool "Socionext ethernet drivers"
> > +   default y
> > +   ---help---
> > + Option to select ethernet drivers for Socionext platforms.
> > +
> > + Note that the answer to this question doesn't directly affect the
> > + kernel: saying N will just cause the configurator to skip all
> > + the questions about Agere devices. If you say Y, you will be asked
> > + for your specific card in the following questions.
> 
> 
> Agere?

It's wrong. I have template strings left.
I'll fix it.

> > +
> > +   dev_info(dev, "Socionext %c%c%c%c Ethernet IP %s (irq=%d, 
> > phy=%s)\n",
> > +(ave_id >> 24) & 0xff, (ave_id >> 16) & 0xff,
> > +(ave_id >> 8) & 0xff, (ave_id >> 0) & 0xff,
> > +buf, ndev->irq, phy_modes(phy_mode));
> > +
> > +   return 0;
> > +err_netdev_register:
> 
> Maybe, a bad label name.
> for ex. out_del_napi or whatever.
> 
> Documentation/process/coding-style.rst says
> "Choose label names which say what the goto does ..."

Yes, I'll rename it according to the document.

> > +   netif_napi_del(>napi_rx);
> > +   netif_napi_del(>napi_tx);
> > +   mdiobus_unregister(priv->mdio);
> > +err_mdiobus_register:
> > +err_mdiobus_alloc:
> > +err_req_irq:
> 
> These three should be merged, for ex.
> out_free_device.

I see. These will be merged.

> I ran sparse for you.
> 
> Please take a look if it is worthwhile.
> Seems endianess around mac_addr.

Okay, I'll check the suspicious code.

> drivers/net/ethernet/socionext/sni_ave.c:423:15: warning: cast to
> restricted __le32
> drivers/net/ethernet/socionext/sni_ave.c:425:20: warning: cast to
> restricted __le16
> drivers/net/ethernet/socionext/sni_ave.c:1194:15: warning: cast to
> restricted __le32
> drivers/net/ethernet/socionext/sni_ave.c:1196:20: warning: cast to
> restricted __le16
> drivers/net/ethernet/socionext/sni_ave.c:1398:15: warning: cast to
> restricted __le32
> drivers/net/ethernet/socionext/sni_ave.c:1400:20: warning: cast to
> restricted __le16
> 
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

private *priv = netdev_priv(ndev);
> > +   struct device *dev = ndev->dev.parent;
> > +   struct device_node *phy_node, *np = dev->of_node;
> > +   struct phy_device *phydev;
> > +   const void *mac_addr;
> > +   u32 supported;
> > +
> > +   /* get mac address */
> > +   mac_addr = of_get_mac_address(np);
> > +   if (mac_addr)
> > +   ether_addr_copy(ndev->dev_addr, mac_addr);
> > +
> > +   /* if the mac address is invalid, use random mac address */
> > +   if (!is_valid_ether_addr(ndev->dev_addr)) {
> > +   eth_hw_addr_random(ndev);
> > +   dev_warn(dev, "Using random MAC address: %pM\n",
> > +ndev->dev_addr);
> > +   }
> > +
> > +   /* attach PHY with MAC */
> > +   phy_node =  of_get_next_available_child(np, NULL);
> 
> ???
> 
> Should this not be looking for a phy-handle property?
> Documentation/devicetree/binds/net/ethernet.txt:
> 
> - phy-handle: phandle, specifies a reference to a node representing a PHY
>   device; this property is described in the Devicetree Specification and so
>   preferred;

Yes, I found it was wrong.
The device node has not a child node but phy-handle to specify PHY.

> > +   phydev = of_phy_connect(ndev, phy_node,
> > +   ave_adjust_link, 0, priv->phy_mode);
> > +   if (!phydev) {
> > +   dev_err(dev, "could not attach to PHY\n");
> > +   return -ENODEV;
> > +   }
> > +   of_node_put(phy_node);
> > +
> > +   priv->phydev = phydev;
> > +   phydev->autoneg = AUTONEG_ENABLE;
> > +   phydev->speed = 0;
> > +   phydev->duplex = 0;
> 
> And this should not be needed.

I understand that these parameters have been initialized.

> > +
> > +   dev_info(dev, "connected to %s phy with id 0x%x\n",
> > +phydev->drv->name, phydev->phy_id);
> 
> phy_attached_info()

It's convernient.

> > +
> > +   if (priv->phy_mode != PHY_INTERFACE_MODE_RGMII) {
> 
> Same comment as above.

Ditto.

> > +   supported = phydev->supported;
> > +   phydev->supported &= ~PHY_GBIT_FEATURES;
> > +   phydev->supported |= supported & PHY_BASIC_FEATURES;
> > +   }
> > +
> > +   /* PHY interrupt stop instruction is needed because the interrupt
> > +* continues to assert.
> > +*/
> > +   phy_stop_interrupts(phydev);
> 
> Could you explain this some more? It sounds like your interrupt
> controller is broken.

I thought that the driver had to stop the own interrupt of PHY device
because it might be already enabled.
But surely we don't think of such a case.

> > +
> > +   /* When PHY driver can't handle its interrupt directly,
> > +* interrupt request always fails and polling method is used
> > +* alternatively. In this case, the libphy says
> > +* "libphy: uniphier-mdio: Can't get IRQ -1 (PHY)".
> > +*/
> > +   phy_start_interrupts(phydev);
> 
> -1 is PHY_POLL. So calling phy_start_interrupts() is wrong. In fact,
> you should not be calling phy_start_interrupts() at all. No other
> Ethernet driver does.

The phy_start_interrupts() calls request_threaded_irq(), and it wll succeed
when PHY node has own 'interrupts' property.
In this case, it will fail and phydev->irq sets PHY_POLL.

However, phydev->irq is initialized PHY_POLL in phy_probe(),
surely it isn't necessary.

> > +
> > +   phy_start_aneg(phydev);
> > +
> > +   return 0;
> > +}
> > +
> > +static void ave_uninit(struct net_device *ndev)
> > +{
> > +   struct ave_private *priv = netdev_priv(ndev);
> > +
> > +   phy_stop_interrupts(priv->phydev);
> 
> And no other Ethernet driver calls phy_stop_interrupts either.
> Please take a look at this.

I see. I'll check it.

> 
> > +   phy_disconnect(priv->phydev);
> > +}
> > +
> 
>   Andrew

---
Best Regards,
Kunihiko Hayashi

[PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |4 +
 drivers/net/ethernet/socionext/sni_ave.c | 1618 ++
 5 files changed, 1646 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index a0a03d4..9f55b36 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..788f26f
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Agere devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..0356341
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..c870777
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1618 @@
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ *
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x0 /* ID */
+#define AVE_VR 0x4 /* Version */
+#define AVE_GRR0x8 /* Global Reset */
+#define AVE_CFGR   0xc /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control */
+#define AVE_MDIOA

[PATCH net-next 0/3] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

Furthermore, this series includes support for realtek RTL8201F PHY to be
implemented on some supported boards.

Jassi Brar (1):
  net: phy: realtek: add RTL8201F phy-id and functions

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   44 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|4 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1618 
 drivers/net/phy/realtek.c  |   45 +
 7 files changed, 1735 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

[PATCH net-next 1/3] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 44 ++
 1 file changed, 44 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..57ae96d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,44 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible:  Should be "socionext,uniphier-ave4"
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: IRQ common for mac and phy interrupts.
+ - phy-mode: See ethernet.txt in the same directory.
+ - #address-cells: Must be <1>.
+ - #size-cells: Must be <0>.
+ - Node, with 'reg' property, for each PHY on the MDIO bus.
+
+Optional properties:
+ - socionext,desc-bits: 32/64 descriptor size. Default 32.
+ - local-mac-address: See ethernet.txt in the same directory.
+ - pinctrl-names: List of assigned state names, see pinctrl
+   binding documentation.
+ - pinctrl-0: List of phandles to configure the GPIO pin used
+   as interrupt line, see also generic and your platform
+   specific pinctrl binding documentation.
+ - socionext,internal-phy-interrupt: Boolean to denote if the
+   PHY interrupt is internally handled by the MAC.
+
+
+Example:
+
+   eth: ethernet@6500 {
+   compatible = "socionext,uniphier-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_ether_rgmii>;
+   phy-mode = "rgmii";
+   socionext,desc-bits = <64>;
+   local-mac-address = [00 00 00 00 00 00];
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
-- 
2.7.4

[PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions

[Kunihiko Hayashi]

From: Jassi Brar <jaswinder.si...@linaro.org>

Add RTL8201F phy-id and the related functions to the driver.

The original patch is as follows:
https://patchwork.kernel.org/patch/2538341/

Signed-off-by: Jongsung Kim <neidhard@lge.com>
Signed-off-by: Jassi Brar <jaswinder.si...@linaro.org>
Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
---
 drivers/net/phy/realtek.c | 45 +
 1 file changed, 45 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 9cbe645..d9974ce 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -29,10 +29,23 @@
 #define RTL8211F_PAGE_SELECT   0x1f
 #define RTL8211F_TX_DELAY  0x100
 
+#define RTL8201F_ISR   0x1e
+#define RTL8201F_PAGE_SELECT   0x1f
+#define RTL8201F_IER   0x13
+
 MODULE_DESCRIPTION("Realtek PHY driver");
 MODULE_AUTHOR("Johnson Leung");
 MODULE_LICENSE("GPL");
 
+static int rtl8201_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_ISR);
+
+   return (err < 0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -54,6 +67,25 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
 }
 
+static int rtl8201_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   /* switch to page 7 */
+   phy_write(phydev, RTL8201F_PAGE_SELECT, 0x7);
+
+   if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_IER,
+   BIT(13) | BIT(12) | BIT(11));
+   else
+   err = phy_write(phydev, RTL8201F_IER, 0);
+
+   /* restore to default page 0 */
+   phy_write(phydev, RTL8201F_PAGE_SELECT, 0x0);
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -129,6 +161,18 @@ static struct phy_driver realtek_drvs[] = {
.config_aneg= _config_aneg,
.read_status= _read_status,
}, {
+   .phy_id = 0x001cc816,
+   .name   = "RTL8201F 10/100Mbps Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   }, {
.phy_id = 0x001cc912,
.name   = "RTL8211B Gigabit Ethernet",
.phy_id_mask= 0x001f,
@@ -181,6 +225,7 @@ static struct phy_driver realtek_drvs[] = {
 module_phy_driver(realtek_drvs);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc914, 0x001f },
{ 0x001cc915, 0x001f },
-- 
2.7.4