| Message ID | 20201030125149.8227-4-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Exynos: Simple QoS for exynos-bus using interconnect | expand |
On Fri, Oct 30, 2020 at 01:51:46PM +0100, Sylwester Nawrocki wrote: > This patch adds registration of a child platform device for the exynos > interconnect driver. It is assumed that the interconnect provider will > only be needed when #interconnect-cells property is present in the bus > DT node, hence the child device will be created only when such a property > is present. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v7, v6: > - none. > > Changes for v5: > - new patch. > --- > drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
Hi Sylwester, On 10/30/20 9:51 PM, Sylwester Nawrocki wrote: > This patch adds registration of a child platform device for the exynos > interconnect driver. It is assumed that the interconnect provider will > only be needed when #interconnect-cells property is present in the bus > DT node, hence the child device will be created only when such a property > is present. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v7, v6: > - none. > > Changes for v5: > - new patch. > --- > drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 1e684a4..ee300ee 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -24,6 +24,7 @@ > > struct exynos_bus { > struct device *dev; > + struct platform_device *icc_pdev; > > struct devfreq *devfreq; > struct devfreq_event_dev **edev; > @@ -156,6 +157,8 @@ static void exynos_bus_exit(struct device *dev) > if (ret < 0) > dev_warn(dev, "failed to disable the devfreq-event devices\n"); > > + platform_device_unregister(bus->icc_pdev); > + > dev_pm_opp_of_remove_table(dev); > clk_disable_unprepare(bus->clk); > if (bus->opp_table) { > @@ -168,6 +171,8 @@ static void exynos_bus_passive_exit(struct device *dev) > { > struct exynos_bus *bus = dev_get_drvdata(dev); > > + platform_device_unregister(bus->icc_pdev); > + > dev_pm_opp_of_remove_table(dev); > clk_disable_unprepare(bus->clk); > } > @@ -432,6 +437,18 @@ static int exynos_bus_probe(struct platform_device *pdev) > if (ret < 0) > goto err; > > + /* Create child platform device for the interconnect provider */ > + if (of_get_property(dev->of_node, "#interconnect-cells", NULL)) { > + bus->icc_pdev = platform_device_register_data( > + dev, "exynos-generic-icc", > + PLATFORM_DEVID_AUTO, NULL, 0); > + > + if (IS_ERR(bus->icc_pdev)) { > + ret = PTR_ERR(bus->icc_pdev); > + goto err; > + } > + } > + > max_state = bus->devfreq->profile->max_state; > min_freq = (bus->devfreq->profile->freq_table[0] / 1000); > max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000); > Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
Hi Sylwester, On 10/30/20 9:51 PM, Sylwester Nawrocki wrote: > This patch adds registration of a child platform device for the exynos > interconnect driver. It is assumed that the interconnect provider will > only be needed when #interconnect-cells property is present in the bus > DT node, hence the child device will be created only when such a property > is present. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v7, v6: > - none. > > Changes for v5: > - new patch. > --- > drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) We don't need to add 'select INTERCONNECT_EXYNOS' in Kconfig? Do you want to remain it as optional according to user? > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 1e684a4..ee300ee 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -24,6 +24,7 @@ > > struct exynos_bus { > struct device *dev; > + struct platform_device *icc_pdev; > > struct devfreq *devfreq; > struct devfreq_event_dev **edev; > @@ -156,6 +157,8 @@ static void exynos_bus_exit(struct device *dev) > if (ret < 0) > dev_warn(dev, "failed to disable the devfreq-event devices\n"); > > + platform_device_unregister(bus->icc_pdev); > + > dev_pm_opp_of_remove_table(dev); > clk_disable_unprepare(bus->clk); > if (bus->opp_table) { > @@ -168,6 +171,8 @@ static void exynos_bus_passive_exit(struct device *dev) > { > struct exynos_bus *bus = dev_get_drvdata(dev); > > + platform_device_unregister(bus->icc_pdev); > + > dev_pm_opp_of_remove_table(dev); > clk_disable_unprepare(bus->clk); > } > @@ -432,6 +437,18 @@ static int exynos_bus_probe(struct platform_device *pdev) > if (ret < 0) > goto err; > > + /* Create child platform device for the interconnect provider */ > + if (of_get_property(dev->of_node, "#interconnect-cells", NULL)) { > + bus->icc_pdev = platform_device_register_data( > + dev, "exynos-generic-icc", > + PLATFORM_DEVID_AUTO, NULL, 0); > + > + if (IS_ERR(bus->icc_pdev)) { > + ret = PTR_ERR(bus->icc_pdev); > + goto err; > + } > + } > + > max_state = bus->devfreq->profile->max_state; > min_freq = (bus->devfreq->profile->freq_table[0] / 1000); > max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000); >
Hi Chanwoo, On 03.11.2020 11:45, Chanwoo Choi wrote: > On 10/30/20 9:51 PM, Sylwester Nawrocki wrote: >> This patch adds registration of a child platform device for the exynos >> interconnect driver. It is assumed that the interconnect provider will >> only be needed when #interconnect-cells property is present in the bus >> DT node, hence the child device will be created only when such a property >> is present. >> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ > We don't need to add 'select INTERCONNECT_EXYNOS' in Kconfig? I think by doing so we could run into some dependency issues. > Do you want to remain it as optional according to user? Yes, I would prefer to keep it selectable through defconfig. Currently it's only needed by one 32-bit ARM board.
On Tue, 3 Nov 2020 at 13:32, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > Hi Chanwoo, > > On 03.11.2020 11:45, Chanwoo Choi wrote: > > On 10/30/20 9:51 PM, Sylwester Nawrocki wrote: > >> This patch adds registration of a child platform device for the exynos > >> interconnect driver. It is assumed that the interconnect provider will > >> only be needed when #interconnect-cells property is present in the bus > >> DT node, hence the child device will be created only when such a property > >> is present. > >> > >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > > >> drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ > > > We don't need to add 'select INTERCONNECT_EXYNOS' in Kconfig? > > I think by doing so we could run into some dependency issues. > > > Do you want to remain it as optional according to user? > Yes, I would prefer to keep it selectable through defconfig. > Currently it's only needed by one 32-bit ARM board. I am fine with it as it is really optional. You could consider then "imply" but then you would need to check for dependencies (the same as with select). Best regards, Krzysztof
Hi Sylwester, On Tue, Nov 3, 2020 at 9:32 PM Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > Hi Chanwoo, > > On 03.11.2020 11:45, Chanwoo Choi wrote: > > On 10/30/20 9:51 PM, Sylwester Nawrocki wrote: > >> This patch adds registration of a child platform device for the exynos > >> interconnect driver. It is assumed that the interconnect provider will > >> only be needed when #interconnect-cells property is present in the bus > >> DT node, hence the child device will be created only when such a property > >> is present. > >> > >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > > >> drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ > > > We don't need to add 'select INTERCONNECT_EXYNOS' in Kconfig? > > I think by doing so we could run into some dependency issues. > > > Do you want to remain it as optional according to user? > Yes, I would prefer to keep it selectable through defconfig. > Currently it's only needed by one 32-bit ARM board. OK.
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index 1e684a4..ee300ee 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -24,6 +24,7 @@ struct exynos_bus { struct device *dev; + struct platform_device *icc_pdev; struct devfreq *devfreq; struct devfreq_event_dev **edev; @@ -156,6 +157,8 @@ static void exynos_bus_exit(struct device *dev) if (ret < 0) dev_warn(dev, "failed to disable the devfreq-event devices\n"); + platform_device_unregister(bus->icc_pdev); + dev_pm_opp_of_remove_table(dev); clk_disable_unprepare(bus->clk); if (bus->opp_table) { @@ -168,6 +171,8 @@ static void exynos_bus_passive_exit(struct device *dev) { struct exynos_bus *bus = dev_get_drvdata(dev); + platform_device_unregister(bus->icc_pdev); + dev_pm_opp_of_remove_table(dev); clk_disable_unprepare(bus->clk); } @@ -432,6 +437,18 @@ static int exynos_bus_probe(struct platform_device *pdev) if (ret < 0) goto err; + /* Create child platform device for the interconnect provider */ + if (of_get_property(dev->of_node, "#interconnect-cells", NULL)) { + bus->icc_pdev = platform_device_register_data( + dev, "exynos-generic-icc", + PLATFORM_DEVID_AUTO, NULL, 0); + + if (IS_ERR(bus->icc_pdev)) { + ret = PTR_ERR(bus->icc_pdev); + goto err; + } + } + max_state = bus->devfreq->profile->max_state; min_freq = (bus->devfreq->profile->freq_table[0] / 1000); max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000);
This patch adds registration of a child platform device for the exynos interconnect driver. It is assumed that the interconnect provider will only be needed when #interconnect-cells property is present in the bus DT node, hence the child device will be created only when such a property is present. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes for v7, v6: - none. Changes for v5: - new patch. --- drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)