On 5/31/21 4:42 PM, Hsin-Yi Wang wrote: > > > On Mon, May 31, 2021 at 3:37 PM Chanwoo Choi <cw00.choi@xxxxxxxxxxx <mailto:cw00.choi@xxxxxxxxxxx>> wrote: > > Hi, > > On 5/31/21 12:22 PM, andrew-sh.cheng wrote: > > On Wed, 2021-05-26 at 12:08 +0900, Chanwoo Choi wrote: > >> Hi, > >> On 5/26/21 11:22 AM, andrew-sh.cheng wrote: > >>> On Thu, 2021-04-08 at 11:47 +0900, Chanwoo Choi wrote: > >>>> On 4/1/21 9:16 AM, Chanwoo Choi wrote: > >>>>> On 3/31/21 10:03 PM, andrew-sh.cheng wrote: > >>>>>> On Wed, 2021-03-31 at 17:35 +0900, Chanwoo Choi wrote: > >>>>>>> On 3/31/21 5:27 PM, Chanwoo Choi wrote: > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> On 3/31/21 5:03 PM, andrew-sh.cheng wrote: > >>>>>>>>> On Thu, 2021-03-25 at 17:14 +0900, Chanwoo Choi wrote: > >>>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>> You are missing to add these patches to linux-pm mailing list. > >>>>>>>>>> Need to send them to linu-pm ML. > >>>>>>>>>> > >>>>>>>>>> Also, before received this series, I tried to clean-up these patches > >>>>>>>>>> on testing branch[1]. So that I add my comment with my clean-up case. > >>>>>>>>>> [1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgAPh9XRRs$ > >>>>>>>>>> > >>>>>>>>>> And 'Saravana Kannan <skannan@xxxxxxxxxxxxxx <mailto:skannan@xxxxxxxxxxxxxx>>' is wrong email address. > >>>>>>>>>> Please update the email or drop this email. > >>>>>>>>> > >>>>>>>>> Hi Chanwoo, > >>>>>>>>> > >>>>>>>>> Thank you for the advices. > >>>>>>>>> I will resend patch v9 (add to linux-pm ML), remove this patch, and note > >>>>>>>>> that my patch set base on > >>>>>>>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$ > >>>>>>>> > >>>>>>>> I has not yet test this patch[1] on devfreq-testing-passive-gov branch. > >>>>>>>> So that if possible, I'd like you to test your patches with this patch[1] > >>>>>>>> and then if there is no problem, could you send the next patches with patch[1]? > >>>>>>>> > >>>>>>>> [1]https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=39c80d11a8f42dd63ecea1e0df595a0ceb83b454__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJR2cQqZs$ > >>>>>>> > >>>>>>> > >>>>>>> Sorry for the confusion. I make the devfreq-testing-passive-gov[1] > >>>>>>> branch based on latest devfreq-next branch. > >>>>>>> [1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$ > >>>>>>> > >>>>>>> First of all, if possible, I want to test them[1] with your patches in this series. > >>>>>>> And then if there are no any problem, please let me know. After confirmed from you, > >>>>>>> I'll send the patches of devfreq-testing-passive-gov[1] branch. > >>>>>>> How about that? > >>>>>>> > >>>>>> Hi Chanwoo~ > >>>>>> > >>>>>> We will use this on Google Chrome project. > >>>>>> Google Hsin-Yi has test your patch + my patch set v8 [2~8] > >>>>>> > >>>>>> make sure cci devfreqs runs with cpufreq. > >>>>>> suspend resume > >>>>>> speedometer2 benchmark > >>>>>> It is okay. > >>>>>> > >>>>>> Please send the patches of devfreq-testing-passive-gov[1] branch. > >>>>>> > >>>>>> I will send patch v9 base on yours latter. > >>>>> > >>>>> Thanks for your test. I'll send the patches today. > >>>> > >>>> I'm sorry for delay because when I tested the patches > >>>> for devfreq parent type on Odroid-xu3, there are some problem > >>>> related to lazy linking of OPP. So I'm trying to analyze them. > >>>> Unfortunately, we need to postpone these patches to next linux > >>>> version. > >>>> > >>> Hi Chanwoo Choi~ > >>> > >>> It is said that you are busy on another task recently. > >>> May I know your plan on this patch? > >>> Thank you. > >> > >> Sorry for late work. I have a question. > >> When I tested exynos-bus.c with adding the 'required-opp' property > >> on odroid-xu3 board. I got some fail about > >> > >> When calling _set_required_opps(), always _set_required_opp() returns > >> -EBUSY error because of following lazy linking case[1]. > >> > >> [1] https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.13-rc3/source/drivers/opp/core.c*L896__;Iw!!CTRNKA9wMg0ARbw!3eNxwDZRy-Ev5BHGxT-BxCz4qrNy0NZohQuBGW36krkwOkl_WX8yBmxlqSk9hxp_kxspMJI$ > >> > >> /* required-opps not fully initialized yet */ > >> if (lazy_linking_pending(opp_table)) > >> return -EBUSY; > >> > >> > >> For calling dev_pm_opp_of_add_table(), lazy_link_required_opp_table() function > >> will be called. But, there is constraint[2]. If is_genpd of opp_table is false, > >> driver/opp/of.c cannot resolve the lazy linking issue. > >> > >> [2] https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.13-rc3/source/drivers/opp/of.c*L386__;Iw!!CTRNKA9wMg0ARbw!3eNxwDZRy-Ev5BHGxT-BxCz4qrNy0NZohQuBGW36krkwOkl_WX8yBmxlqSk9hxp_QFUVY9E$ > >> > >> /* Link required OPPs for all OPPs of the newly added OPP table */ > >> static void lazy_link_required_opp_table(struct opp_table *new_table) > >> { > >> struct opp_table *opp_table, *temp, **required_opp_tables; > >> struct device_node *required_np, *opp_np, *required_table_np; > >> struct dev_pm_opp *opp; > >> int i, ret; > >> > >> /* > >> * We only support genpd's OPPs in the "required-opps" for now, > >> * as we don't know much about other cases. > >> */ > >> if (!new_table->is_genpd) > >> return; > >> > >> Even if this case, there are no problem on your test case? > >> > > > > Hi Chanwoo~ > > Sorry for late reply. > > Yes, we meet similar issue. > > Google member Hsin-Yi had helped deal with this issue on Chrome project. > > > > Patch segment: > > @ /drivers/opp/of.c > > > > /* Link required OPPs for all OPPs of the newly added OPP table */ > > static void lazy_link_required_opp_table(struct opp_table *new_table) > > { > > struct opp_table *opp_table, *temp, **required_opp_tables; > > struct device_node *required_np, *opp_np, *required_table_np; > > struct dev_pm_opp *opp; > > int i, ret; > > > > + /* > > + * We only support genpd's OPPs in the "required-opps" for now, > > + * as we don't know much about other cases. > > + */ > > + if (!new_table->is_genpd) > > + return; > > > > > > Hsin-Yi replied this issue in the discussion list in the original lazy > > link thread: > > https://patchwork.kernel.org/project/linux-pm/patch/20190717222340.137578-4-saravanak@xxxxxxxxxx/#23932203 > > > > Loop Hsin-YI here. > > You can discuss with her if needing more detail. > > > > Thank you both. > > > > Thanks. First of all, we need to resolve and discuss this issue. > > > Hi Chanwoo, > > We think removing the genpd check is sufficient for our use case since we only use the lazy link for opp table translation. Hi Hsin-Yi, IMHO, I think 'is_genpd' checking should be removed for devices except for genpd like as following: diff --git a/drivers/opp/of.c b/drivers/opp/of.c index c582a9ca397b..b54d3a985515 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -201,17 +201,6 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table, lazy = true; continue; } - - /* - * We only support genpd's OPPs in the "required-opps" for now, - * as we don't know how much about other cases. Error out if the - * required OPP doesn't belong to a genpd. - */ - if (!required_opp_tables[i]->is_genpd) { - dev_err(dev, "required-opp doesn't belong to genpd: %pOF\n", - required_np); - goto free_required_tables; - } } /* Let's do the linking later on */ @@ -379,13 +368,6 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) struct dev_pm_opp *opp; int i, ret; - /* - * We only support genpd's OPPs in the "required-opps" for now, - * as we don't know much about other cases. - */ - if (!new_table->is_genpd) - return; - mutex_lock(&opp_table_lock); list_for_each_entry_safe(opp_table, temp, &lazy_opp_tables, lazy) { @@ -874,7 +856,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, return ERR_PTR(-ENOMEM); ret = _read_opp_key(new_opp, opp_table, np, &rate_not_available); - if (ret < 0 && !opp_table->is_genpd) { + if (ret < 0) { dev_err(dev, "%s: opp key field not found\n", __func__); goto free_opp; } -- Best Regards, Chanwoo Choi Samsung Electronics
