Re: [PATCH V2] regulator: fixed: update to devm_* API
On Tuesday, 28. January 2014 12:46:01 Manish Badarkhe wrote:
> Hi Dmitry,
>
> Thank you for your review.
>
> On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov
>
> wrote:
> > Hi Manish,
> >
> > On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
> >> Update the code to use devm_* API so that driver core will manage
> >> resources.
> >>
> >> Signed-off-by: Manish Badarkhe
> >> ---
> >> Changes since V1:
> >> 1. Updated driver to use "devm_kzalloc" to "kstrdup".
> >> 2. Updated commit message.
> >>
> >> Not tested on any board.
> >>
> >> :100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
> >> :
> >> drivers/regulator/fixed.c | 42
> >> --
> >> 1 file changed, 12 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
> >> index 5ea64b9..e9763a4 100644
> >> --- a/drivers/regulator/fixed.c
> >> +++ b/drivers/regulator/fixed.c
> >> @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
> >> platform_device *pdev)>>
> >> GFP_KERNEL);
> >>
> >> if (drvdata == NULL) {
> >>
> >> dev_err(&pdev->dev, "Failed to allocate device data\n");
> >>
> >> - ret = -ENOMEM;
> >> - goto err;
> >> + return -ENOMEM;
> >>
> >> }
> >>
> >> - drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
> >> + drvdata->desc.name = devm_kzalloc(&pdev->dev,
> >> + strlen(config->supply_name) + 1,
> >> + GFP_KERNEL);
> >>
> >> if (drvdata->desc.name == NULL) {
> >>
> >> dev_err(&pdev->dev, "Failed to allocate supply name\n");
> >>
> >> - ret = -ENOMEM;
> >> - goto err;
> >> + return -ENOMEM;
> >>
> >> }
> >
> > Umm, I am fairly certain that devm_kzalloc() can't be used as a
> > substitute for kstrdup, at least not without accompanying memcpy.
>
> Yes, I have provided allocation but it should be followed with assignment.
> Can I modify like this,
>
> + drvdata->desc.name = devm_kzalloc(&pdev->dev,
> + strlen(config->supply_name) + 1,
> + GFP_KERNEL);
> + if (drvdata->desc.name)
> + sprintf(drvdata->desc.name, "%s", config->supply_name);
hmm, so you replaced a general helper function by open coding the string-
duplication. Doesn't this defeat the target of simplifying the code?
Heiko
--
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] regulator: fixed: update to devm_* API
Hi Dmitry,
Thank you for your review.
On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov
wrote:
> Hi Manish,
>
> On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
>> Update the code to use devm_* API so that driver core will manage
>> resources.
>>
>> Signed-off-by: Manish Badarkhe
>> ---
>> Changes since V1:
>> 1. Updated driver to use "devm_kzalloc" to "kstrdup".
>> 2. Updated commit message.
>>
>> Not tested on any board.
>>
>> :100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
>> drivers/regulator/fixed.c | 42 --
>> 1 file changed, 12 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
>> index 5ea64b9..e9763a4 100644
>> --- a/drivers/regulator/fixed.c
>> +++ b/drivers/regulator/fixed.c
>> @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
>> platform_device *pdev)
>> GFP_KERNEL);
>> if (drvdata == NULL) {
>> dev_err(&pdev->dev, "Failed to allocate device data\n");
>> - ret = -ENOMEM;
>> - goto err;
>> + return -ENOMEM;
>> }
>>
>> - drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
>> + drvdata->desc.name = devm_kzalloc(&pdev->dev,
>> + strlen(config->supply_name) + 1,
>> + GFP_KERNEL);
>> if (drvdata->desc.name == NULL) {
>> dev_err(&pdev->dev, "Failed to allocate supply name\n");
>> - ret = -ENOMEM;
>> - goto err;
>> + return -ENOMEM;
>> }
>
> Umm, I am fairly certain that devm_kzalloc() can't be used as a
> substitute for kstrdup, at least not without accompanying memcpy.
Yes, I have provided allocation but it should be followed with assignment.
Can I modify like this,
+ drvdata->desc.name = devm_kzalloc(&pdev->dev,
+ strlen(config->supply_name) + 1,
+ GFP_KERNEL);
+ if (drvdata->desc.name)
+ sprintf(drvdata->desc.name, "%s", config->supply_name);
Thanks
Manish Badakhe
--
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] regulator: fixed: update to devm_* API
Hi Manish,
On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
> Update the code to use devm_* API so that driver core will manage
> resources.
>
> Signed-off-by: Manish Badarkhe
> ---
> Changes since V1:
> 1. Updated driver to use "devm_kzalloc" to "kstrdup".
> 2. Updated commit message.
>
> Not tested on any board.
>
> :100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
> drivers/regulator/fixed.c | 42 --
> 1 file changed, 12 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
> index 5ea64b9..e9763a4 100644
> --- a/drivers/regulator/fixed.c
> +++ b/drivers/regulator/fixed.c
> @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
> platform_device *pdev)
> GFP_KERNEL);
> if (drvdata == NULL) {
> dev_err(&pdev->dev, "Failed to allocate device data\n");
> - ret = -ENOMEM;
> - goto err;
> + return -ENOMEM;
> }
>
> - drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
> + drvdata->desc.name = devm_kzalloc(&pdev->dev,
> + strlen(config->supply_name) + 1,
> + GFP_KERNEL);
> if (drvdata->desc.name == NULL) {
> dev_err(&pdev->dev, "Failed to allocate supply name\n");
> - ret = -ENOMEM;
> - goto err;
> + return -ENOMEM;
> }
Umm, I am fairly certain that devm_kzalloc() can't be used as a
substitute for kstrdup, at least not without accompanying memcpy.
Thanks.
--
Dmitry
--
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/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
Hi Mike,
On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette wrote:
> Quoting Thomas Abraham (2014-01-18 04:10:51)
>> From: Thomas Abraham
>>
>> On some platforms such as the Samsung Exynos, changing the frequency
>> of the CPU clock requires changing the frequency of the PLL that is
>> supplying the CPU clock. To change the frequency of the PLL, the CPU
>> clock is temporarily reparented to another parent clock.
>>
>> The clock frequency of this temporary parent clock could be much higher
>> than the clock frequency of the PLL at the time of reparenting. Due
>> to the temporary increase in the CPU clock speed, the CPU (and any other
>> components in the CPU clock domain such as dividers, mux, etc.) have to
>> to be operated at a higher voltage level, called the safe voltage level.
>> This patch adds optional support to temporarily switch to a safe voltage
>> level during CPU frequency transitions.
>>
>> Cc: Shawn Guo
>> Signed-off-by: Thomas Abraham
>
> I'm not a fan of this change. This corner case should be abstracted away
> somehow. I had talked to Chander Kayshap previously about handling
> voltage changes in clock notifier callbacks, which then renders any
> voltage change as a trivial part of the clock rate transition. That
> means that this "safe voltage" thing could be handled automagically
> without any additional code in the CPUfreq driver.
>
> There are two nice ways to do this with the clock framework. First is
> explicit re-parenting with voltage scaling done in the clock rate-change
> notifiers:
>
> clk_set_parent(cpu_clk, temp_parent);
> /* implicit voltage scaling to "safe voltage" happens above */
> clk_set_rate(pll, some_rate);
> clk_set_parent(cpu_clk, pll);
> /* implicit voltage scaling to nominal OPP voltage happens above */
>
> The above sequence would require a separate exnyos CPUfreq driver, due
> to the added clk_set_parent logic.
>
> The second way to do this is to abstract the clk re-muxing logic out
> into the clk driver, which would allow cpufreq-cpu0 to be used for the
> exynos chips.
This is the approach this patch series takes (patch 2/7). The clock
re-muxing logic is handled by a clock driver code. The difference from
what you suggested is that the safe voltage (that may be optionally)
required before doing the re-muxing is handled here in cpufreq-cpu0
driver.
The safe voltage setup can be done in the notifier as you suggested.
But, doing that in cpufreq-cpu0 driver will help other platforms reuse
this feature if required. Also, if done here, the regulator handling
is localized in this driver which otherwise would need to be handled
in two places, cpufreq-cpu0 driver and the clock notifier.
So I tend to prefer the approach in this patch but I am willing to
consider any suggestions. Shawn, it would be helpful if you could let
us know your thoughts on this. I am almost done with testing the v3 of
this series and want to post it so if there are any objections to the
changes in this patch, please let me know.
Thanks,
Thomas.
>
> I'm more a fan of explicitly listing the Exact Steps for the cpu opp
> transition in a separate exynos-specific CPUfreq driver, but that's
> probably an unpopular view.
>
> Regards,
> Mike
>
>> ---
>> .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt |7
>> drivers/cpufreq/cpufreq-cpu0.c | 37
>> +--
>> 2 files changed, 40 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> index f055515..37453ab 100644
>> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> @@ -19,6 +19,12 @@ Optional properties:
>> - cooling-min-level:
>> - cooling-max-level:
>> Please refer to Documentation/devicetree/bindings/thermal/thermal.txt.
>> +- safe-opp: Certain platforms require that during a opp transition,
>> + a system should not go below a particular opp level. For such systems,
>> + this property specifies the minimum opp to be maintained during the
>> + opp transitions. The safe-opp value is a tuple with first element
>> + representing the safe frequency and the second element representing the
>> + safe voltage.
>>
>> Examples:
>>
>> @@ -36,6 +42,7 @@ cpus {
>> 396000 95
>> 198000 85
>> >;
>> + safe-opp = <396000 95>
>> clock-latency = <61036>; /* two CLK32 periods */
>> #cooling-cells = <2>;
>> cooling-min-level = <0>;
>> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
>> index 0c12ffc..075d3d1 100644
>> --- a/drivers/cpufreq/cpufreq-cpu0.c
>> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>> @@ -27,6 +27,8 @@
>>
>> static unsigned int transition_latency;
>> static unsigned int voltage_tolerance; /* in percentage */
>> +static unsigned
Re: [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
Hi Shawn,
On Mon, Jan 27, 2014 at 12:46 PM, Shawn Guo wrote:
> On Sat, Jan 18, 2014 at 05:40:51PM +0530, Thomas Abraham wrote:
>> From: Thomas Abraham
>>
>> On some platforms such as the Samsung Exynos, changing the frequency
>> of the CPU clock requires changing the frequency of the PLL that is
>> supplying the CPU clock. To change the frequency of the PLL, the CPU
>> clock is temporarily reparented to another parent clock.
>>
>> The clock frequency of this temporary parent clock could be much higher
>> than the clock frequency of the PLL at the time of reparenting. Due
>> to the temporary increase in the CPU clock speed, the CPU (and any other
>> components in the CPU clock domain such as dividers, mux, etc.) have to
>> to be operated at a higher voltage level, called the safe voltage level.
>> This patch adds optional support to temporarily switch to a safe voltage
>> level during CPU frequency transitions.
>>
>> Cc: Shawn Guo
>> Signed-off-by: Thomas Abraham
>> ---
>> .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt |7
>
> The devicetree list should be copied for this change.
Okay, will do in the next version.
>
>> drivers/cpufreq/cpufreq-cpu0.c | 37
>> +--
>> 2 files changed, 40 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> index f055515..37453ab 100644
>> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> @@ -19,6 +19,12 @@ Optional properties:
>> - cooling-min-level:
>> - cooling-max-level:
>> Please refer to Documentation/devicetree/bindings/thermal/thermal.txt.
>> +- safe-opp: Certain platforms require that during a opp transition,
>> + a system should not go below a particular opp level. For such systems,
>> + this property specifies the minimum opp to be maintained during the
>> + opp transitions. The safe-opp value is a tuple with first element
>> + representing the safe frequency and the second element representing the
>> + safe voltage.
>>
>> Examples:
>>
>> @@ -36,6 +42,7 @@ cpus {
>> 396000 95
>> 198000 85
>> >;
>> + safe-opp = <396000 95>
>> clock-latency = <61036>; /* two CLK32 periods */
>> #cooling-cells = <2>;
>> cooling-min-level = <0>;
>> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
>> index 0c12ffc..075d3d1 100644
>> --- a/drivers/cpufreq/cpufreq-cpu0.c
>> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>> @@ -27,6 +27,8 @@
>>
>> static unsigned int transition_latency;
>> static unsigned int voltage_tolerance; /* in percentage */
>> +static unsigned long safe_frequency;
>> +static unsigned long safe_voltage;
>>
>> static struct device *cpu_dev;
>> static struct clk *cpu_clk;
>> @@ -64,17 +66,30 @@ static int cpu0_set_target(struct cpufreq_policy
>> *policy, unsigned int index)
>> volt_old = regulator_get_voltage(cpu_reg);
>> }
>>
>> - pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
>> + pr_debug("\n\n%u MHz, %ld mV --> %u MHz, %ld mV\n",
>
> This is an unnecessary change?
Yes, sorry missed that.
>
> Otherwise,
>
> Acked-by: Shawn Guo
Thanks for your review.
Regards,
Thomas.
>
> Shawn
>
>>old_freq / 1000, volt_old ? volt_old / 1000 : -1,
>>new_freq / 1000, volt ? volt / 1000 : -1);
>>
>> /* scaling up? scale voltage before frequency */
>> - if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
>> + if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
>> + new_freq >= safe_frequency) {
>> ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
>> if (ret) {
>> pr_err("failed to scale voltage up: %d\n", ret);
>> return ret;
>> }
>> + } else if (!IS_ERR(cpu_reg) && old_freq < safe_frequency) {
>> + /*
>> + * the scaled up voltage level for the new_freq is lower
>> + * than the safe voltage level. so set safe_voltage
>> + * as the intermediate voltage level and revert it
>> + * back after the frequency has been changed.
>> + */
>> + ret = regulator_set_voltage_tol(cpu_reg, safe_voltage, tol);
>> + if (ret) {
>> + pr_err("failed to set safe voltage: %d\n", ret);
>> + return ret;
>> + }
>> }
>>
>> ret = clk_set_rate(cpu_clk, freq_exact);
>> @@ -86,7 +101,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
>> unsigned int index)
>> }
>>
>> /* scaling down? scale voltage after frequency */
>> - if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
>> + if (!IS_ERR(cpu_reg) &&
>> +
[PATCH V2] regulator: fixed: update to devm_* API
Update the code to use devm_* API so that driver core will manage
resources.
Signed-off-by: Manish Badarkhe
---
Changes since V1:
1. Updated driver to use "devm_kzalloc" to "kstrdup".
2. Updated commit message.
Not tested on any board.
:100644 100644 5ea64b9... e9763a4... M drivers/regulator/fixed.c
drivers/regulator/fixed.c | 42 --
1 file changed, 12 insertions(+), 30 deletions(-)
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..e9763a4 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct platform_device
*pdev)
GFP_KERNEL);
if (drvdata == NULL) {
dev_err(&pdev->dev, "Failed to allocate device data\n");
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
- drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
+ drvdata->desc.name = devm_kzalloc(&pdev->dev,
+ strlen(config->supply_name) + 1,
+ GFP_KERNEL);
if (drvdata->desc.name == NULL) {
dev_err(&pdev->dev, "Failed to allocate supply name\n");
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
drvdata->desc.type = REGULATOR_VOLTAGE;
drvdata->desc.owner = THIS_MODULE;
@@ -149,13 +149,13 @@ static int reg_fixed_voltage_probe(struct platform_device
*pdev)
drvdata->desc.enable_time = config->startup_delay;
if (config->input_supply) {
- drvdata->desc.supply_name = kstrdup(config->input_supply,
- GFP_KERNEL);
+ drvdata->desc.supply_name = devm_kzalloc(&pdev->dev,
+ strlen(config->input_supply) + 1,
+ GFP_KERNEL);
if (!drvdata->desc.supply_name) {
dev_err(&pdev->dev,
"Failed to allocate input supply\n");
- ret = -ENOMEM;
- goto err_name;
+ return -ENOMEM;
}
}
@@ -186,11 +186,12 @@ static int reg_fixed_voltage_probe(struct platform_device
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev->dev.of_node;
- drvdata->dev = regulator_register(&drvdata->desc, &cfg);
+ drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
+ &cfg);
if (IS_ERR(drvdata->dev)) {
ret = PTR_ERR(drvdata->dev);
dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret);
- goto err_input;
+ return ret;
}
platform_set_drvdata(pdev, drvdata);
@@ -199,24 +200,6 @@ static int reg_fixed_voltage_probe(struct platform_device
*pdev)
drvdata->desc.fixed_uV);
return 0;
-
-err_input:
- kfree(drvdata->desc.supply_name);
-err_name:
- kfree(drvdata->desc.name);
-err:
- return ret;
-}
-
-static int reg_fixed_voltage_remove(struct platform_device *pdev)
-{
- struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
-
- regulator_unregister(drvdata->dev);
- kfree(drvdata->desc.supply_name);
- kfree(drvdata->desc.name);
-
- return 0;
}
#if defined(CONFIG_OF)
@@ -229,7 +212,6 @@ MODULE_DEVICE_TABLE(of, fixed_of_match);
static struct platform_driver regulator_fixed_voltage_driver = {
.probe = reg_fixed_voltage_probe,
- .remove = reg_fixed_voltage_remove,
.driver = {
.name = "reg-fixed-voltage",
.owner = THIS_MODULE,
--
1.7.10.4
--
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: Samsung-clk patches for 3.15
On 28.01.2014 01:09, Kukjin Kim wrote: Tomasz Figa wrote: [Forgot to Cc Mike...] On 24.01.2014 15:38, Tomasz Figa wrote: Hi, Hi, Linux 3.14 is going to include Andrzej Hajda's patches converting Samsung clock drivers to use clock ID defines in include/dt-bindings, instead of local enums, but to avoid unnecessary merge conflicts we have converted only the clock driver, leaving DTS files unchanged yet. We intend to complete the conversion in 3.15, by replacing magic numbers in DTS files with respective preprocessor macros, but to reduce potential conflicts we need help of you, Samsung clock patches authors :). I'd like to ask anybody who already has patches for DTS files adding any clock-related contents still using numeric IDs, e.g. clock properties in nodes or full nodes containing clock properties, to make sure that the patches are merged before Andrzej sends the conversion patches. Then Andrzej's script will generate patches updating all clock properties, leaving no numeric IDs in DTS files. There are several DTS patches in v3.14-drop/soc-exynos-2 branch of my tree for 3.15 and it will be merged after done of multiplatform, I need to rebase them based on v3.14-rc1 though...So I think, would be better if we could update DTS with using Andrzej's script after merging it into arm-soc... Basically my intention is to: 1) Have any existing patches using clock numbers merged in reasonable period of time (to not miss the merge window with DTS conversion patches). 2) Stop accepting such patches anymore. 3) Rerun Andrzej's script and convert all device tree sources to use clock macros. Andrzej's patches for DTSes should go through your tree anyway (as any Samsung DTS patches by default), so it shouldn't be a problem, regardless of merging anything into arm-soc. Best regards, Tomasz -- 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: Samsung-clk patches for 3.15
Tomasz Figa wrote: > > [Forgot to Cc Mike...] > > On 24.01.2014 15:38, Tomasz Figa wrote: > > Hi, > > Hi, > > Linux 3.14 is going to include Andrzej Hajda's patches converting > > Samsung clock drivers to use clock ID defines in include/dt-bindings, > > instead of local enums, but to avoid unnecessary merge conflicts we > > have converted only the clock driver, leaving DTS files unchanged yet. > > > > We intend to complete the conversion in 3.15, by replacing magic > > numbers in DTS files with respective preprocessor macros, but to > > reduce potential conflicts we need help of you, Samsung clock patches > authors :). > > > > I'd like to ask anybody who already has patches for DTS files adding > > any clock-related contents still using numeric IDs, e.g. clock > > properties in nodes or full nodes containing clock properties, to make > > sure that the patches are merged before Andrzej sends the conversion > > patches. Then Andrzej's script will generate patches updating all > > clock properties, leaving no numeric IDs in DTS files. > > There are several DTS patches in v3.14-drop/soc-exynos-2 branch of my tree for 3.15 and it will be merged after done of multiplatform, I need to rebase them based on v3.14-rc1 though...So I think, would be better if we could update DTS with using Andrzej's script after merging it into arm-soc... > > If you are just starting your work on a patch that introduces changes > > as mentioned above, please make sure to already use clock macros, not > > numeric IDs. Otherwise you risk having needless rebases with a lot of > > conflicts. You have been warned ;). > > Thanks, Kukjin -- 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: Samsung-clk patches for 3.15
[Forgot to Cc Mike...] On 24.01.2014 15:38, Tomasz Figa wrote: Hi, Linux 3.14 is going to include Andrzej Hajda's patches converting Samsung clock drivers to use clock ID defines in include/dt-bindings, instead of local enums, but to avoid unnecessary merge conflicts we have converted only the clock driver, leaving DTS files unchanged yet. We intend to complete the conversion in 3.15, by replacing magic numbers in DTS files with respective preprocessor macros, but to reduce potential conflicts we need help of you, Samsung clock patches authors :). I'd like to ask anybody who already has patches for DTS files adding any clock-related contents still using numeric IDs, e.g. clock properties in nodes or full nodes containing clock properties, to make sure that the patches are merged before Andrzej sends the conversion patches. Then Andrzej's script will generate patches updating all clock properties, leaving no numeric IDs in DTS files. If you are just starting your work on a patch that introduces changes as mentioned above, please make sure to already use clock macros, not numeric IDs. Otherwise you risk having needless rebases with a lot of conflicts. You have been warned ;). Best regards, Tomasz ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
Quoting Thomas Abraham (2014-01-18 04:10:51)
> From: Thomas Abraham
>
> On some platforms such as the Samsung Exynos, changing the frequency
> of the CPU clock requires changing the frequency of the PLL that is
> supplying the CPU clock. To change the frequency of the PLL, the CPU
> clock is temporarily reparented to another parent clock.
>
> The clock frequency of this temporary parent clock could be much higher
> than the clock frequency of the PLL at the time of reparenting. Due
> to the temporary increase in the CPU clock speed, the CPU (and any other
> components in the CPU clock domain such as dividers, mux, etc.) have to
> to be operated at a higher voltage level, called the safe voltage level.
> This patch adds optional support to temporarily switch to a safe voltage
> level during CPU frequency transitions.
>
> Cc: Shawn Guo
> Signed-off-by: Thomas Abraham
I'm not a fan of this change. This corner case should be abstracted away
somehow. I had talked to Chander Kayshap previously about handling
voltage changes in clock notifier callbacks, which then renders any
voltage change as a trivial part of the clock rate transition. That
means that this "safe voltage" thing could be handled automagically
without any additional code in the CPUfreq driver.
There are two nice ways to do this with the clock framework. First is
explicit re-parenting with voltage scaling done in the clock rate-change
notifiers:
clk_set_parent(cpu_clk, temp_parent);
/* implicit voltage scaling to "safe voltage" happens above */
clk_set_rate(pll, some_rate);
clk_set_parent(cpu_clk, pll);
/* implicit voltage scaling to nominal OPP voltage happens above */
The above sequence would require a separate exnyos CPUfreq driver, due
to the added clk_set_parent logic.
The second way to do this is to abstract the clk re-muxing logic out
into the clk driver, which would allow cpufreq-cpu0 to be used for the
exynos chips.
I'm more a fan of explicitly listing the Exact Steps for the cpu opp
transition in a separate exynos-specific CPUfreq driver, but that's
probably an unpopular view.
Regards,
Mike
> ---
> .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt |7
> drivers/cpufreq/cpufreq-cpu0.c | 37 +--
> 2 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> index f055515..37453ab 100644
> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> @@ -19,6 +19,12 @@ Optional properties:
> - cooling-min-level:
> - cooling-max-level:
> Please refer to Documentation/devicetree/bindings/thermal/thermal.txt.
> +- safe-opp: Certain platforms require that during a opp transition,
> + a system should not go below a particular opp level. For such systems,
> + this property specifies the minimum opp to be maintained during the
> + opp transitions. The safe-opp value is a tuple with first element
> + representing the safe frequency and the second element representing the
> + safe voltage.
>
> Examples:
>
> @@ -36,6 +42,7 @@ cpus {
> 396000 95
> 198000 85
> >;
> + safe-opp = <396000 95>
> clock-latency = <61036>; /* two CLK32 periods */
> #cooling-cells = <2>;
> cooling-min-level = <0>;
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index 0c12ffc..075d3d1 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -27,6 +27,8 @@
>
> static unsigned int transition_latency;
> static unsigned int voltage_tolerance; /* in percentage */
> +static unsigned long safe_frequency;
> +static unsigned long safe_voltage;
>
> static struct device *cpu_dev;
> static struct clk *cpu_clk;
> @@ -64,17 +66,30 @@ static int cpu0_set_target(struct cpufreq_policy *policy,
> unsigned int index)
> volt_old = regulator_get_voltage(cpu_reg);
> }
>
> - pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> + pr_debug("\n\n%u MHz, %ld mV --> %u MHz, %ld mV\n",
> old_freq / 1000, volt_old ? volt_old / 1000 : -1,
> new_freq / 1000, volt ? volt / 1000 : -1);
>
> /* scaling up? scale voltage before frequency */
> - if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
> + if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
> + new_freq >= safe_frequency) {
> ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> if (ret) {
> pr_err("failed to scale voltage up: %d\n", ret);
> return ret;
> }
> + } else if (!IS_ERR(cpu_reg) && old_freq < safe_frequency) {
> + /*
> +
Re: [PATCH v3 1/2] regulator: s5m8767: Use GPIO for controlling Buck9/eMMC
On Fri, Jan 24, 2014 at 02:37:57PM +0100, Krzysztof Kozlowski wrote: > Add support for GPIO control (enable/disable) over Buck9. The Buck9 > Converter is used as a supply for eMMC Host Controller. Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH v3 2/2] regulator: s5m8767: Document new binding for Buck9 GPIO control
On Fri, Jan 24, 2014 at 02:37:58PM +0100, Krzysztof Kozlowski wrote: > Add documentation for new binding for controlling (enable/disable) the > Buck9 Converter by GPIO (BUCK9EN). Applied, thanks. signature.asc Description: Digital signature
[PATCH V3] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.
Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.
Signed-off-by: Manish Badarkhe
---
:100644 100644 b4513f2... 3e54476... M drivers/power/max8925_power.c
drivers/power/max8925_power.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..3e54476 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct max8925_power_info
*info)
return 0;
}
-#ifdef CONFIG_OF
static struct max8925_power_pdata *
max8925_power_dt_init(struct platform_device *pdev)
{
@@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
int no_insert_detect;
struct max8925_power_pdata *pdata;
- if (!nproot)
- return pdev->dev.platform_data;
-
np = of_find_node_by_name(nproot, "charger");
if (!np) {
dev_err(&pdev->dev, "failed to find charger node\n");
@@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
return pdata;
}
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
- return pdev->dev.platform_data;
-}
-#endif
static int max8925_power_probe(struct platform_device *pdev)
{
@@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device
*pdev)
struct max8925_power_info *info;
int ret;
- pdata = max8925_power_dt_init(pdev);
+ pdata = dev_get_platdata(&pdev->dev);
+
+ if (!pdata && chip->dev->of_node)
+ pdata = max8925_power_dt_init(pdev);
+
if (!pdata) {
dev_err(&pdev->dev, "platform data isn't assigned to "
"power supply\n");
--
1.7.10.4
--
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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, 27 Jan 2014, Russell King - ARM Linux wrote: > On Sun, Jan 26, 2014 at 11:30:00PM -0500, Nicolas Pitre wrote: > > On Sun, 26 Jan 2014, Russell King - ARM Linux wrote: > > > > > On Tue, Jan 21, 2014 at 12:45:05AM +, Alan Cox wrote: > > > > Peter handed it on. Try using git log on Documentation/devices.txt. It > > > > still gets updates. > > > > > > > > Perhaps you'd care to stick to reality and fix the tree instead of > > > > trying > > > > to excuse the mess ? > > > > > > Perhaps returning to reality might be advantageous rather than trying > > > to repeat statements which can't have any bearing on this - especially > > > as the git history which you're referring to only goes back to 2.6.12-rc2, > > > and this predates 2.6.12-rc2 by a long shot. > > > > > > > More importantly certain folks need to stop abusing static numbers > > > > allocated properly. Repeating it having made a total hash of it before > > > > is dismal. > > > > > > And if you continue these stupid accusations which have no basis at all, > > > we're going to get into a real big argument, because you are soo _wrong_ > > > on that point. I was always the one arguing /against/ the re-use of > > > existing major/minor numbers. I was the one arguing /against/ Nicolas' > > > patches to make every serial port appear in the 4,64 ttyS namespace. > > > > If you remember correctly, that was my attempt at making serial port > > minor assignment to be dynamic... just like everything else is today. > > And it seemed to me that you thought this was a good idea. > > I may have thought that a dynamic space for serial devices was a good > idea, but what I was referring to above was specifically the > implementation. Oh I do not dispute the fact that the implementation at the time was not up to needed standards. That would have been fixed eventually though, if it hadn't been for the policy based objection we received. At which point this effort was simply dropped. Nicolas -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Sun, Jan 26, 2014 at 11:30:00PM -0500, Nicolas Pitre wrote: > On Sun, 26 Jan 2014, Russell King - ARM Linux wrote: > > > On Tue, Jan 21, 2014 at 12:45:05AM +, Alan Cox wrote: > > > Peter handed it on. Try using git log on Documentation/devices.txt. It > > > still gets updates. > > > > > > Perhaps you'd care to stick to reality and fix the tree instead of trying > > > to excuse the mess ? > > > > Perhaps returning to reality might be advantageous rather than trying > > to repeat statements which can't have any bearing on this - especially > > as the git history which you're referring to only goes back to 2.6.12-rc2, > > and this predates 2.6.12-rc2 by a long shot. > > > > > More importantly certain folks need to stop abusing static numbers > > > allocated properly. Repeating it having made a total hash of it before > > > is dismal. > > > > And if you continue these stupid accusations which have no basis at all, > > we're going to get into a real big argument, because you are soo _wrong_ > > on that point. I was always the one arguing /against/ the re-use of > > existing major/minor numbers. I was the one arguing /against/ Nicolas' > > patches to make every serial port appear in the 4,64 ttyS namespace. > > If you remember correctly, that was my attempt at making serial port > minor assignment to be dynamic... just like everything else is today. > And it seemed to me that you thought this was a good idea. I may have thought that a dynamic space for serial devices was a good idea, but what I was referring to above was specifically the implementation. Unfortunately, there's precious little public evidence of this as the patches were never posted to a public mailing list. However, what there is (as part of another thread) shows that I held that view: http://archive.arm.linux.org.uk/lurker/message/20041124.164950.92dc25d7.en.html Plus, of course, the comments in the patch system where I picked out a number of further technical issues, such as changing inode->i_rdev, userspace locking, etc. If you want to review them, they're 1427/1 - 1434/1, 1435/2, 1436/2. Unfortunately the authorship of those comments was lost. Hence, my recollection is correct here. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
> Either everything is dynamic, or everything follows proper minor > assignment process. Ultimately everything should probably be dynamic, but trying to get there in one step will simply mean we never get there at all. Alan -- 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] phy: Add new Exynos5 USB 3.0 PHY driver
Hi,
On Monday 20 January 2014 07:12 PM, Vivek Gautam wrote:
> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
> The new driver uses the generic PHY framework and will interact
> with DWC3 controller present on Exynos5 series of SoCs.
> Thereby, removing old phy-samsung-usb3 driver and related code
> used untill now which was based on usb/phy framework.
>
> Signed-off-by: Vivek Gautam
> ---
>
> Changes from v2:
> 1) Added support for multiple PHYs (UTMI+ and PIPE3) and
>related changes in the driver structuring.
> 2) Added a xlate function to get the required phy out of
>number of PHYs in mutiple PHY scenerio.
> 3) Changed the names of few structures and variables to
>have a clearer meaning.
> 4) Added 'usb3phy_config' structure to take care of mutiple
>phys for a SoC having 'exynos5_usb3phy_drv_data' driver data.
> 5) Not deleting support for old driver 'phy-samsung-usb3' until
>required support for generic phy is added to DWC3.
>
> .../devicetree/bindings/phy/samsung-phy.txt| 49 ++
> drivers/phy/Kconfig|8 +
> drivers/phy/Makefile |1 +
> drivers/phy/phy-exynos5-usb3.c | 621
>
> 4 files changed, 679 insertions(+)
> create mode 100644 drivers/phy/phy-exynos5-usb3.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index c0fccaa..57079f8 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -20,3 +20,52 @@ Required properties:
> - compatible : should be "samsung,exynos5250-dp-video-phy";
> - reg : offset and length of the Display Port PHY register set;
> - #phy-cells : from the generic PHY bindings, must be 0;
> +
> +Samsung Exynos5 SoC series USB 3.0 PHY controller
> +--
> +
> +Required properties:
> +- compatible : Should be set to one of the following supported values:
> + - "samsung,exynos5250-usb3phy" - for exynos5250 SoC,
> + - "samsung,exynos5420-usb3phy" - for exynos5420 SoC.
> +- reg : Register offset and length of USB 3.0 PHY register set;
> +- clocks: Clock IDs array as required by the controller
> +- clock-names: names of clocks correseponding to IDs in the clock property;
> + Required clocks:
> + - phy: main PHY clock (same as USB 3.0 controller IP clock),
> + used for register access.
> + - usb3phy_refclk: PHY's reference clock (usually crystal clock),
> + associated by phy name, used to determine bit values for
> + clock settings register.
> + Additional clock required for Exynos5420:
> + - usb30_sclk_100m: Additional special clock used for PHY operation
> +depicted as 'sclk_usbphy30' in CMU of Exynos5420.
> +- samsung,syscon-phandle: phandle for syscon interface, which is used to
> + control pmu registers for power isolation.
> +- #phy-cells : from the generic PHY bindings, must be 1;
> +
> +For "samsung,exynos5250-usb3phy" and "samsung,exynos5420-usb3phy" compatible
> +PHYs, the second cell in the PHY specifier identifies the PHY id, which is
> +interpreted as follows:
> + 0 - UTMI+ type phy,
> + 1 - PIPE3 type phy,
> +
> +Example:
> + usb3_phy: usbphy@1210 {
> + compatible = "samsung,exynos5250-usb3phy";
> + reg = <0x1210 0x100>;
> + clocks = <&clock 286>, <&clock 1>;
> + clock-names = "phy", "usb3phy_refclk";
> + samsung,syscon-phandle = <&pmu_syscon>;
> + #phy-cells = <1>;
> + };
> +
> +- aliases: For SoCs like Exynos5420 having multiple USB PHY controllers,
> +'usb3_phy' nodes should have numbered alias in the aliases node,
> +in the form of usb3phyN, N = 0, 1... (depending on number of
> +controllers).
> +Example:
> + aliases {
> + usb3phy0 = &usb3_phy0;
> + usb3phy1 = &usb3_phy1;
> + };
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 330ef2d..32f9f38 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -51,4 +51,12 @@ config PHY_EXYNOS_DP_VIDEO
> help
> Support for Display Port PHY found on Samsung EXYNOS SoCs.
>
> +config PHY_EXYNOS5_USB3
> + tristate "Exynos5 SoC series USB 3.0 PHY driver"
> + depends on ARCH_EXYNOS5
> + select GENERIC_PHY
> + select MFD_SYSCON
add depends on 'HAS_IOMEM'. Someone reported getting
undefined reference to `devm_ioremap_resource' with it.
> + help
> + Enable USB 3.0 PHY support for Exynos 5 SoC series
> +
> endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..9c06a61 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> obj-$(CONFIG_PHY_EXYNOS_M
