Re: [RFC PATCH 04/10] devfreq: rk3399_dmc / rockchip: pm_domains: Register notify to DMC driver.
Hi, As I already commented[1], I think that it is not proper in order to pass the devfreq instance to power_domain driver by separate defined function (rockchip_pm_register_dmcfreq_notifier()). [1] https://patchwork.kernel.org/patch/10349571/ Maybe, you could check the 'OF graph[1]' or 'device connection[2]' for the device connection. Unfortunately, I'm not sure what is best solution for this issue. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/graph.txt [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f2d9b66d84f3ff5ea3aff111e6a403e04fa8bf37 On 2018년 05월 15일 06:16, Enric Balletbo i Serra wrote: > From: Lin Huang> > The DMC (Dynamic Memory Interface) controller does a SiP call to the > Trusted Firmware-A (TF-A) to change the DDR clock frequency. When this > happens the TF-A writes to the PMU bus idle request register > (PMU_BUS_IDLE_REQ) but at the same time it is possible that the Rockchip > power domain driver writes to the same register. So, add a notification > mechanism to ensure that the DMC and the PD driver does not access to this > register at the same time. > > Signed-off-by: Lin Huang > [rewrite commit message] > Signed-off-by: Enric Balletbo i Serra > --- > As I explained in the cover letter I have doubts regarding this patch > but I did not find another way to do it. So I will appreciate any > feedback on this. > > drivers/devfreq/rk3399_dmc.c | 7 ++ > drivers/soc/rockchip/pm_domains.c | 36 +++ > include/soc/rockchip/rk3399_dmc.h | 14 > 3 files changed, 57 insertions(+) > create mode 100644 include/soc/rockchip/rk3399_dmc.h > > diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c > index cc1bbca3fb15..2c4985a501cb 100644 > --- a/drivers/devfreq/rk3399_dmc.c > +++ b/drivers/devfreq/rk3399_dmc.c > @@ -28,6 +28,7 @@ > #include > #include > > +#include > #include > #include > > @@ -443,6 +444,12 @@ static int rk3399_dmcfreq_probe(struct platform_device > *pdev) > data->dev = dev; > platform_set_drvdata(pdev, data); > > + rockchip_pm_register_dmcfreq_notifier(data->devfreq); > + if (ret < 0) { > + dev_err(dev, "Failed to register dmcfreq notifier\n"); > + return ret; > + } > + > return 0; > } > > diff --git a/drivers/soc/rockchip/pm_domains.c > b/drivers/soc/rockchip/pm_domains.c > index 53efc386b1ad..b0e66f24b3e3 100644 > --- a/drivers/soc/rockchip/pm_domains.c > +++ b/drivers/soc/rockchip/pm_domains.c > @@ -8,6 +8,7 @@ > * published by the Free Software Foundation. > */ > > +#include > #include > #include > #include > @@ -76,9 +77,13 @@ struct rockchip_pmu { > const struct rockchip_pmu_info *info; > struct mutex mutex; /* mutex lock for pmu */ > struct genpd_onecell_data genpd_data; > + struct devfreq *devfreq; > + struct notifier_block dmc_nb; > struct generic_pm_domain *domains[]; > }; > > +static struct rockchip_pmu *dmc_pmu; > + > #define to_rockchip_pd(gpd) container_of(gpd, struct rockchip_pm_domain, > genpd) > > #define DOMAIN(pwr, status, req, idle, ack, wakeup) \ > @@ -601,6 +606,35 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu > *pmu, > return error; > } > > +static int rk3399_dmcfreq_notify(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + if (event == DEVFREQ_PRECHANGE) > + mutex_lock(_pmu->mutex); > + else if (event == DEVFREQ_POSTCHANGE) > + mutex_unlock(_pmu->mutex); > + > + return NOTIFY_OK; > +} > + > +int rockchip_pm_register_dmcfreq_notifier(struct devfreq *devfreq) > +{ > + int ret; > + > + if (!dmc_pmu) > + return -EPROBE_DEFER; > + > + dmc_pmu->devfreq = devfreq; > + dmc_pmu->dmc_nb.notifier_call = rk3399_dmcfreq_notify; > + ret = devm_devfreq_register_notifier(devfreq->dev.parent, > + dmc_pmu->devfreq, > + _pmu->dmc_nb, > + DEVFREQ_TRANSITION_NOTIFIER); > + > + return ret; > +} > +EXPORT_SYMBOL(rockchip_pm_register_dmcfreq_notifier); > + > static int rockchip_pm_domain_probe(struct platform_device *pdev) > { > struct device *dev = >dev; > @@ -694,6 +728,8 @@ static int rockchip_pm_domain_probe(struct > platform_device *pdev) > goto err_out; > } > > + dmc_pmu = pmu; > + > return 0; > > err_out: > diff --git a/include/soc/rockchip/rk3399_dmc.h > b/include/soc/rockchip/rk3399_dmc.h > new file mode 100644 > index ..031a62607f61 > --- /dev/null > +++ b/include/soc/rockchip/rk3399_dmc.h > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c)
Re: [RFC PATCH 04/10] devfreq: rk3399_dmc / rockchip: pm_domains: Register notify to DMC driver.
Hi, As I already commented[1], I think that it is not proper in order to pass the devfreq instance to power_domain driver by separate defined function (rockchip_pm_register_dmcfreq_notifier()). [1] https://patchwork.kernel.org/patch/10349571/ Maybe, you could check the 'OF graph[1]' or 'device connection[2]' for the device connection. Unfortunately, I'm not sure what is best solution for this issue. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/graph.txt [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f2d9b66d84f3ff5ea3aff111e6a403e04fa8bf37 On 2018년 05월 15일 06:16, Enric Balletbo i Serra wrote: > From: Lin Huang > > The DMC (Dynamic Memory Interface) controller does a SiP call to the > Trusted Firmware-A (TF-A) to change the DDR clock frequency. When this > happens the TF-A writes to the PMU bus idle request register > (PMU_BUS_IDLE_REQ) but at the same time it is possible that the Rockchip > power domain driver writes to the same register. So, add a notification > mechanism to ensure that the DMC and the PD driver does not access to this > register at the same time. > > Signed-off-by: Lin Huang > [rewrite commit message] > Signed-off-by: Enric Balletbo i Serra > --- > As I explained in the cover letter I have doubts regarding this patch > but I did not find another way to do it. So I will appreciate any > feedback on this. > > drivers/devfreq/rk3399_dmc.c | 7 ++ > drivers/soc/rockchip/pm_domains.c | 36 +++ > include/soc/rockchip/rk3399_dmc.h | 14 > 3 files changed, 57 insertions(+) > create mode 100644 include/soc/rockchip/rk3399_dmc.h > > diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c > index cc1bbca3fb15..2c4985a501cb 100644 > --- a/drivers/devfreq/rk3399_dmc.c > +++ b/drivers/devfreq/rk3399_dmc.c > @@ -28,6 +28,7 @@ > #include > #include > > +#include > #include > #include > > @@ -443,6 +444,12 @@ static int rk3399_dmcfreq_probe(struct platform_device > *pdev) > data->dev = dev; > platform_set_drvdata(pdev, data); > > + rockchip_pm_register_dmcfreq_notifier(data->devfreq); > + if (ret < 0) { > + dev_err(dev, "Failed to register dmcfreq notifier\n"); > + return ret; > + } > + > return 0; > } > > diff --git a/drivers/soc/rockchip/pm_domains.c > b/drivers/soc/rockchip/pm_domains.c > index 53efc386b1ad..b0e66f24b3e3 100644 > --- a/drivers/soc/rockchip/pm_domains.c > +++ b/drivers/soc/rockchip/pm_domains.c > @@ -8,6 +8,7 @@ > * published by the Free Software Foundation. > */ > > +#include > #include > #include > #include > @@ -76,9 +77,13 @@ struct rockchip_pmu { > const struct rockchip_pmu_info *info; > struct mutex mutex; /* mutex lock for pmu */ > struct genpd_onecell_data genpd_data; > + struct devfreq *devfreq; > + struct notifier_block dmc_nb; > struct generic_pm_domain *domains[]; > }; > > +static struct rockchip_pmu *dmc_pmu; > + > #define to_rockchip_pd(gpd) container_of(gpd, struct rockchip_pm_domain, > genpd) > > #define DOMAIN(pwr, status, req, idle, ack, wakeup) \ > @@ -601,6 +606,35 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu > *pmu, > return error; > } > > +static int rk3399_dmcfreq_notify(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + if (event == DEVFREQ_PRECHANGE) > + mutex_lock(_pmu->mutex); > + else if (event == DEVFREQ_POSTCHANGE) > + mutex_unlock(_pmu->mutex); > + > + return NOTIFY_OK; > +} > + > +int rockchip_pm_register_dmcfreq_notifier(struct devfreq *devfreq) > +{ > + int ret; > + > + if (!dmc_pmu) > + return -EPROBE_DEFER; > + > + dmc_pmu->devfreq = devfreq; > + dmc_pmu->dmc_nb.notifier_call = rk3399_dmcfreq_notify; > + ret = devm_devfreq_register_notifier(devfreq->dev.parent, > + dmc_pmu->devfreq, > + _pmu->dmc_nb, > + DEVFREQ_TRANSITION_NOTIFIER); > + > + return ret; > +} > +EXPORT_SYMBOL(rockchip_pm_register_dmcfreq_notifier); > + > static int rockchip_pm_domain_probe(struct platform_device *pdev) > { > struct device *dev = >dev; > @@ -694,6 +728,8 @@ static int rockchip_pm_domain_probe(struct > platform_device *pdev) > goto err_out; > } > > + dmc_pmu = pmu; > + > return 0; > > err_out: > diff --git a/include/soc/rockchip/rk3399_dmc.h > b/include/soc/rockchip/rk3399_dmc.h > new file mode 100644 > index ..031a62607f61 > --- /dev/null > +++ b/include/soc/rockchip/rk3399_dmc.h > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd > + * Author: Lin Huang > +