Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-17 Thread Chanwoo Choi 
On 2015년 12월 18일 09:34, Chanwoo Choi wrote:
> Hi,
> 
> On 2015년 12월 15일 12:41, Krzysztof Kozlowski wrote:
>> On 14.12.2015 15:38, Chanwoo Choi wrote:
>>> This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>>
>>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>>> and devfreq-event devices which measure the utilization of each bus block.
>>>
>>> Signed-off-by: Chanwoo Choi 
>>> [linux.amoon: Tested on Odroid U3]
>>> Tested-by: Anand Moon 
>>> ---
>>>  drivers/devfreq/Kconfig |  15 ++
>>>  drivers/devfreq/Makefile|   1 +
>>>  drivers/devfreq/exynos/Makefile |   1 +
>>>  drivers/devfreq/exynos/exynos-bus.c | 449 
>>> 
>>>  4 files changed, 466 insertions(+)
>>>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
>>>
>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>> index 64281bb2f650..55ec774f794c 100644
>>> --- a/drivers/devfreq/Kconfig
>>> +++ b/drivers/devfreq/Kconfig
>>> @@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
>>>  
>>>  comment "DEVFREQ Drivers"
>>>  
>>> +config ARM_EXYNOS_BUS_DEVFREQ
>>> +   bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
>>> +   depends on ARCH_EXYNOS
>>> +   select DEVFREQ_GOV_SIMPLE_ONDEMAND
>>> +   select DEVFREQ_EVENT_EXYNOS_PPMU
>>> +   select PM_DEVFREQ_EVENT
>>> +   select PM_OPP
>>> +   help
>>> + This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
>>> + Memory bus has one more group of memory bus (e.g, MIF and INT block).
>>> + Each memory bus group could contain many memoby bus block. It reads
>>> + PPMU counters of memory controllers by using DEVFREQ-event device
>>> + and adjusts the operating frequencies and voltages with OPP support.
>>> + This does not yet operate with optimal voltages.
>>> +
>>>  config ARM_EXYNOS4_BUS_DEVFREQ
>>> bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
>>> depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
>>> !ARCH_MULTIPLATFORM
>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>> index 5134f9ee983d..375ebbb4fcfb 100644
>>> --- a/drivers/devfreq/Makefile
>>> +++ b/drivers/devfreq/Makefile
>>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o
>>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)+= governor_userspace.o
>>>  
>>>  # DEVFREQ Drivers
>>> +obj-$(CONFIG_ARCH_EXYNOS)  += exynos/
>>
>> Why limiting it to ARCH_EXYNOS? Are there real dependencies on exynos
>> mach code? Or on ARM code?
> 
> There is no dependency.
> 
>>
>> If not, then this probably should be obj-y to allow compile testing.
>> Particular objects would be selected by ARM_EXYNOS_BUS_DEVFREQ.
> 
> OK. I'll alter it as obj-y.

As you commented, CONFIG_ARM_EXYNOS_BUS_DEVFREQ is more appropriate
without exynos/ directory as following:

+obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)   += exynos-bus.o

Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-17 Thread Chanwoo Choi 
Hi,

On 2015년 12월 15일 12:41, Krzysztof Kozlowski wrote:
> On 14.12.2015 15:38, Chanwoo Choi wrote:
>> This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>> ---
>>  drivers/devfreq/Kconfig |  15 ++
>>  drivers/devfreq/Makefile|   1 +
>>  drivers/devfreq/exynos/Makefile |   1 +
>>  drivers/devfreq/exynos/exynos-bus.c | 449 
>> 
>>  4 files changed, 466 insertions(+)
>>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 64281bb2f650..55ec774f794c 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
>>  
>>  comment "DEVFREQ Drivers"
>>  
>> +config ARM_EXYNOS_BUS_DEVFREQ
>> +bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
>> +depends on ARCH_EXYNOS
>> +select DEVFREQ_GOV_SIMPLE_ONDEMAND
>> +select DEVFREQ_EVENT_EXYNOS_PPMU
>> +select PM_DEVFREQ_EVENT
>> +select PM_OPP
>> +help
>> +  This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
>> +  Memory bus has one more group of memory bus (e.g, MIF and INT block).
>> +  Each memory bus group could contain many memoby bus block. It reads
>> +  PPMU counters of memory controllers by using DEVFREQ-event device
>> +  and adjusts the operating frequencies and voltages with OPP support.
>> +  This does not yet operate with optimal voltages.
>> +
>>  config ARM_EXYNOS4_BUS_DEVFREQ
>>  bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
>>  depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
>> !ARCH_MULTIPLATFORM
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
> 
> Why limiting it to ARCH_EXYNOS? Are there real dependencies on exynos
> mach code? Or on ARM code?

There is no dependency.

> 
> If not, then this probably should be obj-y to allow compile testing.
> Particular objects would be selected by ARM_EXYNOS_BUS_DEVFREQ.

OK. I'll alter it as obj-y.

Best Regards,
Chanwoo Choi



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-17 Thread Chanwoo Choi 
Hi,

On 2015년 12월 15일 12:41, Krzysztof Kozlowski wrote:
> On 14.12.2015 15:38, Chanwoo Choi wrote:
>> This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>> ---
>>  drivers/devfreq/Kconfig |  15 ++
>>  drivers/devfreq/Makefile|   1 +
>>  drivers/devfreq/exynos/Makefile |   1 +
>>  drivers/devfreq/exynos/exynos-bus.c | 449 
>> 
>>  4 files changed, 466 insertions(+)
>>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 64281bb2f650..55ec774f794c 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
>>  
>>  comment "DEVFREQ Drivers"
>>  
>> +config ARM_EXYNOS_BUS_DEVFREQ
>> +bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
>> +depends on ARCH_EXYNOS
>> +select DEVFREQ_GOV_SIMPLE_ONDEMAND
>> +select DEVFREQ_EVENT_EXYNOS_PPMU
>> +select PM_DEVFREQ_EVENT
>> +select PM_OPP
>> +help
>> +  This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
>> +  Memory bus has one more group of memory bus (e.g, MIF and INT block).
>> +  Each memory bus group could contain many memoby bus block. It reads
>> +  PPMU counters of memory controllers by using DEVFREQ-event device
>> +  and adjusts the operating frequencies and voltages with OPP support.
>> +  This does not yet operate with optimal voltages.
>> +
>>  config ARM_EXYNOS4_BUS_DEVFREQ
>>  bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
>>  depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
>> !ARCH_MULTIPLATFORM
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
> 
> Why limiting it to ARCH_EXYNOS? Are there real dependencies on exynos
> mach code? Or on ARM code?

There is no dependency.

> 
> If not, then this probably should be obj-y to allow compile testing.
> Particular objects would be selected by ARM_EXYNOS_BUS_DEVFREQ.

OK. I'll alter it as obj-y.

Best Regards,
Chanwoo Choi



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-17 Thread Chanwoo Choi 
On 2015년 12월 18일 09:34, Chanwoo Choi wrote:
> Hi,
> 
> On 2015년 12월 15일 12:41, Krzysztof Kozlowski wrote:
>> On 14.12.2015 15:38, Chanwoo Choi wrote:
>>> This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>>
>>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>>> and devfreq-event devices which measure the utilization of each bus block.
>>>
>>> Signed-off-by: Chanwoo Choi 
>>> [linux.amoon: Tested on Odroid U3]
>>> Tested-by: Anand Moon 
>>> ---
>>>  drivers/devfreq/Kconfig |  15 ++
>>>  drivers/devfreq/Makefile|   1 +
>>>  drivers/devfreq/exynos/Makefile |   1 +
>>>  drivers/devfreq/exynos/exynos-bus.c | 449 
>>> 
>>>  4 files changed, 466 insertions(+)
>>>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
>>>
>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>> index 64281bb2f650..55ec774f794c 100644
>>> --- a/drivers/devfreq/Kconfig
>>> +++ b/drivers/devfreq/Kconfig
>>> @@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
>>>  
>>>  comment "DEVFREQ Drivers"
>>>  
>>> +config ARM_EXYNOS_BUS_DEVFREQ
>>> +   bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
>>> +   depends on ARCH_EXYNOS
>>> +   select DEVFREQ_GOV_SIMPLE_ONDEMAND
>>> +   select DEVFREQ_EVENT_EXYNOS_PPMU
>>> +   select PM_DEVFREQ_EVENT
>>> +   select PM_OPP
>>> +   help
>>> + This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
>>> + Memory bus has one more group of memory bus (e.g, MIF and INT block).
>>> + Each memory bus group could contain many memoby bus block. It reads
>>> + PPMU counters of memory controllers by using DEVFREQ-event device
>>> + and adjusts the operating frequencies and voltages with OPP support.
>>> + This does not yet operate with optimal voltages.
>>> +
>>>  config ARM_EXYNOS4_BUS_DEVFREQ
>>> bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
>>> depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
>>> !ARCH_MULTIPLATFORM
>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>> index 5134f9ee983d..375ebbb4fcfb 100644
>>> --- a/drivers/devfreq/Makefile
>>> +++ b/drivers/devfreq/Makefile
>>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o
>>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)+= governor_userspace.o
>>>  
>>>  # DEVFREQ Drivers
>>> +obj-$(CONFIG_ARCH_EXYNOS)  += exynos/
>>
>> Why limiting it to ARCH_EXYNOS? Are there real dependencies on exynos
>> mach code? Or on ARM code?
> 
> There is no dependency.
> 
>>
>> If not, then this probably should be obj-y to allow compile testing.
>> Particular objects would be selected by ARM_EXYNOS_BUS_DEVFREQ.
> 
> OK. I'll alter it as obj-y.

As you commented, CONFIG_ARM_EXYNOS_BUS_DEVFREQ is more appropriate
without exynos/ directory as following:

+obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)   += exynos-bus.o

Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread Krzysztof Kozlowski 
On 14.12.2015 15:38, Chanwoo Choi wrote:
> This patch adds the generic exynos bus frequency driver for AMBA AXI bus
> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
> have the common architecture for bus between DRAM and sub-blocks in SoC.
> This driver can support the generic bus frequency driver for Exynos SoCs.
> 
> In devicetree, Each bus block has a bus clock, regulator, operation-point
> and devfreq-event devices which measure the utilization of each bus block.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> ---
>  drivers/devfreq/Kconfig |  15 ++
>  drivers/devfreq/Makefile|   1 +
>  drivers/devfreq/exynos/Makefile |   1 +
>  drivers/devfreq/exynos/exynos-bus.c | 449 
> 
>  4 files changed, 466 insertions(+)
>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 64281bb2f650..55ec774f794c 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
>  
>  comment "DEVFREQ Drivers"
>  
> +config ARM_EXYNOS_BUS_DEVFREQ
> + bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
> + depends on ARCH_EXYNOS
> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + select DEVFREQ_EVENT_EXYNOS_PPMU
> + select PM_DEVFREQ_EVENT
> + select PM_OPP
> + help
> +   This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
> +   Memory bus has one more group of memory bus (e.g, MIF and INT block).
> +   Each memory bus group could contain many memoby bus block. It reads
> +   PPMU counters of memory controllers by using DEVFREQ-event device
> +   and adjusts the operating frequencies and voltages with OPP support.
> +   This does not yet operate with optimal voltages.
> +
>  config ARM_EXYNOS4_BUS_DEVFREQ
>   bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
>   depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
> !ARCH_MULTIPLATFORM
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 5134f9ee983d..375ebbb4fcfb 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)   += governor_powersave.o
>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)  += governor_userspace.o
>  
>  # DEVFREQ Drivers
> +obj-$(CONFIG_ARCH_EXYNOS)+= exynos/

Why limiting it to ARCH_EXYNOS? Are there real dependencies on exynos
mach code? Or on ARM code?

If not, then this probably should be obj-y to allow compile testing.
Particular objects would be selected by ARM_EXYNOS_BUS_DEVFREQ.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread Chanwoo Choi 
On 2015년 12월 14일 17:28, MyungJoo Ham wrote:
>>   
>>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>>
> 
> Chanwoo, could you please show me testing this set of patches in your site?
> Please let me know when is ok to visit you.
> (I do not have exynos machines right now.)
> 
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)   += exynos/
> 
> CONFIG_ARCH_EXYNOS is true if
>   CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
>   or
>   CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
> Thus, the two lines after you've added have become useless. (dead code)
> 
> Please delete them.

In this series, patch11 deletes all of both exynos4_bus.c and exynos5_bus.c.

> 
> []
>> --- /dev/null
>> +++ b/drivers/devfreq/exynos/exynos-bus.c
> []
>> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
>> flags)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct dev_pm_opp *new_opp;
>> +unsigned long old_freq, new_freq, old_volt, new_volt;
>> +int ret = 0;
>> +
>> +/* Get new opp-bus instance according to new bus clock */
>> +rcu_read_lock();
>> +new_opp = devfreq_recommended_opp(dev, freq, flags);
>> +if (IS_ERR_OR_NULL(new_opp)) {
>> +dev_err(dev, "failed to get recommed opp instance\n");
>> +rcu_read_unlock();
>> +return PTR_ERR(new_opp);
>> +}
>> +
>> +new_freq = dev_pm_opp_get_freq(new_opp);
>> +new_volt = dev_pm_opp_get_voltage(new_opp);
>> +old_freq = dev_pm_opp_get_freq(bus->curr_opp);
>> +old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +if (old_freq == new_freq)
>> +return 0;
>> +
>> +/* Change voltage and frequency according to new OPP level */
>> +mutex_lock(>lock);
>> +
>> +if (old_freq < new_freq) {
>> +ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);
> 
> Setting the maximum volt same as the minimum volt is not recommended.
> Especially for any DVFS mechanisms, I recommend to set values as:
> min_volt = minimum voltage that does not harm the stability
> max_volt = maximum voltage that does not break the circuit
> 
> Please refer to /include/linux/regulator/driver.h
> "@set_voltage" comments.
> 
> For the rest of regulator_set_voltage usages, I'd say the same.

OK.
I'll add the 'voltage-tolerance' property as cpufreq-dt.c driver.
The cpufreq-dt.c get the percentage value by using 'voltage-tolerance'
devicetree property.

For example,
if (of_property_read_u32(np, "exynos,voltage-tolerance",
>voltage_tolerance))
bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;

tol = new_volt * bus->voltage_tolerance / 100;
regulator_set_voltage_tol(regulator, new_volt, tol);

> 
> []
>> +static int exynos_bus_get_dev_status(struct device *dev,
>> + struct devfreq_dev_status *stat)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct devfreq_event_data edata;
>> +int ret;
>> +
>> +rcu_read_lock();
>> +stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +ret = exynos_bus_get_event(bus, );
>> +if (ret < 0) {
>> +stat->total_time = stat->busy_time = 0;
>> +goto err;
>> +}
>> +
>> +stat->busy_time = (edata.load_count * 100) / bus->ratio;
>> +stat->total_time = edata.total_count;
>> +
>> +dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
>> +stat->total_time);
> 
> These two values are unsigned long.

OK. I'll modify it (%ld -> %lu)

> 
> []
>> +static int exynos_bus_parse_of(struct device_node *np,
>> +  struct exynos_bus *bus)
>> +{
>> +struct device *dev = bus->dev;
>> +unsigned long rate;
>> +int i, ret, count, size;
>> +
>> +/* Get the clock to provide each bus with source clock */
>> +bus->clk = devm_clk_get(dev, "bus");
>> +

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread Chanwoo Choi 
On 2015년 12월 14일 17:28, MyungJoo Ham wrote:
>>   
>>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>>
> 
> Chanwoo, could you please show me testing this set of patches in your site?
> Please let me know when is ok to visit you.
> (I do not have exynos machines right now.)

Sure. I can show it tomorrow whenever you want.
Before visiting to me, just let me know. I'll prepare the demonstartion.

Regards,
Chanwo oChoi

> 
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)   += exynos/
> 
> CONFIG_ARCH_EXYNOS is true if
>   CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
>   or
>   CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
> Thus, the two lines after you've added have become useless. (dead code)
> 
> Please delete them.
> 
> []
>> --- /dev/null
>> +++ b/drivers/devfreq/exynos/exynos-bus.c
> []
>> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
>> flags)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct dev_pm_opp *new_opp;
>> +unsigned long old_freq, new_freq, old_volt, new_volt;
>> +int ret = 0;
>> +
>> +/* Get new opp-bus instance according to new bus clock */
>> +rcu_read_lock();
>> +new_opp = devfreq_recommended_opp(dev, freq, flags);
>> +if (IS_ERR_OR_NULL(new_opp)) {
>> +dev_err(dev, "failed to get recommed opp instance\n");
>> +rcu_read_unlock();
>> +return PTR_ERR(new_opp);
>> +}
>> +
>> +new_freq = dev_pm_opp_get_freq(new_opp);
>> +new_volt = dev_pm_opp_get_voltage(new_opp);
>> +old_freq = dev_pm_opp_get_freq(bus->curr_opp);
>> +old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +if (old_freq == new_freq)
>> +return 0;
>> +
>> +/* Change voltage and frequency according to new OPP level */
>> +mutex_lock(>lock);
>> +
>> +if (old_freq < new_freq) {
>> +ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);
> 
> Setting the maximum volt same as the minimum volt is not recommended.
> Especially for any DVFS mechanisms, I recommend to set values as:
> min_volt = minimum voltage that does not harm the stability
> max_volt = maximum voltage that does not break the circuit
> 
> Please refer to /include/linux/regulator/driver.h
> "@set_voltage" comments.
> 
> For the rest of regulator_set_voltage usages, I'd say the same.
> 
> []
>> +static int exynos_bus_get_dev_status(struct device *dev,
>> + struct devfreq_dev_status *stat)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct devfreq_event_data edata;
>> +int ret;
>> +
>> +rcu_read_lock();
>> +stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +ret = exynos_bus_get_event(bus, );
>> +if (ret < 0) {
>> +stat->total_time = stat->busy_time = 0;
>> +goto err;
>> +}
>> +
>> +stat->busy_time = (edata.load_count * 100) / bus->ratio;
>> +stat->total_time = edata.total_count;
>> +
>> +dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
>> +stat->total_time);
> 
> These two values are unsigned long.
> 
> []
>> +static int exynos_bus_parse_of(struct device_node *np,
>> +  struct exynos_bus *bus)
>> +{
>> +struct device *dev = bus->dev;
>> +unsigned long rate;
>> +int i, ret, count, size;
>> +
>> +/* Get the clock to provide each bus with source clock */
>> +bus->clk = devm_clk_get(dev, "bus");
>> +if (IS_ERR(bus->clk)) {
>> +dev_err(dev, "failed to get bus clock\n");
>> +return PTR_ERR(bus->clk);
>> +}
>> +
>> +ret = clk_prepare_enable(bus->clk);
>> +if (ret < 0) {
>> +dev_err(dev, "failed to get enable clock\n");
>> +return ret;
>> +}
> 
> []
> 
>> +err_regulator:
>> +regulator_disable(bus->regulator);
>> +err_opp:
>> +dev_pm_opp_of_remove_table(dev);
>> +

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread MyungJoo Ham 
>   
>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
> have the common architecture for bus between DRAM and sub-blocks in SoC.
> This driver can support the generic bus frequency driver for Exynos SoCs.
> 
> In devicetree, Each bus block has a bus clock, regulator, operation-point
> and devfreq-event devices which measure the utilization of each bus block.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> 

Chanwoo, could you please show me testing this set of patches in your site?
Please let me know when is ok to visit you.
(I do not have exynos machines right now.)

> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 5134f9ee983d..375ebbb4fcfb 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)   += governor_powersave.o
>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)  += governor_userspace.o
>  
>  # DEVFREQ Drivers
> +obj-$(CONFIG_ARCH_EXYNOS)+= exynos/
>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)+= exynos/
>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)+= exynos/

CONFIG_ARCH_EXYNOS is true if
CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
or
CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
Thus, the two lines after you've added have become useless. (dead code)

Please delete them.

[]
> --- /dev/null
> +++ b/drivers/devfreq/exynos/exynos-bus.c
[]
> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
> flags)
> +{
> + struct exynos_bus *bus = dev_get_drvdata(dev);
> + struct dev_pm_opp *new_opp;
> + unsigned long old_freq, new_freq, old_volt, new_volt;
> + int ret = 0;
> +
> + /* Get new opp-bus instance according to new bus clock */
> + rcu_read_lock();
> + new_opp = devfreq_recommended_opp(dev, freq, flags);
> + if (IS_ERR_OR_NULL(new_opp)) {
> + dev_err(dev, "failed to get recommed opp instance\n");
> + rcu_read_unlock();
> + return PTR_ERR(new_opp);
> + }
> +
> + new_freq = dev_pm_opp_get_freq(new_opp);
> + new_volt = dev_pm_opp_get_voltage(new_opp);
> + old_freq = dev_pm_opp_get_freq(bus->curr_opp);
> + old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
> + rcu_read_unlock();
> +
> + if (old_freq == new_freq)
> + return 0;
> +
> + /* Change voltage and frequency according to new OPP level */
> + mutex_lock(>lock);
> +
> + if (old_freq < new_freq) {
> + ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);

Setting the maximum volt same as the minimum volt is not recommended.
Especially for any DVFS mechanisms, I recommend to set values as:
min_volt = minimum voltage that does not harm the stability
max_volt = maximum voltage that does not break the circuit

Please refer to /include/linux/regulator/driver.h
"@set_voltage" comments.

For the rest of regulator_set_voltage usages, I'd say the same.

[]
> +static int exynos_bus_get_dev_status(struct device *dev,
> +  struct devfreq_dev_status *stat)
> +{
> + struct exynos_bus *bus = dev_get_drvdata(dev);
> + struct devfreq_event_data edata;
> + int ret;
> +
> + rcu_read_lock();
> + stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
> + rcu_read_unlock();
> +
> + ret = exynos_bus_get_event(bus, );
> + if (ret < 0) {
> + stat->total_time = stat->busy_time = 0;
> + goto err;
> + }
> +
> + stat->busy_time = (edata.load_count * 100) / bus->ratio;
> + stat->total_time = edata.total_count;
> +
> + dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
> + stat->total_time);

These two values are unsigned long.

[]
> +static int exynos_bus_parse_of(struct device_node *np,
> +   struct exynos_bus *bus)
> +{
> + struct device *dev = bus->dev;
> + unsigned long rate;
> + int i, ret, count, size;
> +
> + /* Get the clock to provide each bus with source clock */
> + bus->clk = devm_clk_get(dev, "bus");
> + if (IS_ERR(bus->clk)) {
> + dev_err(dev, "failed to get bus clock\n");
> + return PTR_ERR(bus->clk);
> + }
> +
> + ret = clk_prepare_enable(bus->clk);
> + if (ret < 0) {
> + dev_err(dev, "failed to get enable clock\n");
> + return ret;
> + }

[]

> +err_regulator:
> + regulator_disable(bus->regulator);
> +err_opp:
> + dev_pm_opp_of_remove_table(dev);
> +
> + return ret;

No clk_disable_unprepare() somewhere in the error handling routines?

[]

> +#ifdef CONFIG_PM_SLEEP
> +static int exynos_bus_resume(struct device *dev)
> +{
[]
> + ret = regulator_enable(bus->regulator);
[]
> +}
> +
> +static int exynos_bus_suspend(struct

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread Krzysztof Kozlowski 
On 14.12.2015 15:38, Chanwoo Choi wrote:
> This patch adds the generic exynos bus frequency driver for AMBA AXI bus
> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
> have the common architecture for bus between DRAM and sub-blocks in SoC.
> This driver can support the generic bus frequency driver for Exynos SoCs.
> 
> In devicetree, Each bus block has a bus clock, regulator, operation-point
> and devfreq-event devices which measure the utilization of each bus block.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> ---
>  drivers/devfreq/Kconfig |  15 ++
>  drivers/devfreq/Makefile|   1 +
>  drivers/devfreq/exynos/Makefile |   1 +
>  drivers/devfreq/exynos/exynos-bus.c | 449 
> 
>  4 files changed, 466 insertions(+)
>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 64281bb2f650..55ec774f794c 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
>  
>  comment "DEVFREQ Drivers"
>  
> +config ARM_EXYNOS_BUS_DEVFREQ
> + bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
> + depends on ARCH_EXYNOS
> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + select DEVFREQ_EVENT_EXYNOS_PPMU
> + select PM_DEVFREQ_EVENT
> + select PM_OPP
> + help
> +   This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
> +   Memory bus has one more group of memory bus (e.g, MIF and INT block).
> +   Each memory bus group could contain many memoby bus block. It reads
> +   PPMU counters of memory controllers by using DEVFREQ-event device
> +   and adjusts the operating frequencies and voltages with OPP support.
> +   This does not yet operate with optimal voltages.
> +
>  config ARM_EXYNOS4_BUS_DEVFREQ
>   bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
>   depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
> !ARCH_MULTIPLATFORM
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 5134f9ee983d..375ebbb4fcfb 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)   += governor_powersave.o
>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)  += governor_userspace.o
>  
>  # DEVFREQ Drivers
> +obj-$(CONFIG_ARCH_EXYNOS)+= exynos/

Why limiting it to ARCH_EXYNOS? Are there real dependencies on exynos
mach code? Or on ARM code?

If not, then this probably should be obj-y to allow compile testing.
Particular objects would be selected by ARM_EXYNOS_BUS_DEVFREQ.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread Chanwoo Choi 
On 2015년 12월 14일 17:28, MyungJoo Ham wrote:
>>   
>>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>>
> 
> Chanwoo, could you please show me testing this set of patches in your site?
> Please let me know when is ok to visit you.
> (I do not have exynos machines right now.)
> 
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)   += exynos/
> 
> CONFIG_ARCH_EXYNOS is true if
>   CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
>   or
>   CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
> Thus, the two lines after you've added have become useless. (dead code)
> 
> Please delete them.

In this series, patch11 deletes all of both exynos4_bus.c and exynos5_bus.c.

> 
> []
>> --- /dev/null
>> +++ b/drivers/devfreq/exynos/exynos-bus.c
> []
>> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
>> flags)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct dev_pm_opp *new_opp;
>> +unsigned long old_freq, new_freq, old_volt, new_volt;
>> +int ret = 0;
>> +
>> +/* Get new opp-bus instance according to new bus clock */
>> +rcu_read_lock();
>> +new_opp = devfreq_recommended_opp(dev, freq, flags);
>> +if (IS_ERR_OR_NULL(new_opp)) {
>> +dev_err(dev, "failed to get recommed opp instance\n");
>> +rcu_read_unlock();
>> +return PTR_ERR(new_opp);
>> +}
>> +
>> +new_freq = dev_pm_opp_get_freq(new_opp);
>> +new_volt = dev_pm_opp_get_voltage(new_opp);
>> +old_freq = dev_pm_opp_get_freq(bus->curr_opp);
>> +old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +if (old_freq == new_freq)
>> +return 0;
>> +
>> +/* Change voltage and frequency according to new OPP level */
>> +mutex_lock(>lock);
>> +
>> +if (old_freq < new_freq) {
>> +ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);
> 
> Setting the maximum volt same as the minimum volt is not recommended.
> Especially for any DVFS mechanisms, I recommend to set values as:
> min_volt = minimum voltage that does not harm the stability
> max_volt = maximum voltage that does not break the circuit
> 
> Please refer to /include/linux/regulator/driver.h
> "@set_voltage" comments.
> 
> For the rest of regulator_set_voltage usages, I'd say the same.

OK.
I'll add the 'voltage-tolerance' property as cpufreq-dt.c driver.
The cpufreq-dt.c get the percentage value by using 'voltage-tolerance'
devicetree property.

For example,
if (of_property_read_u32(np, "exynos,voltage-tolerance",
>voltage_tolerance))
bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;

tol = new_volt * bus->voltage_tolerance / 100;
regulator_set_voltage_tol(regulator, new_volt, tol);

> 
> []
>> +static int exynos_bus_get_dev_status(struct device *dev,
>> + struct devfreq_dev_status *stat)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct devfreq_event_data edata;
>> +int ret;
>> +
>> +rcu_read_lock();
>> +stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +ret = exynos_bus_get_event(bus, );
>> +if (ret < 0) {
>> +stat->total_time = stat->busy_time = 0;
>> +goto err;
>> +}
>> +
>> +stat->busy_time = (edata.load_count * 100) / bus->ratio;
>> +stat->total_time = edata.total_count;
>> +
>> +dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
>> +stat->total_time);
> 
> These two values are unsigned long.

OK. I'll modify it (%ld -> %lu)

> 
> []
>> +static int exynos_bus_parse_of(struct device_node *np,
>> +  struct exynos_bus *bus)
>> +{
>> +struct device *dev = bus->dev;
>> +unsigned long rate;
>> +int i, ret, count, size;
>> +
>> +/* Get the clock to provide each bus with source clock */
>> +

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread MyungJoo Ham 
>   
>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
> have the common architecture for bus between DRAM and sub-blocks in SoC.
> This driver can support the generic bus frequency driver for Exynos SoCs.
> 
> In devicetree, Each bus block has a bus clock, regulator, operation-point
> and devfreq-event devices which measure the utilization of each bus block.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> 

Chanwoo, could you please show me testing this set of patches in your site?
Please let me know when is ok to visit you.
(I do not have exynos machines right now.)

> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 5134f9ee983d..375ebbb4fcfb 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)   += governor_powersave.o
>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)  += governor_userspace.o
>  
>  # DEVFREQ Drivers
> +obj-$(CONFIG_ARCH_EXYNOS)+= exynos/
>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)+= exynos/
>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)+= exynos/

CONFIG_ARCH_EXYNOS is true if
CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
or
CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
Thus, the two lines after you've added have become useless. (dead code)

Please delete them.

[]
> --- /dev/null
> +++ b/drivers/devfreq/exynos/exynos-bus.c
[]
> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
> flags)
> +{
> + struct exynos_bus *bus = dev_get_drvdata(dev);
> + struct dev_pm_opp *new_opp;
> + unsigned long old_freq, new_freq, old_volt, new_volt;
> + int ret = 0;
> +
> + /* Get new opp-bus instance according to new bus clock */
> + rcu_read_lock();
> + new_opp = devfreq_recommended_opp(dev, freq, flags);
> + if (IS_ERR_OR_NULL(new_opp)) {
> + dev_err(dev, "failed to get recommed opp instance\n");
> + rcu_read_unlock();
> + return PTR_ERR(new_opp);
> + }
> +
> + new_freq = dev_pm_opp_get_freq(new_opp);
> + new_volt = dev_pm_opp_get_voltage(new_opp);
> + old_freq = dev_pm_opp_get_freq(bus->curr_opp);
> + old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
> + rcu_read_unlock();
> +
> + if (old_freq == new_freq)
> + return 0;
> +
> + /* Change voltage and frequency according to new OPP level */
> + mutex_lock(>lock);
> +
> + if (old_freq < new_freq) {
> + ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);

Setting the maximum volt same as the minimum volt is not recommended.
Especially for any DVFS mechanisms, I recommend to set values as:
min_volt = minimum voltage that does not harm the stability
max_volt = maximum voltage that does not break the circuit

Please refer to /include/linux/regulator/driver.h
"@set_voltage" comments.

For the rest of regulator_set_voltage usages, I'd say the same.

[]
> +static int exynos_bus_get_dev_status(struct device *dev,
> +  struct devfreq_dev_status *stat)
> +{
> + struct exynos_bus *bus = dev_get_drvdata(dev);
> + struct devfreq_event_data edata;
> + int ret;
> +
> + rcu_read_lock();
> + stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
> + rcu_read_unlock();
> +
> + ret = exynos_bus_get_event(bus, );
> + if (ret < 0) {
> + stat->total_time = stat->busy_time = 0;
> + goto err;
> + }
> +
> + stat->busy_time = (edata.load_count * 100) / bus->ratio;
> + stat->total_time = edata.total_count;
> +
> + dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
> + stat->total_time);

These two values are unsigned long.

[]
> +static int exynos_bus_parse_of(struct device_node *np,
> +   struct exynos_bus *bus)
> +{
> + struct device *dev = bus->dev;
> + unsigned long rate;
> + int i, ret, count, size;
> +
> + /* Get the clock to provide each bus with source clock */
> + bus->clk = devm_clk_get(dev, "bus");
> + if (IS_ERR(bus->clk)) {
> + dev_err(dev, "failed to get bus clock\n");
> + return PTR_ERR(bus->clk);
> + }
> +
> + ret = clk_prepare_enable(bus->clk);
> + if (ret < 0) {
> + dev_err(dev, "failed to get enable clock\n");
> + return ret;
> + }

[]

> +err_regulator:
> + regulator_disable(bus->regulator);
> +err_opp:
> + dev_pm_opp_of_remove_table(dev);
> +
> + return ret;

No clk_disable_unprepare() somewhere in the error handling routines?

[]

> +#ifdef CONFIG_PM_SLEEP
> +static int exynos_bus_resume(struct device *dev)
> +{
[]
> + ret = regulator_enable(bus->regulator);
[]
> +}
> +

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-14 Thread Chanwoo Choi 
On 2015년 12월 14일 17:28, MyungJoo Ham wrote:
>>   
>>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>>
> 
> Chanwoo, could you please show me testing this set of patches in your site?
> Please let me know when is ok to visit you.
> (I do not have exynos machines right now.)

Sure. I can show it tomorrow whenever you want.
Before visiting to me, just let me know. I'll prepare the demonstartion.

Regards,
Chanwo oChoi

> 
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)   += exynos/
> 
> CONFIG_ARCH_EXYNOS is true if
>   CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
>   or
>   CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
> Thus, the two lines after you've added have become useless. (dead code)
> 
> Please delete them.
> 
> []
>> --- /dev/null
>> +++ b/drivers/devfreq/exynos/exynos-bus.c
> []
>> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
>> flags)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct dev_pm_opp *new_opp;
>> +unsigned long old_freq, new_freq, old_volt, new_volt;
>> +int ret = 0;
>> +
>> +/* Get new opp-bus instance according to new bus clock */
>> +rcu_read_lock();
>> +new_opp = devfreq_recommended_opp(dev, freq, flags);
>> +if (IS_ERR_OR_NULL(new_opp)) {
>> +dev_err(dev, "failed to get recommed opp instance\n");
>> +rcu_read_unlock();
>> +return PTR_ERR(new_opp);
>> +}
>> +
>> +new_freq = dev_pm_opp_get_freq(new_opp);
>> +new_volt = dev_pm_opp_get_voltage(new_opp);
>> +old_freq = dev_pm_opp_get_freq(bus->curr_opp);
>> +old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +if (old_freq == new_freq)
>> +return 0;
>> +
>> +/* Change voltage and frequency according to new OPP level */
>> +mutex_lock(>lock);
>> +
>> +if (old_freq < new_freq) {
>> +ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);
> 
> Setting the maximum volt same as the minimum volt is not recommended.
> Especially for any DVFS mechanisms, I recommend to set values as:
> min_volt = minimum voltage that does not harm the stability
> max_volt = maximum voltage that does not break the circuit
> 
> Please refer to /include/linux/regulator/driver.h
> "@set_voltage" comments.
> 
> For the rest of regulator_set_voltage usages, I'd say the same.
> 
> []
>> +static int exynos_bus_get_dev_status(struct device *dev,
>> + struct devfreq_dev_status *stat)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct devfreq_event_data edata;
>> +int ret;
>> +
>> +rcu_read_lock();
>> +stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +ret = exynos_bus_get_event(bus, );
>> +if (ret < 0) {
>> +stat->total_time = stat->busy_time = 0;
>> +goto err;
>> +}
>> +
>> +stat->busy_time = (edata.load_count * 100) / bus->ratio;
>> +stat->total_time = edata.total_count;
>> +
>> +dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
>> +stat->total_time);
> 
> These two values are unsigned long.
> 
> []
>> +static int exynos_bus_parse_of(struct device_node *np,
>> +  struct exynos_bus *bus)
>> +{
>> +struct device *dev = bus->dev;
>> +unsigned long rate;
>> +int i, ret, count, size;
>> +
>> +/* Get the clock to provide each bus with source clock */
>> +bus->clk = devm_clk_get(dev, "bus");
>> +if (IS_ERR(bus->clk)) {
>> +dev_err(dev, "failed to get bus clock\n");
>> +return PTR_ERR(bus->clk);
>> +}
>> +
>> +ret = clk_prepare_enable(bus->clk);
>> +if (ret < 0) {
>> +dev_err(dev, "failed to get enable clock\n");
>> +return ret;
>> +}
> 
> []
> 
>> +err_regulator:
>> +regulator_disable(bus->regulator);
>> +err_opp:

[PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-13 Thread Chanwoo Choi 
This patch adds the generic exynos bus frequency driver for AMBA AXI bus
of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
have the common architecture for bus between DRAM and sub-blocks in SoC.
This driver can support the generic bus frequency driver for Exynos SoCs.

In devicetree, Each bus block has a bus clock, regulator, operation-point
and devfreq-event devices which measure the utilization of each bus block.

Signed-off-by: Chanwoo Choi 
[linux.amoon: Tested on Odroid U3]
Tested-by: Anand Moon 
---
 drivers/devfreq/Kconfig |  15 ++
 drivers/devfreq/Makefile|   1 +
 drivers/devfreq/exynos/Makefile |   1 +
 drivers/devfreq/exynos/exynos-bus.c | 449 
 4 files changed, 466 insertions(+)
 create mode 100644 drivers/devfreq/exynos/exynos-bus.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 64281bb2f650..55ec774f794c 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
 
 comment "DEVFREQ Drivers"
 
+config ARM_EXYNOS_BUS_DEVFREQ
+   bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
+   depends on ARCH_EXYNOS
+   select DEVFREQ_GOV_SIMPLE_ONDEMAND
+   select DEVFREQ_EVENT_EXYNOS_PPMU
+   select PM_DEVFREQ_EVENT
+   select PM_OPP
+   help
+ This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
+ Memory bus has one more group of memory bus (e.g, MIF and INT block).
+ Each memory bus group could contain many memoby bus block. It reads
+ PPMU counters of memory controllers by using DEVFREQ-event device
+ and adjusts the operating frequencies and voltages with OPP support.
+ This does not yet operate with optimal voltages.
+
 config ARM_EXYNOS4_BUS_DEVFREQ
bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
!ARCH_MULTIPLATFORM
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 5134f9ee983d..375ebbb4fcfb 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o
 obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)+= governor_userspace.o
 
 # DEVFREQ Drivers
+obj-$(CONFIG_ARCH_EXYNOS)  += exynos/
 obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)  += exynos/
 obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)  += exynos/
 obj-$(CONFIG_ARM_TEGRA_DEVFREQ)+= tegra-devfreq.o
diff --git a/drivers/devfreq/exynos/Makefile b/drivers/devfreq/exynos/Makefile
index 49bc9175f923..4ec06d322996 100644
--- a/drivers/devfreq/exynos/Makefile
+++ b/drivers/devfreq/exynos/Makefile
@@ -1,3 +1,4 @@
 # Exynos DEVFREQ Drivers
+obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)   += exynos-bus.o
 obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)  += exynos_ppmu.o exynos4_bus.o
 obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)  += exynos_ppmu.o exynos5_bus.o
diff --git a/drivers/devfreq/exynos/exynos-bus.c 
b/drivers/devfreq/exynos/exynos-bus.c
new file mode 100644
index ..f1bc20839650
--- /dev/null
+++ b/drivers/devfreq/exynos/exynos-bus.c
@@ -0,0 +1,449 @@
+/*
+ * Generic Exynos Bus frequency driver with DEVFREQ Framework
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * Author : Chanwoo Choi 
+ *
+ * This driver support Exynos Bus frequency feature by using
+ * DEVFREQ framework and is based on drivers/devfreq/exynos/exynos4_bus.c.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEFAULT_SATURATION_RATIO   40
+
+struct exynos_bus {
+   struct device *dev;
+
+   struct devfreq *devfreq;
+   struct devfreq_event_dev **edev;
+   unsigned int edev_count;
+   struct mutex lock;
+
+   struct dev_pm_opp *curr_opp;
+
+   struct regulator *regulator;
+   struct clk *clk;
+   int ratio;
+};
+
+/*
+ * Control the devfreq-event device to get the current state of bus
+ */
+#define exynos_bus_ops_edev(ops)   \
+static int exynos_bus_##ops(struct exynos_bus *bus)\
+{  \
+   int i, ret; \
+   \
+   for (i = 0; i < bus->edev_count; i++) { \
+   if (!bus->edev[i])  \
+   continue;   \
+   ret = devfreq_event_##ops(bus->edev[i]);\
+   if (ret < 0)\
+   return ret; \
+   }

[PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

2015-12-13 Thread Chanwoo Choi 
This patch adds the generic exynos bus frequency driver for AMBA AXI bus
of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
have the common architecture for bus between DRAM and sub-blocks in SoC.
This driver can support the generic bus frequency driver for Exynos SoCs.

In devicetree, Each bus block has a bus clock, regulator, operation-point
and devfreq-event devices which measure the utilization of each bus block.

Signed-off-by: Chanwoo Choi 
[linux.amoon: Tested on Odroid U3]
Tested-by: Anand Moon 
---
 drivers/devfreq/Kconfig |  15 ++
 drivers/devfreq/Makefile|   1 +
 drivers/devfreq/exynos/Makefile |   1 +
 drivers/devfreq/exynos/exynos-bus.c | 449 
 4 files changed, 466 insertions(+)
 create mode 100644 drivers/devfreq/exynos/exynos-bus.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 64281bb2f650..55ec774f794c 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
 
 comment "DEVFREQ Drivers"
 
+config ARM_EXYNOS_BUS_DEVFREQ
+   bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
+   depends on ARCH_EXYNOS
+   select DEVFREQ_GOV_SIMPLE_ONDEMAND
+   select DEVFREQ_EVENT_EXYNOS_PPMU
+   select PM_DEVFREQ_EVENT
+   select PM_OPP
+   help
+ This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
+ Memory bus has one more group of memory bus (e.g, MIF and INT block).
+ Each memory bus group could contain many memoby bus block. It reads
+ PPMU counters of memory controllers by using DEVFREQ-event device
+ and adjusts the operating frequencies and voltages with OPP support.
+ This does not yet operate with optimal voltages.
+
 config ARM_EXYNOS4_BUS_DEVFREQ
bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
!ARCH_MULTIPLATFORM
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 5134f9ee983d..375ebbb4fcfb 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o
 obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)+= governor_userspace.o
 
 # DEVFREQ Drivers
+obj-$(CONFIG_ARCH_EXYNOS)  += exynos/
 obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)  += exynos/
 obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)  += exynos/
 obj-$(CONFIG_ARM_TEGRA_DEVFREQ)+= tegra-devfreq.o
diff --git a/drivers/devfreq/exynos/Makefile b/drivers/devfreq/exynos/Makefile
index 49bc9175f923..4ec06d322996 100644
--- a/drivers/devfreq/exynos/Makefile
+++ b/drivers/devfreq/exynos/Makefile
@@ -1,3 +1,4 @@
 # Exynos DEVFREQ Drivers
+obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ)   += exynos-bus.o
 obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)  += exynos_ppmu.o exynos4_bus.o
 obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)  += exynos_ppmu.o exynos5_bus.o
diff --git a/drivers/devfreq/exynos/exynos-bus.c 
b/drivers/devfreq/exynos/exynos-bus.c
new file mode 100644
index ..f1bc20839650
--- /dev/null
+++ b/drivers/devfreq/exynos/exynos-bus.c
@@ -0,0 +1,449 @@
+/*
+ * Generic Exynos Bus frequency driver with DEVFREQ Framework
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * Author : Chanwoo Choi 
+ *
+ * This driver support Exynos Bus frequency feature by using
+ * DEVFREQ framework and is based on drivers/devfreq/exynos/exynos4_bus.c.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEFAULT_SATURATION_RATIO   40
+
+struct exynos_bus {
+   struct device *dev;
+
+   struct devfreq *devfreq;
+   struct devfreq_event_dev **edev;
+   unsigned int edev_count;
+   struct mutex lock;
+
+   struct dev_pm_opp *curr_opp;
+
+   struct regulator *regulator;
+   struct clk *clk;
+   int ratio;
+};
+
+/*
+ * Control the devfreq-event device to get the current state of bus
+ */
+#define exynos_bus_ops_edev(ops)   \
+static int exynos_bus_##ops(struct exynos_bus *bus)\
+{  \
+   int i, ret; \
+   \
+   for (i = 0; i < bus->edev_count; i++) { \
+   if (!bus->edev[i])  \
+   continue;   \
+   ret = devfreq_event_##ops(bus->edev[i]);\
+   if (ret < 0)\
+