Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH v9 01/15] ARM: Add Krait L2 register accessor functions
From: Sricharan R @ 2018-05-25  5:40 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
	andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
	linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
	linux
In-Reply-To: <20180524173926.GC14924@minitux>

Hi Bjorn,

On 5/24/2018 11:09 PM, Bjorn Andersson wrote:
> On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
> 
>> From: Stephen Boyd <sboyd@codeaurora.org>
>>
>> Krait CPUs have a handful of L2 cache controller registers that
>> live behind a cp15 based indirection register. First you program
>> the indirection register (l2cpselr) to point the L2 'window'
>> register (l2cpdr) at what you want to read/write.  Then you
>> read/write the 'window' register to do what you want. The
>> l2cpselr register is not banked per-cpu so we must lock around
>> accesses to it to prevent other CPUs from re-pointing l2cpdr
>> underneath us.
>>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> 
> This should have your signed-off-by here as well.
> 

 ok.

> Apart from that:
> 
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 

 Thanks.

Regards,
 Sricharan


> Regards,
> Bjorn
> 
>> ---
>>  arch/arm/common/Kconfig                   |  3 ++
>>  arch/arm/common/Makefile                  |  1 +
>>  arch/arm/common/krait-l2-accessors.c      | 48 +++++++++++++++++++++++++++++++
>>  arch/arm/include/asm/krait-l2-accessors.h | 10 +++++++
>>  4 files changed, 62 insertions(+)
>>  create mode 100644 arch/arm/common/krait-l2-accessors.c
>>  create mode 100644 arch/arm/include/asm/krait-l2-accessors.h
>>
>> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
>> index e5ad070..c8e1986 100644
>> --- a/arch/arm/common/Kconfig
>> +++ b/arch/arm/common/Kconfig
>> @@ -7,6 +7,9 @@ config DMABOUNCE
>>  	bool
>>  	select ZONE_DMA
>>  
>> +config KRAIT_L2_ACCESSORS
>> +	bool
>> +
>>  config SHARP_LOCOMO
>>  	bool
>>  
>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
>> index 70b4a14..eec6cd1 100644
>> --- a/arch/arm/common/Makefile
>> +++ b/arch/arm/common/Makefile
>> @@ -7,6 +7,7 @@ obj-y				+= firmware.o
>>  
>>  obj-$(CONFIG_SA1111)		+= sa1111.o
>>  obj-$(CONFIG_DMABOUNCE)		+= dmabounce.o
>> +obj-$(CONFIG_KRAIT_L2_ACCESSORS) += krait-l2-accessors.o
>>  obj-$(CONFIG_SHARP_LOCOMO)	+= locomo.o
>>  obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
>>  obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
>> diff --git a/arch/arm/common/krait-l2-accessors.c b/arch/arm/common/krait-l2-accessors.c
>> new file mode 100644
>> index 0000000..9a97dda
>> --- /dev/null
>> +++ b/arch/arm/common/krait-l2-accessors.c
>> @@ -0,0 +1,48 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> +
>> +#include <linux/spinlock.h>
>> +#include <linux/export.h>
>> +
>> +#include <asm/barrier.h>
>> +#include <asm/krait-l2-accessors.h>
>> +
>> +static DEFINE_RAW_SPINLOCK(krait_l2_lock);
>> +
>> +void krait_set_l2_indirect_reg(u32 addr, u32 val)
>> +{
>> +	unsigned long flags;
>> +
>> +	raw_spin_lock_irqsave(&krait_l2_lock, flags);
>> +	/*
>> +	 * Select the L2 window by poking l2cpselr, then write to the window
>> +	 * via l2cpdr.
>> +	 */
>> +	asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
>> +	isb();
>> +	asm volatile ("mcr p15, 3, %0, c15, c0, 7 @ l2cpdr" : : "r" (val));
>> +	isb();
>> +
>> +	raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
>> +}
>> +EXPORT_SYMBOL(krait_set_l2_indirect_reg);
>> +
>> +u32 krait_get_l2_indirect_reg(u32 addr)
>> +{
>> +	u32 val;
>> +	unsigned long flags;
>> +
>> +	raw_spin_lock_irqsave(&krait_l2_lock, flags);
>> +	/*
>> +	 * Select the L2 window by poking l2cpselr, then read from the window
>> +	 * via l2cpdr.
>> +	 */
>> +	asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
>> +	isb();
>> +	asm volatile ("mrc p15, 3, %0, c15, c0, 7 @ l2cpdr" : "=r" (val));
>> +
>> +	raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
>> +
>> +	return val;
>> +}
>> +EXPORT_SYMBOL(krait_get_l2_indirect_reg);
>> diff --git a/arch/arm/include/asm/krait-l2-accessors.h b/arch/arm/include/asm/krait-l2-accessors.h
>> new file mode 100644
>> index 0000000..dd7c474
>> --- /dev/null
>> +++ b/arch/arm/include/asm/krait-l2-accessors.h
>> @@ -0,0 +1,10 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> +
>> +#ifndef __ASMARM_KRAIT_L2_ACCESSORS_H
>> +#define __ASMARM_KRAIT_L2_ACCESSORS_H
>> +
>> +extern void krait_set_l2_indirect_reg(u32 addr, u32 val);
>> +extern u32 krait_get_l2_indirect_reg(u32 addr);
>> +
>> +#endif
>> -- 
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: [PATCH v9 02/15] clk: mux: Split out register accessors for reuse
From: Sricharan R @ 2018-05-25  5:38 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
	andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
	linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
	linux
In-Reply-To: <20180524165033.GB14924@minitux>

Hi Bjorn,

On 5/24/2018 10:20 PM, Bjorn Andersson wrote:
> On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
> 
>> From: Stephen Boyd <sboyd@codeaurora.org>
>>
>> We want to reuse the logic in clk-mux.c for other clock drivers
>> that don't use readl as register accessors. Fortunately, there
>> really isn't much to the mux code besides the table indirection
>> and quirk flags if you assume any bit shifting and masking has
>> been done already. Pull that logic out into reusable functions
>> that operate on an optional table and some flags so that other
>> drivers can use the same logic.
>>
>> [Sricharan: Rebased for mainline]
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> 
> This should read as a log, where the first entry is Stephen stating that
> he acquired or wrote the code and can release it according to the
> license requirements. Then you state that you acquired it, changed it
> and are releasing it according to the license requirements.
> 

 ok, will fix this to make it more clear.

> 
> PS. Please expand your last name.
> 

 ok. Just that, all my previous patches has so far gone like this :-)

Regards,
 Sricharan

> Regards,
> Bjorn
> 
>> ---
>>  drivers/clk/clk-mux.c         | 74 +++++++++++++++++++++++++++----------------
>>  drivers/clk/nxp/clk-lpc32xx.c | 21 +++---------
>>  include/linux/clk-provider.h  |  6 ++++
>>  3 files changed, 57 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
>> index 39cabe1..28223fa 100644
>> --- a/drivers/clk/clk-mux.c
>> +++ b/drivers/clk/clk-mux.c
>> @@ -26,35 +26,25 @@
>>   * parent - parent is adjustable through clk_set_parent
>>   */
>>  
>> -static u8 clk_mux_get_parent(struct clk_hw *hw)
>> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
>> +				unsigned int *table,
>> +				unsigned long flags)
>>  {
>> -	struct clk_mux *mux = to_clk_mux(hw);
>>  	int num_parents = clk_hw_get_num_parents(hw);
>> -	u32 val;
>> -
>> -	/*
>> -	 * FIXME need a mux-specific flag to determine if val is bitwise or numeric
>> -	 * e.g. sys_clkin_ck's clksel field is 3 bits wide, but ranges from 0x1
>> -	 * to 0x7 (index starts at one)
>> -	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
>> -	 * val = 0x4 really means "bit 2, index starts at bit 0"
>> -	 */
>> -	val = clk_readl(mux->reg) >> mux->shift;
>> -	val &= mux->mask;
>>  
>> -	if (mux->table) {
>> +	if (table) {
>>  		int i;
>>  
>>  		for (i = 0; i < num_parents; i++)
>> -			if (mux->table[i] == val)
>> +			if (table[i] == val)
>>  				return i;
>>  		return -EINVAL;
>>  	}
>>  
>> -	if (val && (mux->flags & CLK_MUX_INDEX_BIT))
>> +	if (val && (flags & CLK_MUX_INDEX_BIT))
>>  		val = ffs(val) - 1;
>>  
>> -	if (val && (mux->flags & CLK_MUX_INDEX_ONE))
>> +	if (val && (flags & CLK_MUX_INDEX_ONE))
>>  		val--;
>>  
>>  	if (val >= num_parents)
>> @@ -62,23 +52,53 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
>>  
>>  	return val;
>>  }
>> +EXPORT_SYMBOL_GPL(clk_mux_get_parent);
>>  
>> -static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>>  {
>>  	struct clk_mux *mux = to_clk_mux(hw);
>>  	u32 val;
>> -	unsigned long flags = 0;
>>  
>> -	if (mux->table) {
>> -		index = mux->table[index];
>> +	/*
>> +	 * FIXME need a mux-specific flag to determine if val is bitwise or
>> +	 * numeric e.g. sys_clkin_ck's clksel field is 3 bits wide,
>> +	 * but ranges from 0x1 to 0x7 (index starts at one)
>> +	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
>> +	 * val = 0x4 really means "bit 2, index starts at bit 0"
>> +	 */
>> +	val = clk_readl(mux->reg) >> mux->shift;
>> +	val &= mux->mask;
>> +
>> +	return clk_mux_get_parent(hw, val, mux->table, mux->flags);
>> +}
>> +
>> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
>> +			     unsigned long flags)
>> +{
>> +	unsigned int val = index;
>> +
>> +	if (table) {
>> +		val = table[val];
>>  	} else {
>> -		if (mux->flags & CLK_MUX_INDEX_BIT)
>> -			index = 1 << index;
>> +		if (flags & CLK_MUX_INDEX_BIT)
>> +			val = 1 << index;
>>  
>> -		if (mux->flags & CLK_MUX_INDEX_ONE)
>> -			index++;
>> +		if (flags & CLK_MUX_INDEX_ONE)
>> +			val++;
>>  	}
>>  
>> +	return val;
>> +}
>> +EXPORT_SYMBOL_GPL(clk_mux_reindex);
>> +
>> +static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +	struct clk_mux *mux = to_clk_mux(hw);
>> +	u32 val;
>> +	unsigned long flags = 0;
>> +
>> +	index = clk_mux_reindex(index, mux->table, mux->flags);
>> +
>>  	if (mux->lock)
>>  		spin_lock_irqsave(mux->lock, flags);
>>  	else
>> @@ -102,14 +122,14 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>>  }
>>  
>>  const struct clk_ops clk_mux_ops = {
>> -	.get_parent = clk_mux_get_parent,
>> +	.get_parent = _clk_mux_get_parent,
>>  	.set_parent = clk_mux_set_parent,
>>  	.determine_rate = __clk_mux_determine_rate,
>>  };
>>  EXPORT_SYMBOL_GPL(clk_mux_ops);
>>  
>>  const struct clk_ops clk_mux_ro_ops = {
>> -	.get_parent = clk_mux_get_parent,
>> +	.get_parent = _clk_mux_get_parent,
>>  };
>>  EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
>>  
>> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
>> index f5d815f..9b34150 100644
>> --- a/drivers/clk/nxp/clk-lpc32xx.c
>> +++ b/drivers/clk/nxp/clk-lpc32xx.c
>> @@ -999,29 +999,16 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>>  	.set_rate = clk_divider_set_rate,
>>  };
>>  
>> -static u8 clk_mux_get_parent(struct clk_hw *hw)
>> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>>  {
>>  	struct lpc32xx_clk_mux *mux = to_lpc32xx_mux(hw);
>> -	u32 num_parents = clk_hw_get_num_parents(hw);
>>  	u32 val;
>>  
>>  	regmap_read(clk_regmap, mux->reg, &val);
>>  	val >>= mux->shift;
>>  	val &= mux->mask;
>>  
>> -	if (mux->table) {
>> -		u32 i;
>> -
>> -		for (i = 0; i < num_parents; i++)
>> -			if (mux->table[i] == val)
>> -				return i;
>> -		return -EINVAL;
>> -	}
>> -
>> -	if (val >= num_parents)
>> -		return -EINVAL;
>> -
>> -	return val;
>> +	return clk_mux_get_parent(hw, val, mux->table, 0);
>>  }
>>  
>>  static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>> @@ -1036,11 +1023,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>>  }
>>  
>>  static const struct clk_ops lpc32xx_clk_mux_ro_ops = {
>> -	.get_parent = clk_mux_get_parent,
>> +	.get_parent = _clk_mux_get_parent,
>>  };
>>  
>>  static const struct clk_ops lpc32xx_clk_mux_ops = {
>> -	.get_parent = clk_mux_get_parent,
>> +	.get_parent = _clk_mux_get_parent,
>>  	.set_parent = clk_mux_set_parent,
>>  	.determine_rate = __clk_mux_determine_rate,
>>  };
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index f711be6..344ad92 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -488,6 +488,12 @@ struct clk_mux {
>>  extern const struct clk_ops clk_mux_ops;
>>  extern const struct clk_ops clk_mux_ro_ops;
>>  
>> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
>> +				unsigned int *table,
>> +				unsigned long flags);
>> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
>> +			     unsigned long flags);
>> +
>>  struct clk *clk_register_mux(struct device *dev, const char *name,
>>  		const char * const *parent_names, u8 num_parents,
>>  		unsigned long flags,
>> -- 
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: [PATCH v13 1/2] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-25  4:05 UTC (permalink / raw)
  To: Ilia Lin
  Cc: vireshk, nm, sboyd, robh, mark.rutland, rjw, linux-pm, devicetree,
	linux-kernel
In-Reply-To: <DF878CBB-304A-4116-83D5-DF6BCEB7C131@codeaurora.org>

On 25-05-18, 07:00, Ilia Lin wrote:
> 
> 
> On May 25, 2018 6:54:12 AM GMT+03:00, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >On 24-05-18, 18:03, Ilia Lin wrote:
> >> +static int __init qcom_cpufreq_kryo_init(void)
> >> +{
> >> +	struct device_node *np;
> >> +	struct device *cpu_dev;
> >> +	int ret;
> >> +
> >> +	cpu_dev = get_cpu_device(0);
> >> +	if (NULL == cpu_dev)
> >> +		ret = -ENODEV;
> >> +
> >> +	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> >> +	if (IS_ERR(np))
> >> +		return PTR_ERR(np);
> >> +
> >> +	ret = of_device_is_compatible(np, "operating-points-v2-kryo-cpu");
> >> +	of_node_put(np);
> >> +	if (!ret)
> >> +		return -ENOENT;
> >> +
> >
> >I hate the fact that it is taking so long to get done with this. But
> >can't you
> >just check machine compatibility instead of this complicated setup to
> >check OPP
> >node ? Like:
> >
> >        if (!of_device_is_compatible("qcom,apq8096") &&
> >            !of_device_is_compatible("qcom,msm8996"))
> >                return;
> 
> I have to check the "operating-points-v2-kryo-cpu" anyway, so I moved it from probe to the init.

Okay, leave it as is then. Don't send anything yet and wait for Sudeep to
respond.

> >
> >And please see if you can add an entry in MAINTAINERS and add your
> >working email
> >id there.
> 
> Sure. Should this be part of the patch itself?

If you need to send another version of this series, then add it to this patch
itself. Else send it separately and don't resend this stuff.

-- 
viresh

^ permalink raw reply

* Re: [PATCH v13 1/2] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-25  3:54 UTC (permalink / raw)
  To: Ilia Lin
  Cc: vireshk, nm, sboyd, robh, mark.rutland, rjw, linux-pm, devicetree,
	linux-kernel
In-Reply-To: <1527174220-13244-2-git-send-email-ilialin@codeaurora.org>

On 24-05-18, 18:03, Ilia Lin wrote:
> +static int __init qcom_cpufreq_kryo_init(void)
> +{
> +	struct device_node *np;
> +	struct device *cpu_dev;
> +	int ret;
> +
> +	cpu_dev = get_cpu_device(0);
> +	if (NULL == cpu_dev)
> +		ret = -ENODEV;
> +
> +	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> +	if (IS_ERR(np))
> +		return PTR_ERR(np);
> +
> +	ret = of_device_is_compatible(np, "operating-points-v2-kryo-cpu");
> +	of_node_put(np);
> +	if (!ret)
> +		return -ENOENT;
> +

I hate the fact that it is taking so long to get done with this. But can't you
just check machine compatibility instead of this complicated setup to check OPP
node ? Like:

        if (!of_device_is_compatible("qcom,apq8096") &&
            !of_device_is_compatible("qcom,msm8996"))
                return;

And please see if you can add an entry in MAINTAINERS and add your working email
id there.

-- 
viresh

^ permalink raw reply

* Re: [PATCH] cpufreq: reinitialize new policy min/max when writing scaling_(max|min)_freq
From: Wangtao (Kevin, Kirin) @ 2018-05-25  2:54 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Viresh Kumar, Linux PM,
	Linux Kernel Mailing List, gengyanping, sunzhaosheng
In-Reply-To: <CAJZ5v0ih6s9WviSpg9D+kSt2h8T5Vnx-MziOmYuk1H4yZOYLNg@mail.gmail.com>



在 2018/5/24 15:45, Rafael J. Wysocki 写道:
> On Thu, May 24, 2018 at 8:43 AM, Kevin Wangtao
> <kevin.wangtao@hisilicon.com> wrote:
>> consider such situation, current user_policy.min is 1000000,
>> current user_policy.max is 1200000, in cpufreq_set_policy,
>> other driver may update policy.min to 1200000, policy.max to
>> 1300000. After that, If we input "echo 1300000 > scaling_min_freq",
>> then user_policy.min will be 1300000, and user_policy.max is
>> still 1200000, because the input value is checked with policy.max
>> not user_policy.max. if we get all related cpus offline and
>> online again, it will cause cpufreq_init_policy fail because
>> user_policy.min is higher than user_policy.max.
> 
> How do you reproduce this, exactly?
I write a driver register CPUFREQ_POLICY_NOTIFIER, and when event is CPUFREQ_ADJUST,
it will modify policy's min/max according to some conditions, I test it with writing
scaling_(max|min)_freq to traverse all frequencies repeatly, and also repeat hotplug
as background.
> 
>> The solution is when user space tries to write scaling_(max|min)_freq,
>> the min/max of new_policy should be reinitialized with min/max
>> of user_policy, like what cpufreq_update_policy does.
>>
>> Signed-off-by: Kevin Wangtao <kevin.wangtao@hisilicon.com>
>> ---
>>   drivers/cpufreq/cpufreq.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index b79c532..8b33e08 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -697,6 +697,8 @@ static ssize_t store_##file_name                                    \
>>          struct cpufreq_policy new_policy;                               \
>>                                                                          \
>>          memcpy(&new_policy, policy, sizeof(*policy));                   \
>> +       new_policy->min = policy->user_policy.min;                      \
new_policy.min = policy->user_policy.min;
>> +       new_policy->max = policy->user_policy.max;                      \
new_policy.max = policy->user_policy.max
> 
> It looks like you haven't even tried to build this, have you?
sorry for that, I test it on another branch and write this patch manually without build
> 
>>                                                                          \
>>          ret = sscanf(buf, "%u", &new_policy.object);                    \
>>          if (ret != 1)                                                   \
>> --
>> 2.8.1
>>
> 
> .
> 

^ permalink raw reply

* Re: [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-24 21:21 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Rajendra Nayak, Geert Uytterhoeven, Linux PM, Greg Kroah-Hartman,
	Kevin Hilman, Rafael J . Wysocki, Linux Kernel Mailing List,
	Todor Tomov, Viresh Kumar, linux-tegra, Vincent Guittot,
	Linux ARM
In-Reply-To: <96169281-49b2-121f-0b7e-f54aae66db91@nvidia.com>

[...]

>>>
>>> * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>> * @dev: Device to attach.
>>> * @index: The index of the PM domain.
>>>
>>> This naming and description is a bit misleading, because really it is not
>>> attaching the device that is passed, but creating a new device to attach
>>> a
>>> PM domain to. So we should consider renaming and changing the description
>>> and indicate that users need to link the device.
>>
>>
>> I picked the name to be consistent with the existing
>> genpd_dev_pm_attach(). Do you have a better suggestion?
>
>
> Well, it appears to get more of a 'get' function and so I don't see why we
> could not have 'genpd_dev_get_by_id()' and then we could have a
> genpd_dev_put() as well (which would call genpd_dev_pm_detach).
>
>> I agree, some details is missing to the description, let me try to
>> improve it. Actually, I was trying to follow existing descriptions
>> from genpd_dev_pm_attach(), so perhaps that also needs a little
>> update.
>>
>> However, do note that, neither genpd_dev_pm_attach() or
>> genpd_dev_pm_attach_by_id() is supposed to be called by drivers, but
>> rather only by the driver core. So description may not be so
>> important.
>>
>> In regards to good descriptions, for sure the API added in patch9,
>> dev_pm_domain_attach_by_id(), needs a good one, as this is what
>> drivers should be using.
>
>
> OK. Same appears to apply here to the description as I mentioned above.
> Still seems to be more of a 'get' than an attach. So I wonder if it should
> be dev_pm_domain_get_by_id() instead?

Regarding "get" vs "attach", I suggest we continue to discuss that in
patch 9. Whatever is decided, $subject patch needs to follow.

>
>>> Finally, how is a PM domain attached via calling
>>> genpd_dev_pm_attach_by_id()
>>> detached?
>>
>>
>> Via the existing genpd_dev_pm_detach(), according to what I have
>> described in the change log. I clarify the description in regards to
>> this as well.
>
>
> OK, so this bit is a to-do as that is not yet exposed AFAICT. I see that you
> said 'although we need to extend it to cover cleanup of the earlier
> registered device, via calling device_unregister().' So if we do this then
> that would be fine.

Let me clarify the changelog. It's not a to-do, as it's already done
as part of $subject patch.

So I guess we are in agreement that we don't need another API to deal
with detach?

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains
From: Ulf Hansson @ 2018-05-24 21:11 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Rafael J . Wysocki, Linux PM, Greg Kroah-Hartman,
	Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Viresh Kumar,
	Vincent Guittot, Kevin Hilman, Linux Kernel Mailing List,
	Linux ARM, linux-tegra
In-Reply-To: <1c21d18e-954a-f3a8-9817-0117b7cb7e4f@nvidia.com>

On 24 May 2018 at 17:48, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 18/05/18 11:31, Ulf Hansson wrote:
>>
>> The existing dev_pm_domain_attach() function, allows a single PM domain to
>> be attached per device. To be able to support devices that are partitioned
>> across multiple PM domains, let's introduce a new interface,
>> dev_pm_domain_attach_by_id().
>>
>> The dev_pm_domain_attach_by_id() returns a new allocated struct device
>> with
>> the corresponding attached PM domain. This enables for example a driver to
>> operate on the new device from a power management point of view. The
>> driver
>> may then also benefit from using the received device, to set up so called
>> device-links towards its original device. Depending on the situation,
>> these
>> links may then be dynamically changed.
>>
>> The new interface is typically called by drivers during their probe phase,
>> in case they manages devices which uses multiple PM domains. If that is
>> the
>> case, the driver also becomes responsible of managing the detaching of the
>> PM domains, which typically should be done at the remove phase. Detaching
>> is done by calling the existing dev_pm_domain_detach() function and for
>> each of the received devices from dev_pm_domain_attach_by_id().
>>
>> Note, currently its only genpd that supports multiple PM domains per
>> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
>> other PM domain types, if/when needed.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>   drivers/base/power/common.c | 33 ++++++++++++++++++++++++++++++++-
>>   include/linux/pm_domain.h   |  7 +++++++
>>   2 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
>> index 7ae62b6..d3db974 100644
>> --- a/drivers/base/power/common.c
>> +++ b/drivers/base/power/common.c
>> @@ -117,13 +117,44 @@ int dev_pm_domain_attach(struct device *dev, bool
>> power_on)
>>   EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>>     /**
>> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
>
>
> Isn't this more of a 'get'?

I don't think so, at least according to the common understanding of
how we use get and put APIs.

For example, clk_get() returns a cookie to a clk, which you then can
do a hole bunch of different clk specific operations on.

This is different, there are no specific PM domain operations the
caller can or should do. Instead the idea is to keep drivers more or
less transparent, still using runtime PM as before. The only care the
caller need to take is to use device links, which BTW isn't a PM
domain specific thing.

>
>> + * @index: The index of the PM domain.
>> + * @dev: Device to attach.
>
>
> Isn't this just the device associated with the PM domain we are getting?

Correct, but please note, as stated above, I don't think it's a good
idea to return a special PM domain cookie, because we don't want the
caller to run special PM domain operations.

>
>> + *
>> + * As @dev may only be attached to a single PM domain, the backend PM
>> domain
>> + * provider should create a virtual device to attach instead. As
>> attachment
>> + * succeeds, the ->detach() callback in the struct dev_pm_domain should
>> be
>> + * assigned by the corresponding backend attach function.
>> + *
>> + * This function should typically be invoked from drivers during probe
>> phase.
>> + * Especially for those that manages devices which requires power
>> management
>> + * through more than one PM domain.
>> + *
>> + * Callers must ensure proper synchronization of this function with power
>> + * management callbacks.
>> + *
>> + * Returns the virtual attached device in case successfully attached PM
>> domain,
>> + * NULL in case @dev don't need a PM domain, else a PTR_ERR().
>
>
> Should this be 'NULL in the case where the @dev already has a power-domain'?

Yes, I think so. The intent is to be consistent with how
dev_pm_domain_attach() works and according to the latest changes I did
for it. It's queued for 4.18, please have a look in Rafael's tree and
you will see.

>
>> + */
>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>> +                                         unsigned int index)
>> +{
>> +       if (dev->pm_domain)
>
>
> I wonder if this is worthy of a ...
>
>         if (WARN_ON(dev->pm_domain))
>
>> +               return NULL;
>
>
> Don't we consider this an error case? I wonder why not return PTR_ERR here
> as well? This would be consistent with dev_pm_domain_attach().

Please see above comment.

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH v3 21/27] x86/ftrace: Adapt function tracing for PIE support
From: Thomas Garnier via Virtualization @ 2018-05-24 20:41 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Kate Stewart, Nicolas Pitre, the arch/x86 maintainers,
	Sergey Senozhatsky, Len Brown, Tom Lendacky, Peter Zijlstra,
	Yonghong Song, Christopher Li, Dave Hansen, Dominik Brodowski,
	LKML, Masahiro Yamada, Jan Beulich, Pavel Machek, H . Peter Anvin,
	Kernel Hardening, Christoph Lameter, Alok Kataria,
	Linux Doc Mailing List, linux-arch, Jonathan Corbet <corbet@
In-Reply-To: <20180524161622.4202319f@gandalf.local.home>

On Thu, May 24, 2018 at 1:16 PM Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 24 May 2018 13:40:24 +0200
> Petr Mladek <pmladek@suse.com> wrote:

> > On Wed 2018-05-23 12:54:15, Thomas Garnier wrote:
> > > When using -fPIE/PIC with function tracing, the compiler generates a
> > > call through the GOT (call *__fentry__@GOTPCREL). This instruction
> > > takes 6 bytes instead of 5 on the usual relative call.
> > >
> > > If PIE is enabled, replace the 6th byte of the GOT call by a 1-byte
nop
> > > so ftrace can handle the previous 5-bytes as before.
> > >
> > > Position Independent Executable (PIE) support will allow to extended
the
> > > KASLR randomization range below the -2G memory limit.
> > >
> > > Signed-off-by: Thomas Garnier <thgarnie@google.com>
> > > ---
> > >  arch/x86/include/asm/ftrace.h   |  6 +++--
> > >  arch/x86/include/asm/sections.h |  4 ++++
> > >  arch/x86/kernel/ftrace.c        | 42
+++++++++++++++++++++++++++++++--
> > >  3 files changed, 48 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/x86/include/asm/ftrace.h
b/arch/x86/include/asm/ftrace.h
> > > index c18ed65287d5..8f2decce38d8 100644
> > > --- a/arch/x86/include/asm/ftrace.h
> > > +++ b/arch/x86/include/asm/ftrace.h
> > > @@ -25,9 +25,11 @@ extern void __fentry__(void);
> > >  static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > >  {
> > >     /*
> > > -    * addr is the address of the mcount call instruction.
> > > -    * recordmcount does the necessary offset calculation.
> > > +    * addr is the address of the mcount call instruction. PIE has
always a
> > > +    * byte added to the start of the function.
> > >      */
> > > +   if (IS_ENABLED(CONFIG_X86_PIE))
> > > +           addr -= 1;
> >
> > This seems to modify the address even for modules that are _not_
compiled with
> > PIE, see below.

> Can one load a module not compiled for PIE in a kernel with PIE?

> >
> > >     return addr;
> > >  }
> > >
> > > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > > index 01ebcb6f263e..73b3c30cb7a3 100644
> > > --- a/arch/x86/kernel/ftrace.c
> > > +++ b/arch/x86/kernel/ftrace.c
> > > @@ -135,6 +135,44 @@ ftrace_modify_code_direct(unsigned long ip,
unsigned const char *old_code,
> > >     return 0;
> > >  }
> > >
> > > +/* Bytes before call GOT offset */
> > > +const unsigned char got_call_preinsn[] = { 0xff, 0x15 };
> > > +
> > > +static int
> > > +ftrace_modify_initial_code(unsigned long ip, unsigned const char
*old_code,
> > > +                      unsigned const char *new_code)
> > > +{
> > > +   unsigned char replaced[MCOUNT_INSN_SIZE + 1];
> > > +
> > > +   ftrace_expected = old_code;
> > > +
> > > +   /*
> > > +    * If PIE is not enabled or no GOT call was found, default to the
> > > +    * original approach to code modification.
> > > +    */
> > > +   if (!IS_ENABLED(CONFIG_X86_PIE) ||
> > > +       probe_kernel_read(replaced, (void *)ip, sizeof(replaced)) ||
> > > +       memcmp(replaced, got_call_preinsn, sizeof(got_call_preinsn)))
> > > +           return ftrace_modify_code_direct(ip, old_code, new_code);
> >
> > And this looks like an attempt to handle modules compiled without
> > PIE. Does it works with the right ip in that case?

> I'm guessing the || is for the "or no GOT call was found", but it
> doesn't explain why no GOT would be found.

Yes, maybe I could have made it work by using text_ip_addr earlier.


> >
> > I wonder if a better solution would be to update
> > scripts/recordmcount.c to store the incremented location into the
module.

I will look into it.


> If recordmcount.c can handle this, then I think that's the preferred
> approach. Thanks!

> -- Steve

> >
> > IMPORTANT: I have only vague picture about how this all works. It is
> > possible that I am completely wrong. The code might be correct,
> > especially if you tested this situation.
> >
> > Best Regards,
> > Petr



-- 
Thomas

^ permalink raw reply

* Re: [PATCH v3 21/27] x86/ftrace: Adapt function tracing for PIE support
From: Steven Rostedt @ 2018-05-24 20:16 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Kate Stewart, Nicolas Pitre, x86, Sergey Senozhatsky, kvm,
	Len Brown, Peter Zijlstra, Yonghong Song, Christopher Li,
	Dave Hansen, Dominik Brodowski, linux-kernel, Masahiro Yamada,
	Jan Beulich, Pavel Machek, H . Peter Anvin, kernel-hardening,
	Christoph Lameter, Alok Kataria, linux-doc, linux-arch,
	Jonathan Corbet, Herbert Xu, Baoquan He, David Woodhouse
In-Reply-To: <20180524114024.pa67zjipy5qcg4tm@pathway.suse.cz>

On Thu, 24 May 2018 13:40:24 +0200
Petr Mladek <pmladek@suse.com> wrote:

> On Wed 2018-05-23 12:54:15, Thomas Garnier wrote:
> > When using -fPIE/PIC with function tracing, the compiler generates a
> > call through the GOT (call *__fentry__@GOTPCREL). This instruction
> > takes 6 bytes instead of 5 on the usual relative call.
> > 
> > If PIE is enabled, replace the 6th byte of the GOT call by a 1-byte nop
> > so ftrace can handle the previous 5-bytes as before.
> > 
> > Position Independent Executable (PIE) support will allow to extended the
> > KASLR randomization range below the -2G memory limit.
> > 
> > Signed-off-by: Thomas Garnier <thgarnie@google.com>
> > ---
> >  arch/x86/include/asm/ftrace.h   |  6 +++--
> >  arch/x86/include/asm/sections.h |  4 ++++
> >  arch/x86/kernel/ftrace.c        | 42 +++++++++++++++++++++++++++++++--
> >  3 files changed, 48 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> > index c18ed65287d5..8f2decce38d8 100644
> > --- a/arch/x86/include/asm/ftrace.h
> > +++ b/arch/x86/include/asm/ftrace.h
> > @@ -25,9 +25,11 @@ extern void __fentry__(void);
> >  static inline unsigned long ftrace_call_adjust(unsigned long addr)
> >  {
> >  	/*
> > -	 * addr is the address of the mcount call instruction.
> > -	 * recordmcount does the necessary offset calculation.
> > +	 * addr is the address of the mcount call instruction. PIE has always a
> > +	 * byte added to the start of the function.
> >  	 */
> > +	if (IS_ENABLED(CONFIG_X86_PIE))
> > +		addr -= 1;  
> 
> This seems to modify the address even for modules that are _not_ compiled with
> PIE, see below.

Can one load a module not compiled for PIE in a kernel with PIE?

> 
> >  	return addr;
> >  }
> >  
> > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > index 01ebcb6f263e..73b3c30cb7a3 100644
> > --- a/arch/x86/kernel/ftrace.c
> > +++ b/arch/x86/kernel/ftrace.c
> > @@ -135,6 +135,44 @@ ftrace_modify_code_direct(unsigned long ip, unsigned const char *old_code,
> >  	return 0;
> >  }
> >  
> > +/* Bytes before call GOT offset */
> > +const unsigned char got_call_preinsn[] = { 0xff, 0x15 };
> > +
> > +static int
> > +ftrace_modify_initial_code(unsigned long ip, unsigned const char *old_code,
> > +			   unsigned const char *new_code)
> > +{
> > +	unsigned char replaced[MCOUNT_INSN_SIZE + 1];
> > +
> > +	ftrace_expected = old_code;
> > +
> > +	/*
> > +	 * If PIE is not enabled or no GOT call was found, default to the
> > +	 * original approach to code modification.
> > +	 */
> > +	if (!IS_ENABLED(CONFIG_X86_PIE) ||
> > +	    probe_kernel_read(replaced, (void *)ip, sizeof(replaced)) ||
> > +	    memcmp(replaced, got_call_preinsn, sizeof(got_call_preinsn)))
> > +		return ftrace_modify_code_direct(ip, old_code, new_code);  
> 
> And this looks like an attempt to handle modules compiled without
> PIE. Does it works with the right ip in that case?

I'm guessing the || is for the "or no GOT call was found", but it
doesn't explain why no GOT would be found.

> 
> I wonder if a better solution would be to update
> scripts/recordmcount.c to store the incremented location into the module.

If recordmcount.c can handle this, then I think that's the preferred
approach. Thanks!

-- Steve

> 
> IMPORTANT: I have only vague picture about how this all works. It is
> possible that I am completely wrong. The code might be correct,
> especially if you tested this situation.
> 
> Best Regards,
> Petr

^ permalink raw reply

* Re: [PATCH] cpufreq / CPPC: Add cpuinfo_cur_freq support for CPPC
From: Prakash, Prashanth @ 2018-05-24 19:25 UTC (permalink / raw)
  To: George Cherian, linux-kernel, linux-pm; +Cc: rjw, viresh.kumar
In-Reply-To: <1526989324-4183-1-git-send-email-george.cherian@cavium.com>

Hi George,

On 5/22/2018 5:42 AM, George Cherian wrote:
> Per Section 8.4.7.1.3 of ACPI 6.2, The platform provides performance
> feedback via set of performance counters. To determine the actual
> performance level delivered over time, OSPM may read a set of
> performance counters from the Reference Performance Counter Register
> and the Delivered Performance Counter Register.
>
> OSPM calculates the delivered performance over a given time period by
> taking a beginning and ending snapshot of both the reference and
> delivered performance counters, and calculating:
>
> delivered_perf = reference_perf X (delta of delivered_perf counter / delta of reference_perf counter).
>
> Implement the above and hook this to the cpufreq->get method.
>
> Signed-off-by: George Cherian <george.cherian@cavium.com>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 44 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index b15115a..a046915 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -240,10 +240,54 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	return ret;
>  }
>  
> +static int cppc_get_rate_from_fbctrs(struct cppc_perf_fb_ctrs fb_ctrs_t0,
> +				     struct cppc_perf_fb_ctrs fb_ctrs_t1)
> +{
> +	u64 delta_reference, delta_delivered;
> +	u64 reference_perf, ratio;
> +
> +	reference_perf = fb_ctrs_t0.reference_perf;
> +	if (fb_ctrs_t1.reference > fb_ctrs_t0.reference)
> +		delta_reference = fb_ctrs_t1.reference - fb_ctrs_t0.reference;
> +	else /* Counters would have wrapped-around */
> +		delta_reference  = ((u64)(~((u64)0)) - fb_ctrs_t0.reference) +
> +					fb_ctrs_t1.reference;
> +
> +	if (fb_ctrs_t1.delivered > fb_ctrs_t0.delivered)
> +		delta_delivered = fb_ctrs_t1.delivered - fb_ctrs_t0.delivered;
> +	else /* Counters would have wrapped-around */
> +		delta_delivered  = ((u64)(~((u64)0)) - fb_ctrs_t0.delivered) +
> +					fb_ctrs_t1.delivered;
We need to check that the wraparound time is long enough to make sure that
the counters cannot wrap around more than once. We can register a  get() api
only after checking that wraparound time value is reasonably high.

I am not aware of any platforms where wraparound time is soo short, but
wouldn't hurt to check once during init.
> +
> +	if (delta_reference)  /* Check to avoid divide-by zero */
> +		ratio = (delta_delivered * 1000) / delta_reference;
Why not just return the computed value here instead of *1000 and later /1000?
return (ref_per * delta_del) / delta_ref;
> +	else
> +		return -EINVAL;
Instead of EINVAL, i think we should return current frequency.

The counters can pause if CPUs are in idle state during our sampling interval, so
If the counters did not progress, it is reasonable to assume the delivered perf was
equal to desired perf.

Even if platform wanted to limit, since the CPUs were asleep(idle) we could not have
observed lower performance, so we will not throw off  any logic that could be driven
using the returned value.
> +
> +	return (reference_perf * ratio) / 1000;
This should be converted to KHz as cpufreq is not aware of CPPC abstract scale
> +}
> +
> +static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
> +{
> +	struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
> +	int ret;
> +
> +	ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t0);
> +	if (ret)
> +		return ret;
> +
> +	ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t1);
> +	if (ret)
> +		return ret;
> +
> +	return cppc_get_rate_from_fbctrs(fb_ctrs_t0, fb_ctrs_t1);
> +}
We need to make sure that we get a reasonably sample so make sure the reported
performance is accurate.
The counters can capture short transient throttling/limiting, so by sampling a really
short duration of time we could return quite inaccurate measure of performance.

We need to include some reasonable delay between the two calls. What is reasonable
is debatable - so this can be few(2-10) microseconds defined as default. If the same value
doesn't work for all the platforms, we might need to add a platform specific value.

> +
>  static struct cpufreq_driver cppc_cpufreq_driver = {
>  	.flags = CPUFREQ_CONST_LOOPS,
>  	.verify = cppc_verify_policy,
>  	.target = cppc_cpufreq_set_target,
> +	.get = cppc_cpufreq_get_rate,
>  	.init = cppc_cpufreq_cpu_init,
>  	.stop_cpu = cppc_cpufreq_stop_cpu,
>  	.name = "cppc_cpufreq",

^ permalink raw reply

* [PATCH v2 8/8] PM / Domains: Stop deferring probe at the end of initcall
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
	Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
  Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel,
	Pavel Machek, Len Brown, linux-pm
In-Reply-To: <20180524175024.19874-1-robh@kernel.org>

All PM domain drivers must be built-in (at least those using DT), so
there is no point deferring probe after initcalls are done. Continuing
to defer probe may prevent booting successfully even if managing PM
domains is not required. This can happen if the user failed to enable
the driver or if power-domains are added to a platform's DT, but there
is not yet a driver (e.g. a new DTB with an old kernel).

Call the driver core function driver_deferred_probe_check_init_done()
instead of just returning -EPROBE_DEFER to stop deferring probe when
initcalls are done.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 1ea0e2502e8e..6398cf786e6a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2218,7 +2218,7 @@ int genpd_dev_pm_attach(struct device *dev)
 		mutex_unlock(&gpd_list_lock);
 		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
 			__func__, PTR_ERR(pd));
-		return -EPROBE_DEFER;
+		return driver_deferred_probe_check_init_done(dev, true);
 	}
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH v9 01/15] ARM: Add Krait L2 register accessor functions
From: Bjorn Andersson @ 2018-05-24 17:39 UTC (permalink / raw)
  To: Sricharan R
  Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
	andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
	linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
	linux
In-Reply-To: <1520347148-27852-2-git-send-email-sricharan@codeaurora.org>

On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:

> From: Stephen Boyd <sboyd@codeaurora.org>
> 
> Krait CPUs have a handful of L2 cache controller registers that
> live behind a cp15 based indirection register. First you program
> the indirection register (l2cpselr) to point the L2 'window'
> register (l2cpdr) at what you want to read/write.  Then you
> read/write the 'window' register to do what you want. The
> l2cpselr register is not banked per-cpu so we must lock around
> accesses to it to prevent other CPUs from re-pointing l2cpdr
> underneath us.
> 
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

This should have your signed-off-by here as well.

Apart from that:

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  arch/arm/common/Kconfig                   |  3 ++
>  arch/arm/common/Makefile                  |  1 +
>  arch/arm/common/krait-l2-accessors.c      | 48 +++++++++++++++++++++++++++++++
>  arch/arm/include/asm/krait-l2-accessors.h | 10 +++++++
>  4 files changed, 62 insertions(+)
>  create mode 100644 arch/arm/common/krait-l2-accessors.c
>  create mode 100644 arch/arm/include/asm/krait-l2-accessors.h
> 
> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
> index e5ad070..c8e1986 100644
> --- a/arch/arm/common/Kconfig
> +++ b/arch/arm/common/Kconfig
> @@ -7,6 +7,9 @@ config DMABOUNCE
>  	bool
>  	select ZONE_DMA
>  
> +config KRAIT_L2_ACCESSORS
> +	bool
> +
>  config SHARP_LOCOMO
>  	bool
>  
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index 70b4a14..eec6cd1 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -7,6 +7,7 @@ obj-y				+= firmware.o
>  
>  obj-$(CONFIG_SA1111)		+= sa1111.o
>  obj-$(CONFIG_DMABOUNCE)		+= dmabounce.o
> +obj-$(CONFIG_KRAIT_L2_ACCESSORS) += krait-l2-accessors.o
>  obj-$(CONFIG_SHARP_LOCOMO)	+= locomo.o
>  obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
>  obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
> diff --git a/arch/arm/common/krait-l2-accessors.c b/arch/arm/common/krait-l2-accessors.c
> new file mode 100644
> index 0000000..9a97dda
> --- /dev/null
> +++ b/arch/arm/common/krait-l2-accessors.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, The Linux Foundation. All rights reserved.
> +
> +#include <linux/spinlock.h>
> +#include <linux/export.h>
> +
> +#include <asm/barrier.h>
> +#include <asm/krait-l2-accessors.h>
> +
> +static DEFINE_RAW_SPINLOCK(krait_l2_lock);
> +
> +void krait_set_l2_indirect_reg(u32 addr, u32 val)
> +{
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&krait_l2_lock, flags);
> +	/*
> +	 * Select the L2 window by poking l2cpselr, then write to the window
> +	 * via l2cpdr.
> +	 */
> +	asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
> +	isb();
> +	asm volatile ("mcr p15, 3, %0, c15, c0, 7 @ l2cpdr" : : "r" (val));
> +	isb();
> +
> +	raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
> +}
> +EXPORT_SYMBOL(krait_set_l2_indirect_reg);
> +
> +u32 krait_get_l2_indirect_reg(u32 addr)
> +{
> +	u32 val;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&krait_l2_lock, flags);
> +	/*
> +	 * Select the L2 window by poking l2cpselr, then read from the window
> +	 * via l2cpdr.
> +	 */
> +	asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
> +	isb();
> +	asm volatile ("mrc p15, 3, %0, c15, c0, 7 @ l2cpdr" : "=r" (val));
> +
> +	raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
> +
> +	return val;
> +}
> +EXPORT_SYMBOL(krait_get_l2_indirect_reg);
> diff --git a/arch/arm/include/asm/krait-l2-accessors.h b/arch/arm/include/asm/krait-l2-accessors.h
> new file mode 100644
> index 0000000..dd7c474
> --- /dev/null
> +++ b/arch/arm/include/asm/krait-l2-accessors.h
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, The Linux Foundation. All rights reserved.
> +
> +#ifndef __ASMARM_KRAIT_L2_ACCESSORS_H
> +#define __ASMARM_KRAIT_L2_ACCESSORS_H
> +
> +extern void krait_set_l2_indirect_reg(u32 addr, u32 val);
> +extern u32 krait_get_l2_indirect_reg(u32 addr);
> +
> +#endif
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply

* Re: [PATCH v9 02/15] clk: mux: Split out register accessors for reuse
From: Bjorn Andersson @ 2018-05-24 16:50 UTC (permalink / raw)
  To: Sricharan R
  Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
	andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
	linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
	linux
In-Reply-To: <1520347148-27852-3-git-send-email-sricharan@codeaurora.org>

On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:

> From: Stephen Boyd <sboyd@codeaurora.org>
> 
> We want to reuse the logic in clk-mux.c for other clock drivers
> that don't use readl as register accessors. Fortunately, there
> really isn't much to the mux code besides the table indirection
> and quirk flags if you assume any bit shifting and masking has
> been done already. Pull that logic out into reusable functions
> that operate on an optional table and some flags so that other
> drivers can use the same logic.
> 
> [Sricharan: Rebased for mainline]
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

This should read as a log, where the first entry is Stephen stating that
he acquired or wrote the code and can release it according to the
license requirements. Then you state that you acquired it, changed it
and are releasing it according to the license requirements.


PS. Please expand your last name.

Regards,
Bjorn

> ---
>  drivers/clk/clk-mux.c         | 74 +++++++++++++++++++++++++++----------------
>  drivers/clk/nxp/clk-lpc32xx.c | 21 +++---------
>  include/linux/clk-provider.h  |  6 ++++
>  3 files changed, 57 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 39cabe1..28223fa 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -26,35 +26,25 @@
>   * parent - parent is adjustable through clk_set_parent
>   */
>  
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> +				unsigned int *table,
> +				unsigned long flags)
>  {
> -	struct clk_mux *mux = to_clk_mux(hw);
>  	int num_parents = clk_hw_get_num_parents(hw);
> -	u32 val;
> -
> -	/*
> -	 * FIXME need a mux-specific flag to determine if val is bitwise or numeric
> -	 * e.g. sys_clkin_ck's clksel field is 3 bits wide, but ranges from 0x1
> -	 * to 0x7 (index starts at one)
> -	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> -	 * val = 0x4 really means "bit 2, index starts at bit 0"
> -	 */
> -	val = clk_readl(mux->reg) >> mux->shift;
> -	val &= mux->mask;
>  
> -	if (mux->table) {
> +	if (table) {
>  		int i;
>  
>  		for (i = 0; i < num_parents; i++)
> -			if (mux->table[i] == val)
> +			if (table[i] == val)
>  				return i;
>  		return -EINVAL;
>  	}
>  
> -	if (val && (mux->flags & CLK_MUX_INDEX_BIT))
> +	if (val && (flags & CLK_MUX_INDEX_BIT))
>  		val = ffs(val) - 1;
>  
> -	if (val && (mux->flags & CLK_MUX_INDEX_ONE))
> +	if (val && (flags & CLK_MUX_INDEX_ONE))
>  		val--;
>  
>  	if (val >= num_parents)
> @@ -62,23 +52,53 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
>  
>  	return val;
>  }
> +EXPORT_SYMBOL_GPL(clk_mux_get_parent);
>  
> -static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>  {
>  	struct clk_mux *mux = to_clk_mux(hw);
>  	u32 val;
> -	unsigned long flags = 0;
>  
> -	if (mux->table) {
> -		index = mux->table[index];
> +	/*
> +	 * FIXME need a mux-specific flag to determine if val is bitwise or
> +	 * numeric e.g. sys_clkin_ck's clksel field is 3 bits wide,
> +	 * but ranges from 0x1 to 0x7 (index starts at one)
> +	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> +	 * val = 0x4 really means "bit 2, index starts at bit 0"
> +	 */
> +	val = clk_readl(mux->reg) >> mux->shift;
> +	val &= mux->mask;
> +
> +	return clk_mux_get_parent(hw, val, mux->table, mux->flags);
> +}
> +
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> +			     unsigned long flags)
> +{
> +	unsigned int val = index;
> +
> +	if (table) {
> +		val = table[val];
>  	} else {
> -		if (mux->flags & CLK_MUX_INDEX_BIT)
> -			index = 1 << index;
> +		if (flags & CLK_MUX_INDEX_BIT)
> +			val = 1 << index;
>  
> -		if (mux->flags & CLK_MUX_INDEX_ONE)
> -			index++;
> +		if (flags & CLK_MUX_INDEX_ONE)
> +			val++;
>  	}
>  
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(clk_mux_reindex);
> +
> +static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> +	struct clk_mux *mux = to_clk_mux(hw);
> +	u32 val;
> +	unsigned long flags = 0;
> +
> +	index = clk_mux_reindex(index, mux->table, mux->flags);
> +
>  	if (mux->lock)
>  		spin_lock_irqsave(mux->lock, flags);
>  	else
> @@ -102,14 +122,14 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  }
>  
>  const struct clk_ops clk_mux_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  	.set_parent = clk_mux_set_parent,
>  	.determine_rate = __clk_mux_determine_rate,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ops);
>  
>  const struct clk_ops clk_mux_ro_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
>  
> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
> index f5d815f..9b34150 100644
> --- a/drivers/clk/nxp/clk-lpc32xx.c
> +++ b/drivers/clk/nxp/clk-lpc32xx.c
> @@ -999,29 +999,16 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>  	.set_rate = clk_divider_set_rate,
>  };
>  
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
>  {
>  	struct lpc32xx_clk_mux *mux = to_lpc32xx_mux(hw);
> -	u32 num_parents = clk_hw_get_num_parents(hw);
>  	u32 val;
>  
>  	regmap_read(clk_regmap, mux->reg, &val);
>  	val >>= mux->shift;
>  	val &= mux->mask;
>  
> -	if (mux->table) {
> -		u32 i;
> -
> -		for (i = 0; i < num_parents; i++)
> -			if (mux->table[i] == val)
> -				return i;
> -		return -EINVAL;
> -	}
> -
> -	if (val >= num_parents)
> -		return -EINVAL;
> -
> -	return val;
> +	return clk_mux_get_parent(hw, val, mux->table, 0);
>  }
>  
>  static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> @@ -1036,11 +1023,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  }
>  
>  static const struct clk_ops lpc32xx_clk_mux_ro_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  };
>  
>  static const struct clk_ops lpc32xx_clk_mux_ops = {
> -	.get_parent = clk_mux_get_parent,
> +	.get_parent = _clk_mux_get_parent,
>  	.set_parent = clk_mux_set_parent,
>  	.determine_rate = __clk_mux_determine_rate,
>  };
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index f711be6..344ad92 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -488,6 +488,12 @@ struct clk_mux {
>  extern const struct clk_ops clk_mux_ops;
>  extern const struct clk_ops clk_mux_ro_ops;
>  
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> +				unsigned int *table,
> +				unsigned long flags);
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> +			     unsigned long flags);
> +
>  struct clk *clk_register_mux(struct device *dev, const char *name,
>  		const char * const *parent_names, u8 num_parents,
>  		unsigned long flags,
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply

* Re: [PATCH v3 11/27] x86/power/64: Adapt assembly for PIE support
From: Thomas Garnier via Virtualization @ 2018-05-24 16:37 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Kate Stewart, Nicolas Pitre, the arch/x86 maintainers,
	Sergey Senozhatsky, Petr Mladek, Len Brown, Peter Zijlstra,
	Yonghong Song, Christopher Li, Dave Hansen, Dominik Brodowski,
	LKML, Masahiro Yamada, Jan Beulich, H . Peter Anvin,
	Kernel Hardening, Christoph Lameter, Alok Kataria,
	Linux Doc Mailing List, linux-arch, Jonathan Corbet, Herbert Xu
In-Reply-To: <20180524110341.GB20225@amd>

On Thu, May 24, 2018 at 4:04 AM Pavel Machek <pavel@ucw.cz> wrote:

> On Wed 2018-05-23 12:54:05, Thomas Garnier wrote:
> > Change the assembly code to use only relative references of symbols for
the
> > kernel to be PIE compatible.
> >
> > Position Independent Executable (PIE) support will allow to extended the
> > KASLR randomization range below the -2G memory limit.
> >
> > Signed-off-by: Thomas Garnier <thgarnie@google.com>

> Again, was this tested?

Hibernation was tested as much as I can with qemu and my dedicated machine.
Any specific test you think I should use?


> > diff --git a/arch/x86/power/hibernate_asm_64.S
b/arch/x86/power/hibernate_asm_64.S
> > index ce8da3a0412c..6fdd7bbc3c33 100644
> > --- a/arch/x86/power/hibernate_asm_64.S
> > +++ b/arch/x86/power/hibernate_asm_64.S
> > @@ -24,7 +24,7 @@
> >  #include <asm/frame.h>
> >
> >  ENTRY(swsusp_arch_suspend)
> > -     movq    $saved_context, %rax
> > +     leaq    saved_context(%rip), %rax
> >       movq    %rsp, pt_regs_sp(%rax)
> >       movq    %rbp, pt_regs_bp(%rax)
> >       movq    %rsi, pt_regs_si(%rax)
> > @@ -115,7 +115,7 @@ ENTRY(restore_registers)
> >       movq    %rax, %cr4;  # turn PGE back on
> >
> >       /* We don't restore %rax, it must be 0 anyway */
> > -     movq    $saved_context, %rax
> > +     leaq    saved_context(%rip), %rax
> >       movq    pt_regs_sp(%rax), %rsp
> >       movq    pt_regs_bp(%rax), %rbp
> >       movq    pt_regs_si(%rax), %rsi

> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html



-- 
Thomas

^ permalink raw reply

* Re: [PATCH v3 09/27] x86/acpi: Adapt assembly for PIE support
From: Thomas Garnier @ 2018-05-24 16:35 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Herbert Xu, David S . Miller, Thomas Gleixner, Ingo Molnar,
	H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf, Greg KH,
	Philippe Ombredanne, Kate Stewart, Arnaldo Carvalho de Melo,
	Yonghong Song, Andrey Ryabinin, Kees Cook, Tom Lendacky,
	Kirill A. Shutemov, Andy Lutomirski, Dominik Brodowski,
	Borislav Petkov, Borislav Petkov, Rafael J. Wysocki, Len Brown,
	Juergen Gross <jgros>
In-Reply-To: <20180524110306.GA20225@amd>

On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:

> On Wed 2018-05-23 12:54:03, Thomas Garnier wrote:
> > Change the assembly code to use only relative references of symbols for
the
> > kernel to be PIE compatible.
> >
> > Position Independent Executable (PIE) support will allow to extended the
> > KASLR randomization range below the -2G memory limit.

> What testing did this get?

Tested boot, hibernation and performance on qemu and dedicated machine.


> > diff --git a/arch/x86/kernel/acpi/wakeup_64.S
b/arch/x86/kernel/acpi/wakeup_64.S
> > index 50b8ed0317a3..472659c0f811 100644
> > --- a/arch/x86/kernel/acpi/wakeup_64.S
> > +++ b/arch/x86/kernel/acpi/wakeup_64.S
> > @@ -14,7 +14,7 @@
> >        * Hooray, we are in Long 64-bit mode (but still running in low
memory)
> >        */
> >  ENTRY(wakeup_long64)
> > -     movq    saved_magic, %rax
> > +     movq    saved_magic(%rip), %rax
> >       movq    $0x123456789abcdef0, %rdx
> >       cmpq    %rdx, %rax
> >       jne     bogus_64_magic

> Because, as comment says, this is rather tricky code.

I agree, I think maintainers feedback is very important for this patchset.


Pavel

> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html



-- 
Thomas

^ permalink raw reply

* Re: [PATCH RFC] suspend: Prevent might sleep splats
From: Rafael J. Wysocki @ 2018-05-24 15:57 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Linux PM,
	Thomas Gleixner, Rafael J. Wysocki, Pavel Machek, Len Brown
In-Reply-To: <20180524154505.d5r77w7xinpbc36n@linutronix.de>

On Thu, May 24, 2018 at 5:45 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> On 2018-05-24 17:07:16 [+0200], Rafael J. Wysocki wrote:
>> On Thu, May 24, 2018 at 4:24 PM, Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de> wrote:
>> > From: Thomas Gleixner <tglx@linutronix.de>
>> >
>> > timekeeping suspend/resume calls read_persistent_clock() which takes
>> > rtc_lock. That results in might sleep warnings because at that point
>> > we run with interrupts disabled.
>> >
>> > We cannot convert rtc_lock to a raw spinlock as that would trigger
>> > other might sleep warnings.
>> >
>> > As a workaround we disable the might sleep warnings by setting
>> > system_state to SYSTEM_SUSPEND before calling sysdev_suspend() and
>> > restoring it to SYSTEM_RUNNING afer sysdev_resume().
>> >
>> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>
>> Hmm.  Don't we also need to cover suspend-to-idle?
>
> Well, if you agree with the approach then I would look into it.

As long as the SYSTEM_SUSPEND system state is defined unambiguously, I
don't have a problem with doing this.

^ permalink raw reply

* Re: [PATCH 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains
From: Jon Hunter @ 2018-05-24 15:48 UTC (permalink / raw)
  To: Ulf Hansson, Rafael J . Wysocki, linux-pm
  Cc: Greg Kroah-Hartman, Geert Uytterhoeven, Todor Tomov,
	Rajendra Nayak, Viresh Kumar, Vincent Guittot, Kevin Hilman,
	linux-kernel, linux-arm-kernel, linux-tegra
In-Reply-To: <1526639490-12167-10-git-send-email-ulf.hansson@linaro.org>


On 18/05/18 11:31, Ulf Hansson wrote:
> The existing dev_pm_domain_attach() function, allows a single PM domain to
> be attached per device. To be able to support devices that are partitioned
> across multiple PM domains, let's introduce a new interface,
> dev_pm_domain_attach_by_id().
> 
> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
> the corresponding attached PM domain. This enables for example a driver to
> operate on the new device from a power management point of view. The driver
> may then also benefit from using the received device, to set up so called
> device-links towards its original device. Depending on the situation, these
> links may then be dynamically changed.
> 
> The new interface is typically called by drivers during their probe phase,
> in case they manages devices which uses multiple PM domains. If that is the
> case, the driver also becomes responsible of managing the detaching of the
> PM domains, which typically should be done at the remove phase. Detaching
> is done by calling the existing dev_pm_domain_detach() function and for
> each of the received devices from dev_pm_domain_attach_by_id().
> 
> Note, currently its only genpd that supports multiple PM domains per
> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
> other PM domain types, if/when needed.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>   drivers/base/power/common.c | 33 ++++++++++++++++++++++++++++++++-
>   include/linux/pm_domain.h   |  7 +++++++
>   2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
> index 7ae62b6..d3db974 100644
> --- a/drivers/base/power/common.c
> +++ b/drivers/base/power/common.c
> @@ -117,13 +117,44 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>   EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>   
>   /**
> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.

Isn't this more of a 'get'?

> + * @index: The index of the PM domain.
> + * @dev: Device to attach.

Isn't this just the device associated with the PM domain we are getting?

> + *
> + * As @dev may only be attached to a single PM domain, the backend PM domain
> + * provider should create a virtual device to attach instead. As attachment
> + * succeeds, the ->detach() callback in the struct dev_pm_domain should be
> + * assigned by the corresponding backend attach function.
> + *
> + * This function should typically be invoked from drivers during probe phase.
> + * Especially for those that manages devices which requires power management
> + * through more than one PM domain.
> + *
> + * Callers must ensure proper synchronization of this function with power
> + * management callbacks.
> + *
> + * Returns the virtual attached device in case successfully attached PM domain,
> + * NULL in case @dev don't need a PM domain, else a PTR_ERR().

Should this be 'NULL in the case where the @dev already has a power-domain'?

> + */
> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +					  unsigned int index)
> +{
> +	if (dev->pm_domain)

I wonder if this is worthy of a ...

	if (WARN_ON(dev->pm_domain))

> +		return NULL;

Don't we consider this an error case? I wonder why not return PTR_ERR 
here as well? This would be consistent with dev_pm_domain_attach().

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* Re: [PATCH RFC] suspend: Prevent might sleep splats
From: Sebastian Andrzej Siewior @ 2018-05-24 15:45 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux Kernel Mailing List, Linux PM, Thomas Gleixner,
	Rafael J. Wysocki, Pavel Machek, Len Brown
In-Reply-To: <CAJZ5v0jyGVP4h6B=wkhHb7UV2yePHZVjkzodj0Djx-gUBxyMkA@mail.gmail.com>

On 2018-05-24 17:07:16 [+0200], Rafael J. Wysocki wrote:
> On Thu, May 24, 2018 at 4:24 PM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > From: Thomas Gleixner <tglx@linutronix.de>
> >
> > timekeeping suspend/resume calls read_persistent_clock() which takes
> > rtc_lock. That results in might sleep warnings because at that point
> > we run with interrupts disabled.
> >
> > We cannot convert rtc_lock to a raw spinlock as that would trigger
> > other might sleep warnings.
> >
> > As a workaround we disable the might sleep warnings by setting
> > system_state to SYSTEM_SUSPEND before calling sysdev_suspend() and
> > restoring it to SYSTEM_RUNNING afer sysdev_resume().
> >
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Hmm.  Don't we also need to cover suspend-to-idle?

Well, if you agree with the approach then I would look into it.

Sebastian

^ permalink raw reply

* Re: [PATCH RFC] suspend: Prevent might sleep splats
From: Rafael J. Wysocki @ 2018-05-24 15:07 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Linux Kernel Mailing List, Linux PM, Thomas Gleixner,
	Rafael J. Wysocki, Pavel Machek, Len Brown
In-Reply-To: <20180524142426.jeyszeqdrjvb3tob@linutronix.de>

On Thu, May 24, 2018 at 4:24 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> timekeeping suspend/resume calls read_persistent_clock() which takes
> rtc_lock. That results in might sleep warnings because at that point
> we run with interrupts disabled.
>
> We cannot convert rtc_lock to a raw spinlock as that would trigger
> other might sleep warnings.
>
> As a workaround we disable the might sleep warnings by setting
> system_state to SYSTEM_SUSPEND before calling sysdev_suspend() and
> restoring it to SYSTEM_RUNNING afer sysdev_resume().
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Hmm.  Don't we also need to cover suspend-to-idle?

> ---
>  include/linux/kernel.h   |    1 +
>  kernel/power/hibernate.c |    7 +++++++
>  kernel/power/suspend.c   |    4 ++++
>  3 files changed, 12 insertions(+)
>
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -532,6 +532,7 @@ extern enum system_states {
>         SYSTEM_HALT,
>         SYSTEM_POWER_OFF,
>         SYSTEM_RESTART,
> +       SYSTEM_SUSPEND,
>  } system_state;
>
>  #define TAINT_PROPRIETARY_MODULE       0
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -287,6 +287,8 @@ static int create_image(int platform_mod
>
>         local_irq_disable();
>
> +       system_state = SYSTEM_SUSPEND;
> +
>         error = syscore_suspend();
>         if (error) {
>                 pr_err("Some system devices failed to power down, aborting hibernation\n");
> @@ -317,6 +319,7 @@ static int create_image(int platform_mod
>         syscore_resume();
>
>   Enable_irqs:
> +       system_state = SYSTEM_RUNNING;
>         local_irq_enable();
>
>   Enable_cpus:
> @@ -445,6 +448,7 @@ static int resume_target_kernel(bool pla
>                 goto Enable_cpus;
>
>         local_irq_disable();
> +       system_state = SYSTEM_SUSPEND;
>
>         error = syscore_suspend();
>         if (error)
> @@ -478,6 +482,7 @@ static int resume_target_kernel(bool pla
>         syscore_resume();
>
>   Enable_irqs:
> +       system_state = SYSTEM_RUNNING;
>         local_irq_enable();
>
>   Enable_cpus:
> @@ -563,6 +568,7 @@ int hibernation_platform_enter(void)
>                 goto Enable_cpus;
>
>         local_irq_disable();
> +       system_state = SYSTEM_SUSPEND;
>         syscore_suspend();
>         if (pm_wakeup_pending()) {
>                 error = -EAGAIN;
> @@ -575,6 +581,7 @@ int hibernation_platform_enter(void)
>
>   Power_up:
>         syscore_resume();
> +       system_state = SYSTEM_RUNNING;
>         local_irq_enable();
>
>   Enable_cpus:
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -428,6 +428,8 @@ static int suspend_enter(suspend_state_t
>         arch_suspend_disable_irqs();
>         BUG_ON(!irqs_disabled());
>
> +       system_state = SYSTEM_SUSPEND;
> +
>         error = syscore_suspend();
>         if (!error) {
>                 *wakeup = pm_wakeup_pending();
> @@ -443,6 +445,8 @@ static int suspend_enter(suspend_state_t
>                 syscore_resume();
>         }
>
> +       system_state = SYSTEM_RUNNING;
> +
>         arch_suspend_enable_irqs();
>         BUG_ON(irqs_disabled());
>

^ permalink raw reply

* [PATCH v13 2/2] dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu
From: Ilia Lin @ 2018-05-24 15:03 UTC (permalink / raw)
  To: vireshk, nm, sboyd, robh, mark.rutland, rjw
  Cc: linux-pm, devicetree, linux-kernel, ilialin
In-Reply-To: <1527174220-13244-1-git-send-email-ilialin@codeaurora.org>

The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
to provide the OPP framework with required information.
This is used to determine the voltage and frequency value for each OPP of
operating-points-v2 table when it is parsed by the OPP framework.

This change adds documentation for the DT bindings.
The "operating-points-v2-kryo-cpu" DT extends the "operating-points-v2"
with following parameters:
- nvmem-cells (NVMEM area containig the speedbin information)
- opp-supported-hw: A single 32 bit bitmap value,
  representing compatible HW:
			0:	MSM8996 V3, speedbin 0
			1:	MSM8996 V3, speedbin 1
			2:	MSM8996 V3, speedbin 2
			3:	unused
			4:	MSM8996 SG, speedbin 0
			5:	MSM8996 SG, speedbin 1
			6:	MSM8996 SG, speedbin 2
			7-31:	unused

Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/opp/kryo-cpufreq.txt       | 680 +++++++++++++++++++++
 1 file changed, 680 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/opp/kryo-cpufreq.txt

diff --git a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt b/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
new file mode 100644
index 0000000..c2127b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
@@ -0,0 +1,680 @@
+Qualcomm Technologies, Inc. KRYO CPUFreq and OPP bindings
+===================================
+
+In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996
+that have KRYO processors, the CPU ferequencies subset and voltage value
+of each OPP varies based on the silicon variant in use.
+Qualcomm Technologies, Inc. Process Voltage Scaling Tables
+defines the voltage and frequency value based on the msm-id in SMEM
+and speedbin blown in the efuse combination.
+The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
+to provide the OPP framework with required information (existing HW bitmap).
+This is used to determine the voltage and frequency value for each OPP of
+operating-points-v2 table when it is parsed by the OPP framework.
+
+Required properties:
+--------------------
+In 'cpus' nodes:
+- operating-points-v2: Phandle to the operating-points-v2 table to use.
+
+In 'operating-points-v2' table:
+- compatible: Should be
+	- 'operating-points-v2-kryo-cpu' for apq8096 and msm8996.
+- nvmem-cells: A phandle pointing to a nvmem-cells node representing the
+		efuse registers that has information about the
+		speedbin that is used to select the right frequency/voltage
+		value pair.
+		Please refer the for nvmem-cells
+		bindings Documentation/devicetree/bindings/nvmem/nvmem.txt
+		and also examples below.
+
+In every OPP node:
+- opp-supported-hw: A single 32 bit bitmap value, representing compatible HW.
+		    Bitmap:
+			0:	MSM8996 V3, speedbin 0
+			1:	MSM8996 V3, speedbin 1
+			2:	MSM8996 V3, speedbin 2
+			3:	unused
+			4:	MSM8996 SG, speedbin 0
+			5:	MSM8996 SG, speedbin 1
+			6:	MSM8996 SG, speedbin 2
+			7-31:	unused
+
+Example 1:
+---------
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		CPU0: cpu@0 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+			clocks = <&kryocc 0>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster0_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_0>;
+			L2_0: l2-cache {
+			      compatible = "cache";
+			      cache-level = <2>;
+			};
+		};
+
+		CPU1: cpu@1 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x1>;
+			enable-method = "psci";
+			clocks = <&kryocc 0>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster0_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_0>;
+		};
+
+		CPU2: cpu@100 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x100>;
+			enable-method = "psci";
+			clocks = <&kryocc 1>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster1_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_1>;
+			L2_1: l2-cache {
+			      compatible = "cache";
+			      cache-level = <2>;
+			};
+		};
+
+		CPU3: cpu@101 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x101>;
+			enable-method = "psci";
+			clocks = <&kryocc 1>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster1_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_1>;
+		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&CPU0>;
+				};
+
+				core1 {
+					cpu = <&CPU1>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&CPU2>;
+				};
+
+				core1 {
+					cpu = <&CPU3>;
+				};
+			};
+		};
+	};
+
+	cluster0_opp: opp_table0 {
+		compatible = "operating-points-v2-kryo-cpu";
+		nvmem-cells = <&speedbin_efuse>;
+		opp-shared;
+
+		opp-307200000 {
+			opp-hz = /bits/ 64 <307200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x77>;
+			clock-latency-ns = <200000>;
+		};
+		opp-384000000 {
+			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-422400000 {
+			opp-hz = /bits/ 64 <422400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-460800000 {
+			opp-hz = /bits/ 64 <460800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-537600000 {
+			opp-hz = /bits/ 64 <537600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-556800000 {
+			opp-hz = /bits/ 64 <556800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-614400000 {
+			opp-hz = /bits/ 64 <614400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-691200000 {
+			opp-hz = /bits/ 64 <691200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-729600000 {
+			opp-hz = /bits/ 64 <729600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-768000000 {
+			opp-hz = /bits/ 64 <768000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-844800000 {
+			opp-hz = /bits/ 64 <844800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x77>;
+			clock-latency-ns = <200000>;
+		};
+		opp-902400000 {
+			opp-hz = /bits/ 64 <902400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-960000000 {
+			opp-hz = /bits/ 64 <960000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-979200000 {
+			opp-hz = /bits/ 64 <979200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1036800000 {
+			opp-hz = /bits/ 64 <1036800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1113600000 {
+			opp-hz = /bits/ 64 <1113600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1132800000 {
+			opp-hz = /bits/ 64 <1132800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1190400000 {
+			opp-hz = /bits/ 64 <1190400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1209600000 {
+			opp-hz = /bits/ 64 <1209600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1228800000 {
+			opp-hz = /bits/ 64 <1228800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1286400000 {
+			opp-hz = /bits/ 64 <1286400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x5>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1363200000 {
+			opp-hz = /bits/ 64 <1363200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x72>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1401600000 {
+			opp-hz = /bits/ 64 <1401600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x5>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1440000000 {
+			opp-hz = /bits/ 64 <1440000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1478400000 {
+			opp-hz = /bits/ 64 <1478400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1497600000 {
+			opp-hz = /bits/ 64 <1497600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x4>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1516800000 {
+			opp-hz = /bits/ 64 <1516800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1593600000 {
+			opp-hz = /bits/ 64 <1593600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x71>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1996800000 {
+			opp-hz = /bits/ 64 <1996800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x20>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2188800000 {
+			opp-hz = /bits/ 64 <2188800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x10>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
+	cluster1_opp: opp_table1 {
+		compatible = "operating-points-v2-kryo-cpu";
+		nvmem-cells = <&speedbin_efuse>;
+		opp-shared;
+
+		opp-307200000 {
+			opp-hz = /bits/ 64 <307200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x77>;
+			clock-latency-ns = <200000>;
+		};
+		opp-384000000 {
+			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-403200000 {
+			opp-hz = /bits/ 64 <403200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-460800000 {
+			opp-hz = /bits/ 64 <460800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-537600000 {
+			opp-hz = /bits/ 64 <537600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-556800000 {
+			opp-hz = /bits/ 64 <556800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-614400000 {
+			opp-hz = /bits/ 64 <614400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-691200000 {
+			opp-hz = /bits/ 64 <691200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-729600000 {
+			opp-hz = /bits/ 64 <729600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-748800000 {
+			opp-hz = /bits/ 64 <748800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-806400000 {
+			opp-hz = /bits/ 64 <806400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-825600000 {
+			opp-hz = /bits/ 64 <825600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-883200000 {
+			opp-hz = /bits/ 64 <883200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-902400000 {
+			opp-hz = /bits/ 64 <902400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-940800000 {
+			opp-hz = /bits/ 64 <940800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-979200000 {
+			opp-hz = /bits/ 64 <979200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1036800000 {
+			opp-hz = /bits/ 64 <1036800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1113600000 {
+			opp-hz = /bits/ 64 <1113600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1132800000 {
+			opp-hz = /bits/ 64 <1132800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1190400000 {
+			opp-hz = /bits/ 64 <1190400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1209600000 {
+			opp-hz = /bits/ 64 <1209600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1248000000 {
+			opp-hz = /bits/ 64 <1248000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1286400000 {
+			opp-hz = /bits/ 64 <1286400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1363200000 {
+			opp-hz = /bits/ 64 <1363200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1401600000 {
+			opp-hz = /bits/ 64 <1401600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1440000000 {
+			opp-hz = /bits/ 64 <1440000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1478400000 {
+			opp-hz = /bits/ 64 <1478400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1516800000 {
+			opp-hz = /bits/ 64 <1516800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1555200000 {
+			opp-hz = /bits/ 64 <1555200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1593600000 {
+			opp-hz = /bits/ 64 <1593600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1632000000 {
+			opp-hz = /bits/ 64 <1632000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1670400000 {
+			opp-hz = /bits/ 64 <1670400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1708800000 {
+			opp-hz = /bits/ 64 <1708800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1747200000 {
+			opp-hz = /bits/ 64 <1747200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1785600000 {
+			opp-hz = /bits/ 64 <1785600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1804800000 {
+			opp-hz = /bits/ 64 <1804800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x6>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1824000000 {
+			opp-hz = /bits/ 64 <1824000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x71>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1900800000 {
+			opp-hz = /bits/ 64 <1900800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x74>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1920000000 {
+			opp-hz = /bits/ 64 <1920000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1977600000 {
+			opp-hz = /bits/ 64 <1977600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x30>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1996800000 {
+			opp-hz = /bits/ 64 <1996800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2054400000 {
+			opp-hz = /bits/ 64 <2054400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x30>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2073600000 {
+			opp-hz = /bits/ 64 <2073600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2150400000 {
+			opp-hz = /bits/ 64 <2150400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x31>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2246400000 {
+			opp-hz = /bits/ 64 <2246400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x10>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2342400000 {
+			opp-hz = /bits/ 64 <2342400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x10>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
+....
+
+reserved-memory {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	ranges;
+....
+	smem_mem: smem-mem@86000000 {
+		reg = <0x0 0x86000000 0x0 0x200000>;
+		no-map;
+	};
+....
+};
+
+smem {
+	compatible = "qcom,smem";
+	memory-region = <&smem_mem>;
+	hwlocks = <&tcsr_mutex 3>;
+};
+
+soc {
+....
+	qfprom: qfprom@74000 {
+		compatible = "qcom,qfprom";
+		reg = <0x00074000 0x8ff>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		....
+		speedbin_efuse: speedbin@133 {
+			reg = <0x133 0x1>;
+			bits = <5 3>;
+		};
+	};
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH v13 1/2] cpufreq: Add Kryo CPU scaling driver
From: Ilia Lin @ 2018-05-24 15:03 UTC (permalink / raw)
  To: vireshk, nm, sboyd, robh, mark.rutland, rjw
  Cc: linux-pm, devicetree, linux-kernel, ilialin
In-Reply-To: <1527174220-13244-1-git-send-email-ilialin@codeaurora.org>

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
the CPU frequency subset and voltage value of each OPP varies
based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
defines the voltage and frequency value based on the msm-id in SMEM
and speedbin blown in the efuse combination.
The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
to provide the OPP framework with required information.
This is used to determine the voltage and frequency value for each OPP of
operating-points-v2 table when it is parsed by the OPP framework.

Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
---
 drivers/cpufreq/Kconfig.arm          |  10 ++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 206 +++++++++++++++++++++++++++++++++++
 4 files changed, 220 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index de55c7d..0bfd40e 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+	bool "Qualcomm Kryo based CPUFreq"
+	depends on QCOM_QFPROM
+	depends on QCOM_SMEM
+	select PM_OPP
+	help
+	  This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+	  If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 8d24ade..fb4a2ec 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 3b585e4..77d6ab8 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -118,6 +118,9 @@
 
 	{ .compatible = "nvidia,tegra124", },
 
+	{ .compatible = "qcom,apq8096", },
+	{ .compatible = "qcom,msm8996", },
+
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
 
diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c
new file mode 100644
index 0000000..bc2e885
--- /dev/null
+++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+/*
+ * In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
+ * the CPU frequency subset and voltage value of each OPP varies
+ * based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
+ * defines the voltage and frequency value based on the msm-id in SMEM
+ * and speedbin blown in the efuse combination.
+ * The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
+ * to provide the OPP framework with required information.
+ * This is used to determine the voltage and frequency value for each OPP of
+ * operating-points-v2 table when it is parsed by the OPP framework.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+
+#define MSM_ID_SMEM	137
+
+enum _msm_id {
+	MSM8996V3 = 0xF6ul,
+	APQ8096V3 = 0x123ul,
+	MSM8996SG = 0x131ul,
+	APQ8096SG = 0x138ul,
+};
+
+enum _msm8996_version {
+	MSM8996_V3,
+	MSM8996_SG,
+	NUM_OF_MSM8996_VERSIONS,
+};
+
+static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
+{
+	size_t len;
+	u32 *msm_id;
+	enum _msm8996_version version;
+
+	msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
+	if (IS_ERR(msm_id))
+		return NUM_OF_MSM8996_VERSIONS;
+
+	/* The first 4 bytes are format, next to them is the actual msm-id */
+	msm_id++;
+
+	switch ((enum _msm_id)*msm_id) {
+	case MSM8996V3:
+	case APQ8096V3:
+		version = MSM8996_V3;
+		break;
+	case MSM8996SG:
+	case APQ8096SG:
+		version = MSM8996_SG;
+		break;
+	default:
+		version = NUM_OF_MSM8996_VERSIONS;
+	}
+
+	return version;
+}
+
+static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
+{
+	struct opp_table *opp_tables[NR_CPUS] = {0};
+	struct platform_device *cpufreq_dt_pdev;
+	enum _msm8996_version msm8996_version;
+	struct nvmem_cell *speedbin_nvmem;
+	struct device_node *np;
+	struct device *cpu_dev;
+	unsigned cpu;
+	u8 *speedbin;
+	u32 versions;
+	size_t len;
+	int ret;
+
+	cpu_dev = get_cpu_device(0);
+	if (NULL == cpu_dev)
+		ret = -ENODEV;
+
+	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+		dev_err(cpu_dev, "Not Snapdragon 820/821!");
+		return -ENODEV;
+	}
+
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+	if (IS_ERR(np))
+		return PTR_ERR(np);
+
+	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+	of_node_put(np);
+	if (IS_ERR(speedbin_nvmem)) {
+		dev_err(cpu_dev, "Could not get nvmem cell: %ld\n",
+			PTR_ERR(speedbin_nvmem));
+		return PTR_ERR(speedbin_nvmem);
+	}
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+	nvmem_cell_put(speedbin_nvmem);
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);
+		break;
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	for_each_possible_cpu(cpu) {
+		cpu_dev = get_cpu_device(cpu);
+		if (NULL == cpu_dev) {
+			ret = -ENODEV;
+			goto free_opp;
+		}
+
+		opp_tables[cpu] = dev_pm_opp_set_supported_hw(cpu_dev,
+							      &versions, 1);
+		if (IS_ERR(opp_tables[cpu])) {
+			ret = PTR_ERR(opp_tables[cpu]);
+			dev_err(cpu_dev, "Failed to set supported hardware\n");
+			goto free_opp;
+		}
+	}
+
+	cpufreq_dt_pdev = platform_device_register_simple("cpufreq-dt", -1,
+							  NULL, 0);
+	if (!IS_ERR(cpufreq_dt_pdev))
+		return 0;
+
+	ret = PTR_ERR(cpufreq_dt_pdev);
+	dev_err(cpu_dev, "Failed to register platform device\n");
+
+free_opp:
+	for_each_possible_cpu(cpu) {
+		if (IS_ERR_OR_NULL(opp_tables[cpu]))
+			break;
+		dev_pm_opp_put_supported_hw(opp_tables[cpu]);
+	}
+
+	return ret;
+}
+
+static struct platform_driver qcom_cpufreq_kryo_driver = {
+	.probe = qcom_cpufreq_kryo_probe,
+	.driver = {
+		.name = "qcom-cpufreq-kryo",
+	},
+};
+
+/*
+ * Since the driver depends on smem and nvmem drivers, which may
+ * return EPROBE_DEFER, all the real activity is done in the probe,
+ * which may be defered as well. The init here is only registering
+ * the driver and the platform device.
+ */
+static int __init qcom_cpufreq_kryo_init(void)
+{
+	struct device_node *np;
+	struct device *cpu_dev;
+	int ret;
+
+	cpu_dev = get_cpu_device(0);
+	if (NULL == cpu_dev)
+		ret = -ENODEV;
+
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+	if (IS_ERR(np))
+		return PTR_ERR(np);
+
+	ret = of_device_is_compatible(np, "operating-points-v2-kryo-cpu");
+	of_node_put(np);
+	if (!ret)
+		return -ENOENT;
+
+	ret = platform_driver_register(&qcom_cpufreq_kryo_driver);
+	if (unlikely(ret < 0))
+		return ret;
+
+	ret = PTR_ERR_OR_ZERO(platform_device_register_simple(
+		"qcom-cpufreq-kryo", -1, NULL, 0));
+	if (0 == ret)
+		return 0;
+
+	platform_driver_unregister(&qcom_cpufreq_kryo_driver);
+	return ret;
+}
+module_init(qcom_cpufreq_kryo_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related

* [PATCH v13 0/2] Kryo CPU scaling driver
From: Ilia Lin @ 2018-05-24 15:03 UTC (permalink / raw)
  To: vireshk, nm, sboyd, robh, mark.rutland, rjw
  Cc: linux-pm, devicetree, linux-kernel, ilialin

[v13]
 * Addressed comment from Sudeep about DT compatible check on init

[v12]
 * Addressed comments from Sudeep and Viresh about the single init

[v11]
 * Addressed comment from Russel about device_node reference
 * Addressed comment from Sudeep about the late_initcall
 * Transformed init into probe to take care of deferals

[v10]
 * Split the series into domains
 * Addressed comments from Viresh and Sudeep about logical CPU numbering.

The qcom-cpufreq-kryo driver is aimed to support different SOC versions.
The driver reads eFuse information and chooses the required OPP subset
by passing the OPP supported-hw parameter.

The series depends on the series from Viresh:
https://patchwork.kernel.org/patch/10418139/

The previous spin was here:
https://patchwork.kernel.org/patch/10423137/

Ilia Lin (2):
  cpufreq: Add Kryo CPU scaling driver
  dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu

 .../devicetree/bindings/opp/kryo-cpufreq.txt       | 680 +++++++++++++++++++++
 drivers/cpufreq/Kconfig.arm                        |  10 +
 drivers/cpufreq/Makefile                           |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c               |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c                | 206 +++++++
 5 files changed, 900 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
From: Sudeep Holla @ 2018-05-24 15:02 UTC (permalink / raw)
  To: ilialin
  Cc: Sudeep Holla, linux-pm, devicetree, linux-kernel, vireshk, nm,
	sboyd, robh, mark.rutland, rjw
In-Reply-To: <000901d3f36e$c9c995e0$5d5cc1a0$@codeaurora.org>



On 24/05/18 15:52, ilialin@codeaurora.org wrote:
> OK, got you.
> Is "operating-points-v2-kryo-cpu" good enough?
> 

Not sure if I can answer that as I don't have full knowledge on PM on
all Kryo CPUs. Will this driver work on all those platforms which will
have "operating-points-v2-kryo-cpu" ? If yes, then go ahead.

I would prefer using some compatible in the CPU node rather that OPP,
but that's just my opinion. Check with other QCOM guys on the list who
have worked on CPUFreq or still working.(Taniya, Saravana, S Boyd, ...?)

-- 
Regards,
Sudeep

^ permalink raw reply

* RE: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
From: ilialin @ 2018-05-24 14:52 UTC (permalink / raw)
  To: 'Sudeep Holla'
  Cc: linux-pm, devicetree, linux-kernel, vireshk, nm, sboyd, robh,
	mark.rutland, rjw
In-Reply-To: <15d6025c-1307-64e0-6715-34240fcaf8eb@arm.com>

OK, got you.
Is "operating-points-v2-kryo-cpu" good enough?

> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@arm.com>
> Sent: Thursday, May 24, 2018 17:48
> To: ilialin@codeaurora.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; linux-pm@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> vireshk@kernel.org; nm@ti.com; sboyd@kernel.org; robh@kernel.org;
> mark.rutland@arm.com; rjw@rjwysocki.net
> Subject: Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
> 
> 
> 
> On 24/05/18 15:10, ilialin@codeaurora.org wrote:
> > Thank you for the explanation. However, could you suggest, which
> > condition should I check then? Device tree?
> >
> 
> Yes some compatible which is applicable for all the SoCs or platforms on
> which this driver can work ?
> 
> --
> Regards,
> Sudeep

^ permalink raw reply

* Re: [PATCH v12 1/2] cpufreq: Add Kryo CPU scaling driver
From: Sudeep Holla @ 2018-05-24 14:47 UTC (permalink / raw)
  To: ilialin
  Cc: Sudeep Holla, linux-pm, devicetree, linux-kernel, vireshk, nm,
	sboyd, robh, mark.rutland, rjw
In-Reply-To: <000701d3f368$fd31e410$f795ac30$@codeaurora.org>



On 24/05/18 15:10, ilialin@codeaurora.org wrote:
> Thank you for the explanation. However, could you suggest, which
> condition should I check then? Device tree?
> 

Yes some compatible which is applicable for all the SoCs or platforms on
which this driver can work ?

-- 
Regards,
Sudeep

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox