Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH 00/12] Add cpufreq and cci devfreq for mt8183, and SVS support
       [not found] ` <20200520034307.20435-1-andrew-sh.cheng@mediatek.com>
@ 2020-05-20  4:10   ` Chanwoo Choi
       [not found]     ` <1589953015.8243.2.camel@mtksdaap41>
       [not found]   ` <20200520034307.20435-10-andrew-sh.cheng@mediatek.com>
       [not found]   ` <20200520034307.20435-2-andrew-sh.cheng@mediatek.com>
  2 siblings, 1 reply; 6+ messages in thread
From: Chanwoo Choi @ 2020-05-20  4:10 UTC (permalink / raw)
  To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
  Cc: linux-pm, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, srv_heupstream

Hi Andrew,

Could you explain the base commit of these patches?
When I tried to apply them to v5.7-rc1 for testing,
the merge conflict occurs.

Thanks,
Chanwoo Choi

On 5/20/20 12:42 PM, Andrew-sh.Cheng wrote:
> MT8183 supports CPU DVFS and CCI DVFS, and LITTLE cpus and CCI are in the same voltage domain.
> So, this series is to add drivers to handle the voltage coupling between CPU and CCI DVFS.
> 
> For SVS support, need OPP_EVENT_ADJUST_VOLTAGE and corresponding reaction.
> 
> Change since v5:
> 	- Changing dt-binding format to yaml.
> 	- Extending current devfreq passive_governor instead of create a new one.
> 	- Resend depending patches of Sravana Kannan base on kernel-5.7
> 
> 
> Andrew-sh.Cheng (6):
>   cpufreq: mediatek: add clock and regulator enable for intermediate
>     clock
>   dt-bindings: devfreq: add compatible for mt8183 cci devfreq
>   devfreq: add mediatek cci devfreq
>   opp: Modify opp API, dev_pm_opp_get_freq(), find freq in opp, even it
>     is disabled
>   cpufreq: mediatek: add opp notification for SVS support
>   devfreq: mediatek: cci devfreq register opp notification for SVS
>     support
> 
> Saravana Kannan (6):
>   OPP: Allow required-opps even if the device doesn't have power-domains
>   OPP: Add function to look up required OPP's for a given OPP
>   OPP: Improve required-opps linking
>   PM / devfreq: Cache OPP table reference in devfreq
>   PM / devfreq: Add required OPPs support to passive governor
>   PM / devfreq: Add cpu based scaling support to passive_governor
> 
>  .../devicetree/bindings/devfreq/mt8183-cci.yaml    |  51 ++++
>  drivers/cpufreq/mediatek-cpufreq.c                 | 122 ++++++++-
>  drivers/devfreq/Kconfig                            |  12 +
>  drivers/devfreq/Makefile                           |   1 +
>  drivers/devfreq/devfreq.c                          |   6 +
>  drivers/devfreq/governor_passive.c                 | 298 +++++++++++++++++++--
>  drivers/devfreq/mt8183-cci-devfreq.c               | 233 ++++++++++++++++
>  drivers/opp/core.c                                 |  85 +++++-
>  drivers/opp/of.c                                   | 108 ++++----
>  drivers/opp/opp.h                                  |   5 +
>  include/linux/devfreq.h                            |  42 ++-
>  include/linux/pm_opp.h                             |  11 +
>  12 files changed, 874 insertions(+), 100 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
>  create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 00/12] Add cpufreq and cci devfreq for mt8183, and SVS support
       [not found]     ` <1589953015.8243.2.camel@mtksdaap41>
@ 2020-05-20  6:24       ` Chanwoo Choi
       [not found]         ` <1589958625.23971.2.camel@mtksdaap41>
  0 siblings, 1 reply; 6+ messages in thread
From: Chanwoo Choi @ 2020-05-20  6:24 UTC (permalink / raw)
  To: andrew-sh.cheng
  Cc: MyungJoo Ham, Kyungmin Park, Rob Herring, Mark Rutland,
	Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown,
	linux-pm, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, srv_heupstream

Hi,

On 5/20/20 2:36 PM, andrew-sh.cheng wrote:
> On Wed, 2020-05-20 at 13:10 +0900, Chanwoo Choi wrote:
>> Hi Andrew,
>>
>> Could you explain the base commit of these patches?
>> When I tried to apply them to v5.7-rc1 for testing,
>> the merge conflict occurs.
>>
>> Thanks,

>> Chanwoo Choi
> 
> Hi Chanwoo Choi,
> 
> My base commit is
> commit 8f3d9f354286745c751374f5f1fcafee6b3f3136
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Sun Apr 12 12:35:55 2020 -0700
> 
>     Linux 5.7-rc1
> 
> Could you show me the conflict error?


When I tried to apply first patch with 'git am',
the merge conflict occurred.

git am \[PATCH\ 01_12\]\ OPP\:\ Allow\ required-opps\ even\ if\ the\ device\ doesn\'t\ have\ power-domains.eml
Applying: OPP: Allow required-opps even if the device doesn't have power-domains
error: patch failed: drivers/opp/core.c:755
error: drivers/opp/core.c: patch does not apply
error: patch failed: drivers/opp/of.c:195																																																																												
error: drivers/opp/of.c: patch does not apply
Patch failed at 0001 OPP: Allow required-opps even if the device doesn't have power-domains
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Regards,
Chanwoo Choi

> 
> BR,
> Andrew-sh.Cheng
>>
>> On 5/20/20 12:42 PM, Andrew-sh.Cheng wrote:
>>> MT8183 supports CPU DVFS and CCI DVFS, and LITTLE cpus and CCI are in the same voltage domain.
>>> So, this series is to add drivers to handle the voltage coupling between CPU and CCI DVFS.
>>>
>>> For SVS support, need OPP_EVENT_ADJUST_VOLTAGE and corresponding reaction.
>>>
>>> Change since v5:
>>> 	- Changing dt-binding format to yaml.
>>> 	- Extending current devfreq passive_governor instead of create a new one.
>>> 	- Resend depending patches of Sravana Kannan base on kernel-5.7
>>>
>>>
>>> Andrew-sh.Cheng (6):
>>>   cpufreq: mediatek: add clock and regulator enable for intermediate
>>>     clock
>>>   dt-bindings: devfreq: add compatible for mt8183 cci devfreq
>>>   devfreq: add mediatek cci devfreq
>>>   opp: Modify opp API, dev_pm_opp_get_freq(), find freq in opp, even it
>>>     is disabled
>>>   cpufreq: mediatek: add opp notification for SVS support
>>>   devfreq: mediatek: cci devfreq register opp notification for SVS
>>>     support
>>>
>>> Saravana Kannan (6):
>>>   OPP: Allow required-opps even if the device doesn't have power-domains
>>>   OPP: Add function to look up required OPP's for a given OPP
>>>   OPP: Improve required-opps linking
>>>   PM / devfreq: Cache OPP table reference in devfreq
>>>   PM / devfreq: Add required OPPs support to passive governor
>>>   PM / devfreq: Add cpu based scaling support to passive_governor
>>>
>>>  .../devicetree/bindings/devfreq/mt8183-cci.yaml    |  51 ++++
>>>  drivers/cpufreq/mediatek-cpufreq.c                 | 122 ++++++++-
>>>  drivers/devfreq/Kconfig                            |  12 +
>>>  drivers/devfreq/Makefile                           |   1 +
>>>  drivers/devfreq/devfreq.c                          |   6 +
>>>  drivers/devfreq/governor_passive.c                 | 298 +++++++++++++++++++--
>>>  drivers/devfreq/mt8183-cci-devfreq.c               | 233 ++++++++++++++++
>>>  drivers/opp/core.c                                 |  85 +++++-
>>>  drivers/opp/of.c                                   | 108 ++++----
>>>  drivers/opp/opp.h                                  |   5 +
>>>  include/linux/devfreq.h                            |  42 ++-
>>>  include/linux/pm_opp.h                             |  11 +
>>>  12 files changed, 874 insertions(+), 100 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
>>>  create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
>>>
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 09/12] devfreq: add mediatek cci devfreq
       [not found]   ` <20200520034307.20435-10-andrew-sh.cheng@mediatek.com>
@ 2020-05-20 12:31     ` Mark Brown
  2020-05-21  8:52       ` andrew-sh.cheng
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2020-05-20 12:31 UTC (permalink / raw)
  To: Andrew-sh.Cheng
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, linux-pm,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	srv_heupstream


[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

On Wed, May 20, 2020 at 11:43:04AM +0800, Andrew-sh.Cheng wrote:

> +	cci_df->proc_reg = devm_regulator_get_optional(cci_dev, "proc");
> +	ret = PTR_ERR_OR_ZERO(cci_df->proc_reg);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
> +				ret);
> +		return ret;
> +	}
> +	ret = regulator_enable(cci_df->proc_reg);

The code appears to require a regulator (and I'm guessing the device
needs power) so why is this using regulator_get_optional()?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 00/12] Add cpufreq and cci devfreq for mt8183, and SVS support
       [not found]         ` <1589958625.23971.2.camel@mtksdaap41>
@ 2020-05-20 14:53           ` Matthias Brugger
  0 siblings, 0 replies; 6+ messages in thread
From: Matthias Brugger @ 2020-05-20 14:53 UTC (permalink / raw)
  To: andrew-sh.cheng, Chanwoo Choi
  Cc: MyungJoo Ham, Kyungmin Park, Rob Herring, Mark Rutland,
	Rafael J . Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd,
	Liam Girdwood, Mark Brown, linux-pm, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, srv_heupstream



On 20/05/2020 09:10, andrew-sh.cheng wrote:
> On Wed, 2020-05-20 at 15:24 +0900, Chanwoo Choi wrote:
>> Hi,
>>
>> On 5/20/20 2:36 PM, andrew-sh.cheng wrote:
>>> On Wed, 2020-05-20 at 13:10 +0900, Chanwoo Choi wrote:
>>>> Hi Andrew,
>>>>
>>>> Could you explain the base commit of these patches?
>>>> When I tried to apply them to v5.7-rc1 for testing,
>>>> the merge conflict occurs.
>>>>
>>>> Thanks,
>>
>>>> Chanwoo Choi
>>>
>>> Hi Chanwoo Choi,
>>>
>>> My base commit is
>>> commit 8f3d9f354286745c751374f5f1fcafee6b3f3136
>>> Author: Linus Torvalds <torvalds@linux-foundation.org>
>>> Date:   Sun Apr 12 12:35:55 2020 -0700
>>>
>>>     Linux 5.7-rc1
>>>
>>> Could you show me the conflict error?
>>
>>
>> When I tried to apply first patch with 'git am',
>> the merge conflict occurred.
>>
>> git am \[PATCH\ 01_12\]\ OPP\:\ Allow\ required-opps\ even\ if\ the\ device\ doesn\'t\ have\ power-domains.eml
>> Applying: OPP: Allow required-opps even if the device doesn't have power-domains
>> error: patch failed: drivers/opp/core.c:755
>> error: drivers/opp/core.c: patch does not apply
>> error: patch failed: drivers/opp/of.c:195																																																																												
>> error: drivers/opp/of.c: patch does not apply
>> Patch failed at 0001 OPP: Allow required-opps even if the device doesn't have power-domains
>> Use 'git am --show-current-patch' to see the failed patch
>> When you have resolved this problem, run "git am --continue".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>>
>> Regards,
>> Chanwoo Choi
> 
> Hi Chanwoo,
> 
> I just make a new folder to get code and check.
> Below is my command list.
> Please help check the different with you.
>   505  repo init -u http://gerrit.mediatek.inc:8080/cros-kernel/manifest
> -b upstream
>   506  repo sync -j8
>   507  repo start kern-dev --all
>   508   git remote add main
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>   509  git remote add main
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>   510  ls
>   511  cd kernel/mediatek/
>   512   git remote add main
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>   513  git fetch main
>   514  git checkout v5.7-rc1
>   515  git am
> Add-cpufreq-and-cci-devfreq-for-mt8183-and-SVS-support.patch
>   516  history
> 

For reference I just tried with b4.sh [1]:
# b4.sh am -l -o /tmp -n patch  1589958625.23971.2.camel@mtksdaap41
# git am -3 -s /tmp/patch.mbx

Applies without conflicts.

Regards,
Matthias

[1] https://git.kernel.org/pub/scm/utils/b4/b4.git

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 01/12] OPP: Allow required-opps even if the device doesn't have power-domains
       [not found]   ` <20200520034307.20435-2-andrew-sh.cheng@mediatek.com>
@ 2020-05-20 14:54     ` Matthias Brugger
  0 siblings, 0 replies; 6+ messages in thread
From: Matthias Brugger @ 2020-05-20 14:54 UTC (permalink / raw)
  To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Rob Herring, Mark Rutland, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
  Cc: linux-pm, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, srv_heupstream, Saravana Kannan



On 20/05/2020 05:42, Andrew-sh.Cheng wrote:
> From: Saravana Kannan <saravanak@google.com>
> 
> A Device-A can have a (minimum) performance requirement on another
> Device-B to be able to function correctly. This performance requirement
> on Device-B can also change based on the current performance level of
> Device-A.
> 
> The existing required-opps feature fits well to describe this need. So,
> instead of limiting required-opps to point to only PM-domain devices,
> allow it to point to any device.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Please check all patches, they are missing your
Signed-off-by

Regards,
Matthias

> ---
>  drivers/opp/core.c |  2 +-
>  drivers/opp/of.c   | 11 -----------
>  2 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index ba43e6a3dc0a..51403c1f2481 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -755,7 +755,7 @@ static int _set_required_opps(struct device *dev,
>  		return 0;
>  
>  	/* Single genpd case */
> -	if (!genpd_virt_devs) {
> +	if (!genpd_virt_devs && required_opp_tables[0]->is_genpd) {
>  		pstate = likely(opp) ? opp->required_opps[0]->pstate : 0;
>  		ret = dev_pm_genpd_set_performance_state(dev, pstate);
>  		if (ret) {
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index 9cd8f0adacae..6d33de668a7b 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -195,17 +195,6 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table,
>  
>  		if (IS_ERR(required_opp_tables[i]))
>  			goto free_required_tables;
> -
> -		/*
> -		 * We only support genpd's OPPs in the "required-opps" for now,
> -		 * as we don't know how much about other cases. Error out if the
> -		 * required OPP doesn't belong to a genpd.
> -		 */
> -		if (!required_opp_tables[i]->is_genpd) {
> -			dev_err(dev, "required-opp doesn't belong to genpd: %pOF\n",
> -				required_np);
> -			goto free_required_tables;
> -		}
>  	}
>  
>  	goto put_np;
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 09/12] devfreq: add mediatek cci devfreq
  2020-05-20 12:31     ` [PATCH 09/12] devfreq: add mediatek cci devfreq Mark Brown
@ 2020-05-21  8:52       ` andrew-sh.cheng
  0 siblings, 0 replies; 6+ messages in thread
From: andrew-sh.cheng @ 2020-05-21  8:52 UTC (permalink / raw)
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Liam Girdwood, linux-pm,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	srv_heupstream


On Wed, 2020-05-20 at 13:31 +0100, Mark Brown wrote:
> On Wed, May 20, 2020 at 11:43:04AM +0800, Andrew-sh.Cheng wrote:
> 
> > +	cci_df->proc_reg = devm_regulator_get_optional(cci_dev, "proc");
> > +	ret = PTR_ERR_OR_ZERO(cci_df->proc_reg);
> > +	if (ret) {
> > +		if (ret != -EPROBE_DEFER)
> > +			dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
> > +				ret);
> > +		return ret;
> > +	}
> > +	ret = regulator_enable(cci_df->proc_reg);
> 
> The code appears to require a regulator (and I'm guessing the device
> needs power) so why is this using regulator_get_optional()?

Hi Mark,

Do you mean, why not use regulator_get_exclusive() or regulator_get()?
Because cci and cpu litter core shared buck, it cannot use
regulator_get_exclusive().
Because both cci and cpu want to tune voltage, it cannot use
regulator_get(), otherwise it will get dummy regulator even this buck
doesn't register.as regulator.

BR,
Andrew-sh.Cheng

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200520034324epcas1p3affbd24bd1f3fe40d51baade07c1abba@epcas1p3.samsung.com>
     [not found] ` <20200520034307.20435-1-andrew-sh.cheng@mediatek.com>
2020-05-20  4:10   ` [PATCH 00/12] Add cpufreq and cci devfreq for mt8183, and SVS support Chanwoo Choi
     [not found]     ` <1589953015.8243.2.camel@mtksdaap41>
2020-05-20  6:24       ` Chanwoo Choi
     [not found]         ` <1589958625.23971.2.camel@mtksdaap41>
2020-05-20 14:53           ` Matthias Brugger
     [not found]   ` <20200520034307.20435-10-andrew-sh.cheng@mediatek.com>
2020-05-20 12:31     ` [PATCH 09/12] devfreq: add mediatek cci devfreq Mark Brown
2020-05-21  8:52       ` andrew-sh.cheng
     [not found]   ` <20200520034307.20435-2-andrew-sh.cheng@mediatek.com>
2020-05-20 14:54     ` [PATCH 01/12] OPP: Allow required-opps even if the device doesn't have power-domains Matthias Brugger

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git