2015-11-30 16:28:10

by Paul Burton

[permalink] [raw]
Subject: [PATCH 24/28] net: pch_gbe: add device tree support

Introduce support for retrieving the PHY reset GPIO from device tree,
which will be used on the MIPS Boston development board. This requires
support for probe deferral in order to work correctly, since the order
of device probe is not guaranteed & typically the EG20T GPIO controller
device will be probed after the ethernet MAC.

Signed-off-by: Paul Burton <[email protected]>
---

.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 33 +++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 824ff9e..f2a9a38 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -23,6 +23,8 @@
#include <linux/net_tstamp.h>
#include <linux/ptp_classify.h>
#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of_gpio.h>

#define DRV_VERSION "1.01"
const char pch_driver_version[] = DRV_VERSION;
@@ -2594,13 +2596,41 @@ static void pch_gbe_remove(struct pci_dev *pdev)
free_netdev(netdev);
}

+static int pch_gbe_parse_dt(struct pci_dev *pdev,
+ struct pch_gbe_privdata **pdata)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct gpio_desc *gpio;
+
+ if (!config_enabled(CONFIG_OF) || !np)
+ return 0;
+
+ if (!*pdata)
+ *pdata = devm_kzalloc(&pdev->dev, sizeof(**pdata), GFP_KERNEL);
+ if (!*pdata)
+ return -ENOMEM;
+
+ gpio = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_ASIS);
+ if (IS_ERR(gpio))
+ return PTR_ERR(gpio);
+
+ (*pdata)->phy_reset_gpio = gpio;
+ return 0;
+}
+
static int pch_gbe_probe(struct pci_dev *pdev,
const struct pci_device_id *pci_id)
{
struct net_device *netdev;
struct pch_gbe_adapter *adapter;
+ struct pch_gbe_privdata *pdata;
int ret;

+ pdata = (struct pch_gbe_privdata *)pci_id->driver_data;
+ ret = pch_gbe_parse_dt(pdev, &pdata);
+ if (ret)
+ goto err_out;
+
ret = pcim_enable_device(pdev);
if (ret)
return ret;
@@ -2638,7 +2668,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
adapter->pdev = pdev;
adapter->hw.back = adapter;
adapter->hw.reg = pcim_iomap_table(pdev)[PCH_GBE_PCI_BAR];
- adapter->pdata = (struct pch_gbe_privdata *)pci_id->driver_data;
+ adapter->pdata = pdata;
if (adapter->pdata && adapter->pdata->platform_init)
adapter->pdata->platform_init(pdev, pdata);

@@ -2729,6 +2759,7 @@ err_free_adapter:
pch_gbe_hal_phy_hw_reset(&adapter->hw);
err_free_netdev:
free_netdev(netdev);
+err_out:
return ret;
}

--
2.6.2


2015-12-13 01:22:21

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 24/28] net: pch_gbe: add device tree support

On Mon, Nov 30, 2015 at 6:21 PM, Paul Burton <[email protected]> wrote:
> Introduce support for retrieving the PHY reset GPIO from device tree,
> which will be used on the MIPS Boston development board. This requires
> support for probe deferral in order to work correctly, since the order
> of device probe is not guaranteed & typically the EG20T GPIO controller
> device will be probed after the ethernet MAC.
>
> Signed-off-by: Paul Burton <[email protected]>
> ---
>
> .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 33 +++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> index 824ff9e..f2a9a38 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -23,6 +23,8 @@
> #include <linux/net_tstamp.h>
> #include <linux/ptp_classify.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/of_gpio.h>
>
> #define DRV_VERSION "1.01"
> const char pch_driver_version[] = DRV_VERSION;
> @@ -2594,13 +2596,41 @@ static void pch_gbe_remove(struct pci_dev *pdev)
> free_netdev(netdev);
> }
>
> +static int pch_gbe_parse_dt(struct pci_dev *pdev,
> + struct pch_gbe_privdata **pdata)

Why not to return pdata as it done in many other drivers?
You have ERR_PTR() macro to pass errors.

> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct gpio_desc *gpio;
> +
> + if (!config_enabled(CONFIG_OF) || !np)

Before I saw IS_ENABLED(). Is this one a preferable new API?

> + return 0;
> +
> + if (!*pdata)
> + *pdata = devm_kzalloc(&pdev->dev, sizeof(**pdata), GFP_KERNEL);
> + if (!*pdata)
> + return -ENOMEM;
> +
> + gpio = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_ASIS);
> + if (IS_ERR(gpio))
> + return PTR_ERR(gpio);
> +
> + (*pdata)->phy_reset_gpio = gpio;
> + return 0;
> +}
> +
> static int pch_gbe_probe(struct pci_dev *pdev,
> const struct pci_device_id *pci_id)
> {
> struct net_device *netdev;
> struct pch_gbe_adapter *adapter;
> + struct pch_gbe_privdata *pdata;
> int ret;
>
> + pdata = (struct pch_gbe_privdata *)pci_id->driver_data;
> + ret = pch_gbe_parse_dt(pdev, &pdata);

So, I didn;t see anything related to dt in that function.
Maybe you just call it always? In that case remove check for np.

> + if (ret)
> + goto err_out;
> +
> ret = pcim_enable_device(pdev);
> if (ret)
> return ret;
> @@ -2638,7 +2668,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> adapter->pdev = pdev;
> adapter->hw.back = adapter;
> adapter->hw.reg = pcim_iomap_table(pdev)[PCH_GBE_PCI_BAR];
> - adapter->pdata = (struct pch_gbe_privdata *)pci_id->driver_data;
> + adapter->pdata = pdata;
> if (adapter->pdata && adapter->pdata->platform_init)
> adapter->pdata->platform_init(pdev, pdata);
>
> @@ -2729,6 +2759,7 @@ err_free_adapter:
> pch_gbe_hal_phy_hw_reset(&adapter->hw);
> err_free_netdev:
> free_netdev(netdev);
> +err_out:

Redundant.

> return ret;
> }

For now it's a common practice to mix styles in probe due to usage of
devres API.

--
With Best Regards,
Andy Shevchenko