[4/9] PM / devfreq: exynos-bus: Replace deprecated 'devfreq' property
diff mbox series

Message ID 20191217055738.28445-5-cw00.choi@samsung.com
State New
Headers show
Series
  • [1/9] PM / devfreq: Add devfreq_get_devfreq_by_node function
Related show

Commit Message

Chanwoo Choi Dec. 17, 2019, 5:57 a.m. UTC
In order to remove the deprecated 'devfreq' property, replace with
new 'exynos,parent-bus' property in order to get the parent devfreq device
in devicetree file instead of 'devfreq' property. But, to guarantee the
backward-compatibility, keep the support 'devfreq' property.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 .../bindings/devfreq/exynos-bus.txt           | 16 +++++++--------
 drivers/devfreq/exynos-bus.c                  | 20 ++++++++++++-------
 2 files changed, 21 insertions(+), 15 deletions(-)

Comments

Rob Herring Dec. 26, 2019, 9:01 p.m. UTC | #1
On Tue, Dec 17, 2019 at 02:57:33PM +0900, Chanwoo Choi wrote:
> In order to remove the deprecated 'devfreq' property, replace with
> new 'exynos,parent-bus' property in order to get the parent devfreq device
> in devicetree file instead of 'devfreq' property. But, to guarantee the
> backward-compatibility, keep the support 'devfreq' property.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  .../bindings/devfreq/exynos-bus.txt           | 16 +++++++--------
>  drivers/devfreq/exynos-bus.c                  | 20 ++++++++++++-------
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> index e71f752cc18f..c948cee01124 100644
> --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> @@ -45,7 +45,7 @@ Required properties only for parent bus device:
>    of buses.
>  
>  Required properties only for passive bus device:
> -- devfreq: the parent bus device.
> +- exynos,parent-bus: the parent bus device.

If you are going to do something new, why not use the interconnect 
binding here?

Rob
Chanwoo Choi Dec. 27, 2019, 12:09 a.m. UTC | #2
On 12/27/19 6:01 AM, Rob Herring wrote:
> On Tue, Dec 17, 2019 at 02:57:33PM +0900, Chanwoo Choi wrote:
>> In order to remove the deprecated 'devfreq' property, replace with
>> new 'exynos,parent-bus' property in order to get the parent devfreq device
>> in devicetree file instead of 'devfreq' property. But, to guarantee the
>> backward-compatibility, keep the support 'devfreq' property.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  .../bindings/devfreq/exynos-bus.txt           | 16 +++++++--------
>>  drivers/devfreq/exynos-bus.c                  | 20 ++++++++++++-------
>>  2 files changed, 21 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>> index e71f752cc18f..c948cee01124 100644
>> --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>> @@ -45,7 +45,7 @@ Required properties only for parent bus device:
>>    of buses.
>>  
>>  Required properties only for passive bus device:
>> -- devfreq: the parent bus device.
>> +- exynos,parent-bus: the parent bus device.
> 
> If you are going to do something new, why not use the interconnect 
> binding here?

As I knew, interconnect make the data path among multiple nodes
and set the average and peak bandwidth to the specific data path.

It means that some data will be flowed from node_a to node_d
or the reverse way because each node has the tightly coupled
dependency for data flow.

	node_a <-> node_b <-> node_c <-> node_d


On the other hand, exynos-bus.c driver is not related to 'data path'.
Each bus just need to control the their own frequency and voltage.
But, share the power line (regulator) between exynos-bus device
even if there are no any dependency of data flow.

'exynos,parent-bus' property just indicate the specific
devfreq device(parent bus device) which controls
the shared power line(regulator) in order to prevent
the h/w problem due to the wrong pair of frequency and voltage.

'exynos,parent-bus' property is only used to catch
the change timing of shared power line.


And,
as you commented, there are some data path among the exynos-bus
devices for the display h/w as following:

	bus_display -> bus_leftbus -> bus_dmc

In order to make the data path between bus devices,
interconnect binding is required. This approach[1] was posted.
[1] https://patchwork.kernel.org/cover/11305265/
- [RFC,v3,0/7] PM / devfreq: Simple QoS for exynos-bus using interconnect
Chanwoo Choi Jan. 6, 2020, 1:38 a.m. UTC | #3
Hi Rob,

Gently Ping.

On 12/27/19 9:09 AM, Chanwoo Choi wrote:
> On 12/27/19 6:01 AM, Rob Herring wrote:
>> On Tue, Dec 17, 2019 at 02:57:33PM +0900, Chanwoo Choi wrote:
>>> In order to remove the deprecated 'devfreq' property, replace with
>>> new 'exynos,parent-bus' property in order to get the parent devfreq device
>>> in devicetree file instead of 'devfreq' property. But, to guarantee the
>>> backward-compatibility, keep the support 'devfreq' property.
>>>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> ---
>>>  .../bindings/devfreq/exynos-bus.txt           | 16 +++++++--------
>>>  drivers/devfreq/exynos-bus.c                  | 20 ++++++++++++-------
>>>  2 files changed, 21 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>> index e71f752cc18f..c948cee01124 100644
>>> --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>> @@ -45,7 +45,7 @@ Required properties only for parent bus device:
>>>    of buses.
>>>  
>>>  Required properties only for passive bus device:
>>> -- devfreq: the parent bus device.
>>> +- exynos,parent-bus: the parent bus device.
>>
>> If you are going to do something new, why not use the interconnect 
>> binding here?
> 
> As I knew, interconnect make the data path among multiple nodes
> and set the average and peak bandwidth to the specific data path.
> 
> It means that some data will be flowed from node_a to node_d
> or the reverse way because each node has the tightly coupled
> dependency for data flow.
> 
> 	node_a <-> node_b <-> node_c <-> node_d
> 
> 
> On the other hand, exynos-bus.c driver is not related to 'data path'.
> Each bus just need to control the their own frequency and voltage.
> But, share the power line (regulator) between exynos-bus device
> even if there are no any dependency of data flow.
> 
> 'exynos,parent-bus' property just indicate the specific
> devfreq device(parent bus device) which controls
> the shared power line(regulator) in order to prevent
> the h/w problem due to the wrong pair of frequency and voltage.
> 
> 'exynos,parent-bus' property is only used to catch
> the change timing of shared power line.
> 
> 
> And,
> as you commented, there are some data path among the exynos-bus
> devices for the display h/w as following:
> 
> 	bus_display -> bus_leftbus -> bus_dmc
> 
> In order to make the data path between bus devices,
> interconnect binding is required. This approach[1] was posted.
> [1] https://patchwork.kernel.org/cover/11305265/
> - [RFC,v3,0/7] PM / devfreq: Simple QoS for exynos-bus using interconnect
> 

Are there any other commentss?
Chanwoo Choi Jan. 14, 2020, 6:57 p.m. UTC | #4
Hi Rob,

On Mon, Jan 6, 2020 at 10:32 AM Chanwoo Choi <cw00.choi@samsung.com> wrote:
>
> Hi Rob,
>
> Gently Ping.

Once again, ping. Could you please review?

On v2[1], made separate patches for dt-binding.
[1] https://patchwork.kernel.org/cover/11304545/

>
> On 12/27/19 9:09 AM, Chanwoo Choi wrote:
> > On 12/27/19 6:01 AM, Rob Herring wrote:
> >> On Tue, Dec 17, 2019 at 02:57:33PM +0900, Chanwoo Choi wrote:
> >>> In order to remove the deprecated 'devfreq' property, replace with
> >>> new 'exynos,parent-bus' property in order to get the parent devfreq device
> >>> in devicetree file instead of 'devfreq' property. But, to guarantee the
> >>> backward-compatibility, keep the support 'devfreq' property.
> >>>
> >>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> >>> ---
> >>>  .../bindings/devfreq/exynos-bus.txt           | 16 +++++++--------
> >>>  drivers/devfreq/exynos-bus.c                  | 20 ++++++++++++-------
> >>>  2 files changed, 21 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> >>> index e71f752cc18f..c948cee01124 100644
> >>> --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> >>> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> >>> @@ -45,7 +45,7 @@ Required properties only for parent bus device:
> >>>    of buses.
> >>>
> >>>  Required properties only for passive bus device:
> >>> -- devfreq: the parent bus device.
> >>> +- exynos,parent-bus: the parent bus device.
> >>
> >> If you are going to do something new, why not use the interconnect
> >> binding here?
> >
> > As I knew, interconnect make the data path among multiple nodes
> > and set the average and peak bandwidth to the specific data path.
> >
> > It means that some data will be flowed from node_a to node_d
> > or the reverse way because each node has the tightly coupled
> > dependency for data flow.
> >
> >       node_a <-> node_b <-> node_c <-> node_d
> >
> >
> > On the other hand, exynos-bus.c driver is not related to 'data path'.
> > Each bus just need to control the their own frequency and voltage.
> > But, share the power line (regulator) between exynos-bus device
> > even if there are no any dependency of data flow.
> >
> > 'exynos,parent-bus' property just indicate the specific
> > devfreq device(parent bus device) which controls
> > the shared power line(regulator) in order to prevent
> > the h/w problem due to the wrong pair of frequency and voltage.
> >
> > 'exynos,parent-bus' property is only used to catch
> > the change timing of shared power line.
> >
> >
> > And,
> > as you commented, there are some data path among the exynos-bus
> > devices for the display h/w as following:
> >
> >       bus_display -> bus_leftbus -> bus_dmc
> >
> > In order to make the data path between bus devices,
> > interconnect binding is required. This approach[1] was posted.
> > [1] https://patchwork.kernel.org/cover/11305265/
> > - [RFC,v3,0/7] PM / devfreq: Simple QoS for exynos-bus using interconnect
> >
>
> Are there any other commentss?
>
>
> --
> Best Regards,
> Chanwoo Choi
> Samsung Electronics

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
index e71f752cc18f..c948cee01124 100644
--- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
+++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
@@ -45,7 +45,7 @@  Required properties only for parent bus device:
   of buses.
 
 Required properties only for passive bus device:
-- devfreq: the parent bus device.
+- exynos,parent-bus: the parent bus device.
 
 Optional properties only for parent bus device:
 - exynos,saturation-ratio: the percentage value which is used to calibrate
@@ -386,36 +386,36 @@  Example2 :
 	};
 
 	&bus_rightbus {
-		devfreq = <&bus_leftbus>;
+		exynos,parent-bus = <&bus_leftbus>;
 		status = "okay";
 	};
 
 	&bus_lcd0 {
-		devfreq = <&bus_leftbus>;
+		exynos,parent-bus = <&bus_leftbus>;
 		status = "okay";
 	};
 
 	&bus_fsys {
-		devfreq = <&bus_leftbus>;
+		exynos,parent-bus = <&bus_leftbus>;
 		status = "okay";
 	};
 
 	&bus_mcuisp {
-		devfreq = <&bus_leftbus>;
+		exynos,parent-bus = <&bus_leftbus>;
 		status = "okay";
 	};
 
 	&bus_isp {
-		devfreq = <&bus_leftbus>;
+		exynos,parent-bus = <&bus_leftbus>;
 		status = "okay";
 	};
 
 	&bus_peril {
-		devfreq = <&bus_leftbus>;
+		exynos,parent-bus = <&bus_leftbus>;
 		status = "okay";
 	};
 
 	&bus_mfc {
-		devfreq = <&bus_leftbus>;
+		exynos,parent-bus = <&bus_leftbus>;
 		status = "okay";
 	};
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 7893c3b99e60..60d61b168153 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -148,11 +148,17 @@  static int exynos_bus_get_dev_status(struct device *dev,
 
 static struct devfreq *get_parent_devfreq_by_node(struct device_node *np)
 {
-	struct device_node *node = of_parse_phandle(np, "devfreq", 0);
-
-	if (!node)
-		return ERR_PTR(-ENODEV);
-
+	struct device_node *node = of_parse_phandle(np, "exynos,parent-bus", 0);
+
+	if (!node) {
+		/*
+		 * Check the deprecated 'devfreq' property
+		 * to support backward-compatibility.
+		 */
+		node = of_parse_phandle(np, "devfreq", 0);
+		if (!node)
+			return ERR_PTR(-ENODEV);
+	}
 	return devfreq_get_devfreq_by_node(node);
 }
 
@@ -376,7 +382,7 @@  static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
 
 	/* Get the instance of parent devfreq device */
 	parent_devfreq = get_parent_devfreq_by_node(dev->of_node);
-	if (IS_ERR(parent_devfreq)) {
+	if (IS_ERR(parent_devfreq))
 		return -EPROBE_DEFER;
 
 	passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
@@ -423,7 +429,7 @@  static int exynos_bus_probe(struct platform_device *pdev)
 	if (!profile)
 		return -ENOMEM;
 
-	node = of_parse_phandle(dev->of_node, "devfreq", 0);
+	node = of_parse_phandle(dev->of_node, "exynos,parent-bus", 0);
 	if (node) {
 		of_node_put(node);
 		passive = true;