From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Add a variant of stmmac_pltfr_remove() that only frees resources
allocated by stmmac_pltfr_probe() and - unlike stmmac_pltfr_remove() -
does not call stmmac_remove_config_dt().

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 .../ethernet/stmicro/stmmac/stmmac_platform.c | 20 +++++++++++++++++--
 .../ethernet/stmicro/stmmac/stmmac_platform.h |  1 +
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index df417cdab8c1..58d5c5cc2269 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -762,6 +762,23 @@ int stmmac_pltfr_probe(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(stmmac_pltfr_probe);
 
+/**
+ * stmmac_pltfr_remove_no_dt
+ * @pdev: pointer to the platform device
+ * Description: This undoes the effects of stmmac_pltfr_probe() by removing the
+ * driver and calling the platform's exit() callback.
+ */
+void stmmac_pltfr_remove_no_dt(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct plat_stmmacenet_data *plat = priv->plat;
+
+	stmmac_dvr_remove(&pdev->dev);
+	stmmac_pltfr_exit(pdev, plat);
+}
+EXPORT_SYMBOL_GPL(stmmac_pltfr_remove_no_dt);
+
 /**
  * stmmac_pltfr_remove
  * @pdev: platform device pointer
@@ -774,8 +791,7 @@ void stmmac_pltfr_remove(struct platform_device *pdev)
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	struct plat_stmmacenet_data *plat = priv->plat;
 
-	stmmac_dvr_remove(&pdev->dev);
-	stmmac_pltfr_exit(pdev, plat);
+	stmmac_pltfr_remove_no_dt(pdev);
 	stmmac_remove_config_dt(pdev, plat);
 }
 EXPORT_SYMBOL_GPL(stmmac_pltfr_remove);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
index f968e658c9d2..af52d5aa2b9a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
@@ -27,6 +27,7 @@ void stmmac_pltfr_exit(struct platform_device *pdev,
 int stmmac_pltfr_probe(struct platform_device *pdev,
 		       struct plat_stmmacenet_data *plat,
 		       struct stmmac_resources *res);
+void stmmac_pltfr_remove_no_dt(struct platform_device *pdev);
 void stmmac_pltfr_remove(struct platform_device *pdev);
 extern const struct dev_pm_ops stmmac_pltfr_pm_ops;
 
-- 
2.39.2

On Wed, Jun 21, 2023 at 05:36:46PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Add a variant of stmmac_pltfr_remove() that only frees resources
> allocated by stmmac_pltfr_probe() and - unlike stmmac_pltfr_remove() -
> does not call stmmac_remove_config_dt().
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  .../ethernet/stmicro/stmmac/stmmac_platform.c | 20 +++++++++++++++++--
>  .../ethernet/stmicro/stmmac/stmmac_platform.h |  1 +
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 

Hi Bartosz,

some minor feedback from my side as it looks like there will be a v2 anyway.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index df417cdab8c1..58d5c5cc2269 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -762,6 +762,23 @@ int stmmac_pltfr_probe(struct platform_device *pdev,
>  }
>  EXPORT_SYMBOL_GPL(stmmac_pltfr_probe);
>  
> +/**
> + * stmmac_pltfr_remove_no_dt
> + * @pdev: pointer to the platform device
> + * Description: This undoes the effects of stmmac_pltfr_probe() by removing the
> + * driver and calling the platform's exit() callback.
> + */
> +void stmmac_pltfr_remove_no_dt(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	struct plat_stmmacenet_data *plat = priv->plat;

nit: please use reverse xmas tree - longest line to shortest - for
     new Networking code.

     e.g.:

	struct net_device *ndev = platform_get_drvdata(pdev);
	struct stmmac_priv *priv = netdev_priv(ndev);
	struct plat_stmmacenet_data *plat = plat;

	plat = priv->plat;

> +
> +	stmmac_dvr_remove(&pdev->dev);
> +	stmmac_pltfr_exit(pdev, plat);
> +}
> +EXPORT_SYMBOL_GPL(stmmac_pltfr_remove_no_dt);
> +
>  /**
>   * stmmac_pltfr_remove
>   * @pdev: platform device pointer

...

On Thu, Jun 22, 2023 at 2:15 PM Simon Horman <simon.horman@corigine.com> wrote:
>
> On Wed, Jun 21, 2023 at 05:36:46PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Add a variant of stmmac_pltfr_remove() that only frees resources
> > allocated by stmmac_pltfr_probe() and - unlike stmmac_pltfr_remove() -
> > does not call stmmac_remove_config_dt().
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  .../ethernet/stmicro/stmmac/stmmac_platform.c | 20 +++++++++++++++++--
> >  .../ethernet/stmicro/stmmac/stmmac_platform.h |  1 +
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> >
>
> Hi Bartosz,
>
> some minor feedback from my side as it looks like there will be a v2 anyway.
>
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index df417cdab8c1..58d5c5cc2269 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -762,6 +762,23 @@ int stmmac_pltfr_probe(struct platform_device *pdev,
> >  }
> >  EXPORT_SYMBOL_GPL(stmmac_pltfr_probe);
> >
> > +/**
> > + * stmmac_pltfr_remove_no_dt
> > + * @pdev: pointer to the platform device
> > + * Description: This undoes the effects of stmmac_pltfr_probe() by removing the
> > + * driver and calling the platform's exit() callback.
> > + */
> > +void stmmac_pltfr_remove_no_dt(struct platform_device *pdev)
> > +{
> > +     struct net_device *ndev = platform_get_drvdata(pdev);
> > +     struct stmmac_priv *priv = netdev_priv(ndev);
> > +     struct plat_stmmacenet_data *plat = priv->plat;
>
> nit: please use reverse xmas tree - longest line to shortest - for
>      new Networking code.
>
>      e.g.:
>
>         struct net_device *ndev = platform_get_drvdata(pdev);
>         struct stmmac_priv *priv = netdev_priv(ndev);
>         struct plat_stmmacenet_data *plat = plat;
>
>         plat = priv->plat;
>

I normally stick to this convention but here, you need 5 lines for the
same effect and you make it more confusing by initializing some of the
variables at their declaration and some not. In other places in this
driver the same approach is used i.e. not adhering to reverse xmas
tree when all variables are initialized when declared.

Bart

> > +
> > +     stmmac_dvr_remove(&pdev->dev);
> > +     stmmac_pltfr_exit(pdev, plat);
> > +}
> > +EXPORT_SYMBOL_GPL(stmmac_pltfr_remove_no_dt);
> > +
> >  /**
> >   * stmmac_pltfr_remove
> >   * @pdev: platform device pointer
>
> ...