mbox series

[0/9] pmdomain: Convert to platform remove callback returning void

Message ID 20231124080623.564924-1-u.kleine-koenig@pengutronix.de (mailing list archive)
Headers show
Series pmdomain: Convert to platform remove callback returning void | expand

Message

Uwe Kleine-König Nov. 24, 2023, 8:06 a.m. UTC
Hello,

this patch set converts all drivers below drivers/pmdomain to use struct
platform_driver::remove_new(). See commit 5c5a7680e67b ("platform:
Provide a remove callback that returns no value") for an extended
explanation and the eventual goal.

While working on drivers/pmdomain/imx/gpc.c I noticed three issues, but
didn't address them:

 - The driver uses builtin_platform_driver twice. The documentation
   however mandates that "Each driver may only use this macro once".
   I don't know if the documentation is wrong and using it twice works
   as intended.

 - imx_gpc_remove() only removes two PDs, but there might be up to four?!

 - In imx_gpc_remove() if
   pm_genpd_remove(&imx_gpc_domains[GPC_PGC_DOMAIN_PU].base) fails,
   removing the ARM PD is skipped. So together with the previous item
   the driver leaks up to three genpd instances.

Maybe someone caring for this driver will pick these up and prepare
patches? Ideally pm_genpd_remove() should return void caring for still
existing providers, parents and devices in generic code. I think that
erroring out in genpd_remove() before the PM domain is removed from the
various lists might result in use-after-free errors.

Best regards
Uwe

Uwe Kleine-König (9):
  pmdomain: imx-pgc: Convert to platform remove callback returning void
  pmdomain: imx-gpc: Convert to platform remove callback returning void
  pmdomain: imx-gpcv2: Convert to platform remove callback returning
    void
  pmdomain: imx8m-blk-ctrl: Convert to platform remove callback
    returning void
  pmdomain: imx8mp-blk-ctrl: Convert to platform remove callback
    returning void
  pmdomain: imx93-blk-ctrl: Convert to platform remove callback
    returning void
  pmdomain: imx93-pd: Convert to platform remove callback returning void
  pmdomain: qcom-cpr: Convert to platform remove callback returning void
  pmdomain: xilinx/zynqmp: Convert to platform remove callback returning
    void

 drivers/pmdomain/imx/gpc.c                  | 28 +++++++++++----------
 drivers/pmdomain/imx/gpcv2.c                |  6 ++---
 drivers/pmdomain/imx/imx8m-blk-ctrl.c       |  6 ++---
 drivers/pmdomain/imx/imx8mp-blk-ctrl.c      |  6 ++---
 drivers/pmdomain/imx/imx93-blk-ctrl.c       |  6 ++---
 drivers/pmdomain/imx/imx93-pd.c             |  6 ++---
 drivers/pmdomain/qcom/cpr.c                 |  6 ++---
 drivers/pmdomain/xilinx/zynqmp-pm-domains.c |  6 ++---
 8 files changed, 29 insertions(+), 41 deletions(-)

base-commit: 8c9660f6515396aba78d1168d2e17951d653ebf2

Comments

Ulf Hansson Nov. 30, 2023, 11:30 a.m. UTC | #1
On Fri, 24 Nov 2023 at 09:10, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello,
>
> this patch set converts all drivers below drivers/pmdomain to use struct
> platform_driver::remove_new(). See commit 5c5a7680e67b ("platform:
> Provide a remove callback that returns no value") for an extended
> explanation and the eventual goal.
>
> While working on drivers/pmdomain/imx/gpc.c I noticed three issues, but
> didn't address them:
>
>  - The driver uses builtin_platform_driver twice. The documentation
>    however mandates that "Each driver may only use this macro once".
>    I don't know if the documentation is wrong and using it twice works
>    as intended.
>
>  - imx_gpc_remove() only removes two PDs, but there might be up to four?!
>
>  - In imx_gpc_remove() if
>    pm_genpd_remove(&imx_gpc_domains[GPC_PGC_DOMAIN_PU].base) fails,
>    removing the ARM PD is skipped. So together with the previous item
>    the driver leaks up to three genpd instances.
>
> Maybe someone caring for this driver will pick these up and prepare
> patches? Ideally pm_genpd_remove() should return void caring for still
> existing providers, parents and devices in generic code. I think that
> erroring out in genpd_remove() before the PM domain is removed from the
> various lists might result in use-after-free errors.
>
> Best regards
> Uwe
>
> Uwe Kleine-König (9):
>   pmdomain: imx-pgc: Convert to platform remove callback returning void
>   pmdomain: imx-gpc: Convert to platform remove callback returning void
>   pmdomain: imx-gpcv2: Convert to platform remove callback returning
>     void
>   pmdomain: imx8m-blk-ctrl: Convert to platform remove callback
>     returning void
>   pmdomain: imx8mp-blk-ctrl: Convert to platform remove callback
>     returning void
>   pmdomain: imx93-blk-ctrl: Convert to platform remove callback
>     returning void
>   pmdomain: imx93-pd: Convert to platform remove callback returning void
>   pmdomain: qcom-cpr: Convert to platform remove callback returning void
>   pmdomain: xilinx/zynqmp: Convert to platform remove callback returning
>     void
>
>  drivers/pmdomain/imx/gpc.c                  | 28 +++++++++++----------
>  drivers/pmdomain/imx/gpcv2.c                |  6 ++---
>  drivers/pmdomain/imx/imx8m-blk-ctrl.c       |  6 ++---
>  drivers/pmdomain/imx/imx8mp-blk-ctrl.c      |  6 ++---
>  drivers/pmdomain/imx/imx93-blk-ctrl.c       |  6 ++---
>  drivers/pmdomain/imx/imx93-pd.c             |  6 ++---
>  drivers/pmdomain/qcom/cpr.c                 |  6 ++---
>  drivers/pmdomain/xilinx/zynqmp-pm-domains.c |  6 ++---
>  8 files changed, 29 insertions(+), 41 deletions(-)
>

The series applied for next, thanks!

Kind regards
Uffe