[PATCH 0/5] ks8851 DT updates

[PATCH 0/5] ks8851 DT updates

[Stephen Boyd]

This set of patches properly documents the micrel ks8851 spi ethernet
controller, converts to devm_regulator_get_optional() to make error
paths slightly simpler, and finally adds supports for another
optional regulator and a reset gpio.

Stephen Boyd (5):
  devicetree: bindings: Document micrel vendor prefix
  devicetree: bindings: Properly document micrel ks8851 SPI chips
  net: ks8851: Use devm_regulator_get_optional()
  net: ks8851: Add optional vdd_io regulator and reset gpio
  net: ks8851: Add of match table

 .../devicetree/bindings/net/micrel-ks8851.txt      | 15 ++++-
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 drivers/net/ethernet/micrel/ks8851.c               | 71 +++++++++++++++++++---
 3 files changed, 75 insertions(+), 12 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 1/5] devicetree: bindings: Document micrel vendor prefix

[Stephen Boyd]

There's one existing use of 'micrel' in the documentation so use
'micrel' instead of the company's ticker symbol 'mcrl'.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

This is mostly here as the first patch to make checkpatch quiet. I expect DT
maintainers to pick this one up.

 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 1bc2174e1a05..2fe06ad1d248 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -76,6 +76,7 @@ linux	Linux-specific binding
 lsi	LSI Corp. (LSI Logic)
 marvell	Marvell Technology Group Ltd.
 maxim	Maxim Integrated Products
+micrel	Micrel Inc.
 microchip	Microchip Technology Inc.
 mosaixtech	Mosaix Technologies, Inc.
 moxa	Moxa
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 1/5] devicetree: bindings: Document micrel vendor prefix

[Rob Herring]

On Thu, May 22, 2014 at 4:00 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> There's one existing use of 'micrel' in the documentation so use
> 'micrel' instead of the company's ticker symbol 'mcrl'.
>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

Applied for 3.16.

Rob

> ---
>
> This is mostly here as the first patch to make checkpatch quiet. I expect DT
> maintainers to pick this one up.
>
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 1bc2174e1a05..2fe06ad1d248 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -76,6 +76,7 @@ linux Linux-specific binding
>  lsi    LSI Corp. (LSI Logic)
>  marvell        Marvell Technology Group Ltd.
>  maxim  Maxim Integrated Products
> +micrel Micrel Inc.
>  microchip      Microchip Technology Inc.
>  mosaixtech     Mosaix Technologies, Inc.
>  moxa   Moxa
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>

[PATCH 2/5] devicetree: bindings: Properly document micrel ks8851 SPI chips

[Stephen Boyd]

The ks8851 SPI ethernet wasn't documented, but we documented the
optional regulator supply for it under the mll based ethernet
chip. Furthermore, that compatible string needed another 'l'. Fix
all of this and document the newly added optional vdd-io and
reset-gpios properties.

Cc: Nishanth Menon <nm@ti.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 Documentation/devicetree/bindings/net/micrel-ks8851.txt | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/micrel-ks8851.txt b/Documentation/devicetree/bindings/net/micrel-ks8851.txt
index d54d0cc79487..bbdf9a7359a2 100644
--- a/Documentation/devicetree/bindings/net/micrel-ks8851.txt
+++ b/Documentation/devicetree/bindings/net/micrel-ks8851.txt
@@ -1,9 +1,18 @@
-Micrel KS8851 Ethernet mac
+Micrel KS8851 Ethernet mac (MLL)
 
 Required properties:
-- compatible = "micrel,ks8851-ml" of parallel interface
+- compatible = "micrel,ks8851-mll" of parallel interface
 - reg : 2 physical address and size of registers for data and command
 - interrupts : interrupt connection
 
+Micrel KS8851 Ethernet mac (SPI)
+
+Required properties:
+- compatible = "micrel,ks8851" or the deprecated "ks8851"
+- reg : chip select number
+- interrupts : interrupt connection
+
 Optional properties:
-- vdd-supply:	supply for Ethernet mac
+- vdd-supply: analog 3.3V supply for Ethernet mac
+- vdd-io-supply: digital 1.8V IO supply for Ethernet mac
+- reset-gpios: reset_n input pin
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 3/5] net: ks8851: Use devm_regulator_get_optional()

[Stephen Boyd]

This simplifies error paths and removes the need to
regulator_put().

Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/net/ethernet/micrel/ks8851.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index 13767eb36a48..f2bfc708880c 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -1417,7 +1417,7 @@ static int ks8851_probe(struct spi_device *spi)
 	ks->spidev = spi;
 	ks->tx_space = 6144;
 
-	ks->vdd_reg = regulator_get_optional(&spi->dev, "vdd");
+	ks->vdd_reg = devm_regulator_get_optional(&spi->dev, "vdd");
 	if (IS_ERR(ks->vdd_reg)) {
 		ret = PTR_ERR(ks->vdd_reg);
 		if (ret == -EPROBE_DEFER)
@@ -1427,7 +1427,7 @@ static int ks8851_probe(struct spi_device *spi)
 		if (ret) {
 			dev_err(&spi->dev, "regulator enable fail: %d\n",
 				ret);
-			goto err_reg_en;
+			goto err_reg;
 		}
 	}
 
@@ -1530,9 +1530,6 @@ err_irq:
 err_id:
 	if (!IS_ERR(ks->vdd_reg))
 		regulator_disable(ks->vdd_reg);
-err_reg_en:
-	if (!IS_ERR(ks->vdd_reg))
-		regulator_put(ks->vdd_reg);
 err_reg:
 	free_netdev(ndev);
 	return ret;
@@ -1547,10 +1544,8 @@ static int ks8851_remove(struct spi_device *spi)
 
 	unregister_netdev(priv->netdev);
 	free_irq(spi->irq, priv);
-	if (!IS_ERR(priv->vdd_reg)) {
+	if (!IS_ERR(priv->vdd_reg))
 		regulator_disable(priv->vdd_reg);
-		regulator_put(priv->vdd_reg);
-	}
 	free_netdev(priv->netdev);
 
 	return 0;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 4/5] net: ks8851: Add optional vdd_io regulator and reset gpio

[Stephen Boyd]

Allow the ks8851 driver to enable an optional 1.8V vdd_io
regulator and assert the reset pin to the phy if a reset gpio is
present in device tree.

Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/net/ethernet/micrel/ks8851.c | 54 +++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index f2bfc708880c..3a322daba5ff 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -26,6 +26,8 @@
 #include <linux/regulator/consumer.h>
 
 #include <linux/spi/spi.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
 
 #include "ks8851.h"
 
@@ -85,6 +87,8 @@ union ks8851_tx_hdr {
  * @eeprom_size: Companion eeprom size in Bytes, 0 if no eeprom
  * @eeprom: 93CX6 EEPROM state for accessing on-board EEPROM.
  * @vdd_reg:	Optional regulator supplying the chip
+ * @vdd_io: Optional digital power supply for IO
+ * @gpio: Optional reset_n gpio
  *
  * The @lock ensures that the chip is protected when certain operations are
  * in progress. When the read or write packet transfer is in progress, most
@@ -133,6 +137,8 @@ struct ks8851_net {
 
 	struct eeprom_93cx6	eeprom;
 	struct regulator	*vdd_reg;
+	struct regulator	*vdd_io;
+	int			gpio;
 };
 
 static int msg_enable;
@@ -1404,6 +1410,7 @@ static int ks8851_probe(struct spi_device *spi)
 	struct ks8851_net *ks;
 	int ret;
 	unsigned cider;
+	int gpio;
 
 	ndev = alloc_etherdev(sizeof(struct ks8851_net));
 	if (!ndev)
@@ -1417,6 +1424,37 @@ static int ks8851_probe(struct spi_device *spi)
 	ks->spidev = spi;
 	ks->tx_space = 6144;
 
+	gpio = of_get_named_gpio_flags(spi->dev.of_node, "reset-gpios",
+				       0, NULL);
+	if (gpio == -EPROBE_DEFER) {
+		ret = gpio;
+		goto err_gpio;
+	}
+
+	ks->gpio = gpio;
+	if (gpio_is_valid(gpio)) {
+		ret = devm_gpio_request_one(&spi->dev, gpio,
+					    GPIOF_OUT_INIT_LOW, "ks8851_rst_n");
+		if (ret) {
+			dev_err(&spi->dev, "reset gpio request failed\n");
+			goto err_gpio;
+		}
+	}
+
+	ks->vdd_io = devm_regulator_get_optional(&spi->dev, "vdd-io");
+	if (IS_ERR(ks->vdd_io)) {
+		ret = PTR_ERR(ks->vdd_io);
+		if (ret == -EPROBE_DEFER)
+			goto err_reg_io;
+	} else {
+		ret = regulator_enable(ks->vdd_io);
+		if (ret) {
+			dev_err(&spi->dev, "regulator vdd_io enable fail: %d\n",
+				ret);
+			goto err_reg_io;
+		}
+	}
+
 	ks->vdd_reg = devm_regulator_get_optional(&spi->dev, "vdd");
 	if (IS_ERR(ks->vdd_reg)) {
 		ret = PTR_ERR(ks->vdd_reg);
@@ -1425,12 +1463,16 @@ static int ks8851_probe(struct spi_device *spi)
 	} else {
 		ret = regulator_enable(ks->vdd_reg);
 		if (ret) {
-			dev_err(&spi->dev, "regulator enable fail: %d\n",
+			dev_err(&spi->dev, "regulator vdd enable fail: %d\n",
 				ret);
 			goto err_reg;
 		}
 	}
 
+	if (gpio_is_valid(gpio)) {
+		usleep_range(10000, 11000);
+		gpio_set_value(gpio, 1);
+	}
 
 	mutex_init(&ks->lock);
 	spin_lock_init(&ks->statelock);
@@ -1527,10 +1569,16 @@ err_netdev:
 	free_irq(ndev->irq, ks);
 
 err_irq:
+	if (gpio_is_valid(gpio))
+		gpio_set_value(gpio, 0);
 err_id:
 	if (!IS_ERR(ks->vdd_reg))
 		regulator_disable(ks->vdd_reg);
 err_reg:
+	if (!IS_ERR(ks->vdd_io))
+		regulator_disable(ks->vdd_io);
+err_reg_io:
+err_gpio:
 	free_netdev(ndev);
 	return ret;
 }
@@ -1544,8 +1592,12 @@ static int ks8851_remove(struct spi_device *spi)
 
 	unregister_netdev(priv->netdev);
 	free_irq(spi->irq, priv);
+	if (gpio_is_valid(priv->gpio))
+		gpio_set_value(priv->gpio, 0);
 	if (!IS_ERR(priv->vdd_reg))
 		regulator_disable(priv->vdd_reg);
+	if (!IS_ERR(priv->vdd_io))
+		regulator_disable(priv->vdd_io);
 	free_netdev(priv->netdev);
 
 	return 0;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 5/5] net: ks8851: Add of match table

[Stephen Boyd]

Users are currently just providing "ks8851" as the compatible for
this driver in device tree. Add a compatible string that provides
the vendor name along with the driver name to be more explicit.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/net/ethernet/micrel/ks8851.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index 3a322daba5ff..e72918970a58 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -1603,9 +1603,15 @@ static int ks8851_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ks8851_match_table[] = {
+	{ .compatible = "micrel,ks8851" },
+	{ }
+};
+
 static struct spi_driver ks8851_driver = {
 	.driver = {
 		.name = "ks8851",
+		.of_match_table = ks8851_match_table,
 		.owner = THIS_MODULE,
 		.pm = &ks8851_pm_ops,
 	},
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 0/5] ks8851 DT updates

[David Miller]

From: Stephen Boyd <sboyd@codeaurora.org>
Date: Thu, 22 May 2014 14:00:08 -0700

> This set of patches properly documents the micrel ks8851 spi ethernet
> controller, converts to devm_regulator_get_optional() to make error
> paths slightly simpler, and finally adds supports for another
> optional regulator and a reset gpio.

Please do not submit patch sets that you expect different trees to
integrate, this is painful for maintainers to deal with.

Instead, submit the changes separately.  One series for one
maintainer's tree, another for the other maintainer's tree.

Re: [PATCH 0/5] ks8851 DT updates

[Stephen Boyd]

On 05/23/14 12:15, David Miller wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
> Date: Thu, 22 May 2014 14:00:08 -0700
>
>> This set of patches properly documents the micrel ks8851 spi ethernet
>> controller, converts to devm_regulator_get_optional() to make error
>> paths slightly simpler, and finally adds supports for another
>> optional regulator and a reset gpio.
> Please do not submit patch sets that you expect different trees to
> integrate, this is painful for maintainers to deal with.
>
> Instead, submit the changes separately.  One series for one
> maintainer's tree, another for the other maintainer's tree.
>

Ok. Shall I resend this series with the proper separation? Or can you
ignore the first patch? There isn't any functional dependency, more just
a checkpatch dependency which is not really all that important.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 0/5] ks8851 DT updates

[David Miller]

From: Stephen Boyd <sboyd@codeaurora.org>
Date: Fri, 23 May 2014 12:34:01 -0700

> On 05/23/14 12:15, David Miller wrote:
>> From: Stephen Boyd <sboyd@codeaurora.org>
>> Date: Thu, 22 May 2014 14:00:08 -0700
>>
>>> This set of patches properly documents the micrel ks8851 spi ethernet
>>> controller, converts to devm_regulator_get_optional() to make error
>>> paths slightly simpler, and finally adds supports for another
>>> optional regulator and a reset gpio.
>> Please do not submit patch sets that you expect different trees to
>> integrate, this is painful for maintainers to deal with.
>>
>> Instead, submit the changes separately.  One series for one
>> maintainer's tree, another for the other maintainer's tree.
>>
> 
> Ok. Shall I resend this series with the proper separation? Or can you
> ignore the first patch? There isn't any functional dependency, more just
> a checkpatch dependency which is not really all that important.

To avoid all ambiguity please resubmit the new series.