| Message ID | 20191217055738.28445-5-cw00.choi@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] PM / devfreq: Add devfreq_get_devfreq_by_node function | expand |
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
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
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?
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
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;
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(-)