diff mbox series

[v7,3/6] PM / devfreq: exynos-bus: Add registration of interconnect child device

Message ID 20201030125149.8227-4-s.nawrocki@samsung.com (mailing list archive)
State New, archived
Headers show
Series [v7,1/6] dt-bindings: devfreq: Add documentation for the interconnect properties | expand

Commit Message

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(+)

Comments

Krzysztof Kozlowski Oct. 31, 2020, 12:40 p.m. UTC | #1
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
Chanwoo Choi Nov. 2, 2020, 4:28 a.m. UTC | #2
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>
Chanwoo Choi Nov. 3, 2020, 10:45 a.m. UTC | #3
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.
Krzysztof Kozlowski Nov. 3, 2020, 1:11 p.m. UTC | #5
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
Chanwoo Choi Nov. 3, 2020, 2:07 p.m. UTC | #6
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 mbox series

Patch

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);