Re: snowball does not compile on linux-next
On Tue, Jan 3, 2012 at 2:20 PM, Christian Robottom Reis k...@linaro.org wrote: On Tue, Jan 03, 2012 at 08:19:37AM +0100, Linus Walleij wrote: It should be in the defconfig already, the problem showed up for Daniel exactly because it was configured in. Okay, then the question is what do we need to do to make a box go red and have people notified when it breaks? Since this was a simple compilation issue, it's indicated already by Stephen Rothwells kisskb system: http://kisskb.ellerman.id.au/kisskb/branch/9/ I actually poll the above web page for my machines, sadly not every day due to other duties... Yours, Linus Walleij ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [lm-sensors] [RFC PATCH 2/3] thermal: exynos4: Register the tmu sensor with the thermal interface layer
Hi Guenter,
The main idea of this work is to leave the current userspace based
notification scheme and add the kernel based cooling scheme on top of
it. Anyway, It is a good idea to move the file hwmon/exynos4_tmu.c as
this creates 2 hwmon entries.
Adding CC: Donggeun Kim to know his opinion.
Thanks,
Amit Daniel
On 4 January 2012 03:55, Guenter Roeck guenter.ro...@ericsson.com wrote:
On Wed, 2011-12-21 at 06:59 -0500, Amit Daniel Kachhap wrote:
Export and register information from the hwmon tmu sensor to the samsung
exynos kernel thermal framework where different cooling devices and thermal
zone are binded. The exported information is based according to the data
structure thermal_sensor_conf present in exynos_thermal.h. HWMON sysfs
functions are currently left although all of them are present in generic
linux thermal layer.
Also the platform data structure is modified to pass frequency cooling
in percentages for each thermal level.
Hi Amit,
wouldn't it make more sense to merge the code as necessary from
hwmon/exynos4_tmu.c into the new thermal/exynos_thermal.c, and drop
hwmon/exynos4_tmu.c entirely ?
With that, you should get the hwmon entries for free, and we would not
have to maintain two drivers with overlapping functionality.
Thanks,
Guenter
Signed-off-by: Amit Daniel Kachhap amit.kach...@linaro.org
---
drivers/hwmon/exynos4_tmu.c | 34
++--
include/linux/platform_data/exynos4_tmu.h | 7 ++
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/exynos4_tmu.c b/drivers/hwmon/exynos4_tmu.c
index f2359a0..6912a7f 100644
--- a/drivers/hwmon/exynos4_tmu.c
+++ b/drivers/hwmon/exynos4_tmu.c
@@ -37,6 +37,9 @@
#include linux/hwmon-sysfs.h
#include linux/platform_data/exynos4_tmu.h
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+#include linux/exynos_thermal.h
+#endif
#define EXYNOS4_TMU_REG_TRIMINFO 0x0
#define EXYNOS4_TMU_REG_CONTROL 0x20
@@ -248,10 +251,13 @@ static void exynos4_tmu_work(struct work_struct *work)
kobject_uevent(data-hwmon_dev-kobj, KOBJ_CHANGE);
- enable_irq(data-irq);
clk_disable(data-clk);
mutex_unlock(data-lock);
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+ exynos4_report_trigger();
+#endif
+ enable_irq(data-irq);
}
static irqreturn_t exynos4_tmu_irq(int irq, void *id)
@@ -345,6 +351,14 @@ static const struct attribute_group
exynos4_tmu_attr_group = {
.attrs = exynos4_tmu_attributes,
};
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+static struct thermal_sensor_conf exynos4_sensor_conf = {
+ .name = exynos4-therm,
+ .read_temperature = (int (*)(void *))exynos4_tmu_read,
+};
+#endif
+/*CONFIG_SAMSUNG_THERMAL_INTERFACE*/
+
static int __devinit exynos4_tmu_probe(struct platform_device *pdev)
{
struct exynos4_tmu_data *data;
@@ -432,9 +446,20 @@ static int __devinit exynos4_tmu_probe(struct
platform_device *pdev)
}
exynos4_tmu_control(pdev, true);
-
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+ (exynos4_sensor_conf)-private_data = data;
+ (exynos4_sensor_conf)-sensor_data = pdata;
+ ret = exynos4_register_thermal(exynos4_sensor_conf);
+ if (ret) {
+ dev_err(pdev-dev, Failed to register thermal interface\n);
+ goto err_hwmon_device;
+ }
+#endif
return 0;
-
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+err_hwmon_device:
+ hwmon_device_unregister(data-hwmon_dev);
+#endif
err_create_group:
sysfs_remove_group(pdev-dev.kobj, exynos4_tmu_attr_group);
err_clk:
@@ -458,6 +483,9 @@ static int __devexit exynos4_tmu_remove(struct
platform_device *pdev)
exynos4_tmu_control(pdev, false);
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+ exynos4_unregister_thermal();
+#endif
hwmon_device_unregister(data-hwmon_dev);
sysfs_remove_group(pdev-dev.kobj, exynos4_tmu_attr_group);
diff --git a/include/linux/platform_data/exynos4_tmu.h
b/include/linux/platform_data/exynos4_tmu.h
index 39e038c..642c508 100644
--- a/include/linux/platform_data/exynos4_tmu.h
+++ b/include/linux/platform_data/exynos4_tmu.h
@@ -21,6 +21,7 @@
#ifndef _LINUX_EXYNOS4_TMU_H
#define _LINUX_EXYNOS4_TMU_H
+#include linux/cpu_cooling.h
enum calibration_type {
TYPE_ONE_POINT_TRIMMING,
@@ -64,6 +65,9 @@ enum calibration_type {
* in the positive-TC generator block
* 0 = reference_voltage = 31
* @cal_type: calibration type for temperature
+ * @freq_pctg_table: Table representing frequency reduction percentage.
+ * @freq_tab_count: Count of the above table as frequency reduction may
+ * applicable to only some of the trigger levels.
*
* This structure is required for configuration of exynos4_tmu driver.
*/
@@ -79,5 +83,8 @@ struct exynos4_tmu_platform_data {
u8 reference_voltage;
enum calibration_type cal_type;
+
+ struct
RE: [linux-pm] [lm-sensors] [RFC PATCH 2/3] thermal: exynos4: Register the tmu sensor with the thermal interface layer
Hi Amit Daniel, Hi Guenter, The main idea of this work is to leave the current userspace based notification scheme and add the kernel based cooling scheme on top of it. Anyway, It is a good idea to move the file hwmon/exynos4_tmu.c as But, What I feel is, kernel based cooling scheme will work only for Controlling 'CPU' frequency. But in SoC's there are other devices that Contribute to Thermal. For example, GPU, Display, Battery (during charging) etc.. In this case, we need a user space to control these devices. So, in a way, the user space notification mechanism is a unified solution for throttling all devices and keeps the kernel code light weight. I am also curious to know why the existing mechanism did not work for you ? Thanks, Durga this creates 2 hwmon entries. Adding CC: Donggeun Kim to know his opinion. Thanks, Amit Daniel [snip.] ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v4 3/6] clk: introduce the common clock framework
On 01/03/2012 08:15 PM, Richard Zhao wrote:
On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner t...@linutronix.de wrote:
On Tue, 13 Dec 2011, Mike Turquette wrote:
snip
+/**
+ * clk_init - initialize the data structures in a struct clk
+ * @dev: device initializing this clk, placeholder for now
+ * @clk: clk being initialized
+ *
+ * Initializes the lists in struct clk, queries the hardware for the
+ * parent and rate and sets them both. Adds the clk to the sysfs tree
+ * topology.
+ *
+ * Caller must populate clk-name and clk-flags before calling
I'm not too happy about this construct. That leaves struct clk and its
members exposed to the world instead of making it a real opaque
cookie. I know from my own painful experience, that this will lead to
random fiddling in that data structure in drivers and arch code just
because the core code has a shortcoming.
Why can't we make struct clk a real cookie and confine the data
structure to the core code ?
That would change the init call to something like:
struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
struct clk *parent)
And have:
struct clk_hw {
struct clk_hw_ops *ops;
const char*name;
unsigned long flags;
};
Implementers can do:
struct my_clk_hw {
struct clk_hwhw;
mydata;
};
And then change the clk ops callbacks to take struct clk_hw * as an
argument.
We have to define static clocks before we adopt DT binding.
If clk is opaque and allocate memory in clk core, it'll make hard
to define static clocks. And register/init will pass a long parameter
list.
DT is not a prerequisite for having dynamically created clocks. You can
make clock init dynamic without DT.
What data goes in struct clk vs. struct clk_hw could change over time.
So perhaps we can start with some data in clk_hw and plan to move it to
struct clk later. Even if almost everything ends up in clk_hw initially,
at least the structure is in place to have common, core-only data
separate from platform data.
What is the actual data you need to be static and accessible to the
platform code? A ptr to parent clk is the main thing I've seen for
static initialization. So make the parent ptr be struct clk_hw* and
allow the platforms to access.
Rob
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
RE: [linux-pm] [lm-sensors] [RFC PATCH 2/3] thermal: exynos4: Register the tmu sensor with the thermal interface layer
On Wed, 2012-01-04 at 05:23 -0500, R, Durgadoss wrote: Hi Amit Daniel, Hi Guenter, The main idea of this work is to leave the current userspace based notification scheme and add the kernel based cooling scheme on top of it. Anyway, It is a good idea to move the file hwmon/exynos4_tmu.c as But, What I feel is, kernel based cooling scheme will work only for Controlling 'CPU' frequency. But in SoC's there are other devices that Contribute to Thermal. For example, GPU, Display, Battery (during charging) etc.. In this case, we need a user space to control these devices. So, in a way, the user space notification mechanism is a unified solution for throttling all devices and keeps the kernel code light weight. I am also curious to know why the existing mechanism did not work for you ? That is one question. For me, the main concern is that the proposed implementation creates duplicate hwmon entries for the same device, which is simply messy. If both the kernel thermal mechanism and the userspace mechanism are needed, I think it would make more sense to use a thermal driver and have that thermal driver generate the necessary userspace events. Thanks, Guenter Thanks, Durga this creates 2 hwmon entries. Adding CC: Donggeun Kim to know his opinion. Thanks, Amit Daniel [snip.] ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Binary toolchain preview
Hi there. We're putting together pre-built versions of the Linaro toolchain that cross-compile for the Linaro LEB and run on generic Linux or Windows. I've put a preview release together and would love some feedback. The binaries are available here: http://people.linaro.org/~michaelh/incoming/binaries/ The bug tracker is here: https://bugs.launchpad.net/linaro-toolchain-binaries There are a number of usability bugs logged already but nothing severe. You can also check out the requirements[1] and basic documentation[2]. Feedback is appreciated! -- Michael [1] https://wiki.linaro.org/WorkingGroups/ToolChain/Specs/BinaryToolchain [2] http://people.linaro.org/~michaelh/incoming/binaries/README.html ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V6 0/7] add a generic cpufreq driver
hi Russell, May I have your ACK, you merge it? Thanks Richard ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
RFC: Ideas for Linaro
Hi Gang, During some roadmapping work today the discussion of capturing and voting on ideas for Linaro came up. The idea was to possibly recreate a Linaro version of this http://brainstorm.ubuntu.com/ for everyone to contribute to. I'm looking for +1 and -1 feedback from folks about whether this would be useful (vs interesting). Joey ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v4 3/6] clk: introduce the common clock framework
On Wed, Jan 4, 2012 at 6:32 AM, Rob Herring robherri...@gmail.com wrote:
On 01/03/2012 08:15 PM, Richard Zhao wrote:
On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner t...@linutronix.de wrote:
On Tue, 13 Dec 2011, Mike Turquette wrote:
snip
+/**
+ * clk_init - initialize the data structures in a struct clk
+ * @dev: device initializing this clk, placeholder for now
+ * @clk: clk being initialized
+ *
+ * Initializes the lists in struct clk, queries the hardware for the
+ * parent and rate and sets them both. Adds the clk to the sysfs tree
+ * topology.
+ *
+ * Caller must populate clk-name and clk-flags before calling
I'm not too happy about this construct. That leaves struct clk and its
members exposed to the world instead of making it a real opaque
cookie. I know from my own painful experience, that this will lead to
random fiddling in that data structure in drivers and arch code just
because the core code has a shortcoming.
Why can't we make struct clk a real cookie and confine the data
structure to the core code ?
That would change the init call to something like:
struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
struct clk *parent)
And have:
struct clk_hw {
struct clk_hw_ops *ops;
const char *name;
unsigned long flags;
};
Implementers can do:
struct my_clk_hw {
struct clk_hw hw;
mydata;
};
And then change the clk ops callbacks to take struct clk_hw * as an
argument.
We have to define static clocks before we adopt DT binding.
If clk is opaque and allocate memory in clk core, it'll make hard
to define static clocks. And register/init will pass a long parameter
list.
DT is not a prerequisite for having dynamically created clocks. You can
make clock init dynamic without DT.
Agreed.
What data goes in struct clk vs. struct clk_hw could change over time.
So perhaps we can start with some data in clk_hw and plan to move it to
struct clk later. Even if almost everything ends up in clk_hw initially,
at least the structure is in place to have common, core-only data
separate from platform data.
What is the point of this?
The original clk_hw was defined simply as:
struct clk_hw {
struct clk *clk;
};
It's only purpose in life was as a handle for navigation between the
opaque struct clk and the hardware-specific struct my_clk_hw. struct
clk_hw is defined in clk.h and everyone can see it. If we're suddenly
OK putting clk data in this structure then why bother with an opaque
struct clk at all?
What is the actual data you need to be static and accessible to the
platform code? A ptr to parent clk is the main thing I've seen for
static initialization. So make the parent ptr be struct clk_hw* and
allow the platforms to access.
To answer your question on what data we're trying to expose: platform
code commonly needs the parent pointer and the clk rate (and by
extension, the rate of the parent). For debug/error prints it is also
nice to have the clk name. Generic clk flags are also conceivably
something that platform code might want.
I'd like to spin the question around: if we're OK exposing some stuff
(in your example above, the parent pointer), then what clk data are
you trying to hide?
Regards,
Mike
Rob
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re:RFC: Ideas for Linaro
At 2012-01-05 07:21:32,Joey STANFORD j...@linaro.org wrote: Hi Gang, During some roadmapping work today the discussion of capturing and voting on ideas for Linaro came up. The idea was to possibly recreate a Linaro version of this http://brainstorm.ubuntu.com/ for everyone to contribute to. I'm looking for +1 and -1 feedback from folks about whether this would be useful (vs interesting). Joey ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev Hi,Joey! Idea is so good.Catogetory is good! B.R TOMMY ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v4 3/6] clk: introduce the common clock framework
On Wed, Jan 04, 2012 at 05:01:43PM -0800, Turquette, Mike wrote:
On Wed, Jan 4, 2012 at 6:32 AM, Rob Herring robherri...@gmail.com wrote:
On 01/03/2012 08:15 PM, Richard Zhao wrote:
On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner t...@linutronix.de
wrote:
On Tue, 13 Dec 2011, Mike Turquette wrote:
snip
+/**
+ * clk_init - initialize the data structures in a struct clk
+ * @dev: device initializing this clk, placeholder for now
+ * @clk: clk being initialized
+ *
+ * Initializes the lists in struct clk, queries the hardware for the
+ * parent and rate and sets them both. Adds the clk to the sysfs tree
+ * topology.
+ *
+ * Caller must populate clk-name and clk-flags before calling
I'm not too happy about this construct. That leaves struct clk and its
members exposed to the world instead of making it a real opaque
cookie. I know from my own painful experience, that this will lead to
random fiddling in that data structure in drivers and arch code just
because the core code has a shortcoming.
Why can't we make struct clk a real cookie and confine the data
structure to the core code ?
That would change the init call to something like:
struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
struct clk *parent)
And have:
struct clk_hw {
struct clk_hw_ops *ops;
const char *name;
unsigned long flags;
};
Implementers can do:
struct my_clk_hw {
struct clk_hw hw;
mydata;
};
And then change the clk ops callbacks to take struct clk_hw * as an
argument.
We have to define static clocks before we adopt DT binding.
If clk is opaque and allocate memory in clk core, it'll make hard
to define static clocks. And register/init will pass a long parameter
list.
DT is not a prerequisite for having dynamically created clocks. You can
make clock init dynamic without DT.
I can not find clock info at runtime without DT. If I use static info, I
find it was hard/strange to define and register it, using Mike's early patches.
Agreed.
What data goes in struct clk vs. struct clk_hw could change over time.
So perhaps we can start with some data in clk_hw and plan to move it to
struct clk later. Even if almost everything ends up in clk_hw initially,
at least the structure is in place to have common, core-only data
separate from platform data.
What is the point of this?
The original clk_hw was defined simply as:
struct clk_hw {
struct clk *clk;
};
It's only purpose in life was as a handle for navigation between the
opaque struct clk and the hardware-specific struct my_clk_hw. struct
clk_hw is defined in clk.h and everyone can see it. If we're suddenly
OK putting clk data in this structure then why bother with an opaque
struct clk at all?
I think Rob meant one time a step to make it opaque. But it'll make
clk core always changing, easier mess, and let clk driver confused.
What is the actual data you need to be static and accessible to the
platform code? A ptr to parent clk is the main thing I've seen for
static initialization. So make the parent ptr be struct clk_hw* and
allow the platforms to access.
To answer your question on what data we're trying to expose: platform
code commonly needs the parent pointer and the clk rate (and by
extension, the rate of the parent). For debug/error prints it is also
nice to have the clk name. Generic clk flags are also conceivably
something that platform code might want.
I'd like to spin the question around: if we're OK exposing some stuff
(in your example above, the parent pointer), then what clk data are
you trying to hide?
Regards,
Mike
Rob
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v4 3/6] clk: introduce the common clock framework
On 01/04/2012 07:01 PM, Turquette, Mike wrote:
On Wed, Jan 4, 2012 at 6:32 AM, Rob Herring robherri...@gmail.com wrote:
On 01/03/2012 08:15 PM, Richard Zhao wrote:
On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner t...@linutronix.de
wrote:
On Tue, 13 Dec 2011, Mike Turquette wrote:
snip
+/**
+ * clk_init - initialize the data structures in a struct clk
+ * @dev: device initializing this clk, placeholder for now
+ * @clk: clk being initialized
+ *
+ * Initializes the lists in struct clk, queries the hardware for the
+ * parent and rate and sets them both. Adds the clk to the sysfs tree
+ * topology.
+ *
+ * Caller must populate clk-name and clk-flags before calling
I'm not too happy about this construct. That leaves struct clk and its
members exposed to the world instead of making it a real opaque
cookie. I know from my own painful experience, that this will lead to
random fiddling in that data structure in drivers and arch code just
because the core code has a shortcoming.
Why can't we make struct clk a real cookie and confine the data
structure to the core code ?
That would change the init call to something like:
struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
struct clk *parent)
And have:
struct clk_hw {
struct clk_hw_ops *ops;
const char*name;
unsigned long flags;
};
Implementers can do:
struct my_clk_hw {
struct clk_hwhw;
mydata;
};
And then change the clk ops callbacks to take struct clk_hw * as an
argument.
We have to define static clocks before we adopt DT binding.
If clk is opaque and allocate memory in clk core, it'll make hard
to define static clocks. And register/init will pass a long parameter
list.
DT is not a prerequisite for having dynamically created clocks. You can
make clock init dynamic without DT.
Agreed.
What data goes in struct clk vs. struct clk_hw could change over time.
So perhaps we can start with some data in clk_hw and plan to move it to
struct clk later. Even if almost everything ends up in clk_hw initially,
at least the structure is in place to have common, core-only data
separate from platform data.
What is the point of this?
To have a way forward. It would be nice to have a clk infrastructure
before I retire...
The original clk_hw was defined simply as:
struct clk_hw {
struct clk *clk;
};
It's only purpose in life was as a handle for navigation between the
opaque struct clk and the hardware-specific struct my_clk_hw. struct
clk_hw is defined in clk.h and everyone can see it. If we're suddenly
OK putting clk data in this structure then why bother with an opaque
struct clk at all?
What is the actual data you need to be static and accessible to the
platform code? A ptr to parent clk is the main thing I've seen for
static initialization. So make the parent ptr be struct clk_hw* and
allow the platforms to access.
To answer your question on what data we're trying to expose: platform
code commonly needs the parent pointer and the clk rate (and by
extension, the rate of the parent). For debug/error prints it is also
nice to have the clk name. Generic clk flags are also conceivably
something that platform code might want.
I agree with the need to have the parent and flags from a static init
perspective. There's not really a good reason the others can't be
accessed thru accessors though.
I'd like to spin the question around: if we're OK exposing some stuff
(in your example above, the parent pointer), then what clk data are
you trying to hide?
Well, everything from drivers which the current clk implementations do
hide. Catching abuse in with drivers coming in from all different trees
and lists will be impossible.
For platform code it is more fuzzy. I don't think platform code should
be allowed to muck with prepare/enable counts for example.
Rob
Regards,
Mike
Rob
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v4 3/6] clk: introduce the common clock framework
On Wed, Jan 4, 2012 at 6:11 PM, Rob Herring robherri...@gmail.com wrote:
On 01/04/2012 07:01 PM, Turquette, Mike wrote:
On Wed, Jan 4, 2012 at 6:32 AM, Rob Herring robherri...@gmail.com wrote:
On 01/03/2012 08:15 PM, Richard Zhao wrote:
On Fri, Dec 16, 2011 at 04:45:48PM -0800, Turquette, Mike wrote:
On Wed, Dec 14, 2011 at 5:18 AM, Thomas Gleixner t...@linutronix.de
wrote:
On Tue, 13 Dec 2011, Mike Turquette wrote:
snip
+/**
+ * clk_init - initialize the data structures in a struct clk
+ * @dev: device initializing this clk, placeholder for now
+ * @clk: clk being initialized
+ *
+ * Initializes the lists in struct clk, queries the hardware for the
+ * parent and rate and sets them both. Adds the clk to the sysfs tree
+ * topology.
+ *
+ * Caller must populate clk-name and clk-flags before calling
I'm not too happy about this construct. That leaves struct clk and its
members exposed to the world instead of making it a real opaque
cookie. I know from my own painful experience, that this will lead to
random fiddling in that data structure in drivers and arch code just
because the core code has a shortcoming.
Why can't we make struct clk a real cookie and confine the data
structure to the core code ?
That would change the init call to something like:
struct clk *clk_init(struct device *dev, const struct clk_hw *hw,
struct clk *parent)
And have:
struct clk_hw {
struct clk_hw_ops *ops;
const char *name;
unsigned long flags;
};
Implementers can do:
struct my_clk_hw {
struct clk_hw hw;
mydata;
};
And then change the clk ops callbacks to take struct clk_hw * as an
argument.
We have to define static clocks before we adopt DT binding.
If clk is opaque and allocate memory in clk core, it'll make hard
to define static clocks. And register/init will pass a long parameter
list.
DT is not a prerequisite for having dynamically created clocks. You can
make clock init dynamic without DT.
Agreed.
What data goes in struct clk vs. struct clk_hw could change over time.
So perhaps we can start with some data in clk_hw and plan to move it to
struct clk later. Even if almost everything ends up in clk_hw initially,
at least the structure is in place to have common, core-only data
separate from platform data.
What is the point of this?
To have a way forward. It would be nice to have a clk infrastructure
before I retire...
Haha, agreed.
The original clk_hw was defined simply as:
struct clk_hw {
struct clk *clk;
};
It's only purpose in life was as a handle for navigation between the
opaque struct clk and the hardware-specific struct my_clk_hw. struct
clk_hw is defined in clk.h and everyone can see it. If we're suddenly
OK putting clk data in this structure then why bother with an opaque
struct clk at all?
What is the actual data you need to be static and accessible to the
platform code? A ptr to parent clk is the main thing I've seen for
static initialization. So make the parent ptr be struct clk_hw* and
allow the platforms to access.
To answer your question on what data we're trying to expose: platform
code commonly needs the parent pointer and the clk rate (and by
extension, the rate of the parent). For debug/error prints it is also
nice to have the clk name. Generic clk flags are also conceivably
something that platform code might want.
I agree with the need to have the parent and flags from a static init
perspective. There's not really a good reason the others can't be
accessed thru accessors though.
I'd like to spin the question around: if we're OK exposing some stuff
(in your example above, the parent pointer), then what clk data are
you trying to hide?
Well, everything from drivers which the current clk implementations do
hide. Catching abuse in with drivers coming in from all different trees
and lists will be impossible.
For platform code it is more fuzzy. I don't think platform code should
be allowed to muck with prepare/enable counts for example.
So there is a clear dichotomy: drivers shouldn't be exposed to any of
it and platform code should be exposed to some of it.
How about a drivers/clk/clk-private.h which will define struct clk and
must only be included by clk drivers in drivers/clk/*?
This establishes a bright line between those things which are allowed
to know the details of struct clk and those that are not: namely that
clk drivers in drivers/clk/ may use '#include clk-private.h'.
Obviously struct clk is opaque to the rest of the kernel (in the same
way it has been prior to the common clk patches) and there is no need
for struct clk_hw anymore. Also helper functions are no longer needed
for clock driver code, which I think *is* a manageable set of code to
review. Also clk drivers must live in drivers/clk/ for this to work
(without a big ugly path in someone's #include directive somewhere).
Thoughts?
Regards,
Mike
Rob
Re: [PATCH V4 0/5] ARM: exynos: Add l2 retention mode cpuidle state
On 3 January 2012 18:52, Kukjin Kim kgene@samsung.com wrote: amit kachhap wrote: Hi Mr kim, All the comments have been addressed for the Exynos cpu idle patchset. The updated patchset was posted about one month back and there have been no further comments on the patchset since then. As this patchset seems to be stable now, do you think these these patches can merged in this 3.3 merge window? Kindly let me know your opinion. This conflicts with local common.[ch] working for ARM restart. Please wait until after the merge window for this patches. Thanks for the update. Actually I have fixed the merging issue in common.[ch] files so sending the V5 series of the patchset shortly. If possible add in this merge window or next merge window is also fine. Regards, Amit daniel Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [U-Boot] [PATCH 4/6] EEYNOS: Add SMDK5250 board support
Dear Minkyu Kang,
On 27 December 2011 14:18, Chander Kashyap chander.kash...@linaro.org wrote:
Dear minkyu Kang
On 23 December 2011 11:21, Chander Kashyap chander.kash...@linaro.org wrote:
Dear Minkyu Kang,
On 23 December 2011 09:51, Minkyu Kang proms...@gmail.com wrote:
Dear Chander Kashyap,
On 22 December 2011 19:52, Chander Kashyap chander.kash...@linaro.org
wrote:
SMDK5250 board is based on Samsungs EXYNOS5250 SoC.
Signed-off-by: Chander Kashyap chander.kash...@linaro.org
---
MAINTAINERS | 1 +
arch/arm/cpu/armv7/exynos/clock.c | 24 +-
arch/arm/include/asm/arch-exynos/clock.h | 232 +
arch/arm/include/asm/arch-exynos/clock_exynos4.h | 255 +
arch/arm/include/asm/arch-exynos/clock_exynos5.h | 352 +
arch/arm/include/asm/arch-exynos/cpu.h | 71 ++--
arch/arm/include/asm/arch-exynos/cpu_exynos4.h | 51 ++
arch/arm/include/asm/arch-exynos/cpu_exynos5.h | 39 ++
arch/arm/include/asm/arch-exynos/gpio.h | 51 ++-
board/samsung/smdk5250/Makefile | 48 ++
board/samsung/smdk5250/lowlevel_init.S | 524
+++
board/samsung/smdk5250/mem_setup.S | 600
++
board/samsung/smdk5250/smdk5250.c | 125 +
board/samsung/smdk5250/smdk5250_setup.h | 583
+
boards.cfg | 1 +
include/configs/smdk5250.h | 182 +++
16 files changed, 2867 insertions(+), 272 deletions(-)
create mode 100644 arch/arm/include/asm/arch-exynos/clock_exynos4.h
create mode 100644 arch/arm/include/asm/arch-exynos/clock_exynos5.h
create mode 100644 arch/arm/include/asm/arch-exynos/cpu_exynos4.h
create mode 100644 arch/arm/include/asm/arch-exynos/cpu_exynos5.h
create mode 100644 board/samsung/smdk5250/Makefile
create mode 100644 board/samsung/smdk5250/lowlevel_init.S
create mode 100644 board/samsung/smdk5250/mem_setup.S
create mode 100644 board/samsung/smdk5250/smdk5250.c
create mode 100644 board/samsung/smdk5250/smdk5250_setup.h
create mode 100644 include/configs/smdk5250.h
Please split this patch for board and SoC.
Ok
diff --git a/MAINTAINERS b/MAINTAINERS
index a56ca10..abf88be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -704,6 +704,7 @@ Chander Kashyap k.chan...@samsung.com
origen ARM ARMV7 (EXYNOS4210 SoC)
SMDKV310 ARM ARMV7 (EXYNOS4210 SoC)
+ SMDK5250 ARM ARMV7 (EXYNOS5250 SoC)
Torsten Koschorrek koschor...@synertronixx.de
scb9328 ARM920T (i.MXL)
diff --git a/arch/arm/cpu/armv7/exynos/clock.c
b/arch/arm/cpu/armv7/exynos/clock.c
index b101f96..88e2fc0 100644
--- a/arch/arm/cpu/armv7/exynos/clock.c
+++ b/arch/arm/cpu/armv7/exynos/clock.c
@@ -125,10 +125,14 @@ static unsigned long exynos_get_pwm_clk(void)
if (s5p_get_cpu_rev() == 0) {
/*
- * CLK_SRC_PERIL0
+ * CLK_SRC_{PERIL0 | PERIC0}
* PWM_SEL [27:24]
*/
+#ifdef CONFIG_EXYNOS5
+ sel = readl(clk-src_peric0);
+#else
sel = readl(clk-src_peril0);
+#endif
NAK.
We don't allow to using ifdef for separating SoCs.
Please refer s5pc1xx case for solve it.
This comment apply to this patch globally.
Please remove '#ifdef CONFIG_EXYNOS5'.
I have tried to reuse the code. It is possible to remove
#ifdef CONFIG_EXYNOS5' in clock.c with cpu_is_s5pcXXX check.
Is it a acceptable solution? Or is it necessary to write SoC specific
function
in clock.c as done in case of s5pc1xx/clock.c.
Please Advice
Removing CONFIG_EXYNOS5 and following s5pc1xx case will not allow to
reuse the code in clock.c.
What is the technical hindrance of not using ifdefs?
What are yours comments regarding this issue?
Any comments regrading the above discussion ?
Thanks.
Minkyu Kang.
--
from. prom.
www.promsoft.net
--
with warm regards,
Chander Kashyap
--
with warm regards,
Chander Kashyap
--
with warm regards,
Chander Kashyap
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [U-Boot] [PATCH 4/6] EEYNOS: Add SMDK5250 board support
Dear Chander Kashyap,
On 27 December 2011 17:48, Chander Kashyap chander.kash...@linaro.org wrote:
Torsten Koschorrek koschor...@synertronixx.de
scb9328 ARM920T (i.MXL)
diff --git a/arch/arm/cpu/armv7/exynos/clock.c
b/arch/arm/cpu/armv7/exynos/clock.c
index b101f96..88e2fc0 100644
--- a/arch/arm/cpu/armv7/exynos/clock.c
+++ b/arch/arm/cpu/armv7/exynos/clock.c
@@ -125,10 +125,14 @@ static unsigned long exynos_get_pwm_clk(void)
if (s5p_get_cpu_rev() == 0) {
/*
- * CLK_SRC_PERIL0
+ * CLK_SRC_{PERIL0 | PERIC0}
* PWM_SEL [27:24]
*/
+#ifdef CONFIG_EXYNOS5
+ sel = readl(clk-src_peric0);
+#else
sel = readl(clk-src_peril0);
+#endif
NAK.
We don't allow to using ifdef for separating SoCs.
Please refer s5pc1xx case for solve it.
This comment apply to this patch globally.
Please remove '#ifdef CONFIG_EXYNOS5'.
I have tried to reuse the code. It is possible to remove
#ifdef CONFIG_EXYNOS5' in clock.c with cpu_is_s5pcXXX check.
Is it a acceptable solution? Or is it necessary to write SoC specific
function
in clock.c as done in case of s5pc1xx/clock.c.
Please Advice
Removing CONFIG_EXYNOS5 and following s5pc1xx case will not allow to
reuse the code in clock.c.
What is the technical hindrance of not using ifdefs?
No need to reuse the code, if SoCs are different.
If need, please separate the functions.
like this,
unsigned long get_arm_clk(void)
{
if (cpu_is_s5pc110())
return s5pc110_get_arm_clk();
else
return s5pc100_get_arm_clk();
}
Thanks.
Minkyu Kang
--
from. prom.
www.promsoft.net
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
