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

2018-04-13 Thread Rob Herring
On Mon, Apr 09, 2018 at 03:38:43PM +0900, Kunihiko Hayashi wrote:
> 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 
> Signed-off-by: Kunihiko Hayashi 
> ---
>  .../bindings/net/socionext,uniphier-ave4.txt   |  13 ++-

Reviewed-by: Rob Herring 

>  drivers/net/ethernet/socionext/sni_ave.c   | 108 
> -
>  2 files changed, 96 insertions(+), 25 deletions(-)


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

2018-04-09 Thread 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 
Signed-off-by: Kunihiko Hayashi 
---
 .../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) {
-   dev_err(dev, "can't deassert reset\n");
-   goto out_clk_disable;
+
+   for (nr = 0; nr < priv->nrsts; nr++) {
+   ret = reset_control_deassert(priv->rst[nr]);
+   if (ret) {
+