| Message ID | 20230621153650.440350-8-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: introduce devres helpers for stmmac platform drivers | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 24 this patch: 24 |
| netdev/cc_maintainers | success | CCed 11 of 11 maintainers |
| netdev/build_clang | success | Errors and warnings before: 8 this patch: 8 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 24 this patch: 24 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 39 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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 > > ...
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;