[GIT PULL] extcon next for v5.13
Dear Greg, This is extcon-next pull request for v5.13. I add detailed description of this pull request on below. Please pull extcon with following updates. Best Regards, Chanwoo Choi The following changes since commit e49d033bddf5b565044e2abe4241353959bc9120: Linux 5.12-rc6 (2021-04-04 14:15:36 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git tags/extcon-next-for-5.13 for you to fetch changes up to 7b1222b224aff41739319ae81cd266825464ad8f: extcon: qcom-spmi: Add support for VBUS detection (2021-04-08 13:10:16 +0900) Detailed description for this pull request: 1. Update extcon provider driver - Add the support of charging interrupt to detect charger connector for extcon-max8997.c - Detect OTG when USB_ID pin is connected to ground for extcon-sm5502.c - Add the support for VBUS detection for extcon-qcom-spmi-misc.c and replace qcom,pm8941-misc binding document with yaml style. Anirudh Ghayal (1): extcon: qcom-spmi: Add support for VBUS detection Guru Das Srinagesh (2): bindings: pm8941-misc: Convert bindings to YAML bindings: pm8941-misc: Add support for VBUS detection Nikita Travkin (1): extcon: sm5502: Detect OTG when USB_ID is connected to ground Timon Baetz (1): extcon: max8997: Add CHGINS and CHGRM interrupt handling .../bindings/extcon/qcom,pm8941-misc.txt | 41 - .../bindings/extcon/qcom,pm8941-misc.yaml | 62 ++ drivers/extcon/extcon-max8997.c| 4 + drivers/extcon/extcon-qcom-spmi-misc.c | 99 +- drivers/extcon/extcon-sm5502.c | 22 - 5 files changed, 166 insertions(+), 62 deletions(-) delete mode 100644 Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.txt create mode 100644 Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml
[GIT PULL] devfreq next for v5.13
Dear Rafael, This is devfreq-next pull request for v5.13-rc1. I add detailed description of this pull request on the following tag. Please pull devfreq with following updates. - tag name : devfreq-next-for-5.12 This pull request contains the immutable branch to keep the immutable patch[1] between devfreq and drm for gpu driver. [1] https://patchwork.kernel.org/project/linux-pm/patch/20210308133041.10516-1-daniel.lezc...@linaro.org/ Best Regards, Chanwoo Choi The following changes since commit e49d033bddf5b565044e2abe4241353959bc9120: Linux 5.12-rc6 (2021-04-04 14:15:36 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git tags/devfreq-next-for-5.13 for you to fetch changes up to 0a7dc8318c2817fb33dc50946f7ca6e0ff28f036: PM / devfreq: imx8m-ddrc: Remove unneeded of_match_ptr() (2021-04-08 13:14:51 +0900) Detailed description for this pull request: 1. Update devfreq core - Register devfreq as a cooling device when adding devfreq device basically if the flag of devfreq device is set as cooling device. - Add missing unlock opeation in devfreq_add_device - Use the next frequency as resume_freq instead of previous frequency when using opp-suspend property - Check whether .get_dev_status in struct devfreq_dev_profile is implemented or not by devfreq driver before calling .get_dev_status function. 2. Update devfreq driver - Use dev_err_probe to simplify the error handling on rk3399_dmc.c - Remove unused .get_dev_status function on imx8m-ddrc.c because imx8m_ddrc_get_dev_status has not returend the any current status of device. - Remove unneeded of_match_ptr() from imx-bus.c and imx8m-ddrc.ca - Add missing phandle informatioon and replace undefined variable with integer value for rk3399_dmc.txt devicetree binding document. 3. Update minor issue - Fix the wrong set_freq path of userspace governor in drivers/devfreq/Kconfig - Remove the invalid description from .get_target_freq in struct devfreq_dev_profile Chanwoo Choi (1): Merge branch 'immutable-devfreq-v5.13-rc1' into devfreq-next Daniel Lezcano (1): PM / devfreq: Register devfreq as a cooling device on demand Dong Aisheng (5): PM / devfreq: Use more accurate returned new_freq as resume_freq PM / devfreq: Fix the wrong set_freq path for userspace governor in Kconfig PM / devfreq: Check get_dev_status in devfreq_update_stats PM / devfreq: Remove the invalid description for get_target_freq PM / devfreq: imx8m-ddrc: Remove imx8m_ddrc_get_dev_status Enric Balletbo i Serra (1): dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle. Fabio Estevam (2): PM / devfreq: imx-bus: Remove unneeded of_match_ptr() PM / devfreq: imx8m-ddrc: Remove unneeded of_match_ptr() Gaël PORTAY (1): dt-bindings: devfreq: rk3399_dmc: Remove references of unexistant defines Krzysztof Kozlowski (1): PM / devfreq: rk3399_dmc: Simplify with dev_err_probe() Lukasz Luba (1): PM / devfreq: Unlock mutex and free devfreq struct in error path Documentation/ABI/testing/sysfs-class-devfreq | 5 +- .../devicetree/bindings/devfreq/rk3399_dmc.txt | 75 +++--- drivers/devfreq/Kconfig| 2 +- drivers/devfreq/devfreq.c | 14 +++- drivers/devfreq/governor.h | 5 +- drivers/devfreq/imx-bus.c | 2 +- drivers/devfreq/imx8m-ddrc.c | 16 + drivers/devfreq/rk3399_dmc.c | 20 ++ include/linux/devfreq.h| 9 +++ 9 files changed, 70 insertions(+), 78 deletions(-)
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
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 ' 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=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. [snip] -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
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 ' 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=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. > > >> >>> >>>> >>>> >>>>> >>>>> >>>>> On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote: >>>>>> From: Saravana Kannan >>>>>> >>>>>> Many CPU architectures have caches that can scale independent of the >>>>>> CPUs. Frequency scaling of the caches is necessary to make sure that the >>>>>> cache is not a performance bottleneck that leads to poor performance and >>>>>> power. The same idea applies for RAM/DDR. >>>>>> >>>>>> To achieve this, this patch adds support for cpu based scaling to the >>>>>> passive governor. This is accomplished by taking the current frequency >>>>>> of each CPU frequency domain and then adjust the frequency of the cache >>>>>> (or any devfreq device) based on the frequency of the CPUs. It listens >>>>>> to CPU frequency transition notifiers to keep itself up to date on the >>>>>> current CPU frequency. >>>>>> >>>>>> To decide the frequency of the device, the governor does one of the >>>>>> following: >>>>>> * Derives the optimal devfreq device opp from required-opps property of >>>>>> the parent cpu opp_table. >>>>>> >>>>>> * Scales the device frequency in proportion to the CPU frequency.
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
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 ' 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://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov > > 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://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov=39c80d11a8f42dd63ecea1e0df595a0ceb83b454 Sorry for the confusion. I make the devfreq-testing-passive-gov[1] branch based on latest devfreq-next branch. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov 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? > >> >> >>> >>> >>> On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote: >>>> From: Saravana Kannan >>>> >>>> Many CPU architectures have caches that can scale independent of the >>>> CPUs. Frequency scaling of the caches is necessary to make sure that the >>>> cache is not a performance bottleneck that leads to poor performance and >>>> power. The same idea applies for RAM/DDR. >>>> >>>> To achieve this, this patch adds support for cpu based scaling to the >>>> passive governor. This is accomplished by taking the current frequency >>>> of each CPU frequency domain and then adjust the frequency of the cache >>>> (or any devfreq device) based on the frequency of the CPUs. It listens >>>> to CPU frequency transition notifiers to keep itself up to date on the >>>> current CPU frequency. >>>> >>>> To decide the frequency of the device, the governor does one of the >>>> following: >>>> * Derives the optimal devfreq device opp from required-opps property of >>>> the parent cpu opp_table. >>>> >>>> * Scales the device frequency in proportion to the CPU frequency. So, if >>>> the CPUs are running at their max frequency, the device runs at its >>>> max frequency. If the CPUs are running at their min frequency, the >>>> device runs at its min frequency. It is interpolated for frequencies >>>> in between. >>>> >>>> Andrew-sh.Cheng change >>>> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq >>>> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value >>>> after kernel-5.7 >>>> Don't return -EINVAL in devfreq_passive_event_handler() >>>> since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases. >>>> >>>> Signed-off-by: Saravana Kannan >>>> [Sibi: Integrated cpu-freqmap governor into passive_governor] >>>> Signed-off-by: Sibi Sankar >>>> Signed-off-by: Andrew-sh.Cheng >>>> --- >>>> drivers/devfreq/Kconfig| 2 + >>>> drivers/devfreq/governor_passive.c | 329 >>>> +++-- >>>> include/linux/devfreq.h| 29 +++- >>>> 3 files changed, 342 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig >>>> index 00704efe6398..f56132b0ae64 100644 >>>> --- a/drivers/devfreq/Kconfig >>>> +++ b/drivers/devfreq/Kconfig >&
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
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 ' 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://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov 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://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov=39c80d11a8f42dd63ecea1e0df595a0ceb83b454 > > >> >> >> On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote: >>> From: Saravana Kannan >>> >>> Many CPU architectures have caches that can scale independent of the >>> CPUs. Frequency scaling of the caches is necessary to make sure that the >>> cache is not a performance bottleneck that leads to poor performance and >>> power. The same idea applies for RAM/DDR. >>> >>> To achieve this, this patch adds support for cpu based scaling to the >>> passive governor. This is accomplished by taking the current frequency >>> of each CPU frequency domain and then adjust the frequency of the cache >>> (or any devfreq device) based on the frequency of the CPUs. It listens >>> to CPU frequency transition notifiers to keep itself up to date on the >>> current CPU frequency. >>> >>> To decide the frequency of the device, the governor does one of the >>> following: >>> * Derives the optimal devfreq device opp from required-opps property of >>> the parent cpu opp_table. >>> >>> * Scales the device frequency in proportion to the CPU frequency. So, if >>> the CPUs are running at their max frequency, the device runs at its >>> max frequency. If the CPUs are running at their min frequency, the >>> device runs at its min frequency. It is interpolated for frequencies >>> in between. >>> >>> Andrew-sh.Cheng change >>> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq >>> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value >>> after kernel-5.7 >>> Don't return -EINVAL in devfreq_passive_event_handler() >>> since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases. >>> >>> Signed-off-by: Saravana Kannan >>> [Sibi: Integrated cpu-freqmap governor into passive_governor] >>> Signed-off-by: Sibi Sankar >>> Signed-off-by: Andrew-sh.Cheng >>> --- >>> drivers/devfreq/Kconfig| 2 + >>> drivers/devfreq/governor_passive.c | 329 >>> +++-- >>> include/linux/devfreq.h| 29 +++- >>> 3 files changed, 342 insertions(+), 18 deletions(-) >>> >>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig >>> index 00704efe6398..f56132b0ae64 100644 >>> --- a/drivers/devfreq/Kconfig >>> +++ b/drivers/devfreq/Kconfig >>> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE >>> device. This governor does not change the frequency by itself >>> through sysfs entries. The passive governor recommends that >>> devfreq device uses the OPP table to get the frequency/voltage. >>> + Alternatively the governor can also be chosen to scale based on >>> + the online CPUs current frequency. >>> >>> comment "DEVFREQ Drivers" >>> >>> diff --git a/drivers/devfreq/governor_passive.c >>> b/drivers/devfreq/governor_passive.c >>> index b094132bd20b..9cc57b083839 100644 >>> --- a/drivers/devfreq/governor_passive.c >>> +++ b/drivers/devfreq/governor_passive.c >>> @@ -8,11 +8,103 @@ >>> */ >>> >>
Re: [PATCH V8 7/8] devfreq: mediatek: cci devfreq register opp notification for SVS support
Hi,
I think that you can squash this patch to patch4.
On 3/23/21 8:34 PM, Andrew-sh.Cheng wrote:
> From: "Andrew-sh.Cheng"
>
> SVS will change the voltage of opp item.
What it the full name of SVS?
> CCI devfreq need to react to change frequency.
>
> Signed-off-by: Andrew-sh.Cheng
> ---
> drivers/devfreq/mt8183-cci-devfreq.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c
> b/drivers/devfreq/mt8183-cci-devfreq.c
> index 018543db7bae..6942a48f3f4f 100644
> --- a/drivers/devfreq/mt8183-cci-devfreq.c
> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
> @@ -21,6 +21,7 @@ struct cci_devfreq {
> struct clk *cci_clk;
> int old_vproc;
> unsigned long old_freq;
> + struct notifier_block opp_nb;
> };
>
> static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
> @@ -89,6 +90,26 @@ static int mtk_cci_devfreq_target(struct device *dev,
> unsigned long *freq,
> return 0;
> }
>
> +static int ccidevfreq_opp_notifier(struct notifier_block *nb,
I think that you better to change the function name as following:
ccidevfreq_opp_notifier -> mtk_cci_devfreq_opp_notifier
> +unsigned long event, void *data)
> +{
> + struct dev_pm_opp *opp = data;
> + struct cci_devfreq *cci_df = container_of(nb, struct cci_devfreq,
> + opp_nb);
> + unsigned long freq, volt;
> +
> + if (event == OPP_EVENT_ADJUST_VOLTAGE) {
> + freq = dev_pm_opp_get_freq(opp);
> + /* current opp item is changed */
> + if (freq == cci_df->old_freq) {
> + volt = dev_pm_opp_get_voltage(opp);
> + mtk_cci_set_voltage(cci_df, volt);
> + }
> + }
> +
> + return 0;
> +}
> +
> static struct devfreq_dev_profile cci_devfreq_profile = {
> .target = mtk_cci_devfreq_target,
> };
> @@ -98,12 +119,15 @@ static int mtk_cci_devfreq_probe(struct platform_device
> *pdev)
> struct device *cci_dev = >dev;
> struct cci_devfreq *cci_df;
> struct devfreq_passive_data *passive_data;
> + struct notifier_block *opp_nb;
> int ret;
>
> cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
> if (!cci_df)
> return -ENOMEM;
>
> + opp_nb = _df->opp_nb;
Just move this code at the neighborhood of 'opp_nb->notifier_call' init code.
> +
> cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
> ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
> if (ret) {
> @@ -152,6 +176,9 @@ static int mtk_cci_devfreq_probe(struct platform_device
> *pdev)
> goto err_opp;
> }
>
> + opp_nb->notifier_call = ccidevfreq_opp_notifier;
> + dev_pm_opp_register_notifier(cci_dev, opp_nb);
Need to check whether return value is valid or not.
> +
> return 0;
>
> err_opp:
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
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://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
And 'Saravana Kannan ' is wrong email address.
Please update the email or drop this email.
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
> From: Saravana Kannan
>
> Many CPU architectures have caches that can scale independent of the
> CPUs. Frequency scaling of the caches is necessary to make sure that the
> cache is not a performance bottleneck that leads to poor performance and
> power. The same idea applies for RAM/DDR.
>
> To achieve this, this patch adds support for cpu based scaling to the
> passive governor. This is accomplished by taking the current frequency
> of each CPU frequency domain and then adjust the frequency of the cache
> (or any devfreq device) based on the frequency of the CPUs. It listens
> to CPU frequency transition notifiers to keep itself up to date on the
> current CPU frequency.
>
> To decide the frequency of the device, the governor does one of the
> following:
> * Derives the optimal devfreq device opp from required-opps property of
> the parent cpu opp_table.
>
> * Scales the device frequency in proportion to the CPU frequency. So, if
> the CPUs are running at their max frequency, the device runs at its
> max frequency. If the CPUs are running at their min frequency, the
> device runs at its min frequency. It is interpolated for frequencies
> in between.
>
> Andrew-sh.Cheng change
> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
> after kernel-5.7
> Don't return -EINVAL in devfreq_passive_event_handler()
> since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
>
> Signed-off-by: Saravana Kannan
> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> Signed-off-by: Sibi Sankar
> Signed-off-by: Andrew-sh.Cheng
> ---
> drivers/devfreq/Kconfig| 2 +
> drivers/devfreq/governor_passive.c | 329
> +++--
> include/linux/devfreq.h| 29 +++-
> 3 files changed, 342 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 00704efe6398..f56132b0ae64 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
> device. This governor does not change the frequency by itself
> through sysfs entries. The passive governor recommends that
> devfreq device uses the OPP table to get the frequency/voltage.
> + Alternatively the governor can also be chosen to scale based on
> + the online CPUs current frequency.
>
> comment "DEVFREQ Drivers"
>
> diff --git a/drivers/devfreq/governor_passive.c
> b/drivers/devfreq/governor_passive.c
> index b094132bd20b..9cc57b083839 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -8,11 +8,103 @@
> */
>
> #include
> +#include
> +#include
> +#include
> #include
> #include
> +#include
> #include "governor.h"
>
> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> +struct devfreq_cpu_state {
> + unsigned int curr_freq;
> + unsigned int min_freq;
> + unsigned int max_freq;
> + unsigned int first_cpu;
> + struct device *cpu_dev;
> + struct opp_table *opp_table;
> +};
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current
frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev:reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is
Re: [PATCH V8 4/8] devfreq: add mediatek cci devfreq
{
> + pr_err("%s: failed cci to set rate: %d\n", __func__,
> +ret);
> + mtk_cci_set_voltage(cci_df, old_voltage);
> + return ret;
> + }
> +
> + // scale down: set freq first then voltage
> + if (opp_voltage < old_voltage) {
> + ret = mtk_cci_set_voltage(cci_df, opp_voltage);
> + if (ret) {
> + pr_err("cci: failed to scale down voltage\n");
> + clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
> + return ret;
> + }
> + }
> +
> + cci_df->old_freq = *freq;
> +
> + return 0;
> +}
> +
> +static struct devfreq_dev_profile cci_devfreq_profile = {
> + .target = mtk_cci_devfreq_target,
> +};
> +
> +static int mtk_cci_devfreq_probe(struct platform_device *pdev)
> +{
> + struct device *cci_dev = >dev;
> + struct cci_devfreq *cci_df;
> + struct devfreq_passive_data *passive_data;
> + int ret;
> +
> + cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
> + if (!cci_df)
> + return -ENOMEM;
> +
> + cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
> + ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(cci_dev, "failed to get clock for CCI: %d\n",
> + ret);
Use dev_err_probe() to handle EPROBE_DEFER case. It makes code more simple.
> + return ret;
> + }
> + cci_df->cpu_reg = devm_regulator_get_optional(cci_dev, "proc");
> + ret = PTR_ERR_OR_ZERO(cci_df->cpu_reg);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(cci_dev, "failed to get regulator for CCI:
> %d\n",
> + ret);
ditto. Use dev_err_probe()
> + return ret;
> + }
> + ret = regulator_enable(cci_df->cpu_reg);
> + if (ret) {
> + dev_err(cci_dev, "enable buck for cci fail\n");
> + return ret;
> + }
> +
> + ret = dev_pm_opp_of_add_table(cci_dev);
> + if (ret) {
> + dev_err(cci_dev, "Fail to get OPP table for CCI: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, cci_df);
> +
> + passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
> + if (!passive_data) {
> + ret = -ENOMEM;
> + goto err_opp;
> + }
> +
> + passive_data->parent_type = CPUFREQ_PARENT_DEV;
> +
> + cci_df->devfreq = devm_devfreq_add_device(cci_dev,
> + _devfreq_profile,
> + DEVFREQ_GOV_PASSIVE,
> + passive_data);
> + if (IS_ERR(cci_df->devfreq)) {
> + ret = PTR_ERR(cci_df->devfreq);
> + dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
> + goto err_opp;
> + }
> +
> + return 0;
> +
> +err_opp:
> + dev_pm_opp_of_remove_table(cci_dev);
> + return ret;
> +}
> +
> +static int mtk_cci_devfreq_remove(struct platform_device *pdev)
> +{
> + struct device *cci_dev = >dev;
> + struct cci_devfreq *cci_df;
> + struct notifier_block *opp_nb;
> +
> + cci_df = platform_get_drvdata(pdev);
> + opp_nb = _df->opp_nb;
> +
> + dev_pm_opp_unregister_notifier(cci_dev, opp_nb);
Why do you call this function without registration?
If you want to catch the OPP changes of devfreq,
you can use devfreq_register_opp_notifier/devfreq_unregister_opp_notifier
functions.
> + dev_pm_opp_of_remove_table(cci_dev);
> + regulator_disable(cci_df->cpu_reg);
> +
> + return 0;
> +}
> +
> +static const __maybe_unused struct of_device_id
> + mediatek_cci_of_match[] = {
Need to change it as following at same line:
static const __maybe_unused struct of_device_idmediatek_cci_of_match[] = {
> + { .compatible = "mediatek,mt8183-cci" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mediatek_cci_of_match);
> +
> +static struct platform_driver cci_devfreq_driver = {
> + .probe = mtk_cci_devfreq_probe,
> + .remove = mtk_cci_devfreq_remove,
> + .driver = {
> + .name = "mediatek-cci-devfreq",
> + .of_match_table = of_match_ptr(mediatek_cci_of_match),
> + },
> +};
> +
> +module_platform_driver(cci_devfreq_driver);
> +
> +MODULE_DESCRIPTION("Mediatek CCI devfreq driver");
> +MODULE_AUTHOR("Andrew-sh.Cheng ");
> +MODULE_LICENSE("GPL v2");
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
urrent frequency of the cpu.
> + * @min_freq:the min frequency of the cpu.
> + * @max_freq:the max frequency of the cpu.
> + * @first_cpu: the cpumask of the first cpu of a policy.
> + * @dev: reference to cpu device.
> + * @opp_table: reference to cpu opp table.
> + *
> + * This structure stores the required cpu_state of a cpu.
> + * This is auto-populated by the governor.
> + */
> +struct devfreq_cpu_state;
> +
> +enum devfreq_parent_dev_type {
> + DEVFREQ_PARENT_DEV,
> + CPUFREQ_PARENT_DEV,
> +};
> +
> +/**
> * struct devfreq_passive_data - ``void *data`` fed to struct devfreq
> * and devfreq_add_device
> * @parent: the devfreq instance of parent device.
> @@ -290,13 +309,15 @@ struct devfreq_simple_ondemand_data {
> * using governors except for passive governor.
> * If the devfreq device has the specific method to decide
> * the next frequency, should use this callback.
> + * @parent_type: parent type of the device
> * @this:the devfreq instance of own device.
> * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
> + * @cpu_state: the state min/max/current frequency of all
> online cpu's
> *
> * The devfreq_passive_data have to set the devfreq instance of parent
> * device with governors except for the passive governor. But, don't need to
> - * initialize the 'this' and 'nb' field because the devfreq core will handle
> - * them.
> + * initialize the 'this', 'nb' and 'cpu_state' field because the devfreq core
> + * will handle them.
> */
> struct devfreq_passive_data {
> /* Should set the devfreq instance of parent device */
> @@ -305,9 +326,13 @@ struct devfreq_passive_data {
> /* Optional callback to decide the next frequency of passvice device */
> int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
>
> + /* Should set the type of parent device */
> + enum devfreq_parent_dev_type parent_type;
> +
> /* For passive governor's internal use. Don't need to set them */
> struct devfreq *this;
> struct notifier_block nb;
> + struct devfreq_cpu_state *cpu_state[NR_CPUS];
> };
> #endif
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH 1/2] extcon: extcon-gpio: Log error if work-queue init fails
On 3/24/21 6:51 PM, Vaittinen, Matti wrote:
> Hello Hans, Chanwoo, Greg,
>
> On Wed, 2021-03-24 at 10:25 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 3/24/21 10:21 AM, Matti Vaittinen wrote:
>>> Add error print for probe failure when resource managed work-queue
>>> initialization fails.
>>>
>>> Signed-off-by: Matti Vaittinen
>>> Suggested-by: Chanwoo Choi
>>> ---
>>> drivers/extcon/extcon-gpio.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-
>>> gpio.c
>>> index 4105df74f2b0..8ea2cda8f7f3 100644
>>> --- a/drivers/extcon/extcon-gpio.c
>>> +++ b/drivers/extcon/extcon-gpio.c
>>> @@ -114,8 +114,10 @@ static int gpio_extcon_probe(struct
>>> platform_device *pdev)
>>> return ret;
>>>
>>> ret = devm_delayed_work_autocancel(dev, >work,
>>> gpio_extcon_work);
>>> - if (ret)
>>> + if (ret) {
>>> + dev_err(dev, "Failed to initialize delayed_work");
>>> return ret;
>>> + }
>>
>> The only ret which we can have here is -ENOMEM and as a rule we don't
>> log
>> errors for those, because the kernel memory-management code already
>> complains
>> loudly when this happens.
>
> I know. This is why I originally omitted the print. Besides, if the
> memory is so low that devres adding fails - then we probably have
> plenty of other complaints as well... But as Chanwoo maintains the
> driver and wanted to have the print - I do not have objections to that
> either. Maybe someone some-day adds another error path to wq
> initialization in which case seeing it failed could make sense.
>
>> So IMHO this patch should be dropped.
> Fine for me - as well as keeping it. I have no strong opinion on this.
If it is the same handling way for -ENOMEM, don't need to add log ss Hans said.
Thanks for Hans.
>
> Br,
> Matti
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH v3 3/8] extconn: Clean-up few drivers by using managed work init
Hi,
Need to fix the work as following:
s/extconn/extcon
And I'd like you to use the more correct patch title like the following example:
"extcon: Use resource-managed function for delayed work"
Thanks,
Chanwoo Choi
On 3/23/21 10:57 PM, Matti Vaittinen wrote:
> Few drivers implement remove call-back only for ensuring a delayed
> work gets cancelled prior driver removal. Clean-up these by switching
> to use devm_delayed_work_autocancel() instead.
>
> Additionally, this helps avoiding mixing devm and manual resource
> management and cleans up a (theoretical?) bug from extconn-palmas.c
> and extcon-qcom-spmi-misc.c where (devm managed)IRQ might schedule
> new work item after wq was cleaned at remove().
>
> This change is compile-tested only. All testing is appreciated.
>
> Signed-off-by: Matti Vaittinen
> ---
> Changelog from RFCv2:
> - RFC dropped. No functional changes.
>
> drivers/extcon/extcon-gpio.c | 15 ---
> drivers/extcon/extcon-intel-int3496.c | 16
> drivers/extcon/extcon-palmas.c | 17 ++---
> drivers/extcon/extcon-qcom-spmi-misc.c | 17 ++---
> 4 files changed, 20 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index c211222f5d0c..4105df74f2b0 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -9,6 +9,7 @@
> * (originally switch class is supported)
> */
>
> +#include
> #include
> #include
> #include
> @@ -112,7 +113,9 @@ static int gpio_extcon_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - INIT_DELAYED_WORK(>work, gpio_extcon_work);
> + ret = devm_delayed_work_autocancel(dev, >work, gpio_extcon_work);
> + if (ret)
> + return ret;
Need to add the error log as following:
if (ret) {
dev_err(dev, "Failed to initialize delayed_work");
return ret;
}
>
> /*
>* Request the interrupt of gpio to detect whether external connector
> @@ -131,15 +134,6 @@ static int gpio_extcon_probe(struct platform_device
> *pdev)
> return 0;
> }
>
> -static int gpio_extcon_remove(struct platform_device *pdev)
> -{
> - struct gpio_extcon_data *data = platform_get_drvdata(pdev);
> -
> - cancel_delayed_work_sync(>work);
> -
> - return 0;
> -}
> -
> #ifdef CONFIG_PM_SLEEP
> static int gpio_extcon_resume(struct device *dev)
> {
> @@ -158,7 +152,6 @@ static SIMPLE_DEV_PM_OPS(gpio_extcon_pm_ops, NULL,
> gpio_extcon_resume);
>
> static struct platform_driver gpio_extcon_driver = {
> .probe = gpio_extcon_probe,
> - .remove = gpio_extcon_remove,
> .driver = {
> .name = "extcon-gpio",
> .pm = _extcon_pm_ops,
> diff --git a/drivers/extcon/extcon-intel-int3496.c
> b/drivers/extcon/extcon-intel-int3496.c
> index 80c9abcc3f97..fb527c23639e 100644
> --- a/drivers/extcon/extcon-intel-int3496.c
> +++ b/drivers/extcon/extcon-intel-int3496.c
> @@ -11,6 +11,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -101,7 +102,9 @@ static int int3496_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> data->dev = dev;
> - INIT_DELAYED_WORK(>work, int3496_do_usb_id);
> + ret = devm_delayed_work_autocancel(dev, >work, int3496_do_usb_id);
> + if (ret)
> + return ret;
>
> data->gpio_usb_id = devm_gpiod_get(dev, "id", GPIOD_IN);
> if (IS_ERR(data->gpio_usb_id)) {
> @@ -155,16 +158,6 @@ static int int3496_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static int int3496_remove(struct platform_device *pdev)
> -{
> - struct int3496_data *data = platform_get_drvdata(pdev);
> -
> - devm_free_irq(>dev, data->usb_id_irq, data);
> - cancel_delayed_work_sync(>work);
> -
> - return 0;
> -}
> -
> static const struct acpi_device_id int3496_acpi_match[] = {
> { "INT3496" },
> { }
> @@ -177,7 +170,6 @@ static struct platform_driver int3496_driver = {
> .acpi_match_table = int3496_acpi_match,
> },
> .probe = int3496_probe,
> - .remove = int3496_remove,
> };
>
> module_platform_driver(int3496_driver);
> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
> index a2852bcc5f0d..d2c1a8b89c08 100644
> --- a/drivers/extcon/extcon-palmas.c
> +++ b/drivers/extcon/extcon-palmas.c
> @@ -9,6 +
Re: [PATCH] PM: devfreq: Couple of typo fixes
On 21. 3. 18. 오후 8:20, Bhaskar Chowdhury wrote: s/stoping/stopping/ s/opeations/operations/ Signed-off-by: Bhaskar Chowdhury --- drivers/devfreq/devfreq-event.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c index 6765c03334bc..2f841d7d9d8d 100644 --- a/drivers/devfreq/devfreq-event.c +++ b/drivers/devfreq/devfreq-event.c @@ -155,7 +155,7 @@ EXPORT_SYMBOL_GPL(devfreq_event_set_event); * @edata : the calculated data of devfreq-event device * * Note that this function get the calculated event data from devfreq-event dev - * after stoping the progress of whole sequence of devfreq-event dev. + * after stopping the progress of whole sequence of devfreq-event dev. */ int devfreq_event_get_event(struct devfreq_event_dev *edev, struct devfreq_event_data *edata) @@ -184,7 +184,7 @@ int devfreq_event_get_event(struct devfreq_event_dev *edev, EXPORT_SYMBOL_GPL(devfreq_event_get_event); /** - * devfreq_event_reset_event() - Reset all opeations of devfreq-event dev. + * devfreq_event_reset_event() - Reset all operations of devfreq-event dev. * @edev : the devfreq-event device * * Note that this function stop all operations of devfreq-event dev and reset -- 2.26.2 Thanks for fixup. I think that you can squash this patch with patch[1] as you sent. [1] [PATCH] PM/devfreq: event: A typo fix And please use the following patch title -PM / devfreq: event: Fix typos -- Best Regards, Samsung Electronics Chanwoo Choi
Re: [PATCH V2 RESEND 3/4] PM / devfreq: bail out early if no freq changes in devfreq_set_target
On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
It's unnecessary to set the same freq again and run notifier calls.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/devfreq.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 85927bd7ee76..a6ee91dd17bd 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -352,13 +352,16 @@ static int devfreq_set_target(struct devfreq *devfreq,
unsigned long new_freq,
{
struct devfreq_freqs freqs;
unsigned long cur_freq;
- int err = 0;
+ int err;
if (devfreq->profile->get_cur_freq)
devfreq->profile->get_cur_freq(devfreq->dev.parent, _freq);
else
cur_freq = devfreq->previous_freq;
+ if (new_freq == cur_freq)
+ return 0;
cur_freq is one of the OPP frequencies. But, new_freq is calculated from
governor algorithm. It means that new_freq is not one of the
frequencies. Actually, it is not efficient.
After devfreq->profile->target() which almost uses
devfreq_recommended_opp(), new_freq is one of OPP frequencies.
+
freqs.old = cur_freq;
freqs.new = new_freq;
devfreq_notify_transition(devfreq, , DEVFREQ_PRECHANGE);
@@ -375,7 +378,7 @@ static int devfreq_set_target(struct devfreq *devfreq,
unsigned long new_freq,
* and DEVFREQ_POSTCHANGE because for showing the correct frequency
* change order of between devfreq device and passive devfreq device.
*/
- if (trace_devfreq_frequency_enabled() && new_freq != cur_freq)
+ if (trace_devfreq_frequency_enabled())
trace_devfreq_frequency(devfreq, new_freq, cur_freq);
freqs.new = new_freq;
@@ -390,7 +393,7 @@ static int devfreq_set_target(struct devfreq *devfreq,
unsigned long new_freq,
if (devfreq->suspend_freq)
devfreq->resume_freq = new_freq;
- return err;
+ return 0;
}
/**
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH V2 RESEND 4/4] PM / devfreq: imx8m-ddrc: remove imx8m_ddrc_get_dev_status
On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
Current driver actually does not support simple ondemand governor
as it's unable to provide device load information. So removing
the unnecessary callback to avoid confusing.
Right now the driver is using userspace governor by default.
polling_ms was also dropped as it's not needed for non-ondemand
governor.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/imx8m-ddrc.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index bc82d3653bff..ecb9375aa877 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -280,18 +280,6 @@ static int imx8m_ddrc_get_cur_freq(struct device *dev,
unsigned long *freq)
return 0;
}
-static int imx8m_ddrc_get_dev_status(struct device *dev,
-struct devfreq_dev_status *stat)
-{
- struct imx8m_ddrc *priv = dev_get_drvdata(dev);
-
- stat->busy_time = 0;
- stat->total_time = 0;
- stat->current_frequency = clk_get_rate(priv->dram_core);
-
- return 0;
-}
-
static int imx8m_ddrc_init_freq_info(struct device *dev)
{
struct imx8m_ddrc *priv = dev_get_drvdata(dev);
@@ -429,9 +417,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
if (ret < 0)
goto err;
- priv->profile.polling_ms = 1000;
priv->profile.target = imx8m_ddrc_target;
- priv->profile.get_dev_status = imx8m_ddrc_get_dev_status;
priv->profile.exit = imx8m_ddrc_exit;
priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
priv->profile.initial_freq = clk_get_rate(priv->dram_core);
Applied it. Thanks.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH V2 RESEND 2/4] PM / devfreq: Remove the invalid description for get_target_freq
On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
First of all, no_central_polling was removed since
commit 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices
which can idle")
Secondly, get_target_freq() is not only called only with update_devfreq()
notified by OPP now, but also min/max freq qos notifier.
So remove this invalid description now to avoid confusing.
Signed-off-by: Dong Aisheng
---
Documentation/ABI/testing/sysfs-class-devfreq | 5 +
drivers/devfreq/governor.h| 2 --
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-devfreq
b/Documentation/ABI/testing/sysfs-class-devfreq
index 386bc230a33d..5e6b74f30406 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -97,10 +97,7 @@ Description:
object. The values are represented in ms. If the value is
less than 1 jiffy, it is considered to be 0, which means
no polling. This value is meaningless if the governor is
- not polling; thus. If the governor is not using
- devfreq-provided central polling
- (/sys/class/devfreq/.../central_polling is 0), this value
- may be useless.
+ not polling.
A list of governors that support the node:
- simple_ondmenad
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 244634465170..2d69a0ce6291 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -57,8 +57,6 @@
*Basically, get_target_freq will run
*devfreq_dev_profile.get_dev_status() to get the
*status of the device (load = busy_time / total_time).
- * If no_central_polling is set, this callback is called
- * only with update_devfreq() notified by OPP.
* @event_handler: Callback for devfreq core framework to notify events
* to governors. Events include per device governor
* init and exit, opp changes out of devfreq, suspend
Applied it. Thanks.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH V2 RESEND 1/4] PM / devfreq: Use more accurate returned new_freq as resume_freq
On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
Use the more accurate returned new_freq as resume_freq.
It's the same as how devfreq->previous_freq was updated.
Fixes: 83f8ca45afbf0 ("PM / devfreq: add support for suspend/resume of a
devfreq device")
Signed-off-by: Dong Aisheng
---
drivers/devfreq/devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d3e7db0b09..85927bd7ee76 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -388,7 +388,7 @@ static int devfreq_set_target(struct devfreq *devfreq,
unsigned long new_freq,
devfreq->previous_freq = new_freq;
if (devfreq->suspend_freq)
- devfreq->resume_freq = cur_freq;
+ devfreq->resume_freq = new_freq;
return err;
}
Applied it.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH V2 0/6] PM / devfreq: a few small fixes and improvements
Hi, On 3/23/21 3:11 PM, Aisheng Dong wrote: >> From: Chanwoo Choi >> Sent: Tuesday, March 23, 2021 12:11 PM >> >> Hi, >> >> On 3/23/21 12:25 PM, Dong Aisheng wrote: >>> Hi Chanwoo, >>> >>> On Tue, Mar 23, 2021 at 11:13 AM Dong Aisheng >> wrote: >>>> >>>> A few small fixes and improvements >>>> >>>> ChangeLog: >>>> v1->v2: >>>> * squash a few patches >>>> * rebase to devfreq-testing >>> >>> I have to rebase to devfreq-testing instead of devfreq-next because >>> below two patches only exist in devfreq-testing. >>> 5cc75e9252e9 (devfreq/devfreq-testing) PM / devfreq: Add >>> devfreq_transitions debugfs file >>> dc9e557845c1 PM / devfreq: Add new up_threshold and down_differential >>> sysfs attrs My patch 5 needs change based on it according to your >>> suggestion. So i have to rebase to that branch. >>> >>> However, i found devfreq-testing can't build with GOV_PASSVIE enabled. >>> Patch 1 fixed it. You can squash to the original one when apply. >>> >>> Please help take a look at this new series. >> >> Please rebase your patches either devfreq-next or linux-next.git Because >> devfreq-testing branch is not official. > > Okay, then how about the patch 5 below? > [PATCH V2 5/6] PM / devfreq: governor: optimize simpleondemand get_target_freq > > Should I also rebase it to devfreq-next or drop it first and then resend when > your patch > merged into mainline? Yes. Thanks. [snip] -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH V2 0/6] PM / devfreq: a few small fixes and improvements
Hi, On 3/23/21 12:25 PM, Dong Aisheng wrote: > Hi Chanwoo, > > On Tue, Mar 23, 2021 at 11:13 AM Dong Aisheng wrote: >> >> A few small fixes and improvements >> >> ChangeLog: >> v1->v2: >> * squash a few patches >> * rebase to devfreq-testing > > I have to rebase to devfreq-testing instead of devfreq-next because > below two patches > only exist in devfreq-testing. > 5cc75e9252e9 (devfreq/devfreq-testing) PM / devfreq: Add > devfreq_transitions debugfs file > dc9e557845c1 PM / devfreq: Add new up_threshold and down_differential > sysfs attrs > My patch 5 needs change based on it according to your suggestion. So i have to > rebase to that branch. > > However, i found devfreq-testing can't build with GOV_PASSVIE enabled. > Patch 1 fixed it. You can squash to the original one when apply. > > Please help take a look at this new series. Please rebase your patches either devfreq-next or linux-next.git Because devfreq-testing branch is not official. > Thanks > > Regards > Aisheng > >> * drop two patches which are already in devfreq-next >> >> Dong Aisheng (6): >> PM / devfreq: fix build error when DEVFREQ_GOV_PASSIVE enabled >> PM / devfreq: Use more accurate returned new_freq as resume_freq >> PM / devfreq: Remove the invalid description for get_target_freq >> PM / devfreq: bail out early if no freq changes in devfreq_set_target >> PM / devfreq: governor: optimize simpleondemand get_target_freq >> PM / devfreq: imx8m-ddrc: remove imx8m_ddrc_get_dev_status >> >> Documentation/ABI/testing/sysfs-class-devfreq | 5 +-- >> drivers/devfreq/devfreq.c | 37 +++ >> drivers/devfreq/governor.h| 2 - >> drivers/devfreq/governor_simpleondemand.c | 31 ++-- >> drivers/devfreq/imx8m-ddrc.c | 14 --- >> 5 files changed, 35 insertions(+), 54 deletions(-) >> >> -- >> 2.25.1 >> > > -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 07/11] PM / devfreq: check get_dev_status before start monitor
On 3/18/21 6:54 PM, Chanwoo Choi wrote:
> On 3/18/21 5:03 PM, Dong Aisheng wrote:
>> Hi Chanwoo,
>>
>> On Sat, Mar 13, 2021 at 2:45 PM Dong Aisheng wrote:
>>>
>>> On Sat, Mar 13, 2021 at 12:09 AM Chanwoo Choi wrote:
>>>>
>>>> On 21. 3. 12. 오후 7:57, Dong Aisheng wrote:
>>>>> On Thu, Mar 11, 2021 at 2:54 PM Chanwoo Choi
>>>>> wrote:
>>>>>>
>>>>>> On 3/10/21 1:56 PM, Dong Aisheng wrote:
>>>>>>> On Wed, Mar 10, 2021 at 11:08 AM Chanwoo Choi
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On 3/10/21 11:56 AM, Dong Aisheng wrote:
>>>>>>>>> On Wed, Mar 10, 2021 at 12:12 AM Chanwoo Choi
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> On 21. 3. 10. 오전 12:58, Chanwoo Choi wrote:
>>>>>>>>>>> On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
>>>>>>>>>>>> The devfreq monitor depends on the device to provide load
>>>>>>>>>>>> information
>>>>>>>>>>>> by .get_dev_status() to calculate the next target freq.
>>>>>>>>>>>>
>>>>>>>>>>>> And this will cause changing governor to simple ondemand fail
>>>>>>>>>>>> if device can't support.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Dong Aisheng
>>>>>>>>>>>> ---
>>>>>>>>>>>>drivers/devfreq/devfreq.c | 10 +++---
>>>>>>>>>>>>drivers/devfreq/governor.h| 2 +-
>>>>>>>>>>>>drivers/devfreq/governor_simpleondemand.c | 3 +--
>>>>>>>>>>>>3 files changed, 9 insertions(+), 6 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>>>>>>>>>> index 7231fe6862a2..d1787b6c7d7c 100644
>>>>>>>>>>>> --- a/drivers/devfreq/devfreq.c
>>>>>>>>>>>> +++ b/drivers/devfreq/devfreq.c
>>>>>>>>>>>> @@ -482,10 +482,13 @@ static void devfreq_monitor(struct
>>>>>>>>>>>> work_struct
>>>>>>>>>>>> *work)
>>>>>>>>>>>> * to be called from governor in response to DEVFREQ_GOV_START
>>>>>>>>>>>> * event when device is added to devfreq framework.
>>>>>>>>>>>> */
>>>>>>>>>>>> -void devfreq_monitor_start(struct devfreq *devfreq)
>>>>>>>>>>>> +int devfreq_monitor_start(struct devfreq *devfreq)
>>>>>>>>>>>>{
>>>>>>>>>>>>if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
>>>>>>>>>>>> -return;
>>>>>>>>>>>> +return 0;
>>>>>>>>>>>> +
>>>>>>>>>>>> +if (!devfreq->profile->get_dev_status)
>>>>>>>>>>>> +return -EINVAL;
>>>>>>>>>>
>>>>>>>>>> Again, I think that get_dev_status is not used for all governors.
>>>>>>>>>> So that it cause the governor start fail. Don't check whether
>>>>>>>>>> .get_dev_status is NULL or not.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'm not quite understand your point.
>>>>>>>>> it is used by governor_simpleondemand.c and tegra_devfreq_governor.
>>>>>>>>> get_target_freq -> devfreq_update_stats -> get_dev_status
>>>>>>>>
>>>>>>>> The devfreq can add the new governor by anyone.
>>>>>>>> So these functions like devfreq_monitor_* have to support
>>>>>>>> the governors and also must support the governor to be added
>>>>>>>> in the future.
>>>>>>>
>>>>>>> Yes, but devfreq_monitor_* is only used by polling mode,
Re: [PATCH 07/11] PM / devfreq: check get_dev_status before start monitor
On 3/18/21 5:03 PM, Dong Aisheng wrote:
> Hi Chanwoo,
>
> On Sat, Mar 13, 2021 at 2:45 PM Dong Aisheng wrote:
>>
>> On Sat, Mar 13, 2021 at 12:09 AM Chanwoo Choi wrote:
>>>
>>> On 21. 3. 12. 오후 7:57, Dong Aisheng wrote:
>>>> On Thu, Mar 11, 2021 at 2:54 PM Chanwoo Choi wrote:
>>>>>
>>>>> On 3/10/21 1:56 PM, Dong Aisheng wrote:
>>>>>> On Wed, Mar 10, 2021 at 11:08 AM Chanwoo Choi
>>>>>> wrote:
>>>>>>>
>>>>>>> On 3/10/21 11:56 AM, Dong Aisheng wrote:
>>>>>>>> On Wed, Mar 10, 2021 at 12:12 AM Chanwoo Choi
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On 21. 3. 10. 오전 12:58, Chanwoo Choi wrote:
>>>>>>>>>> On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
>>>>>>>>>>> The devfreq monitor depends on the device to provide load
>>>>>>>>>>> information
>>>>>>>>>>> by .get_dev_status() to calculate the next target freq.
>>>>>>>>>>>
>>>>>>>>>>> And this will cause changing governor to simple ondemand fail
>>>>>>>>>>> if device can't support.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Dong Aisheng
>>>>>>>>>>> ---
>>>>>>>>>>>drivers/devfreq/devfreq.c | 10 +++---
>>>>>>>>>>>drivers/devfreq/governor.h| 2 +-
>>>>>>>>>>>drivers/devfreq/governor_simpleondemand.c | 3 +--
>>>>>>>>>>>3 files changed, 9 insertions(+), 6 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>>>>>>>>> index 7231fe6862a2..d1787b6c7d7c 100644
>>>>>>>>>>> --- a/drivers/devfreq/devfreq.c
>>>>>>>>>>> +++ b/drivers/devfreq/devfreq.c
>>>>>>>>>>> @@ -482,10 +482,13 @@ static void devfreq_monitor(struct work_struct
>>>>>>>>>>> *work)
>>>>>>>>>>> * to be called from governor in response to DEVFREQ_GOV_START
>>>>>>>>>>> * event when device is added to devfreq framework.
>>>>>>>>>>> */
>>>>>>>>>>> -void devfreq_monitor_start(struct devfreq *devfreq)
>>>>>>>>>>> +int devfreq_monitor_start(struct devfreq *devfreq)
>>>>>>>>>>>{
>>>>>>>>>>>if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
>>>>>>>>>>> -return;
>>>>>>>>>>> +return 0;
>>>>>>>>>>> +
>>>>>>>>>>> +if (!devfreq->profile->get_dev_status)
>>>>>>>>>>> +return -EINVAL;
>>>>>>>>>
>>>>>>>>> Again, I think that get_dev_status is not used for all governors.
>>>>>>>>> So that it cause the governor start fail. Don't check whether
>>>>>>>>> .get_dev_status is NULL or not.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I'm not quite understand your point.
>>>>>>>> it is used by governor_simpleondemand.c and tegra_devfreq_governor.
>>>>>>>> get_target_freq -> devfreq_update_stats -> get_dev_status
>>>>>>>
>>>>>>> The devfreq can add the new governor by anyone.
>>>>>>> So these functions like devfreq_monitor_* have to support
>>>>>>> the governors and also must support the governor to be added
>>>>>>> in the future.
>>>>>>
>>>>>> Yes, but devfreq_monitor_* is only used by polling mode, right?
>>>>>> The governor using it has to implement get_dev_status unless
>>>>>> there's an exception in the future.
>>>>>>
>>>>>> Currently this patch wants to address the issue that user can switch
>>>>>> to ondemand governor (polling mode) by sysfs even devices does
>>&
Re: [PATCH v2] PM / devfreq: Unlock mutex and free devfreq struct in error path
On 3/15/21 6:31 PM, Lukasz Luba wrote:
> The devfreq->lock is held for time of setup. Release the lock in the
> error path, before jumping to the end of the function.
>
> Change the goto destination which frees the allocated memory.
>
> Cc: v5.9+ # v5.9+
> Fixes: 4dc3bab8687f ("PM / devfreq: Add support delayed timer for polling
> mode")
> Signed-off-by: Lukasz Luba
> ---
> v2:
> - added fixes tag and CC stable v5.9+
> - used capital letter in commit header
>
>
> drivers/devfreq/devfreq.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index b6d3e7db0b09..99b2eeedc238 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -822,7 +822,8 @@ struct devfreq *devfreq_add_device(struct device *dev,
>
> if (devfreq->profile->timer < 0
> || devfreq->profile->timer >= DEVFREQ_TIMER_NUM) {
> - goto err_out;
> + mutex_unlock(>lock);
> + goto err_dev;
> }
>
> if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
>
Applied it. Thanks.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH] PM / devfreq: unlock mutex and free devfreq struct in error path
Hi Lukasz,
On 3/13/21 3:45 AM, Lukasz Luba wrote:
> The devfreq->lock is held for time of setup. Release the lock in the
> error path, before jumping to the end of the function.
>
> Change the goto destination which frees the allocated memory.
>
> Signed-off-by: Lukasz Luba
> ---
> drivers/devfreq/devfreq.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index b6d3e7db0b09..99b2eeedc238 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -822,7 +822,8 @@ struct devfreq *devfreq_add_device(struct device *dev,
>
> if (devfreq->profile->timer < 0
> || devfreq->profile->timer >= DEVFREQ_TIMER_NUM) {
> - goto err_out;
> + mutex_unlock(>lock);
> + goto err_dev;
> }
>
> if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
>
Looks good to me. But, need to add the following information
and please use capital letter of the first character of patch title. Thanks.
Fixes: 4dc3bab8687f ("PM / devfreq: Add support delayed timer for polling mode")
Also, need to send it to stable lkml.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
[GIT PULL] extcon fixes for v5.12-rc4
Dear Greg, This is extcon-fixes pull request for v5.12. I add detailed description of this pull request on below. Please pull extcon with following updates. Best Regards, Chanwoo Choi The following changes since commit 1e28eed17697bcf343c6743f0028cc3b5dd88bf0: Linux 5.12-rc3 (2021-03-14 14:41:02 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git tags/extcon-fixes-for-5.12-rc4 for you to fetch changes up to d3bdd1c3140724967ca4136755538fa7c05c2b4e: extcon: Fix error handling in extcon_dev_register (2021-03-15 11:09:38 +0900) Dinghao Liu (1): extcon: Fix error handling in extcon_dev_register Krzysztof Kozlowski (1): extcon: Add stubs for extcon_register_notifier_all() functions drivers/extcon/extcon.c | 1 + include/linux/extcon.h | 23 +++ 2 files changed, 24 insertions(+) -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 07/11] PM / devfreq: check get_dev_status before start monitor
On 21. 3. 12. 오후 7:57, Dong Aisheng wrote:
On Thu, Mar 11, 2021 at 2:54 PM Chanwoo Choi wrote:
On 3/10/21 1:56 PM, Dong Aisheng wrote:
On Wed, Mar 10, 2021 at 11:08 AM Chanwoo Choi wrote:
On 3/10/21 11:56 AM, Dong Aisheng wrote:
On Wed, Mar 10, 2021 at 12:12 AM Chanwoo Choi wrote:
On 21. 3. 10. 오전 12:58, Chanwoo Choi wrote:
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
The devfreq monitor depends on the device to provide load information
by .get_dev_status() to calculate the next target freq.
And this will cause changing governor to simple ondemand fail
if device can't support.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/devfreq.c | 10 +++---
drivers/devfreq/governor.h| 2 +-
drivers/devfreq/governor_simpleondemand.c | 3 +--
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7231fe6862a2..d1787b6c7d7c 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -482,10 +482,13 @@ static void devfreq_monitor(struct work_struct
*work)
* to be called from governor in response to DEVFREQ_GOV_START
* event when device is added to devfreq framework.
*/
-void devfreq_monitor_start(struct devfreq *devfreq)
+int devfreq_monitor_start(struct devfreq *devfreq)
{
if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
-return;
+return 0;
+
+if (!devfreq->profile->get_dev_status)
+return -EINVAL;
Again, I think that get_dev_status is not used for all governors.
So that it cause the governor start fail. Don't check whether
.get_dev_status is NULL or not.
I'm not quite understand your point.
it is used by governor_simpleondemand.c and tegra_devfreq_governor.
get_target_freq -> devfreq_update_stats -> get_dev_status
The devfreq can add the new governor by anyone.
So these functions like devfreq_monitor_* have to support
the governors and also must support the governor to be added
in the future.
Yes, but devfreq_monitor_* is only used by polling mode, right?
The governor using it has to implement get_dev_status unless
there's an exception in the future.
Currently this patch wants to address the issue that user can switch
to ondemand governor (polling mode) by sysfs even devices does
not support it (no get_dev_status implemented).
As I commented, I'll fix this issue. If devfreq driver doesn't implement
the .get_dev_status, don't show it via available_governors. I think that
it is fundamental solution to fix this issue.
Sounds good
So on this version,
don't add the this conditional statement on this function
Almost all this patch did is adding a checking for get_dev_status.
So do you mean drop this patch?
I wonder it's still a necessary checking to explicitly tell devfreq monitor
users that get_dev_status is needed during governor startup.
I think that the it is enough to check .get_dev_status in
devfreq_update_stats. We have to check it on where it is used.
And on next version, please use the capital letter for first character
on patch title as following:
- PM / devfreq: Check get_dev_status before start monitor
Okay to me.
Thanks for the suggestion.
Regards
Aisheng
Regards
Aisheng
Without checking, device can switch to ondemand governor if it does not support.
Am i missed something?
Regards
Aisheng
switch (devfreq->profile->timer) {
case DEVFREQ_TIMER_DEFERRABLE:
@@ -495,12 +498,13 @@ void devfreq_monitor_start(struct devfreq *devfreq)
INIT_DELAYED_WORK(>work, devfreq_monitor);
break;
default:
-return;
+return -EINVAL;
}
if (devfreq->profile->polling_ms)
queue_delayed_work(devfreq_wq, >work,
msecs_to_jiffies(devfreq->profile->polling_ms));
+return 0;
}
EXPORT_SYMBOL(devfreq_monitor_start);
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 5cee3f64fe2b..31af6d072a10 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -75,7 +75,7 @@ struct devfreq_governor {
unsigned int event, void *data);
};
-void devfreq_monitor_start(struct devfreq *devfreq);
+int devfreq_monitor_start(struct devfreq *devfreq);
void devfreq_monitor_stop(struct devfreq *devfreq);
void devfreq_monitor_suspend(struct devfreq *devfreq);
void devfreq_monitor_resume(struct devfreq *devfreq);
diff --git a/drivers/devfreq/governor_simpleondemand.c
b/drivers/devfreq/governor_simpleondemand.c
index d57b82a2b570..ea287b57cbf3 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -89,8 +89,7 @@ static int devfreq_simple_ondemand_handler(struct
devfreq *devfreq,
{
switch (event) {
case DEVFREQ_GOV_START:
-devfreq_monitor_start(devfreq);
-break;
+return devfreq_monitor_start(devfreq);
case DEVFREQ
Re: [PATCH 07/11] PM / devfreq: check get_dev_status before start monitor
On 3/10/21 1:56 PM, Dong Aisheng wrote:
> On Wed, Mar 10, 2021 at 11:08 AM Chanwoo Choi wrote:
>>
>> On 3/10/21 11:56 AM, Dong Aisheng wrote:
>>> On Wed, Mar 10, 2021 at 12:12 AM Chanwoo Choi wrote:
>>>>
>>>> On 21. 3. 10. 오전 12:58, Chanwoo Choi wrote:
>>>>> On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
>>>>>> The devfreq monitor depends on the device to provide load information
>>>>>> by .get_dev_status() to calculate the next target freq.
>>>>>>
>>>>>> And this will cause changing governor to simple ondemand fail
>>>>>> if device can't support.
>>>>>>
>>>>>> Signed-off-by: Dong Aisheng
>>>>>> ---
>>>>>> drivers/devfreq/devfreq.c | 10 +++---
>>>>>> drivers/devfreq/governor.h| 2 +-
>>>>>> drivers/devfreq/governor_simpleondemand.c | 3 +--
>>>>>> 3 files changed, 9 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>>>> index 7231fe6862a2..d1787b6c7d7c 100644
>>>>>> --- a/drivers/devfreq/devfreq.c
>>>>>> +++ b/drivers/devfreq/devfreq.c
>>>>>> @@ -482,10 +482,13 @@ static void devfreq_monitor(struct work_struct
>>>>>> *work)
>>>>>>* to be called from governor in response to DEVFREQ_GOV_START
>>>>>>* event when device is added to devfreq framework.
>>>>>>*/
>>>>>> -void devfreq_monitor_start(struct devfreq *devfreq)
>>>>>> +int devfreq_monitor_start(struct devfreq *devfreq)
>>>>>> {
>>>>>> if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
>>>>>> -return;
>>>>>> +return 0;
>>>>>> +
>>>>>> +if (!devfreq->profile->get_dev_status)
>>>>>> +return -EINVAL;
>>>>
>>>> Again, I think that get_dev_status is not used for all governors.
>>>> So that it cause the governor start fail. Don't check whether
>>>> .get_dev_status is NULL or not.
>>>>
>>>
>>> I'm not quite understand your point.
>>> it is used by governor_simpleondemand.c and tegra_devfreq_governor.
>>> get_target_freq -> devfreq_update_stats -> get_dev_status
>>
>> The devfreq can add the new governor by anyone.
>> So these functions like devfreq_monitor_* have to support
>> the governors and also must support the governor to be added
>> in the future.
>
> Yes, but devfreq_monitor_* is only used by polling mode, right?
> The governor using it has to implement get_dev_status unless
> there's an exception in the future.
>
> Currently this patch wants to address the issue that user can switch
> to ondemand governor (polling mode) by sysfs even devices does
> not support it (no get_dev_status implemented).
As I commented, I'll fix this issue. If devfreq driver doesn't implement
the .get_dev_status, don't show it via available_governors. I think that
it is fundamental solution to fix this issue. So on this version,
don't add the this conditional statement on this function
And on next version, please use the capital letter for first character
on patch title as following:
- PM / devfreq: Check get_dev_status before start monitor
>
> Regards
> Aisheng
>
>>
>>>
>>> Without checking, device can switch to ondemand governor if it does not
>>> support.
>>>
>>> Am i missed something?
>>>
>>> Regards
>>> Aisheng
>>>
>>>>>> switch (devfreq->profile->timer) {
>>>>>> case DEVFREQ_TIMER_DEFERRABLE:
>>>>>> @@ -495,12 +498,13 @@ void devfreq_monitor_start(struct devfreq *devfreq)
>>>>>> INIT_DELAYED_WORK(>work, devfreq_monitor);
>>>>>> break;
>>>>>> default:
>>>>>> -return;
>>>>>> +return -EINVAL;
>>>>>> }
>>>>>> if (devfreq->profile->polling_ms)
>>>>>> queue_delayed_work(devfreq_wq, >work,
>>>>>> msecs_to_jiffies(devfreq->profile->polling_ms));
>>>>>> +return 0;
>>>>>> }
>>>>>> EXPORT_SYMBOL(devfreq_monitor_start);
>>>>>&g
Re: [PATCH 08/11] PM / devfreq: check get_dev_status in devfreq_update_stats
On 3/10/21 12:00 PM, Dong Aisheng wrote:
> On Wed, Mar 10, 2021 at 12:20 AM Chanwoo Choi wrote:
>>
>> On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
>>> Check .get_dev_status() in devfreq_update_stats in case it's abused
>>> when a device does not provide it.
>>>
>>> Signed-off-by: Dong Aisheng
>>> ---
>>> drivers/devfreq/governor.h | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
>>> index 31af6d072a10..67a6dbdd5d23 100644
>>> --- a/drivers/devfreq/governor.h
>>> +++ b/drivers/devfreq/governor.h
>>> @@ -89,6 +89,9 @@ int devfreq_update_target(struct devfreq *devfreq,
>>> unsigned long freq);
>>>
>>> static inline int devfreq_update_stats(struct devfreq *df)
>>> {
>>> + if (!df->profile->get_dev_status)
>>> + return -EINVAL;
>>> +
>>
>> I'm considering the following method instead of returning the error
>> when .get_dev_status is NULL.
>>
>> if (!df->profile->get_dev_status) {
>> df->last_status.total_time = 0;
>> df->last_status.busy_time = 0;
>> df->last_status.current_frequency = 0;
>> return 0;
>> }
>
> I might suggest not cause it's meaningless for ondemand governor but
> introducing confusing. Simply return error could make the life a bit easier.
> does it make sense to you?
Actually, I considered the some corner case as following:
We can see the simple_ondemand governor through available_governors
even if the devfreq driver doesn't implement the .get_dev_status.
In this corner case, My intention tried to prevent the error
on this case. But, actually, it is different issue. I'll fix
this issue when get_dev_status is NULL, don't show the
simple_ondemand governor name through available_governors
on other patch.
And I applied it. Thanks.
>
> Regards
> Aisheng
>
>>
>>> return df->profile->get_dev_status(df->dev.parent, >last_status);
>>> }
>>> #endif /* _GOVERNOR_H */
>>>
>>
>>
>> --
>> Best Regards,
>> Samsung Electronics
>> Chanwoo Choi
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH 07/11] PM / devfreq: check get_dev_status before start monitor
On 3/10/21 11:56 AM, Dong Aisheng wrote:
> On Wed, Mar 10, 2021 at 12:12 AM Chanwoo Choi wrote:
>>
>> On 21. 3. 10. 오전 12:58, Chanwoo Choi wrote:
>>> On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
>>>> The devfreq monitor depends on the device to provide load information
>>>> by .get_dev_status() to calculate the next target freq.
>>>>
>>>> And this will cause changing governor to simple ondemand fail
>>>> if device can't support.
>>>>
>>>> Signed-off-by: Dong Aisheng
>>>> ---
>>>> drivers/devfreq/devfreq.c | 10 +++---
>>>> drivers/devfreq/governor.h| 2 +-
>>>> drivers/devfreq/governor_simpleondemand.c | 3 +--
>>>> 3 files changed, 9 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>> index 7231fe6862a2..d1787b6c7d7c 100644
>>>> --- a/drivers/devfreq/devfreq.c
>>>> +++ b/drivers/devfreq/devfreq.c
>>>> @@ -482,10 +482,13 @@ static void devfreq_monitor(struct work_struct
>>>> *work)
>>>>* to be called from governor in response to DEVFREQ_GOV_START
>>>>* event when device is added to devfreq framework.
>>>>*/
>>>> -void devfreq_monitor_start(struct devfreq *devfreq)
>>>> +int devfreq_monitor_start(struct devfreq *devfreq)
>>>> {
>>>> if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
>>>> -return;
>>>> +return 0;
>>>> +
>>>> +if (!devfreq->profile->get_dev_status)
>>>> +return -EINVAL;
>>
>> Again, I think that get_dev_status is not used for all governors.
>> So that it cause the governor start fail. Don't check whether
>> .get_dev_status is NULL or not.
>>
>
> I'm not quite understand your point.
> it is used by governor_simpleondemand.c and tegra_devfreq_governor.
> get_target_freq -> devfreq_update_stats -> get_dev_status
The devfreq can add the new governor by anyone.
So these functions like devfreq_monitor_* have to support
the governors and also must support the governor to be added
in the future.
>
> Without checking, device can switch to ondemand governor if it does not
> support.
>
> Am i missed something?
>
> Regards
> Aisheng
>
>>>> switch (devfreq->profile->timer) {
>>>> case DEVFREQ_TIMER_DEFERRABLE:
>>>> @@ -495,12 +498,13 @@ void devfreq_monitor_start(struct devfreq *devfreq)
>>>> INIT_DELAYED_WORK(>work, devfreq_monitor);
>>>> break;
>>>> default:
>>>> -return;
>>>> +return -EINVAL;
>>>> }
>>>> if (devfreq->profile->polling_ms)
>>>> queue_delayed_work(devfreq_wq, >work,
>>>> msecs_to_jiffies(devfreq->profile->polling_ms));
>>>> +return 0;
>>>> }
>>>> EXPORT_SYMBOL(devfreq_monitor_start);
>>>> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
>>>> index 5cee3f64fe2b..31af6d072a10 100644
>>>> --- a/drivers/devfreq/governor.h
>>>> +++ b/drivers/devfreq/governor.h
>>>> @@ -75,7 +75,7 @@ struct devfreq_governor {
>>>> unsigned int event, void *data);
>>>> };
>>>> -void devfreq_monitor_start(struct devfreq *devfreq);
>>>> +int devfreq_monitor_start(struct devfreq *devfreq);
>>>> void devfreq_monitor_stop(struct devfreq *devfreq);
>>>> void devfreq_monitor_suspend(struct devfreq *devfreq);
>>>> void devfreq_monitor_resume(struct devfreq *devfreq);
>>>> diff --git a/drivers/devfreq/governor_simpleondemand.c
>>>> b/drivers/devfreq/governor_simpleondemand.c
>>>> index d57b82a2b570..ea287b57cbf3 100644
>>>> --- a/drivers/devfreq/governor_simpleondemand.c
>>>> +++ b/drivers/devfreq/governor_simpleondemand.c
>>>> @@ -89,8 +89,7 @@ static int devfreq_simple_ondemand_handler(struct
>>>> devfreq *devfreq,
>>>> {
>>>> switch (event) {
>>>> case DEVFREQ_GOV_START:
>>>> -devfreq_monitor_start(devfreq);
>>>> -break;
>>>> +return devfreq_monitor_start(devfreq);
>>>> case DEVFREQ_GOV_STOP:
>>>> devfreq_monitor_stop(devfreq);
>>>>
>>>
>>> Need to handle the all points of devfreq_monitor_start() usage.
>>> please check the tegra30-devfreq.c for this update.
>>>
>>> $ grep -rn "devfreq_monitor_start" drivers/
>>> drivers/devfreq/governor_simpleondemand.c:92:
>>> devfreq_monitor_start(devfreq);
>>> drivers/devfreq/tegra30-devfreq.c:744:
>>> devfreq_monitor_start(devfreq);
>>> ..
>>>
>>
>>
>> --
>> Best Regards,
>> Samsung Electronics
>> Chanwoo Choi
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH 05/11] PM / devfreq: use more accurate returned new_freq as resume_freq
On 3/10/21 11:43 AM, Dong Aisheng wrote:
> On Tue, Mar 9, 2021 at 11:53 PM Chanwoo Choi wrote:
>>
>> On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
>>> Use the more accurate returned new_freq as resume_freq.
>>> It's the same as how devfreq->previous_freq was updated.
>>>
>>> Signed-off-by: Dong Aisheng
>>> ---
>>> drivers/devfreq/devfreq.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index 6e80bf70e7b3..ce569bd9adfa 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -390,7 +390,7 @@ static int devfreq_set_target(struct devfreq *devfreq,
>>> unsigned long new_freq,
>>> devfreq->previous_freq = new_freq;
>>>
>>> if (devfreq->suspend_freq)
>>> - devfreq->resume_freq = cur_freq;
>>> + devfreq->resume_freq = new_freq;
>>>
>>> return err;
>>> }
>>>
>>
>> This patch fixes the previous patch[1]. So that you need to
>> add 'Fixes' tag as following:
>>
>> Fixes: 83f8ca45afbf0 ("PM / devfreq: add support for suspend/resume of a
>> devfreq device")
>>
>
> Will add Fixes tag in next version.
On next version, recommend to place this patch at the first.
>
>> commit 83f8ca45afbf041e312909f442128b99657d90b7
>> Refs: v4.20-rc6-2-g83f8ca45afbf
>> Author: Lukasz Luba
>> AuthorDate: Wed Dec 5 12:05:53 2018 +0100
>> Commit: MyungJoo Ham
>> CommitDate: Tue Dec 11 11:09:47 2018 +0900
>>
>> PM / devfreq: add support for suspend/resume of a devfreq device
>>
>>
>> --
>> Best Regards,
>> Samsung Electronics
>> Chanwoo Choi
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH 11/11] PM / devfreq: imx8m-ddrc: drop polling_ms
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote: polling_ms is only used by simple ondemand governor which this driver can't support. Drop it to avoid confusing. Signed-off-by: Dong Aisheng --- drivers/devfreq/imx8m-ddrc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c index 0a6b7a1c829d..ecb9375aa877 100644 --- a/drivers/devfreq/imx8m-ddrc.c +++ b/drivers/devfreq/imx8m-ddrc.c @@ -417,7 +417,6 @@ static int imx8m_ddrc_probe(struct platform_device *pdev) if (ret < 0) goto err; - priv->profile.polling_ms = 1000; priv->profile.target = imx8m_ddrc_target; priv->profile.exit = imx8m_ddrc_exit; priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq; You can squash this patch with patch10 because polling_ms is related to .get_dev_status. -- Best Regards, Samsung Electronics Chanwoo Choi
Re: [PATCH 10/11] PM / devfreq: imx8m-ddrc: remove imx8m_ddrc_get_dev_status
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
Current driver actually does not support simple ondemand governor
as it's unable to provide device load information. So removing
the unnecessary callback to avoid confusing.
Right now the driver is using userspace governor by default.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/imx8m-ddrc.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index bc82d3653bff..0a6b7a1c829d 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -280,18 +280,6 @@ static int imx8m_ddrc_get_cur_freq(struct device *dev,
unsigned long *freq)
return 0;
}
-static int imx8m_ddrc_get_dev_status(struct device *dev,
-struct devfreq_dev_status *stat)
-{
- struct imx8m_ddrc *priv = dev_get_drvdata(dev);
-
- stat->busy_time = 0;
- stat->total_time = 0;
- stat->current_frequency = clk_get_rate(priv->dram_core);
-
- return 0;
-}
-
static int imx8m_ddrc_init_freq_info(struct device *dev)
{
struct imx8m_ddrc *priv = dev_get_drvdata(dev);
@@ -431,7 +419,6 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
priv->profile.polling_ms = 1000;
priv->profile.target = imx8m_ddrc_target;
- priv->profile.get_dev_status = imx8m_ddrc_get_dev_status;
priv->profile.exit = imx8m_ddrc_exit;
priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
priv->profile.initial_freq = clk_get_rate(priv->dram_core);
Acked-by: Chanwoo Choi
Will merge this patch after finishing the discussion of
prev patch related to.get_dev_status.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 08/11] PM / devfreq: check get_dev_status in devfreq_update_stats
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
Check .get_dev_status() in devfreq_update_stats in case it's abused
when a device does not provide it.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/governor.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 31af6d072a10..67a6dbdd5d23 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -89,6 +89,9 @@ int devfreq_update_target(struct devfreq *devfreq, unsigned
long freq);
static inline int devfreq_update_stats(struct devfreq *df)
{
+ if (!df->profile->get_dev_status)
+ return -EINVAL;
+
I'm considering the following method instead of returning the error
when .get_dev_status is NULL.
if (!df->profile->get_dev_status) {
df->last_status.total_time = 0;
df->last_status.busy_time = 0;
df->last_status.current_frequency = 0;
return 0;
}
return df->profile->get_dev_status(df->dev.parent, >last_status);
}
#endif /* _GOVERNOR_H */
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 07/11] PM / devfreq: check get_dev_status before start monitor
On 21. 3. 10. 오전 12:58, Chanwoo Choi wrote:
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
The devfreq monitor depends on the device to provide load information
by .get_dev_status() to calculate the next target freq.
And this will cause changing governor to simple ondemand fail
if device can't support.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/devfreq.c | 10 +++---
drivers/devfreq/governor.h | 2 +-
drivers/devfreq/governor_simpleondemand.c | 3 +--
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7231fe6862a2..d1787b6c7d7c 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -482,10 +482,13 @@ static void devfreq_monitor(struct work_struct
*work)
* to be called from governor in response to DEVFREQ_GOV_START
* event when device is added to devfreq framework.
*/
-void devfreq_monitor_start(struct devfreq *devfreq)
+int devfreq_monitor_start(struct devfreq *devfreq)
{
if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
- return;
+ return 0;
+
+ if (!devfreq->profile->get_dev_status)
+ return -EINVAL;
Again, I think that get_dev_status is not used for all governors.
So that it cause the governor start fail. Don't check whether
.get_dev_status is NULL or not.
switch (devfreq->profile->timer) {
case DEVFREQ_TIMER_DEFERRABLE:
@@ -495,12 +498,13 @@ void devfreq_monitor_start(struct devfreq *devfreq)
INIT_DELAYED_WORK(>work, devfreq_monitor);
break;
default:
- return;
+ return -EINVAL;
}
if (devfreq->profile->polling_ms)
queue_delayed_work(devfreq_wq, >work,
msecs_to_jiffies(devfreq->profile->polling_ms));
+ return 0;
}
EXPORT_SYMBOL(devfreq_monitor_start);
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 5cee3f64fe2b..31af6d072a10 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -75,7 +75,7 @@ struct devfreq_governor {
unsigned int event, void *data);
};
-void devfreq_monitor_start(struct devfreq *devfreq);
+int devfreq_monitor_start(struct devfreq *devfreq);
void devfreq_monitor_stop(struct devfreq *devfreq);
void devfreq_monitor_suspend(struct devfreq *devfreq);
void devfreq_monitor_resume(struct devfreq *devfreq);
diff --git a/drivers/devfreq/governor_simpleondemand.c
b/drivers/devfreq/governor_simpleondemand.c
index d57b82a2b570..ea287b57cbf3 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -89,8 +89,7 @@ static int devfreq_simple_ondemand_handler(struct
devfreq *devfreq,
{
switch (event) {
case DEVFREQ_GOV_START:
- devfreq_monitor_start(devfreq);
- break;
+ return devfreq_monitor_start(devfreq);
case DEVFREQ_GOV_STOP:
devfreq_monitor_stop(devfreq);
Need to handle the all points of devfreq_monitor_start() usage.
please check the tegra30-devfreq.c for this update.
$ grep -rn "devfreq_monitor_start" drivers/
drivers/devfreq/governor_simpleondemand.c:92:
devfreq_monitor_start(devfreq);
drivers/devfreq/tegra30-devfreq.c:744:
devfreq_monitor_start(devfreq);
..
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 09/11] PM / devfreq: governor: optimize simpleondemand get_target_freq
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
devfreq_simple_ondemand_data only needs to be initialized once when
calling devm_devfreq_add_device. It's unnecessary to put the data
check logic in the hot path (.get_target_freq()) where it will be
called all the time during polling. Instead, we only check and initialize
it one time during DEVFREQ_GOV_START.
This also helps check data validability in advance during DEVFREQ_GOV_START
rather than checking it later when running .get_target_freq().
Signed-off-by: Dong Aisheng
---
drivers/devfreq/governor_simpleondemand.c | 50 +++
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/devfreq/governor_simpleondemand.c
b/drivers/devfreq/governor_simpleondemand.c
index ea287b57cbf3..341eb7e9dc04 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -15,15 +15,19 @@
/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
#define DFSO_UPTHRESHOLD (90)
#define DFSO_DOWNDIFFERENCTIAL(5)
+
+static struct devfreq_simple_ondemand_data od_default = {
+ .upthreshold = DFSO_UPTHRESHOLD,
+ .downdifferential = DFSO_DOWNDIFFERENCTIAL,
+};
+
static int devfreq_simple_ondemand_func(struct devfreq *df,
unsigned long *freq)
{
int err;
struct devfreq_dev_status *stat;
unsigned long long a, b;
- unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
- unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
- struct devfreq_simple_ondemand_data *data = df->data;
+ struct devfreq_simple_ondemand_data *od = df->data;
err = devfreq_update_stats(df);
if (err)
@@ -31,16 +35,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
stat = >last_status;
- if (data) {
- if (data->upthreshold)
- dfso_upthreshold = data->upthreshold;
- if (data->downdifferential)
- dfso_downdifferential = data->downdifferential;
- }
- if (dfso_upthreshold > 100 ||
- dfso_upthreshold < dfso_downdifferential)
- return -EINVAL;
-
/* Assume MAX if it is going to be divided by zero */
if (stat->total_time == 0) {
*freq = DEVFREQ_MAX_FREQ;
@@ -55,7 +49,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
/* Set MAX if it's busy enough */
if (stat->busy_time * 100 >
- stat->total_time * dfso_upthreshold) {
+ stat->total_time * od->upthreshold) {
*freq = DEVFREQ_MAX_FREQ;
return 0;
}
@@ -68,7 +62,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
/* Keep the current frequency */
if (stat->busy_time * 100 >
- stat->total_time * (dfso_upthreshold - dfso_downdifferential)) {
+ stat->total_time * (od->upthreshold - od->downdifferential)) {
*freq = stat->current_frequency;
return 0;
}
@@ -78,17 +72,41 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
a *= stat->current_frequency;
b = div_u64(a, stat->total_time);
b *= 100;
- b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
+ b = div_u64(b, (od->upthreshold - od->downdifferential / 2));
*freq = (unsigned long) b;
return 0;
}
+static int devfreq_simple_ondemand_check_od(struct devfreq *devfreq)
+{
+ struct devfreq_simple_ondemand_data *od = devfreq->data;
+
+ if (od) {
+ if (!od->upthreshold)
+ od->upthreshold = DFSO_UPTHRESHOLD;
+
+ if (!od->downdifferential)
+ od->downdifferential = DFSO_DOWNDIFFERENCTIAL;
+
+ if (od->upthreshold > 100 ||
+ od->upthreshold < od->downdifferential)
+ return -EINVAL;
+ } else {
+ od = _default;
+ }
+
+ return 0;
+}
+
static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
switch (event) {
case DEVFREQ_GOV_START:
+ if (devfreq_simple_ondemand_check_od(devfreq))
+ return -EINVAL;
+
return devfreq_monitor_start(devfreq);
case DEVFREQ_GOV_STOP:
I'm editing the upthreshold and downdifferential for exposing them
via sysfs. So that after my work to expose them via sysfs,
send the patches if you think that need to do more about them.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing=dc9e557845c17cee173a6adcc3ae14940da03f44
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 08/11] PM / devfreq: check get_dev_status in devfreq_update_stats
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
Check .get_dev_status() in devfreq_update_stats in case it's abused
when a device does not provide it.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/governor.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 31af6d072a10..67a6dbdd5d23 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -89,6 +89,9 @@ int devfreq_update_target(struct devfreq *devfreq, unsigned
long freq);
static inline int devfreq_update_stats(struct devfreq *df)
{
+ if (!df->profile->get_dev_status)
+ return -EINVAL;
+
return df->profile->get_dev_status(df->dev.parent, >last_status);
}
#endif /* _GOVERNOR_H */
Applied it. Thanks.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 07/11] PM / devfreq: check get_dev_status before start monitor
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
The devfreq monitor depends on the device to provide load information
by .get_dev_status() to calculate the next target freq.
And this will cause changing governor to simple ondemand fail
if device can't support.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/devfreq.c | 10 +++---
drivers/devfreq/governor.h| 2 +-
drivers/devfreq/governor_simpleondemand.c | 3 +--
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7231fe6862a2..d1787b6c7d7c 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -482,10 +482,13 @@ static void devfreq_monitor(struct work_struct *work)
* to be called from governor in response to DEVFREQ_GOV_START
* event when device is added to devfreq framework.
*/
-void devfreq_monitor_start(struct devfreq *devfreq)
+int devfreq_monitor_start(struct devfreq *devfreq)
{
if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
- return;
+ return 0;
+
+ if (!devfreq->profile->get_dev_status)
+ return -EINVAL;
switch (devfreq->profile->timer) {
case DEVFREQ_TIMER_DEFERRABLE:
@@ -495,12 +498,13 @@ void devfreq_monitor_start(struct devfreq *devfreq)
INIT_DELAYED_WORK(>work, devfreq_monitor);
break;
default:
- return;
+ return -EINVAL;
}
if (devfreq->profile->polling_ms)
queue_delayed_work(devfreq_wq, >work,
msecs_to_jiffies(devfreq->profile->polling_ms));
+ return 0;
}
EXPORT_SYMBOL(devfreq_monitor_start);
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 5cee3f64fe2b..31af6d072a10 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -75,7 +75,7 @@ struct devfreq_governor {
unsigned int event, void *data);
};
-void devfreq_monitor_start(struct devfreq *devfreq);
+int devfreq_monitor_start(struct devfreq *devfreq);
void devfreq_monitor_stop(struct devfreq *devfreq);
void devfreq_monitor_suspend(struct devfreq *devfreq);
void devfreq_monitor_resume(struct devfreq *devfreq);
diff --git a/drivers/devfreq/governor_simpleondemand.c
b/drivers/devfreq/governor_simpleondemand.c
index d57b82a2b570..ea287b57cbf3 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -89,8 +89,7 @@ static int devfreq_simple_ondemand_handler(struct devfreq
*devfreq,
{
switch (event) {
case DEVFREQ_GOV_START:
- devfreq_monitor_start(devfreq);
- break;
+ return devfreq_monitor_start(devfreq);
case DEVFREQ_GOV_STOP:
devfreq_monitor_stop(devfreq);
Need to handle the all points of devfreq_monitor_start() usage.
please check the tegra30-devfreq.c for this update.
$ grep -rn "devfreq_monitor_start" drivers/
drivers/devfreq/governor_simpleondemand.c:92:
devfreq_monitor_start(devfreq);
drivers/devfreq/tegra30-devfreq.c:744: devfreq_monitor_start(devfreq);
..
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 05/11] PM / devfreq: use more accurate returned new_freq as resume_freq
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
Use the more accurate returned new_freq as resume_freq.
It's the same as how devfreq->previous_freq was updated.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 6e80bf70e7b3..ce569bd9adfa 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -390,7 +390,7 @@ static int devfreq_set_target(struct devfreq *devfreq,
unsigned long new_freq,
devfreq->previous_freq = new_freq;
if (devfreq->suspend_freq)
- devfreq->resume_freq = cur_freq;
+ devfreq->resume_freq = new_freq;
return err;
}
This patch fixes the previous patch[1]. So that you need to
add 'Fixes' tag as following:
Fixes: 83f8ca45afbf0 ("PM / devfreq: add support for suspend/resume of a
devfreq device")
commit 83f8ca45afbf041e312909f442128b99657d90b7
Refs: v4.20-rc6-2-g83f8ca45afbf
Author: Lukasz Luba
AuthorDate: Wed Dec 5 12:05:53 2018 +0100
Commit: MyungJoo Ham
CommitDate: Tue Dec 11 11:09:47 2018 +0900
PM / devfreq: add support for suspend/resume of a devfreq device
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 04/11] PM / devfreq: bail out early if no freq changes in devfreq_set_target
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote: It's unnecessary to set the same freq again and run notifier calls. Signed-off-by: Dong Aisheng --- drivers/devfreq/devfreq.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index bf3047896e41..6e80bf70e7b3 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -358,6 +358,9 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, else cur_freq = devfreq->previous_freq; + if (new_freq == cur_freq) + return 0; + freqs.old = cur_freq; freqs.new = new_freq; devfreq_notify_transition(devfreq, , DEVFREQ_PRECHANGE); @@ -374,7 +377,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq, * and DEVFREQ_POSTCHANGE because for showing the correct frequency * change order of between devfreq device and passive devfreq device. */ - if (trace_devfreq_frequency_enabled() && new_freq != cur_freq) + if (trace_devfreq_frequency_enabled()) trace_devfreq_frequency(devfreq, new_freq, cur_freq); freqs.new = new_freq; I'd like you to squash patch4 with patch6 because actually patch6 is too minor clean-up. I think it is possible. -- Best Regards, Samsung Electronics Chanwoo Choi
Re: [PATCH 03/11] PM / devfreq: fix the wrong set_freq path for userspace governor
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote: Fix the wrong set_freq path for userspace governor. Signed-off-by: Dong Aisheng --- drivers/devfreq/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig index 00704efe6398..20373a893b44 100644 --- a/drivers/devfreq/Kconfig +++ b/drivers/devfreq/Kconfig @@ -62,7 +62,7 @@ config DEVFREQ_GOV_USERSPACE help Sets the frequency at the user specified one. This governor returns the user configured frequency if there - has been an input to /sys/devices/.../power/devfreq_set_freq. + has been an input to /sys/devices/.../userspace/set_freq. Otherwise, the governor does not change the frequency given at the initialization. Looks good. But this patch just fix the information in Kconfig instead of fixing the wrong operation. To clarify the commit message, I'll change the patch title and commit message as following: PM / devfreq: Fix the wrong set_freq path for userspace governor in Kconfig Fix the wrong set_freq path for userspace governor in Kconfig. Signed-off-by: Dong Aisheng Applied it. But, if you have any other objection, please let me know. -- Best Regards, Samsung Electronics Chanwoo Choi
Re: [PATCH 02/11] PM / devfreq: remove the invalid description for get_target_freq
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
First of all, no_central_polling was removed since
commit 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices
which can idle")
Secondly, get_target_freq() is not only called only with update_devfreq()
notified by OPP now, but also min/max freq qos notifier.
So remove this invalid description now to avoid confusing.
Signed-off-by: Dong Aisheng
---
drivers/devfreq/governor.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 70f44b3ca42e..5cee3f64fe2b 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -57,8 +57,6 @@
*Basically, get_target_freq will run
*devfreq_dev_profile.get_dev_status() to get the
*status of the device (load = busy_time / total_time).
- * If no_central_polling is set, this callback is called
- * only with update_devfreq() notified by OPP.
* @event_handler: Callback for devfreq core framework to notify events
* to governors. Events include per device governor
* init and exit, opp changes out of devfreq, suspend
As I replied from patch1, I recommend that squash it with patch1.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH 01/11] doc: ABI: devfreq: remove invalid central_polling description
On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
no_central_polling has been removed since
commit 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices
which can idle")
Signed-off-by: Dong Aisheng
---
Documentation/ABI/testing/sysfs-class-devfreq | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-devfreq
b/Documentation/ABI/testing/sysfs-class-devfreq
index 386bc230a33d..5e6b74f30406 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -97,10 +97,7 @@ Description:
object. The values are represented in ms. If the value is
less than 1 jiffy, it is considered to be 0, which means
no polling. This value is meaningless if the governor is
- not polling; thus. If the governor is not using
- devfreq-provided central polling
- (/sys/class/devfreq/.../central_polling is 0), this value
- may be useless.
+ not polling.
A list of governors that support the node:
- simple_ondmenad
Looks good. But, you better to make one patch with patch1/2 by
squashing.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [RESEND PATCH v5 4/4] dt-bindings: devfreq: rk3399_dmc: Remove references of unexistant defines
DS_60ohm.
> + value is 60.
>
> - rockchip,lpddr4_dq_odt : When the DRAM type is LPDDR4, this parameter
> defines
> the DRAM side ODT on DQS/DQ line strength in
> ohms.
> - Default value is LP4_DQ_ODT_40ohm.
> + Default value is 40.
>
> - rockchip,lpddr4_ca_odt : When the DRAM type is LPDDR4, this parameter
> defines
> the DRAM side ODT on CA line strength in ohms.
> - Default value is LP4_CA_ODT_40ohm.
> + Default value is 40.
>
> - rockchip,phy_lpddr4_ca_drv : When the DRAM type is LPDDR4, this
> parameter defines
> the PHY side CA line (including command
> address
> - line) driver strength. Default value is
> - PHY_DRV_ODT_40.
> + line) driver strength. Default value is 40.
>
> - rockchip,phy_lpddr4_ck_cs_drv : When the DRAM type is LPDDR4, this
> parameter defines
> the PHY side clock line and CS line driver
> - strength. Default value is PHY_DRV_ODT_80.
> + strength. Default value is 80.
>
> - rockchip,phy_lpddr4_dq_drv : When the DRAM type is LPDDR4, this
> parameter defines
> the PHY side DQ line (including DQS/DQ/DM
> line)
> - driver strength. Default value is
> PHY_DRV_ODT_80.
> + driver strength. Default value is 80.
>
> - rockchip,phy_lpddr4_odt :When the DRAM type is LPDDR4, this parameter
> defines
> - the PHY side ODT strength. Default value is
> - PHY_DRV_ODT_60.
> + the PHY side ODT strength. Default value is
> 60.
>
> Example:
> dmc_opp_table: dmc_opp_table {
> @@ -195,23 +190,23 @@ Example:
> rockchip,phy_dll_dis_freq = <125>;
> rockchip,auto_pd_dis_freq = <666>;
> rockchip,ddr3_odt_dis_freq = <333>;
> - rockchip,ddr3_drv = ;
> - rockchip,ddr3_odt = ;
> - rockchip,phy_ddr3_ca_drv = ;
> - rockchip,phy_ddr3_dq_drv = ;
> - rockchip,phy_ddr3_odt = ;
> + rockchip,ddr3_drv = <40>;
> + rockchip,ddr3_odt = <120>;
> + rockchip,phy_ddr3_ca_drv = <40>;
> + rockchip,phy_ddr3_dq_drv = <40>;
> + rockchip,phy_ddr3_odt = <240>;
> rockchip,lpddr3_odt_dis_freq = <333>;
> - rockchip,lpddr3_drv = ;
> - rockchip,lpddr3_odt = ;
> - rockchip,phy_lpddr3_ca_drv = ;
> - rockchip,phy_lpddr3_dq_drv = ;
> - rockchip,phy_lpddr3_odt = ;
> + rockchip,lpddr3_drv = <34>;
> + rockchip,lpddr3_odt = <240>;
> + rockchip,phy_lpddr3_ca_drv = <40>;
> + rockchip,phy_lpddr3_dq_drv = <40>;
> + rockchip,phy_lpddr3_odt = <240>;
> rockchip,lpddr4_odt_dis_freq = <333>;
> - rockchip,lpddr4_drv = ;
> - rockchip,lpddr4_dq_odt = ;
> - rockchip,lpddr4_ca_odt = ;
> - rockchip,phy_lpddr4_ca_drv = ;
> - rockchip,phy_lpddr4_ck_cs_drv = ;
> - rockchip,phy_lpddr4_dq_drv = ;
> - rockchip,phy_lpddr4_odt = ;
> + rockchip,lpddr4_drv = <60>;
> + rockchip,lpddr4_dq_odt = <40>;
> + rockchip,lpddr4_ca_odt = <40>;
> + rockchip,phy_lpddr4_ca_drv = <40>;
> + rockchip,phy_lpddr4_ck_cs_drv = <80>;
> + rockchip,phy_lpddr4_dq_drv = <80>;
> + rockchip,phy_lpddr4_odt = <60>;
> };
>
Applied it with following minor fixup (unneeded spaces). Thanks
diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
index ddde2c4f97df..ac189dd82b08 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
@@ -154,7 +154,7 @@ Following properties relate to DDR timing:
driver strength. Default value is 80.
- rockchip,phy_lpddr4_odt : When the DRAM type is LPDDR4, this parameter
defines
- the PHY side ODT strength. Default value is
60.
+ the PHY side ODT strength. Default value is
60.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [RESEND PATCH v5 1/4] dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle.
On 3/9/21 8:38 AM, Daniel Lezcano wrote: > From: Enric Balletbo i Serra > > The Rockchip DMC (Dynamic Memory Interface) needs to access to the PMU > general register files to know the DRAM type, so add a phandle to the > syscon that manages these registers. > > Signed-off-by: Enric Balletbo i Serra > Reviewed-by: Chanwoo Choi > Acked-by: Rob Herring > Signed-off-by: Gaël PORTAY > Acked-by: MyungJoo Ham > Signed-off-by: Daniel Lezcano > --- > Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt > b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt > index a10d1f6d85c6..a41bcfef95c8 100644 > --- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt > +++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt > @@ -12,6 +12,8 @@ Required properties: >for details. > - center-supply: DMC supply node. > - status: Marks the node enabled/disabled. > +- rockchip,pmu: Phandle to the syscon managing the "PMU > general register > + files". > > Optional properties: > - interrupts: The CPU interrupt number. The interrupt > specifier > Applied it. Thanks. -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH] power: supply: max8997_charger: make EXTCON dependency unconditional
Hi Krzysztof, On 3/9/21 1:11 AM, Krzysztof Kozlowski wrote: > On Mon, 8 Mar 2021 at 17:02, Arnd Bergmann wrote: >> >> On Mon, Mar 8, 2021 at 4:33 PM Krzysztof Kozlowski wrote: >>> >>> On 08/03/2021 16:29, Arnd Bergmann wrote: >>>> From: Arnd Bergmann >>>> >>>> Some of the extcon interfaces have a fallback implementation that can >>>> be used when EXTCON is disabled, but some others do not, causing a >>>> build failure: >>>> >>>> drivers/power/supply/max8997_charger.c:261:9: error: implicit declaration >>>> of function 'devm_extcon_register_notifier_all' >>>> [-Werror,-Wimplicit-function-declaration] >>>> ret = devm_extcon_register_notifier_all(>dev, >>>> charger->edev, >>>> ^ >>>> drivers/power/supply/max8997_charger.c:261:9: note: did you mean >>>> 'devm_extcon_register_notifier'? >>>> include/linux/extcon.h:263:19: note: 'devm_extcon_register_notifier' >>>> declared here >>>> static inline int devm_extcon_register_notifier(struct device *dev, >>>> >>>> I assume there is no reason to actually build this driver without extcon >>>> support, so a hard dependency is the easiest fix. Alternatively the >>>> header file could be extended to provide additional inline stubs. >>> >>> Hi Arnd, >>> >>> Thanks for the patch but I think I got it covered with: >>> https://lore.kernel.org/lkml/20210215100610.19911-2-cw00.c...@samsung.com/ >>> (sent via extcon tree). >>> >>> Did you experience a new/different issue? >> >> The patch should be fine and address the problem, I just didn't see it was >> already fixed in linux-next as I'm still testing on mainline (rc2 at >> the moment). >> >> I assume the fix will make it into a future -rc then. > > It's still only in linux-next via extcon tree, so it seems Greg did > not take it yet. > > Chanwoo, > You might need to follow up on this, so your pull request won't get lost. I'm sorry. Because of my fault, the previous pull request was not merged to v5.12-rc1. To fix this issue, I'll send the pull request for rc3 to Greg. Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v6 1/4] PM / devfreq: Register devfreq as a cooling device on demand
Hi, On 3/8/21 10:30 PM, Daniel Lezcano wrote: > Currently the default behavior is to manually having the devfreq > backend to register themselves as a devfreq cooling device. > > Instead of adding the code in the drivers for the thermal cooling > device registering, let's provide a flag in the devfreq's profile to > tell the common devfreq code to register the newly created devfreq as > a cooling device. > > Suggested-by: Chanwoo Choi > Signed-off-by: Daniel Lezcano > Reviewed-by: Steven Price > Reviewed-by: Lukasz Luba > --- > V6: >- Did one line string instead of multiline >- Removed thermal.h header inclusion as it is in devfreq_cooling.h >- Added forward declaration for thermal_cooling_device pointer in devfreq.h > V5: >- Changed subject prefix by: PM / devfreq > V4: >- Replaced thermal_cooling_device_unregister() by > devfreq_cooling_unregister() > V3: >- Rebased on linux-pm branch without units.h >- Set the cdev to NULL in case of error >- Added description for the cdev field in the devfreq structure > V2: >- Added is_cooling_device boolean in profile structure >- Register cooling device when the is_cooling_device boolean is set >- Remove devfreq cooling device registration in the backend drivers > V1: >- Register devfreq as a cooling device unconditionnally > --- > drivers/devfreq/devfreq.c | 9 + > include/linux/devfreq.h | 9 + > 2 files changed, 18 insertions(+) (snip) Applied it. Thanks. I made the pull request with immutable branch for preventing the conflict on other git repo as following: -- Best Regards, Chanwoo Choi Samsung Electronics The following changes since commit fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8: Linux 5.12-rc1 (2021-02-28 16:05:19 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git immutable-devfreq-v5.13-rc1 for you to fetch changes up to 1224451bb6f938023dd7fa4e7ba43bfb185bc9e3: PM / devfreq: Register devfreq as a cooling device on demand (2021-03-09 15:40:16 +0900) Daniel Lezcano (1): PM / devfreq: Register devfreq as a cooling device on demand drivers/devfreq/devfreq.c | 9 + include/linux/devfreq.h | 9 + 2 files changed, 18 insertions(+)
Re: [PATCH v3 1/4] devfreq: Register devfreq as a cooling device on demand
On 21. 3. 7. 오후 11:28, Daniel Lezcano wrote: On 07/03/2021 15:16, Chanwoo Choi wrote: On 21. 3. 7. 오후 6:45, Daniel Lezcano wrote: Currently the default behavior is to manually having the devfreq backend to register themselves as a devfreq cooling device. Instead of adding the code in the drivers for the thermal cooling device registering, let's provide a flag in the devfreq's profile to tell the common devfreq code to register the newly created devfreq as a cooling device. Suggested-by: Chanwoo Choi Signed-off-by: Daniel Lezcano --- V3: - Rebased on linux-pm branch without units.h - Set the cdev to NULL in case of error - Added description for the cdev field in the devfreq structure V2: - Added is_cooling_device boolean in profile structure - Register cooling device when the is_cooling_device boolean is set - Remove devfreq cooling device registration in the backend drivers V1: - Register devfreq as a cooling device unconditionnally --- [ ... ] return devfreq; err_init: @@ -960,6 +971,8 @@ int devfreq_remove_device(struct devfreq *devfreq) if (!devfreq) return -EINVAL; + thermal_cooling_device_unregister(devfreq->cdev); I have a question. Why don't you use devfreq_cooling_unregister()? When thermal_cooling_device_unregister(), how can we remove the pm_qos_request of devfreq device? You are perfectly right. I failed to call the right function :/ Will fix it with a v4. Thanks. And, please add 'PM /' prefix for patch in order to keep the consistent patch title format. PM / devfreq: Register devfreq as a cooling device on demand -- Best Regards, Samsung Electronics Chanwoo Choi
Re: [PATCH v3 1/4] devfreq: Register devfreq as a cooling device on demand
On 21. 3. 7. 오후 6:45, Daniel Lezcano wrote:
> Currently the default behavior is to manually having the devfreq
> backend to register themselves as a devfreq cooling device.
>
> Instead of adding the code in the drivers for the thermal cooling
> device registering, let's provide a flag in the devfreq's profile to
> tell the common devfreq code to register the newly created devfreq as
> a cooling device.
>
> Suggested-by: Chanwoo Choi
> Signed-off-by: Daniel Lezcano
> ---
> V3:
> - Rebased on linux-pm branch without units.h
> - Set the cdev to NULL in case of error
> - Added description for the cdev field in the devfreq structure
> V2:
> - Added is_cooling_device boolean in profile structure
> - Register cooling device when the is_cooling_device boolean is set
> - Remove devfreq cooling device registration in the backend drivers
> V1:
> - Register devfreq as a cooling device unconditionnally
> ---
> drivers/devfreq/devfreq.c | 13 +
> include/linux/devfreq.h | 8
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index bf3047896e41..cf64aeec468f 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -26,6 +27,7 @@
> #include
> #include
> #include
> +#include
> #include "governor.h"
>
> #define CREATE_TRACE_POINTS
> @@ -935,6 +937,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
>
> mutex_unlock(_list_lock);
>
> + if (devfreq->profile->is_cooling_device) {
> + devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
> + if (IS_ERR(devfreq->cdev)) {
> + dev_info(dev, "Failed to register devfreq "
> + "cooling device\n");
> + devfreq->cdev = NULL;
> + }
> + }
> +
> return devfreq;
>
> err_init:
> @@ -960,6 +971,8 @@ int devfreq_remove_device(struct devfreq *devfreq)
> if (!devfreq)
> return -EINVAL;
>
> + thermal_cooling_device_unregister(devfreq->cdev);
I have a question. Why don't you use devfreq_cooling_unregister()?
When thermal_cooling_device_unregister(), how can we remove
the pm_qos_request of devfreq device?
> +
> if (devfreq->governor) {
> devfreq->governor->event_handler(devfreq,
>DEVFREQ_GOV_STOP, NULL);
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 26ea0850be9b..aba7ace11b72 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -98,11 +98,15 @@ struct devfreq_dev_status {
>* @freq_table: Optional list of frequencies to support
> statistics
>* and freq_table must be generated in ascending order.
>* @max_state: The size of freq_table.
> + *
> + * @is_cooling_device: A self-explanatory boolean giving the device a
> + * cooling effect property.
>*/
> struct devfreq_dev_profile {
> unsigned long initial_freq;
> unsigned int polling_ms;
> enum devfreq_timer timer;
> + bool is_cooling_device;
>
> int (*target)(struct device *dev, unsigned long *freq, u32 flags);
> int (*get_dev_status)(struct device *dev,
> @@ -156,6 +160,7 @@ struct devfreq_stats {
>* @suspend_count: suspend requests counter for a device.
>* @stats: Statistics of devfreq device behavior
>* @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER
> notifier
> + * @cdev:Cooling device pointer if the devfreq has cooling property
>* @nb_min: Notifier block for DEV_PM_QOS_MIN_FREQUENCY
>* @nb_max: Notifier block for DEV_PM_QOS_MAX_FREQUENCY
>*
> @@ -198,6 +203,9 @@ struct devfreq {
>
> struct srcu_notifier_head transition_notifier_list;
>
> + /* Pointer to the cooling device if used for thermal mitigation */
> + struct thermal_cooling_device *cdev;
> +
> struct notifier_block nb_min;
> struct notifier_block nb_max;
> };
>
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH v3 1/4] devfreq: Register devfreq as a cooling device on demand
On 21. 3. 7. 오후 6:45, Daniel Lezcano wrote:
Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.
Instead of adding the code in the drivers for the thermal cooling
device registering, let's provide a flag in the devfreq's profile to
tell the common devfreq code to register the newly created devfreq as
a cooling device.
Suggested-by: Chanwoo Choi
Signed-off-by: Daniel Lezcano
---
V3:
- Rebased on linux-pm branch without units.h
- Set the cdev to NULL in case of error
- Added description for the cdev field in the devfreq structure
V2:
- Added is_cooling_device boolean in profile structure
- Register cooling device when the is_cooling_device boolean is set
- Remove devfreq cooling device registration in the backend drivers
V1:
- Register devfreq as a cooling device unconditionnally
---
drivers/devfreq/devfreq.c | 13 +
include/linux/devfreq.h | 8
2 files changed, 21 insertions(+)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index bf3047896e41..cf64aeec468f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -26,6 +27,7 @@
#include
#include
#include
+#include
#include "governor.h"
#define CREATE_TRACE_POINTS
@@ -935,6 +937,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
mutex_unlock(_list_lock);
+ if (devfreq->profile->is_cooling_device) {
+ devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
+ if (IS_ERR(devfreq->cdev)) {
+ dev_info(dev, "Failed to register devfreq "
+"cooling device\n");
+ devfreq->cdev = NULL;
+ }
+ }
+
return devfreq;
err_init:
@@ -960,6 +971,8 @@ int devfreq_remove_device(struct devfreq *devfreq)
if (!devfreq)
return -EINVAL;
+ thermal_cooling_device_unregister(devfreq->cdev);
I have a question. Why don't you use devfreq_cooling_unregister()?
When thermal_cooling_device_unregister(), how can we remove
the pm_qos_request of devfreq device?
+
if (devfreq->governor) {
devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_STOP, NULL);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 26ea0850be9b..aba7ace11b72 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -98,11 +98,15 @@ struct devfreq_dev_status {
* @freq_table: Optional list of frequencies to support
statistics
*and freq_table must be generated in ascending order.
* @max_state:The size of freq_table.
+ *
+ * @is_cooling_device: A self-explanatory boolean giving the device a
+ * cooling effect property.
*/
struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
enum devfreq_timer timer;
+ bool is_cooling_device;
int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,
@@ -156,6 +160,7 @@ struct devfreq_stats {
* @suspend_count: suspend requests counter for a device.
* @stats:Statistics of devfreq device behavior
* @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER
notifier
+ * @cdev: Cooling device pointer if the devfreq has cooling property
* @nb_min: Notifier block for DEV_PM_QOS_MIN_FREQUENCY
* @nb_max: Notifier block for DEV_PM_QOS_MAX_FREQUENCY
*
@@ -198,6 +203,9 @@ struct devfreq {
struct srcu_notifier_head transition_notifier_list;
+ /* Pointer to the cooling device if used for thermal mitigation */
+ struct thermal_cooling_device *cdev;
+
struct notifier_block nb_min;
struct notifier_block nb_max;
};
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH v2 1/4] devfreq: Register devfreq as a cooling device on demand
Hi,
On 21. 3. 6. 오전 2:03, Daniel Lezcano wrote:
Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.
Instead of adding the code in the drivers for the thermal cooling
device registering, let's provide a flag in the devfreq's profile to
tell the common devfreq code to register the newly created devfreq as
a cooling device.
Suggested-by: Chanwoo Choi
Signed-off-by: Daniel Lezcano
---
drivers/devfreq/devfreq.c | 11 +++
include/linux/devfreq.h | 7 +++
2 files changed, 18 insertions(+)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d63f02d293..5c0fdd3a48d2 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -26,6 +27,7 @@
#include
#include
#include
+#include > #include
Acutally, current devfreq-next branch doesn't contain previous
your patch about units clean-up. So that when I applied it to next
branch, merge conflict happen because of ''.
Just send the patches again regardless of 'devfreq: Use HZ macros'.
#include "governor.h"
@@ -935,6 +937,13 @@ struct devfreq *devfreq_add_device(struct device *dev,
mutex_unlock(_list_lock);
+ if (devfreq->profile->is_cooling_device) {
+ devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
+ if (IS_ERR(devfreq->cdev))
+ dev_info(dev, "Failed to register devfreq "
+"cooling device\n");
Have to initialize devfreq->cdev by NULL when error happen.
Because of just returning from thermal_cooling_device_unregister
when cdev is not used.
+ }
+
return devfreq;
err_init:
@@ -960,6 +969,8 @@ int devfreq_remove_device(struct devfreq *devfreq)
if (!devfreq)
return -EINVAL;
+ thermal_cooling_device_unregister(devfreq->cdev);
+
if (devfreq->governor) {
devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_STOP, NULL);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 26ea0850be9b..554e7904b0c5 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -98,11 +98,15 @@ struct devfreq_dev_status {
* @freq_table: Optional list of frequencies to support
statistics
*and freq_table must be generated in ascending order.
* @max_state:The size of freq_table.
+ *
+ * @is_cooling_device: A self-explanatory boolean giving the device a
+ * cooling effect property.
*/
struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
enum devfreq_timer timer;
+ bool is_cooling_device;
int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,
@@ -198,6 +202,9 @@ struct devfreq {
struct srcu_notifier_head transition_notifier_list;
+ /* Pointer to the cooling device if used for thermal mitigation */ > + struct thermal_cooling_device *cdev;
+
You need to the description of cdev instance to 'struct devfreq'.
struct notifier_block nb_min;
struct notifier_block nb_max;
};
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [PATCH] devfreq: Register devfreq as a cooling device
Hi Daniel,
As Lukasz's comment, actually some devfreq devices like memory bus
might not affect the thermal critically. In the mainline,
there are four types devfreq as following:
1. GPU
2. UFS Storage
3. DMC (Memory Controller)
4. Memory bus like AMBA AXI
I think that you can specify this devfreq device will be used
for cooling device by editing the devfreq_dev_profile structure.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index bf3047896e41..77966a17d03f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -935,6 +935,13 @@ struct devfreq *devfreq_add_device(struct device *dev,
mutex_unlock(_list_lock);
+ if (devfreq->profile->is_cooling_device) {
+ devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
+ if (IS_ERR(devfreq->cdev))
+ dev_info(dev,
+ "Failed to register devfreq cooling
device\n");
+ }
+
return devfreq;
err_init:
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 26ea0850be9b..26dc69f1047b 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -103,6 +103,7 @@ struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
enum devfreq_timer timer;
+ bool is_cooling_device;
int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,
Thanks
Chanwoo Choi
On 21. 3. 4. 오후 9:50, Daniel Lezcano wrote:
Currently the default behavior is to manually having the devfreq
backend to register themselves as a devfreq cooling device.
There are no so many and actually it makes more sense to register the
devfreq device when adding it.
Consequently, every devfreq becomes a cooling device like cpufreq is.
Having a devfreq being registered as a cooling device can not mitigate
a thermal zone if it is not bound to this one. Thus, the current
configurations are not impacted by this change.
Signed-off-by: Daniel Lezcano
---
drivers/devfreq/devfreq.c | 8
drivers/gpu/drm/lima/lima_devfreq.c | 13 -
drivers/gpu/drm/lima/lima_devfreq.h | 2 --
drivers/gpu/drm/msm/msm_gpu.c | 11 ---
drivers/gpu/drm/msm/msm_gpu.h | 2 --
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 -
include/linux/devfreq.h | 3 +++
7 files changed, 11 insertions(+), 41 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d63f02d293..19149b31b000 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -26,6 +27,7 @@
#include
#include
#include
+#include
#include
#include "governor.h"
@@ -935,6 +937,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
mutex_unlock(_list_lock);
+ devfreq->cdev = devfreq_cooling_em_register(devfreq, NULL);
+ if (IS_ERR(devfreq->cdev))
+ dev_info(dev, "Failed to register devfreq cooling device\n");
+
return devfreq;
err_init:
@@ -960,6 +966,8 @@ int devfreq_remove_device(struct devfreq *devfreq)
if (!devfreq)
return -EINVAL;
+ thermal_cooling_device_unregister(devfreq->cdev);
+
if (devfreq->governor) {
devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_STOP, NULL);
diff --git a/drivers/gpu/drm/lima/lima_devfreq.c
b/drivers/gpu/drm/lima/lima_devfreq.c
index 5686ad4aaf7c..a696eff1642c 100644
--- a/drivers/gpu/drm/lima/lima_devfreq.c
+++ b/drivers/gpu/drm/lima/lima_devfreq.c
@@ -7,7 +7,6 @@
*/
#include
#include
-#include
#include
#include
#include
@@ -90,11 +89,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
{
struct lima_devfreq *devfreq = >devfreq;
- if (devfreq->cooling) {
- devfreq_cooling_unregister(devfreq->cooling);
- devfreq->cooling = NULL;
- }
-
if (devfreq->devfreq) {
devm_devfreq_remove_device(ldev->dev, devfreq->devfreq);
devfreq->devfreq = NULL;
@@ -110,7 +104,6 @@ void lima_devfreq_fini(struct lima_device *ldev)
int lima_devfreq_init(struct lima_device *ldev)
{
- struct thermal_cooling_device *cooling;
struct device *dev = ldev->dev;
struct opp_table *opp_table;
struct devfreq *devfreq;
@@ -173,12 +166,6 @@ int lima_devfreq_init(struct lima_device *ldev)
ldevfreq->devfreq = devfreq;
- cooling = of_devfreq_cooling_register(dev->of_node, devfreq);
- if (IS_ERR(cooling))
- dev_info(dev, "Failed to register cooling device\n");
- else
-
Re: [PATCH] devfreq: rk3399_dmc: Simplify with dev_err_probe()
On 21. 2. 28. 오전 1:35, Krzysztof Kozlowski wrote:
On Sat, 29 Aug 2020 at 15:10, Chanwoo Choi wrote:
On Sat, Aug 29, 2020 at 12:31 AM Krzysztof Kozlowski wrote:
Common pattern of handling deferred probe can be simplified with
dev_err_probe(). Less code and the error value gets printed.
Signed-off-by: Krzysztof Kozlowski
---
drivers/devfreq/rk3399_dmc.c | 20 ++--
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 027769e39f9b..35b3542f1f7b 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -324,22 +324,14 @@ static int rk3399_dmcfreq_probe(struct platform_device
*pdev)
mutex_init(>lock);
data->vdd_center = devm_regulator_get(dev, "center");
- if (IS_ERR(data->vdd_center)) {
- if (PTR_ERR(data->vdd_center) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
-
- dev_err(dev, "Cannot get the regulator \"center\"\n");
- return PTR_ERR(data->vdd_center);
- }
+ if (IS_ERR(data->vdd_center))
+ return dev_err_probe(dev, PTR_ERR(data->vdd_center),
+"Cannot get the regulator \"center\"\n");
data->dmc_clk = devm_clk_get(dev, "dmc_clk");
- if (IS_ERR(data->dmc_clk)) {
- if (PTR_ERR(data->dmc_clk) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
-
- dev_err(dev, "Cannot get the clk dmc_clk\n");
- return PTR_ERR(data->dmc_clk);
- }
+ if (IS_ERR(data->dmc_clk))
+ return dev_err_probe(dev, PTR_ERR(data->dmc_clk),
+"Cannot get the clk dmc_clk\n");
data->edev = devfreq_event_get_edev_by_phandle(dev, 0);
if (IS_ERR(data->edev))
--
2.17.1
Applied it. Thanks.
Hi Chanwoo,
Do you know what happened with this patch? You replied that it is
applied but I cannot find it in the Linus' or next trees.
Hi Krzysztof,
There was some my mistake. I'm sorry.
I applied it again to next branch.
--
Best Regards,
Samsung Electronics
Chanwoo Choi
Re: [RFC 00/19] Rework support for i.MX8MQ interconnect with devfreq
Hi, You missed sending the patches to linux-pm mailing list. On next version, please send it linux-pm. Thanks, Chanwoo Choi On 21. 2. 20. 오전 12:59, Abel Vesa wrote: This has been on my queue for quite some time now. It is more of a proof-of-concept. This rework is done with the compatibility of future i.MX platforms in mind. For example, the i.MX8MP platform has multiple NoCs. This patchsets prepares the imx interconnect and imx devfreq for that too. As of now, none of the drivers involved are being used and there is no icc consumer on any off the i.MX platforms. Basically, the steps taken here are the following: 1. Make the dram_apb clock "reparantable" from kernel. This is needed in order to keep track of the actual parent of the dram_apb clock in the kernel clock hierarchy. Note that the actual switch is done EL3 (TF-A). 2. Rework the imx-bus so the actual link between the icc and the NoCs or the pl301s is not tightly coupled. This allows us to have as many NoCs as necessary but also allows as to use the same driver for the pl301s. The pl301s have their own clocks too, so we need to reduce their rates too. 3. Rework the imx8m-ddrc driver. Remove the support for dts defined OPPs. The EL3 provides those. So instead of havingi to keep the OPP table in both EL3 and kernel in sync, we rely on what the EL3 gives us. Also, when the platform suspends, the bus needs to be running at highest rate, otherwise there is a chance it might not resume anymore. By adding the late system sleep PM ops we can handle that easily. 4. Rework the imx interconnect driver to use the fsl,icc-id instead of the robust imx_icc_node_adj_desc for linking with the target node. By adding the fsl,icc-id property to all the NoC and pl301 dts nodes, we can link each icc node to their corresponding NoC, pl301 or dram. Basically, when the imx interconnect platform specific driver probes, it will take each node defined for that platform and look-up the corresponding dts node based on the id and add that as the qos device. 5. Added the fec and usdhc as icc consumers. This is just as an example. All the other consumers can be added later. Basically, each consumer will add a path to their device node and in the driver will have to handle that icc path accordingly. Abel Vesa (19): clk: imx8mq: Replace critical with ignore_unused flag for dram_apb clock dt-bindings: interconnect: imx8mq: Add missing pl301 and SAI ids devfreq: imx-bus: Switch governor to powersave devfreq: imx-bus: Decouple imx-bus from icc made devfreq: imx8m-ddrc: Change governor to powersave devfreq: imx8m-ddrc: Use the opps acquired from EL3 devfreq: imx8m-ddrc: Add late system sleep PM ops interconnect: imx: Switch from imx_icc_node_adj_desc to fsl,icc-id node assignment interconnect: imx8: Remove the imx_icc_node_adj_desc interconnect: imx8mq: Add the pl301_per_m and pl301_wakeup nodes and subnodes interconnect: imx8mq: Add of_match_table interconnect: imx: Add imx_icc_get_bw and imx_icc_aggregate functions arm64: dts: imx8mq: Add fsl,icc-id property to ddrc node arm64: dts: imx8mq: Add fsl,icc-id to noc node arm64: dts: imx8mq: Add all pl301 nodes arm64: dts: imx8mq: Add the interconnect node arm64: dts: imx8mq: Add interconnect properties to icc consumer nodes net: ethernet: fec_main: Add interconnect support mmc: sdhci-esdhc-imx: Add interconnect support arch/arm64/boot/dts/freescale/imx8mq.dtsi | 200 +- drivers/clk/imx/clk-imx8mq.c | 2 +- drivers/devfreq/imx-bus.c | 42 + drivers/devfreq/imx8m-ddrc.c | 75 +++- drivers/interconnect/imx/imx.c| 92 +- drivers/interconnect/imx/imx.h| 19 +- drivers/interconnect/imx/imx8mm.c | 32 ++-- drivers/interconnect/imx/imx8mn.c | 28 +-- drivers/interconnect/imx/imx8mq.c | 59 --- drivers/mmc/host/sdhci-esdhc-imx.c| 26 +++ drivers/net/ethernet/freescale/fec.h | 3 + drivers/net/ethernet/freescale/fec_main.c | 19 ++ include/dt-bindings/interconnect/imx8mq.h | 9 + scripts/dtc/fdtoverlay| Bin 0 -> 61280 bytes 14 files changed, 393 insertions(+), 213 deletions(-) create mode 100755 scripts/dtc/fdtoverlay
Re: [RFC 04/19] devfreq: imx-bus: Decouple imx-bus from icc made
On 21. 2. 20. 오전 1:00, Abel Vesa wrote:
The link between an imx-bus device and its icc id will be done
through the fsl,icc-id property in each dts node. The imx
interconnect driver will pick up all the dts nodes that have that
property defined and will link them to the rightfull icc id.
Signed-off-by: Abel Vesa
---
drivers/devfreq/imx-bus.c | 40 +++
1 file changed, 3 insertions(+), 37 deletions(-)
diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
index 1c0c92d0eb08..9f0df88b29c1 100644
--- a/drivers/devfreq/imx-bus.c
+++ b/drivers/devfreq/imx-bus.c
@@ -65,36 +65,6 @@ static void imx_bus_exit(struct device *dev)
platform_device_unregister(priv->icc_pdev);
}
-/* imx_bus_init_icc() - register matching icc provider if required */
-static int imx_bus_init_icc(struct device *dev)
-{
- struct imx_bus *priv = dev_get_drvdata(dev);
- const char *icc_driver_name;
-
- if (!of_get_property(dev->of_node, "#interconnect-cells", 0))
- return 0;
- if (!IS_ENABLED(CONFIG_INTERCONNECT_IMX)) {
- dev_warn(dev, "imx interconnect drivers disabled\n");
- return 0;
- }
-
- icc_driver_name = of_device_get_match_data(dev);
- if (!icc_driver_name) {
- dev_err(dev, "unknown interconnect driver\n");
- return 0;
- }
-
- priv->icc_pdev = platform_device_register_data(
- dev, icc_driver_name, -1, NULL, 0);
- if (IS_ERR(priv->icc_pdev)) {
- dev_err(dev, "failed to register icc provider %s: %ld\n",
- icc_driver_name, PTR_ERR(priv->icc_pdev));
- return PTR_ERR(priv->icc_pdev);
- }
-
- return 0;
-}
-
static int imx_bus_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
@@ -144,10 +114,6 @@ static int imx_bus_probe(struct platform_device *pdev)
goto err;
}
- ret = imx_bus_init_icc(dev);
- if (ret)
- goto err;
-
return 0;
err:
@@ -156,9 +122,9 @@ static int imx_bus_probe(struct platform_device *pdev)
}
static const struct of_device_id imx_bus_of_match[] = {
- { .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
- { .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
- { .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
+ { .compatible = "fsl,imx8mq-noc",},
+ { .compatible = "fsl,imx8mm-noc",},
+ { .compatible = "fsl,imx8mn-noc",},
{ .compatible = "fsl,imx8m-noc", },
{ .compatible = "fsl,imx8m-nic", },
{ /* sentinel */ },
For this patch, you need to get the ack of use of new 'fsl,icc-id
property'. IMO, looks good to me.
Acked-by: Chanwoo Choi
And you must update the DT binding document.
[1] Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
Thanks,
Chanwoo Choi
Re: [RFC 07/19] devfreq: imx8m-ddrc: Add late system sleep PM ops
On 21. 2. 20. 오전 1:00, Abel Vesa wrote:
Seems that, in order to be able to resume from suspend, the dram rate
needs to be the highest one available. Therefore, add the late system
suspend/resume PM ops which set the highest rate on suspend and the
latest one used before suspending on resume.
Signed-off-by: Abel Vesa
---
drivers/devfreq/imx8m-ddrc.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index 33de83acfd8b..04347dee781b 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -72,6 +72,8 @@ struct imx8m_ddrc {
struct clk *dram_alt;
struct clk *dram_apb;
+ unsigned long suspend_rate;
+ unsigned long resume_rate;
int freq_count;
struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
};
@@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev, unsigned
long *freq, u32 flags)
return ret;
}
+static int imx8m_ddrc_suspend(struct device *dev)
+{
+ struct imx8m_ddrc *priv = dev_get_drvdata(dev);
+
+ priv->resume_rate = clk_get_rate(priv->dram_core);
+
+ return imx8m_ddrc_target(dev, >suspend_rate, 0);
+}
+
+static int imx8m_ddrc_resume(struct device *dev)
+{
+ struct imx8m_ddrc *priv = dev_get_drvdata(dev);
+
+ return imx8m_ddrc_target(dev, >resume_rate, 0);
+}
+
static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
{
struct imx8m_ddrc *priv = dev_get_drvdata(dev);
@@ -336,6 +354,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
if (dev_pm_opp_add(dev, freq->rate * 25, 0))
return -ENODEV;
+
+ if (index == 0)
+ priv->suspend_rate = freq->rate * 25;
}
return 0;
@@ -412,10 +433,15 @@ static const struct of_device_id imx8m_ddrc_of_match[] = {
};
MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
+static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend, imx8m_ddrc_resume)
+};
Are there any reason to use suspend_late instead of suspend?
Usually, it is enough to change the frequency on normal suspend() step.
The devfreq supports the 'opp-suspend' property[1]. If you keep the OPP
entries on DT, you simply support your goal with 'opp-suspend'.
[1]
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/opp/opp.txt#L156
+
static struct platform_driver imx8m_ddrc_platdrv = {
.probe = imx8m_ddrc_probe,
.driver = {
.name = "imx8m-ddrc-devfreq",
+ .pm = _ddrc_pm_ops,
.of_match_table = of_match_ptr(imx8m_ddrc_of_match),
},
};
Re: [RFC 06/19] devfreq: imx8m-ddrc: Use the opps acquired from EL3
Hi,
On 21. 2. 20. 오전 1:00, Abel Vesa wrote:
i.MX8M platforms get their dram OPPs from the EL3.
We don't need to duplicate that in the kernel dram dts node.
We should just trust the OPPs provided by the EL3.
Signed-off-by: Abel Vesa
---
drivers/devfreq/imx8m-ddrc.c | 49 ++--
1 file changed, 2 insertions(+), 47 deletions(-)
diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index 3a6c04ba4f2e..33de83acfd8b 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -333,38 +333,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
if (freq->dram_core_parent_index == 2 &&
freq->dram_alt_parent_index == 0)
return -ENODEV;
- }
-
- return 0;
-}
-
-static int imx8m_ddrc_check_opps(struct device *dev)
-{
- struct imx8m_ddrc *priv = dev_get_drvdata(dev);
- struct imx8m_ddrc_freq *freq_info;
- struct dev_pm_opp *opp;
- unsigned long freq;
- int i, opp_count;
-
- /* Enumerate DT OPPs and disable those not supported by firmware */
- opp_count = dev_pm_opp_get_opp_count(dev);
- if (opp_count < 0)
- return opp_count;
- for (i = 0, freq = 0; i < opp_count; ++i, ++freq) {
- opp = dev_pm_opp_find_freq_ceil(dev, );
- if (IS_ERR(opp)) {
- dev_err(dev, "Failed enumerating OPPs: %ld\n",
- PTR_ERR(opp));
- return PTR_ERR(opp);
- }
- dev_pm_opp_put(opp);
- freq_info = imx8m_ddrc_find_freq(priv, freq);
- if (!freq_info) {
- dev_info(dev, "Disable unsupported OPP %luHz %luMT/s\n",
- freq, DIV_ROUND_CLOSEST(freq, 25));
- dev_pm_opp_disable(dev, freq);
- }
+ if (dev_pm_opp_add(dev, freq->rate * 25, 0))
+ return -ENODEV;
}
return 0;
@@ -372,7 +343,6 @@ static int imx8m_ddrc_check_opps(struct device *dev)
static void imx8m_ddrc_exit(struct device *dev)
{
- dev_pm_opp_of_remove_table(dev);
}
static int imx8m_ddrc_probe(struct platform_device *pdev)
@@ -419,16 +389,6 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
return ret;
}
- ret = dev_pm_opp_of_add_table(dev);
- if (ret < 0) {
- dev_err(dev, "failed to get OPP table\n");
- return ret;
- }
-
- ret = imx8m_ddrc_check_opps(dev);
- if (ret < 0)
- goto err;
In the devfreq_add_device(), if devfreq->profile->freq_table is NULL,
set_freq_table() function fill out the devfreq->profile->freq_table by
using the OPP information.
After this patch, devfreq->profile.freq_table is NULL and there are no
any OPP entries for this device, I think that this driver cannot add
devfreq device without any frequency information. Is it working without
any device frequency information?
-
priv->profile.polling_ms = 1000;
priv->profile.target = imx8m_ddrc_target;
priv->profile.get_dev_status = imx8m_ddrc_get_dev_status;
@@ -441,13 +401,8 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
if (IS_ERR(priv->devfreq)) {
ret = PTR_ERR(priv->devfreq);
dev_err(dev, "failed to add devfreq device: %d\n", ret);
- goto err;
}
- return 0;
-
-err:
- dev_pm_opp_of_remove_table(dev);
return ret;
}
Re: [RFC 03/19] devfreq: imx-bus: Switch governor to powersave
On 21. 2. 20. 오전 1:00, Abel Vesa wrote:
By switching to powersave governor, we allow the imx-bus to always run
at minimum rate needed by all the running masters.
Signed-off-by: Abel Vesa
---
drivers/devfreq/imx-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
index 4f38455ad742..1c0c92d0eb08 100644
--- a/drivers/devfreq/imx-bus.c
+++ b/drivers/devfreq/imx-bus.c
@@ -99,7 +99,7 @@ static int imx_bus_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct imx_bus *priv;
- const char *gov = DEVFREQ_GOV_USERSPACE;
+ const char *gov = DEVFREQ_GOV_POWERSAVE;
int ret;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
Acked-by: Chanwoo Choi
Thanks,
Chanwoo Choi
Re: [RFC 05/19] devfreq: imx8m-ddrc: Change governor to powersave
On 21. 2. 20. 오전 1:00, Abel Vesa wrote:
By switching to powersave governor, we allow the imx8m-ddrc to always
run at minimum rate needed by all the running masters.
Signed-off-by: Abel Vesa
---
drivers/devfreq/imx8m-ddrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index bc82d3653bff..3a6c04ba4f2e 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -379,7 +379,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct imx8m_ddrc *priv;
- const char *gov = DEVFREQ_GOV_USERSPACE;
+ const char *gov = DEVFREQ_GOV_POWERSAVE;
int ret;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
Acked-by: Chanwoo Choi
Thanks,
Chanwoo Choi
Re: [PATCH v2 3/9] devfreq: Use HZ macros
On 21. 2. 26. 오후 5:51, Daniel Lezcano wrote: On 26/02/2021 07:08, Chanwoo Choi wrote: On 21. 2. 24. 오후 11:42, Daniel Lezcano wrote: HZ unit conversion macros are available in units.h, use them and remove the duplicate definition. Signed-off-by: Daniel Lezcano Reviewed-by: Christian Eggers Acked-by: Chanwoo Choi --- drivers/devfreq/devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index bf3047896e41..b6d63f02d293 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "governor.h" #define CREATE_TRACE_POINTS @@ -33,7 +34,6 @@ #define IS_SUPPORTED_FLAG(f, name) ((f & DEVFREQ_GOV_FLAG_##name) ? true : false) #define IS_SUPPORTED_ATTR(f, name) ((f & DEVFREQ_GOV_ATTR_##name) ? true : false) -#define HZ_PER_KHZ 1000 static struct class *devfreq_class; static struct dentry *devfreq_debugfs; I changed the patch title with 'PM /' prefix as following in order to keep the consistent patch style if there are no any special objection. - PM / devfreq: Use HZ macros Applied it. It should not compile. This patch and others depend on 1/9. It would make sense to merge all of them through linux-pm. OK. I'm OK to apply it to linux-pm. Thanks, Chanwoo Choi
Re: [PATCH v2 3/9] devfreq: Use HZ macros
On 21. 2. 24. 오후 11:42, Daniel Lezcano wrote: HZ unit conversion macros are available in units.h, use them and remove the duplicate definition. Signed-off-by: Daniel Lezcano Reviewed-by: Christian Eggers Acked-by: Chanwoo Choi --- drivers/devfreq/devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index bf3047896e41..b6d63f02d293 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "governor.h" #define CREATE_TRACE_POINTS @@ -33,7 +34,6 @@ #define IS_SUPPORTED_FLAG(f, name) ((f & DEVFREQ_GOV_FLAG_##name) ? true : false) #define IS_SUPPORTED_ATTR(f, name) ((f & DEVFREQ_GOV_ATTR_##name) ? true : false) -#define HZ_PER_KHZ 1000 static struct class *devfreq_class; static struct dentry *devfreq_debugfs; I changed the patch title with 'PM /' prefix as following in order to keep the consistent patch style if there are no any special objection. - PM / devfreq: Use HZ macros Applied it. Thanks, Chanwoo Choi
Re: [PATCH] extcon: Fix error handling in extcon_dev_register
Hi,
On 21. 1. 19. 오후 5:10, Dinghao Liu wrote:
When devm_kcalloc() fails, we should execute device_unregister()
to unregister edev->dev from system.
Fixes: 046050f6e623e ("extcon: Update the prototype of extcon_register_notifier()
with enum extcon")
Signed-off-by: Dinghao Liu
---
drivers/extcon/extcon.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 0a6438cbb3f3..e7a9561a826d 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1241,6 +1241,7 @@ int extcon_dev_register(struct extcon_dev *edev)
sizeof(*edev->nh), GFP_KERNEL);
if (!edev->nh) {
ret = -ENOMEM;
+ device_unregister(>dev);
goto err_dev;
}
Thanks for fixup. Applied it.
Thanks,
Chanwoo Choi
Re: [RESEND PATCH v6 2/3] bindings: pm8941-misc: Add support for VBUS detection
On 21. 1. 26. 오전 9:38, Guru Das Srinagesh wrote: Add interrupt support for reporting VBUS detection status that can be detected via a dedicated PMIC pin. Signed-off-by: Anirudh Ghayal Signed-off-by: Guru Das Srinagesh Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml b/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml index e8eea83..6a9c96f 100644 --- a/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml +++ b/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml @@ -22,11 +22,14 @@ properties: maxItems: 1 interrupts: -maxItems: 1 +minItems: 1 +maxItems: 2 interrupt-names: +minItems: 1 items: - const: usb_id + - const: usb_vbus required: - compatible Applied it. Thanks, Chanwoo Choi
Re: [RESEND PATCH v6 1/3] bindings: pm8941-misc: Convert bindings to YAML
On 21. 1. 26. 오전 9:38, Guru Das Srinagesh wrote:
Convert bindings from txt to YAML.
Signed-off-by: Guru Das Srinagesh
Reviewed-by: Rob Herring
---
.../bindings/extcon/qcom,pm8941-misc.txt | 41 ---
.../bindings/extcon/qcom,pm8941-misc.yaml | 59 ++
2 files changed, 59 insertions(+), 41 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.txt
create mode 100644
Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml
diff --git a/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.txt
b/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.txt
deleted file mode 100644
index 35383adb..000
--- a/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-Qualcomm's PM8941 USB ID Extcon device
-
-Some Qualcomm PMICs have a "misc" module that can be used to detect when
-the USB ID pin has been pulled low or high.
-
-PROPERTIES
-
-- compatible:
-Usage: required
-Value type:
-Definition: Should contain "qcom,pm8941-misc";
-
-- reg:
-Usage: required
-Value type:
-Definition: Should contain the offset to the misc address space
-
-- interrupts:
-Usage: required
-Value type:
-Definition: Should contain the usb id interrupt
-
-- interrupt-names:
-Usage: required
-Value type:
-Definition: Should contain the string "usb_id" for the usb id interrupt
-
-Example:
-
- pmic {
- usb_id: misc@900 {
- compatible = "qcom,pm8941-misc";
- reg = <0x900>;
- interrupts = <0x0 0x9 0 IRQ_TYPE_EDGE_BOTH>;
- interrupt-names = "usb_id";
- };
- }
-
- usb-controller {
- extcon = <_id>;
- };
diff --git a/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml
b/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml
new file mode 100644
index 000..e8eea83
--- /dev/null
+++ b/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/extcon/qcom,pm8941-misc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. PM8941 USB ID Extcon device
+
+maintainers:
+ - Guru Das Srinagesh
+
+description: |
+ Some Qualcomm PMICs have a "misc" module that can be used to detect when
+ the USB ID pin has been pulled low or high.
+
+properties:
+ compatible:
+items:
+ - const: qcom,pm8941-misc
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ interrupt-names:
+items:
+ - const: usb_id
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+pmic {
+#address-cells = <1>;
+#size-cells = <0>;
+interrupt-controller;
+#interrupt-cells = <4>;
+
+usb_id: misc@900 {
+compatible = "qcom,pm8941-misc";
+reg = <0x900>;
+interrupts = <0x0 0x9 0 IRQ_TYPE_EDGE_BOTH>;
+interrupt-names = "usb_id";
+};
+};
+
+usb-controller {
+ extcon = <_id>;
+};
Applied it.
Thanks,
Chanwoo Choi
Re: [RESEND PATCH v6 3/3] extcon: qcom-spmi: Add support for VBUS detection
o->vbus_irq, NULL,
qcom_usb_irq_handler,
IRQF_TRIGGER_RISING |
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
pdev->name, info);
- if (ret < 0) {
- dev_err(dev, "failed to request handler for ID IRQ\n");
- return ret;
+ if (ret < 0) {
+ dev_err(dev, "failed to request handler for VBUS
IRQ\n");
+ return ret;
+ }
+ }
+
+ if (info->id_irq < 0 && info->vbus_irq < 0) {
+ dev_err(dev, "ID and VBUS IRQ not found\n");
+ return -EINVAL;
}
platform_set_drvdata(pdev, info);
@@ -120,8 +173,12 @@ static int qcom_usb_extcon_suspend(struct device *dev)
struct qcom_usb_extcon_info *info = dev_get_drvdata(dev);
int ret = 0;
- if (device_may_wakeup(dev))
- ret = enable_irq_wake(info->irq);
+ if (device_may_wakeup(dev)) {
+ if (info->id_irq > 0)
+ ret = enable_irq_wake(info->id_irq);
+ if (info->vbus_irq > 0)
+ ret = enable_irq_wake(info->vbus_irq);
+ }
return ret;
}
@@ -131,8 +188,12 @@ static int qcom_usb_extcon_resume(struct device *dev)
struct qcom_usb_extcon_info *info = dev_get_drvdata(dev);
int ret = 0;
- if (device_may_wakeup(dev))
- ret = disable_irq_wake(info->irq);
+ if (device_may_wakeup(dev)) {
+ if (info->id_irq > 0)
+ ret = disable_irq_wake(info->id_irq);
+ if (info->vbus_irq > 0)
+ ret = disable_irq_wake(info->vbus_irq);
+ }
return ret;
}
Looks good to me. Apply it. for v5.13-rc1.
Thanks,
Chanwoo Choi
Re: [RFC][PATCH 1/3] PM /devfreq: add user frequency limits into devfreq struct
On 2/16/21 7:41 PM, Lukasz Luba wrote:
> Hi Chanwoo,
>
> On 2/15/21 3:00 PM, Chanwoo Choi wrote:
>> Hi Lukasz,
>>
>> On Fri, Feb 12, 2021 at 7:28 AM Lukasz Luba wrote:
>>>
>>>
>>>
>>> On 2/11/21 11:07 AM, Lukasz Luba wrote:
>>>> Hi Chanwoo,
>>>>
>>>> On 2/3/21 10:21 AM, Lukasz Luba wrote:
>>>>> Hi Chanwoo,
>>>>>
>>>>> Thank you for looking at this.
>>>>>
>>>>> On 2/3/21 10:11 AM, Chanwoo Choi wrote:
>>>>>> Hi Lukasz,
>>>>>>
>>>>>> When accessing the max_freq and min_freq at devfreq-cooling.c,
>>>>>> even if can access 'user_max_freq' and 'lock' by using the 'devfreq'
>>>>>> instance,
>>>>>> I think that the direct access of variables
>>>>>> (lock/user_max_freq/user_min_freq)
>>>>>> of struct devfreq are not good.
>>>>>>
>>>>>> Instead, how about using the 'DEVFREQ_TRANSITION_NOTIFIER'
>>>>>> notification with following changes of 'struct devfreq_freq'?
>>>>>
>>>>> I like the idea with devfreq notification. I will have to go through the
>>>>> code to check that possibility.
>>>>>
>>>>>> Also, need to add codes into devfreq_set_target() for initializing
>>>>>> 'new_max_freq' and 'new_min_freq' before sending the DEVFREQ_POSTCHANGE
>>>>>> notification.
>>>>>>
>>>>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>>>>>> index 147a229056d2..d5726592d362 100644
>>>>>> --- a/include/linux/devfreq.h
>>>>>> +++ b/include/linux/devfreq.h
>>>>>> @@ -207,6 +207,8 @@ struct devfreq {
>>>>>> struct devfreq_freqs {
>>>>>> unsigned long old;
>>>>>> unsigned long new;
>>>>>> + unsigned long new_max_freq;
>>>>>> + unsigned long new_min_freq;
>>>>>> };
>>>>>>
>>>>>>
>>>>>> And I think that new 'user_min_freq'/'user_max_freq' are not necessary.
>>>>>> You can get the current max_freq/min_freq by using the following steps:
>>>>>>
>>>>>> get_freq_range(devfreq, _freq, _freq);
>>>>>> dev_pm_opp_find_freq_floor(pdev, _freq);
>>>>>> dev_pm_opp_find_freq_floor(pdev, _freq);
>>>>>>
>>>>>> So that you can get the 'max_freq/min_freq' and then
>>>>>> initialize the 'freqs.new_max_freq and freqs.new_min_freq'
>>>>>> with them as following:
>>>>>>
>>>>>> in devfreq_set_target()
>>>>>> get_freq_range(devfreq, _freq, _freq);
>>>>>> dev_pm_opp_find_freq_floor(pdev, _freq);
>>>>>> dev_pm_opp_find_freq_floor(pdev, _freq);
>>>>>> freqs.new_max_freq = min_freq;
>>>>>> freqs.new_max_freq = max_freq;
>>>>>> devfreq_notify_transition(devfreq, , DEVFREQ_POSTCHANGE);
>>>>>
>>>>> I will plumb it in and check that option. My concern is that function
>>>>> get_freq_range() would give me the max_freq value from PM QoS, which
>>>>> might be my thermal limit - lower that user_max_freq. Then I still
>>>>> need
>>>>>
>>>>> I've been playing with PM QoS notifications because I thought it would
>>>>> be possible to be notified in thermal for all new set values - even from
>>>>> devfreq sysfs user max_freq write, which has value higher that the
>>>>> current limit set by thermal governor. Unfortunately PM QoS doesn't
>>>>> send that information by design. PM QoS also by desing won't allow
>>>>> me to check first two limits in the plist - which would be thermal
>>>>> and user sysfs max_freq.
>>>>>
>>>>> I will experiment with this notifications and share the results.
>>>>> That you for your comments.
>>>>
>>>> I have experimented with your proposal. Unfortunately, the value stored
>>>> in the pm_qos which is read by get_freq_range() is not the user max
>>>> freq. It's the value from thermal devfreq cooling when that one is
>>>> lower. Which is OK in the overall design, but not for my IPA use case.
>>>>
>>&
Re: [PATCH 2/2] units: Use the HZ_PER_KHZ macro
On 2/24/21 5:30 AM, Daniel Lezcano wrote: > The HZ_PER_KHZ macro definition is duplicated in different subsystems. > > The macro now exists in include/linux/units.h, make use of it and > remove all the duplicated ones. > > Signed-off-by: Daniel Lezcano > --- > drivers/devfreq/devfreq.c | 2 +- > drivers/iio/light/as73211.c | 3 +-- > drivers/thermal/devfreq_cooling.c | 2 +- > 3 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 6aa10de792b3..4c636c336ace 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > #include "governor.h" > > #define CREATE_TRACE_POINTS > @@ -33,7 +34,6 @@ > > #define IS_SUPPORTED_FLAG(f, name) ((f & DEVFREQ_GOV_FLAG_##name) ? true : > false) > #define IS_SUPPORTED_ATTR(f, name) ((f & DEVFREQ_GOV_ATTR_##name) ? true : > false) > -#define HZ_PER_KHZ 1000 > > static struct class *devfreq_class; > static struct dentry *devfreq_debugfs; > diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c > index 7b32dfaee9b3..3ba2378df3dd 100644 > --- a/drivers/iio/light/as73211.c > +++ b/drivers/iio/light/as73211.c > @@ -24,8 +24,7 @@ > #include > #include > #include > - > -#define HZ_PER_KHZ 1000 > +#include > > #define AS73211_DRV_NAME "as73211" > > diff --git a/drivers/thermal/devfreq_cooling.c > b/drivers/thermal/devfreq_cooling.c > index fed3121ff2a1..fa5b8b0c7604 100644 > --- a/drivers/thermal/devfreq_cooling.c > +++ b/drivers/thermal/devfreq_cooling.c > @@ -19,10 +19,10 @@ > #include > #include > #include > +#include > > #include > > -#define HZ_PER_KHZ 1000 > #define SCALE_ERROR_MITIGATION 100 > > static DEFINE_IDA(devfreq_ida); > For devfreq part, Acked-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [RESEND PATCH v6 3/3] extcon: qcom-spmi: Add support for VBUS detection
Hi,
Please use get_maintainer.pl script when you send the patches.
I didn't receive the patches. I'll review.
Thanks,
Chanwoo Choi
On 21. 1. 26. 오전 9:38, Guru Das Srinagesh wrote:
From: Anirudh Ghayal
VBUS can be detected via a dedicated PMIC pin. Add support
for reporting the VBUS status.
Signed-off-by: Anirudh Ghayal
Signed-off-by: Kavya Nunna
Signed-off-by: Guru Das Srinagesh
---
drivers/extcon/extcon-qcom-spmi-misc.c | 99 +++---
1 file changed, 80 insertions(+), 19 deletions(-)
diff --git a/drivers/extcon/extcon-qcom-spmi-misc.c
b/drivers/extcon/extcon-qcom-spmi-misc.c
index 6b836ae..9e8ccfb 100644
--- a/drivers/extcon/extcon-qcom-spmi-misc.c
+++ b/drivers/extcon/extcon-qcom-spmi-misc.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/**
* extcon-qcom-spmi-misc.c - Qualcomm USB extcon driver to support USB ID
- * detection based on extcon-usb-gpio.c.
+ * and VBUS detection based on extcon-usb-gpio.c.
*
* Copyright (C) 2016 Linaro, Ltd.
* Stephen Boyd
@@ -21,30 +21,56 @@
struct qcom_usb_extcon_info {
struct extcon_dev *edev;
- int irq;
+ int id_irq;
+ int vbus_irq;
struct delayed_work wq_detcable;
unsigned long debounce_jiffies;
};
static const unsigned int qcom_usb_extcon_cable[] = {
+ EXTCON_USB,
EXTCON_USB_HOST,
EXTCON_NONE,
};
static void qcom_usb_extcon_detect_cable(struct work_struct *work)
{
- bool id;
+ bool state = false;
int ret;
+ union extcon_property_value val;
struct qcom_usb_extcon_info *info = container_of(to_delayed_work(work),
struct qcom_usb_extcon_info,
wq_detcable);
- /* check ID and update cable state */
- ret = irq_get_irqchip_state(info->irq, IRQCHIP_STATE_LINE_LEVEL, );
- if (ret)
- return;
+ if (info->id_irq > 0) {
+ /* check ID and update cable state */
+ ret = irq_get_irqchip_state(info->id_irq,
+ IRQCHIP_STATE_LINE_LEVEL, );
+ if (ret)
+ return;
+
+ if (!state) {
+ val.intval = true;
+ extcon_set_property(info->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_SS, val);
+ }
+ extcon_set_state_sync(info->edev, EXTCON_USB_HOST, !state);
+ }
- extcon_set_state_sync(info->edev, EXTCON_USB_HOST, !id);
+ if (info->vbus_irq > 0) {
+ /* check VBUS and update cable state */
+ ret = irq_get_irqchip_state(info->vbus_irq,
+ IRQCHIP_STATE_LINE_LEVEL, );
+ if (ret)
+ return;
+
+ if (state) {
+ val.intval = true;
+ extcon_set_property(info->edev, EXTCON_USB,
+ EXTCON_PROP_USB_SS, val);
+ }
+ extcon_set_state_sync(info->edev, EXTCON_USB, state);
+ }
}
static irqreturn_t qcom_usb_irq_handler(int irq, void *dev_id)
@@ -79,21 +105,48 @@ static int qcom_usb_extcon_probe(struct platform_device
*pdev)
return ret;
}
+ ret = extcon_set_property_capability(info->edev,
+ EXTCON_USB, EXTCON_PROP_USB_SS);
+ ret |= extcon_set_property_capability(info->edev,
+ EXTCON_USB_HOST, EXTCON_PROP_USB_SS);
+ if (ret) {
+ dev_err(dev, "failed to register extcon props rc=%d\n",
+ ret);
+ return ret;
+ }
+
info->debounce_jiffies = msecs_to_jiffies(USB_ID_DEBOUNCE_MS);
INIT_DELAYED_WORK(>wq_detcable, qcom_usb_extcon_detect_cable);
- info->irq = platform_get_irq_byname(pdev, "usb_id");
- if (info->irq < 0)
- return info->irq;
+ info->id_irq = platform_get_irq_byname(pdev, "usb_id");
+ if (info->id_irq > 0) {
+ ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
+ qcom_usb_irq_handler,
+ IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ pdev->name, info);
+ if (ret < 0) {
+ dev_err(dev, "failed to request handler for ID IRQ\n");
+ return ret;
+ }
+ }
- ret = devm_request_threaded_irq(dev, info->irq, NULL,
+ info->vbus_irq = platform_get_irq_byname(pd
Re: [RFC][PATCH 1/3] PM /devfreq: add user frequency limits into devfreq struct
Hi Lukasz,
On Fri, Feb 12, 2021 at 7:28 AM Lukasz Luba wrote:
>
>
>
> On 2/11/21 11:07 AM, Lukasz Luba wrote:
> > Hi Chanwoo,
> >
> > On 2/3/21 10:21 AM, Lukasz Luba wrote:
> >> Hi Chanwoo,
> >>
> >> Thank you for looking at this.
> >>
> >> On 2/3/21 10:11 AM, Chanwoo Choi wrote:
> >>> Hi Lukasz,
> >>>
> >>> When accessing the max_freq and min_freq at devfreq-cooling.c,
> >>> even if can access 'user_max_freq' and 'lock' by using the 'devfreq'
> >>> instance,
> >>> I think that the direct access of variables
> >>> (lock/user_max_freq/user_min_freq)
> >>> of struct devfreq are not good.
> >>>
> >>> Instead, how about using the 'DEVFREQ_TRANSITION_NOTIFIER'
> >>> notification with following changes of 'struct devfreq_freq'?
> >>
> >> I like the idea with devfreq notification. I will have to go through the
> >> code to check that possibility.
> >>
> >>> Also, need to add codes into devfreq_set_target() for initializing
> >>> 'new_max_freq' and 'new_min_freq' before sending the DEVFREQ_POSTCHANGE
> >>> notification.
> >>>
> >>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> >>> index 147a229056d2..d5726592d362 100644
> >>> --- a/include/linux/devfreq.h
> >>> +++ b/include/linux/devfreq.h
> >>> @@ -207,6 +207,8 @@ struct devfreq {
> >>> struct devfreq_freqs {
> >>> unsigned long old;
> >>> unsigned long new;
> >>> + unsigned long new_max_freq;
> >>> + unsigned long new_min_freq;
> >>> };
> >>>
> >>>
> >>> And I think that new 'user_min_freq'/'user_max_freq' are not necessary.
> >>> You can get the current max_freq/min_freq by using the following steps:
> >>>
> >>> get_freq_range(devfreq, _freq, _freq);
> >>> dev_pm_opp_find_freq_floor(pdev, _freq);
> >>> dev_pm_opp_find_freq_floor(pdev, _freq);
> >>>
> >>> So that you can get the 'max_freq/min_freq' and then
> >>> initialize the 'freqs.new_max_freq and freqs.new_min_freq'
> >>> with them as following:
> >>>
> >>> in devfreq_set_target()
> >>> get_freq_range(devfreq, _freq, _freq);
> >>> dev_pm_opp_find_freq_floor(pdev, _freq);
> >>> dev_pm_opp_find_freq_floor(pdev, _freq);
> >>> freqs.new_max_freq = min_freq;
> >>> freqs.new_max_freq = max_freq;
> >>> devfreq_notify_transition(devfreq, , DEVFREQ_POSTCHANGE);
> >>
> >> I will plumb it in and check that option. My concern is that function
> >> get_freq_range() would give me the max_freq value from PM QoS, which
> >> might be my thermal limit - lower that user_max_freq. Then I still
> >> need
> >>
> >> I've been playing with PM QoS notifications because I thought it would
> >> be possible to be notified in thermal for all new set values - even from
> >> devfreq sysfs user max_freq write, which has value higher that the
> >> current limit set by thermal governor. Unfortunately PM QoS doesn't
> >> send that information by design. PM QoS also by desing won't allow
> >> me to check first two limits in the plist - which would be thermal
> >> and user sysfs max_freq.
> >>
> >> I will experiment with this notifications and share the results.
> >> That you for your comments.
> >
> > I have experimented with your proposal. Unfortunately, the value stored
> > in the pm_qos which is read by get_freq_range() is not the user max
> > freq. It's the value from thermal devfreq cooling when that one is
> > lower. Which is OK in the overall design, but not for my IPA use case.
> >
> > What comes to my mind is two options:
> > 1) this patch proposal, with simple solution of two new variables
> > protected by mutex, which would maintain user stored values
> > 2) add a new notification chain in devfreq to notify about new
> > user written value, to which devfreq cooling would register; that
> > would allow devfreq cooling to get that value instantly and store
> > locally
>
> 3) How about new define for existing notification chain:
> #define DEVFREQ_USER_CHANGE(2)
I think that if we add the notification with specific actor like user change
or OPP change or others, it is not proper. But, we can add the notification
for min or max frequenc
[PATCH 3/3] extcon: sm5502: Detect OTG when USB_ID is connected to ground
From: Nikita Travkin
In it's curent state this driver ignores OTG adapters with ID pin
connected to ground. This commit adds a check to set extcon into
host mode when such OTG adapter is connected.
Signed-off-by: Nikita Travkin
Signed-off-by: Chanwoo Choi
---
drivers/extcon/extcon-sm5502.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
index 106d4da647bd..db41d1c58efd 100644
--- a/drivers/extcon/extcon-sm5502.c
+++ b/drivers/extcon/extcon-sm5502.c
@@ -144,6 +144,7 @@ enum sm5502_muic_acc_type {
SM5502_MUIC_ADC_AUDIO_TYPE1_FULL_REMOTE = 0x3e, /* | 001|0| */
SM5502_MUIC_ADC_AUDIO_TYPE1_SEND_END = 0x5e,/* | 010|0| */
/* |Dev Type1|--ADC| */
+ SM5502_MUIC_ADC_GROUND_USB_OTG = 0x80, /* | 100|0| */
SM5502_MUIC_ADC_OPEN_USB = 0x5f,/* | 010|1| */
SM5502_MUIC_ADC_OPEN_TA = 0xdf, /* | 110|1| */
SM5502_MUIC_ADC_OPEN_USB_OTG = 0xff,/* | 111|1| */
@@ -291,11 +292,27 @@ static unsigned int sm5502_muic_get_cable_type(struct
sm5502_muic_info *info)
* connected with to MUIC device.
*/
cable_type = adc & SM5502_REG_ADC_MASK;
- if (cable_type == SM5502_MUIC_ADC_GROUND)
- return SM5502_MUIC_ADC_GROUND;
switch (cable_type) {
case SM5502_MUIC_ADC_GROUND:
+ ret = regmap_read(info->regmap, SM5502_REG_DEV_TYPE1,
+ _type1);
+ if (ret) {
+ dev_err(info->dev, "failed to read DEV_TYPE1 reg\n");
+ return ret;
+ }
+
+ switch (dev_type1) {
+ case SM5502_REG_DEV_TYPE1_USB_OTG_MASK:
+ cable_type = SM5502_MUIC_ADC_GROUND_USB_OTG;
+ break;
+ default:
+ dev_dbg(info->dev,
+ "cannot identify the cable type: adc(0x%x),
dev_type1(0x%x)\n",
+ adc, dev_type1);
+ return -EINVAL;
+ }
+ break;
case SM5502_MUIC_ADC_SEND_END_BUTTON:
case SM5502_MUIC_ADC_REMOTE_S1_BUTTON:
case SM5502_MUIC_ADC_REMOTE_S2_BUTTON:
@@ -396,6 +413,7 @@ static int sm5502_muic_cable_handler(struct
sm5502_muic_info *info,
con_sw = DM_DP_SWITCH_OPEN;
vbus_sw = VBUSIN_SWITCH_VBUSOUT;
break;
+ case SM5502_MUIC_ADC_GROUND_USB_OTG:
case SM5502_MUIC_ADC_OPEN_USB_OTG:
id = EXTCON_USB_HOST;
con_sw = DM_DP_SWITCH_USB;
--
2.17.1
[PATCH 1/3] extcon: Add stubs for extcon_register_notifier_all() functions
From: Krzysztof Kozlowski
Add stubs for extcon_register_notifier_all() function for !CONFIG_EXTCON
case. This is useful for compile testing and for drivers which use
EXTCON but do not require it (therefore do not depend on CONFIG_EXTCON).
Fixes: 815429b39d94 ("extcon: Add new extcon_register_notifier_all() to monitor
all external connectors")
Reported-by: kernel test robot
Signed-off-by: Krzysztof Kozlowski
Signed-off-by: Chanwoo Choi
---
include/linux/extcon.h | 23 +++
1 file changed, 23 insertions(+)
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index fd183fb9c20f..0c19010da77f 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -271,6 +271,29 @@ static inline void devm_extcon_unregister_notifier(struct
device *dev,
struct extcon_dev *edev, unsigned int id,
struct notifier_block *nb) { }
+static inline int extcon_register_notifier_all(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int extcon_unregister_notifier_all(struct extcon_dev *edev,
+struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int devm_extcon_register_notifier_all(struct device *dev,
+ struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline void devm_extcon_unregister_notifier_all(struct device *dev,
+ struct extcon_dev *edev,
+ struct notifier_block
*nb) { }
+
static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
{
return ERR_PTR(-ENODEV);
--
2.17.1
[PATCH 0/3] extcon: Add missing function defintion and update extcon driver
Instead of pull request for extcon next of v5.12, send the patches because the pull request was too late for merging them. Detailed description of this patchset as following: - Add missing function definitions for extcon_register_notifier() - Add charging interrupt handling such as CHGINS and CHGRM interrupt for extcon-max8997.c - Detect OTG when USB_ID is connected to ground for extcon-sm5502.c Reference of previous pull request, [1] [GIT PULL ,v3] extcon next for v5.12 - https://lore.kernel.org/patchwork/patch/1380533/ [2] [GIT PULL v2] extcon next for v5.12 - https://lore.kernel.org/patchwork/patch/1379836/ [3] [GIT PULL] extcon next for v5.12 - https://lore.kernel.org/patchwork/patch/1378261/ Krzysztof Kozlowski (1): extcon: Add stubs for extcon_register_notifier_all() functions Nikita Travkin (1): extcon: sm5502: Detect OTG when USB_ID is connected to ground Timon Baetz (1): extcon: max8997: Add CHGINS and CHGRM interrupt handling drivers/extcon/extcon-max8997.c | 4 drivers/extcon/extcon-sm5502.c | 22 -- include/linux/extcon.h | 23 +++ 3 files changed, 47 insertions(+), 2 deletions(-) -- 2.17.1
[PATCH 2/3] extcon: max8997: Add CHGINS and CHGRM interrupt handling
From: Timon Baetz
This allows the MAX8997 charger to set the current limit depending on
the detected extcon charger type.
Signed-off-by: Timon Baetz
Signed-off-by: Chanwoo Choi
---
drivers/extcon/extcon-max8997.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 337b0eea4e62..e1408075ef7d 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -44,6 +44,8 @@ static struct max8997_muic_irq muic_irqs[] = {
{ MAX8997_MUICIRQ_ChgDetRun,"muic-CHGDETRUN" },
{ MAX8997_MUICIRQ_ChgTyp, "muic-CHGTYP" },
{ MAX8997_MUICIRQ_OVP, "muic-OVP" },
+ { MAX8997_PMICIRQ_CHGINS, "pmic-CHGINS" },
+ { MAX8997_PMICIRQ_CHGRM,"pmic-CHGRM" },
};
/* Define supported cable type */
@@ -538,6 +540,8 @@ static void max8997_muic_irq_work(struct work_struct *work)
case MAX8997_MUICIRQ_DCDTmr:
case MAX8997_MUICIRQ_ChgDetRun:
case MAX8997_MUICIRQ_ChgTyp:
+ case MAX8997_PMICIRQ_CHGINS:
+ case MAX8997_PMICIRQ_CHGRM:
/* Handle charger cable */
ret = max8997_muic_chg_handler(info);
break;
--
2.17.1
Re: [GIT PULL v3] extcon next for v5.12
Dear Greg, On 2/15/21 6:26 PM, Greg KH wrote: > On Mon, Feb 15, 2021 at 06:35:16PM +0900, Chanwoo Choi wrote: >> Dear Greg, >> >> This is extcon-next pull request for v5.12. I add detailed description of >> this pull request on below. Please pull extcon with following updates. > > As 5.11 is now out, I can't take new things in my tree until 5.12-rc1 is > out, sorry. > > Please break this up into bugfixes and new features and send them to me > for 5.12-rc1. > > Or just send them to me now as patches in emails and I will queue them > up to my trees properly when 5.12-rc1 it out. That might be the easiest > thing for you to do now. I'm sorry for late reply. I'll send the patchset right now instead of pull request. Thanks for guide. -- Best Regards, Chanwoo Choi Samsung Electronics
[GIT PULL v3] extcon next for v5.12
Dear Greg, This is extcon-next pull request for v5.12. I add detailed description of this pull request on below. Please pull extcon with following updates. Changes from v2: - Add missing the diff statistics information Changes from v1: - Add missing committer information Best Regards, Chanwoo Choi The following changes since commit 6ee1d745b7c9fd573fba142a2efdad76a9f1cb04: Linux 5.11-rc5 (2021-01-24 16:47:14 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git tags/extcon-next-for-5.12-v2 for you to fetch changes up to bd30a35764e136dc372e74c4856de633cb7ed8de: extcon: sm5502: Detect OTG when USB_ID is connected to ground (2021-02-10 08:15:28 +0900) Krzysztof Kozlowski (1): extcon: Add stubs for extcon_register_notifier_all() functions Nikita Travkin (1): extcon: sm5502: Detect OTG when USB_ID is connected to ground Timon Baetz (1): extcon: max8997: Add CHGINS and CHGRM interrupt handling drivers/extcon/extcon-max8997.c | 4 drivers/extcon/extcon-sm5502.c | 22 -- include/linux/extcon.h | 23 +++ 3 files changed, 47 insertions(+), 2 deletions(-)
Re: [GIT PULL v2] extcon next for v5.12
Dear Greg, On 2/12/21 4:50 PM, Greg KH wrote: > On Fri, Feb 12, 2021 at 02:19:37PM +0900, Chanwoo Choi wrote: >> Dear Greg, >> >> This is extcon-next pull request for v5.12. I add detailed description of >> this pull request on below. Please pull extcon with following updates. >> >> Changes from v1: >> - Add missing committer information >> >> Best Regards, >> Chanwoo Choi >> >> >> The following changes since commit 6ee1d745b7c9fd573fba142a2efdad76a9f1cb04: >> >> Linux 5.11-rc5 (2021-01-24 16:47:14 -0800) >> >> are available in the git repository at: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git >> tags/extcon-next-for-5.12-v2 >> >> for you to fetch changes up to bd30a35764e136dc372e74c4856de633cb7ed8de: >> >> extcon: sm5502: Detect OTG when USB_ID is connected to ground >> (2021-02-10 08:15:28 +0900) >> >> >> Krzysztof Kozlowski (1): >> extcon: Add stubs for extcon_register_notifier_all() functions >> >> Nikita Travkin (1): >> extcon: sm5502: Detect OTG when USB_ID is connected to ground >> >> Timon Baetz (1): >> extcon: max8997: Add CHGINS and CHGRM interrupt handling > > No diffstat??? How did you generate this pull request? When I copied and pasted the body of pull request, I was not adding the diff statistics. I'm sorry about my mistake again. -- Best Regards, Chanwoo Choi Samsung Electronics
[GIT PULL v2] extcon next for v5.12
Dear Greg, This is extcon-next pull request for v5.12. I add detailed description of this pull request on below. Please pull extcon with following updates. Changes from v1: - Add missing committer information Best Regards, Chanwoo Choi The following changes since commit 6ee1d745b7c9fd573fba142a2efdad76a9f1cb04: Linux 5.11-rc5 (2021-01-24 16:47:14 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git tags/extcon-next-for-5.12-v2 for you to fetch changes up to bd30a35764e136dc372e74c4856de633cb7ed8de: extcon: sm5502: Detect OTG when USB_ID is connected to ground (2021-02-10 08:15:28 +0900) Krzysztof Kozlowski (1): extcon: Add stubs for extcon_register_notifier_all() functions Nikita Travkin (1): extcon: sm5502: Detect OTG when USB_ID is connected to ground Timon Baetz (1): extcon: max8997: Add CHGINS and CHGRM interrupt handling
Re: [GIT PULL] extcon next for v5.12
Dear Greg,
On Tue, Feb 9, 2021 at 7:43 PM Greg KH wrote:
>
> On Tue, Feb 09, 2021 at 07:49:59PM +0900, Chanwoo Choi wrote:
> > Dear Greg,
> >
> > This is extcon-next pull request for v5.12. I add detailed description of
> > this pull request on below. Please pull extcon with following updates.
> >
> > Best Regards,
> > Chanwoo Choi
>
> I see the following error in this repo when trying to pull it:
>
> Commit d8cc19be483a ("extcon: sm5502: Detect OTG when USB_ID is connected to
> ground")
> committer Signed-off-by missing
> author email:nikitos...@gmail.com
> committer email: cw00.c...@samsung.com
> Signed-off-by: Nikita Travkin
>
> Please fix up.
It's my fault. I'm sorry. I'll resend the pull request.
Best Regards,
Chanwoo Choi
[GIT PULL] extcon next for v5.12
Dear Greg, This is extcon-next pull request for v5.12. I add detailed description of this pull request on below. Please pull extcon with following updates. Best Regards, Chanwoo Choi The following changes since commit 6ee1d745b7c9fd573fba142a2efdad76a9f1cb04: Linux 5.11-rc5 (2021-01-24 16:47:14 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git tags/extcon-next-for-5.12 for you to fetch changes up to d8cc19be483ab2d74a1dd372d50ff9ac6cf37fa7: extcon: sm5502: Detect OTG when USB_ID is connected to ground (2021-02-09 19:42:06 +0900) Krzysztof Kozlowski (1): extcon: Add stubs for extcon_register_notifier_all() functions Nikita Travkin (1): extcon: sm5502: Detect OTG when USB_ID is connected to ground Timon Baetz (1): extcon: max8997: Add CHGINS and CHGRM interrupt handling drivers/extcon/extcon-max8997.c | 4 drivers/extcon/extcon-sm5502.c | 22 -- include/linux/extcon.h | 23 +++ 3 files changed, 47 insertions(+), 2 deletions(-)
Re: [PATCH v4 resend 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support
On 2/4/21 8:24 PM, Hans de Goede wrote: > Hi all, > > Here is v4 of my series to rework the arizona codec jack-detect support > to use the snd_soc_jack helpers instead of direct extcon reporting. > > This is a resend with some extra *-by tags collected and with the extcon > folks added to the "To:" list, which I somehow missed with the original > v4 posting, sorry. > > This is done by reworking the extcon driver into an arizona-jackdet > library and then modifying the codec drivers to use that directly, > replacing the old separate extcon child-devices and extcon-driver. > > This brings the arizona-codec jack-detect handling inline with how > all other ASoC codec driver do this. This was developed and tested on > a Lenovo Yoga Tablet 1051L with a WM5102 codec. > > This was also tested by Charles Keepax, one of the Cirrus Codec folks. > > This depends on the previously posted "[PATCH v4 0/5] MFD/ASoC: Add > support for Intel Bay Trail boards with WM5102 codec" series and there > are various interdependencies between the patches in this series. > > Lee Jones, the MFD maintainer has agreed to take this series upstream > through the MFD tree and to provide an immutable branch for the ASoC > and extcon subsystems to merge. > > Mark and extcon-maintainers may we have your ack for merging these > through the MFD tree ? About patch2~patch6, I agree to take these patches to MFD tree. Acke-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v5 0/3] Add required-opps support to devfreq passive gov
Hi Viresh, On 2/4/21 2:41 PM, Viresh Kumar wrote: > On 03-02-21, 17:23, Hsin-Yi Wang wrote: >> The devfreq passive governor scales the frequency of a "child" device based >> on the current frequency of a "parent" device (not parent/child in the >> sense of device hierarchy). As of today, the passive governor requires one >> of the following to work correctly: >> 1. The parent and child device have the same number of frequencies >> 2. The child device driver passes a mapping function to translate from >>parent frequency to child frequency. >> >> When (1) is not true, (2) is the only option right now. But often times, >> all that is required is a simple mapping from parent's frequency to child's >> frequency. >> >> Since OPPs already support pointing to other "required-opps", add support >> for using that to map from parent device frequency to child device >> frequency. That way, every child device driver doesn't have to implement a >> separate mapping function anytime (1) isn't true. > > So you guys aren't interested in dev_pm_opp_set_opp() but just the > translation of the required-OPPs ? > > I am fine with most of the stuff and I would like to take it via OPP > tree, hope that would be fine ? I agree. Take these patches to OPP tree. -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v6 3/3] PM / devfreq: Add required OPPs support to passive governor
Hi Hsin-Yi,
I have reviewed this patch already.
This version looks good to me.
I add my acked-by again.
Acked-by: Chanwoo Choi
Thanks,
Chanwoo Choi
On 2/4/21 5:14 PM, Hsin-Yi Wang wrote:
> From: Saravana Kannan
>
> Look at the required OPPs of the "parent" device to determine the OPP that
> is required from the slave device managed by the passive governor. This
> allows having mappings between a parent device and a slave device even when
> they don't have the same number of OPPs.
>
> Signed-off-by: Saravana Kannan
> Acked-by: MyungJoo Ham
> Acked-by: Chanwoo Choi
> Signed-off-by: Hsin-Yi Wang
> ---
> drivers/devfreq/governor_passive.c | 25 ++---
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/devfreq/governor_passive.c
> b/drivers/devfreq/governor_passive.c
> index 63332e4a65ae8..8fd51cc9b991a 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -19,7 +19,7 @@ static int devfreq_passive_get_target_freq(struct devfreq
> *devfreq,
> = (struct devfreq_passive_data *)devfreq->data;
> struct devfreq *parent_devfreq = (struct devfreq *)p_data->parent;
> unsigned long child_freq = ULONG_MAX;
> - struct dev_pm_opp *opp;
> + struct dev_pm_opp *opp, *p_opp = ERR_PTR(-ENODEV);
> int i, count, ret = 0;
>
> /*
> @@ -29,7 +29,7 @@ static int devfreq_passive_get_target_freq(struct devfreq
> *devfreq,
>*/
> if (p_data->get_target_freq) {
> ret = p_data->get_target_freq(devfreq, freq);
> - goto out;
> + return ret;
> }
>
> /*
> @@ -56,13 +56,22 @@ static int devfreq_passive_get_target_freq(struct devfreq
> *devfreq,
>* list of parent device. Because in this case, *freq is temporary
>* value which is decided by ondemand governor.
>*/
> - opp = devfreq_recommended_opp(parent_devfreq->dev.parent, freq, 0);
> - if (IS_ERR(opp)) {
> - ret = PTR_ERR(opp);
> - goto out;
> + p_opp = devfreq_recommended_opp(parent_devfreq->dev.parent, freq, 0);
> + if (IS_ERR(p_opp)) {
> + ret = PTR_ERR(p_opp);
> + return ret;
> }
>
> - dev_pm_opp_put(opp);
> + if (devfreq->opp_table && parent_devfreq->opp_table) {
> + opp = dev_pm_opp_xlate_required_opp(parent_devfreq->opp_table,
> + devfreq->opp_table, p_opp);
> + if (!IS_ERR(opp)) {
> + *freq = dev_pm_opp_get_freq(opp);
> + dev_pm_opp_put(opp);
> + } else
> + ret = PTR_ERR(opp);
> + goto out;
> + }
>
> /*
>* Get the OPP table's index of decided freqeuncy by governor
> @@ -89,6 +98,8 @@ static int devfreq_passive_get_target_freq(struct devfreq
> *devfreq,
> *freq = child_freq;
>
> out:
> + dev_pm_opp_put(p_opp);
> +
> return ret;
> }
>
>
Re: [PATCH v6 2/3] PM / devfreq: Cache OPP table reference in devfreq
Hi Hsin-Yi,
On 2/4/21 5:14 PM, Hsin-Yi Wang wrote:
> From: Saravana Kannan
>
> The OPP table can be used often in devfreq. Trying to get it each time can
> be expensive, so cache it in the devfreq struct.
>
> Signed-off-by: Saravana Kannan
> Reviewed-by: Chanwoo Choi
> Acked-by: MyungJoo Ham
> Signed-off-by: Hsin-Yi Wang
> ---
> drivers/devfreq/devfreq.c | 6 ++
> include/linux/devfreq.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 6aa10de792b33..a5899c9ae16fc 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -757,6 +757,8 @@ static void devfreq_dev_release(struct device *dev)
> if (devfreq->profile->exit)
> devfreq->profile->exit(devfreq->dev.parent);
>
> + if (devfreq->opp_table)
> + dev_pm_opp_put_opp_table(devfreq->opp_table);
> mutex_destroy(>lock);
> kfree(devfreq);
> }
> @@ -844,6 +846,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
> }
>
> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> + devfreq->opp_table = dev_pm_opp_get_opp_table(dev);
> + if (IS_ERR(devfreq->opp_table))
> + devfreq->opp_table = NULL;
> +
> atomic_set(>suspend_count, 0);
>
> dev_set_name(>dev, "%s", dev_name(dev));
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index b6d3bae1c74d8..26ea0850be9bb 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -137,6 +137,7 @@ struct devfreq_stats {
> * using devfreq.
> * @profile: device-specific devfreq profile
> * @governor:method how to choose frequency based on the usage.
> + * @opp_table: Reference to OPP table of dev.parent, if one exists.
> * @nb: notifier block used to notify devfreq object that it
> should
> * reevaluate operable frequencies. Devfreq users may use
> * devfreq.nb to the corresponding register notifier call chain.
> @@ -173,6 +174,7 @@ struct devfreq {
> struct device dev;
> struct devfreq_dev_profile *profile;
> const struct devfreq_governor *governor;
> + struct opp_table *opp_table;
> struct notifier_block nb;
> struct delayed_work work;
>
>
Acked-by: Chanwoo Choi
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH v5 0/3] Add required-opps support to devfreq passive gov
vana > > v4 -> v5: > - drop patch "OPP: Improve required-opps linking" and rebase to > > https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/log/?h=opp/linux-next > - Compare pointers in dev_pm_opp_xlate_required_opp(). > > v3 -> v4: > - Fixed documentation comments > - Fixed order of functions in .h file > - Renamed the new xlate API > - Caused _set_required_opps() to fail if all required opps tables aren't > linked. > v2 -> v3: > - Rebased onto linux-next. > - Added documentation comment for new fields. > - Added support for lazy required-opps linking. > - Updated Ack/Reviewed-bys. > v1 -> v2: > - Cached OPP table reference in devfreq to avoid looking up every time. > - Renamed variable in passive governor to be more intuitive. > - Updated cover letter with examples. > > Saravana Kannan (3): > OPP: Add function to look up required OPP's for a given OPP > PM / devfreq: Cache OPP table reference in devfreq > PM / devfreq: Add required OPPs support to passive governor > > drivers/devfreq/devfreq.c | 6 > drivers/devfreq/governor_passive.c | 20 --- > drivers/opp/core.c | 58 ++ > include/linux/devfreq.h| 2 ++ > include/linux/pm_opp.h | 11 ++ > 5 files changed, 92 insertions(+), 5 deletions(-) > -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [RFC][PATCH 1/3] PM /devfreq: add user frequency limits into devfreq struct
d_freq_floor(pdev, );
> + if (IS_ERR(opp))
> + return count;
> +
> + dev_pm_opp_put(opp);
> +
> + mutex_lock(>lock);
> + df->user_max_freq = value;
> + mutex_unlock(>lock);
> +
> return count;
> }
>
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index b6d3bae1c74d..147a229056d2 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -147,6 +147,8 @@ struct devfreq_stats {
> * touch this.
> * @user_min_freq_req: PM QoS minimum frequency request from user (via
> sysfs)
> * @user_max_freq_req: PM QoS maximum frequency request from user (via
> sysfs)
> + * @user_min_freq: User's minimum frequency
> + * @user_max_freq: User's maximum frequency
> * @scaling_min_freq:Limit minimum frequency requested by OPP
> interface
> * @scaling_max_freq:Limit maximum frequency requested by OPP
> interface
> * @stop_polling: devfreq polling status of a device.
> @@ -185,6 +187,8 @@ struct devfreq {
> struct dev_pm_qos_request user_max_freq_req;
> unsigned long scaling_min_freq;
> unsigned long scaling_max_freq;
> + unsigned long user_min_freq;
> + unsigned long user_max_freq;
> bool stop_polling;
>
> unsigned long suspend_freq;
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [RFC][PATCH 0/3] New thermal interface allowing IPA to get max power
Hi Lukasz, I'll review this patchset until tomorrow. Thanks. Chanwoo Choi On 2/1/21 8:23 PM, Lukasz Luba wrote: > Daniel, Chanwoo > > Gentle ping. Have you have a chance to check these patches? > > On 1/26/21 10:39 AM, Lukasz Luba wrote: >> Hi all, >> >> This patch set tries to add the missing feature in the Intelligent Power >> Allocation (IPA) governor which is: frequency limit set by user space. >> User can set max allowed frequency for a given device which has impact on >> max allowed power. In current design there is no mechanism to figure this >> out. IPA must know the maximum allowed power for every device. It is then >> used for proper power split and divvy-up. When the user limit for max >> frequency is not know, IPA assumes it is the highest possible frequency. >> It causes wrong power split across the devices. >> >> This new mechanism provides the max allowed frequency to the thermal >> framework and then max allowed power to the IPA. >> The implementation is done in this way because currently there is no way >> to retrieve the limits from the PM QoS, without uncapping the local >> thermal limit and reading the next value. It would be a heavy way of >> doing these things, since it should be done every polling time (e.g. 50ms). >> Also, the value stored in PM QoS can be different than the real OPP 'rate' >> so still would need conversion into proper OPP for comparison with EM. >> Furthermore, uncapping the device in thermal just to check the user freq >> limit is not the safest way. >> Thus, this simple implementation moves the calculation of the proper >> frequency to the sysfs write code, since it's called less often. The value >> is then used as-is in the thermal framework without any hassle. >> >> As it's a RFC, it still misses the cpufreq sysfs implementation, but would >> be addressed if all agree. >> >> Regards, >> Lukasz Luba >> >> Lukasz Luba (3): >> PM /devfreq: add user frequency limits into devfreq struct >> thermal: devfreq_cooling: add new callback to get user limit for min >> state >> thermal: power_allocator: get proper max power limited by user >> >> drivers/devfreq/devfreq.c | 41 --- >> drivers/thermal/devfreq_cooling.c | 33 +++++ >> drivers/thermal/gov_power_allocator.c | 17 +-- >> include/linux/devfreq.h | 4 +++ >> include/linux/thermal.h | 1 + >> 5 files changed, 90 insertions(+), 6 deletions(-) >> > > -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH] PM / devfreq: remove unneeded semicolon
On 2/2/21 3:55 PM, Yang Li wrote: > Eliminate the following coccicheck warning: > ./drivers/devfreq/rk3399_dmc.c:403:2-3: Unneeded semicolon > > Reported-by: Abaci Robot > Signed-off-by: Yang Li > --- > drivers/devfreq/rk3399_dmc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c > index 2e91216..9e9d3b4c 100644 > --- a/drivers/devfreq/rk3399_dmc.c > +++ b/drivers/devfreq/rk3399_dmc.c > @@ -400,7 +400,7 @@ static int rk3399_dmcfreq_probe(struct platform_device > *pdev) > default: > ret = -EINVAL; > goto err_edev; > - }; > + } > > no_pmu: > arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0, > Applied it. I changed the patch title as following: - PM / devfreq: rk3399_dmc: Remove unneeded semicolon -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled
On 2/1/21 2:04 AM, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), fix this by calling
> clk_prepare_enable() directly from clock provider driver.
>
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
>
> Signed-off-by: Paweł Chmiel
> ---
> Changes from v2:
> - Avoid __clk_lookup() call when enabling clock
> Changes from v1:
> - Instead of marking clock as critical, enable it manually in driver.
> ---
> drivers/clk/samsung/clk-exynos7.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos7.c
> b/drivers/clk/samsung/clk-exynos7.c
> index c1ff715e960c..24d3fc5c5d0f 100644
> --- a/drivers/clk/samsung/clk-exynos7.c
> +++ b/drivers/clk/samsung/clk-exynos7.c
> @@ -6,6 +6,7 @@
>
> #include
> #include
> +#include
>
> #include "clk.h"
> #include
> @@ -570,7 +571,18 @@ static const struct samsung_cmu_info top1_cmu_info
> __initconst = {
>
> static void __init exynos7_clk_top1_init(struct device_node *np)
> {
> - samsung_cmu_register_one(np, _cmu_info);
> + struct samsung_clk_provider *ctx;
> + struct clk_hw **hws;
> +
> + ctx = samsung_cmu_register_one(np, _cmu_info);
> + if (!ctx)
> + return;
> + hws = ctx->clk_data.hws;
> +
> + /*
> + * Keep top FSYS1 aclk enabled permanently. It's required for CMU
> register access.
> + */
> + clk_prepare_enable(hws[CLK_ACLK_FSYS1_200]->clk);
> }
>
> CLK_OF_DECLARE(exynos7_clk_top1, "samsung,exynos7-clock-top1",
>
Acked-by: Chanwoo Choi
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH V2 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
On 1/27/21 6:10 PM, Viresh Kumar wrote:
> dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
> be used instead. Migrate to the new API.
>
> We don't want the OPP core to manage the clk for this driver, migrate to
> dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp()
> doesn't have any side effects.
>
> Signed-off-by: Viresh Kumar
> ---
> Dmitry,
>
> This is based over the patches sent here:
>
> https://protect2.fireeye.com/v1/url?k=72d88562-2d43bc78-72d90e2d-000babff24ad-e4764030101eaedc=1=a25b5db7-346b-47b2-9c0a-3945e579f389=https%3A%2F%2Flore.kernel.org%2Flkml%2F6c2160ff30a8f421563793020264cf9f533f293c.1611738228.git.viresh.kumar%40linaro.org%2F
>
> This should fix the problem you mentioned earlier. Will push this for
> linux-next unless you have any issues with it.
>
> drivers/devfreq/tegra30-devfreq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c
> b/drivers/devfreq/tegra30-devfreq.c
> index 117cad7968ab..31f7dec5990b 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev,
> unsigned long *freq,
> return PTR_ERR(opp);
> }
>
> - ret = dev_pm_opp_set_bw(dev, opp);
> + ret = dev_pm_opp_set_opp(dev, opp);
> dev_pm_opp_put(opp);
>
> return ret;
> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device
> *pdev)
> return err;
> }
>
> - err = dev_pm_opp_of_add_table(>dev);
> + err = dev_pm_opp_of_add_table_noclk(>dev);
> if (err) {
> dev_err(>dev, "Failed to add OPP table: %d\n", err);
> goto put_hw;
>
Acked-by: Chanwoo Choi
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH] PM / devfreq: Add sysfs attributes to simple_ondemand governor
freq_simple_ondemand_sysfs_setup(struct devfreq *df)
> +{
> + struct devfreq_simple_ondemand_data *data;
> + int ret;
> +
> + if (!df->data) {
> + /* The memory will be freed automatically */
> + df->data = devm_kzalloc(>dev,
> + sizeof(struct devfreq_simple_ondemand_data),
> + GFP_KERNEL);
> + if (!df->data) {
> + dev_warn(>dev, "Unable to allocate memory");
> + return;
> + }
> + }
> +
> + data = df->data;
> +
> + /* After new allocation setup default values, since they are used */
> + if (!data->upthreshold)
> + data->upthreshold = DFSO_UPTHRESHOLD;
> +
> + if (!data->downdifferential)
> + data->downdifferential = DFSO_DOWNDIFFERENCTIAL;
> +
> + ret = sysfs_create_file(>dev.kobj, _attr_upthreshold.attr);
> + if (ret < 0)
> + dev_warn(>dev, "Unable to create 'upthreshold' attr\n");
> +
> + ret = sysfs_create_file(>dev.kobj, _attr_downdifferential.attr);
> + if (ret < 0)
> + dev_warn(>dev, "Unable to create 'downdifferential'
> attr\n");
> +}
> +
> +static void devfreq_simple_ondemand_sysfs_remove(struct devfreq *df)
> +{
> + sysfs_remove_file(>dev.kobj, _attr_upthreshold.attr);
> + sysfs_remove_file(>dev.kobj, _attr_downdifferential.attr);
> +}
> +
> static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
> unsigned int event, void *data)
> {
> switch (event) {
> case DEVFREQ_GOV_START:
> devfreq_monitor_start(devfreq);
> + devfreq_simple_ondemand_sysfs_setup(devfreq);
> break;
>
> case DEVFREQ_GOV_STOP:
> + devfreq_simple_ondemand_sysfs_remove(devfreq);
> devfreq_monitor_stop(devfreq);
> break;
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH] dt-bindings: Add missing array size constraints
Hi Rob, On Tue, Jan 5, 2021 at 8:03 AM Rob Herring wrote: > > DT properties which can have multiple entries need to specify what the > entries are and define how many entries there can be. In the case of > only a single entry, just 'maxItems: 1' is sufficient. > > Add the missing entry constraints. These were found with a modified > meta-schema. Unfortunately, there are a few cases where the size > constraints are not defined such as common bindings, so the meta-schema > can't be part of the normal checks. > > Cc: Jens Axboe > Cc: Stephen Boyd > Cc: Thierry Reding > Cc: MyungJoo Ham > Cc: Chanwoo Choi > Cc: Linus Walleij > Cc: Bartosz Golaszewski > Cc: Jonathan Cameron > Cc: Dmitry Torokhov > Cc: Thomas Gleixner > Cc: Marc Zyngier > Cc: Mauro Carvalho Chehab > Cc: Chen-Yu Tsai > Cc: Ulf Hansson > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: Sebastian Reichel > Cc: Ohad Ben-Cohen > Cc: Bjorn Andersson > Cc: Greg Kroah-Hartman > Signed-off-by: Rob Herring > --- > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: dri-de...@lists.freedesktop.org > Cc: linux-g...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux-in...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: net...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: linux-remotep...@vger.kernel.org > Cc: linux-ri...@lists.infradead.org > Cc: linux-ser...@vger.kernel.org > Cc: alsa-de...@alsa-project.org > Cc: linux-...@vger.kernel.org > Cc: linux-...@vger.kernel.org > --- > .../socionext,uniphier-system-cache.yaml | 4 ++-- > .../bindings/ata/sata_highbank.yaml | 1 + > .../bindings/clock/canaan,k210-clk.yaml | 1 + > .../bindings/display/brcm,bcm2711-hdmi.yaml | 1 + > .../bindings/display/brcm,bcm2835-hdmi.yaml | 1 + > .../display/panel/jdi,lt070me05000.yaml | 1 + > .../display/panel/mantix,mlaf057we51-x.yaml | 3 ++- > .../display/panel/novatek,nt36672a.yaml | 1 + > .../devicetree/bindings/dsp/fsl,dsp.yaml | 2 +- > .../devicetree/bindings/eeprom/at25.yaml | 3 +-- > .../bindings/extcon/extcon-ptn5150.yaml | 2 ++ For extcon part, Acked-by: Chanwoo Choi (snip) Best Regards, Chanwoo Choi
Re: [PATCH 28/31] PM / devfreq: imx8m-ddrc: convert to use devm_pm_opp_* API
Hi Yangtao,
On Tue, Jan 5, 2021 at 1:13 PM Chanwoo Choi wrote:
>
> On Sun, Jan 3, 2021 at 12:58 PM Yangtao Li wrote:
> >
> > Use devm_pm_opp_* API to simplify code.
> >
> > Signed-off-by: Yangtao Li
> > ---
> > drivers/devfreq/imx8m-ddrc.c | 15 ++-
> > 1 file changed, 2 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
> > index bc82d3653bff..9383d6e5538b 100644
> > --- a/drivers/devfreq/imx8m-ddrc.c
> > +++ b/drivers/devfreq/imx8m-ddrc.c
> > @@ -370,11 +370,6 @@ static int imx8m_ddrc_check_opps(struct device *dev)
> > return 0;
> > }
> >
> > -static void imx8m_ddrc_exit(struct device *dev)
> > -{
> > - dev_pm_opp_of_remove_table(dev);
> > -}
> > -
> > static int imx8m_ddrc_probe(struct platform_device *pdev)
> > {
> > struct device *dev = >dev;
> > @@ -419,7 +414,7 @@ static int imx8m_ddrc_probe(struct platform_device
> > *pdev)
> > return ret;
> > }
> >
> > - ret = dev_pm_opp_of_add_table(dev);
> > + ret = devm_pm_opp_of_add_table(dev);
> > if (ret < 0) {
> > dev_err(dev, "failed to get OPP table\n");
> > return ret;
> > @@ -427,12 +422,11 @@ static int imx8m_ddrc_probe(struct platform_device
> > *pdev)
> >
> > ret = imx8m_ddrc_check_opps(dev);
> > if (ret < 0)
> > - goto err;
> > + return ret;
> >
> > priv->profile.polling_ms = 1000;
> > priv->profile.target = imx8m_ddrc_target;
> > priv->profile.get_dev_status = imx8m_ddrc_get_dev_status;
> > - priv->profile.exit = imx8m_ddrc_exit;
> > priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
> > priv->profile.initial_freq = clk_get_rate(priv->dram_core);
> >
> > @@ -441,13 +435,8 @@ static int imx8m_ddrc_probe(struct platform_device
> > *pdev)
> > if (IS_ERR(priv->devfreq)) {
> > ret = PTR_ERR(priv->devfreq);
> > dev_err(dev, "failed to add devfreq device: %d\n", ret);
> > - goto err;
> > }
> >
> > - return 0;
> > -
> > -err:
> > - dev_pm_opp_of_remove_table(dev);
> > return ret;
>
> devm_devfreq_add_device() doesn't return any integer value.
> Even if devm_devfreq_add_device() returns the right devfreq instance,
> the 'ret' value is not the return value of devm_devfreq_add_device().
>
> On this patch, 'ret' value of 'return ret' is from imx8m_ddrc_check_opps().
> Surely, it is well working with this modification. But, it is not code
> for exception handling.
> So, we need to remain the following codes:
>
> return 0;
> err:
> return ret;
>
'err' is not necessary. You better to edit it as following:
if (IS_ERR(priv->devfreq)) {
dev_err(dev, "failed to add devfreq device: %d\n", ret);
return PTR_ERR(priv->devfreq);
}
return 0;
--
Best Regards,
Chanwoo Choi
Re: [PATCH 31/31] PM / devfreq: convert to devm_pm_opp_register_notifier and remove unused API
On Sun, Jan 3, 2021 at 12:59 PM Yangtao Li wrote:
>
> Use devm_pm_opp_* API to simplify code.
>
> Signed-off-by: Yangtao Li
> ---
> drivers/devfreq/devfreq.c | 66 +--
> include/linux/devfreq.h | 23 --
> 2 files changed, 1 insertion(+), 88 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 6aa10de792b3..f593f30529ec 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -2004,40 +2004,6 @@ struct dev_pm_opp *devfreq_recommended_opp(struct
> device *dev,
> }
> EXPORT_SYMBOL(devfreq_recommended_opp);
>
> -/**
> - * devfreq_register_opp_notifier() - Helper function to get devfreq notified
> - * for any changes in the OPP availability
> - * changes
> - * @dev: The devfreq user device. (parent of devfreq)
> - * @devfreq: The devfreq object.
> - */
> -int devfreq_register_opp_notifier(struct device *dev, struct devfreq
> *devfreq)
> -{
> - return dev_pm_opp_register_notifier(dev, >nb);
> -}
> -EXPORT_SYMBOL(devfreq_register_opp_notifier);
> -
> -/**
> - * devfreq_unregister_opp_notifier() - Helper function to stop getting
> devfreq
> - *notified for any changes in the OPP
> - *availability changes anymore.
> - * @dev: The devfreq user device. (parent of devfreq)
> - * @devfreq: The devfreq object.
> - *
> - * At exit() callback of devfreq_dev_profile, this must be included if
> - * devfreq_recommended_opp is used.
> - */
> -int devfreq_unregister_opp_notifier(struct device *dev, struct devfreq
> *devfreq)
> -{
> - return dev_pm_opp_unregister_notifier(dev, >nb);
> -}
> -EXPORT_SYMBOL(devfreq_unregister_opp_notifier);
> -
> -static void devm_devfreq_opp_release(struct device *dev, void *res)
> -{
> - devfreq_unregister_opp_notifier(dev, *(struct devfreq **)res);
> -}
> -
> /**
> * devm_devfreq_register_opp_notifier() - Resource-managed
> * devfreq_register_opp_notifier()
> @@ -2047,40 +2013,10 @@ static void devm_devfreq_opp_release(struct device
> *dev, void *res)
> int devm_devfreq_register_opp_notifier(struct device *dev,
>struct devfreq *devfreq)
> {
> - struct devfreq **ptr;
> - int ret;
> -
> - ptr = devres_alloc(devm_devfreq_opp_release, sizeof(*ptr),
> GFP_KERNEL);
> - if (!ptr)
> - return -ENOMEM;
> -
> - ret = devfreq_register_opp_notifier(dev, devfreq);
> - if (ret) {
> - devres_free(ptr);
> - return ret;
> - }
> -
> - *ptr = devfreq;
> - devres_add(dev, ptr);
> -
> - return 0;
> + return devm_pm_opp_register_notifier(dev, >nb);
> }
> EXPORT_SYMBOL(devm_devfreq_register_opp_notifier);
>
> -/**
> - * devm_devfreq_unregister_opp_notifier() - Resource-managed
> - * devfreq_unregister_opp_notifier()
> - * @dev: The devfreq user device. (parent of devfreq)
> - * @devfreq: The devfreq object.
> - */
> -void devm_devfreq_unregister_opp_notifier(struct device *dev,
> -struct devfreq *devfreq)
> -{
> - WARN_ON(devres_release(dev, devm_devfreq_opp_release,
> - devm_devfreq_dev_match, devfreq));
> -}
> -EXPORT_SYMBOL(devm_devfreq_unregister_opp_notifier);
Need to support devm_devfreq_unregister_opp_notifier()
because sometimes, the user wants to release the resource by himself.
(snip)
Best Regards,
Chanwoo Choi
Re: [PATCH 30/31] PM / devfreq: exynos: convert to use devm_pm_opp_* API
q(opp); > dev_pm_opp_put(opp); > > return 0; > > -err_opp: > - dev_pm_opp_of_remove_table(dev); > err_clk: > clk_disable_unprepare(bus->clk); > > @@ -425,7 +405,7 @@ static int exynos_bus_probe(struct platform_device *pdev) > /* Parse the device-tree to get the resource information */ > ret = exynos_bus_parse_of(np, bus); > if (ret < 0) > - goto err_reg; > + return ret; > > if (passive) > ret = exynos_bus_profile_init_passive(bus, profile); > @@ -456,11 +436,7 @@ static int exynos_bus_probe(struct platform_device *pdev) > return 0; > > err: > - dev_pm_opp_of_remove_table(dev); > clk_disable_unprepare(bus->clk); > -err_reg: > - dev_pm_opp_put_regulators(bus->opp_table); > - bus->opp_table = NULL; > > return ret; > } > -- > 2.25.1 > -- Best Regards, Chanwoo Choi
Re: [PATCH 29/31] PM / devfreq: imx-bus: convert to use devm_pm_opp_* API
On Sun, Jan 3, 2021 at 12:58 PM Yangtao Li wrote:
>
> Use devm_pm_opp_* API to simplify code.
>
> Signed-off-by: Yangtao Li
> ---
> drivers/devfreq/imx-bus.c | 14 +++---
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/devfreq/imx-bus.c b/drivers/devfreq/imx-bus.c
> index 4f38455ad742..ff26ef049b1b 100644
> --- a/drivers/devfreq/imx-bus.c
> +++ b/drivers/devfreq/imx-bus.c
> @@ -61,7 +61,6 @@ static void imx_bus_exit(struct device *dev)
> {
> struct imx_bus *priv = dev_get_drvdata(dev);
>
> - dev_pm_opp_of_remove_table(dev);
> platform_device_unregister(priv->icc_pdev);
> }
>
> @@ -123,7 +122,7 @@ static int imx_bus_probe(struct platform_device *pdev)
> }
> platform_set_drvdata(pdev, priv);
>
> - ret = dev_pm_opp_of_add_table(dev);
> + ret = devm_pm_opp_of_add_table(dev);
> if (ret < 0) {
> dev_err(dev, "failed to get OPP table\n");
> return ret;
> @@ -141,18 +140,11 @@ static int imx_bus_probe(struct platform_device *pdev)
> if (IS_ERR(priv->devfreq)) {
> ret = PTR_ERR(priv->devfreq);
> dev_err(dev, "failed to add devfreq device: %d\n", ret);
> - goto err;
> + return ret;
> }
>
> - ret = imx_bus_init_icc(dev);
> - if (ret)
> - goto err;
> -
> - return 0;
> + return imx_bus_init_icc(dev);
Remove the unneeded blank line under '.return imx_bus_init_icc(dev);".
> -err:
> - dev_pm_opp_of_remove_table(dev);
> - return ret;
> }
>
> static const struct of_device_id imx_bus_of_match[] = {
> --
> 2.25.1
>
--
Best Regards,
Chanwoo Choi
Re: [PATCH 28/31] PM / devfreq: imx8m-ddrc: convert to use devm_pm_opp_* API
On Sun, Jan 3, 2021 at 12:58 PM Yangtao Li wrote:
>
> Use devm_pm_opp_* API to simplify code.
>
> Signed-off-by: Yangtao Li
> ---
> drivers/devfreq/imx8m-ddrc.c | 15 ++-
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
> index bc82d3653bff..9383d6e5538b 100644
> --- a/drivers/devfreq/imx8m-ddrc.c
> +++ b/drivers/devfreq/imx8m-ddrc.c
> @@ -370,11 +370,6 @@ static int imx8m_ddrc_check_opps(struct device *dev)
> return 0;
> }
>
> -static void imx8m_ddrc_exit(struct device *dev)
> -{
> - dev_pm_opp_of_remove_table(dev);
> -}
> -
> static int imx8m_ddrc_probe(struct platform_device *pdev)
> {
> struct device *dev = >dev;
> @@ -419,7 +414,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = dev_pm_opp_of_add_table(dev);
> + ret = devm_pm_opp_of_add_table(dev);
> if (ret < 0) {
> dev_err(dev, "failed to get OPP table\n");
> return ret;
> @@ -427,12 +422,11 @@ static int imx8m_ddrc_probe(struct platform_device
> *pdev)
>
> ret = imx8m_ddrc_check_opps(dev);
> if (ret < 0)
> - goto err;
> + return ret;
>
> priv->profile.polling_ms = 1000;
> priv->profile.target = imx8m_ddrc_target;
> priv->profile.get_dev_status = imx8m_ddrc_get_dev_status;
> - priv->profile.exit = imx8m_ddrc_exit;
> priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
> priv->profile.initial_freq = clk_get_rate(priv->dram_core);
>
> @@ -441,13 +435,8 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
> if (IS_ERR(priv->devfreq)) {
> ret = PTR_ERR(priv->devfreq);
> dev_err(dev, "failed to add devfreq device: %d\n", ret);
> - goto err;
> }
>
> - return 0;
> -
> -err:
> - dev_pm_opp_of_remove_table(dev);
> return ret;
devm_devfreq_add_device() doesn't return any integer value.
Even if devm_devfreq_add_device() returns the right devfreq instance,
the 'ret' value is not the return value of devm_devfreq_add_device().
On this patch, 'ret' value of 'return ret' is from imx8m_ddrc_check_opps().
Surely, it is well working with this modification. But, it is not code
for exception handling.
So, we need to remain the following codes:
return 0;
err:
return ret;
> }
>
> --
> 2.25.1
>
--
Best Regards,
Chanwoo Choi
Re: [PATCH 27/31] PM / devfreq: rk3399_dmc: convert to use devm_pm_opp_* API
Hi,
Do you make this patch on latest source?
When I apply this patch for test, it make the merge conflict error.
On Sun, Jan 3, 2021 at 12:57 PM Yangtao Li wrote:
>
> Use devm_pm_opp_* API to simplify code. Since devres release
> can guarantee the order, let's remove
> devm_devfreq_unregister_opp_notifier().
>
> Signed-off-by: Yangtao Li
> ---
> drivers/devfreq/rk3399_dmc.c | 22 +++---
> 1 file changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index 2e912166a993..9b8ab8be29d1 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -432,7 +432,7 @@ static int rk3399_dmcfreq_probe(struct platform_device
> *pdev)
> * We add a devfreq driver to our parent since it has a device tree
> node
> * with operating points.
> */
> - if (dev_pm_opp_of_add_table(dev)) {
> + if (devm_pm_opp_of_add_table(dev)) {
> dev_err(dev, "Invalid operating-points in device tree.\n");
> ret = -EINVAL;
> goto err_edev;
> @@ -448,7 +448,7 @@ static int rk3399_dmcfreq_probe(struct platform_device
> *pdev)
> opp = devfreq_recommended_opp(dev, >rate, 0);
> if (IS_ERR(opp)) {
> ret = PTR_ERR(opp);
> - goto err_free_opp;
> + goto err_edev;
> }
>
> data->rate = dev_pm_opp_get_freq(opp);
> @@ -463,7 +463,7 @@ static int rk3399_dmcfreq_probe(struct platform_device
> *pdev)
>>ondemand_data);
> if (IS_ERR(data->devfreq)) {
> ret = PTR_ERR(data->devfreq);
> - goto err_free_opp;
> + goto err_edev;
> }
>
> devm_devfreq_register_opp_notifier(dev, data->devfreq);
> @@ -473,27 +473,12 @@ static int rk3399_dmcfreq_probe(struct platform_device
> *pdev)
>
> return 0;
>
> -err_free_opp:
> - dev_pm_opp_of_remove_table(>dev);
> err_edev:
> devfreq_event_disable_edev(data->edev);
>
> return ret;
> }
>
> -static int rk3399_dmcfreq_remove(struct platform_device *pdev)
> -{
> - struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(>dev);
> -
> - /*
> -* Before remove the opp table we need to unregister the opp notifier.
> -*/
> - devm_devfreq_unregister_opp_notifier(dmcfreq->dev, dmcfreq->devfreq);
> - dev_pm_opp_of_remove_table(dmcfreq->dev);
As the comment, we need to unregister the opp notifier before removing the OPP.
Do you guarantee this sequence on your patch?
> -
> - return 0;
> -}
> -
> static const struct of_device_id rk3399dmc_devfreq_of_match[] = {
> { .compatible = "rockchip,rk3399-dmc" },
> { },
> @@ -502,7 +487,6 @@ MODULE_DEVICE_TABLE(of, rk3399dmc_devfreq_of_match);
>
> static struct platform_driver rk3399_dmcfreq_driver = {
> .probe = rk3399_dmcfreq_probe,
> - .remove = rk3399_dmcfreq_remove,
> .driver = {
> .name = "rk3399-dmc-freq",
> .pm = _dmcfreq_pm,
> --
> 2.25.1
>
--
Best Regards,
Chanwoo Choi
Re: [PATCH 26/31] PM / devfreq: tegra30: convert to use devm_pm_opp_* API
Hi,
On Sun, Jan 3, 2021 at 12:57 PM Yangtao Li wrote:
>
> Use devm_pm_opp_* API to simplify code, and remove opp_table
> from tegra_devfreq.
>
> Signed-off-by: Yangtao Li
> ---
> drivers/devfreq/tegra30-devfreq.c | 21 +++--
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c
> b/drivers/devfreq/tegra30-devfreq.c
> index 117cad7968ab..4984cb91e9ea 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -178,7 +178,6 @@ struct tegra_devfreq_soc_data {
>
> struct tegra_devfreq {
> struct devfreq *devfreq;
> - struct opp_table*opp_table;
>
> struct reset_control*reset;
> struct clk *clock;
> @@ -794,6 +793,7 @@ static int tegra_devfreq_probe(struct platform_device
> *pdev)
> struct tegra_devfreq_device *dev;
> struct tegra_devfreq *tegra;
> struct devfreq *devfreq;
> + struct opp_tabl *opp_table;
opp_tabl -> opp_table. It will make the build error. Please do build
test at least.
> unsigned int i;
> long rate;
> int err;
> @@ -841,25 +841,25 @@ static int tegra_devfreq_probe(struct platform_device
> *pdev)
> return err;
> }
>
> - tegra->opp_table = dev_pm_opp_set_supported_hw(>dev,
> - _version, 1);
> - err = PTR_ERR_OR_ZERO(tegra->opp_table);
> + opp_table = devm_pm_opp_set_supported_hw(>dev,
> +_version, 1);
> + err = PTR_ERR_OR_ZERO(opp_table);
> if (err) {
> dev_err(>dev, "Failed to set supported HW: %d\n", err);
> return err;
> }
>
> - err = dev_pm_opp_of_add_table(>dev);
> + err = devm_pm_opp_of_add_table(>dev);
> if (err) {
> dev_err(>dev, "Failed to add OPP table: %d\n", err);
> - goto put_hw;
> + return err;
> }
>
> err = clk_prepare_enable(tegra->clock);
> if (err) {
> dev_err(>dev,
> "Failed to prepare and enable ACTMON clock\n");
> - goto remove_table;
> + return err;
> }
>
> err = reset_control_reset(tegra->reset);
> @@ -917,10 +917,6 @@ static int tegra_devfreq_probe(struct platform_device
> *pdev)
> reset_control_reset(tegra->reset);
> disable_clk:
> clk_disable_unprepare(tegra->clock);
> -remove_table:
> - dev_pm_opp_of_remove_table(>dev);
> -put_hw:
> - dev_pm_opp_put_supported_hw(tegra->opp_table);
You might remove the 'devm_pm_opp_remove_all_dynamic(>dev)
under ' remove_opp' goto statement.
(snip)
--
Best Regards,
Chanwoo Choi
Re: [PATCH 1/1] PM / devfreq: Replace devfreq->dev.parent as dev in devfreq_add_device
On Tue, Dec 29, 2020 at 12:25 AM pierre kuo wrote: > > Hi myungjoo, kyungmin and cw: > Would you please help to review this patch? > > Thanks a lot. > > pierre Kuo 於 2020年12月16日 週三 上午10:26寫道： > > > > In devfreq_add_device, replace devfreq->dev.parent > > as dev to keep code simple. > > > > Signed-off-by: pierre Kuo > > --- > > drivers/devfreq/devfreq.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > > index 6aa10de792b3..94cc25fd68da 100644 > > --- a/drivers/devfreq/devfreq.c > > +++ b/drivers/devfreq/devfreq.c > > @@ -893,13 +893,13 @@ struct devfreq *devfreq_add_device(struct device *dev, > > goto err_devfreq; > > > > devfreq->nb_min.notifier_call = qos_min_notifier_call; > > - err = dev_pm_qos_add_notifier(devfreq->dev.parent, >nb_min, > > + err = dev_pm_qos_add_notifier(dev, >nb_min, > > DEV_PM_QOS_MIN_FREQUENCY); > > if (err) > > goto err_devfreq; > > > > devfreq->nb_max.notifier_call = qos_max_notifier_call; > > - err = dev_pm_qos_add_notifier(devfreq->dev.parent, >nb_max, > > + err = dev_pm_qos_add_notifier(dev, >nb_max, > > DEV_PM_QOS_MAX_FREQUENCY); > > if (err) > > goto err_devfreq; > > -- > > 2.17.1 > > Applied it. Thanks. -- Best Regards, Chanwoo Choi
Re: [PATCH] PM / devfreq: spelling correction in a comment
Hi Lukasz, On Mon, Jan 4, 2021 at 11:01 PM Lukasz Luba wrote: > > The device attribute exposed in sysfs is called 'polling_interval'. Align > the comment. > > Signed-off-by: Lukasz Luba > --- > Hi Chanwoo, > > While grepping the devfreq polling_interval sources I've found > this trivial mistake in the comment. > > Regards, > Lukasz > > drivers/devfreq/governor.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h > index 2a52f97b542d..70f44b3ca42e 100644 > --- a/drivers/devfreq/governor.h > +++ b/drivers/devfreq/governor.h > @@ -40,7 +40,7 @@ > /* > * Definition of governor attribute flags except for common sysfs attributes > * - DEVFREQ_GOV_ATTR_POLLING_INTERVAL > - * : Indicate polling_interal sysfs attribute > + * : Indicate polling_interval sysfs attribute > * - DEVFREQ_GOV_ATTR_TIMER > * : Indicate timer sysfs attribute > */ > -- > 2.17.1 > Applied it. Thanks. -- Best Regards, Chanwoo Choi
Re: [PATCH] extcon: Add stubs for extcon_register_notifier_all() functions
Hi Krzysztof,
On Thu, Dec 31, 2020 at 5:56 PM Krzysztof Kozlowski wrote:
>
> Add stubs for extcon_register_notifier_all() function for !CONFIG_EXTCON
> case. This is useful for compile testing and for drivers which use
> EXTCON but do not require it (therefore do not depend on CONFIG_EXTCON).
>
> Fixes: 815429b39d94 ("extcon: Add new extcon_register_notifier_all() to
> monitor all external connectors")
> Reported-by: kernel test robot
> Signed-off-by: Krzysztof Kozlowski
> ---
> include/linux/extcon.h | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index fd183fb9c20f..0c19010da77f 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -271,6 +271,29 @@ static inline void
> devm_extcon_unregister_notifier(struct device *dev,
> struct extcon_dev *edev, unsigned int id,
> struct notifier_block *nb) { }
>
> +static inline int extcon_register_notifier_all(struct extcon_dev *edev,
> + struct notifier_block *nb)
> +{
> + return 0;
> +}
> +
> +static inline int extcon_unregister_notifier_all(struct extcon_dev *edev,
> +struct notifier_block *nb)
> +{
> + return 0;
> +}
> +
> +static inline int devm_extcon_register_notifier_all(struct device *dev,
> + struct extcon_dev *edev,
> + struct notifier_block *nb)
> +{
> + return 0;
> +}
> +
> +static inline void devm_extcon_unregister_notifier_all(struct device *dev,
> + struct extcon_dev
> *edev,
> + struct notifier_block
> *nb) { }
> +
> static inline struct extcon_dev *extcon_get_extcon_dev(const char
> *extcon_name)
> {
> return ERR_PTR(-ENODEV);
> --
> 2.25.1
>
Applied it. Thanks.
--
Best Regards,
Chanwoo Choi
Re: [PATCH v6 1/8] extcon: max8997: Add CHGINS and CHGRM interrupt handling
On Thu, Dec 31, 2020 at 5:54 AM Timon Baetz wrote:
>
> This allows the MAX8997 charger to set the current limit depending on
> the detected extcon charger type.
>
> Signed-off-by: Timon Baetz
> ---
> v6: No change.
> v5: No change.
> v4: No change.
> v3: No change.
> v2: Remove empty line.
>
> drivers/extcon/extcon-max8997.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
> index 337b0eea4e62..e1408075ef7d 100644
> --- a/drivers/extcon/extcon-max8997.c
> +++ b/drivers/extcon/extcon-max8997.c
> @@ -44,6 +44,8 @@ static struct max8997_muic_irq muic_irqs[] = {
> { MAX8997_MUICIRQ_ChgDetRun,"muic-CHGDETRUN" },
> { MAX8997_MUICIRQ_ChgTyp, "muic-CHGTYP" },
> { MAX8997_MUICIRQ_OVP, "muic-OVP" },
> + { MAX8997_PMICIRQ_CHGINS, "pmic-CHGINS" },
> + { MAX8997_PMICIRQ_CHGRM,"pmic-CHGRM" },
> };
>
> /* Define supported cable type */
> @@ -538,6 +540,8 @@ static void max8997_muic_irq_work(struct work_struct
> *work)
> case MAX8997_MUICIRQ_DCDTmr:
> case MAX8997_MUICIRQ_ChgDetRun:
> case MAX8997_MUICIRQ_ChgTyp:
> + case MAX8997_PMICIRQ_CHGINS:
> + case MAX8997_PMICIRQ_CHGRM:
> /* Handle charger cable */
> ret = max8997_muic_chg_handler(info);
> break;
> --
> 2.25.1
>
>
Applied it. Thans.
--
Best Regards,
Chanwoo Choi
