Message ID | 20210101165507.19486-1-tiny.windzz@gmail.com |
---|---|
Headers | show |
Series | Introduce devm_pm_opp_* API | expand |
01.01.2021 19:54, Yangtao Li пишет: > Hi, > > This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, > devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, > devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and > devm_pm_opp_register_notifier. > > Yangtao Li (31): > opp: Add devres wrapper for dev_pm_opp_set_clkname and > dev_pm_opp_put_clkname > opp: Add devres wrapper for dev_pm_opp_set_regulators and > dev_pm_opp_put_regulators > opp: Add devres wrapper for dev_pm_opp_set_supported_hw > opp: Add devres wrapper for dev_pm_opp_of_add_table > opp: Add devres wrapper for dev_pm_opp_register_notifier > serial: qcom_geni_serial: fix potential mem leak in > qcom_geni_serial_probe() > serial: qcom_geni_serial: convert to use devm_pm_opp_* API > spi: spi-qcom-qspi: fix potential mem leak in spi_geni_probe() > spi: spi-qcom-qspi: fix potential mem leak in spi_geni_probe() > qcom-geni-se: remove opp_table > mmc: sdhci-msm: fix potential mem leak in sdhci_msm_probe() > mmc: sdhci-msm: convert to use devm_pm_opp_* API > spi: spi-qcom-qspi: fix potential mem leak in qcom_qspi_probe() > spi: spi-qcom-qspi: convert to use devm_pm_opp_* API > drm/msm: fix potential mem leak > drm/msm: convert to use devm_pm_opp_* API and remove dp_ctrl_put > drm/lima: convert to use devm_pm_opp_* API > drm/lima: remove unneeded devm_devfreq_remove_device() > drm/panfrost: convert to use devm_pm_opp_* API > media: venus: fix error check in core_get_v4() > media: venus: convert to use devm_pm_opp_* API > memory: samsung: exynos5422-dmc: fix return error in > exynos5_init_freq_table > memory: samsung: exynos5422-dmc: convert to use devm_pm_opp_* API > memory: tegra20: convert to use devm_pm_opp_* API > memory: tegra30: convert to use devm_pm_opp_* API > PM / devfreq: tegra30: convert to use devm_pm_opp_* API > PM / devfreq: rk3399_dmc: convert to use devm_pm_opp_* API > PM / devfreq: imx8m-ddrc: convert to use devm_pm_opp_* API > PM / devfreq: imx-bus: convert to use devm_pm_opp_* API > PM / devfreq: exynos: convert to use devm_pm_opp_* API > PM / devfreq: convert to devm_pm_opp_register_notifier and remove > unused API > > drivers/devfreq/devfreq.c | 66 +------ > drivers/devfreq/exynos-bus.c | 42 +---- > drivers/devfreq/imx-bus.c | 14 +- > drivers/devfreq/imx8m-ddrc.c | 15 +- > drivers/devfreq/rk3399_dmc.c | 22 +-- > drivers/devfreq/tegra30-devfreq.c | 21 +-- > drivers/gpu/drm/lima/lima_devfreq.c | 45 +---- > drivers/gpu/drm/lima/lima_devfreq.h | 2 - > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 31 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 - > drivers/gpu/drm/msm/dp/dp_ctrl.c | 29 +-- > drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 - > drivers/gpu/drm/msm/dp/dp_display.c | 5 +- > drivers/gpu/drm/msm/dsi/dsi_host.c | 23 ++- > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 34 +--- > drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 - > .../media/platform/qcom/venus/pm_helpers.c | 22 +-- > drivers/memory/samsung/exynos5422-dmc.c | 13 +- > drivers/memory/tegra/tegra20-emc.c | 29 +-- > drivers/memory/tegra/tegra30-emc.c | 29 +-- > drivers/mmc/host/sdhci-msm.c | 27 ++- > drivers/opp/core.c | 173 ++++++++++++++++++ > drivers/opp/of.c | 36 ++++ > drivers/spi/spi-geni-qcom.c | 23 ++- > drivers/spi/spi-qcom-qspi.c | 25 ++- > drivers/tty/serial/qcom_geni_serial.c | 31 ++-- > include/linux/devfreq.h | 23 --- > include/linux/pm_opp.h | 38 ++++ > include/linux/qcom-geni-se.h | 2 - > 32 files changed, 402 insertions(+), 428 deletions(-) > Hello, Could you please add helper for dev_pm_opp_attach_genpd() and make cpufreq drivers to use the helpers? I'd also like to see a devm helper for dev_pm_opp_register_set_opp_helper(), which should become useful for Tegra drivers sometime soon.
HI, On Sun, Jan 3, 2021 at 8:52 PM Dmitry Osipenko <digetx@gmail.com> wrote: > > 01.01.2021 19:54, Yangtao Li пишет: > > Hi, > > > > This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, > > devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, > > devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and > > devm_pm_opp_register_notifier. > > > > Yangtao Li (31): > > opp: Add devres wrapper for dev_pm_opp_set_clkname and > > dev_pm_opp_put_clkname > > opp: Add devres wrapper for dev_pm_opp_set_regulators and > > dev_pm_opp_put_regulators > > opp: Add devres wrapper for dev_pm_opp_set_supported_hw > > opp: Add devres wrapper for dev_pm_opp_of_add_table > > opp: Add devres wrapper for dev_pm_opp_register_notifier > > serial: qcom_geni_serial: fix potential mem leak in > > qcom_geni_serial_probe() > > serial: qcom_geni_serial: convert to use devm_pm_opp_* API > > spi: spi-qcom-qspi: fix potential mem leak in spi_geni_probe() > > spi: spi-qcom-qspi: fix potential mem leak in spi_geni_probe() > > qcom-geni-se: remove opp_table > > mmc: sdhci-msm: fix potential mem leak in sdhci_msm_probe() > > mmc: sdhci-msm: convert to use devm_pm_opp_* API > > spi: spi-qcom-qspi: fix potential mem leak in qcom_qspi_probe() > > spi: spi-qcom-qspi: convert to use devm_pm_opp_* API > > drm/msm: fix potential mem leak > > drm/msm: convert to use devm_pm_opp_* API and remove dp_ctrl_put > > drm/lima: convert to use devm_pm_opp_* API > > drm/lima: remove unneeded devm_devfreq_remove_device() > > drm/panfrost: convert to use devm_pm_opp_* API > > media: venus: fix error check in core_get_v4() > > media: venus: convert to use devm_pm_opp_* API > > memory: samsung: exynos5422-dmc: fix return error in > > exynos5_init_freq_table > > memory: samsung: exynos5422-dmc: convert to use devm_pm_opp_* API > > memory: tegra20: convert to use devm_pm_opp_* API > > memory: tegra30: convert to use devm_pm_opp_* API > > PM / devfreq: tegra30: convert to use devm_pm_opp_* API > > PM / devfreq: rk3399_dmc: convert to use devm_pm_opp_* API > > PM / devfreq: imx8m-ddrc: convert to use devm_pm_opp_* API > > PM / devfreq: imx-bus: convert to use devm_pm_opp_* API > > PM / devfreq: exynos: convert to use devm_pm_opp_* API > > PM / devfreq: convert to devm_pm_opp_register_notifier and remove > > unused API > > > > drivers/devfreq/devfreq.c | 66 +------ > > drivers/devfreq/exynos-bus.c | 42 +---- > > drivers/devfreq/imx-bus.c | 14 +- > > drivers/devfreq/imx8m-ddrc.c | 15 +- > > drivers/devfreq/rk3399_dmc.c | 22 +-- > > drivers/devfreq/tegra30-devfreq.c | 21 +-- > > drivers/gpu/drm/lima/lima_devfreq.c | 45 +---- > > drivers/gpu/drm/lima/lima_devfreq.h | 2 - > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +- > > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- > > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 31 ++-- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 - > > drivers/gpu/drm/msm/dp/dp_ctrl.c | 29 +-- > > drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 - > > drivers/gpu/drm/msm/dp/dp_display.c | 5 +- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 23 ++- > > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 34 +--- > > drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 - > > .../media/platform/qcom/venus/pm_helpers.c | 22 +-- > > drivers/memory/samsung/exynos5422-dmc.c | 13 +- > > drivers/memory/tegra/tegra20-emc.c | 29 +-- > > drivers/memory/tegra/tegra30-emc.c | 29 +-- > > drivers/mmc/host/sdhci-msm.c | 27 ++- > > drivers/opp/core.c | 173 ++++++++++++++++++ > > drivers/opp/of.c | 36 ++++ > > drivers/spi/spi-geni-qcom.c | 23 ++- > > drivers/spi/spi-qcom-qspi.c | 25 ++- > > drivers/tty/serial/qcom_geni_serial.c | 31 ++-- > > include/linux/devfreq.h | 23 --- > > include/linux/pm_opp.h | 38 ++++ > > include/linux/qcom-geni-se.h | 2 - > > 32 files changed, 402 insertions(+), 428 deletions(-) > > > > Hello, > > Could you please add helper for dev_pm_opp_attach_genpd() and make > cpufreq drivers to use the helpers? Thank you for reminding me. But we shouldn't use this for CPU devices as the CPU device doesn't get bound to a driver, it is rather a fake platform device which gets the cpufreq drivers probed. > > I'd also like to see a devm helper for > dev_pm_opp_register_set_opp_helper(), which should become useful for > Tegra drivers sometime soon. For non-cpu devices? BR / Yangtao
03.01.2021 17:30, Frank Lee пишет: > HI, > > On Sun, Jan 3, 2021 at 8:52 PM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> 01.01.2021 19:54, Yangtao Li пишет: >>> Hi, >>> >>> This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, >>> devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, >>> devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and >>> devm_pm_opp_register_notifier. >>> >>> Yangtao Li (31): >>> opp: Add devres wrapper for dev_pm_opp_set_clkname and >>> dev_pm_opp_put_clkname >>> opp: Add devres wrapper for dev_pm_opp_set_regulators and >>> dev_pm_opp_put_regulators >>> opp: Add devres wrapper for dev_pm_opp_set_supported_hw >>> opp: Add devres wrapper for dev_pm_opp_of_add_table >>> opp: Add devres wrapper for dev_pm_opp_register_notifier >>> serial: qcom_geni_serial: fix potential mem leak in >>> qcom_geni_serial_probe() >>> serial: qcom_geni_serial: convert to use devm_pm_opp_* API >>> spi: spi-qcom-qspi: fix potential mem leak in spi_geni_probe() >>> spi: spi-qcom-qspi: fix potential mem leak in spi_geni_probe() >>> qcom-geni-se: remove opp_table >>> mmc: sdhci-msm: fix potential mem leak in sdhci_msm_probe() >>> mmc: sdhci-msm: convert to use devm_pm_opp_* API >>> spi: spi-qcom-qspi: fix potential mem leak in qcom_qspi_probe() >>> spi: spi-qcom-qspi: convert to use devm_pm_opp_* API >>> drm/msm: fix potential mem leak >>> drm/msm: convert to use devm_pm_opp_* API and remove dp_ctrl_put >>> drm/lima: convert to use devm_pm_opp_* API >>> drm/lima: remove unneeded devm_devfreq_remove_device() >>> drm/panfrost: convert to use devm_pm_opp_* API >>> media: venus: fix error check in core_get_v4() >>> media: venus: convert to use devm_pm_opp_* API >>> memory: samsung: exynos5422-dmc: fix return error in >>> exynos5_init_freq_table >>> memory: samsung: exynos5422-dmc: convert to use devm_pm_opp_* API >>> memory: tegra20: convert to use devm_pm_opp_* API >>> memory: tegra30: convert to use devm_pm_opp_* API >>> PM / devfreq: tegra30: convert to use devm_pm_opp_* API >>> PM / devfreq: rk3399_dmc: convert to use devm_pm_opp_* API >>> PM / devfreq: imx8m-ddrc: convert to use devm_pm_opp_* API >>> PM / devfreq: imx-bus: convert to use devm_pm_opp_* API >>> PM / devfreq: exynos: convert to use devm_pm_opp_* API >>> PM / devfreq: convert to devm_pm_opp_register_notifier and remove >>> unused API >>> >>> drivers/devfreq/devfreq.c | 66 +------ >>> drivers/devfreq/exynos-bus.c | 42 +---- >>> drivers/devfreq/imx-bus.c | 14 +- >>> drivers/devfreq/imx8m-ddrc.c | 15 +- >>> drivers/devfreq/rk3399_dmc.c | 22 +-- >>> drivers/devfreq/tegra30-devfreq.c | 21 +-- >>> drivers/gpu/drm/lima/lima_devfreq.c | 45 +---- >>> drivers/gpu/drm/lima/lima_devfreq.h | 2 - >>> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +- >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- >>> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 31 ++-- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 - >>> drivers/gpu/drm/msm/dp/dp_ctrl.c | 29 +-- >>> drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 - >>> drivers/gpu/drm/msm/dp/dp_display.c | 5 +- >>> drivers/gpu/drm/msm/dsi/dsi_host.c | 23 ++- >>> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 34 +--- >>> drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 - >>> .../media/platform/qcom/venus/pm_helpers.c | 22 +-- >>> drivers/memory/samsung/exynos5422-dmc.c | 13 +- >>> drivers/memory/tegra/tegra20-emc.c | 29 +-- >>> drivers/memory/tegra/tegra30-emc.c | 29 +-- >>> drivers/mmc/host/sdhci-msm.c | 27 ++- >>> drivers/opp/core.c | 173 ++++++++++++++++++ >>> drivers/opp/of.c | 36 ++++ >>> drivers/spi/spi-geni-qcom.c | 23 ++- >>> drivers/spi/spi-qcom-qspi.c | 25 ++- >>> drivers/tty/serial/qcom_geni_serial.c | 31 ++-- >>> include/linux/devfreq.h | 23 --- >>> include/linux/pm_opp.h | 38 ++++ >>> include/linux/qcom-geni-se.h | 2 - >>> 32 files changed, 402 insertions(+), 428 deletions(-) >>> >> >> Hello, >> >> Could you please add helper for dev_pm_opp_attach_genpd() and make >> cpufreq drivers to use the helpers? > > Thank you for reminding me. But we shouldn't use this for CPU devices > as the CPU device doesn't get bound to a driver, it is rather a fake platform > device which gets the cpufreq drivers probed. Indeed, the CPU device exists seprately from cpufreq driver. >> I'd also like to see a devm helper for >> dev_pm_opp_register_set_opp_helper(), which should become useful for >> Tegra drivers sometime soon. > > For non-cpu devices? For DRM driver I'd want to use devm for both set_opp_helper() and opp_attach_genpd(). https://patchwork.ozlabs.org/project/linux-tegra/patch/20201217180638.22748-39-digetx@gmail.com/
On 01-01-21, 16:54, Yangtao Li wrote: > Add devres wrapper for dev_pm_opp_register_notifier() to simplify driver > code. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/opp/core.c | 38 ++++++++++++++++++++++++++++++++++++++ > include/linux/pm_opp.h | 6 ++++++ > 2 files changed, 44 insertions(+) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index 6b83e373f0d8..ef3544f8cecd 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -2599,6 +2599,44 @@ int dev_pm_opp_unregister_notifier(struct device *dev, > } > EXPORT_SYMBOL(dev_pm_opp_unregister_notifier); > > +static void devm_pm_opp_notifier_release(struct device *dev, void *res) > +{ > + struct notifier_block *nb = *(struct notifier_block **)res; > + > + WARN_ON(dev_pm_opp_unregister_notifier(dev, nb)); > +} > + > +/** > + * devm_pm_opp_register_notifier() - Register OPP notifier for the device > + * @dev: Device for which notifier needs to be registered > + * @nb: Notifier block to be registered > + * > + * Return: 0 on success or a negative error value. > + * > + * The notifier will be unregistered after the device is destroyed. > + */ > +int devm_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb) > +{ > + struct notifier_block **ptr; > + int ret; > + > + ptr = devres_alloc(devm_pm_opp_notifier_release, sizeof(*ptr), GFP_KERNEL); > + if (!ptr) > + return -ENOMEM; > + > + ret = dev_pm_opp_register_notifier(dev, nb); > + if (ret) { > + devres_free(ptr); > + return ret; > + } > + > + *ptr = nb; > + devres_add(dev, ptr); > + > + return 0; > +} > +EXPORT_SYMBOL(devm_pm_opp_register_notifier); I am not in favor of this patch, and it only has one user, which makes it more unwanted.
On 01-01-21, 16:54, Yangtao Li wrote: > Use devm_pm_opp_* API to simplify code, and we don't need > to make opp_table glabal. > > Let's remove opp_table from geni_se later. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/tty/serial/qcom_geni_serial.c | 23 +++++++++-------------- > 1 file changed, 9 insertions(+), 14 deletions(-) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index 5aada7ebae35..36a92df8ec11 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -1352,6 +1352,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > int irq; > bool console = false; > struct uart_driver *drv; > + struct opp_table *opp_table; > > if (of_device_is_compatible(pdev->dev.of_node, "qcom,geni-debug-uart")) > console = true; > @@ -1433,13 +1434,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) > port->cts_rts_swap = true; > > - port->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se"); > - if (IS_ERR(port->se.opp_table)) > - return PTR_ERR(port->se.opp_table); > + opp_table = devm_pm_opp_set_clkname(&pdev->dev, "se"); > + if (IS_ERR(opp_table)) > + return PTR_ERR(opp_table); > /* OPP table is optional */ > - ret = dev_pm_opp_of_add_table(&pdev->dev); > + ret = devm_pm_opp_of_add_table(&pdev->dev); > if (ret) { > - dev_pm_opp_put_clkname(port->se.opp_table); > + devm_pm_opp_put_clkname(&pdev->dev, opp_table); We shouldn't be doing this here, i.e. put_clkname. Even when the OPP table isn't present, this driver calls dev_pm_opp_set_rate() which behaves like clk_set_rate() in this case and so the clk name is still required by the OPP core. > if (ret != -ENODEV) { > dev_err(&pdev->dev, "invalid OPP table in device tree\n"); > return ret; > @@ -1453,7 +1454,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > > ret = uart_add_one_port(drv, uport); > if (ret) > - goto err; > + return ret; > > irq_set_status_flags(uport->irq, IRQ_NOAUTOEN); > ret = devm_request_irq(uport->dev, uport->irq, qcom_geni_serial_isr, > @@ -1461,7 +1462,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > if (ret) { > dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret); > uart_remove_one_port(drv, uport); > - goto err; > + return ret; > } > > /* > @@ -1478,15 +1479,11 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > if (ret) { > device_init_wakeup(&pdev->dev, false); > uart_remove_one_port(drv, uport); > - goto err; > + return ret; > } > } > > return 0; > -err: > - dev_pm_opp_of_remove_table(&pdev->dev); > - dev_pm_opp_put_clkname(port->se.opp_table); > - return ret; > } > > static int qcom_geni_serial_remove(struct platform_device *pdev) > @@ -1494,8 +1491,6 @@ static int qcom_geni_serial_remove(struct platform_device *pdev) > struct qcom_geni_serial_port *port = platform_get_drvdata(pdev); > struct uart_driver *drv = port->private_data.drv; > > - dev_pm_opp_of_remove_table(&pdev->dev); > - dev_pm_opp_put_clkname(port->se.opp_table); > dev_pm_clear_wake_irq(&pdev->dev); > device_init_wakeup(&pdev->dev, false); > uart_remove_one_port(drv, &port->uport); > -- > 2.25.1 -- viresh
Dropped lots of people from cc list On 04-01-21, 12:49, Viresh Kumar wrote: > On 01-01-21, 16:54, Yangtao Li wrote: > > Use devm_pm_opp_* API to simplify code, and we don't need > > to make opp_table glabal. > > > > Let's remove opp_table from geni_se later. > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > --- > > drivers/tty/serial/qcom_geni_serial.c | 23 +++++++++-------------- > > 1 file changed, 9 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > > index 5aada7ebae35..36a92df8ec11 100644 > > --- a/drivers/tty/serial/qcom_geni_serial.c > > +++ b/drivers/tty/serial/qcom_geni_serial.c > > @@ -1352,6 +1352,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > > int irq; > > bool console = false; > > struct uart_driver *drv; > > + struct opp_table *opp_table; > > > > if (of_device_is_compatible(pdev->dev.of_node, "qcom,geni-debug-uart")) > > console = true; > > @@ -1433,13 +1434,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > > if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) > > port->cts_rts_swap = true; > > > > - port->se.opp_table = dev_pm_opp_set_clkname(&pdev->dev, "se"); > > - if (IS_ERR(port->se.opp_table)) > > - return PTR_ERR(port->se.opp_table); > > + opp_table = devm_pm_opp_set_clkname(&pdev->dev, "se"); > > + if (IS_ERR(opp_table)) > > + return PTR_ERR(opp_table); > > /* OPP table is optional */ > > - ret = dev_pm_opp_of_add_table(&pdev->dev); > > + ret = devm_pm_opp_of_add_table(&pdev->dev); > > if (ret) { > > - dev_pm_opp_put_clkname(port->se.opp_table); > > + devm_pm_opp_put_clkname(&pdev->dev, opp_table); > > We shouldn't be doing this here, i.e. put_clkname. Even when the OPP > table isn't present, this driver calls dev_pm_opp_set_rate() which > behaves like clk_set_rate() in this case and so the clk name is still > required by the OPP core. The same problem is there with multiple patches, fix them all please. -- viresh
On 01-01-21, 16:54, Yangtao Li wrote: > Hi, > > This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, > devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, > devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and > devm_pm_opp_register_notifier. You can't put so many names in the cclist, we are getting failure messages while replying to your patches now. Put all people you want to inform in the bcc section and only the important ones in to/cc list.
On 01-01-21, 16:54, Yangtao Li wrote: > +/** > + * devm_pm_opp_put_clkname() - Releases resources blocked for clk. > + * @dev: Device for which we do this operation. > + * @opp_table: OPP table returned from devm_pm_opp_set_clkname(). > + */ > +void devm_pm_opp_put_clkname(struct device *dev, struct opp_table *opp_table) > +{ > + devm_release_action(dev, devm_pm_opp_clkname_release, opp_table); > +} > +EXPORT_SYMBOL_GPL(devm_pm_opp_put_clkname); We shouldn't be needing changes like this, please drop them for all patches.
On 01-01-21, 16:54, Yangtao Li wrote: > Hi, > > This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, > devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, > devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and > devm_pm_opp_register_notifier. Please also mention next time to all the maintainers that you need their Acks for their patches and that all these patches should get merged via the OPP tree.
On Fri, Jan 01, 2021 at 04:54:45PM +0000, Yangtao Li wrote: > Use devm_pm_opp_* API to simplify code, and we don't need > to make opp_table glabal. Acked-by: Mark brown <broonie@kernel.org>
On Fri, Jan 01, 2021 at 04:54:49PM +0000, Yangtao Li wrote: > We should use dev_pm_opp_put_clkname() to free opp table each time > dev_pm_opp_of_add_table() got error. Acked-by: Mark Brown <broonie@kernel.org>
On Fri, Jan 01, 2021 at 04:54:59PM +0000, Yangtao Li wrote: > Use devm_pm_opp_* API to simplify code. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/memory/samsung/exynos5422-dmc.c | 21 +++++---------------- > 1 file changed, 5 insertions(+), 16 deletions(-) Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On 01/01/2021 16:54, Yangtao Li wrote: > Use devm_pm_opp_* API to simplify code, and remove opp_table > from panfrost_devfreq. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> Reviewed-by: Steven Price <steven.price@arm.com> > --- > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 34 ++++++--------------- > drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 - > 2 files changed, 10 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > index f44d28fad085..c42fa9eb43b1 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > @@ -92,25 +92,26 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > struct thermal_cooling_device *cooling; > struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; > > - opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > + opp_table = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names, > pfdev->comp->num_supplies); > if (IS_ERR(opp_table)) { > ret = PTR_ERR(opp_table); > /* Continue if the optional regulator is missing */ > if (ret != -ENODEV) { > DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n"); > - goto err_fini; > + return ret; > } > - } else { > - pfdevfreq->regulators_opp_table = opp_table; > } > > - ret = dev_pm_opp_of_add_table(dev); > + ret = devm_pm_opp_of_add_table(dev); > if (ret) { > + if (!IS_ERR(opp_table)) > + devm_pm_opp_put_regulators(dev, opp_table); > + > /* Optional, continue without devfreq */ > if (ret == -ENODEV) > ret = 0; > - goto err_fini; > + return ret; > } > pfdevfreq->opp_of_table_added = true; > > @@ -121,10 +122,8 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > cur_freq = clk_get_rate(pfdev->clock); > > opp = devfreq_recommended_opp(dev, &cur_freq, 0); > - if (IS_ERR(opp)) { > - ret = PTR_ERR(opp); > - goto err_fini; > - } > + if (IS_ERR(opp)) > + return PTR_ERR(opp); > > panfrost_devfreq_profile.initial_freq = cur_freq; > dev_pm_opp_put(opp); > @@ -133,8 +132,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > DEVFREQ_GOV_SIMPLE_ONDEMAND, NULL); > if (IS_ERR(devfreq)) { > DRM_DEV_ERROR(dev, "Couldn't initialize GPU devfreq\n"); > - ret = PTR_ERR(devfreq); > - goto err_fini; > + return PTR_ERR(devfreq); > } > pfdevfreq->devfreq = devfreq; > > @@ -145,10 +143,6 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > pfdevfreq->cooling = cooling; > > return 0; > - > -err_fini: > - panfrost_devfreq_fini(pfdev); > - return ret; > } > > void panfrost_devfreq_fini(struct panfrost_device *pfdev) > @@ -159,14 +153,6 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev) > devfreq_cooling_unregister(pfdevfreq->cooling); > pfdevfreq->cooling = NULL; > } > - > - if (pfdevfreq->opp_of_table_added) { > - dev_pm_opp_of_remove_table(&pfdev->pdev->dev); > - pfdevfreq->opp_of_table_added = false; > - } > - > - dev_pm_opp_put_regulators(pfdevfreq->regulators_opp_table); > - pfdevfreq->regulators_opp_table = NULL; > } > > void panfrost_devfreq_resume(struct panfrost_device *pfdev) > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h b/drivers/gpu/drm/panfrost/panfrost_devfreq.h > index db6ea48e21f9..a51854cc8c06 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h > @@ -15,7 +15,6 @@ struct panfrost_device; > > struct panfrost_devfreq { > struct devfreq *devfreq; > - struct opp_table *regulators_opp_table; > struct thermal_cooling_device *cooling; > bool opp_of_table_added; > >
01.01.2021 19:54, Yangtao Li пишет: > Add devres wrapper for dev_pm_opp_set_supported_hw() to simplify driver > code. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/opp/core.c | 38 ++++++++++++++++++++++++++++++++++++++ > include/linux/pm_opp.h | 8 ++++++++ > 2 files changed, 46 insertions(+) Reviewed-by: Dmitry Osipenko <digetx@gmail.com> Tested-by: Dmitry Osipenko <digetx@gmail.com>
01.01.2021 19:54, Yangtao Li пишет: > Hi, > > This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, > devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, > devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and > devm_pm_opp_register_notifier. Hello Yangtao, Thank you for your effort, looking forward to v2!
20.01.2021 19:01, Dmitry Osipenko пишет: > 01.01.2021 19:54, Yangtao Li пишет: >> Hi, >> >> This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, >> devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, >> devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and >> devm_pm_opp_register_notifier. > > Hello Yangtao, > > Thank you for your effort, looking forward to v2! Yangtao, could you please let me know what is the status of this series? Will you be able to make a v2 anytime soon?
On 02-03-21, 16:40, Dmitry Osipenko wrote: > 20.01.2021 19:01, Dmitry Osipenko пишет: > > 01.01.2021 19:54, Yangtao Li пишет: > >> Hi, > >> > >> This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, > >> devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, > >> devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and > >> devm_pm_opp_register_notifier. > > > > Hello Yangtao, > > > > Thank you for your effort, looking forward to v2! > > Yangtao, could you please let me know what is the status of this series? > Will you be able to make a v2 anytime soon? Dmitry, if Yangtao doesn't reply back this week with a proposal, please go ahead and respin the patches yourself. Thanks.
03.03.2021 07:01, Viresh Kumar пишет: > On 02-03-21, 16:40, Dmitry Osipenko wrote: >> 20.01.2021 19:01, Dmitry Osipenko пишет: >>> 01.01.2021 19:54, Yangtao Li пишет: >>>> Hi, >>>> >>>> This patchset add devm_pm_opp_set_clkname, devm_pm_opp_put_clkname, >>>> devm_pm_opp_set_regulators, devm_pm_opp_put_regulators, >>>> devm_pm_opp_set_supported_hw, devm_pm_opp_of_add_table and >>>> devm_pm_opp_register_notifier. >>> >>> Hello Yangtao, >>> >>> Thank you for your effort, looking forward to v2! >> >> Yangtao, could you please let me know what is the status of this series? >> Will you be able to make a v2 anytime soon? > > Dmitry, if Yangtao doesn't reply back this week with a proposal, please go ahead > and respin the patches yourself. Thanks. > Alright!