Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-02-07 Thread Sudeep Holla 
On 07/02/14 15:19, Thomas Abraham wrote:
> From: Thomas Abraham 
> 
> Add a new optional boost-frequency binding for specifying the frequencies
> usable in boost mode.
> 
> Cc: Nishanth Menon 
> Cc: Lukasz Majewski 
> Cc: Rob Herring 
> Cc: Pawel Moll 
> Cc: Mark Rutland 
> Cc: Ian Campbell 
> Cc: Kumar Gala 
> Signed-off-by: Thomas Abraham 
> ---
>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 
> +++
>  1 file changed, 11 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt 
> b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
> new file mode 100644
> index 000..d925e38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
> @@ -0,0 +1,11 @@
> +* Device tree binding for CPU boost frequency (aka over-clocking)
> +
> +Certain CPU's can be operated in optional 'boost' mode (or sometimes 
> referred as
> +overclocking) in which the CPU can operate in frequencies beyond the normal
> +operating conditions.
> +
> +Optional Properties:
> +- boost-frequency: list of frequencies in KHz to be used only in boost mode.
> +  This list should be a subset of frequencies listed in "operating-points"
> +  property. Refer to Documentation/devicetree/bindings/power/opp.txt for
> +  details about "operating-points" property.
> 

Won't single entry for boost frequency suffice which would be the starting
frequency in the boost range. IWO will there be OPP list with frequencies:
A > B > C > D, but only B and C are boost frequency. That seems little odd,
unless it's some configuration chosen purely on software basis rather than
hardware. For me B marks the beginning of over-clocking.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-02-07 Thread Sudeep Holla 
On 07/02/14 16:15, Sudeep Holla wrote:
> On 07/02/14 15:19, Thomas Abraham wrote:
>> From: Thomas Abraham 
>>
>> Add a new optional boost-frequency binding for specifying the frequencies
>> usable in boost mode.
>>
>> Cc: Nishanth Menon 
>> Cc: Lukasz Majewski 
>> Cc: Rob Herring 
>> Cc: Pawel Moll 
>> Cc: Mark Rutland 
>> Cc: Ian Campbell 
>> Cc: Kumar Gala 
>> Signed-off-by: Thomas Abraham 
>> ---
>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 
>> +++
>>  1 file changed, 11 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt 
>> b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>> new file mode 100644
>> index 000..d925e38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>> @@ -0,0 +1,11 @@
>> +* Device tree binding for CPU boost frequency (aka over-clocking)
>> +
>> +Certain CPU's can be operated in optional 'boost' mode (or sometimes 
>> referred as
>> +overclocking) in which the CPU can operate in frequencies beyond the normal
>> +operating conditions.
>> +
>> +Optional Properties:
>> +- boost-frequency: list of frequencies in KHz to be used only in boost mode.
>> +  This list should be a subset of frequencies listed in "operating-points"
>> +  property. Refer to Documentation/devicetree/bindings/power/opp.txt for
>> +  details about "operating-points" property.
>>
> 
> Won't single entry for boost frequency suffice which would be the starting
> frequency in the boost range. IOW will there be OPP list with frequencies:
> A > B > C > D, but only B and C are boost frequency. That seems little odd,
> unless it's some configuration chosen purely on software basis rather than
> hardware. For me B marks the beginning of over-clocking.
> 
Ah, I meant A < B < C < D in the above example.

Regards,
Sudeep


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/7] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data

2014-02-07 Thread Sudeep Holla 
On 07/02/14 15:55, Thomas Abraham wrote:
> From: Thomas Abraham 
> 
> For all Exynos based platforms, add CPU nodes, operating points and cpu
> clock data for migrating from Exynos specific cpufreq driver to using
> generic cpufreq-cpu0 driver.
> 
> Signed-off-by: Thomas Abraham 

[...]

> diff --git a/arch/arm/boot/dts/exynos4x12.dtsi 
> b/arch/arm/boot/dts/exynos4x12.dtsi
> index 5c412aa..c613fc2 100644
> --- a/arch/arm/boot/dts/exynos4x12.dtsi
> +++ b/arch/arm/boot/dts/exynos4x12.dtsi
> @@ -31,6 +31,42 @@
> mshc0 = &mshc_0;
> };
> 
> +   cpus {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   cpu@0 {
> +   device_type = "cpu";
> +   compatible = "arm,cortex-a9";
> +   reg = <0>;
> +   clocks = <&clock 12>;
> +   clock-names = "cpu";
> +
> +   operating-points = <
> +   150 135
> +   140 1287500
> +   130 125
> +   120 1187500
> +   110 1137500
> +   100 1087500
> +90 1037500
> +80 100
> +70  987500
> +60  975000
> +50  95
> +40  925000
> +30  90
> +20  90
> +   >;
> +   clock-latency = <20>;
> +   boost-frequency = <150 135>;

This is confusing, 135 is not in the OPP frequency list or this is still
following old binding with voltage. Either case this needs to be fixed.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-02-07 Thread Sudeep Holla 
On 07/02/14 16:43, Nishanth Menon wrote:
> On Fri, Feb 7, 2014 at 10:28 AM, Sudeep Holla  wrote:
>> On 07/02/14 16:15, Sudeep Holla wrote:
>>> On 07/02/14 15:19, Thomas Abraham wrote:
>>>> From: Thomas Abraham 
>>>>
>>>> Add a new optional boost-frequency binding for specifying the frequencies
>>>> usable in boost mode.
>>>>
>>>> Cc: Nishanth Menon 
>>>> Cc: Lukasz Majewski 
>>>> Cc: Rob Herring 
>>>> Cc: Pawel Moll 
>>>> Cc: Mark Rutland 
>>>> Cc: Ian Campbell 
>>>> Cc: Kumar Gala 
>>>> Signed-off-by: Thomas Abraham 
>>>> ---
>>>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 
>>>> +++
>>>>  1 file changed, 11 insertions(+)
>>>>  create mode 100644 
>>>> Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt 
>>>> b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>> new file mode 100644
>>>> index 000..d925e38
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>> @@ -0,0 +1,11 @@
>>>> +* Device tree binding for CPU boost frequency (aka over-clocking)
>>>> +
>>>> +Certain CPU's can be operated in optional 'boost' mode (or sometimes 
>>>> referred as
>>>> +overclocking) in which the CPU can operate in frequencies beyond the 
>>>> normal
>>>> +operating conditions.
>>>> +
>>>> +Optional Properties:
>>>> +- boost-frequency: list of frequencies in KHz to be used only in boost 
>>>> mode.
>>>> +  This list should be a subset of frequencies listed in "operating-points"
>>>> +  property. Refer to Documentation/devicetree/bindings/power/opp.txt for
>>>> +  details about "operating-points" property.
>>>>
>>>
>>> Won't single entry for boost frequency suffice which would be the starting
>>> frequency in the boost range. IOW will there be OPP list with frequencies:
>>> A > B > C > D, but only B and C are boost frequency. That seems little odd,
>>> unless it's some configuration chosen purely on software basis rather than
>>> hardware. For me B marks the beginning of over-clocking.
>>>
>> Ah, I meant A < B < C < D in the above example.
>
> Should'nt we let the SoC dts define that - traditionally, yes, but
> consider the following:
> A, B, C uses clk_parent X which describes B, C as overclocked.
> and say D uses clk_parent Y which is not "over clocked", then you have
> the scenario that on the first look seems counter-intutive.
> 

Yes I thought of exactly similar clock setup, but was not convinced that it
should be part of OPP. In that case it looks like we are trying to represent
clock internals through some OPP bindings.

Yes I think its counter-intuitive as it's visible to the userspace(list of
frequencies and the boost parameters are exposed through sysfs)

Regards,
Sudeep


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-02-07 Thread Sudeep Holla 
On 07/02/14 17:37, Nishanth Menon wrote:
> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla  wrote:

[...]

>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>> frequencies and the boost parameters are exposed through sysfs)
> 
> That will be a different problem -> as currently every single
> frequency in the cpufreq list has ability to be marked as boost
> frequency - if userspace does not maintain that, then, IMHO, fix the
> userspace :D
> 

/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
the list of frequencies based on the state of the boost feature at anytime.

Intuitively the list without boost shouldn't have any frequency above the range
when it's enabled :), that's what I was referring to. So I am not talking about
any issue with user-space maintenance.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree

2014-02-07 Thread Sudeep Holla 
On 07/02/14 15:19, Thomas Abraham wrote:
> From: Thomas Abraham 
> 
> Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds
> support for CPU boost mode. This patch adds support for finding available
> boost frequencies from device tree and marking them as usable in boost mode.
> 
> Cc: Nishanth Menon 
> Cc: Lukasz Majewski 
> Signed-off-by: Thomas Abraham 
> ---
>  drivers/base/power/opp.c |   34 +-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index fa41874..b636826 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>   struct device_opp *dev_opp;
>   struct dev_pm_opp *opp;
>   struct cpufreq_frequency_table *freq_table;
> - int i = 0;
> + int i = 0, j, len, ret;
> + u32 *boost_freqs = NULL;
>  
>   /* Pretend as if I am an updater */
>   mutex_lock(&dev_opp_list_lock);
> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>   return -ENOMEM;
>   }
>  
> + if (of_find_property(dev->of_node, "boost-frequency", &len)) {
> + if (len == 0 || (len & (sizeof(u32) - 1)) != 0) {
> + dev_err(dev, "%s: invalid boost frequency\n", __func__);
> + ret = -EINVAL;
> + goto err_boost;
> + }
> +
> + boost_freqs = kzalloc(len, GFP_KERNEL);
> + if (!boost_freqs) {
> + dev_warn(dev, "%s: no memory for boost freq table\n",
> + __func__);
> + ret = -ENOMEM;
> + goto err_boost;
> + }
> + of_property_read_u32_array(dev->of_node, "boost-frequency",
> + boost_freqs, len / sizeof(u32));
> + }
> +
>   list_for_each_entry(opp, &dev_opp->opp_list, node) {
>   if (opp->available) {
>   freq_table[i].driver_data = i;
>   freq_table[i].frequency = opp->rate / 1000;
> + for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
> + if (boost_freqs[j] == freq_table[i].frequency) {
> + freq_table[i].driver_data =
> + CPUFREQ_BOOST_FREQ;
> + break;
> + }
> + }
>   i++;
>   }
>   }
IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, but
this change seems to be cpufreq only.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree

2014-02-10 Thread Sudeep Holla 
On 08/02/14 05:10, Thomas Abraham wrote:
> On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla  wrote:
>> On 07/02/14 15:19, Thomas Abraham wrote:
>>> From: Thomas Abraham 
>>>
>>> Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds
>>> support for CPU boost mode. This patch adds support for finding available
>>> boost frequencies from device tree and marking them as usable in boost mode.
>>>
>>> Cc: Nishanth Menon 
>>> Cc: Lukasz Majewski 
>>> Signed-off-by: Thomas Abraham 
>>> ---
>>>  drivers/base/power/opp.c |   34 +-
>>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>>> index fa41874..b636826 100644
>>> --- a/drivers/base/power/opp.c
>>> +++ b/drivers/base/power/opp.c
>>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>   struct device_opp *dev_opp;
>>>   struct dev_pm_opp *opp;
>>>   struct cpufreq_frequency_table *freq_table;
>>> - int i = 0;
>>> + int i = 0, j, len, ret;
>>> + u32 *boost_freqs = NULL;
>>>
>>>   /* Pretend as if I am an updater */
>>>   mutex_lock(&dev_opp_list_lock);
>>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>   return -ENOMEM;
>>>   }
>>>
>>> + if (of_find_property(dev->of_node, "boost-frequency", &len)) {
>>> + if (len == 0 || (len & (sizeof(u32) - 1)) != 0) {
>>> + dev_err(dev, "%s: invalid boost frequency\n", 
>>> __func__);
>>> + ret = -EINVAL;
>>> + goto err_boost;
>>> + }
>>> +
>>> + boost_freqs = kzalloc(len, GFP_KERNEL);
>>> + if (!boost_freqs) {
>>> + dev_warn(dev, "%s: no memory for boost freq table\n",
>>> + __func__);
>>> + ret = -ENOMEM;
>>> + goto err_boost;
>>> + }
>>> + of_property_read_u32_array(dev->of_node, "boost-frequency",
>>> + boost_freqs, len / sizeof(u32));
>>> + }
>>> +
>>>   list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>>   if (opp->available) {
>>>   freq_table[i].driver_data = i;
>>>   freq_table[i].frequency = opp->rate / 1000;
>>> + for (j = 0; j < len / sizeof(u32) && boost_freqs; 
>>> j++) {
>>> + if (boost_freqs[j] == 
>>> freq_table[i].frequency) {
>>> + freq_table[i].driver_data =
>>> + CPUFREQ_BOOST_FREQ;
>>> + break;
>>> + }
>>> + }
>>>   i++;
>>>   }
>>>   }
>> IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, 
>> but
>> this change seems to be cpufreq only.
> 
> Yes, but as you have initiated the discussion on extending the OPP
> binding, this has been limited to cpufreq only. If the new OPP library
> has support for listing boost frequency, this can be migrated to the
> new OPP libaray.
> 

Fair enough, just wanted to check as I couldn't get the info following the
thread. I might have missed it.

Regards,
Sudeep


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-02-10 Thread Sudeep Holla 
On 08/02/14 06:47, Thomas Abraham wrote:
> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla  wrote:
>> On 07/02/14 17:37, Nishanth Menon wrote:
>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla  wrote:
>>
>> [...]
>>
>>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>>> frequencies and the boost parameters are exposed through sysfs)
>>>
>>> That will be a different problem -> as currently every single
>>> frequency in the cpufreq list has ability to be marked as boost
>>> frequency - if userspace does not maintain that, then, IMHO, fix the
>>> userspace :D
>>>
>>
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
>> the list of frequencies based on the state of the boost feature at anytime.
> 
> The list of frequencies in
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> does not change based in the state of the boost feature (enabled or
> disabled). But the scaling_max_frequency and scaling_min_frequency are
> updated based on the set of available + boost frequencies available.
> 

Ah ok, sorry just glanced the code and misunderstood it. It make sense to update
only max_frequency.

Regards,
Sudeep


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-02-10 Thread Sudeep Holla 
On 10/02/14 07:38, Lukasz Majewski wrote:
> Hi Thomas, Sudeep,
> 
>> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla 
>> wrote:
>>> On 07/02/14 17:37, Nishanth Menon wrote:
>>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
>>>>  wrote:
>>>
>>> [...]
>>>
>>>>> Yes I think its counter-intuitive as it's visible to the
>>>>> userspace(list of frequencies and the boost parameters are
>>>>> exposed through sysfs)
>>>>
>>>> That will be a different problem -> as currently every single
>>>> frequency in the cpufreq list has ability to be marked as boost
>>>> frequency - if userspace does not maintain that, then, IMHO, fix
>>>> the userspace :D
>>>>
>>>
>>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>>> gives the list of frequencies based on the state of the boost
>>> feature at anytime.
>>
>> The list of frequencies in
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>> does not change based in the state of the boost feature (enabled or
>> disabled). But the scaling_max_frequency and scaling_min_frequency are
>> updated based on the set of available + boost frequencies available.
> 
> With boost intended behavior is as follow:
> 
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies [1]
> 
> shows the non boost frequencies no matter if boost is enabled or not.
> Those are the "normal" frequencies.
> 
> When boost is supported (by enabling the CONFIG_CPU_FREQ_BOOST_SW)
> extra sysfs attribute shows up:
> 
> /sys/devices/system/cpu/cpu0/cpufreq/scaling_boost_frequencies [2]
> in which are listed only the boost frequencies.
> 

Correct, sorry I misunderstood this to dynamic change in
scaling_available_frequencies based on state of boot.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-02-10 Thread Sudeep Holla 
On 10/02/14 07:53, Lukasz Majewski wrote:
> Hi Thomas, Sudeep,
> 
>> On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon  wrote:
>>> On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla
>>>  wrote:
>>>> On 07/02/14 17:37, Nishanth Menon wrote:
>>>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
>>>>>  wrote:
>>>>
>>>> [...]
>>>>
>>>>>> Yes I think its counter-intuitive as it's visible to the
>>>>>> userspace(list of frequencies and the boost parameters are
>>>>>> exposed through sysfs)
>>>>>
>>>>> That will be a different problem -> as currently every single
>>>>> frequency in the cpufreq list has ability to be marked as boost
>>>>> frequency - if userspace does not maintain that, then, IMHO, fix
>>>>> the userspace :D
>>>>>
>>>>
>>>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>>>> gives the list of frequencies based on the state of the boost
>>>> feature at anytime.
>>>>
>>>> Intuitively the list without boost shouldn't have any frequency
>>>> above the range when it's enabled :), that's what I was referring
>>>> to. So I am not talking about any issue with user-space
>>>> maintenance.
>>> Fair enough - but i still think it has nothing to do with dt binding
>>> itself -> and i think the discussion we've had should be good for
>>> the binding provided in this patch.. I hope.. if documentation
>>> needs a bit of better explanation to prevent a repeat of the same
>>> discussion at a later point of time, now might be a good time to
>>> add it in.
>>
Nishanth, though I am not convinced that it should be list, since you have a
valid point that this should not prevent in having this feature, I am fine with
the list.

>> The term boost and over-clocking have been described in the bindings
>> document as being the same. Since the term over-clocking refers to
>> running a CPU beyond normal operating frequencies, I tend to agree
>> with Sudeep that it is not intuitive if a normal operating frequency
>> is greater than a boost mode frequency.
>>
>> Otherwise, when userspace does "echo 1 >
>> /sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
>> think the original intent of boost mode patches was to allow CPU to
>> operate at frequencies greater than the normal operating frequencies.
>>
>> Lukasz, how would you want this to be handled?
> 
> Please consider an example:
> 
> normal-freqs: 140, 120, 100, 80, 60, 40, 20
> [1]
> boost-freqs: 170, 160, 150. [2]
> 
> All those freqs shall be represented as OPPs freq and voltage tuple.
> 
> Best would be to specify all the boost-freqs as:
> boost-freqs = <170 160 150> to be prepared for future
> quirks or problems (or special cases which might show up latter).
> If anybody can foresee any potential threads - like platform on which
> boost freqs are 170 and 150, but not 160, then please share
> this information.
> 
If that's the case then why should it be included in the list of OPPs.
I know Nishanth had a valid point in other thread previously(like including
SoC.dtsi having OPPs in platform files), but that's different problem.

> However, I think that it would be also acceptable to specify only:
> boost-freq = <150> and mark all freqs greater or equal to it as
> CPUFREQ_BOOST_FREQ.
> 
> If there aren't any potential problems, then I think the second option
> would be a good solution (we would have only one BOOST attribute stored
> at CPUs DTS node).
> 

Yes I prefer this to keep it simple and as per the definition of overclocking
or turbo boost in hardware terms if possible.

Just another thought, not sure how much this is true for real platforms, sharing
it anyway. IIUC these boost frequencies have certain constraints like thermal
and can't be sustained all the processors for long time.

So does it make sense to specify normal max frequency instead of boost-frequency
which by definition would be: "The maximum frequency that all processors are can
sustain simultaneously without any thermal constraints".
Ignore this if this is not the definition of boost on real platforms.


Regards,
Sudeep


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 7/8] ARM: Exynos: switch to using generic cpufreq-cpu0 driver

2014-05-14 Thread Sudeep Holla 


On 14/05/14 15:03, Thomas Abraham wrote:

On Wed, May 14, 2014 at 6:41 PM, Heiko Stübner  wrote:

Am Mittwoch, 14. Mai 2014, 18:35:29 schrieb Viresh Kumar:

On 14 May 2014 18:20, Arnd Bergmann  wrote:

Could we please come up with a way to probe this from DT in the
cpufreq-cpu0 driver itself, so we don't have to add a device in every
platform using it?

Its followed that way because DT Maintainers had strong objections
to creating virtual device nodes and haven't allowed creation of nodes
for cpufreq drivers.. For which there is no physical device, as CPU already
has a separate node..


as we already have the "enable-method" property for enabling/disabling cpus,
would something like a "scaling-method" be feasible?



"scaling-method" also sounds like a software specific property. Would
that be something that will be acceptable in dt?


How about dvfs-method ? But the value should not be based on the driver they
use, but something more generic.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-05-14 Thread Sudeep Holla 



On 14/05/14 02:03, Thomas Abraham wrote:

From: Thomas Abraham 

Add a new optional boost-frequency binding for specifying the frequencies
usable in boost mode.

Cc: Nishanth Menon 
Cc: Lukasz Majewski 
Cc: Rob Herring 
Cc: Pawel Moll 
Cc: Mark Rutland 
Cc: Ian Campbell 
Cc: Kumar Gala 
Signed-off-by: Thomas Abraham 
---
  .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   11 +++
  1 files changed, 11 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt 
b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
new file mode 100644
index 000..d925e38
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
@@ -0,0 +1,11 @@
+* Device tree binding for CPU boost frequency (aka over-clocking)
+
+Certain CPU's can be operated in optional 'boost' mode (or sometimes referred 
as
+overclocking) in which the CPU can operate in frequencies beyond the normal
+operating conditions.
+
+Optional Properties:
+- boost-frequency: list of frequencies in KHz to be used only in boost mode.
+  This list should be a subset of frequencies listed in "operating-points"
+  property. Refer to Documentation/devicetree/bindings/power/opp.txt for
+  details about "operating-points" property.



Though I understand the need of it, I think the binding is designed to suffice
the need of the boost frequency support in cpufreq.

Typically SoC would provide characteristics like under-drive(hits performance
but most energy efficient), nominal(highest sustained performance w/o any
external constraint like power, thermal) and over-drive(maximum performance but
not sustainable for long periods)

IMO the binding could represent these unique points on the curve instead.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-05-30 Thread Sudeep Holla 



On 30/05/14 19:15, Tomasz Figa wrote:



On 30.05.2014 20:05, Thomas Abraham wrote:

Hi Mark,

On Fri, May 30, 2014 at 6:38 PM, Mark Rutland  wrote:

Hi,

Apologies for being somewhat late w.r.t. review on this.

On Fri, May 30, 2014 at 10:01:17AM +0100, Thomas Abraham wrote:

From: Thomas Abraham 

Add a new optional boost-frequency binding for specifying the frequencies
usable in boost mode.

Cc: Rob Herring 
Cc: Pawel Moll 
Cc: Mark Rutland 
Cc: Ian Campbell 
Cc: Kumar Gala 
Signed-off-by: Thomas Abraham 
Acked-by: Viresh Kumar 
Acked-by: Nishanth Menon 
Acked-by: Lukasz Majewski 
---
  .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   38 
  1 file changed, 38 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt 
b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
new file mode 100644
index 000..63ed0fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
@@ -0,0 +1,38 @@
+* Device tree binding for CPU boost frequency (aka over-clocking)
+
+Certain CPU's can be operated in optional 'boost' mode (or sometimes referred 
as


Nit: CPUs (we're not greengrocers [1])


+overclocking) in which the CPU can operate at frequencies which are not
+specified by the manufacturer as CPU's operating frequency.
+
+Optional Properties:
+- boost-frequencies: list of frequencies in KHz to be used only in boost mode.
+  This list should be a subset of frequencies listed in "operating-points"
+  property. Refer to Documentation/devicetree/bindings/power/opp.txt for
+  details about "operating-points" property.


What is 'boost-mode'?


boost-mode activates additional one or more cpu clock speeds (which
are not specified as operating frequency of the cpu by the
manufacturer). The cpu is allowed to operate in these boost mode
speeds when the boost mode is active. The boost mode speeds are
usually undocumented. Some of the chip samples could be clocked in
boost mode speeds and only such samples can be safely operated in
boost mode.



IMHO the most important part that I believe should be stated in the
documentation is that CPU usually can operate in boost mode for limited
amount of time, which depends on thermal conditions, which makes the
boost operating points separate from normal ones, which can be used at
any time.



Yes exactly what I mentioned couple of times on previous version of this
patch set[1][2]


The mechanism of entry into and exit out of boost mode is outside the
scope of this documentation.



What are the limitations on boost frequencies? When is a CPU expected to
go to these frequencies and for now long? When should I as a dt author
place elements in boost-frequencies?


I will add these details in the next revision of this patch.



Why are these in both operating-points and boost-frequencies? It'll be
really easy to accidentally forget to mark something as a
boost-frequency this way. Why not have a boost-points instead?




I was told that index is not preferred based on the previous discussions
when the OPP bindings were designed. In addition the OPP binding doesn't
enforce any ordering. There are thermal bindings that assume otherwise and
is broken. So boost-points is not feasible.


Does boost-points mean a set of clock speeds which are not listed as
part of operating-points property? If yes, that also is a possible
implementation (it was implemented in one of the earlier version of
this series). Could you confirm that you want the boost mode speeds to
be exclusive of the speeds listed in operating-points?


It seems reasonable to have boost operating points completely separate
from normal ones, so that a kernel without support for boost mode will
not use them. Also considering my comment above, logically boost
operating points are not considered normal operating points, due to
various constraints that need to be met to use them (i.e. mostly thermal
conditions).



IMO, at-least the existing OPP binding can't distinguish between under-,
nominal- and over-drive OPP points. So my suggestion was to have a property
that provides the beginning of these 3 points on the OPP curve.

Regards,
Sudeep

[1] https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg26250.html
[2] http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/31552

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

2014-05-30 Thread Sudeep Holla 



On 30/05/14 19:41, Tomasz Figa wrote:

On 30.05.2014 20:38, Sudeep Holla wrote:

On 30/05/14 19:15, Tomasz Figa wrote:

On 30.05.2014 20:05, Thomas Abraham wrote:

On Fri, May 30, 2014 at 6:38 PM, Mark Rutland 
wrote:


[snip]


Why are these in both operating-points and boost-frequencies? It'll be
really easy to accidentally forget to mark something as a
boost-frequency this way. Why not have a boost-points instead?




I was told that index is not preferred based on the previous discussions
when the OPP bindings were designed. In addition the OPP binding doesn't
enforce any ordering. There are thermal bindings that assume otherwise and
is broken. So boost-points is not feasible.



My understanding of Mark's comment was that the boost-points property
would use the same format as operating-points and parsing code would
just concatenate operating points with boost points after making the
latter with necessary flag or whatever.



Ah, I misunderstood that. That should be fine as it avoids duplication.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 16/19] ARM: dts: s5pv210: replace legacy *,wakeup property with wakeup-source

2015-10-21 Thread Sudeep Holla 
Though the keyboard and other driver will continue to support the legacy
"gpio-key,wakeup", "linux,input-wakeup" boolean property to enable the
wakeup source, "wakeup-source" is the new standard binding.

This patch replaces all the legacy wakeup properties with the unified
"wakeup-source" property in order to avoid any futher copy-paste
duplication.

Cc: Marek Szyprowski 
Cc: Kukjin Kim 
Cc: linux-samsung-soc@vger.kernel.org
Signed-off-by: Sudeep Holla 
---
 arch/arm/boot/dts/s5pv210-aquila.dts   | 4 ++--
 arch/arm/boot/dts/s5pv210-goni.dts | 4 ++--
 arch/arm/boot/dts/s5pv210-smdkv210.dts | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts 
b/arch/arm/boot/dts/s5pv210-aquila.dts
index f00cea7aca2f..72f162e7040b 100644
--- a/arch/arm/boot/dts/s5pv210-aquila.dts
+++ b/arch/arm/boot/dts/s5pv210-aquila.dts
@@ -257,7 +257,7 @@
linux,code = ;
label = "power";
debounce-interval = <1>;
-   gpio-key,wakeup;
+   wakeup-source;
};
};
 };
@@ -268,7 +268,7 @@
 
 &keypad {
linux,input-no-autorepeat;
-   linux,input-wakeup;
+   wakeup-source;
samsung,keypad-num-rows = <3>;
samsung,keypad-num-columns = <3>;
pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/s5pv210-goni.dts 
b/arch/arm/boot/dts/s5pv210-goni.dts
index a3d4643b202e..f4d0a023c31c 100644
--- a/arch/arm/boot/dts/s5pv210-goni.dts
+++ b/arch/arm/boot/dts/s5pv210-goni.dts
@@ -239,7 +239,7 @@
linux,code = ;
label = "power";
debounce-interval = <1>;
-   gpio-key,wakeup;
+   wakeup-source;
};
};
 };
@@ -250,7 +250,7 @@
 
 &keypad {
linux,input-no-autorepeat;
-   linux,input-wakeup;
+   wakeup-source;
samsung,keypad-num-rows = <3>;
samsung,keypad-num-columns = <3>;
pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/s5pv210-smdkv210.dts 
b/arch/arm/boot/dts/s5pv210-smdkv210.dts
index da7d210df670..54fcc3fc82e2 100644
--- a/arch/arm/boot/dts/s5pv210-smdkv210.dts
+++ b/arch/arm/boot/dts/s5pv210-smdkv210.dts
@@ -59,7 +59,7 @@
 
 &keypad {
linux,input-no-autorepeat;
-   linux,input-wakeup;
+   wakeup-source;
samsung,keypad-num-rows = <8>;
samsung,keypad-num-columns = <8>;
pinctrl-names = "default";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/19] ARM: dts: exynos: replace legacy *,wakeup property with wakeup-source

2015-10-21 Thread Sudeep Holla 
Though the keyboard and other driver will continue to support the legacy
"gpio-key,wakeup", "linux-keypad,wakeup" boolean property to enable the
wakeup source, "wakeup-source" is the new standard binding.

This patch replaces all the legacy wakeup properties with the unified
"wakeup-source" property in order to avoid any futher copy-paste
duplication.

Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: linux-samsung-soc@vger.kernel.org
Signed-off-by: Sudeep Holla 
---
 arch/arm/boot/dts/exynos3250-monk.dts   |  6 +++---
 arch/arm/boot/dts/exynos3250-rinato.dts |  6 +++---
 arch/arm/boot/dts/exynos4210-origen.dts | 10 +-
 arch/arm/boot/dts/exynos4210-smdkv310.dts   |  2 +-
 arch/arm/boot/dts/exynos4210-trats.dts  |  2 +-
 arch/arm/boot/dts/exynos4210-universal_c210.dts |  4 ++--
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi |  2 +-
 arch/arm/boot/dts/exynos4412-odroidx.dts|  2 +-
 arch/arm/boot/dts/exynos4412-origen.dts |  2 +-
 arch/arm/boot/dts/exynos4412-smdk4412.dts   |  2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts |  4 ++--
 arch/arm/boot/dts/exynos5250-arndale.dts| 12 ++--
 arch/arm/boot/dts/exynos5250-snow.dts   |  4 ++--
 arch/arm/boot/dts/exynos5250-spring.dts |  4 ++--
 arch/arm/boot/dts/exynos5420-arndale-octa.dts   |  2 +-
 arch/arm/boot/dts/exynos5420-peach-pit.dts  |  4 ++--
 arch/arm/boot/dts/exynos5800-peach-pi.dts   |  4 ++--
 17 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250-monk.dts 
b/arch/arm/boot/dts/exynos3250-monk.dts
index 540a0adf2be6..b77ea2163a0a 100644
--- a/arch/arm/boot/dts/exynos3250-monk.dts
+++ b/arch/arm/boot/dts/exynos3250-monk.dts
@@ -43,7 +43,7 @@
linux,code = ;
label = "power key";
debounce-interval = <10>;
-   gpio-key,wakeup;
+   wakeup-source;
};
};
 
@@ -67,7 +67,7 @@
interrupt-parent = <&gpx1>;
interrupts = <5 0>;
reg = <0x25>;
-   wakeup;
+   wakeup-source;
 
muic: max77836-muic {
compatible = "maxim,max77836-muic";
@@ -184,7 +184,7 @@
interrupt-parent = <&gpx0>;
interrupts = <7 0>;
reg = <0x66>;
-   wakeup;
+   wakeup-source;
 
s2mps14_osc: clocks {
compatible = "samsung,s2mps14-clk";
diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts 
b/arch/arm/boot/dts/exynos3250-rinato.dts
index 41a5fafb9aa9..7acd11d2d957 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -43,7 +43,7 @@
linux,code = ;
label = "power key";
debounce-interval = <10>;
-   gpio-key,wakeup;
+   wakeup-source;
};
};
 
@@ -58,7 +58,7 @@
interrupt-parent = <&gpx1>;
interrupts = <5 0>;
reg = <0x25>;
-   wakeup;
+   wakeup-source;
 
muic: max77836-muic {
compatible = "maxim,max77836-muic";
@@ -245,7 +245,7 @@
interrupt-parent = <&gpx0>;
interrupts = <7 0>;
reg = <0x66>;
-   wakeup;
+   wakeup-source;
 
s2mps14_osc: clocks {
compatible = "samsung,s2mps14-clk";
diff --git a/arch/arm/boot/dts/exynos4210-origen.dts 
b/arch/arm/boot/dts/exynos4210-origen.dts
index e050d85cdacd..9e97f6065493 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -59,35 +59,35 @@
label = "Up";
gpios = <&gpx2 0 1>;
linux,code = ;
-   gpio-key,wakeup;
+   wakeup-source;
};
 
down {
label = "Down";
gpios = <&gpx2 1 1>;
linux,code = ;
-   gpio-key,wakeup;
+   wakeup-source;
};
 
back {
label = "Back";
gpios = <&gpx1 7 1>;
linux,code = ;
-   gpio-key,wakeup;
+   wakeup-source;
};

Re: [PATCH v5 0/12] cpufreq: Add support for Exynos 5800, 5420, and 5422

2015-12-03 Thread Sudeep Holla 



On 03/12/15 06:05, Viresh Kumar wrote:

[...]


@Sudeep: What would it take you to use cpufreq-dt for ARM's platforms
?



The main difference is that we get the OPPs from the firmware rather
than DT. We may just need to abstract that part and we should be able to
use it. I will have a look at it and get back to you will more details.
It has been a while since I looked at cpufreq-dt.

--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

2015-06-12 Thread Sudeep Holla 



On 12/06/15 06:43, Javier Martinez Canillas wrote:

The Exynos interrupt combiner IP loses its state when the SoC enters
into a low power state during a Suspend-to-RAM. This means that if a
IRQ is used as a source, the interrupts for the devices are disabled
when the system is resumed from a sleep state so are not triggered.

Save the interrupt enable set register for each combiner group and
restore it after resume to make sure that the interrupts are enabled.



Not sure if you need this. IMO it's not clean and redundant though I
admit many drivers do exactly same thing. I am trying to remove or point
out those redundant code as irqchip core has options/flags to do what
you need. I assume there are no wakeup sources connected to this
combiner. Setting irqchip flags should solve this problem. A simple
patch below should do the job ?

-->8

diff --git a/drivers/irqchip/exynos-combiner.c 
b/drivers/irqchip/exynos-combiner.c

index 5945223b73fa..c0bcec59f829 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -111,6 +111,7 @@ static struct irq_chip combiner_chip = {
 #ifdef CONFIG_SMP
.irq_set_affinity   = combiner_set_affinity,
 #endif
+   .flags  = IRQCHIP_MASK_ON_SUSPEND,
 };


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

2015-06-12 Thread Sudeep Holla 



On 12/06/15 11:42, Krzysztof Kozlowski wrote:

On 12.06.2015 19:10, Sudeep Holla wrote:



On 12/06/15 06:43, Javier Martinez Canillas wrote:

The Exynos interrupt combiner IP loses its state when the SoC enters
into a low power state during a Suspend-to-RAM. This means that if a
IRQ is used as a source, the interrupts for the devices are disabled
when the system is resumed from a sleep state so are not triggered.

Save the interrupt enable set register for each combiner group and
restore it after resume to make sure that the interrupts are enabled.



Not sure if you need this. IMO it's not clean and redundant though I
admit many drivers do exactly same thing. I am trying to remove or point
out those redundant code as irqchip core has options/flags to do what
you need. I assume there are no wakeup sources connected to this
combiner.


It may have wake up sources connected. Correct me if I am wrong but (at
least) on Exynos5250 combiner takes care of gpx1 GPIO pins which may be
external interrupts (e.g. power key on Exynos5250 Snow). I didn't check
other boards.



In that case, this irqchip should implement irq_set_wake and the driver
implementing power key should use enable_irq_wake in the suspend path.
Just saving all the mask/enable registers is not scalable solution and
also useless if it's just one or to interrupts that are very few IRQs
registered/actively being used.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

2015-06-12 Thread Sudeep Holla 



On 12/06/15 12:27, Javier Martinez Canillas wrote:

Hello Sudeep,

Thanks a lot for the feedback.

On 06/12/2015 12:10 PM, Sudeep Holla wrote:



On 12/06/15 06:43, Javier Martinez Canillas wrote:

The Exynos interrupt combiner IP loses its state when the SoC enters
into a low power state during a Suspend-to-RAM. This means that if a
IRQ is used as a source, the interrupts for the devices are disabled
when the system is resumed from a sleep state so are not triggered.

Save the interrupt enable set register for each combiner group and
restore it after resume to make sure that the interrupts are enabled.



Not sure if you need this. IMO it's not clean and redundant though I
admit many drivers do exactly same thing. I am trying to remove or point
out those redundant code as irqchip core has options/flags to do what
you need. I assume there are no wakeup sources connected to this


Yes, there are wakeup sources connected to this combiner. As Krzysztof
said some wakeup sources use a GPIO line as IRQ whose interrupt parent
is the combiner. As an example the Power gpio-key and cros_ec keyboard
for both Exynos5250 Snow and Exynos5420 Peach Pit/Pi.

Both drivers/input/keyboard/gpio_keysc and drivers/mfd/cros_ec.c do the
right thing though and call {enable,disable}_irq_wake() on the S2R path.

And in fact, the peripherals resume the system after a suspend but after
resume, interrupts are not triggered anymore.



I agree for the wakeup source, but I need to go through the irqchip core
again to understand this better.


combiner. Setting irqchip flags should solve this problem. A
simple patch below should do the job ?



I don't see how the below patch is going to work for the case I'm
trying to solve. If I understand correctly, the
IRQCHIP_MASK_ON_SUSPEND flag is used to force masking non wakeup
interrupt in the suspend path (instead of just disabling the
interrupts on suspend and not masking at the hardware level)



It also takes re-enables all the IRQs in the resume path that were
disabled on the suspend path.


But my problem is not about interrupts needed to be masked on suspend
but the opposite, that interrupts have to be unmasked on resume since
the IP loses its state so after a resume, interrupts are not
triggered anymore.



As I mentioned above this happens for all non-wake up interrupts.
However this needs to done for wake up interrupts IIUC. BTW if these
registers are lost assuming the combiner was powered down, even the
status register will be lost and you will not know exactly the wakeup
reason right ?


So I also don't see how implementing irq_set_wake() as you suggested
is going to work. Maybe we need a IRQCHIP_UNMASK_ON_RESUME flag for
this case?



IIRC these combiner feeds to main interrupt controller and you need to
make sure that interrupt is set as wakeup if required. Right ? so you
may still need irq_set_wake IMO.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

2015-06-15 Thread Sudeep Holla 



On 12/06/15 21:17, Doug Anderson wrote:

Hi,

On Fri, Jun 12, 2015 at 12:36 PM, Javier Martinez Canillas
 wrote:

registers are lost assuming the combiner was powered down, even the
status register will be lost and you will not know exactly the wakeup
reason right ?



Good question, I didn't find in the documentation I've access to that
this happen but just found through experimentation that the IRQ enable
set register values are lost after a resume and that saving/restoring
the values makes the interrupts to be triggered again.



I'll share here too the findings I mentioned over IRC. As you suggested I
add some printouts and noticed that the ISTRn (Interrupt Status) registers
values are indeed preserved on resume so I can know for example if the
wakeup source was the power gpio-key or cros_ec keyboard. I've checked the
values of the registers against the Exynos manual and they corresponds to
the interrupt sources in each case so the values are correct.

So as you said, it seems that is not that the IP block loses its state on
S2R but that something is blindly writing the IESRn (Interrupt Enable Set)
registers.


I'll postulate an alternate theory here.  You can tell me if you buy
it or if you think I've been out in the sun too long.

Let's say that the interrupt combiner's status registers show the raw
status as asserted by whatever is hooked up to the combiner.  This
means that even if the combiner got reset we could still read the
status register and get the status of the source.  Imagine that the
combiner is like a GPIO bank.  If you reset the GPIO bank, you'll lose
all kinds of config (input vs. output, edge interrupt status, maybe
pulls, etc).  ...but you can still read the state asserted by an
external source, right?



Right, this makes sense. I assumed status was latched output and might
be lost. But that's wrong assumption I believe.


In this case the combiner's interrupt source is 'EINT 11'.  ...and I'm
pretty sure that the controller managine 'EINT 11' _doesn't_ lose
power across suspend/resume...


I'll further bolster my theory by saying that _almost nothing_ in the
exynos keeps power across suspend/resume.  The UART?  Nope.  The GPIO
controllers?  Nuh-uh.  The GIC?  Sorry, but no.  The clock tree?  It
might be nice, but you're out of luck.  ...so it would make me
terribly surprised to see the combiner keep power.



Interesting even GIC loses power ? I would be interested in knowing more
details as who will wake up the system then. Is the wakeup source
offloaded to some external power controller ?




To reduce the possible s/w components that could be doing this, I booted a
signed FIT image directly using the RO U-Boot instead of chain loading a
mainline nv-uboot. In this configuration I've the same issue so it seems
that if something is zeroing those registers on S2R, this can't be changed
without void the warranty of these machines.

I also looked at the downstream ChromiumOS v3.8 tree [0] and I see that
they have a very similar solution than my patch, the IESRn are also saved
but using a notifier for the CPU_PM_ENTER and CPU_PM_EXIT events instead
or registering a syscore ops but the idea is basically the same.


Yup, you can see where kliegs added it in
.  As per the
comments in that CL, this was probably broken in:

063bd6f ARM: EXYNOS: Remove GIC save & restore function



I have to take a look to the U-boot that is shipped on the device, I think
the correct branch is [1] but I'm not sure if that is the correct one.


It is the right one.  If U-Boot were touching this (which would
greatly surprise me) it should be here:

arch/arm/include/asm/arch-exynos/cpu.h

...and it's not.  Doing a grep for '1044' (the combiner base
address) doesn't find anything in U-Boot either, which makes it less
likely.  ...and it's even less likely since the amount of code that is
in U-Boot that runs at resume time is a very small subset and I'm
fairly familiar with it and I would have remembered it touching the
combiner.



Yes, I did search and find nothing, so definitely not U-Boot if I am
looking at the right source.


It's POSSIBLE that the internal ROM in exynos is clobbering this, but
as per above it seems crazy unlikely and I think it's just losing
power.



Agreed, not because I believe ROM code are not crazy but because of the
theory you have provided above :)

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

2015-06-15 Thread Sudeep Holla 



On 15/06/15 08:46, Javier Martinez Canillas wrote:
[...]



Sudeep, so we may need something like $subject after all from Doug's
explanations since the combiner chip state is lost during a S2R. I know
that it adds more duplicated code (others irqchip drivers do the same)
and it may not scale well if a chip has many registers but is the best
solution I could came with.



OK


If you have a suggestion for a better alternative, I can give a try and
write the patch. But I think $subject could also land to fix this issue
since is a very non intrusive change and later can be changed once the
irqchip core supports this use case.



Agreed. But I would suggest also to add MASK_ON_SUSPEND and set_irq_wake
also and then you can restore iff it's non-zero as irq core will take
care of most of the non-wakeup sources. Because I am planning to push
MASK_ON_SUSPEND to GIC and it will break this combiner if it assumes the
combiner interrupts are always on in GIC. Implement set_irq_wake as
enable_irq_wake (comb_irq_to_GIC).

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

2015-06-15 Thread Sudeep Holla 



On 15/06/15 16:00, Javier Martinez Canillas wrote:

Hello Sudeep,

On 06/15/2015 11:01 AM, Sudeep Holla wrote:



On 15/06/15 08:46, Javier Martinez Canillas wrote:
[...]



Sudeep, so we may need something like $subject after all from Doug's
explanations since the combiner chip state is lost during a S2R. I know
that it adds more duplicated code (others irqchip drivers do the same)
and it may not scale well if a chip has many registers but is the best
solution I could came with.



OK


If you have a suggestion for a better alternative, I can give a try and
write the patch. But I think $subject could also land to fix this issue
since is a very non intrusive change and later can be changed once the
irqchip core supports this use case.



Agreed. But I would suggest also to add MASK_ON_SUSPEND and set_irq_wake
also and then you can restore iff it's non-zero as irq core will take
care of most of the non-wakeup sources. Because I am planning to push


I've looking at this and a problem I found is that IIUC the set_irq_wake
is not propagated from the the Exynos pinctrl / GPIO driver which is the
combiner's external interrupt source so the callback is never called.
Which means that right now only the state of the wakeup source IRQs can't
be saved since that information is not present.

The drivers/pinctrl/samsung/pinctrl-exynos.c driver enables and disables
the combiner interrupts but its .irq_set_wake handler only updates the
wakeup source mask for the external interrupts but does not call the
combiner .set_irq_wake so that should be changed as well.



Thanks for the looking at this.


But even for non-wakeup interrupts, I found that they are not enabled when
adding the MASK_ON_SUSPEND on my tests. I don't know if that is related
to the pinctrl driver missing something else though.



Even GIC is not masking any interrupt on suspend and if other irqchip or
drivers are assuming that, it will work fine for now. But once I
introduce that change in GIC it will break.


MASK_ON_SUSPEND to GIC and it will break this combiner if it assumes the
combiner interrupts are always on in GIC. Implement set_irq_wake as
enable_irq_wake (comb_irq_to_GIC).



Right, but can we do this as a follow-up? S2R is currently broken on these
machines and $subject is I think a reasonable small fix that won't introduce
any regressions.



No issues, I just wanted to understand the issue better and also make
sure we understand that things might break in future once that GIC
change is introduced. So we already know the reason or atleast have some
basic understanding as why would it break if it breaks :)


To do a more intrusive change, I should better understand the interactions
between the Exynos pinctrl / GPIO, interrupt combiner and the GIC and in the
meantime S2R will continue to be broken on these platforms unless someone
more familiar with all this could point me in the right direction.



As I said I am fine with this patch for now and I don't want to block it.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] irqchip: exynos-combiner: Save IRQ enable set on suspend

2015-06-16 Thread Sudeep Holla 



On 16/06/15 13:32, Tomasz Figa wrote:

2015-06-16 0:00 GMT+09:00 Javier Martinez Canillas 
:

On 06/15/2015 11:01 AM, Sudeep Holla wrote:


[...]



Agreed. But I would suggest also to add MASK_ON_SUSPEND and
set_irq_wake also and then you can restore iff it's non-zero as
irq core will take care of most of the non-wakeup sources.
Because I am planning to push


I've looking at this and a problem I found is that IIUC the
set_irq_wake is not propagated from the the Exynos pinctrl / GPIO
driver which is the combiner's external interrupt source so the
callback is never called. Which means that right now only the
state of the wakeup source IRQs can't be saved since that
information is not present.

The drivers/pinctrl/samsung/pinctrl-exynos.c driver enables and
disables the combiner interrupts but its .irq_set_wake handler
only updates the wakeup source mask for the external interrupts but
does not call the combiner .set_irq_wake so that should be changed
as well.



As far as I'm aware of, wake-up events from pin controllers don't go
 through GIC, but rather directly to PMU, which is a dedicated unit
responsible for power management and not a standalone interrupt
controller (well actually I saw a series making it a cascaded
controller some time ago, but I'm not sure if that went in). Based on
this, I don't think we have to call set_irq_wake on GIC. Correct me
if I'm wrong, though.


Thanks for the details, this was my assumption when Doug confirmed that
the combiner is also powered down, either there must be some bypass or a
dedicated logic to wakeup. But I was not sure though so insisted
set_irq_wake but based on what you say it's not required.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 4/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210/5250/5420

2014-11-19 Thread Sudeep Holla 



On 20/10/14 12:41, Thomas Abraham wrote:

The new CPU clock type allows the use of generic CPUfreq drivers. So for
Exynos4210/5250, switch to using generic cpufreq driver. For Exynos5420,
which did not have CPUfreq driver support, enable the use of generic
CPUfreq driver.

Suggested-by: Tomasz Figa 
Cc: Kukjin Kim 
Signed-off-by: Thomas Abraham 
Reviewed-by: Tomasz Figa 
Tested-by: Javier Martinez Canillas 
Tested-by: Chander Kashyap 
---
  arch/arm/mach-exynos/exynos.c |   24 +++-
  1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 6b283eb..a1be294 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -282,6 +282,28 @@ static void __init exynos_init_irq(void)
exynos_map_pmu();
  }

+static const struct of_device_id exynos_cpufreq_matches[] = {
+   { .compatible = "samsung,exynos5420", .data = "arm-bL-cpufreq-dt" },


Sorry for raising this issue always with Exynos cpufreq drivers. IMO the
bindings for "arm-bL-cpufreq-dt" is broken. Currently no one is using it
and it's better to fix it before we have a real user of it.

If you look at the binding document for it[1], it has a fixme which
shouldn't have been there at first place. It assumes the ordering of
CPU's specified in the DT and the logical index allocation to them.

It even breaks for hotplug especially if you hotplug-in back in
different order. We can work around that probably, but it's better to
fix the binding. I failed to grab much attention in my previous attempts
to address this[2]. Viresh also started a discussion more recently[3].

Regards,
Sudeep

[1] Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
[2] http://www.spinics.net/lists/arm-kernel/msg303977.html
[3] https://lkml.org/lkml/2014/6/25/152

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 4/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210/5250/5420

2014-11-24 Thread Sudeep Holla 



On 20/11/14 03:48, Viresh Kumar wrote:

Oh, you are still alive? I thought you were about to get married :)
Just kidding !!

On 20 November 2014 00:58, Sudeep Holla  wrote:

Sorry for raising this issue always with Exynos cpufreq drivers. IMO the
bindings for "arm-bL-cpufreq-dt" is broken. Currently no one is using it
and it's better to fix it before we have a real user of it.


Hmm, yeah if we can. I haven't found a easy way to go ahead and then
got caught in other activities.



Agreed, it's not easy but needs to be fixed :)


If you look at the binding document for it[1], it has a fixme which
shouldn't have been there at first place. It assumes the ordering of
CPU's specified in the DT and the logical index allocation to them.


Ok, I believe the FIXME is a bit outdated. From the code I can see only
this limitation.

- For every cluster, the cpu which boots up first should carry the OPPs.
Otherwise there is no restriction on ordering of CPUs.



Not entirely true, it assuming the fact that the logical index provided 
by the OS is completely based on the ordering in the DT.



- I believe CPUs boot in the order they are present in DT except for the
boot CPU. So, the first node for every cluster should have it.

Correct ? Then we can update the fixme.



No, I disagree as you trying to implicitly depend on the logic Linux 
uses to assign logical index. It can be changed any time, and depending

on it might break things in future which can't be fixed easily later
especially if it's DT related.


It even breaks for hotplug especially if you hotplug-in back in
different order.


Hmm, I never thought about it. But yes the CPU with the OPPs should
be the first one to come back.



How can you assume that ? I can write a simple test which hot-plugs out
all the CPUs in the (esp. multi-cluster) system in ascending order of
logical index and hot-plug back in descending order. Then the above
logic fails.


We can work around that probably, but it's better to
fix the binding. I failed to grab much attention in my previous attempts
to address this[2]. Viresh also started a discussion more recently[3].


They are just stuck and went nowhere :(



The platforms needing it have to get involved in such discussions to
make any progress.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: exynos_defconfig: disable CONFIG_EXYNOS5420_MCPM; not stable

2014-11-27 Thread Sudeep Holla 



On 26/11/14 18:41, Nicolas Pitre wrote:

On Wed, 26 Nov 2014, Kevin Hilman wrote:


Abhilash Kesavan  writes:


Hi Kevin,

On Wed, Nov 26, 2014 at 6:30 AM, Kevin Hilman  wrote:

[...]

More specifically, with only the loopback call to turn off CCI commented
out, the imprecise aborts go away.


I can't see how enabling snoops for the boot cluster is causing these
aborts. Perhaps as Krzysztof commented it has something to do with the
secure firmware/tz software on these boards ? Other than there does
not appear to be any difference between the working/non-working
setups.


Perhaps the secure firmware is preventing the CCI to be enabled by the
kernel, and that is causing the imprecise abort?


That is well possible.

Now.. if the bootloader/firmware does not let Linux deal with both
the CCI and caches then MCPM simply has no more purpose for this board.
The whole point of MCPM is actually to handle the CCI properly and the
most efficient way despite all the possible races and opportunities for
memory corruptions. And yes, this is a complex task.

So there is actually two choices: the firmware let Linux take care of it
via the MCPM layer (easy), or the firmware has to implement it all
_properly_ (hard) behind an interface such as PSCI, at which point MCPM
should be configured out.

If the firmware does not let Linux interact with the CCI _and_ does not
implement full MCPM-like services then the platform is broken and only a
firmware upgrade could fix that.  It might still be possible to boot all
CPUs through other means, but power management would then be severely
limited.



Thanks Nico for the detailed description on the requirements for using
MCPM. This is the kind of issue I was worried in the other thread on
Fijitsu platform. That's the reason I was asking the information about
their secure firmware and what exactly it configures so that we won't
end up with similar situation on there too and definitely not to push
PSCI. I completely agree with you that making a some change in firmware
to give control of CCI to kernel is easy.

Probably if the vendors disagree to apply this small fix to the firmware
we should provide them with *only choice* of PSCI implementation which
is quite complex and easy to get it wrong. That might trigger them to
provide a small fix to use MCPM.

Regards,
Sudeep



--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFT 1/2] drivers: bus: check cci device tree node status

2014-12-09 Thread Sudeep Holla 

Hi Abhilash,

On Wednesday 10 December 2014 09:31 AM, Abhilash Kesavan wrote:

Hi,

On Fri, Nov 28, 2014 at 8:20 PM, Abhilash Kesavan  wrote:

The arm-cci driver completes the probe sequence even if the cci node is
marked as disabled. Add a check in the driver to honour the cci status
in the device tree.

Signed-off-by: Abhilash Kesavan 


This patch helps disable CCI on the Arndale Octa board thus resolving
some imprecise aborts seen on that board. Kindly review.

Regards,
Abhilash

---
  drivers/bus/arm-cci.c |3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 860da40..0ce5e2d 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -1312,6 +1312,9 @@ static int cci_probe(void)
 if (!np)
 return -ENODEV;

+   if (!of_device_is_available(np))
+   return -ENODEV;
+


IIUC, by this change you are disabling the MCPM boot protocol here.
Is there any alternative boot protocol that works on this platform
to boot all 8 cores ? Sorry by quick grep couldn't find one, hence
so I am asking.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFT 1/2] drivers: bus: check cci device tree node status

2014-12-09 Thread Sudeep Holla 



On Wednesday 10 December 2014 09:55 AM, Abhilash Kesavan wrote:

Hi Sudeep,

On Wed, Dec 10, 2014 at 9:44 AM, Sudeep Holla  wrote:

Hi Abhilash,

On Wednesday 10 December 2014 09:31 AM, Abhilash Kesavan wrote:


Hi,

On Fri, Nov 28, 2014 at 8:20 PM, Abhilash Kesavan 
wrote:


The arm-cci driver completes the probe sequence even if the cci node is
marked as disabled. Add a check in the driver to honour the cci status
in the device tree.

Signed-off-by: Abhilash Kesavan 



This patch helps disable CCI on the Arndale Octa board thus resolving
some imprecise aborts seen on that board. Kindly review.

Regards,
Abhilash


---
   drivers/bus/arm-cci.c |3 +++
   1 file changed, 3 insertions(+)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 860da40..0ce5e2d 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -1312,6 +1312,9 @@ static int cci_probe(void)
  if (!np)
  return -ENODEV;

+   if (!of_device_is_available(np))
+   return -ENODEV;
+



IIUC, by this change you are disabling the MCPM boot protocol here.
Is there any alternative boot protocol that works on this platform
to boot all 8 cores ? Sorry by quick grep couldn't find one, hence
so I am asking.


Thanks for the reply.
On disabling MCPM, we will default to platsmp.c/firmware.c which boots
4 cores as per Kevin's comment here[1]. This was the original behavior
before MCPM was enabled for all 5420 based SoCs.



Thanks for pointing that out. I assume the firmware can handle the
alternate boot protocol and no more workarounds are needed especially
when getting CPUIdle working in this mode.

Anyways the patch makes sense irrespective how it works on exynos, so
you can add,

Acked-by: Sudeep Holla 

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFT 1/2] drivers: bus: check cci device tree node status

2015-01-07 Thread Sudeep Holla 

Hi Abhilash,

On Wednesday 10 December 2014 10:46 AM, Sudeep Holla wrote:



On Wednesday 10 December 2014 09:55 AM, Abhilash Kesavan wrote:

Hi Sudeep,

On Wed, Dec 10, 2014 at 9:44 AM, Sudeep Holla  wrote:

Hi Abhilash,

On Wednesday 10 December 2014 09:31 AM, Abhilash Kesavan wrote:


Hi,

On Fri, Nov 28, 2014 at 8:20 PM, Abhilash Kesavan 
wrote:


The arm-cci driver completes the probe sequence even if the cci node is
marked as disabled. Add a check in the driver to honour the cci status
in the device tree.

Signed-off-by: Abhilash Kesavan 



This patch helps disable CCI on the Arndale Octa board thus resolving
some imprecise aborts seen on that board. Kindly review.

Regards,
Abhilash


---
drivers/bus/arm-cci.c |3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 860da40..0ce5e2d 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -1312,6 +1312,9 @@ static int cci_probe(void)
   if (!np)
   return -ENODEV;

+   if (!of_device_is_available(np))
+   return -ENODEV;
+



IIUC, by this change you are disabling the MCPM boot protocol here.
Is there any alternative boot protocol that works on this platform
to boot all 8 cores ? Sorry by quick grep couldn't find one, hence
so I am asking.


Thanks for the reply.
On disabling MCPM, we will default to platsmp.c/firmware.c which boots
4 cores as per Kevin's comment here[1]. This was the original behavior
before MCPM was enabled for all 5420 based SoCs.



Thanks for pointing that out. I assume the firmware can handle the
alternate boot protocol and no more workarounds are needed especially
when getting CPUIdle working in this mode.

Anyways the patch makes sense irrespective how it works on exynos, so
you can add,

Acked-by: Sudeep Holla 



What's the status of this patch. It was useful for me on vexpress for 
some testing. Please feel free to add


Tested-by: Sudeep Holla 

if this is not yet queued.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFT 1/2] drivers: bus: check cci device tree node status

2015-01-08 Thread Sudeep Holla 



On Thursday 08 January 2015 08:57 PM, Abhilash Kesavan wrote:

Hi Sudeep,

On Thu, Jan 8, 2015 at 12:15 PM, Sudeep Holla  wrote:

Hi Abhilash,


[...]



What's the status of this patch. It was useful for me on vexpress for some
testing. Please feel free to add

Tested-by: Sudeep Holla 

if this is not yet queued.


Thanks for the tested-by. This patch has not been merged yet; I am not
quite sure who is supposed to pick this up.



So far, most of the CCI patches are merged through arm-soc.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] irqchip: exynos-combiner: Fix compilation error on ARM64

2014-09-02 Thread Sudeep Holla 



On 02/09/14 16:24, Naveen Krishna Chatradhi wrote:

The following compilation error occurs on 64-bit Exynos7 SoC:

drivers/irqchip/exynos-combiner.c: In function ‘combiner_irq_domain_map’:
drivers/irqchip/exynos-combiner.c:162:2: error: implicit declaration of 
function ‘set_irq_flags’ [-Werror=implicit-function-declaration]
   set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
   ^
drivers/irqchip/exynos-combiner.c:162:21: error: ‘IRQF_VALID’ undeclared (first 
use in this function)
   set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
  ^
drivers/irqchip/exynos-combiner.c:162:21: note: each undeclared identifier is 
reported only once for each function it appears in
drivers/irqchip/exynos-combiner.c:162:34: error: ‘IRQF_PROBE’ undeclared (first 
use in this function)
   set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);

Fix the build error by including asm/hardirq.h.



You should avoid using asm headers whenever possible esp. in driver
code. linux/hardirq.h or much better linux/interrupt.h should fix this
error.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/19] ARM: dts: exynos: replace legacy *,wakeup property with wakeup-source

2015-12-15 Thread Sudeep Holla 



On 21/10/15 11:10, Sudeep Holla wrote:

Though the keyboard and other driver will continue to support the legacy
"gpio-key,wakeup", "linux-keypad,wakeup" boolean property to enable the
wakeup source, "wakeup-source" is the new standard binding.

This patch replaces all the legacy wakeup properties with the unified
"wakeup-source" property in order to avoid any futher copy-paste
duplication.

Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 


Ping, do you prefer taking via your tree or should I send to armsoc
directly ?

--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 16/19] ARM: dts: s5pv210: replace legacy *,wakeup property with wakeup-source

2015-12-15 Thread Sudeep Holla 



On 21/10/15 11:10, Sudeep Holla wrote:

Though the keyboard and other driver will continue to support the legacy
"gpio-key,wakeup", "linux,input-wakeup" boolean property to enable the
wakeup source, "wakeup-source" is the new standard binding.

This patch replaces all the legacy wakeup properties with the unified
"wakeup-source" property in order to avoid any futher copy-paste
duplication.

Cc: Marek Szyprowski 
Cc: Kukjin Kim 



Ping, do you prefer taking via your tree or should I send to armsoc
directly ?

--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/19] ARM: dts: exynos: replace legacy *,wakeup property with wakeup-source

2015-12-16 Thread Sudeep Holla 



On 15/12/15 23:32, Krzysztof Kozlowski wrote:

On 16.12.2015 01:35, Sudeep Holla wrote:



On 21/10/15 11:10, Sudeep Holla wrote:

Though the keyboard and other driver will continue to support the legacy
"gpio-key,wakeup", "linux-keypad,wakeup" boolean property to enable the
wakeup source, "wakeup-source" is the new standard binding.

This patch replaces all the legacy wakeup properties with the unified
"wakeup-source" property in order to avoid any futher copy-paste
duplication.

Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 


Ping, do you prefer taking via your tree or should I send to armsoc
directly ?


Usually it depends on the dependencies... but:
1. I just received this patch - 12/19.


Sorry for that. The initial idea was to show all the changes but merge
the binding and dependencies first, so didn't cc all.


2. No cover letter, no other patches, no explanation about any
dependencies or other related changes. Nothing. Just one patch.


Yes this was initially posted as part of the series, but only
dependencies were merged for v4.4


3. The patch mentions new standard binding. Was the new binding already
merged? Or is it part of these series? What about backward compatibility?


Not a brand new binding, but made existing one a standard so that we
don't continue to be creative and generate more bindings for the same
thing. Yes binding and dependent patches are already in mainline. And
yes backward compatibility is maintained as mentioned in the commit
message.

--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html