[PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[Linus Walleij]

This kills off the platform data support from the bitbanged
GPIO-based MDIO driver and moves it over to using GPIO
descriptors exclusively.

We are certainly not going to merge any more platforms into
the kernel using platform data, and nothing is using it at the
moment. The only concern would be out-of-tree platforms, and
those are not the concern of the kernel community. They need
to move to use device tree (or ACPI etc) like everyone else.

This was tested on the bit-banged GPIO MDIO on the D-Link
DNS-313 and works fine for me.

Linus Walleij (5):
  net: mdio-gpio: Localize platform data
  net: mdio-gpio: Allocate state in probe()
  net: mdio-gpio: Remove non-DT probe path
  net: mdio-gpio: Merge platform data into state
  net: mdio-gpio: Move to gpiod API

 MAINTAINERS                             |   1 -
 drivers/net/phy/Kconfig                 |   2 +-
 drivers/net/phy/mdio-gpio.c             | 151 ++++++++++----------------------
 include/linux/platform_data/mdio-gpio.h |  33 -------
 4 files changed, 47 insertions(+), 140 deletions(-)
 delete mode 100644 include/linux/platform_data/mdio-gpio.h

-- 
2.14.3

[PATCH net-next 1/5] net: mdio-gpio: Localize platform data

[Linus Walleij]

It is late on the day for platforms using platform data to pass
information to drivers. As of today, the only thing in the kernel
including the <linux/platform_data/mdio-gpio.h> file is the MDIO
GPIO driver itself.

Essentially it is exposing a kernel-internal interface unused by
any in-kernel code.

Let's decomission this and make the MDIO GPIO driver more
self-contained by starting to move this struct into the driver.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 MAINTAINERS                             |  1 -
 drivers/net/phy/mdio-gpio.c             | 19 ++++++++++++++++++-
 include/linux/platform_data/mdio-gpio.h | 33 ---------------------------------
 3 files changed, 18 insertions(+), 35 deletions(-)
 delete mode 100644 include/linux/platform_data/mdio-gpio.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3bdc260e36b7..e5a1a06c09e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5333,7 +5333,6 @@ F:	include/linux/*mdio*.h
 F:	include/linux/of_net.h
 F:	include/linux/phy.h
 F:	include/linux/phy_fixed.h
-F:	include/linux/platform_data/mdio-gpio.h
 F:	include/linux/platform_data/mdio-bcm-unimac.h
 F:	include/trace/events/mdio.h
 F:	include/uapi/linux/mdio.h
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index 4333c6e14742..6d669f24c0e6 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -24,12 +24,29 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/mdio-bitbang.h>
 #include <linux/gpio.h>
-#include <linux/platform_data/mdio-gpio.h>
 
 #include <linux/of_gpio.h>
 #include <linux/of_mdio.h>
 
+struct mdio_gpio_platform_data {
+	/* GPIO numbers for bus pins */
+	unsigned int mdc;
+	unsigned int mdio;
+	unsigned int mdo;
+
+	bool mdc_active_low;
+	bool mdio_active_low;
+	bool mdo_active_low;
+
+	u32 phy_mask;
+	u32 phy_ignore_ta_mask;
+	int irqs[PHY_MAX_ADDR];
+	/* reset callback */
+	int (*reset)(struct mii_bus *bus);
+};
+
 struct mdio_gpio_info {
 	struct mdiobb_ctrl ctrl;
 	struct gpio_desc *mdc, *mdio, *mdo;
diff --git a/include/linux/platform_data/mdio-gpio.h b/include/linux/platform_data/mdio-gpio.h
deleted file mode 100644
index 11f00cdabe3d..000000000000
--- a/include/linux/platform_data/mdio-gpio.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * MDIO-GPIO bus platform data structures
- *
- * Copyright (C) 2008, Paulius Zaleckas <paulius.zaleckas@teltonika.lt>
- *
- * This file is licensed under the terms of the GNU General Public License
- * version 2. This program is licensed "as is" without any warranty of any
- * kind, whether express or implied.
- */
-
-#ifndef __LINUX_MDIO_GPIO_H
-#define __LINUX_MDIO_GPIO_H
-
-#include <linux/mdio-bitbang.h>
-
-struct mdio_gpio_platform_data {
-	/* GPIO numbers for bus pins */
-	unsigned int mdc;
-	unsigned int mdio;
-	unsigned int mdo;
-
-	bool mdc_active_low;
-	bool mdio_active_low;
-	bool mdo_active_low;
-
-	u32 phy_mask;
-	u32 phy_ignore_ta_mask;
-	int irqs[PHY_MAX_ADDR];
-	/* reset callback */
-	int (*reset)(struct mii_bus *bus);
-};
-
-#endif /* __LINUX_MDIO_GPIO_H */
-- 
2.14.3

[PATCH net-next 2/5] net: mdio-gpio: Allocate state in probe()

[Linus Walleij]

Allocate the state container for the driver, struct mdio_gpio_info
inside the probe() function instead of in the mdio_gpio_bus_init()
function. Create the local struct device *dev variable in probe()
and pass that around instead of constantly dereferencing the
struct platform_data.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/phy/mdio-gpio.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index 6d669f24c0e6..d95bb45eb67b 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -52,14 +52,14 @@ struct mdio_gpio_info {
 	struct gpio_desc *mdc, *mdio, *mdo;
 };
 
-static void *mdio_gpio_of_get_data(struct platform_device *pdev)
+static void *mdio_gpio_of_get_data(struct device *dev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = dev->of_node;
 	struct mdio_gpio_platform_data *pdata;
 	enum of_gpio_flags flags;
 	int ret;
 
-	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return NULL;
 
@@ -142,21 +142,17 @@ static const struct mdiobb_ops mdio_gpio_ops = {
 };
 
 static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
+					  struct mdio_gpio_info *bitbang,
 					  struct mdio_gpio_platform_data *pdata,
 					  int bus_id)
 {
 	struct mii_bus *new_bus;
-	struct mdio_gpio_info *bitbang;
 	int i;
 	int mdc, mdio, mdo;
 	unsigned long mdc_flags = GPIOF_OUT_INIT_LOW;
 	unsigned long mdio_flags = GPIOF_DIR_IN;
 	unsigned long mdo_flags = GPIOF_OUT_INIT_HIGH;
 
-	bitbang = devm_kzalloc(dev, sizeof(*bitbang), GFP_KERNEL);
-	if (!bitbang)
-		goto out;
-
 	bitbang->ctrl.ops = &mdio_gpio_ops;
 	bitbang->ctrl.reset = pdata->reset;
 	mdc = pdata->mdc;
@@ -234,35 +230,41 @@ static void mdio_gpio_bus_destroy(struct device *dev)
 static int mdio_gpio_probe(struct platform_device *pdev)
 {
 	struct mdio_gpio_platform_data *pdata;
+	struct device *dev = &pdev->dev;
+	struct mdio_gpio_info *bitbang;
 	struct mii_bus *new_bus;
 	int ret, bus_id;
 
+	bitbang = devm_kzalloc(dev, sizeof(*bitbang), GFP_KERNEL);
+	if (!bitbang)
+		return -ENOMEM;
+
 	if (pdev->dev.of_node) {
-		pdata = mdio_gpio_of_get_data(pdev);
-		bus_id = of_alias_get_id(pdev->dev.of_node, "mdio-gpio");
+		pdata = mdio_gpio_of_get_data(dev);
+		bus_id = of_alias_get_id(dev->of_node, "mdio-gpio");
 		if (bus_id < 0) {
-			dev_warn(&pdev->dev, "failed to get alias id\n");
+			dev_warn(dev, "failed to get alias id\n");
 			bus_id = 0;
 		}
 	} else {
-		pdata = dev_get_platdata(&pdev->dev);
+		pdata = dev_get_platdata(dev);
 		bus_id = pdev->id;
 	}
 
 	if (!pdata)
 		return -ENODEV;
 
-	new_bus = mdio_gpio_bus_init(&pdev->dev, pdata, bus_id);
+	new_bus = mdio_gpio_bus_init(dev, bitbang, pdata, bus_id);
 	if (!new_bus)
 		return -ENODEV;
 
-	if (pdev->dev.of_node)
-		ret = of_mdiobus_register(new_bus, pdev->dev.of_node);
+	if (dev->of_node)
+		ret = of_mdiobus_register(new_bus, dev->of_node);
 	else
 		ret = mdiobus_register(new_bus);
 
 	if (ret)
-		mdio_gpio_bus_deinit(&pdev->dev);
+		mdio_gpio_bus_deinit(dev);
 
 	return ret;
 }
-- 
2.14.3

[PATCH net-next 3/5] net: mdio-gpio: Remove non-DT probe path

[Linus Walleij]

This driver can now only be created using the device tree.
Remove the platform data probe path and require OF_MDIO
in Kconfig.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/phy/Kconfig     |  2 +-
 drivers/net/phy/mdio-gpio.c | 21 ++++++---------------
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index bdfbabb86ee0..27efc5d6fbe2 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -92,7 +92,7 @@ config MDIO_CAVIUM
 
 config MDIO_GPIO
 	tristate "GPIO lib-based bitbanged MDIO buses"
-	depends on MDIO_BITBANG && GPIOLIB
+	depends on MDIO_BITBANG && GPIOLIB && OF_MDIO
 	---help---
 	  Supports GPIO lib-based MDIO busses.
 
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index d95bb45eb67b..96c953d086c6 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -239,16 +239,11 @@ static int mdio_gpio_probe(struct platform_device *pdev)
 	if (!bitbang)
 		return -ENOMEM;
 
-	if (pdev->dev.of_node) {
-		pdata = mdio_gpio_of_get_data(dev);
-		bus_id = of_alias_get_id(dev->of_node, "mdio-gpio");
-		if (bus_id < 0) {
-			dev_warn(dev, "failed to get alias id\n");
-			bus_id = 0;
-		}
-	} else {
-		pdata = dev_get_platdata(dev);
-		bus_id = pdev->id;
+	pdata = mdio_gpio_of_get_data(dev);
+	bus_id = of_alias_get_id(dev->of_node, "mdio-gpio");
+	if (bus_id < 0) {
+		dev_warn(dev, "failed to get alias id\n");
+		bus_id = 0;
 	}
 
 	if (!pdata)
@@ -258,11 +253,7 @@ static int mdio_gpio_probe(struct platform_device *pdev)
 	if (!new_bus)
 		return -ENODEV;
 
-	if (dev->of_node)
-		ret = of_mdiobus_register(new_bus, dev->of_node);
-	else
-		ret = mdiobus_register(new_bus);
-
+	ret = of_mdiobus_register(new_bus, dev->of_node);
 	if (ret)
 		mdio_gpio_bus_deinit(dev);
 
-- 
2.14.3

[PATCH net-next 4/5] net: mdio-gpio: Merge platform data into state

[Linus Walleij]

There is no instantiation without DT data, we can now move
to a single state container and merge the DT property
retrieveal into mdio_gpio_bus_init().

We decomission the phy_mask, phy_ignore_ta_mask and
irqs array and the reset() callback that were all just
sitting unused and taking up space.

If bitbanged GPIOs need to set up reset() callbacks these
should be done in the device tree using proper bindings.

If bitbanged GPIOs need to handle IRQs, these should be
done in the device tree using the proper bindings.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/phy/mdio-gpio.c | 130 ++++++++++++++++----------------------------
 1 file changed, 48 insertions(+), 82 deletions(-)

diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index 96c953d086c6..9146077b5278 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -30,61 +30,11 @@
 #include <linux/of_gpio.h>
 #include <linux/of_mdio.h>
 
-struct mdio_gpio_platform_data {
-	/* GPIO numbers for bus pins */
-	unsigned int mdc;
-	unsigned int mdio;
-	unsigned int mdo;
-
-	bool mdc_active_low;
-	bool mdio_active_low;
-	bool mdo_active_low;
-
-	u32 phy_mask;
-	u32 phy_ignore_ta_mask;
-	int irqs[PHY_MAX_ADDR];
-	/* reset callback */
-	int (*reset)(struct mii_bus *bus);
-};
-
 struct mdio_gpio_info {
 	struct mdiobb_ctrl ctrl;
 	struct gpio_desc *mdc, *mdio, *mdo;
 };
 
-static void *mdio_gpio_of_get_data(struct device *dev)
-{
-	struct device_node *np = dev->of_node;
-	struct mdio_gpio_platform_data *pdata;
-	enum of_gpio_flags flags;
-	int ret;
-
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return NULL;
-
-	ret = of_get_gpio_flags(np, 0, &flags);
-	if (ret < 0)
-		return NULL;
-
-	pdata->mdc = ret;
-	pdata->mdc_active_low = flags & OF_GPIO_ACTIVE_LOW;
-
-	ret = of_get_gpio_flags(np, 1, &flags);
-	if (ret < 0)
-		return NULL;
-	pdata->mdio = ret;
-	pdata->mdio_active_low = flags & OF_GPIO_ACTIVE_LOW;
-
-	ret = of_get_gpio_flags(np, 2, &flags);
-	if (ret > 0) {
-		pdata->mdo = ret;
-		pdata->mdo_active_low = flags & OF_GPIO_ACTIVE_LOW;
-	}
-
-	return pdata;
-}
-
 static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir)
 {
 	struct mdio_gpio_info *bitbang =
@@ -142,31 +92,60 @@ static const struct mdiobb_ops mdio_gpio_ops = {
 };
 
 static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
-					  struct mdio_gpio_info *bitbang,
-					  struct mdio_gpio_platform_data *pdata,
-					  int bus_id)
+					  struct mdio_gpio_info *bitbang)
 {
-	struct mii_bus *new_bus;
-	int i;
-	int mdc, mdio, mdo;
+	unsigned long mdo_flags = GPIOF_OUT_INIT_HIGH;
 	unsigned long mdc_flags = GPIOF_OUT_INIT_LOW;
 	unsigned long mdio_flags = GPIOF_DIR_IN;
-	unsigned long mdo_flags = GPIOF_OUT_INIT_HIGH;
+	struct device_node *np = dev->of_node;
+	enum of_gpio_flags flags;
+	struct mii_bus *new_bus;
+	bool mdio_active_low;
+	bool mdc_active_low;
+	bool mdo_active_low;
+	unsigned int mdio;
+	unsigned int mdc;
+	unsigned int mdo;
+	int bus_id;
+	int ret, i;
+
+	ret = of_get_gpio_flags(np, 0, &flags);
+	if (ret < 0)
+		return NULL;
+
+	mdc = ret;
+	mdc_active_low = flags & OF_GPIO_ACTIVE_LOW;
+
+	ret = of_get_gpio_flags(np, 1, &flags);
+	if (ret < 0)
+		return NULL;
+	mdio = ret;
+	mdio_active_low = flags & OF_GPIO_ACTIVE_LOW;
+
+	ret = of_get_gpio_flags(np, 2, &flags);
+	if (ret > 0) {
+		mdo = ret;
+		mdo_active_low = flags & OF_GPIO_ACTIVE_LOW;
+	} else {
+		mdo = 0;
+	}
+
+	bus_id = of_alias_get_id(np, "mdio-gpio");
+	if (bus_id < 0) {
+		dev_warn(dev, "failed to get alias id\n");
+		bus_id = 0;
+	}
 
 	bitbang->ctrl.ops = &mdio_gpio_ops;
-	bitbang->ctrl.reset = pdata->reset;
-	mdc = pdata->mdc;
 	bitbang->mdc = gpio_to_desc(mdc);
-	if (pdata->mdc_active_low)
+	if (mdc_active_low)
 		mdc_flags = GPIOF_OUT_INIT_HIGH | GPIOF_ACTIVE_LOW;
-	mdio = pdata->mdio;
 	bitbang->mdio = gpio_to_desc(mdio);
-	if (pdata->mdio_active_low)
+	if (mdio_active_low)
 		mdio_flags |= GPIOF_ACTIVE_LOW;
-	mdo = pdata->mdo;
 	if (mdo) {
 		bitbang->mdo = gpio_to_desc(mdo);
-		if (pdata->mdo_active_low)
+		if (mdo_active_low)
 			mdo_flags = GPIOF_OUT_INIT_LOW | GPIOF_ACTIVE_LOW;
 	}
 
@@ -175,10 +154,6 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
 		goto out;
 
 	new_bus->name = "GPIO Bitbanged MDIO",
-
-	new_bus->phy_mask = pdata->phy_mask;
-	new_bus->phy_ignore_ta_mask = pdata->phy_ignore_ta_mask;
-	memcpy(new_bus->irq, pdata->irqs, sizeof(new_bus->irq));
 	new_bus->parent = dev;
 
 	if (new_bus->phy_mask == ~0)
@@ -229,31 +204,22 @@ static void mdio_gpio_bus_destroy(struct device *dev)
 
 static int mdio_gpio_probe(struct platform_device *pdev)
 {
-	struct mdio_gpio_platform_data *pdata;
 	struct device *dev = &pdev->dev;
 	struct mdio_gpio_info *bitbang;
 	struct mii_bus *new_bus;
-	int ret, bus_id;
+	struct device_node *np;
+	int ret;
 
+	np = dev->of_node;
 	bitbang = devm_kzalloc(dev, sizeof(*bitbang), GFP_KERNEL);
 	if (!bitbang)
 		return -ENOMEM;
 
-	pdata = mdio_gpio_of_get_data(dev);
-	bus_id = of_alias_get_id(dev->of_node, "mdio-gpio");
-	if (bus_id < 0) {
-		dev_warn(dev, "failed to get alias id\n");
-		bus_id = 0;
-	}
-
-	if (!pdata)
-		return -ENODEV;
-
-	new_bus = mdio_gpio_bus_init(dev, bitbang, pdata, bus_id);
+	new_bus = mdio_gpio_bus_init(dev, bitbang);
 	if (!new_bus)
 		return -ENODEV;
 
-	ret = of_mdiobus_register(new_bus, dev->of_node);
+	ret = of_mdiobus_register(new_bus, np);
 	if (ret)
 		mdio_gpio_bus_deinit(dev);
 
-- 
2.14.3

[PATCH net-next 5/5] net: mdio-gpio: Move to gpiod API

[Linus Walleij]

Move the bitbanged GPIO based MDIO driver over to using the
gpiolib GPIO descriptors and transfer the line inversion
and optional MDO handling semantics to gpiolib.

The driver has been parsing the device tree to handle GPIO
semantics on its own, but this is completely unnecessary as
the gpiolib can handle all inversion and optional line
semantics.

This cuts down the code a lot and makes the driver simpler.

Switch mdio_gpio_bus_init() to return an error pointer and
handle this in probe() so we can back out of e.g. -EPROBE_DEFER
properly if we need to.

After this the GPIO MDIO driver only use GPIO descriptors
and is completely decoupled from the old GPIO API.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/phy/mdio-gpio.c | 81 +++++++++++++--------------------------------
 1 file changed, 23 insertions(+), 58 deletions(-)

diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index 9146077b5278..5740f16a0f30 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -25,9 +25,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/mdio-bitbang.h>
-#include <linux/gpio.h>
-
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of_mdio.h>
 
 struct mdio_gpio_info {
@@ -94,41 +92,23 @@ static const struct mdiobb_ops mdio_gpio_ops = {
 static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
 					  struct mdio_gpio_info *bitbang)
 {
-	unsigned long mdo_flags = GPIOF_OUT_INIT_HIGH;
-	unsigned long mdc_flags = GPIOF_OUT_INIT_LOW;
-	unsigned long mdio_flags = GPIOF_DIR_IN;
 	struct device_node *np = dev->of_node;
-	enum of_gpio_flags flags;
 	struct mii_bus *new_bus;
-	bool mdio_active_low;
-	bool mdc_active_low;
-	bool mdo_active_low;
-	unsigned int mdio;
-	unsigned int mdc;
-	unsigned int mdo;
 	int bus_id;
 	int ret, i;
 
-	ret = of_get_gpio_flags(np, 0, &flags);
-	if (ret < 0)
-		return NULL;
-
-	mdc = ret;
-	mdc_active_low = flags & OF_GPIO_ACTIVE_LOW;
-
-	ret = of_get_gpio_flags(np, 1, &flags);
-	if (ret < 0)
-		return NULL;
-	mdio = ret;
-	mdio_active_low = flags & OF_GPIO_ACTIVE_LOW;
-
-	ret = of_get_gpio_flags(np, 2, &flags);
-	if (ret > 0) {
-		mdo = ret;
-		mdo_active_low = flags & OF_GPIO_ACTIVE_LOW;
-	} else {
-		mdo = 0;
-	}
+	bitbang->mdc =
+		devm_gpiod_get_index(dev, NULL, 0, GPIOD_OUT_LOW);
+	if (IS_ERR(bitbang->mdc))
+		return ERR_CAST(bitbang->mdc);
+	bitbang->mdio =
+		devm_gpiod_get_index(dev, NULL, 1, GPIOD_IN);
+	if (IS_ERR(bitbang->mdio))
+		return ERR_CAST(bitbang->mdio);
+	bitbang->mdo =
+		devm_gpiod_get_index_optional(dev, NULL, 2, GPIOD_OUT_HIGH);
+	if (IS_ERR(bitbang->mdo))
+		return ERR_CAST(bitbang->mdo);
 
 	bus_id = of_alias_get_id(np, "mdio-gpio");
 	if (bus_id < 0) {
@@ -137,27 +117,21 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
 	}
 
 	bitbang->ctrl.ops = &mdio_gpio_ops;
-	bitbang->mdc = gpio_to_desc(mdc);
-	if (mdc_active_low)
-		mdc_flags = GPIOF_OUT_INIT_HIGH | GPIOF_ACTIVE_LOW;
-	bitbang->mdio = gpio_to_desc(mdio);
-	if (mdio_active_low)
-		mdio_flags |= GPIOF_ACTIVE_LOW;
-	if (mdo) {
-		bitbang->mdo = gpio_to_desc(mdo);
-		if (mdo_active_low)
-			mdo_flags = GPIOF_OUT_INIT_LOW | GPIOF_ACTIVE_LOW;
-	}
 
 	new_bus = alloc_mdio_bitbang(&bitbang->ctrl);
-	if (!new_bus)
+	if (!new_bus) {
+		ret = -ENOMEM;
 		goto out;
+	}
 
 	new_bus->name = "GPIO Bitbanged MDIO",
 	new_bus->parent = dev;
 
-	if (new_bus->phy_mask == ~0)
+	if (new_bus->phy_mask == ~0) {
+		dev_err(dev, "no PHY in mask\n");
+		ret = -ENODEV;
 		goto out_free_bus;
+	}
 
 	for (i = 0; i < PHY_MAX_ADDR; i++)
 		if (!new_bus->irq[i])
@@ -168,15 +142,6 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
 	else
 		strncpy(new_bus->id, "gpio", MII_BUS_ID_SIZE);
 
-	if (devm_gpio_request_one(dev, mdc, mdc_flags, "mdc"))
-		goto out_free_bus;
-
-	if (devm_gpio_request_one(dev, mdio, mdio_flags, "mdio"))
-		goto out_free_bus;
-
-	if (mdo && devm_gpio_request_one(dev, mdo, mdo_flags, "mdo"))
-		goto out_free_bus;
-
 	dev_set_drvdata(dev, new_bus);
 
 	return new_bus;
@@ -184,7 +149,7 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
 out_free_bus:
 	free_mdio_bitbang(new_bus);
 out:
-	return NULL;
+	return ERR_PTR(ret);
 }
 
 static void mdio_gpio_bus_deinit(struct device *dev)
@@ -216,8 +181,8 @@ static int mdio_gpio_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	new_bus = mdio_gpio_bus_init(dev, bitbang);
-	if (!new_bus)
-		return -ENODEV;
+	if (IS_ERR(new_bus))
+		return PTR_ERR(new_bus);
 
 	ret = of_mdiobus_register(new_bus, np);
 	if (ret)
-- 
2.14.3

Re: [PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[Andrew Lunn]

On Sun, Feb 25, 2018 at 01:51:27PM +0100, Linus Walleij wrote:
> This kills off the platform data support from the bitbanged
> GPIO-based MDIO driver and moves it over to using GPIO
> descriptors exclusively.

Hi Linus

I like where this ends up. I wounder about the path it takes to get
there. There seems to be quite a lot of code which gets moved around
and then in the end deleted. Maybe changing the order of the patches
would help. Converting to devm_gpiod_get_index() first would remove
all the active_low flags. Then remove all the unused reset callback,
irq, etc?

     Thanks
	Andrew

Re: [PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[Linus Walleij]

On Sun, Feb 25, 2018 at 8:08 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Sun, Feb 25, 2018 at 01:51:27PM +0100, Linus Walleij wrote:
>> This kills off the platform data support from the bitbanged
>> GPIO-based MDIO driver and moves it over to using GPIO
>> descriptors exclusively.
>
> Hi Linus
>
> I like where this ends up. I wounder about the path it takes to get
> there. There seems to be quite a lot of code which gets moved around
> and then in the end deleted. Maybe changing the order of the patches
> would help. Converting to devm_gpiod_get_index() first would remove
> all the active_low flags. Then remove all the unused reset callback,
> irq, etc?

It's mainly a matter of which order we want bisects to end up...

I was thinking to order the patches from least-likely to cause
regressions to most-likely to cause regressions so that it is
later possible to revert the more likely regression cause without
getting into conflict with the rest.

Yours,
Linus Walleij

Re: [PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[Florian Fainelli]

On 02/25/2018 04:51 AM, Linus Walleij wrote:
> This kills off the platform data support from the bitbanged
> GPIO-based MDIO driver and moves it over to using GPIO
> descriptors exclusively.
> 
> We are certainly not going to merge any more platforms into
> the kernel using platform data, and nothing is using it at the
> moment. The only concern would be out-of-tree platforms, and
> those are not the concern of the kernel community. They need
> to move to use device tree (or ACPI etc) like everyone else.

Please refrain from making statements like those because there are
perfectly valid use cases for never seeing ACPI or Device Tree for a
given platform, yes there will be push back, and yes, if DT or ACPI is
possible, it should be used but I can guarantee you there are platforms
out there that won't be converted to DT (e.g: tons of MIPS-based router,
x86 add-on modules etc.).

Nack on patches 1 and 3, because I am slowly resuming work on an x86
platform driver that uses the mdio-gpio driver with platform data, and
DT is not an option there, and I would rather not have to revert your
changes.

> 
> This was tested on the bit-banged GPIO MDIO on the D-Link
> DNS-313 and works fine for me.
> 
> Linus Walleij (5):
>   net: mdio-gpio: Localize platform data
>   net: mdio-gpio: Allocate state in probe()
>   net: mdio-gpio: Remove non-DT probe path
>   net: mdio-gpio: Merge platform data into state
>   net: mdio-gpio: Move to gpiod API
> 
>  MAINTAINERS                             |   1 -
>  drivers/net/phy/Kconfig                 |   2 +-
>  drivers/net/phy/mdio-gpio.c             | 151 ++++++++++----------------------
>  include/linux/platform_data/mdio-gpio.h |  33 -------
>  4 files changed, 47 insertions(+), 140 deletions(-)
>  delete mode 100644 include/linux/platform_data/mdio-gpio.h
> 

-- 
Florian

Re: [PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[Andrew Lunn]

> Nack on patches 1 and 3, because I am slowly resuming work on an x86
> platform driver that uses the mdio-gpio driver with platform data, and
> DT is not an option there, and I would rather not have to revert your
> changes.

Hi Florian

Would it be O.K. to change the platform_data to use a gpio descriptor,
rather than gpio number and active_low flag? That would simplify a lot
of code.

   Thanks
      Andrew

Re: [PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[Florian Fainelli]

On 02/27/2018 03:10 PM, Andrew Lunn wrote:
>> Nack on patches 1 and 3, because I am slowly resuming work on an x86
>> platform driver that uses the mdio-gpio driver with platform data, and
>> DT is not an option there, and I would rather not have to revert your
>> changes.
> 
> Hi Florian
> 
> Would it be O.K. to change the platform_data to use a gpio descriptor,
> rather than gpio number and active_low flag? That would simplify a lot
> of code.

Oh absolutely, the move towards gpio descriptors is entirely fine and
reasonable, it was just the plain removal of platform_data instantiation
that I am objecting to. Thanks!
-- 
Florian

Re: [PATCH net-next 0/5] Modernize bitbanged GPIO MDIO

[Linus Walleij]

On Tue, Feb 27, 2018 at 3:00 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 02/25/2018 04:51 AM, Linus Walleij wrote:
>> This kills off the platform data support from the bitbanged
>> GPIO-based MDIO driver and moves it over to using GPIO
>> descriptors exclusively.
>>
>> We are certainly not going to merge any more platforms into
>> the kernel using platform data, and nothing is using it at the
>> moment. The only concern would be out-of-tree platforms, and
>> those are not the concern of the kernel community. They need
>> to move to use device tree (or ACPI etc) like everyone else.
>
> Please refrain from making statements like those because there are
> perfectly valid use cases for never seeing ACPI or Device Tree for a
> given platform, yes there will be push back, and yes, if DT or ACPI is
> possible, it should be used but I can guarantee you there are platforms
> out there that won't be converted to DT (e.g: tons of MIPS-based router,
> x86 add-on modules etc.).
>
> Nack on patches 1 and 3, because I am slowly resuming work on an x86
> platform driver that uses the mdio-gpio driver with platform data, and
> DT is not an option there, and I would rather not have to revert your
> changes.

OK fair enough, there are some platform data use cases that
remain or are on their way in.

I will try to rewrite the series a bit as Andrew suggested to just do
descriptor conversion and take it from there, and also
try to make sure that it will work fine with any descriptor also if
it is coming from a board file.

Anyway, the platform data we have is:

struct mdio_gpio_platform_data {
        /* GPIO numbers for bus pins */
        unsigned int mdc;
        unsigned int mdio;
        unsigned int mdo;

        bool mdc_active_low;
        bool mdio_active_low;
        bool mdo_active_low;

        u32 phy_mask;
        u32 phy_ignore_ta_mask;
        int irqs[PHY_MAX_ADDR];
        /* reset callback */
        int (*reset)(struct mii_bus *bus);
};

So moving to using descriptors will get rid of the six first
platform data entries.

What about the last four things, phy_mask, phy_ignore_ta_mask,
irqs[] and the .reset() callback? None of this is documented or
used anywhere so I was a bit confused about what to do with it...
Does boardfiles adding GPIO MDIO need all this?

Yours,
Linus Walleij