[PATCH v2 0/2] net: stmmac: dwc-qos: ACPI device support

[PATCH v2 0/2] net: stmmac: dwc-qos: ACPI device support

[Ajay Gupta]

From: Ajay Gupta <ajayg@nvidia.com>

Hi David

Reposting the changes after rebasing since the merge window is open now
as per http://vger.kernel.org/~davem/net-next.html

These two changes are needed to enable ACPI based devices to use stmmac
driver. First patch is to use generic device api (device_*) instead of
device tree based api (of_*). Second patch avoids clock and reset accesses
for Tegra ACPI based devices. ACPI interface will be used to access clock
and reset for Tegra ACPI devices in later patches.

Thanks
Ajay


Ajay Gupta (2):
  net: stmmac: dwc-qos: use generic device api
  net: stmmac: dwc-qos: avoid clk and reset for acpi device

 .../stmicro/stmmac/dwmac-dwc-qos-eth.c        | 141 ++++++++++--------
 1 file changed, 77 insertions(+), 64 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] net: stmmac: dwc-qos: use generic device api

[Ajay Gupta]

From: Ajay Gupta <ajayg@nvidia.com>

Use generic device api so that driver can work both with DT
or ACPI based devices.

Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
---
Change from v1->v2: Rebased.

 .../stmicro/stmmac/dwmac-dwc-qos-eth.c        | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index dd9967aeda22..f87306b3cdae 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -40,7 +40,7 @@ struct tegra_eqos {
 static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
 				   struct plat_stmmacenet_data *plat_dat)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
 	u32 burst_map = 0;
 	u32 bit_index = 0;
 	u32 a_index = 0;
@@ -52,9 +52,10 @@ static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
 			return -ENOMEM;
 	}
 
-	plat_dat->axi->axi_lpi_en = of_property_read_bool(np, "snps,en-lpi");
-	if (of_property_read_u32(np, "snps,write-requests",
-				 &plat_dat->axi->axi_wr_osr_lmt)) {
+	plat_dat->axi->axi_lpi_en = device_property_read_bool(dev,
+							      "snps,en-lpi");
+	if (device_property_read_u32(dev, "snps,write-requests",
+				     &plat_dat->axi->axi_wr_osr_lmt)) {
 		/**
 		 * Since the register has a reset value of 1, if property
 		 * is missing, default to 1.
@@ -68,8 +69,8 @@ static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
 		plat_dat->axi->axi_wr_osr_lmt--;
 	}
 
-	if (of_property_read_u32(np, "snps,read-requests",
-				 &plat_dat->axi->axi_rd_osr_lmt)) {
+	if (device_property_read_u32(dev, "snps,read-requests",
+				     &plat_dat->axi->axi_rd_osr_lmt)) {
 		/**
 		 * Since the register has a reset value of 1, if property
 		 * is missing, default to 1.
@@ -82,7 +83,7 @@ static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
 		 */
 		plat_dat->axi->axi_rd_osr_lmt--;
 	}
-	of_property_read_u32(np, "snps,burst-map", &burst_map);
+	device_property_read_u32(dev, "snps,burst-map", &burst_map);
 
 	/* converts burst-map bitmask to burst array */
 	for (bit_index = 0; bit_index < 7; bit_index++) {
@@ -421,7 +422,7 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
 	void *priv;
 	int ret;
 
-	data = of_device_get_match_data(&pdev->dev);
+	data = device_get_match_data(&pdev->dev);
 
 	memset(&stmmac_res, 0, sizeof(struct stmmac_resources));
 
@@ -478,7 +479,7 @@ static int dwc_eth_dwmac_remove(struct platform_device *pdev)
 	const struct dwc_eth_dwmac_data *data;
 	int err;
 
-	data = of_device_get_match_data(&pdev->dev);
+	data = device_get_match_data(&pdev->dev);
 
 	err = stmmac_dvr_remove(&pdev->dev);
 	if (err < 0)
-- 
2.17.1

[PATCH v2 2/2] net: stmmac: dwc-qos: avoid clk and reset for acpi device

[Ajay Gupta]

From: Ajay Gupta <ajayg@nvidia.com>

There are no clocks or resets referenced by Tegra ACPI device
so don't access clocks or resets interface with ACPI device.

Clocks and resets for ACPI devices will be handled via ACPI
interface.

Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
---
Change from v1->v2: Rebased.

 .../stmicro/stmmac/dwmac-dwc-qos-eth.c        | 122 ++++++++++--------
 1 file changed, 67 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index f87306b3cdae..70e8c41f7761 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -272,6 +272,7 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 			      struct stmmac_resources *res)
 {
 	struct tegra_eqos *eqos;
+	struct device *dev = &pdev->dev;
 	int err;
 
 	eqos = devm_kzalloc(&pdev->dev, sizeof(*eqos), GFP_KERNEL);
@@ -283,77 +284,88 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 	eqos->dev = &pdev->dev;
 	eqos->regs = res->addr;
 
-	eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
-	if (IS_ERR(eqos->clk_master)) {
-		err = PTR_ERR(eqos->clk_master);
-		goto error;
-	}
+	if (is_of_node(dev->fwnode)) {
+		eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
+		if (IS_ERR(eqos->clk_master)) {
+			err = PTR_ERR(eqos->clk_master);
+			goto error;
+		}
 
-	err = clk_prepare_enable(eqos->clk_master);
-	if (err < 0)
-		goto error;
+		err = clk_prepare_enable(eqos->clk_master);
+		if (err < 0)
+			goto error;
 
-	eqos->clk_slave = devm_clk_get(&pdev->dev, "slave_bus");
-	if (IS_ERR(eqos->clk_slave)) {
-		err = PTR_ERR(eqos->clk_slave);
-		goto disable_master;
-	}
+		eqos->clk_slave = devm_clk_get(&pdev->dev, "slave_bus");
+		if (IS_ERR(eqos->clk_slave)) {
+			err = PTR_ERR(eqos->clk_slave);
+			goto disable_master;
+		}
 
-	data->stmmac_clk = eqos->clk_slave;
+		data->stmmac_clk = eqos->clk_slave;
 
-	err = clk_prepare_enable(eqos->clk_slave);
-	if (err < 0)
-		goto disable_master;
+		err = clk_prepare_enable(eqos->clk_slave);
+		if (err < 0)
+			goto disable_master;
 
-	eqos->clk_rx = devm_clk_get(&pdev->dev, "rx");
-	if (IS_ERR(eqos->clk_rx)) {
-		err = PTR_ERR(eqos->clk_rx);
-		goto disable_slave;
-	}
+		eqos->clk_rx = devm_clk_get(&pdev->dev, "rx");
+		if (IS_ERR(eqos->clk_rx)) {
+			err = PTR_ERR(eqos->clk_rx);
+			goto disable_slave;
+		}
 
-	err = clk_prepare_enable(eqos->clk_rx);
-	if (err < 0)
-		goto disable_slave;
+		err = clk_prepare_enable(eqos->clk_rx);
+		if (err < 0)
+			goto disable_slave;
 
-	eqos->clk_tx = devm_clk_get(&pdev->dev, "tx");
-	if (IS_ERR(eqos->clk_tx)) {
-		err = PTR_ERR(eqos->clk_tx);
-		goto disable_rx;
-	}
+		eqos->clk_tx = devm_clk_get(&pdev->dev, "tx");
+		if (IS_ERR(eqos->clk_tx)) {
+			err = PTR_ERR(eqos->clk_tx);
+			goto disable_rx;
+		}
 
-	err = clk_prepare_enable(eqos->clk_tx);
-	if (err < 0)
-		goto disable_rx;
+		err = clk_prepare_enable(eqos->clk_tx);
+		if (err < 0)
+			goto disable_rx;
 
-	eqos->reset = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_OUT_HIGH);
-	if (IS_ERR(eqos->reset)) {
-		err = PTR_ERR(eqos->reset);
-		goto disable_tx;
-	}
+		eqos->reset = devm_gpiod_get(&pdev->dev, "phy-reset",
+					     GPIOD_OUT_HIGH);
+		if (IS_ERR(eqos->reset)) {
+			err = PTR_ERR(eqos->reset);
+			goto disable_tx;
+		}
 
-	usleep_range(2000, 4000);
-	gpiod_set_value(eqos->reset, 0);
+		usleep_range(2000, 4000);
+		gpiod_set_value(eqos->reset, 0);
 
-	/* MDIO bus was already reset just above */
-	data->mdio_bus_data->needs_reset = false;
+		/* MDIO bus was already reset just above */
+		data->mdio_bus_data->needs_reset = false;
 
-	eqos->rst = devm_reset_control_get(&pdev->dev, "eqos");
-	if (IS_ERR(eqos->rst)) {
-		err = PTR_ERR(eqos->rst);
-		goto reset_phy;
-	}
+		eqos->rst = devm_reset_control_get(&pdev->dev, "eqos");
+		if (IS_ERR(eqos->rst)) {
+			err = PTR_ERR(eqos->rst);
+			goto reset_phy;
+		}
 
-	err = reset_control_assert(eqos->rst);
-	if (err < 0)
-		goto reset_phy;
+		err = reset_control_assert(eqos->rst);
+		if (err < 0)
+			goto reset_phy;
 
-	usleep_range(2000, 4000);
+		usleep_range(2000, 4000);
 
-	err = reset_control_deassert(eqos->rst);
-	if (err < 0)
-		goto reset_phy;
+		err = reset_control_deassert(eqos->rst);
+		if (err < 0)
+			goto reset_phy;
 
-	usleep_range(2000, 4000);
+		usleep_range(2000, 4000);
+	} else {
+		/* set clk and reset handle to NULL for non DT device */
+		eqos->clk_master = NULL;
+		eqos->clk_slave = NULL;
+		data->stmmac_clk = NULL;
+		eqos->clk_rx = NULL;
+		eqos->clk_tx = NULL;
+		eqos->reset = NULL;
+	}
 
 	data->fix_mac_speed = tegra_eqos_fix_speed;
 	data->init = tegra_eqos_init;
-- 
2.17.1

Re: [PATCH v2 2/2] net: stmmac: dwc-qos: avoid clk and reset for acpi device

[Jakub Kicinski]

On Tue, 10 Dec 2019 23:11:25 -0800, Ajay Gupta wrote:
> From: Ajay Gupta <ajayg@nvidia.com>
> 
> There are no clocks or resets referenced by Tegra ACPI device
> so don't access clocks or resets interface with ACPI device.
> 
> Clocks and resets for ACPI devices will be handled via ACPI
> interface.
> 
> Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> ---
> Change from v1->v2: Rebased.
> 
>  .../stmicro/stmmac/dwmac-dwc-qos-eth.c        | 122 ++++++++++--------
>  1 file changed, 67 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> index f87306b3cdae..70e8c41f7761 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> @@ -272,6 +272,7 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
>  			      struct stmmac_resources *res)
>  {
>  	struct tegra_eqos *eqos;
> +	struct device *dev = &pdev->dev;
>  	int err;

Looks like this file uses reverse xmas tree variable ordering (longest
to shortest total line length), please comply and add the dev first.

>  	eqos = devm_kzalloc(&pdev->dev, sizeof(*eqos), GFP_KERNEL);
> @@ -283,77 +284,88 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
>  	eqos->dev = &pdev->dev;
>  	eqos->regs = res->addr;
>  
> -	eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
> -	if (IS_ERR(eqos->clk_master)) {
> -		err = PTR_ERR(eqos->clk_master);
> -		goto error;
> -	}
> +	if (is_of_node(dev->fwnode)) {
> +		eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
> +		if (IS_ERR(eqos->clk_master)) {
> +			err = PTR_ERR(eqos->clk_master);
> +			goto error;
> +		}

You're indenting most of this function (~70 lines?) Would it be
possible to move this code out into a separate function?

[..]
> +	} else {
> +		/* set clk and reset handle to NULL for non DT device */
> +		eqos->clk_master = NULL;
> +		eqos->clk_slave = NULL;
> +		data->stmmac_clk = NULL;
> +		eqos->clk_rx = NULL;
> +		eqos->clk_tx = NULL;
> +		eqos->reset = NULL;

Not sure about data, but eqos is zalloced so there should be no need to
set the pointers to NULL.

> +	}