Re: [lm-sensors] [RESEND PATCH V1 0/9] thermal: introduce DT thermal zone build
On Fri, Jul 19, 2013 at 02:56:19PM -0400, Eduardo Valentin wrote: On 19-07-2013 14:45, Stephen Warren wrote: On 07/19/2013 07:38 AM, Eduardo Valentin wrote: On 18-07-2013 17:11, Guenter Roeck wrote: On Thu, Jul 18, 2013 at 09:53:05AM -0400, Eduardo Valentin wrote: Hello Guenter, On 17-07-2013 18:09, Guenter Roeck wrote: On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote: Hello all, As you noticed, I am working in a way to represent thermal data using device tree [1]. Essentially, this should be a way to say what to do with a sensor and how to associate (cooling) actions with it. Seems to me that goes way beyond the supposed scope of devicetree data. Devicetree data is supposed to describe hardware, not its configuration or use. This is clearly a use case. Thanks for rising your voice here. It is important to know what hwmon ppl think about this. Sorry, I don't know what ppl stands for. Guenter As your answers to the series are giving same argument, I chose to answer on patch 0. I would be happier if you could elaborate a bit more on your concern, specially if you take hwmon cap here, and give your view with that perspective. I also considered that this work could be abusing of DT purposes. But let me explain why I still think it makes sense to have it. Ultimately, you are making my point here. If you considered it, did you ask devicetree experts for an opinion ? Did you discuss the subject on the devicetree-discuss mailing list ? If so, what was the result ? Although I have asked, I didn't get any feedback. https://lkml.org/lkml/2013/4/11/760 But now I am requesting feedback in a formal (patch) way. Consider this patch series as official request for (devicetree experts and everyone involved) opinions. I might suggest (a) sending the email To the DT maintainer, rather than just CC'ing him, (b) perhaps start a new thread just to present the proposed DT binding, and get feedback on that. A thread with a new subject like [RFC] DT binding for thermal zones might get more attention than a patch submission; the subject line of this patch doesn't stand much (since it implies to me it's more about build issues than DT bindings even though it does mention DT). OK. I will do that. Sounds reasonable. Resending this series as RFC again, but now addressed to DT folks. It might help to not just send the series, but to start a thread to discuss the proposed bindings. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [lm-sensors] [RESEND PATCH V1 0/9] thermal: introduce DT thermal zone build
On Fri, Jul 19, 2013 at 12:48:39PM -0600, Stephen Warren wrote: On 07/18/2013 03:21 PM, Guenter Roeck wrote: On Thu, Jul 18, 2013 at 11:18:05AM -0600, Stephen Warren wrote: On 07/18/2013 07:53 AM, Eduardo Valentin wrote: Hello Guenter, On 17-07-2013 18:09, Guenter Roeck wrote: On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote: Hello all, As you noticed, I am working in a way to represent thermal data using device tree [1]. Essentially, this should be a way to say what to do with a sensor and how to associate (cooling) actions with it. Seems to me that goes way beyond the supposed scope of devicetree data. Devicetree data is supposed to describe hardware, not its configuration or use. This is clearly a use case. Thanks for rising your voice here. It is important to know what hwmon ppl think about this. I meant to find time to read Guenter's original email where he initially objected to putting data into DT, and determine exactly what was being objected to. I still haven't:-( However, the arguments that Eduardo stated in his email do make sense to me; I agree that temperature limits really are a description of HW. Details of which cooling methods to invoke when certain temperature limits are reached is also part of the HW/system design, and hence I would tend to agree that they're appropriate to include in DT. Anyway, that's just my 2 cents on the matter:-) Many systems have multiple profiles for various use cases (high performance, low power etc), and limits are different based on the use case. If that means you are going to have multiple devicetree variants based on the profile, I would argue that you crossed the line. Yes, I can see that argument. However, a counter-point: * I believe we do need a DT binding to describe the absolute thermal limits of a system, for safety/correctness of system operation. * We need to define a syntax/schema to represent that. * If we then want to implement additional profiles with stricter limits, do we really want to invent a different syntax/schema to represent those? Representing them in the same way seems like good use of the design, mind-share, etc. * Perhaps that doesn't mean that the additional profiles have to be in DT though; just that we somehow make any other representation of those profiles as close to the DT representation in syntax/structure as we can, to get maximum re-use. With thermal profiles it gets even more complicated, as those parameters may be played around with and changed multiple times to find the best settings to achieve optimal cooling. To me, that sounds more like fixing a bug in the initial data, rather than something which fundamentally affects how the data should be represented. For me, a bug in devicetree data would be if a wrong gpio pin is assigned ... if a wrong clock frequency is specified, or a wrong clock input. I would not expect a clock rate or pin assignment to be played around with to find the optimal setting. Either it works or it doesn't. Note that we don't really talk about pure thermal limits here. We are talking about association between thermal sensors and cooling devices, which is much more abstract and not as well defined as clock pin assignments. In many case, more than one cooling device exists, and there is no well defined means to cool the system. It may be cooled with a fan, or by reducing the CPU clock speed. There may be multiple fans interacting with each other, and not all may be present at all times. There may be secondary cooling options if primary cooling fails (eg reduce CPU speed or increase speed of a secondary fan if a primary fan fails). Yes, I understand you need a means to describe thermal profiles. Putting one of those into DT and declaring it to be the absolute thermal limit, however, is really just asking for putting other thermal profiles into DT as well .. if you want a different thermal profile, just load a different devicetree, and possibly make it configurable in BIOS/ROMMON. Of course you can declare that not to be intentional, but what do you think would happen in the real world ? Do you really think anyone would bother putting one profile into DT and others into some other database ? Not really, especially if the kernel would not even support reading that other database. And if it would, you would not need the DT data in the first place. This is completely different to the intended use of devicetree information. An instance of devicetree data should be tied to a specific piece of hardware. Changing it may enable or disable pieces of that hardware, but change its operation, as in optimize for performance or optimize for low power consumption ? Yes, again, I understand you are saying that this is not the _intent_, but that is what it enables. Other but related subject .. from a thermal / hwmon driver's perspective, if such a driver supports thermal subsystem
Re: [RESEND PATCH V1 8/9] hwmon: lm75: expose to thermal fw via DT nodes
On Wed, Jul 17, 2013 at 11:17:27AM -0400, Eduardo Valentin wrote:
This patch adds to lm75 temperature sensor the possibility
to expose itself as thermal zone device, registered on the
thermal framework.
The thermal zone is built only if a device tree node
describing a thermal zone for this sensor is present
inside the lm75 DT node. Otherwise, the driver behavior
will be the same.
As mentioned before, your usage of devicetree information is clearly
outside the scope for devicetree data.
One clear guideline for devicetree data is that it is supposed to be usable
across multiple operating systems. Your proposal is clearly operating system
and even subsystem specific. It does _not_ describe hardware.
NACK.
Guenter
Cc: Jean Delvare kh...@linux-fr.org
Cc: Guenter Roeck li...@roeck-us.net
Cc: lm-sens...@lm-sensors.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com
---
drivers/hwmon/lm75.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index c03b490..0aa5e28 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -27,6 +27,8 @@
#include linux/hwmon-sysfs.h
#include linux/err.h
#include linux/mutex.h
+#include linux/thermal.h
+#include linux/of.h
#include lm75.h
@@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
/* Each client has this additional data */
struct lm75_data {
struct device *hwmon_dev;
+ struct thermal_zone_device *tz;
struct mutexupdate_lock;
u8 orig_conf;
u8 resolution; /* In bits, between 9 and 12 */
@@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device
*dev);
/* sysfs attributes for hwmon */
+static int lm75_read_temp(void *dev, unsigned long *temp)
+{
+ struct lm75_data *data = lm75_update_device(dev);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ *temp = ((data-temp[0] (16 - data-resolution)) * 1000)
+ (data-resolution - 8);
+
+ return 0;
+}
+
static ssize_t show_temp(struct device *dev, struct device_attribute *da,
char *buf)
{
@@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct
i2c_device_id *id)
goto exit_remove;
}
+ if (of_find_node_by_name(client-dev.of_node, thermal_zone)) {
+ data-tz = thermal_zone_of_device_register(client-dev,
+client-dev,
+lm75_read_temp);
+ if (IS_ERR(data-tz)) {
+ status = PTR_ERR(data-tz);
+ goto exit_hwmon;
+ }
+ }
+
dev_info(client-dev, %s: sensor '%s'\n,
dev_name(data-hwmon_dev), client-name);
return 0;
+exit_hwmon:
+ hwmon_device_unregister(data-hwmon_dev);
exit_remove:
sysfs_remove_group(client-dev.kobj, lm75_group);
return status;
@@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
{
struct lm75_data *data = i2c_get_clientdata(client);
+ thermal_zone_device_unregister(data-tz);
hwmon_device_unregister(data-hwmon_dev);
sysfs_remove_group(client-dev.kobj, lm75_group);
lm75_write_value(client, LM75_REG_CONF, data-orig_conf);
--
1.8.2.1.342.gfa7285d
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RESEND PATCH V1 9/9] hwmon: tmp102: expose to thermal fw via DT nodes
On Wed, Jul 17, 2013 at 11:17:28AM -0400, Eduardo Valentin wrote: This patch adds to tmp102 temperature sensor the possibility to expose itself as thermal zone device, registered on the thermal framework. The thermal zone is built only if a device tree node describing a thermal zone for this sensor is present inside the tmp102 DT node. Otherwise, the driver behavior will be the same. NACK, same reason as for the lm75 patch. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [lm-sensors] [RESEND PATCH V1 0/9] thermal: introduce DT thermal zone build
On Thu, Jul 18, 2013 at 09:53:05AM -0400, Eduardo Valentin wrote: Hello Guenter, On 17-07-2013 18:09, Guenter Roeck wrote: On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote: Hello all, As you noticed, I am working in a way to represent thermal data using device tree [1]. Essentially, this should be a way to say what to do with a sensor and how to associate (cooling) actions with it. Seems to me that goes way beyond the supposed scope of devicetree data. Devicetree data is supposed to describe hardware, not its configuration or use. This is clearly a use case. Thanks for rising your voice here. It is important to know what hwmon ppl think about this. Sorry, I don't know what ppl stands for. Guenter As your answers to the series are giving same argument, I chose to answer on patch 0. I would be happier if you could elaborate a bit more on your concern, specially if you take hwmon cap here, and give your view with that perspective. I also considered that this work could be abusing of DT purposes. But let me explain why I still think it makes sense to have it. Ultimately, you are making my point here. If you considered it, did you ask devicetree experts for an opinion ? Did you discuss the subject on the devicetree-discuss mailing list ? If so, what was the result ? First thing is that this series intend to describe the thermal data required for a specific board. That means, considering your board layout, mechanics, power dissipation and composition of your ICs, etc, that will impose thermal requirements on your system. That is not configuration, but part of your board design and non-functional requirement. To me, configuration would be something like saying you want to use a specific keyboard layout, or defining your wifi card channel, or display size, etc. Here what is described and setup may get confused with configuration, but it is not because what goes in DT in this case must be actually derived from your HW needs. Putting a sensor close to your battery has a strong meaning behind. Same if you put a sensor close to your processor. For instance, we have cases we need to consider external heat in order to properly determine our CPU hotspot level, using a board sensor. That is what I mean by HW requirement/need. Again, just to refresh our minds, this is about protecting your board and ICs from operating out of their spec and extending their lifetime. This is not about configuring something just because user has chosen to. That is definitely not a configuration. Being a use case, well, these new DT nodes can still be seen as a use case, yes. But depends on your understanding of use case. Because a sensor device may be used on different needs, composing different use cases. But still here we are talking about HW needs and composition. And yes, if you take that perspective, there are use cases already described in DT. For instance, just because you use an LDO to power a MMC, does it mean you always will use it to power MMC on all boards. Would that be a use case? And in that example, because your MMC requires 2.8V, if you have a DT property to say that, does it mean it is configuration? Well, yes, but that is based on HW needs, otherwise it wont operate properly. Same thing here, leave your hw operating out of temperature specs and you will see what is the outcome. Similar example would be cpufreq, or OPPs for opp layer. Defining an OPP in DT could be considered a configuration? Well in theory yes, one can pick what ever configuration for your (D)PLL then match with a minimalist voltage level. And in theory, a voltage level can sustain more than one frequency level. An OPP is still a virtual concept, and we still describe it in DT. Why? To me is because it is a HW need, because HW folks have actually validated that configuration of PLL (frequency) and voltage level. Sometimes they have even optimized it (for low power consumption for instance), as one may achieve same OPP with different configuration. Then why thermal data, which is again, a 'HW configuration' that has been optimized by HW folks, known to be a HW requirement, cannot be described in DT? Similar argument goes to the fact that one may think this is subsystem specific. Again, describing your OPPs is not OPP layer specific? Besides, one can still read the device tree nodes I have written for describing thermal data and implement the HW requirement elsewhere, if wanted (say in user land). I don't see as subsystem specific. Keep in mind that these very same concepts are actually derived from ACPI specification. They don't come from nowhere. And just because your system does not have ACPI support, does not mean it won't have a need to describe thermal? So, because with this work (i) we are describing something that is required for properly usage of your HW (and not choice of the user), because (ii) same data
Re: [lm-sensors] [RESEND PATCH V1 0/9] thermal: introduce DT thermal zone build
On Thu, Jul 18, 2013 at 11:18:05AM -0600, Stephen Warren wrote: On 07/18/2013 07:53 AM, Eduardo Valentin wrote: Hello Guenter, On 17-07-2013 18:09, Guenter Roeck wrote: On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote: Hello all, As you noticed, I am working in a way to represent thermal data using device tree [1]. Essentially, this should be a way to say what to do with a sensor and how to associate (cooling) actions with it. Seems to me that goes way beyond the supposed scope of devicetree data. Devicetree data is supposed to describe hardware, not its configuration or use. This is clearly a use case. Thanks for rising your voice here. It is important to know what hwmon ppl think about this. I meant to find time to read Guenter's original email where he initially objected to putting data into DT, and determine exactly what was being objected to. I still haven't:-( However, the arguments that Eduardo stated in his email do make sense to me; I agree that temperature limits really are a description of HW. Details of which cooling methods to invoke when certain temperature limits are reached is also part of the HW/system design, and hence I would tend to agree that they're appropriate to include in DT. Anyway, that's just my 2 cents on the matter:-) Many systems have multiple profiles for various use cases (high performance, low power etc), and limits are different based on the use case. If that means you are going to have multiple devicetree variants based on the profile, I would argue that you crossed the line. With thermal profiles it gets even more complicated, as those parameters may be played around with and changed multiple times to find the best settings to achieve optimal cooling. Does this describe hardware ? I don't think so, but, as I mentioned before, maybe I am wrong. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [lm-sensors] [RESEND PATCH V1 0/9] thermal: introduce DT thermal zone build
On Wed, Jul 17, 2013 at 11:17:19AM -0400, Eduardo Valentin wrote: Hello all, As you noticed, I am working in a way to represent thermal data using device tree [1]. Essentially, this should be a way to say what to do with a sensor and how to associate (cooling) actions with it. Seems to me that goes way beyond the supposed scope of devicetree data. Devicetree data is supposed to describe hardware, not its configuration or use. This is clearly a use case. Guenter The motivation to create such infrastructure is: (i) - to reuse the existing temperature sensor code base; (ii) - have a way to easily describe thermal data across different boards for the same sensor. Say you have an i2c temp sensor, which is placed close to your battery on board A but on board B, another instance of this same senor is placed close to your display, for instance. This series introduces then a DT parser. The data expected in DT must contain the needed properties to build a thermal zone out of the desired sensor. All properties are documented and they are derived from the existing requirements of current thermal API. In order to perform a binding with cooling devices, the new thermal zone built using DT nodes will use the existing thermal API that uses binding parameters. This is the current proposed way to register thermal zones with platform information, written by Durga. There are some virtual concepts that are pushed to device tree, I know. But I believe using device tree to do this makes sense because we are still describing the HW and how they are related to each other. Things like cooling devices are not represented in device tree though, as I believe that will cause a lot of confusion with real devices (as already does). So the series is short but I think it makes sense to describe how it is organized, as it touches several places. First four patches are a preparation for this parser. There is a change on cpufreq-cpu0, so that it knows now how to load its corresponding cooling device. There is a change on thermal_core to split its hwmon code, because I think we may need to improve this code base when we start to integrate better with hwmon temperature sensors. Then, another needed preparation is to improve the bind_params, so that we are able to bind with upper and lower limits. The remaining patches are the changes with the proposed DT parser. A part from the DT thermal zone builder itself (patch 05), I also changed the ti-soc-thermal driver to use this new API and the omap4430 bandgap DT node, as an example (this has been tested on panda omap4430); and also changed the hwmon drivers lm75 and tmp102 to have the option to build a thermal zone using DT. I haven't touched any dts file using lm75 or tmp102 because this should come on a need basis. I believe this code is pretty usable the way it is, but might need to be revisited, in case the enhancement proposed by Durga get in. This is because representing virtual thermal zones built out of several sensors may need a different representation in DT. [1] - RFC of this work: http://comments.gmane.org/gmane.linux.power-management.general/35874 Changes from RFC: - Added a way to load cpufreq cooling device out of DT - Added a way to bind with upper and lower limits using bind_params - Added a way to specify upper and lower binding limits on DT - Added DT thermal builder support to lm75 and tmp102 hwmon drivers - Changed ERANGE to EDOM - Added thermal constants for zone type and bind limit, so that dts file could be more readable Tested on panda omap4430 (3.11-rc1 with minor changes for getting cpufreq working) BR, Eduardo Valentin (9): cpufreq: cpufreq-cpu0: add dt node parsing for 'needs-cooling' thermal: hwmon: move hwmon support to single file thermal: thermal_core: allow binding with limits on bind_params arm: dts: flag omap4430 with needs-cooling for cpu node thermal: introduce device tree parser thermal: ti-soc-thermal: use thermal DT infrastructure arm: dts: add omap4430 thermal data hwmon: lm75: expose to thermal fw via DT nodes hwmon: tmp102: expose to thermal fw via DT nodes .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt | 3 + .../devicetree/bindings/thermal/thermal.txt| 133 ++ Documentation/thermal/sysfs-api.txt| 7 + arch/arm/boot/dts/omap443x.dtsi| 32 +- drivers/cpufreq/cpufreq-cpu0.c | 8 + drivers/hwmon/lm75.c | 29 ++ drivers/hwmon/tmp102.c | 25 ++ drivers/thermal/Kconfig| 22 + drivers/thermal/Makefile | 4 + drivers/thermal/thermal_core.c | 274 +--- drivers/thermal/thermal_dt.c | 458 + drivers/thermal/thermal_hwmon.c| 269
Re: [Ksummit-2013-discuss] [ATTEND] Handling of devicetree bindings
On Mon, Jul 15, 2013 at 08:29:15AM -0600, Jonathan Corbet wrote: On Mon, 15 Jul 2013 10:36:22 +0200 Linus Walleij linus.wall...@linaro.org wrote: Devicetree is supposed to describe the hardware, but in many cases there is an overlap between hardware description and configuration and/or use. Where is the boundary ? Well this is what we need everyone to know so I guess this confirms the need to bring it up at the kernel summit... Do we need a kernel summit discussion, or do we just need a good document? Or, to phrase the question another way, are we lacking a consensus among the clueful regarding how device tree bindings should be designed, or are we simply lacking education? If it's the latter, I don't think a kernel summit session will help much. For my part I lack education, and KS session won't help me as I won't be there, unless the results are written down in a way that lets me get that education. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [Ksummit-2013-discuss] [ATTEND] Handling of devicetree bindings
On Sun, Jul 14, 2013 at 01:46:45AM +0200, Linus Walleij wrote: On Sat, Jul 13, 2013 at 10:49 PM, Guenter Roeck li...@roeck-us.net wrote: On Sat, Jul 13, 2013 at 08:26:47PM +0100, Wolfram Sang wrote: I think the KS would be a good opportunity to present the status quo, show some rules of thumb and finally discuss if we can improve the process even further. E.g., should first the bindings be accepted, then the driver? What could be the process if the need for a generic binding arises? And spreading the word, so at least the basic issues are understood by most maintainers. Sounds like a good idea, but I think you'll need some deadline. When reviewing hwmon drivers I usually wait for a couple of weeks if there is any feedback from devicetree-discuss before I accept bindings, but what should I do if there is no feedback ? Holding the driver hostage doesn't seem like a good idea either. Nobody rarely say anything on devicetree-discuss. And if something is said it will usually be about syntax rather than semantics. And if it's semantics, it's usually a subsystem or arch maintainer saying it. I've been ranting a bit about this recently, and one of the problems with device tree now being driven by the Linux community (basically - it used to be a IEEE thing at one point in the past) is that as the bindings are merged by the subsystem maintainers, they alone get to decide when they are finished. They are all in Documentation/devicetree/bindings/* so theoretically we could have a dedicated maintainer for it, but that requires someone to step up :-/ I wonder if some subsystem maintainers who are more used to things like ACPI or PCI where someone else has already done the thinking and written a large (non-perfect, mind you) specification even think that reviewing hardware descriptions is their problem at all? Maybe some just consider this some documentation, think it has been defined by someone who thought it over and merge it without looking closely. For my part I do have a close look, but I don't really know if my understanding of acceptable properties matches the understanding of the devicetree community. Devicetree is supposed to describe the hardware, but in many cases there is an overlap between hardware description and configuration and/or use. Where is the boundary ? My approach is to accept properties which would otherwise require platform data. Is that acceptable ? How can I know if devicetree-discuss is mostly silent on such matters, as you point out ? Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature
On Fri, Jun 28, 2013 at 05:24:43PM +0200, Lars-Peter Clausen wrote: On 06/28/2013 04:50 PM, Alexandre Belloni wrote: On 28/06/2013 16:18, Lars-Peter Clausen wrote: On 06/27/2013 09:26 PM, Alexandre Belloni wrote: They are already registered as IIO_TEMP but only implement read_raw. Also, iio_hwmon_read_val() is using iio_read_channel_processed() and that will basically only read one of the 2 channels. As I documented, you actually need to read both channel 8 and channel 9 and then compute the value in Kelvins. I'm not sure how you want me to do that in the current framework. What are these two channels actually measuring? Is the value of a single channel meaningful on it's own? If not it might make sense to update the IIO driver to just have one temperature channel. It's not actually meaningful on its own. So, what you would do is expose one iio channel for two ADC channels and do the computation in read_raw ? or read_processed ? Then using iio-hwon to export it. ? Regards, Yes, return channel9 - channel8 as the raw value for the temperature channel and provide proper scale and offset values, so that iio_read_channel_processed() will return the correct value. Agreed. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature
On Thu, Jun 27, 2013 at 11:17:32AM +0200, Maxime Ripard wrote: On Wed, Jun 26, 2013 at 07:39:27AM -0700, Guenter Roeck wrote: On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote: The low resolution ADC of the mxs is able to read an internal temperature sensor, expose that using hwmon. Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com --- Wouldn't it make more sense to use iio-hwmon and improve it if necessary ? Actually, I wonder if we should not just put the hwmon driver capabilities directly into the mxs-lradc driver, just like it's already been done in this driver for the touchscreen support. The probing of this hwmon driver doesn't really belong to the DT, it's not really realistic to probe it from the machine definition, and it really is the IP that is wired that way. Merging iio-hwmon functionality into an adc driver seems just as bad (or even worse) as copying it into a new driver. If the lradc driver knows that the ADC channels are temperature sensors, it should register them with the iio subsystem as IIO_TEMP type. Then you should be able to use iio_hwmon as is. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/4] hwmon: Add a simple driver to read the MXS SoC temperature
On Wed, Jun 26, 2013 at 10:51:12AM +0200, Alexandre Belloni wrote:
The low resolution ADC of the mxs is able to read an internal temperature
sensor, expose that using hwmon.
Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com
---
Wouldn't it make more sense to use iio-hwmon and improve it if necessary ?
Guenter
.../devicetree/bindings/hwmon/mxs-cputemp.txt | 18 +++
Documentation/hwmon/mxs-cputemp| 29 +
drivers/hwmon/Kconfig | 10 ++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/mxs-cputemp.c| 132
+
5 files changed, 190 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
create mode 100644 Documentation/hwmon/mxs-cputemp
create mode 100644 drivers/hwmon/mxs-cputemp.c
diff --git a/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
b/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
new file mode 100644
index 000..7d3ae47
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/mxs-cputemp.txt
@@ -0,0 +1,18 @@
+mxs cputemp hwmon sensors
+-
+
+See: Documentation/hwmon/mxs-cputemp
+
+Required properties:
+- compatible: should be fsl,mxs-internal-temp
+- io-channels: should list the two adc channels needed to calculate the
+temperature
+- io-channel-names: should map the previously listed adc channels to the
min
+ and max value
+
+Example:
+ temp {
+ compatible = fsl,mxs-internal-temp;
+ io-channels = lradc 8, lradc 9;
+ io-channel-names = min, max;
+ };
diff --git a/Documentation/hwmon/mxs-cputemp b/Documentation/hwmon/mxs-cputemp
new file mode 100644
index 000..6c6201f
--- /dev/null
+++ b/Documentation/hwmon/mxs-cputemp
@@ -0,0 +1,29 @@
+Kernel driver mxs-cputemp
+=
+
+Supported chips:
+ * Freescale i.mx28
+Datasheet: i.MX28 Applications Processor Reference Manual, Rev. 1, 2010
+http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
+
+Author: Alexandre Belloni
+
+Description
+---
+This driver permits reading the internal die temperature sensor embedded
inside
+Freescale i.mx28 SoCs. This sensor is read through two channels of the on
chip
+Low-Resolution ADC. After calculation, the three-sigma error of the
temperature
+sensor should be within ± 1.5% in degrees Kelvin. Additionally, the
temperature
+sampling has a three-sigma sample-to-sample variation of 2 degrees Kelvin. If
+desired, this error can be removed by oversampling and averaging the
temperature
+result.
+
+The formula is:
+ (Channel9 – Channel8) * Gain_correction/4
+
+As recommended by the datasheet, Gain_correction is equal to 1.012.
+
+sysfs entries
+-
+temp1_input Measured and corrected temperature in millidegrees Celsius
+
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0428e8a..2daf794 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -929,6 +929,16 @@ config SENSORS_MCP3021
This driver can also be built as a module. If so, the module
will be called mcp3021.
+config SENSORS_MXS_CPU
+ tristate MXS internal CPU temperature sensor
+ depends on MXS_LRADC
+ help
+ If you say yes here you get support for the i.mx28 internal
+ temperature sensor.
+
+ This driver can also be built as a module. If so, the module
+ will be called mxs-cputemp
+
config SENSORS_NCT6775
tristate Nuvoton NCT6775F and compatibles
depends on !PPC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d17d3e6..366c92d 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
obj-$(CONFIG_SENSORS_MAX6697)+= max6697.o
obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
obj-$(CONFIG_SENSORS_MCP3021)+= mcp3021.o
+obj-$(CONFIG_SENSORS_MXS_CPU)+= mxs-cputemp.o
obj-$(CONFIG_SENSORS_NCT6775)+= nct6775.o
obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
obj-$(CONFIG_SENSORS_PC87360)+= pc87360.o
diff --git a/drivers/hwmon/mxs-cputemp.c b/drivers/hwmon/mxs-cputemp.c
new file mode 100644
index 000..a312fb5
--- /dev/null
+++ b/drivers/hwmon/mxs-cputemp.c
@@ -0,0 +1,132 @@
+/*
+ * Driver for the mxs internal temperature sensor
+ *
+ * Copyright 2013 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#define DRVNAME mxs-hwmon
+
+#include linux/device.h
+#include linux/err.h
+#include linux/hwmon.h
+#include linux/hwmon-sysfs.h
+#include linux/module.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/platform_device.h
+#include linux/iio/consumer.h
+
+#define GAIN_CORRECTION 1012
+
+/*
Re: [PATCHv8 1/1] Add support for GMT G762/G763 PWM fan controllers
On Sun, Jun 23, 2013 at 05:39:32PM +0200, Simon Guinot wrote: On Thu, Jun 20, 2013 at 10:21:04PM +0200, Arnaud Ebalard wrote: GMT G762/763 fan speed PWM controller is connected directly to a fan and performs closed-loop or open-loop control of the fan speed. Two modes - PWM or DC - are supported by the chip. Introduced driver provides various knobs to control the operations of the chip (via sysfs interface). Specific characteristics of the system can be passed either using board init code or via DT. Documentation for both the driver and DT bindings are also provided. Signed-off-by: Arnaud Ebalard a...@natisbad.org --- Hi Guenter, I guess we can wait for Simon's tests against its 5Big Network to check everything is ok on another platform. Note that I also tested the patch on my (Armada 370 based) ReadyNAS 102 and it works as expected. To be very accurate, I had to revert eda6bee6c7 to get both an out of tree driver for ISL 12057 chip and the g762 work *on the 102*, but this is an unrelated story (Debian bug #622325 [1] has more on the topic) for which I will create a separate thread. Simon, the symptom you reported for your read failures are different from those I get so I don't think the revert will fix your problem but it may be worth trying it. [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=622325 Hi Arnaud and Guenter, Actually, reverting the commit eda6bee6c7 seems to solve the -ENODEV issue... Did you let the i2c maintainers know ? Then, I have been able to test successfully the g762 driver against the following boards: - 2Big NAS (open-loop mode) - 2Big Network v2 (open-loop mode) - 5Big Network v2 (closed-loop mode) For the last, I have also been able to configure the clock frequency through the platform_data structure. Tested-by: Simon Guinot simon.gui...@sequanux.org Thanks a lot! Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv8 1/1] Add support for GMT G762/G763 PWM fan controllers
On Thu, Jun 20, 2013 at 10:21:04PM +0200, Arnaud Ebalard wrote:
GMT G762/763 fan speed PWM controller is connected directly to a fan
and performs closed-loop or open-loop control of the fan speed. Two
modes - PWM or DC - are supported by the chip. Introduced driver
provides various knobs to control the operations of the chip (via
sysfs interface). Specific characteristics of the system can be passed
either using board init code or via DT. Documentation for both the
driver and DT bindings are also provided.
Signed-off-by: Arnaud Ebalard a...@natisbad.org
Looks good to me.
I'll wait for additional feedback for a couple of days, then apply to -next.
Thanks,
Guenter
---
Hi Guenter,
I guess we can wait for Simon's tests against its 5Big Network to check
everything is ok on another platform. Note that I also tested the patch
on my (Armada 370 based) ReadyNAS 102 and it works as expected.
To be very accurate, I had to revert eda6bee6c7 to get both an out of
tree driver for ISL 12057 chip and the g762 work *on the 102*, but this
is an unrelated story (Debian bug #622325 [1] has more on the topic) for
which I will create a separate thread. Simon, the symptom you reported
for your read failures are different from those I get so I don't think
the revert will fix your problem but it may be worth trying it.
[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=622325
Cheers,
a+
Changes since v7:
Fixed bad test for NULL condition in g762_of_clock_disable()
Removed useless mutex protection in g762_fan_init()
Removed some useless return variables
Changes since v6:
s/data-clk/data-clk_freq/
add missing calls to clk_put() where necessary
cache clk in struct g762_data to avoid unecessary call to of_clk_get()
Changes since v5:
fixed useless ret parameter init in various functions
removed useless goto in favour of direct return
enable fan detection and fan ooc protection in an init function
fixed patch version (previous v5 was mistakenly named v4)
correctly balance clk_prepare_enable() by clk_disable_unprepare()
more tests w/o CONFIG_OF and with driver as module (load/unload)
Changes since v4:
Removed unused defines
fixed some comments
s/pwm_freq/clk_freq/g and fixed associated comments
removed useless settings of data-valid to false
do not hide original return code in g762_one_prop_commit()
replaced ref_clk property by a required reference to a clock in .dts
drop fan_pulses property in DT and provide sysfs attribute
removed whole G762_VAL_TEST_BIT hack
still not supporting 0 for pwm_enable: fixed comment
fixed comment for get/set_pwm()
fixed reversed polarity setter logic
simplified do_set_pwm()
merged all three patches into one
Fixed issue reported by Simon (open-loop not working when set_cnt is 255)
Changes since v3:
set is against current head of Linus tree
removed dev_err() calls when i2c_smbus_read/write_byte_data() fails
pwm1 sets SET_OUT reg, fan1_target sets SET_CNT reg, both unconditionally
removed all DT and platform_data knobs available via sysfs
updated documentation files to reflect two previous changes
Changes since v2:
set ref_clk value to 32768 if not overloaded
fixed multi-line comment format in g762.h
removed static const G762_DEFAULT_PDATA in g762.h for a function
CodingStyle: add spaces between operatoirs when missing
check return value of i2c_smbus_{read,write}_byte_data()
remove { } is not needed in single-statement conditionals
introduced G762_ATTR_VAL() to allow sparse init of platform_data struc
changed missed reference to linear mode for DC mode
Changes since v1
Changed author
removed bad tabs
Provide datasheet link w/o fud in g762 documentation
removed documentation for removed fan_gear_mode sysfs entry
removed tested-by from patch
removed FSF address in header file
removed useless include of linux/slab.h
removed useless parenthesis against HZ in define
use spaces around binary operators
use i2c_smbus_{read,write}_byte_data() instead of
g762_{read,write}_value()
use return value of i2c_smbus_write_byte_data()
use true for initializing boolean
removed useless blank lines
do not enforce single return point rule where less readable
use dev_err() and dev_dbg() instead of dev_info() when it makes sense
remove leading '' for function passed as pointers
allow passing parameter via platform_data struct for non-DT enabled boards
set data-valid to false when config is modified
s/linear/DC/ for mode (g762 datasheet uses linear)
more tests on rpm_from_cnt() and cnt_from_rpm() formula
dont overload
Changes since v0
removed forward declaration
use bool for valid field instead of bit field.
protect macro args
fixed typo in subject line
Re: [PATCH v2] hwmon: (ina2xx) Add device tree support to pass the shunt resistor
On Wed, Jun 19, 2013 at 02:50:20PM +0800, yuantian.t...@freescale.com wrote: From: Tang Yuantian yuantian.t...@freescale.com Adding another way that is device tree to pass the shunt resistor value to driver except for platform data. Signed-off-by: Tang Yuantian yuantian.t...@freescale.com I noticed that of.h include was missing. No need to re-send; I added it myself. Applied to -next. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RESEND,PATCHv6 1/1] Add support for GMT G762/G763 PWM fan controllers
On Wed, Jun 19, 2013 at 10:15:56PM +0200, Arnaud Ebalard wrote: Hi Simon, Simon Guinot simon.gui...@sequanux.org writes: I have tested this patch on my 2Big Network v2 board. Sometimes I get a weird No such device error while reading or writing the hwmon sysfs attributes. For example: # cat pwm1 cat: read error: No such device # cat pwm1 0 This is odd, since ENODEV should not be returned as error code from i2c read operations. What is the i2c controller ? Thanks, Guenter Weird, I cannot reproduce that at all on my ReadyNAS Duo v2. Note, that if the same command is executed again (just after the faulty one), then it will succeed most of the time. The errors seems to happen quite randomly and not very often. I'd say one time over ten tries. I suppose this error is now visible due to the enhanced error path in g762_update_client. It looks like we learn something by checking the return value for i2c_smbus_read_byte_data :) Except that, all is working as expected. This new version fixes effectively open-loop driving when the set_cnt register is set to 0xff. Also I am planning (hopefully this Wednesday) to test your patch on my 5Big Network v2 board. This last also embed a g762 controller but with a three wires fan instead. Then, I should be able to test the closed-loop mode behaviour. Thanks for your tests, Simon. Cheers, a+ ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RESEND,PATCHv6 1/1] Add support for GMT G762/G763 PWM fan controllers
On Wed, Jun 19, 2013 at 11:26:33PM +0200, Simon Guinot wrote: On Wed, Jun 19, 2013 at 01:53:21PM -0700, Guenter Roeck wrote: On Wed, Jun 19, 2013 at 10:15:56PM +0200, Arnaud Ebalard wrote: Hi Simon, Simon Guinot simon.gui...@sequanux.org writes: I have tested this patch on my 2Big Network v2 board. Sometimes I get a weird No such device error while reading or writing the hwmon sysfs attributes. For example: # cat pwm1 cat: read error: No such device # cat pwm1 0 This is odd, since ENODEV should not be returned as error code from i2c read operations. What is the i2c controller ? The controller is the very same as for the ReadyNAS Duo v2. It is built in the Marvell Kirkwood SoC and the driver is i2c-mv64xxx. I will try an another board to see if I reproduce the error. Might be a flaky i2c connection or a problematic board. The i2c-mv64xxx driver does return ENODEV in error cases. I submitted a patch to change it to ENXIO; we'll see if it gets accepted. Guenter Regards, Simon Thanks, Guenter Weird, I cannot reproduce that at all on my ReadyNAS Duo v2. Note, that if the same command is executed again (just after the faulty one), then it will succeed most of the time. The errors seems to happen quite randomly and not very often. I'd say one time over ten tries. I suppose this error is now visible due to the enhanced error path in g762_update_client. It looks like we learn something by checking the return value for i2c_smbus_read_byte_data :) Except that, all is working as expected. This new version fixes effectively open-loop driving when the set_cnt register is set to 0xff. Also I am planning (hopefully this Wednesday) to test your patch on my 5Big Network v2 board. This last also embed a g762 controller but with a three wires fan instead. Then, I should be able to test the closed-loop mode behaviour. Thanks for your tests, Simon. Cheers, a+ ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv7 1/1] Add support for GMT G762/G763 PWM fan controllers
On Wed, Jun 19, 2013 at 09:34:34PM +0200, Arnaud Ebalard wrote:
GMT G762/763 fan speed PWM controller is connected directly to a fan
and performs closed-loop or open-loop control of the fan speed. Two
modes - PWM or DC - are supported by the chip. Introduced driver
provides various knobs to control the operations of the chip (via
sysfs interface). Specific characteristics of the system can be passed
either using board init code or via DT. Documentation for both the
driver and DT bindings are also provided.
Signed-off-by: Arnaud Ebalard a...@natisbad.org
Couple of additional nitpicks plus a bug.
Guenter
---
Changes since v6:
s/data-clk/data-clk_freq/
cache clk in struct g762_data to avoid unecessary call to of_clk_get()
add missing calls to clk_put() where necessary
fixed some comments
Changes since v5:
fixed useless ret parameter init in various functions
removed useless goto in favour of direct return
enable fan detection and fan ooc protection in an init function
fixed patch version (previous v5 was mistakenly named v4)
correctly balance clk_prepare_enable() by clk_disable_unprepare()
more tests w/o CONFIG_OF and with driver as module (load/unload)
Changes since v4:
Removed unused defines
fixed some comments
s/pwm_freq/clk_freq/g and fixed associated comments
removed useless settings of data-valid to false
do not hide original return code in g762_one_prop_commit()
replaced ref_clk property by a required reference to a clock in .dts
drop fan_pulses property in DT and provide sysfs attribute
removed whole G762_VAL_TEST_BIT hack
still not supporting 0 for pwm_enable: fixed comment
fixed comment for get/set_pwm()
fixed reversed polarity setter logic
simplified do_set_pwm()
merged all three patches into one
Fixed issue reported by Simon (open-loop not working when set_cnt is 255)
Changes since v3:
set is against current head of Linus tree
removed dev_err() calls when i2c_smbus_read/write_byte_data() fails
pwm1 sets SET_OUT reg, fan1_target sets SET_CNT reg, both unconditionally
removed all DT and platform_data knobs available via sysfs
updated documentation files to reflect two previous changes
Changes since v2:
set ref_clk value to 32768 if not overloaded
fixed multi-line comment format in g762.h
removed static const G762_DEFAULT_PDATA in g762.h for a function
CodingStyle: add spaces between operatoirs when missing
check return value of i2c_smbus_{read,write}_byte_data()
remove { } is not needed in single-statement conditionals
introduced G762_ATTR_VAL() to allow sparse init of platform_data struc
changed missed reference to linear mode for DC mode
Changes since v1
Changed author
removed bad tabs
Provide datasheet link w/o fud in g762 documentation
removed documentation for removed fan_gear_mode sysfs entry
removed tested-by from patch
removed FSF address in header file
removed useless include of linux/slab.h
removed useless parenthesis against HZ in define
use spaces around binary operators
use i2c_smbus_{read,write}_byte_data() instead of
g762_{read,write}_value()
use return value of i2c_smbus_write_byte_data()
use true for initializing boolean
removed useless blank lines
do not enforce single return point rule where less readable
use dev_err() and dev_dbg() instead of dev_info() when it makes sense
remove leading '' for function passed as pointers
allow passing parameter via platform_data struct for non-DT enabled boards
set data-valid to false when config is modified
s/linear/DC/ for mode (g762 datasheet uses linear)
more tests on rpm_from_cnt() and cnt_from_rpm() formula
dont overload
Changes since v0
removed forward declaration
use bool for valid field instead of bit field.
protect macro args
fixed typo in subject line
Added mention for G763 support in Kconfig
fixed typo in driver name in Kconfig
do not use DRVNAME in i2c_device_id g762_id[]
Following discussions, kept DEVICE_ATTR (no switch to SENSOR_DEVICE_ATTR)
removed useless casts when flipping bit values
Sanity check user input value (e.g. to prevent 256 to silenty become 0)
Added extra lines for multi line comments when needed
removed various testing knobs
make removed knobs available via DT
passed checkpatch script on the patch
removed useless lock protection againt clk setting
moved all setter at the beginning of the file
removed bad (u16) casts in g762_write_value() calls
Documentation/devicetree/bindings/hwmon/g762.txt | 47 +
Documentation/hwmon/g762 | 65 ++
drivers/hwmon/Kconfig| 10 +
drivers/hwmon/Makefile |1 +
Re: [PATCH v2] hwmon: (ina2xx) Add device tree support to pass the shunt resistor
On Thu, Jun 20, 2013 at 02:27:18AM +, Tang Yuantian-B29983 wrote: -Original Message- From: Guenter Roeck [mailto:li...@roeck-us.net] Sent: 2013年6月19日 星期三 23:43 To: Tang Yuantian-B29983 Cc: kh...@linux-fr.org; lm-sens...@lm-sensors.org; devicetree- disc...@lists.ozlabs.org; linux-...@vger.kernel.org Subject: Re: [PATCH v2] hwmon: (ina2xx) Add device tree support to pass the shunt resistor On Wed, Jun 19, 2013 at 02:50:20PM +0800, yuantian.t...@freescale.com wrote: From: Tang Yuantian yuantian.t...@freescale.com Adding another way that is device tree to pass the shunt resistor value to driver except for platform data. Signed-off-by: Tang Yuantian yuantian.t...@freescale.com I noticed that of.h include was missing. No need to re-send; I added it myself. Applied to -next. Thanks for your help. Of.h is already included in i2c.h. If it is really needed, I should have seen the warning or error when compile. Including it explicitly is also a good point. Common rule is to include a file if its definitions are used. We can not rely on some other file including it for us, since it is not guaranteed that this will always be the case. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5] watchdog: Add Broadcom BCM2835 watchdog timer driver
On Tue, Jun 18, 2013 at 07:44:48PM +0200, Lubomir Rintel wrote:
This adds a driver for watchdog timer hardware present on Broadcom BCM2835
SoC,
used in Raspberry Pi and Roku 2 devices.
Signed-off-by: Lubomir Rintel lkund...@v3.sk
Tested-by: Stephen Warren swar...@wwwdotorg.org
Cc: Stephen Warren swar...@wwwdotorg.org
Cc: Wim Van Sebroeck w...@iguana.be
Cc: Guenter Roeck li...@roeck-us.net
Cc: linux-rpi-ker...@lists.infradead.org
Cc: linux-watch...@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
Reviewed-by: Guenter Roeck li...@roeck-us.net
---
Changes for v2:
- Use per-device structure instead of global variables for lock and
memory base
- Fix default timeout setting
- Do not leak memory if probe fails
- Style fixes
- Split out defconfig into a separate commit
- Meaningful commit message with credit
Changes for v3:
- Add proper copyright notice to header
- Drop status constants, we don't use them
- Fix heartbeat parameter's type
- Log driver initialization message once the device is probed
Changes for v4:
- Drop an useless initializer
- Add a signoff from downstream
- Do not announce the driver to the log if we failed to probe
- Set platform data earlier and do not explicitly unset it
Changes for v5:
- Copyright clarification
.../bindings/watchdog/brcm,bcm2835-pm-wdog.txt |5 +
drivers/watchdog/Kconfig | 11 ++
drivers/watchdog/Makefile |1 +
drivers/watchdog/bcm2835_wdt.c | 189
4 files changed, 206 insertions(+), 0 deletions(-)
create mode 100644 drivers/watchdog/bcm2835_wdt.c
diff --git
a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
index d209366..f801d71 100644
--- a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
@@ -5,9 +5,14 @@ Required properties:
- compatible : should be brcm,bcm2835-pm-wdt
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+
+- timeout-sec : Contains the watchdog timeout in seconds
+
Example:
watchdog {
compatible = brcm,bcm2835-pm-wdt;
reg = 0x7e10 0x28;
+ timeout-sec = 10;
};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..c3d3b16 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1098,6 +1098,17 @@ config BCM63XX_WDT
To compile this driver as a loadable module, choose M here.
The module will be called bcm63xx_wdt.
+config BCM2835_WDT
+ tristate Broadcom BCM2835 hardware watchdog
+ depends on ARCH_BCM2835
+ select WATCHDOG_CORE
+ help
+ Watchdog driver for the built in watchdog hardware in Broadcom
+ BCM2835 SoC.
+
+ To compile this driver as a loadable module, choose M here.
+ The module will be called bcm2835_wdt.
+
config LANTIQ_WDT
tristate Lantiq SoC watchdog
depends on LANTIQ
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..1bf5675 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
+obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
# AVR32 Architecture
obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
new file mode 100644
index 000..61566fc
--- /dev/null
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -0,0 +1,189 @@
+/*
+ * Watchdog driver for Broadcom BCM2835
+ *
+ * bcm2708_wdog driver written by Luke Diamand that was obtained from
+ * branch rpi-3.6.y of git://github.com/raspberrypi/linux.git was used
+ * as a hardware reference for the Broadcom BCM2835 watchdog timer.
+ *
+ * Copyright (C) 2013 Lubomir Rintel lkund...@v3.sk
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include linux/types.h
+#include linux/module.h
+#include linux/io.h
+#include linux/watchdog.h
+#include linux/platform_device.h
+#include linux/of_address.h
+#include linux/miscdevice.h
+
+#define PM_RSTC 0x1c
+#define PM_WDOG 0x24
+
+#define PM_PASSWORD 0x5a00
+
+#define PM_WDOG_TIME_SET 0x000f
+#define PM_RSTC_WRCFG_CLR0xffcf
+#define PM_RSTC_WRCFG_SET
Re: [RESEND,PATCHv6 1/1] Add support for GMT G762/G763 PWM fan controllers
On Sun, Jun 16, 2013 at 12:36:20AM +0200, Arnaud Ebalard wrote:
GMT G762/763 fan speed PWM controller is connected directly to a fan
and performs closed-loop or open-loop control of the fan speed. Two
modes - PWM or DC - are supported by the chip. Introduced driver
provides various knobs to control the operations of the chip (via
sysfs interface). Specific characteristics of the system can be passed
either using board init code or via DT. Documentation for both the
driver and DT bindings are also provided.
Signed-off-by: Arnaud Ebalard a...@natisbad.org
---
Hi Guenter,
In the end, I think handling the clock properly for DT path
(clk_prepare_enable() and associated clk_disable_unprepare()
for both error and module unloading) adds some code but I
do not see how to spare this. Comments welcome.
If you have additional comments and a new version is required,
I'll handle those with Simon's tests on its platform.
Cheers,
a+
ps: this is a resend including g762.c in the commit this time.
Changes since v5:
fixed useless ret parameter init in various functions
removed useless goto in favour of direct return
enable fan detection and fan ooc protection in an init function
fixed patch version (previous v5 was mistakenly named v4)
correctly balance clk_prepare_enable() by clk_disable_unprepare()
more tests w/o CONFIG_OF and with driver as module (load/unload)
Changes since v4:
Removed unused defines
fixed some comments
s/pwm_freq/clk_freq/g and fixed associated comments
removed useless settings of data-valid to false
do not hide original return code in g762_one_prop_commit()
replaced ref_clk property by a required reference to a clock in .dts
drop fan_pulses property in DT and provide sysfs attribute
removed whole G762_VAL_TEST_BIT hack
still not supporting 0 for pwm_enable: fixed comment
fixed comment for get/set_pwm()
fixed reversed polarity setter logic
simplified do_set_pwm()
merged all three patches into one
Fixed issue reported by Simon (open-loop not working when set_cnt is 255)
Changes since v3:
set is against current head of Linus tree
removed dev_err() calls when i2c_smbus_read/write_byte_data() fails
pwm1 sets SET_OUT reg, fan1_target sets SET_CNT reg, both unconditionally
removed all DT and platform_data knobs available via sysfs
updated documentation files to reflect two previous changes
Changes since v2:
set ref_clk value to 32768 if not overloaded
fixed multi-line comment format in g762.h
removed static const G762_DEFAULT_PDATA in g762.h for a function
CodingStyle: add spaces between operatoirs when missing
check return value of i2c_smbus_{read,write}_byte_data()
remove { } is not needed in single-statement conditionals
introduced G762_ATTR_VAL() to allow sparse init of platform_data struc
changed missed reference to linear mode for DC mode
Changes since v1
Changed author
removed bad tabs
Provide datasheet link w/o fud in g762 documentation
removed documentation for removed fan_gear_mode sysfs entry
removed tested-by from patch
removed FSF address in header file
removed useless include of linux/slab.h
removed useless parenthesis against HZ in define
use spaces around binary operators
use i2c_smbus_{read,write}_byte_data() instead of
g762_{read,write}_value()
use return value of i2c_smbus_write_byte_data()
use true for initializing boolean
removed useless blank lines
do not enforce single return point rule where less readable
use dev_err() and dev_dbg() instead of dev_info() when it makes sense
remove leading '' for function passed as pointers
allow passing parameter via platform_data struct for non-DT enabled boards
set data-valid to false when config is modified
s/linear/DC/ for mode (g762 datasheet uses linear)
more tests on rpm_from_cnt() and cnt_from_rpm() formula
dont overload
Changes since v0
removed forward declaration
use bool for valid field instead of bit field.
protect macro args
fixed typo in subject line
Added mention for G763 support in Kconfig
fixed typo in driver name in Kconfig
do not use DRVNAME in i2c_device_id g762_id[]
Following discussions, kept DEVICE_ATTR (no switch to SENSOR_DEVICE_ATTR)
removed useless casts when flipping bit values
Sanity check user input value (e.g. to prevent 256 to silenty become 0)
Added extra lines for multi line comments when needed
removed various testing knobs
make removed knobs available via DT
passed checkpatch script on the patch
removed useless lock protection againt clk setting
moved all setter at the beginning of the file
removed bad (u16) casts in g762_write_value() calls
Documentation/devicetree/bindings/hwmon/g762.txt | 47 +
Re: [PATCHv4 1/1] Add support for GMT G762/G763 PWM fan controllers
On Sat, Jun 15, 2013 at 06:13:49PM +0200, Arnaud Ebalard wrote:
GMT G762/763 fan speed PWM controller is connected directly to a fan
and performs closed-loop or open-loop control of the fan speed. Two
modes - PWM or DC - are supported by the chip. Introduced driver
provides various knobs to control the operations of the chip (via
sysfs interface). Specific characteristics of the system can be passed
either using board init code or via DT. Documentation for both the
driver and DT bindings are also provided.
Signed-off-by: Arnaud Ebalard a...@natisbad.org
---
Guenter, do not hesitate to tell me if changes associated with the
switch from a simple property DT for clock frequency to a fixed
clock require some additional work. History of changes is below.
Changes since v4:
Fixed issue reported by Simon (open-loop not working when set_cnt is 255)
replaced ref_clk property by a required reference to a clock in .dts
now use a fixed clock in DT instead of a simple property
Replace fan_pulses property in DT in favour of a sysfs attribute
s/pwm_freq/clk_freq/g and fixed associated comments
removed whole G762_VAL_TEST_BIT hack
merged all three patches into one
removed useless settings of data-valid to false
do not hide original return code in g762_one_prop_commit()
Removed unused defines
fixed various comments
fixed comment for get/set_pwm_enable() (still not supporting 0 for
pwm1_enable)
fixed comment for get/set_pwm()
fixed reversed polarity setter logic
simplified do_set_pwm()
Changes since v3:
set is against current head of Linus tree
removed dev_err() calls when i2c_smbus_read/write_byte_data() fails
pwm1 sets SET_OUT reg, fan1_target sets SET_CNT reg, both unconditionally
removed all DT and platform_data knobs available via sysfs
updated documentation files to reflect two previous changes
Changes since v2:
set ref_clk value to 32768 if not overloaded
fixed multi-line comment format in g762.h
removed static const G762_DEFAULT_PDATA in g762.h for a function
CodingStyle: add spaces between operatoirs when missing
check return value of i2c_smbus_{read,write}_byte_data()
remove { } is not needed in single-statement conditionals
introduced G762_ATTR_VAL() to allow sparse init of platform_data struc
changed missed reference to linear mode for DC mode
Changes since v1
Changed author
removed bad tabs
Provide datasheet link w/o fud in g762 documentation
removed documentation for removed fan_gear_mode sysfs entry
removed tested-by from patch
removed FSF address in header file
removed useless include of linux/slab.h
removed useless parenthesis against HZ in define
use spaces around binary operators
use i2c_smbus_{read,write}_byte_data() instead of
g762_{read,write}_value()
use return value of i2c_smbus_write_byte_data()
use true for initializing boolean
removed useless blank lines
do not enforce single return point rule where less readable
use dev_err() and dev_dbg() instead of dev_info() when it makes sense
remove leading '' for function passed as pointers
allow passing parameter via platform_data struct for non-DT enabled boards
set data-valid to false when config is modified
s/linear/DC/ for mode (g762 datasheet uses linear)
more tests on rpm_from_cnt() and cnt_from_rpm() formula
dont overload
Changes since v0
removed forward declaration
use bool for valid field instead of bit field.
protect macro args
fixed typo in subject line
Added mention for G763 support in Kconfig
fixed typo in driver name in Kconfig
do not use DRVNAME in i2c_device_id g762_id[]
Trailing whitespace above
Following discussions, kept DEVICE_ATTR (i.e. no switch to
SENSOR_DEVICE_ATTR)
removed useless casts when flipping bit values
Sanity check user input value (e.g. to prevent 256 to silenty become 0)
Added extra lines for multi line comments when needed
removed various testing knobs
make removed knobs available via DT
passed checkpatch script on the patch
removed useless lock protection againt clk setting
moved all setter at the beginning of the file
removed bad (u16) casts in g762_write_value() calls
Documentation/devicetree/bindings/hwmon/g762.txt | 47 +
Documentation/hwmon/g762 | 65 ++
drivers/hwmon/Kconfig| 10 +
drivers/hwmon/Makefile |1 +
drivers/hwmon/g762.c | 1134
++
include/linux/platform_data/g762.h | 37 +
6 files changed, 1294 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/g762.txt
create mode 100644 Documentation/hwmon/g762
create mode 100644 drivers/hwmon/g762.c
create
Re: [RESEND,PATCHv6 1/1] Add support for GMT G762/G763 PWM fan controllers
On Sun, Jun 16, 2013 at 12:36:20AM +0200, Arnaud Ebalard wrote: GMT G762/763 fan speed PWM controller is connected directly to a fan and performs closed-loop or open-loop control of the fan speed. Two modes - PWM or DC - are supported by the chip. Introduced driver provides various knobs to control the operations of the chip (via sysfs interface). Specific characteristics of the system can be passed either using board init code or via DT. Documentation for both the driver and DT bindings are also provided. Signed-off-by: Arnaud Ebalard a...@natisbad.org --- Hi Guenter, In the end, I think handling the clock properly for DT path (clk_prepare_enable() and associated clk_disable_unprepare() for both error and module unloading) adds some code but I do not see how to spare this. Comments welcome. If you have additional comments and a new version is required, I'll handle those with Simon's tests on its platform. We might need to be more careful with the clocks. Several drivers save the value retrieved with of_clk_get() and call clk_put() afterwards. But some drivers don't do that, so I don't really know the implications. I'll look into it some more to make sure everything is correct. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4 2/3] Add documentation for g762 driver
On Tue, Jun 04, 2013 at 09:14:47AM +0200, Arnaud Ebalard wrote:
Signed-off-by: Arnaud Ebalard a...@natisbad.org
Please merge the patches into one. I don't see the benefit of having three
patches as it is all part of one driver.
Thanks,
Guenter
---
Documentation/hwmon/g762 | 62
++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/hwmon/g762
diff --git a/Documentation/hwmon/g762 b/Documentation/hwmon/g762
new file mode 100644
index 000..8cdb0dc
--- /dev/null
+++ b/Documentation/hwmon/g762
@@ -0,0 +1,62 @@
+Kernel driver g762
+==
+
+The GMT G762 Fan Speed PWM Controller is connected directly to a fan
+and performs closed-loop or open-loop control of the fan speed. Two
+modes - PWM or DC - are supported by the device.
+
+For additional information, a detailed datasheet is available at
+http://natisbad.org/NAS/ref/GMT_EDS-762_763-080710-0.2.pdf. sysfs
+bindings are described in Documentation/hwmon/sysfs-interface.
+
+The following entries are available to the user in a subdirectory of
+/sys/bus/i2c/drivers/g762/ to control the operation of the device.
+This can be done manually using the following entries but is usually
+done via a userland daemon like fancontrol.
+
+Note that those entries do not provide ways to setup the specific
+hardware characteristics of the system (reference clock, pulses per
+fan revolution, ...); Those can be modified via devicetree bindings
+documented in Documentation/devicetree/bindings/hwmon/g762.txt or
+using a specific platform_data structure in board initialization
+file (see include/linux/platform_data/g762.h).
+
+ fan1_target: set desired fan speed. This only makes sense in closed-loop
+fan speed control (i.e. when pwm1_enable is set to 2).
+
+ fan1_input: provide current fan rotation value in RPM as reported by
+the fan to the device.
+
+ fan1_div: fan clock divisor. Supported value are 1, 2, 4 and 8.
+
+ fan1_fault: reports fan failure, i.e. no transition on fan gear pin for
+about 0.7s (if the fan is not voluntarily set off).
+
+ fan1_alarm: in closed-loop control mode, if fan RPM value is 25% out
+of the programmed value for over 6 seconds 'fan1_alarm' is
+set to 1.
+
+ pwm1_enable: set current fan speed control mode i.e. 1 for manual fan
+speed control (open-loop) via pwm1 described below, 2 for
+automatic fan speed control (closed-loop) via fan1_target
+above (pwm1 is also usable).
+
+ pwm1_mode: set or get fan driving mode: 1 for PWM mode, 0 for DC mode.
+
+ pwm1: get or set PWM fan control value in open-loop mode. This is an
+integer value between 0 and 255. 0 stops the fan, 255 makes
+it run at full speed.
+
+Both in PWM mode ('pwm1_mode' set to 1) and DC mode ('pwm1_mode' set to 0),
+when current fan speed control mode is open-loop ('pwm1_enable' set to 1),
+the fan speed is programmed by setting a value between 0 and 255 via 'pwm1'
+entry (0 stops the fan, 255 makes it run at full speed). In closed-loop mode
+('pwm1_enable' set to 2), the expected rotation speed in RPM can be passed to
+the chip via 'fan1_target'. In closed-loop mode, the target speed is compared
+with current speed (available via 'fan1_input') by the device and a feedback
+is performed to match that target value. The fan speed value is computed
+based on the parameters associated with the physical characteristics of the
+system: a reference clock source frequency, a number of pulses per fan
+revolution, etc.
+
+Note that the driver will update its values at most once per second.
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4 1/3] Add support for GMT G762/G763 PWM fan controller
On Tue, Jun 04, 2013 at 09:14:29AM +0200, Arnaud Ebalard wrote:
Signed-off-by: Arnaud Ebalard a...@natisbad.org
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile |1 +
drivers/hwmon/g762.c |
include/linux/platform_data/g762.h | 47 ++
4 files changed, 1169 insertions(+)
create mode 100644 drivers/hwmon/g762.c
create mode 100644 include/linux/platform_data/g762.h
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0428e8a..142bdf8 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -456,6 +456,16 @@ config SENSORS_G760A
This driver can also be built as a module. If so, the module
will be called g760a.
+config SENSORS_G762
+ tristate GMT G762 and G763
+ depends on I2C
+ help
+ If you say yes here you get support for Global Mixed-mode
+ Technology Inc G762 and G763 fan speed PWM controller chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called g762.
+
config SENSORS_GL518SM
tristate Genesys Logic GL518SM
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d17d3e6..4f0fb52 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
obj-$(CONFIG_SENSORS_FAM15H_POWER) += fam15h_power.o
obj-$(CONFIG_SENSORS_FSCHMD) += fschmd.o
obj-$(CONFIG_SENSORS_G760A) += g760a.o
+obj-$(CONFIG_SENSORS_G762) += g762.o
obj-$(CONFIG_SENSORS_GL518SM)+= gl518sm.o
obj-$(CONFIG_SENSORS_GL520SM)+= gl520sm.o
obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
new file mode 100644
index 000..194be43
--- /dev/null
+++ b/drivers/hwmon/g762.c
@@ -0,0 +1, @@
+/*
+ * g762 - Driver for the Global Mixed-mode Technology Inc. fan speed
+ *PWM controller chips from G762 family, i.e. G762 and G763
+ *
+ * Copyright (C) 2013, Arnaud EBALARD a...@natisbad.org
+ *
+ * This work is based on a basic version for 2.6.31 kernel developed
+ * by Olivier Mouchet for LaCie. Updates and correction have been
+ * performed to run on recent kernels. Additional features, like the
+ * ability to configure various characteristics via .dts file, have
+ * been added. Detailed datasheet on which this development is based
+ * is available here:
+ *
+ * http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf
+ *
+ * Headers from previous developments have been kept below:
+ *
+ * Copyright (c) 2009 LaCie
+ *
+ * Author: Olivier Mouchet olivier.mouc...@gmail.com
+ *
+ * based on g760a code written by Herbert Valerio Riedel h...@gnu.org
+ * Copyright (C) 2007 Herbert Valerio Riedel h...@gnu.org
+ *
+ * g762: minimal datasheet available at:
+ * http://www.gmt.com.tw/product/datasheet/EDS-762_3.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation.
+ */
+
+#include linux/device.h
+#include linux/module.h
+#include linux/init.h
+#include linux/jiffies.h
+#include linux/i2c.h
+#include linux/hwmon.h
+#include linux/hwmon-sysfs.h
+#include linux/err.h
+#include linux/mutex.h
+#include linux/kernel.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/platform_data/g762.h
+
+#define DRVNAME g762
+
+static const struct i2c_device_id g762_id[] = {
+ { g762, 0 },
+ { g763, 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, g762_id);
+
+enum g762_regs {
+ G762_REG_SET_CNT = 0x00,
+ G762_REG_ACT_CNT = 0x01,
+ G762_REG_FAN_STA = 0x02,
+ G762_REG_SET_OUT = 0x03,
+ G762_REG_FAN_CMD1 = 0x04,
+ G762_REG_FAN_CMD2 = 0x05,
+};
+
+/* Config register bits */
+#define G762_REG_FAN_CMD1_DET_FAN_FAIL 0x80 /* enable fan_fail signal */
+#define G762_REG_FAN_CMD1_DET_FAN_OOC 0x40 /* enable fan_out_of_control */
+#define G762_REG_FAN_CMD1_OUT_MODE 0x20 /* out mode, PWM or DC */
+#define G762_REG_FAN_CMD1_FAN_MODE 0x10 /* fan mode: closed/open loop */
+#define G762_REG_FAN_CMD1_CLK_DIV_ID1 0x08 /* clock divisor value */
+#define G762_REG_FAN_CMD1_CLK_DIV_ID0 0x04
+#define G762_REG_FAN_CMD1_PWM_POLARITY 0x02 /* PWM polarity */
+#define G762_REG_FAN_CMD1_PULSE_PER_REV
Re: [PATCHv2 1/3] Add support for GMT G762/G763 PWM fan controller
On Tue, Jun 04, 2013 at 11:23:07PM +0200, Simon Guinot wrote: On Tue, Jun 04, 2013 at 08:52:12AM +0200, Arnaud Ebalard wrote: Hi Simon, Simon Guinot simon.gui...@sequanux.org writes: On Sat, Jun 01, 2013 at 07:26:54PM +0200, Arnaud Ebalard wrote: Hi Simon and Guenter, Simon Guinot simon.gui...@sequanux.org writes: On Tue, May 28, 2013 at 12:03:14AM +0200, Arnaud Ebalard wrote: Signed-off-by: Arnaud Ebalard a...@natisbad.org --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile |1 + drivers/hwmon/g762.c | 1012 include/linux/platform_data/g762.h | 54 ++ 4 files changed, 1077 insertions(+) create mode 100644 drivers/hwmon/g762.c create mode 100644 include/linux/platform_data/g762.h Hi Arnaud, After more tests on my 2Big Network v2 board, it appears that the fan doesn't rotate when PWM mode (the preferred operating mode for this board) is selected. Nevertheless, DC mode is usable (even if not ideal given the hardware). After some investigations I noticed that an extra initialization is needed to enable PWM mode on my board: the set_cnt register must be set to 0 while the default value is 0xff. Is that specific to my hardware ? Is PWM mode working on your ReadyNAS with the default set_cnt value ? First, thanks for testing this! Regarding your problem, I first started by booting current version of my driver on the Duo v2 w/o touching chip registers (only clock reference value). This way, I inherit the values installed by (NETGEAR's) u-boot: $ for k in fan* pwm* ; do echo -n $k: ; echo `cat $k `; done fan1_alarm:0 fan1_div:2 fan1_fault:0 fan1_input:1807 fan1_target:1807 pwm1:221 pwm1_enable:2/* closed-loop, i.e. config is done via set_cnt */ pwm1_mode:1 /* PWM mode */ Sorry, I realize that I have not been very accurate in my description of the problem: The fan doesn't rotate when PWM _and_ open-loop control are selected. On the 2Big2 board, 2 wires are used to drive the fan. Then with pwm1_mode=1, pwm1_enable=1 and set_cnt=0xff (default value), nothing happen whatever the value I write in pwm1. After boot, with set_cnt and set_out untouched by me (set_cnt set to 0x5a by u-boot, i.e. 255-0x5a read on pwm1): fan1_alarm:0 fan1_div:1 fan1_fault:0 fan1_input:1365 fan1_target:1365 pwm1:0 /* set_out is 0 (considering fan1_target, set_cnt is 0x5a)*/ pwm1_enable:2 /* closed-loop */ pwm1_mode:0 /* DC mode */ # echo 1 pwm1_mode/* PWM mode */ # fan rotates # echo 1 pwm1_enable /* open-loop */ # fan stops rotating At that point we have: fan1_alarm:0 fan1_div:1 fan1_fault:0 fan1_input:0 fan1_target:1365 pwm1:0 /* set_out is 0, set_cnt is still 0x5a */ pwm1_enable:1 pwm1_mode:1 Then: # echo 100 pwm1# fan rotates OK you are lucky. Your bootloader initialize set_cnt with 0x5a. Mine don't and it is precisely the issue: set_cnt is still 0xff (the default value) when the driver controls the fan. Please, could you try open-loop mode with set_cnt set to 0xff ? Maybe you can enforce this value for the test purpose ? If you observe the same behaviour than me, then could modify the driver to ensure that set_cnt is not 0xff when open-loop mode is selected ? Maybe by systematically setting a different value (as 0) ? I think it would make sense to set it to 0 when open-loop mode is selected, or at least to set it to a value != 0xff. Question would be if the value has any meaning in open loop mode, other than to enable/disable fan control entirely. Thanks, Guenter By testing, I have discovered that writing 0 into set_cnt allows to work around the issue. I can do this by using the closed-loop control: pwm1_mode=1, pwm1_enable=2 and pwm=255. Now, set_cnt worths 0 and if I switch back to open-loop control, all works as expected. Can you try open-loop control and PWM mode with your board ? I wonder if this issue is specific to the 2Big2 hardware. AFAICT, set_cnt has never been set to 0 in my test and PWM+open-loop works as expected, i.e. just by setting a non-zero value in set_out. Tell me if I missed something or if you want me to perform another test. Actually, I have only problems when set_cnt worths 0xff. Please, could you try the tests mentioned above ? Thanks in advance, Simon Cheers, a+ ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ devicetree-discuss mailing list
Re: [PATCHv2 1/3] Add support for GMT G762/G763 PWM fan controller
On Sun, Jun 02, 2013 at 05:45:58PM +0200, Arnaud Ebalard wrote: Hi, I haven't noticed this issue while testing your v1 patch series because at the time I was using a board with an U-Boot modified by LaCie. This last sets the set_cnt register to 0 while U-Boot mainline don't. Actually, the 2Big fan is not configured nor even started by U-Boot mainline. I have to fix this but maybe that something can be done in the g762 Linux driver too ? At the moment, DT bindings and platform_data structure both allow setting an initial fan_target value. I could add a 'pwm' binding for DT and an assoicated field in platform_data to allow passing an initial value so that maintainers of platforms with a 2 wire fan (open loop) can still make the fan somewhat rotate at boot. Devicetree properties are supposed to describe the system, not its configuration. There are exceptions, but I don't recall seeing a case where a parameter can be set both with devicetree properties and sysfs attributes. Before going further on this, we will need to have an ack from devicetree maintainers which specifically permits your dual-configuration approach. Even with that, you'll still have convince me that you need both (while so far no other driver needed it). Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv3 0/3] Add G762/G763 PWM fan controller
On Sun, Jun 02, 2013 at 10:14:05PM +0200, Arnaud Ebalard wrote: Hi, This series adds support for GMT G762/G763. This work is based on a basic version for 2.6.31 kernel developed Olivier Mouchet for LaCie NAS. Updates have been performed to run on recent kernels. Support has been completed and additional features added: ability to configure various characteristics from .dts file, better initialization, alarms and error reporting support, gear mode, polarity, fan pulse per revolution, fan startup voltage control. The following detailed datasheet has been used as a basis for this work: http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf The patch was developed for and tested against the GMT G762 fan controller used in a Netgear ReadyNAS Duo v2 (kirkwood 88F6282-based NAS). This is the main reason for the device tree bindings provided in first patch. The driver also support init via board file. The patches are against current ARM tree; tell me if you need me to rebase it against something else. Patch 2 and 3 provides documentation for the driver and DT bindings, respectively. As discussed in a previous email: - A macro is defined to pass values of platform data structure in board setup files. If one has a better approach ... - This v3 still has support for pwm1 sysfs entry in closed-loop mode. Guenter, if you still think it is a bad idea, I will change that in v4. If you want me to accept your code, I would suggest to change it such that pwm1 sets the FAN_SET_OUT register, and fan1_target sets the SET_CNT register, both unconditionally. Redefining the pwm1 attribute to set the fan target speed instead of a pwm / voltage value is not acceptable. note: this should not be an issue but this v3 is based on jcooper/mvebu/fixes branch. I don't know about this branch/repository. If there are hwmon fixes in it, they should be submitted to the lm-sensors mailing list. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv3 1/3] Add support for GMT G762/G763 PWM fan controller
On Sun, Jun 02, 2013 at 10:14:19PM +0200, Arnaud Ebalard wrote:
Signed-off-by: Arnaud Ebalard a...@natisbad.org
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile |1 +
drivers/hwmon/g762.c | 1179
include/linux/platform_data/g762.h | 51 ++
4 files changed, 1241 insertions(+)
create mode 100644 drivers/hwmon/g762.c
create mode 100644 include/linux/platform_data/g762.h
[ ... ]
+static inline int g762_read_byte(const struct i2c_client *client, u8 reg,
+ u8 *dest)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(client, reg);
+ if (ret 0) {
+ dev_err(client-dev, failed to read reg %x02x, err %d\n,
+ (int)reg, ret);
I would suggest to write a piece of test code for the above statement and
observe its output. printf(%x02x\n, 0x1234); should do.
Is the error message really needed ? If there is a persistent error,
it will clog the log.
Thanks,
Guenter
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv2 1/3] Add support for GMT G762/G763 PWM fan controller
On Tue, May 28, 2013 at 12:03:14AM +0200, Arnaud Ebalard wrote:
Signed-off-by: Arnaud Ebalard a...@natisbad.org
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile |1 +
drivers/hwmon/g762.c | 1012
include/linux/platform_data/g762.h | 54 ++
4 files changed, 1077 insertions(+)
create mode 100644 drivers/hwmon/g762.c
create mode 100644 include/linux/platform_data/g762.h
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0428e8a..142bdf8 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -456,6 +456,16 @@ config SENSORS_G760A
This driver can also be built as a module. If so, the module
will be called g760a.
+config SENSORS_G762
+ tristate GMT G762 and G763
+ depends on I2C
+ help
+ If you say yes here you get support for Global Mixed-mode
+ Technology Inc G762 and G763 fan speed PWM controller chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called g762.
+
config SENSORS_GL518SM
tristate Genesys Logic GL518SM
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d17d3e6..4f0fb52 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
obj-$(CONFIG_SENSORS_FAM15H_POWER) += fam15h_power.o
obj-$(CONFIG_SENSORS_FSCHMD) += fschmd.o
obj-$(CONFIG_SENSORS_G760A) += g760a.o
+obj-$(CONFIG_SENSORS_G762) += g762.o
obj-$(CONFIG_SENSORS_GL518SM)+= gl518sm.o
obj-$(CONFIG_SENSORS_GL520SM)+= gl520sm.o
obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
new file mode 100644
index 000..d21ec24
--- /dev/null
+++ b/drivers/hwmon/g762.c
@@ -0,0 +1,1012 @@
+/*
+ * g762 - Driver for the Global Mixed-mode Technology Inc. fan speed
+ *PWM controller chips from G762 family, i.e. G762 and G763
+ *
+ * Copyright (C) 2013, Arnaud EBALARD a...@natisbad.org
+ *
+ * This work is based on a basic version for 2.6.31 kernel developed
+ * by Olivier Mouchet for LaCie. Updates and correction have been
+ * performed to run on recent kernels. Additional features, like the
+ * ability to configure various characteristics via .dts file, have
+ * been added. Detailed datasheet on which this development is based
+ * is available here:
+ *
+ * http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf
+ *
+ * Headers from previous developments have been kept below:
+ *
+ * Copyright (c) 2009 LaCie
+ *
+ * Author: Olivier Mouchet olivier.mouc...@gmail.com
+ *
+ * based on g760a code written by Herbert Valerio Riedel h...@gnu.org
+ * Copyright (C) 2007 Herbert Valerio Riedel h...@gnu.org
+ *
+ * g762: minimal datasheet available at:
+ * http://www.gmt.com.tw/product/datasheet/EDS-762_3.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation.
+ */
+
+#include linux/device.h
+#include linux/module.h
+#include linux/init.h
+#include linux/jiffies.h
+#include linux/i2c.h
+#include linux/hwmon.h
+#include linux/hwmon-sysfs.h
+#include linux/err.h
+#include linux/mutex.h
+#include linux/kernel.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/platform_data/g762.h
+
+#define DRVNAME g762
+
+static const struct i2c_device_id g762_id[] = {
+ { g762, 0 },
+ { g763, 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, g762_id);
+
+enum g762_regs {
+ G762_REG_SET_CNT = 0x00,
+ G762_REG_ACT_CNT = 0x01,
+ G762_REG_FAN_STA = 0x02,
+ G762_REG_SET_OUT = 0x03,
+ G762_REG_FAN_CMD1 = 0x04,
+ G762_REG_FAN_CMD2 = 0x05,
+};
+
+/* Config register bits */
+#define G762_REG_FAN_CMD1_DET_FAN_FAIL 0x80 /* enable fan_fail signal */
+#define G762_REG_FAN_CMD1_DET_FAN_OOC 0x40 /* enable fan_out_of_control */
+#define G762_REG_FAN_CMD1_OUT_MODE 0x20 /* out mode, PWM or DC */
+#define G762_REG_FAN_CMD1_FAN_MODE 0x10 /* fan mode: closed/open loop */
+#define G762_REG_FAN_CMD1_CLK_DIV_ID1 0x08 /* clock divisor value */
+#define G762_REG_FAN_CMD1_CLK_DIV_ID0 0x04
+#define G762_REG_FAN_CMD1_PWM_POLARITY 0x02 /* PWM polarity */
+#define G762_REG_FAN_CMD1_PULSE_PER_REV
Re: [PATCHv2 0/3] Add G762/G763 PWM fan controller
On Tue, May 28, 2013 at 01:19:23PM +0200, Thierry Reding wrote: On Tue, May 28, 2013 at 12:15:04PM +0200, Arnaud Ebalard wrote: Hi Arnd, Arnd Bergmann a...@arndb.de writes: On Tuesday 28 May 2013 00:02:29 Arnaud Ebalard wrote: This series adds support for GMT G762/G763. This work is based on a basic version for 2.6.31 kernel developed Olivier Mouchet for LaCie NAS. Updates have been performed to run on recent kernels. Support has been completed and additional features added: ability to configure various characteristics from .dts file, better initialization, alarms and error reporting support, gear mode, polarity, fan pulse per revolution, fan startup voltage control. I wonder if this could be split into two separate drivers, one for the pwm subsystem, and one for a hardware-independent fan controller based on the pwm interfaces. To be honest, I wouldn't even know how to start in order to do that. Additionally, it would be worth the effort if other drivers could be easily refactored using this idea. I'll let hwmon people provide some feedback and try and comply with their directions (based on the cpu cycle I can spend on this). What Arnd proposes doesn't sounds like a very good idea. PWM-controlled fans should be able to work just fine using a generic driver that uses the PWM framework, similar to the PWM backlight and LED drivers. Isn't that exactly what Arnd proposed ? Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv2 0/3] Add G762/G763 PWM fan controller
On Tue, May 28, 2013 at 03:47:49PM +0200, Thierry Reding wrote: On Tue, May 28, 2013 at 05:29:40AM -0700, Guenter Roeck wrote: On Tue, May 28, 2013 at 01:19:23PM +0200, Thierry Reding wrote: On Tue, May 28, 2013 at 12:15:04PM +0200, Arnaud Ebalard wrote: Hi Arnd, Arnd Bergmann a...@arndb.de writes: On Tuesday 28 May 2013 00:02:29 Arnaud Ebalard wrote: This series adds support for GMT G762/G763. This work is based on a basic version for 2.6.31 kernel developed Olivier Mouchet for LaCie NAS. Updates have been performed to run on recent kernels. Support has been completed and additional features added: ability to configure various characteristics from .dts file, better initialization, alarms and error reporting support, gear mode, polarity, fan pulse per revolution, fan startup voltage control. I wonder if this could be split into two separate drivers, one for the pwm subsystem, and one for a hardware-independent fan controller based on the pwm interfaces. To be honest, I wouldn't even know how to start in order to do that. Additionally, it would be worth the effort if other drivers could be easily refactored using this idea. I'll let hwmon people provide some feedback and try and comply with their directions (based on the cpu cycle I can spend on this). What Arnd proposes doesn't sounds like a very good idea. PWM-controlled fans should be able to work just fine using a generic driver that uses the PWM framework, similar to the PWM backlight and LED drivers. Isn't that exactly what Arnd proposed ? Oh, right. I meant to say: What Arnd proposes sounds like a very good idea. =) Sorry for the confusion. I looked into it, and frankly I don't know how to make it work either. The chip is dedicated to fan control and supports both linear and pwm mode. Both modes are controlled differently. The available configuration options are very fan control specific. While I agree that a generic fan control driver for pwm chips would be great, problem here is that this is not a generic pwm controller. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] powerpc: Make Book E watchdog reset type configurable
On Fri, May 03, 2013 at 10:33:13AM +0200, Dirk Eibach wrote: Hi Guenter, diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e89fc31..6048593 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1172,6 +1172,38 @@ config BOOKE_WDT_DEFAULT_TIMEOUT The value can be overridden by the wdt_period command-line parameter. +choice + prompt PowerPC Book-E Watchdog reset type + depends on BOOKE_WDT + default BOOKE_WDT_RESET_CHIP + help + Specify what kind of reset will be executed on watchdog timeout. + Seems to me it would be much better to make this configurable via platform data and/or device tree. good catch. What do the device-tree gurus think to be a nice property name? having a closer look, I realized booke_wdt is not device-tree based yet. Migrating it would come close to a rewrite, breaking compatibility for all current users. Sorry, this is way beyond the Really ? I don't think making it a platform driver would be that hard, or break anything. time I have for this project. So I suggest merging the change the way it is, as it is clearly an improvement. That will be up to Wim to decide. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] powerpc: Make Book E watchdog reset type configurable
On Fri, May 03, 2013 at 10:33:13AM +0200, Dirk Eibach wrote: Hi Guenter, diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e89fc31..6048593 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1172,6 +1172,38 @@ config BOOKE_WDT_DEFAULT_TIMEOUT The value can be overridden by the wdt_period command-line parameter. +choice + prompt PowerPC Book-E Watchdog reset type + depends on BOOKE_WDT + default BOOKE_WDT_RESET_CHIP + help + Specify what kind of reset will be executed on watchdog timeout. + Seems to me it would be much better to make this configurable via platform data and/or device tree. good catch. What do the device-tree gurus think to be a nice property name? having a closer look, I realized booke_wdt is not device-tree based yet. Migrating it would come close to a rewrite, breaking compatibility for all current users. Sorry, this is way beyond the time I have for this project. So I suggest merging the change the way it is, as it is clearly an improvement. I sent a couple of RFC patches converting the driver to a platform driver and adding of support to linux-watchdog and linux-kernel mailing lists. Compile tested only, and missing is automatic driver instantiation. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] powerpc: Make Book E watchdog reset type configurable
On Fri, May 03, 2013 at 08:30:03PM +0200, Dirk Eibach wrote: Am 03.05.2013 15:46, schrieb Guenter Roeck: On Fri, May 03, 2013 at 10:33:13AM +0200, Dirk Eibach wrote: Hi Guenter, diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e89fc31..6048593 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1172,6 +1172,38 @@ config BOOKE_WDT_DEFAULT_TIMEOUT The value can be overridden by the wdt_period command-line parameter. +choice + prompt PowerPC Book-E Watchdog reset type + depends on BOOKE_WDT + default BOOKE_WDT_RESET_CHIP + help + Specify what kind of reset will be executed on watchdog timeout. + Seems to me it would be much better to make this configurable via platform data and/or device tree. good catch. What do the device-tree gurus think to be a nice property name? having a closer look, I realized booke_wdt is not device-tree based yet. Migrating it would come close to a rewrite, breaking compatibility for all current users. Sorry, this is way beyond the Really ? I don't think making it a platform driver would be that hard, or break anything. Maybe I am missing something, but wouldn't making it a device-tree driver mean, that I would have to identify all users of BOOKE_WDT and add it to their device trees? Not necessarily; only if the platform driver doesn't auto-instantiate. I'll play with it and see if I can get it working. On the other side, that wouldn't be too bad either. As far as I can see from a quick search, there is no default configuration in the upstream kernel which has BOOKE_WDT enabled. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] powerpc: Make Book E watchdog reset type configurable
Hi Dirk, On Thu, May 02, 2013 at 09:11:19PM +0200, Dirk Eibach wrote: Hi Guenter, diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e89fc31..6048593 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1172,6 +1172,38 @@ config BOOKE_WDT_DEFAULT_TIMEOUT The value can be overridden by the wdt_period command-line parameter. +choice + prompt PowerPC Book-E Watchdog reset type + depends on BOOKE_WDT + default BOOKE_WDT_RESET_CHIP + help + Specify what kind of reset will be executed on watchdog timeout. + Seems to me it would be much better to make this configurable via platform data and/or device tree. good catch. What do the device-tree gurus think to be a nice property name? How about reset-type ? If that is not acceptable, a more device specific name might be booke-reset-type. I would suggest to write a patch including property description and copy devicetree-discuss when you submit it. If devicetree works for your application, I would not bother with platform data. Btw, do you have an opinion about the timeout ? I'd like to get rid of the configuration parameter, as it is quite confusing (try to match the parameter to seconds on a Freescale processor and you'll understand what I mean). I asked about it on the list a while ago, but did not get any feedback. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller
On Sat, Apr 27, 2013 at 04:03:31PM +0200, Simon Guinot wrote: On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: + /* +* Set default configuration values before passing the structure +* to OF helpers to overload them using those provided by .dts +* file (if any). Final config is then commited. +*/ + g762_config_init(conf); + g762_config_of_overload(client, conf); + err = g762_config_commit(client, conf); + if (err) + goto out; One more comment... Sorry for the multiple replies :) I am not sure that applying a configuration anyway is a good idea. I think it could be best to stick with the bootloader configuration if no platform data nor device tree data are given. Yes, good point. You are absolutely right. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 09/23 v2] watchdog: coh901327: devicetree support
On Fri, Apr 26, 2013 at 04:33:53PM +0200, Linus Walleij wrote: From: Linus Walleij linus.wall...@linaro.org This adds support for probing the COH 901 327 watchdog from the device tree and also adds associated bindings. Cc: Wim Van Sebroeck w...@iguana.be Signed-off-by: Linus Walleij linus.wall...@linaro.org Reviewed-by: Guenter Roeck li...@roeck-us.net ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv1 2/3] hwmon: Add documentation for g762 driver
On Wed, Apr 24, 2013 at 12:06:08AM +0200, Arnaud Ebalard wrote:
Signed-off-by: Arnaud Ebalard a...@natisbad.org
---
Documentation/hwmon/g762 | 67
++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/hwmon/g762
diff --git a/Documentation/hwmon/g762 b/Documentation/hwmon/g762
new file mode 100644
index 000..5015829
--- /dev/null
+++ b/Documentation/hwmon/g762
@@ -0,0 +1,67 @@
+Kernel driver g762
+==
+
+This documentation is based on G760a one, GMT G762 datasheet available
G760a ? If this refers to Documentation/hwmon/g760a, I think it is just
confusing.
+at http://natisbad.org/NAS/ref/GMT_EDS-762_763-080710-0.2.pdf (used
+for the development of the driver) and sysfs bindings described in
I would hope that you used the data sheet for the driver ;), but that
information does not add much if any value here. I would suggest
to drop it. Just point to the data sheet.
+Documentation/hwmon/sysfs-interface.
+
+The GMT G762 Fan Speed PWM Controller is connected directly to a fan
+and performs closed-loop or open-loop control of the fan speed. Two
+modes - PWM or linear - are supported by the device.
+
+The following entries are available to the user in a subdirectory of
+/sys/bus/i2c/drivers/g762/ to control the operation of the device.
+This can be done manually using the following entries but is usually
+done via a userland daemon like fancontrol.
+
+Note that those entries do not provide ways to setup the specific
+hardware characteristics of the system (reference clock, pulses per
+fan revolution, ...); Those can be modified via devicetree bindings
+documented in Documentation/devicetree/bindings/hwmon/g762.txt.
+
+
+ fan1_target: set desired fan speed. This only makes sense in closed-loop
+fan speed control (i.e. when pwm1_enable is set to 2).
+
+ fan1_input: provide current fan rotation value in RPM as reported by
+the fan to the device.
+
+ fan1_div: fan clock divisor. Supported value are 1, 2, 4 and 8. Default
+value is 1.
+
+ fan1_gear_mode: fan gear mode. Supported values are 0, 1 and 2. Default
+value is 0. The value is a characteristic of the system
+and a higher value can be set for more precisely control
+fans with a high rotation speed. Note that this affects the
+measurable speed range, not the read value.
+
This attribute no longer exists.
+ fan1_fault: reports fan failure, i.e. no transition on fan gear pin for
+about 0.7s (if the fan is not voluntarily set off).
+
+ fan1_alarm: in closed-loop control mode, if fan RPM value is 25% out
+of the programmed value for over 6 seconds 'fan1_alarm' is
+set to 1.
+
+ pwm1: get or set PWM fan control value. This is an integer value
+between 0 and 255. 0 stops the fan, 255 makes it run at
+full speed.
+
+ pwm1_enable: set current fan speed control mode i.e. 1 for manual fan
+speed control (open-loop), 2 for automatic fan speed control
+(closed-loop).
+
+ pwm1_mode: set or get fan driving mode: 1 for PWM mode, 0 for linear
+mode. Default is linear mode.
+
+
+In PWM mode ('pwm1_mode' set to 1), the fan speed is programmed either by
+setting a value between 0 and 255 via 'pwm1' entry (0 stops the fan, 255
+makes it run at full speed). This can also be done by passing the
+expected RPM value via 'fan1_target'. Current fan speed value can be
+retrieved via 'fan1_input'. This fan speed value is computed based on
+the parameters associated with the physical characteristics of the
+system: a reference clock source frequency, a number of pulses per fan
+revolution, etc.
+
+Note that the driver will update its values at most once per second.
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller
On Wed, Apr 24, 2013 at 10:28:47PM +0200, Arnaud Ebalard wrote: Hi, Guenter Roeck li...@roeck-us.net writes: You could consider using regmap for holding this cache. http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=now=900sidebar=yesbg=no#.UXdspHLQ5jM http://elinux.org/ELCE_Europe_2012_Presentations Interesting. As I am not yet familiar w/ regmap I would prefer having the driver accepted during merge window with current data structure and then convert it to regmap. But I will take a look (e.g. will study fca1dd03 for instance). Thanks for the pointers. I have not had time for a detailed review, but adding regmap support will not be a requirement. Note that it is too late for 3.10, so the driver will have to wait for 3.11. Well, I think I can live with it; The only bad thing is that it kills my excuses not to spend time on regmap ;-) Your call. +/* + * Helpers to import hardware characteristics from .dts file and overload + * default config values. + */ + +#ifdef CONFIG_OF Can the driver be used without device tree? Would it be simpler to just add depends OF in the Kconfig entry? It can be used if the default params (or those configured by u-boot I guess) fit your needs. I think it would be fairly easy to extend the driver later to expose g762_config struct to allow parameters to be set w/o using OF. If someone wants to do that, I think it is better to not depend on OF in Kconfig at the moment but I have not strong argument other that that one. I'll let you decide. Agreed. As long as there are major platforms not supporting device tree, and as long as device tree overlays are not supported, it must not be made mandatory. Especially for I2C and SPI devices I reserve the right to be able test the hardware on an X86 system and not require a reboot to do so. Understood. Following Simon's post, I think it's useful to implement some init function for non DT-enabled platforms. Sure, as long as Simon commits to test it and - if possible - to provide the necessary platform code. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller
On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote:
Signed-off-by: Arnaud Ebalard a...@natisbad.org
Tested-by: Arnaud Ebalard a...@natisbad.org
Tested-by is not needed here; I sure hope you tested your own code.
It is only used if someone else tested it.
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile |1 +
drivers/hwmon/g762.c | 1058
3 files changed, 1069 insertions(+)
create mode 100644 drivers/hwmon/g762.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 89ac1cb..cb4879c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -423,6 +423,16 @@ config SENSORS_G760A
This driver can also be built as a module. If so, the module
will be called g760a.
+config SENSORS_G762
+ tristate GMT G762 and G763
+ depends on I2C
+ help
+ If you say yes here you get support for Global Mixed-mode
+ Technology Inc G762 and G763 fan speed PWM controller chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called g762.
+
config SENSORS_GL518SM
tristate Genesys Logic GL518SM
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 8d6d97e..4b49504 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
obj-$(CONFIG_SENSORS_FAM15H_POWER) += fam15h_power.o
obj-$(CONFIG_SENSORS_FSCHMD) += fschmd.o
obj-$(CONFIG_SENSORS_G760A) += g760a.o
+obj-$(CONFIG_SENSORS_G762) += g762.o
obj-$(CONFIG_SENSORS_GL518SM)+= gl518sm.o
obj-$(CONFIG_SENSORS_GL520SM)+= gl520sm.o
obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
new file mode 100644
index 000..810b019
--- /dev/null
+++ b/drivers/hwmon/g762.c
@@ -0,0 +1,1058 @@
+/*
+ * g762 - Driver for the Global Mixed-mode Technology Inc. fan speed PWM
+ *controller chips from G762 family, i.e. G762 and G763
+ *
+ * Copyright (C) 2013, Arnaud EBALARD a...@natisbad.org
+ *
+ * This work is based on a basic version for 2.6.31 kernel developed
+ * by Olivier Mouchet for LaCie NAS. Updates have been performed to run
+ * on recent kernels. Additional features have been added. Ability to
+ * configure various characteristics via .dts file has been added.
+ * Detailed datasheet on which this development is based is available
+ * here:
+ *
+ * http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf
+ *
+ * Headers from previous developments have been kept below:
+ *
+ * Copyright (c) 2009 LaCie
+ *
+ * Author: Olivier Mouchet olivier.mouc...@gmail.com
+ *
+ * based on g760a code written by Herbert Valerio Riedel h...@gnu.org
+ * Copyright (C) 2007 Herbert Valerio Riedel h...@gnu.org
+ *
+ * g762: minimal datasheet available at:
+ * http://www.gmt.com.tw/product/datasheet/EDS-762_3.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
Please drop the address.
+ */
+
+#include linux/device.h
+#include linux/module.h
+#include linux/init.h
+#include linux/slab.h
Is this include needed ?
+#include linux/jiffies.h
+#include linux/i2c.h
+#include linux/hwmon.h
+#include linux/hwmon-sysfs.h
+#include linux/err.h
+#include linux/mutex.h
+#include linux/kernel.h
+#include linux/of.h
+#include linux/of_device.h
+
+#define DRVNAME g762
+
+static const struct i2c_device_id g762_id[] = {
+ { g762, 0 },
+ { g763, 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, g762_id);
+
+enum g762_regs {
+ G762_REG_SET_CNT = 0x00,
+ G762_REG_ACT_CNT = 0x01,
+ G762_REG_FAN_STA = 0x02,
+ G762_REG_SET_OUT = 0x03,
+ G762_REG_FAN_CMD1 = 0x04,
+ G762_REG_FAN_CMD2 = 0x05,
+};
+
+/* Config register bits */
+#define G762_REG_FAN_CMD1_DET_FAN_FAIL 0x80 /* enable fan_fail signal */
+#define G762_REG_FAN_CMD1_DET_FAN_OOC 0x40 /* enable fan_out_of_control */
+#define G762_REG_FAN_CMD1_OUT_MODE 0x20 /* out mode, pwm or dac */
+#define G762_REG_FAN_CMD1_FAN_MODE 0x10 /* fan mode: close or open loop
*/
+#define G762_REG_FAN_CMD1_CLK_DIV_ID1 0x08 /* clock divisor value */
+#define
Re: [PATCH 09/23] watchdog: coh901327: devicetree support
On Mon, Apr 22, 2013 at 11:55:05AM +0200, Linus Walleij wrote:
From: Linus Walleij linus.wall...@linaro.org
This adds support for probing the COH 901 327 watchdog from
the device tree and also adds associated bindings.
Cc: Wim Van Sebroeck w...@iguana.be
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
Hi Wim, I'm seeking an ACK on this patch to take it into
ARM SoC along with the patches making use of this new
binding.
---
.../devicetree/bindings/watchdog/stericsson-coh901327.txt | 15
+++
drivers/watchdog/coh901327_wdt.c | 6 ++
2 files changed, 21 insertions(+)
create mode 100644
Documentation/devicetree/bindings/watchdog/stericsson-coh901327.txt
diff --git
a/Documentation/devicetree/bindings/watchdog/stericsson-coh901327.txt
b/Documentation/devicetree/bindings/watchdog/stericsson-coh901327.txt
new file mode 100644
index 000..3d7c958
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/stericsson-coh901327.txt
@@ -0,0 +1,15 @@
+ST-Ericsson COH 901 327 Watchdog timer
+
+Required properties:
+- compatible: must be stericsson,coh901327.
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- interrupts: the interrupt used for the watchdog timeout warning.
+
+Example:
+
+watchdog: watchdog@c0012000 {
+ compatible = stericsson,coh901327;
+ reg = 0xc0012000 0x1000;
+ interrupts = 3;
+};
If you add a call to watchdog_init_timeout(), it would also support
setting the timeout through the timeout-sec property.
Guenter
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller
On Fri, Apr 19, 2013 at 01:30:51PM +0200, Arnaud Ebalard wrote: Hi, Andrew Lunn and...@lunn.ch writes: Hi Arnaud +static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, get_pwm, set_pwm); +static DEVICE_ATTR(pwm1_polarity, S_IWUSR | S_IRUGO, + get_pwm_polarity, set_pwm_polarity); +static DEVICE_ATTR(pwm1_mode, S_IWUSR | S_IRUGO, + get_pwm_mode, set_pwm_mode); +static DEVICE_ATTR(pwm1_enable, S_IWUSR | S_IRUGO, + get_speed_control_mode, set_speed_control_mode); +static DEVICE_ATTR(pwm1_freq, S_IWUSR | S_IRUGO, + get_pwm_freq, set_pwm_freq); + +static DEVICE_ATTR(fan1_input, S_IRUGO, get_fan_rpm, NULL); +static DEVICE_ATTR(fan1_alarm, S_IRUGO, get_fan_ooc, NULL); +static DEVICE_ATTR(fan1_alarm_detection, S_IWUSR | S_IRUGO, + get_fan_ooc_detection, set_fan_ooc_detection); +static DEVICE_ATTR(fan1_fault, S_IRUGO, get_fan_failure, NULL); +static DEVICE_ATTR(fan1_fault_detection, S_IWUSR | S_IRUGO, + get_fan_failure_detection, set_fan_failure_detection); +static DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO, + get_fan_target, set_fan_target); +static DEVICE_ATTR(fan1_pulses, S_IWUSR | S_IRUGO, + get_fan_pulses, set_fan_pulses); +static DEVICE_ATTR(fan1_div, S_IWUSR | S_IRUGO, + get_fan_clk_div, set_fan_clk_div); +static DEVICE_ATTR(fan1_gear_mode, S_IWUSR | S_IRUGO, + get_fan_gear_mode, set_fan_gear_mode); +static DEVICE_ATTR(fan1_startup_voltage, S_IWUSR | S_IRUGO, + get_fan_startup_voltage, set_fan_startup_voltage); It is normal to use SENSOR_ATTR, not DEVICE_ATTR. Take a look at the existing fan drivers. I will take a look and use SENSOR_ATTR if it is the expected way. For the records, the g760a.c which I used as basis uses DEVICE_ATTR ;-) You only need SENSOR_ATTR if you have an index parameter. Otherwise DEVICE_ATTR is just fine. I would actually reject the patch if I notice that it uses SENSOR_ATTR without need for a parameter. Besides, in the context used, it would probably be SENSOR_DEVICE_ATTR. I also think a lot of these are not needed. They are fixed properties of the board and cannot change dynamically. They are set once using DT and user space does not need to care. I added those knobs for a simple reason: I don't have all the various characteristics of the target platform I am working on (Netgear ReadyNAS Duo v2) and needed to be able to test various values w/o rebooting to get those rights. I thought this knobs would be useful for people with the same issue (for instance, the new ReadyNAS 102 also has a G762) and would not hurt. Testing is not a reason to add non-standard attributes. Thanks, Guenter Anyway, I will split the patch in two parts and keep the useless knobs on my side. Thanks for your feedback, Cheers, a+ ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller
On Fri, Apr 19, 2013 at 12:28:21AM +0200, Arnaud Ebalard wrote:
Signed-off-by: Arnaud Ebalard a...@natisbad.org
Tested-by: Arnaud Ebalard a...@natisbad.org
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile |1 +
drivers/hwmon/g762.c | 1159
3 files changed, 1170 insertions(+)
create mode 100644 drivers/hwmon/g762.c
checkpatch says:
total: 1 errors, 15 warnings, 0 checks, 1182 lines checked
Please fix those. Also, please make sure there are spaces around operators.
Additional comments inline. This is not a complete review; I gave up after
a while. Please fix the problems and resubmit.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 89ac1cb..0c6ddee 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -423,6 +423,16 @@ config SENSORS_G760A
This driver can also be built as a module. If so, the module
will be called g760a.
+config SENSORS_G762
+ tristate GMT G762
+ depends on I2C
+ help
+ If you say yes here you get support for Global Mixed-mode
+ Technology Inc G762 fan speed PWM controller chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called g762a.
+
config SENSORS_GL518SM
tristate Genesys Logic GL518SM
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 8d6d97e..4b49504 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
obj-$(CONFIG_SENSORS_FAM15H_POWER) += fam15h_power.o
obj-$(CONFIG_SENSORS_FSCHMD) += fschmd.o
obj-$(CONFIG_SENSORS_G760A) += g760a.o
+obj-$(CONFIG_SENSORS_G762) += g762.o
obj-$(CONFIG_SENSORS_GL518SM)+= gl518sm.o
obj-$(CONFIG_SENSORS_GL520SM)+= gl520sm.o
obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
new file mode 100644
index 000..8c4cb39
--- /dev/null
+++ b/drivers/hwmon/g762.c
@@ -0,0 +1,1159 @@
+/*
+ * g762 - Driver for the Global Mixed-mode Technology Inc. fan speed PWM
+ *controller chips from G762 family, i.e. G762 and G763
+ *
+ * Copyright (C) 2013, Arnaud EBALARD a...@natisbad.org
+ *
+ * This work is based on a basic version for 2.6.31 kernel developed
+ * by Olivier Mouchet for LaCie NAS. Updates have been performed to run
+ * on recent kernels. Supported has been completed and additional
+ * features added: ability to configure various characteristics from
+ * .dts file, better initialization, alarms and error reporting support,
+ * gear mode, polarity, fan pulse per revolution, fan startup voltage
+ * control. The following detailed datasheet has been used as a basis
+ * for this work:
+ *
+ * http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf
+ *
+ * Headers from previous developments have been kept below:
+ *
+ * Copyright (c) 2009 LaCie
+ *
+ * Author: Olivier Mouchet olivier.mouc...@gmail.com
+ *
+ * based on g760a code written by Herbert Valerio Riedel h...@gnu.org
+ * Copyright (C) 2007 Herbert Valerio Riedel h...@gnu.org
+ *
+ * g762: minimal datasheet available at:
+ * http://www.gmt.com.tw/product/datasheet/EDS-762_3.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include linux/device.h
+#include linux/module.h
+#include linux/init.h
+#include linux/slab.h
+#include linux/jiffies.h
+#include linux/i2c.h
+#include linux/hwmon.h
+#include linux/hwmon-sysfs.h
+#include linux/err.h
+#include linux/mutex.h
+#include linux/kernel.h
+#include linux/of.h
+#include linux/of_device.h
+
+#define DRVNAME g762
+
+static const struct i2c_device_id g762_id[] = {
+ { DRVNAME, 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, g762_id);
+
+enum g762_regs {
+ G762_REG_SET_CNT = 0x00,
+ G762_REG_ACT_CNT = 0x01,
+ G762_REG_FAN_STA = 0x02,
+ G762_REG_SET_OUT = 0x03,
+ G762_REG_FAN_CMD1 = 0x04,
+ G762_REG_FAN_CMD2 = 0x05,
+};
+
+/* Config register bits */
+#define G762_REG_FAN_CMD1_DET_FAN_FAIL 0x80 /* enable fan_fail signal
*/
+#define G762_REG_FAN_CMD1_DET_FAN_OOC0x40 /* enable
fan_out_of_control */
+#define
Re: [PATCH v5 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver
On Tue, Apr 16, 2013 at 11:36:33AM +0200, Wolfram Sang wrote: Doug, [ ... ] callenge response? ... diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c new file mode 100644 index 000..bda020a --- /dev/null +++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c @@ -0,0 +1,252 @@ +/* + * GPIO-based I2C Arbitration callenge response? s/callenge/challenge/g ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] watchdog: Revert the AT91RM9200_WATCHDOG dependency
On Wed, Apr 10, 2013 at 02:36:22PM +0200, Nicolas Ferre wrote: Compiling the at91rm9200_wdt.c driver without at91rm9200 support was leading to several errors: drivers/built-in.o: In function `at91_wdt_close': at91_adc.c:(.text+0xc9fe4): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91_wdt_write': at91_adc.c:(.text+0xca004): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91wdt_shutdown': at91_adc.c:(.text+0xca01c): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91wdt_suspend': at91_adc.c:(.text+0xca038): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91_wdt_open': at91_adc.c:(.text+0xca0cc): undefined reference to `at91_st_base' drivers/built-in.o:at91_adc.c:(.text+0xca2c8): more undefined references to `at91_st_base' follow So, reverting the modification of the depends Kconfig line introduced by patch a6a1bcd37 (watchdog: at91rm9200: add DT support) seems to be the good solution. Really ? Why ? I mean, this was supposed to be for at91rm9200, wasn't it ? And why would want try to compile a watchdog for at91rm9200 without at91rm9200 support ? I understand there is a problem, I just don't see how removing that line would solgve it. Guenter Signed-off-by: Nicolas Ferre nicolas.fe...@atmel.com --- drivers/watchdog/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 9fcc70c..e89fc31 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -117,7 +117,7 @@ config ARM_SP805_WATCHDOG config AT91RM9200_WATCHDOG tristate AT91RM9200 watchdog - depends on ARCH_AT91 + depends on ARCH_AT91RM9200 help Watchdog timer embedded into AT91RM9200 chips. This will reboot your system when the timeout is reached. -- 1.8.0 -- To unsubscribe from this list: send the line unsubscribe linux-watchdog in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] watchdog: Revert the AT91RM9200_WATCHDOG dependency
On Wed, Apr 10, 2013 at 06:33:51AM -0700, Guenter Roeck wrote: On Wed, Apr 10, 2013 at 02:36:22PM +0200, Nicolas Ferre wrote: Compiling the at91rm9200_wdt.c driver without at91rm9200 support was leading to several errors: drivers/built-in.o: In function `at91_wdt_close': at91_adc.c:(.text+0xc9fe4): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91_wdt_write': at91_adc.c:(.text+0xca004): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91wdt_shutdown': at91_adc.c:(.text+0xca01c): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91wdt_suspend': at91_adc.c:(.text+0xca038): undefined reference to `at91_st_base' drivers/built-in.o: In function `at91_wdt_open': at91_adc.c:(.text+0xca0cc): undefined reference to `at91_st_base' drivers/built-in.o:at91_adc.c:(.text+0xca2c8): more undefined references to `at91_st_base' follow So, reverting the modification of the depends Kconfig line introduced by patch a6a1bcd37 (watchdog: at91rm9200: add DT support) seems to be the good solution. Really ? Why ? I mean, this was supposed to be for at91rm9200, wasn't it ? And why would want try to compile a watchdog for at91rm9200 without at91rm9200 support ? I understand there is a problem, I just don't see how removing that line would solve it. Me confused, sorry. I somehow thought you were removing the ARCH_AT91RM9200 dependency, not adding it. Acked-by: Guenter Roeck li...@roeck-us.net Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [v5] clk: add si5351 i2c common clock driver
On Mon, Apr 08, 2013 at 08:11:38AM +0200, Sebastian Hesselbarth wrote:
On 04/08/2013 02:17 AM, Guenter Roeck wrote:
On Mon, Apr 08, 2013 at 01:49:24AM +0200, Sebastian Hesselbarth wrote:
On 04/08/2013 12:50 AM, Guenter Roeck wrote:
On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver supports
DT kernels only and VXCO feature of si5351b is not implemented. DT
bindings selectively allow to overwrite stored Si5351 configuration
which is very helpful for clock generators with empty eeprom
configuration. Corresponding device tree binding documentation is
also added.
Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
Tested-by: Daniel Mackzon...@gmail.com
[ ... ]
+static inline void _si5351_msynth_set_pll_master(
+ struct si5351_driver_data *drvdata, unsigned char num, int is_master)
+{
+ unsigned long flags;
+
+ if (num 8 ||
+ (drvdata-variant == SI5351_VARIANT_A3 num 3))
+ return;
+
+ flags = __clk_get_flags(drvdata-msynth[num].hw.clk);
+ if (is_master)
+ flags |= CLK_SET_RATE_PARENT;
+ else
+ flags= ~CLK_SET_RATE_PARENT;
+ __clk_set_flags(drvdata-msynth[num].hw.clk, flags);
+}
+
Unless I am missing something, neither __clk_get_flags() nor the new
__clk_set_flags is exported.
Did you try to build and load the driver as module ?
Well, good catch. I didn't try to build v5 as a module, but I guess it
will fail. But I consider this as something that has to be addressed in
clk framework itself, not in this patch. There will be other
clk-providers built as module in the future for sure.
Sure, but you provided the patch to make __clk_set_flags global. To avoid
build failures, I would suggest to either submit a patch to export the
missing functions, or to remove the ability to build the driver as module.
Actually, I knew that __clk_set_flags patch will not be accepted
before posting it ;)
Ah, but part of that is to get you to think about it again, and to defend it if
it is really needed. After all, it can be abused applies to pretty much every
API.
Key question is if you _really_ need run-time flag modifications, or if you can
live with initialization-time settings. If you really need it, you'll have to
explain the reasons.
On a side note, do you happen to know anyone working on drivers for Si5319 or
Si5368 ?
No.
Too bad ... I may have to write that code myself then.
Thanks,
Guenter
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] clk: add si5351 i2c common clock driver
On Mon, Apr 08, 2013 at 06:46:57PM +0200, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver does not
support VXCO feature of si5351b. Passing platform_data or DT bindings
selectively allow to overwrite stored Si5351 configuration which is
very helpful for clock generators with empty eeprom configuration.
Corresponding device tree binding documentation is also added.
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
[ ... ]
+
+/*
+ * Si5351 i2c probe and DT
+ */
+#ifdef CONFIG_OF
+static const struct of_device_id si5351_dt_ids[] = {
+ { .compatible = silabs,si5351a, .data = (void *)SI5351_VARIANT_A, },
+ { .compatible = silabs,si5351a-msop,
+ .data = (void *)SI5351_VARIANT_A3, },
+ { .compatible = silabs,si5351b, .data = (void *)SI5351_VARIANT_B, },
+ { .compatible = silabs,si5351c, .data = (void *)SI5351_VARIANT_C, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, si5351_dt_ids);
+
+static int si5351_dt_parse(struct i2c_client *client)
+{
+ struct device_node *child, *np = client-dev.of_node;
+ struct si5351_platform_data *pdata;
+ const struct of_device_id *match;
+ struct property *prop;
+ const __be32 *p;
+ int num = 0;
+ u32 val;
+
+ if (np == NULL)
+ return 0;
+
+ match = of_match_node(si5351_dt_ids, np);
+ if (match == NULL)
+ return -EINVAL;
+
+ pdata = devm_kzalloc(client-dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ pdata-variant = (enum si5351_variant)match-data;
+ pdata-clk_xtal = of_clk_get(np, 0);
+ if (!IS_ERR(pdata-clk_xtal))
+ clk_put(pdata-clk_xtal);
+ pdata-clk_clkin = of_clk_get(np, 1);
+ if (!IS_ERR(pdata-clk_clkin))
+ clk_put(pdata-clk_clkin);
+
+ /*
+ * property silabs,pll-source : num src, [..]
+ * allow to selectively set pll source
+ */
+ of_property_for_each_u32(np, silabs,pll-source, prop, p, num) {
+ if (num = 2) {
+ dev_err(client-dev,
+ invalid pll %d on pll-source prop\n, num);
+ break;
You report dev_err here, but you don't return an error to the caller.
Is this on purpose ? If it is not a fatal error, maybe it should be dev_info ?
+ }
+
+ p = of_prop_next_u32(prop, p, val);
+ if (!p)
+ break;
+
+ switch (val) {
+ case 0:
+ pdata-pll_src[num] = SI5351_PLL_SRC_XTAL;
+ break;
+ case 1:
+ pdata-pll_src[num] = SI5351_PLL_SRC_CLKIN;
+ break;
+ default:
+ dev_warn(client-dev,
+ invalid parent %d for pll %d\n, val, num);
+ continue;
Same here, and a couple of times below. Given the context, I think it would
be better if the error cases would return an error. After all, the condition
suggests that the device tree data is wrong, meaning one has to assume it
won't work anyway and should be fixed in the device tree and not be ignored
by the driver.
+ }
+ }
+
+ /* per clkout properties */
+ num = of_get_child_count(np);
+
+ if (num == 0)
+ return 0;
+
This doesn't set client-dev.platform_data yet returns no error, meaning the
calling code will either use provided platform data or fail after all if it is
NULL. That seems to be inconsistent, given that a dt entry was already detected.
The user might end up wondering why the provided device tree data is not used,
not realizing that it is incomplete.
If children are not mandatory, you could just drop the code above and go through
for_each_child_of_node() anyway; it won't do anything and set
client-dev.platform_data at the end. If children are mandatory, it might make
sense to return an eror here (if there is dt information, it should be complete
and consistent).
+ for_each_child_of_node(np, child) {
+ if (of_property_read_u32(child, reg, num)) {
+ dev_err(client-dev, missing reg property of %s\n,
+ child-name);
+ continue;
+ }
+
What if num returns 100 ? Or 1000 ?
+ if (!of_property_read_u32(child, silabs,multisynth-source,
+ val)) {
+ switch (val) {
+ case 0:
+ pdata-clkout[num].multisynth_src =
+ SI5351_MULTISYNTH_SRC_VCO0;
+ break;
+ case 1:
+ pdata-clkout[num].multisynth_src =
+
Re: [v5] clk: add si5351 i2c common clock driver
On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver supports
DT kernels only and VXCO feature of si5351b is not implemented. DT
bindings selectively allow to overwrite stored Si5351 configuration
which is very helpful for clock generators with empty eeprom
configuration. Corresponding device tree binding documentation is
also added.
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Tested-by: Daniel Mack zon...@gmail.com
[ ... ]
+static inline void _si5351_msynth_set_pll_master(
+ struct si5351_driver_data *drvdata, unsigned char num, int is_master)
+{
+ unsigned long flags;
+
+ if (num 8 ||
+ (drvdata-variant == SI5351_VARIANT_A3 num 3))
+ return;
+
+ flags = __clk_get_flags(drvdata-msynth[num].hw.clk);
+ if (is_master)
+ flags |= CLK_SET_RATE_PARENT;
+ else
+ flags = ~CLK_SET_RATE_PARENT;
+ __clk_set_flags(drvdata-msynth[num].hw.clk, flags);
+}
+
Unless I am missing something, neither __clk_get_flags() nor the new
__clk_set_flags is exported.
Did you try to build and load the driver as module ?
Thanks,
Guenter
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [v5] clk: add si5351 i2c common clock driver
On Mon, Apr 08, 2013 at 01:49:24AM +0200, Sebastian Hesselbarth wrote:
On 04/08/2013 12:50 AM, Guenter Roeck wrote:
On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote:
This patch adds a common clock driver for Silicon Labs Si5351a/b/c
i2c programmable clock generators. Currently, the driver supports
DT kernels only and VXCO feature of si5351b is not implemented. DT
bindings selectively allow to overwrite stored Si5351 configuration
which is very helpful for clock generators with empty eeprom
configuration. Corresponding device tree binding documentation is
also added.
Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com
Tested-by: Daniel Mackzon...@gmail.com
[ ... ]
+static inline void _si5351_msynth_set_pll_master(
+ struct si5351_driver_data *drvdata, unsigned char num, int is_master)
+{
+ unsigned long flags;
+
+ if (num 8 ||
+ (drvdata-variant == SI5351_VARIANT_A3 num 3))
+ return;
+
+ flags = __clk_get_flags(drvdata-msynth[num].hw.clk);
+ if (is_master)
+ flags |= CLK_SET_RATE_PARENT;
+ else
+ flags= ~CLK_SET_RATE_PARENT;
+ __clk_set_flags(drvdata-msynth[num].hw.clk, flags);
+}
+
Unless I am missing something, neither __clk_get_flags() nor the new
__clk_set_flags is exported.
Did you try to build and load the driver as module ?
Well, good catch. I didn't try to build v5 as a module, but I guess it
will fail. But I consider this as something that has to be addressed in
clk framework itself, not in this patch. There will be other
clk-providers built as module in the future for sure.
Sure, but you provided the patch to make __clk_set_flags global. To avoid
build failures, I would suggest to either submit a patch to export the
missing functions, or to remove the ability to build the driver as module.
On a side note, do you happen to know anyone working on drivers for Si5319 or
Si5368 ?
Thanks,
Guenter
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 3/5] net/cadence/macb: Simplify OF dependencies
With of_get_mac_address() and of_get_phy_mode() now defined as dummy
functions if OF_NET is not configured, it is no longer necessary to
provide OF dependent functions as front-end. Also, the two functions
depend on OF_NET, not on OF, so the conditional code was not correct
anyway.
Drop the front-end functions and call of_get_mac_address() and
of_get_phy_mode() directly instead.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/cadence/macb.c | 43 +--
1 file changed, 6 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c
b/drivers/net/ethernet/cadence/macb.c
index 7903943..b2a9d20 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1472,41 +1472,7 @@ static const struct of_device_id macb_dt_ids[] = {
{ .compatible = cdns,gem },
{ /* sentinel */ }
};
-
MODULE_DEVICE_TABLE(of, macb_dt_ids);
-
-static int macb_get_phy_mode_dt(struct platform_device *pdev)
-{
- struct device_node *np = pdev-dev.of_node;
-
- if (np)
- return of_get_phy_mode(np);
-
- return -ENODEV;
-}
-
-static int macb_get_hwaddr_dt(struct macb *bp)
-{
- struct device_node *np = bp-pdev-dev.of_node;
- if (np) {
- const char *mac = of_get_mac_address(np);
- if (mac) {
- memcpy(bp-dev-dev_addr, mac, ETH_ALEN);
- return 0;
- }
- }
-
- return -ENODEV;
-}
-#else
-static int macb_get_phy_mode_dt(struct platform_device *pdev)
-{
- return -ENODEV;
-}
-static int macb_get_hwaddr_dt(struct macb *bp)
-{
- return -ENODEV;
-}
#endif
static int __init macb_probe(struct platform_device *pdev)
@@ -1519,6 +1485,7 @@ static int __init macb_probe(struct platform_device *pdev)
u32 config;
int err = -ENXIO;
struct pinctrl *pinctrl;
+ const char *mac;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
@@ -1592,11 +1559,13 @@ static int __init macb_probe(struct platform_device
*pdev)
config |= macb_dbw(bp);
macb_writel(bp, NCFGR, config);
- err = macb_get_hwaddr_dt(bp);
- if (err 0)
+ mac = of_get_mac_address(pdev-dev.of_node);
+ if (mac)
+ memcpy(bp-dev-dev_addr, mac, ETH_ALEN);
+ else
macb_get_hwaddr(bp);
- err = macb_get_phy_mode_dt(pdev);
+ err = of_get_phy_mode(pdev-dev.of_node);
if (err 0) {
pdata = pdev-dev.platform_data;
if (pdata pdata-is_rmii)
--
1.7.9.7
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 0/5] Provide empty functions if OF_NET is not configured
Provide empty functions for of_get_phy_mode() and of_get_mac_address() if OF_NET is not configured. Modify affected drivers to rely on the now available functions. Guenter Roeck (5): of_net.h: Provide dummy functions if OF_NET is not configured net/cadence/at91_ether: Simplify OF dependencies net/cadence/macb: Simplify OF dependencies net/freescale/fec: Simplify OF dependencies net/nxp/lpc_eth: Drop ifdef CONFIG_OF_NET drivers/net/ethernet/cadence/at91_ether.c | 44 - drivers/net/ethernet/cadence/macb.c | 43 drivers/net/ethernet/freescale/fec.c | 19 + drivers/net/ethernet/nxp/lpc_eth.c|2 -- include/linux/of_net.h| 10 +++ 5 files changed, 23 insertions(+), 95 deletions(-) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 1/5] of_net.h: Provide empty functions if OF_NET is not configured
of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
is configured. While most callers check for the define, not all do, and those
who do require #ifdef around the code. For those who don't, the missing check
can result in errors such as
arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
Provide empty functions if OF_NET is not configured.
This is safe because all callers do check the return values.
Cc: David Daney david.da...@cavium.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
Acked-by: Rob Herring rob.herr...@calxeda.com
---
include/linux/of_net.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index f474641..61bf53b 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,16 @@
#include linux/of.h
extern const int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
+#else
+static inline const int of_get_phy_mode(struct device_node *np)
+{
+ return -ENODEV;
+}
+
+static inline const void *of_get_mac_address(struct device_node *np)
+{
+ return NULL;
+}
#endif
#endif /* __LINUX_OF_NET_H */
--
1.7.9.7
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 2/5] net/cadence/at91_ether: Simplify OF dependencies
With of_get_mac_address() and of_get_phy_mode() now defined as dummy
functions if OF_NET is not configured, it is no longer necessary to
provide OF dependent functions as front-end. Also, the two functions
depend on OF_NET, not on OF, so the conditional code was not correct
anyway.
Drop the front-end functions and call of_get_mac_address() and
of_get_phy_mode() directly instead.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/cadence/at91_ether.c | 44 -
1 file changed, 6 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/cadence/at91_ether.c
b/drivers/net/ethernet/cadence/at91_ether.c
index 3becdb2..50565cb 100644
--- a/drivers/net/ethernet/cadence/at91_ether.c
+++ b/drivers/net/ethernet/cadence/at91_ether.c
@@ -303,42 +303,7 @@ static const struct of_device_id at91ether_dt_ids[] = {
{ .compatible = cdns,emac },
{ /* sentinel */ }
};
-
MODULE_DEVICE_TABLE(of, at91ether_dt_ids);
-
-static int at91ether_get_phy_mode_dt(struct platform_device *pdev)
-{
- struct device_node *np = pdev-dev.of_node;
-
- if (np)
- return of_get_phy_mode(np);
-
- return -ENODEV;
-}
-
-static int at91ether_get_hwaddr_dt(struct macb *bp)
-{
- struct device_node *np = bp-pdev-dev.of_node;
-
- if (np) {
- const char *mac = of_get_mac_address(np);
- if (mac) {
- memcpy(bp-dev-dev_addr, mac, ETH_ALEN);
- return 0;
- }
- }
-
- return -ENODEV;
-}
-#else
-static int at91ether_get_phy_mode_dt(struct platform_device *pdev)
-{
- return -ENODEV;
-}
-static int at91ether_get_hwaddr_dt(struct macb *bp)
-{
- return -ENODEV;
-}
#endif
/* Detect MAC PHY and perform ethernet interface initialization */
@@ -352,6 +317,7 @@ static int __init at91ether_probe(struct platform_device
*pdev)
struct macb *lp;
int res;
u32 reg;
+ const char *mac;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs)
@@ -403,11 +369,13 @@ static int __init at91ether_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, pdev-dev);
- res = at91ether_get_hwaddr_dt(lp);
- if (res 0)
+ mac = of_get_mac_address(pdev-dev.of_node);
+ if (mac)
+ memcpy(lp-dev-dev_addr, mac, ETH_ALEN);
+ else
macb_get_hwaddr(lp);
- res = at91ether_get_phy_mode_dt(pdev);
+ res = of_get_phy_mode(pdev-dev.of_node);
if (res 0) {
if (board_data board_data-is_rmii)
lp-phy_interface = PHY_INTERFACE_MODE_RMII;
--
1.7.9.7
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 4/5] net/freescale/fec: Simplify OF dependencies
Since of_get_mac_address() is now defined even if CONFIG_OF_NET
is not configured, the ifdef around the code calling it is no longer
necessary and can be removed.
Similar, since of_get_phy_mode() is now defined as dummy function
if OF_NET is not configured, it is no longer necessary to provide
an OF dependent function as front-end. Also, the function depends
on OF_NET, not on OF, so the conditional code was not correct anyway.
Drop the front-end function and call of_get_phy_mode() directly.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/freescale/fec.c | 19 +--
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c
b/drivers/net/ethernet/freescale/fec.c
index 911d025..d374a94 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -868,7 +868,6 @@ static void fec_get_mac(struct net_device *ndev)
*/
iap = macaddr;
-#ifdef CONFIG_OF
/*
* 2) from device tree data
*/
@@ -880,7 +879,6 @@ static void fec_get_mac(struct net_device *ndev)
iap = (unsigned char *) mac;
}
}
-#endif
/*
* 3) from flash or fuse (via platform data)
@@ -1666,16 +1664,6 @@ static int fec_enet_init(struct net_device *ndev)
}
#ifdef CONFIG_OF
-static int fec_get_phy_mode_dt(struct platform_device *pdev)
-{
- struct device_node *np = pdev-dev.of_node;
-
- if (np)
- return of_get_phy_mode(np);
-
- return -ENODEV;
-}
-
static void fec_reset_phy(struct platform_device *pdev)
{
int err, phy_reset;
@@ -1704,11 +1692,6 @@ static void fec_reset_phy(struct platform_device *pdev)
gpio_set_value(phy_reset, 1);
}
#else /* CONFIG_OF */
-static int fec_get_phy_mode_dt(struct platform_device *pdev)
-{
- return -ENODEV;
-}
-
static void fec_reset_phy(struct platform_device *pdev)
{
/*
@@ -1773,7 +1756,7 @@ fec_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ndev);
- ret = fec_get_phy_mode_dt(pdev);
+ ret = of_get_phy_mode(pdev-dev.of_node);
if (ret 0) {
pdata = pdev-dev.platform_data;
if (pdata)
--
1.7.9.7
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 5/5] net/nxp/lpc_eth: Drop ifdef CONFIG_OF_NET
Since of_get_mac_address() is now declared even if CONFIG_OF_NET
is not configured, the ifdef is no longer necessary and can be
removed.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/net/ethernet/nxp/lpc_eth.c |2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
b/drivers/net/ethernet/nxp/lpc_eth.c
index efa29b7..99276d7 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -1434,13 +1434,11 @@ static int lpc_eth_drv_probe(struct platform_device
*pdev)
/* Get MAC address from current HW setting (POR state is all zeros) */
__lpc_get_mac(pldat, ndev-dev_addr);
-#ifdef CONFIG_OF_NET
if (!is_valid_ether_addr(ndev-dev_addr)) {
const char *macaddr = of_get_mac_address(pdev-dev.of_node);
if (macaddr)
memcpy(ndev-dev_addr, macaddr, ETH_ALEN);
}
-#endif
if (!is_valid_ether_addr(ndev-dev_addr))
eth_hw_addr_random(ndev);
--
1.7.9.7
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
is configured. While most callers check for the define, not all do, and those
who do require #ifdef around the code. For those who don't, the missing check
can result in errors such as
arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
Provide dummy function declarations if OF_NET is not configured. This is safe
because all callers do check the return values. If desired, at least some of
the #ifdefs in the code can subsequently be removed.
Cc: David Daney david.da...@cavium.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
include/linux/of_net.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index f474641..61bf53b 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,16 @@
#include linux/of.h
extern const int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
+#else
+static inline const int of_get_phy_mode(struct device_node *np)
+{
+ return -ENODEV;
+}
+
+static inline const void *of_get_mac_address(struct device_node *np)
+{
+ return NULL;
+}
#endif
#endif /* __LINUX_OF_NET_H */
--
1.7.9.7
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
On Mon, Apr 01, 2013 at 01:44:24PM -0500, Rob Herring wrote:
On 04/01/2013 01:19 PM, Guenter Roeck wrote:
of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
is configured. While most callers check for the define, not all do, and
those
who do require #ifdef around the code. For those who don't, the missing
check
can result in errors such as
How about removing the ifdef from those callers?
That would be the next step, after/if this one is accepted.
If not, it doesn't make sense to waste my time.
Thanks,
Guenter
Rob
arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
function 'of_get_mac_address' [-Werror=implicit-function-declaration]
Provide dummy function declarations if OF_NET is not configured. This is
safe
because all callers do check the return values. If desired, at least some of
the #ifdefs in the code can subsequently be removed.
Cc: David Daney david.da...@cavium.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
include/linux/of_net.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index f474641..61bf53b 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,16 @@
#include linux/of.h
extern const int of_get_phy_mode(struct device_node *np);
extern const void *of_get_mac_address(struct device_node *np);
+#else
+static inline const int of_get_phy_mode(struct device_node *np)
+{
+ return -ENODEV;
+}
+
+static inline const void *of_get_mac_address(struct device_node *np)
+{
+ return NULL;
+}
#endif
#endif /* __LINUX_OF_NET_H */
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v10 04/12] watchdog: add Palmas Watchdog support
On Fri, Mar 22, 2013 at 02:55:14PM +, Ian Lartey wrote:
From: Graeme Gregory g...@slimlogic.co.uk
Add support for the Palmas watchdog timer which has a timeout configurable
from 1s to 128s.
Signed-off-by: Graeme Gregory g...@slimlogic.co.uk
Signed-off-by: Ian Lartey i...@slimlogic.co.uk
---
drivers/watchdog/palmas_wdt.c | 169
+
1 files changed, 169 insertions(+), 0 deletions(-)
create mode 100644 drivers/watchdog/palmas_wdt.c
diff --git a/drivers/watchdog/palmas_wdt.c b/drivers/watchdog/palmas_wdt.c
new file mode 100644
index 000..da7e379
--- /dev/null
+++ b/drivers/watchdog/palmas_wdt.c
@@ -0,0 +1,169 @@
+/*
+ * Driver for Watchdog part of Palmas PMIC Chips
+ *
+ * Copyright 2011-2013 Texas Instruments Inc.
+ *
+ * Author: Graeme Gregory g...@slimlogic.co.uk
+ * Author: Ian Lartey i...@slimlogic.co.uk
+ *
+ * Based on twl4030_wdt.c
+ *
+ * Author: Timo Kokkonen timo.t.kokkonen at nokia.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include linux/module.h
+#include linux/types.h
+#include linux/slab.h
+#include linux/kernel.h
+#include linux/fs.h
+#include linux/watchdog.h
+#include linux/platform_device.h
+#include linux/watchdog.h
+#include linux/mfd/palmas.h
+
+struct palmas_wdt {
+ struct palmas *palmas;
+ struct watchdog_device wdt;
+ struct device *dev;
+
+ int timer_margin;
+};
+
+
One empty line would be sufficient.
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, Watchdog cannot be stopped once started
+ (default= __MODULE_STRING(WATCHDOG_NOWAYOUT) ));
+
+static int palmas_wdt_write(struct palmas *palmas, unsigned int data)
+{
+ unsigned int addr;
+
+ addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_WATCHDOG);
+
+ return palmas_write(palmas, PALMAS_PMU_CONTROL_BASE, addr, addr);
2 x addr ? Should the second one (last parameter) be data ?
+}
+
+static int palmas_wdt_enable(struct watchdog_device *wdt)
+{
+ struct palmas_wdt *driver_data = watchdog_get_drvdata(wdt);
+ struct palmas *palmas = driver_data-palmas;
+
+ return palmas_wdt_write(palmas, driver_data-timer_margin |
+ PALMAS_WATCHDOG_ENABLE);
+}
+
+static int palmas_wdt_disable(struct watchdog_device *wdt)
+{
+ struct palmas_wdt *driver_data = watchdog_get_drvdata(wdt);
+ struct palmas *palmas = driver_data-palmas;
+
+ return palmas_wdt_write(palmas, driver_data-timer_margin);
Just wondering - why not just write 0 ?
+}
+
+static int palmas_wdt_set_timeout(struct watchdog_device *wdt, unsigned
timeout)
+{
+ struct palmas_wdt *driver_data = watchdog_get_drvdata(wdt);
+
+ if (timeout 1 || timeout 128) {
+ dev_warn(driver_data-dev,
+ Timeout can only be in the range [1-128] seconds);
+ return -EINVAL;
+ }
The watchdog core supports limit checking. Might as well use it.
On a side note, it might be an interesting experience if you try setting
a value of 0 or larger than 128. Unless I am missing something,
driver_data-dev is never initialized.
+ driver_data-timer_margin = fls(timeout) - 1;
+ return 0;
+}
+
+static const struct watchdog_info palmas_wdt_info = {
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+ .identity = Palmas Watchdog,
+ .firmware_version = 0,
+};
+
+static const struct watchdog_ops palmas_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = palmas_wdt_enable,
+ .stop = palmas_wdt_disable,
+ .ping = palmas_wdt_enable,
+ .set_timeout = palmas_wdt_set_timeout,
+};
+
+static int palmas_wdt_probe(struct platform_device *pdev)
+{
+ struct palmas *palmas = dev_get_drvdata(pdev-dev.parent);
+ struct palmas_wdt *driver_data;
+ struct watchdog_device *palmas_wdt;
+ int ret = 0;
+
+ driver_data = devm_kzalloc(pdev-dev, sizeof(*driver_data),
+GFP_KERNEL);
+ if (!driver_data) {
+ dev_err(pdev-dev, Unable to alloacate watchdog device\n);
AFAIK devm_ functions already issue a message on errors, so it is unnecessary
to issue another one here.
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ driver_data-palmas = palmas;
+
+ palmas_wdt = driver_data-wdt;
+
+ palmas_wdt-info = palmas_wdt_info;
+ palmas_wdt-ops = palmas_wdt_ops;
+ watchdog_set_nowayout(palmas_wdt, nowayout);
+ watchdog_set_drvdata(palmas_wdt, driver_data);
+
There is no call to watchdog_init_timeout, and the timeout value is not
initialized
in palmas_wdt. So the driver depends on
Re: [PATCH v10 05/12] watchdog: Kconfig for Palmas watchdog
On Fri, Mar 22, 2013 at 02:55:15PM +, Ian Lartey wrote: Add the Kconfig and Makefile for the Palmas watchdog driver Signed-off-by: Ian Lartey i...@slimlogic.co.uk Signed-off-by: Graeme Gregory g...@slimlogic.co.uk --- I think this patch should be merged with the previous one. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4] hwmon: ntc: Add DT with IIO support to NTC thermistor driver
On Wed, Mar 13, 2013 at 01:52:35PM -0700, Doug Anderson wrote: Hi, On Tue, Mar 12, 2013 at 9:08 PM, Naveen Krishna Chatradhi ch.nav...@samsung.com wrote: This patch adds DT support to NTC driver to parse the platform data. Also adds the support to work as an iio device. During the probe ntc driver gets the respective channels of ADC and uses iio_raw_read calls to get the ADC converted value. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com --- Changes since v3: 1. Added a NULL check before iio_channel_release 2. Modified a return statement Guenter Roeck, Its wonderful working with you. Thanks. .../devicetree/bindings/hwmon/ntc_thermistor.txt | 29 drivers/hwmon/Kconfig |1 + drivers/hwmon/ntc_thermistor.c | 145 +--- include/linux/platform_data/ntc_thermistor.h |8 +- 4 files changed, 163 insertions(+), 20 deletions(-) I haven't done a full review of this code, but it I built it in (along with other CLs to enable it on exynos5250-snow) and the thermistors are accessible and report reasonable values. localhost hwmon # grep /sys/class/hwmon/*/device/temp1_input /sys/class/hwmon/hwmon0/device/temp1_input:37890 /sys/class/hwmon/hwmon1/device/temp1_input:38393 /sys/class/hwmon/hwmon2/device/temp1_input:37148 /sys/class/hwmon/hwmon3/device/temp1_input:38059 Tested-by: Doug Anderson diand...@chromium.org Thanks! Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] hwmon: ntc: Add DT with IIO support to NTC thermistor driver
On Tue, Mar 12, 2013 at 02:09:26PM +0530, Naveen Krishna Chatradhi wrote:
This patch adds DT support to NTC driver to parse the
platform data.
Also adds the support to work as an iio device.
During the probe ntc driver gets the respective channels of ADC
and uses iio_raw_read calls to get the ADC converted value.
Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
Much better, but I still have a couple of comments.
---
Changes since v1:
1. Combimed all the IIO and OF/device tree changes under CONFIG_OF
2. parse_dt function now checks for of_node and iio_channels sanity
3. merged both the patches to single, as they cant build independently
4. used pdev_id for proper logging and support of DT and non DT based probing
5. Rebased on linux git tree
6. Added documentation for device tree bindings
Sorry, for posting a patch rebased on local working branch.
Thanks Doug, for pointing out.
.../devicetree/bindings/hwmon/ntc_thermistor.txt | 29 +
drivers/hwmon/ntc_thermistor.c | 133
+---
include/linux/platform_data/ntc_thermistor.h |6 +-
You'll still need the conditional dependency on IIO in Kconfig.
Something like
depends on IIO if OF
if that works, or alternatively
select IIO if OF
if that is acceptable.
3 files changed, 148 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
diff --git a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
new file mode 100644
index 000..731a78f
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
@@ -0,0 +1,29 @@
+NTC Thermistor hwmon sensors
+---
+
+Requires node properties:
+- compatible value : one of
+ ntc,ncp15wb473
+ ntc,ncp18wb473
+ ntc,ncp21wb473
+ ntc,ncp03wb473
+ ntc,ncp15wl333
+- pullup-uVPull up voltage in micro volts
+- pullup-ohm Pull up resistor value in ohms
+- pulldown-ohm Pull down resistor value in ohms
+- connected-positive Always ON, If not specified.
+ Status change is possible, value (1) is assigned.
+- io-channels Channel node of ADC to be used for
+ conversion.
+
+Read more about iio bindings at
+ Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+Example:
+ ncp15wb473@0 {
+ compatible = ntc,ncp15wb473;
+ pullup-uV = 180;
Is uV (uppercase) acceptable in dt bindings ?
+ pullup-ohm = 47000;
+ pulldown-ohm = 0;
+ io-channels = adc 3;
+ };
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index b5f63f9..81d5bdc 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -26,9 +26,16 @@
#include linux/math64.h
#include linux/platform_device.h
#include linux/err.h
+#include linux/of.h
+#include linux/of_device.h
#include linux/platform_data/ntc_thermistor.h
+#include linux/iio/iio.h
+#include linux/iio/machine.h
+#include linux/iio/driver.h
+#include linux/iio/consumer.h
+
#include linux/hwmon.h
#include linux/hwmon-sysfs.h
@@ -37,6 +44,15 @@ struct ntc_compensation {
unsigned intohm;
};
+static const struct platform_device_id ntc_thermistor_id[] = {
+ { ncp15wb473, TYPE_NCPXXWB473 },
+ { ncp18wb473, TYPE_NCPXXWB473 },
+ { ncp21wb473, TYPE_NCPXXWB473 },
+ { ncp03wb473, TYPE_NCPXXWB473 },
+ { ncp15wl333, TYPE_NCPXXWL333 },
+ { },
+};
+
/*
* A compensation table should be sorted by the values of .ohm
* in descending order.
@@ -125,6 +141,76 @@ struct ntc_data {
char name[PLATFORM_NAME_SIZE];
};
+#ifdef CONFIG_OF
+static int ntc_adc_iio_read(struct ntc_thermistor_platform_data *pdata)
+{
+ struct iio_channel *channel = pdata-chan;
+ unsigned int result;
+ int val, ret;
+
+ ret = iio_read_channel_raw(channel, val);
+ if (ret 0) {
+ pr_err(read channel() error: %d\n, ret);
+ return ret;
+ }
+
+ /* unit: mV */
+ result = pdata-pullup_uV * (s64) val;
result is int, so the s64 typecast does not provide any value.
Should result be s64 to avoid overflow errors ?
+ result = 12;
+
+ return result;
+}
+
+static const struct of_device_id ntc_match[] = {
+ { .compatible = ntc,ncp15wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp18wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp21wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp03wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp15wl333,
+ .data =
Re: [PATCH v3] hwmon: ntc: Add DT with IIO support to NTC thermistor driver
On Wed, Mar 13, 2013 at 08:25:09AM +0530, Naveen Krishna Chatradhi wrote:
This patch adds DT support to NTC driver to parse the
platform data.
Also adds the support to work as an iio device.
During the probe ntc driver gets the respective channels of ADC
and uses iio_raw_read calls to get the ADC converted value.
Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
Almost there ...
---
Changes since v2:
1. Modified Kconfig to select IIO only if OF is defined
2. changed pullup-uV to pullup-uv in dt bindings
3. allocate pdata in ntc_thermistor_parse_dt and modify the return
value to platform data pointer, handled other errors for dt bindings.
4. used strlcpy instead of strncpy as the former handles
null termination
5. made a wrapper function for iio_channel release and defined to
null in case of non-DT
6. removed a typecase of (s64) which is not needed.
Guenter Roeck, Thanks for your valuble time and comments. They really
are helping me learn and do better on my future patches.
Doug, Thanks for your support and testing these patches.
.../devicetree/bindings/hwmon/ntc_thermistor.txt | 29
drivers/hwmon/Kconfig |1 +
drivers/hwmon/ntc_thermistor.c | 144
+---
include/linux/platform_data/ntc_thermistor.h |8 +-
4 files changed, 162 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
diff --git a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
new file mode 100644
index 000..c6f6667
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
@@ -0,0 +1,29 @@
+NTC Thermistor hwmon sensors
+---
+
+Requires node properties:
+- compatible value : one of
+ ntc,ncp15wb473
+ ntc,ncp18wb473
+ ntc,ncp21wb473
+ ntc,ncp03wb473
+ ntc,ncp15wl333
+- pullup-uvPull up voltage in micro volts
+- pullup-ohm Pull up resistor value in ohms
+- pulldown-ohm Pull down resistor value in ohms
+- connected-positive Always ON, If not specified.
+ Status change is possible.
+- io-channels Channel node of ADC to be used for
+ conversion.
+
+Read more about iio bindings at
+ Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+Example:
+ ncp15wb473@0 {
+ compatible = ntc,ncp15wb473;
+ pullup-uv = 180;
+ pullup-ohm = 47000;
+ pulldown-ohm = 0;
+ io-channels = adc 3;
+ };
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 89ac1cb..cc47c12 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -879,6 +879,7 @@ config SENSORS_MCP3021
config SENSORS_NTC_THERMISTOR
tristate NTC thermistor support
+ select IIO if OF
help
This driver supports NTC thermistors sensor reading and its
interpretation. The driver can also monitor the temperature and
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index b5f63f9..ba0f7b0 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -26,9 +26,16 @@
#include linux/math64.h
#include linux/platform_device.h
#include linux/err.h
+#include linux/of.h
+#include linux/of_device.h
#include linux/platform_data/ntc_thermistor.h
+#include linux/iio/iio.h
+#include linux/iio/machine.h
+#include linux/iio/driver.h
+#include linux/iio/consumer.h
+
#include linux/hwmon.h
#include linux/hwmon-sysfs.h
@@ -37,6 +44,15 @@ struct ntc_compensation {
unsigned intohm;
};
+static const struct platform_device_id ntc_thermistor_id[] = {
+ { ncp15wb473, TYPE_NCPXXWB473 },
+ { ncp18wb473, TYPE_NCPXXWB473 },
+ { ncp21wb473, TYPE_NCPXXWB473 },
+ { ncp03wb473, TYPE_NCPXXWB473 },
+ { ncp15wl333, TYPE_NCPXXWL333 },
+ { },
+};
+
/*
* A compensation table should be sorted by the values of .ohm
* in descending order.
@@ -125,6 +141,91 @@ struct ntc_data {
char name[PLATFORM_NAME_SIZE];
};
+#ifdef CONFIG_OF
+static int ntc_adc_iio_read(struct ntc_thermistor_platform_data *pdata)
+{
+ struct iio_channel *channel = pdata-chan;
+ unsigned int result;
+ int val, ret;
+
+ ret = iio_read_channel_raw(channel, val);
+ if (ret 0) {
+ pr_err(read channel() error: %d\n, ret);
+ return ret;
+ }
+
+ /* unit: mV */
+ result = pdata-pullup_uV * val;
+ result = 12;
+
+ return result;
+}
+
+static const struct of_device_id ntc_match[] = {
+ { .compatible = ntc,ncp15wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp18wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473
Re: [PATCH v4] hwmon: ntc: Add DT with IIO support to NTC thermistor driver
On Wed, Mar 13, 2013 at 09:38:20AM +0530, Naveen Krishna Chatradhi wrote:
This patch adds DT support to NTC driver to parse the
platform data.
Also adds the support to work as an iio device.
During the probe ntc driver gets the respective channels of ADC
and uses iio_raw_read calls to get the ADC converted value.
Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
---
Changes since v3:
1. Added a NULL check before iio_channel_release
2. Modified a return statement
Guenter Roeck, Its wonderful working with you. Thanks.
My pleasure.
Patch looks ok to me now. I'll wait a couple of days to see if there is some
feedback from the devicetree folks. Unless there are objections, I'll then queue
it for 3.10.
Thanks,
Guenter
.../devicetree/bindings/hwmon/ntc_thermistor.txt | 29
drivers/hwmon/Kconfig |1 +
drivers/hwmon/ntc_thermistor.c | 145
+---
include/linux/platform_data/ntc_thermistor.h |8 +-
4 files changed, 163 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
diff --git a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
new file mode 100644
index 000..c6f6667
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
@@ -0,0 +1,29 @@
+NTC Thermistor hwmon sensors
+---
+
+Requires node properties:
+- compatible value : one of
+ ntc,ncp15wb473
+ ntc,ncp18wb473
+ ntc,ncp21wb473
+ ntc,ncp03wb473
+ ntc,ncp15wl333
+- pullup-uvPull up voltage in micro volts
+- pullup-ohm Pull up resistor value in ohms
+- pulldown-ohm Pull down resistor value in ohms
+- connected-positive Always ON, If not specified.
+ Status change is possible.
+- io-channels Channel node of ADC to be used for
+ conversion.
+
+Read more about iio bindings at
+ Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+Example:
+ ncp15wb473@0 {
+ compatible = ntc,ncp15wb473;
+ pullup-uv = 180;
+ pullup-ohm = 47000;
+ pulldown-ohm = 0;
+ io-channels = adc 3;
+ };
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 89ac1cb..cc47c12 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -879,6 +879,7 @@ config SENSORS_MCP3021
config SENSORS_NTC_THERMISTOR
tristate NTC thermistor support
+ select IIO if OF
help
This driver supports NTC thermistors sensor reading and its
interpretation. The driver can also monitor the temperature and
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index b5f63f9..a6997a1 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -26,9 +26,16 @@
#include linux/math64.h
#include linux/platform_device.h
#include linux/err.h
+#include linux/of.h
+#include linux/of_device.h
#include linux/platform_data/ntc_thermistor.h
+#include linux/iio/iio.h
+#include linux/iio/machine.h
+#include linux/iio/driver.h
+#include linux/iio/consumer.h
+
#include linux/hwmon.h
#include linux/hwmon-sysfs.h
@@ -37,6 +44,15 @@ struct ntc_compensation {
unsigned intohm;
};
+static const struct platform_device_id ntc_thermistor_id[] = {
+ { ncp15wb473, TYPE_NCPXXWB473 },
+ { ncp18wb473, TYPE_NCPXXWB473 },
+ { ncp21wb473, TYPE_NCPXXWB473 },
+ { ncp03wb473, TYPE_NCPXXWB473 },
+ { ncp15wl333, TYPE_NCPXXWL333 },
+ { },
+};
+
/*
* A compensation table should be sorted by the values of .ohm
* in descending order.
@@ -125,6 +141,92 @@ struct ntc_data {
char name[PLATFORM_NAME_SIZE];
};
+#ifdef CONFIG_OF
+static int ntc_adc_iio_read(struct ntc_thermistor_platform_data *pdata)
+{
+ struct iio_channel *channel = pdata-chan;
+ unsigned int result;
+ int val, ret;
+
+ ret = iio_read_channel_raw(channel, val);
+ if (ret 0) {
+ pr_err(read channel() error: %d\n, ret);
+ return ret;
+ }
+
+ /* unit: mV */
+ result = pdata-pullup_uV * val;
+ result = 12;
+
+ return result;
+}
+
+static const struct of_device_id ntc_match[] = {
+ { .compatible = ntc,ncp15wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp18wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp21wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp03wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473] },
+ { .compatible = ntc,ncp15wl333,
+ .data = ntc_thermistor_id[TYPE_NCPXXWL333] },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ntc_match
Re: [PATCH v4] hwmon: ntc: Add DT with IIO support to NTC thermistor driver
On Wed, Mar 13, 2013 at 09:38:20AM +0530, Naveen Krishna Chatradhi wrote:
This patch adds DT support to NTC driver to parse the
platform data.
Also adds the support to work as an iio device.
During the probe ntc driver gets the respective channels of ADC
and uses iio_raw_read calls to get the ADC converted value.
Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
Unfortunately there is still something we'll need to sort out:
fs/sysfs/Kconfig:1: symbol SYSFS is selected by AT91_ADC
drivers/iio/adc/Kconfig:85: symbol AT91_ADC depends on IIO
drivers/iio/Kconfig:5: symbol IIO is selected by SENSORS_NTC_THERMISTOR
drivers/hwmon/Kconfig:900: symbol SENSORS_NTC_THERMISTOR depends on HWMON
drivers/hwmon/Kconfig:5:symbol HWMON is selected by EEEPC_LAPTOP
drivers/platform/x86/Kconfig:477: symbol EEEPC_LAPTOP depends on
HOTPLUG_PCI
drivers/pci/hotplug/Kconfig:5: symbol HOTPLUG_PCI depends on SYSFS
So we can not just select IIO. I'll see if I can find a solution.
Guenter
---
Changes since v3:
1. Added a NULL check before iio_channel_release
2. Modified a return statement
Guenter Roeck, Its wonderful working with you. Thanks.
.../devicetree/bindings/hwmon/ntc_thermistor.txt | 29
drivers/hwmon/Kconfig |1 +
drivers/hwmon/ntc_thermistor.c | 145
+---
include/linux/platform_data/ntc_thermistor.h |8 +-
4 files changed, 163 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
diff --git a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
new file mode 100644
index 000..c6f6667
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
@@ -0,0 +1,29 @@
+NTC Thermistor hwmon sensors
+---
+
+Requires node properties:
+- compatible value : one of
+ ntc,ncp15wb473
+ ntc,ncp18wb473
+ ntc,ncp21wb473
+ ntc,ncp03wb473
+ ntc,ncp15wl333
+- pullup-uvPull up voltage in micro volts
+- pullup-ohm Pull up resistor value in ohms
+- pulldown-ohm Pull down resistor value in ohms
+- connected-positive Always ON, If not specified.
+ Status change is possible.
+- io-channels Channel node of ADC to be used for
+ conversion.
+
+Read more about iio bindings at
+ Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+Example:
+ ncp15wb473@0 {
+ compatible = ntc,ncp15wb473;
+ pullup-uv = 180;
+ pullup-ohm = 47000;
+ pulldown-ohm = 0;
+ io-channels = adc 3;
+ };
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 89ac1cb..cc47c12 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -879,6 +879,7 @@ config SENSORS_MCP3021
config SENSORS_NTC_THERMISTOR
tristate NTC thermistor support
+ select IIO if OF
help
This driver supports NTC thermistors sensor reading and its
interpretation. The driver can also monitor the temperature and
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index b5f63f9..a6997a1 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -26,9 +26,16 @@
#include linux/math64.h
#include linux/platform_device.h
#include linux/err.h
+#include linux/of.h
+#include linux/of_device.h
#include linux/platform_data/ntc_thermistor.h
+#include linux/iio/iio.h
+#include linux/iio/machine.h
+#include linux/iio/driver.h
+#include linux/iio/consumer.h
+
#include linux/hwmon.h
#include linux/hwmon-sysfs.h
@@ -37,6 +44,15 @@ struct ntc_compensation {
unsigned intohm;
};
+static const struct platform_device_id ntc_thermistor_id[] = {
+ { ncp15wb473, TYPE_NCPXXWB473 },
+ { ncp18wb473, TYPE_NCPXXWB473 },
+ { ncp21wb473, TYPE_NCPXXWB473 },
+ { ncp03wb473, TYPE_NCPXXWB473 },
+ { ncp15wl333, TYPE_NCPXXWL333 },
+ { },
+};
+
/*
* A compensation table should be sorted by the values of .ohm
* in descending order.
@@ -125,6 +141,92 @@ struct ntc_data {
char name[PLATFORM_NAME_SIZE];
};
+#ifdef CONFIG_OF
+static int ntc_adc_iio_read(struct ntc_thermistor_platform_data *pdata)
+{
+ struct iio_channel *channel = pdata-chan;
+ unsigned int result;
+ int val, ret;
+
+ ret = iio_read_channel_raw(channel, val);
+ if (ret 0) {
+ pr_err(read channel() error: %d\n, ret);
+ return ret;
+ }
+
+ /* unit: mV */
+ result = pdata-pullup_uV * val;
+ result = 12;
+
+ return result;
+}
+
+static const struct of_device_id ntc_match[] = {
+ { .compatible = ntc,ncp15wb473,
+ .data = ntc_thermistor_id[TYPE_NCPXXWB473
Re: [lm-sensors] [PATCH v4] hwmon: ntc: Add DT with IIO support to NTC thermistor driver
On Tue, Mar 12, 2013 at 09:52:26PM -0700, Guenter Roeck wrote: On Wed, Mar 13, 2013 at 09:38:20AM +0530, Naveen Krishna Chatradhi wrote: This patch adds DT support to NTC driver to parse the platform data. Also adds the support to work as an iio device. During the probe ntc driver gets the respective channels of ADC and uses iio_raw_read calls to get the ADC converted value. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Unfortunately there is still something we'll need to sort out: fs/sysfs/Kconfig:1: symbol SYSFS is selected by AT91_ADC drivers/iio/adc/Kconfig:85: symbol AT91_ADC depends on IIO drivers/iio/Kconfig:5: symbol IIO is selected by SENSORS_NTC_THERMISTOR drivers/hwmon/Kconfig:900: symbol SENSORS_NTC_THERMISTOR depends on HWMON drivers/hwmon/Kconfig:5:symbol HWMON is selected by EEEPC_LAPTOP drivers/platform/x86/Kconfig:477: symbol EEEPC_LAPTOP depends on HOTPLUG_PCI drivers/pci/hotplug/Kconfig:5: symbol HOTPLUG_PCI depends on SYSFS So we can not just select IIO. I'll see if I can find a solution. Here it is: diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index f7adbe8..47d2176 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -899,7 +899,7 @@ config SENSORS_MCP3021 config SENSORS_NTC_THERMISTOR tristate NTC thermistor support -select IIO if OF +depends on (!OF !IIO) || (OF IIO) help This driver supports NTC thermistors sensor reading and its interpretation. The driver can also monitor the temperature and I'll modify the patch accordingly. No need to resubmit. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] hwmon: NTC: add IIO get channel and read support
On Mon, Mar 11, 2013 at 07:39:47AM +0530, Naveen Krishna Chatradhi wrote:
This patch adds the support to work as a iio device.
iio_get_channel and iio_raw_read works.
During the probe ntc driver gets the respective channels of ADC
and uses iio_raw_read calls to get the ADC converted value.
Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
---
Still not sure about the read_uV function parameter change and placement.
Me not either. Is the parameter change really necessary ? There was a good
reason to pass pdev, as it lets the called function deal with the device, eg for
error messages.
There were a few CamelCase warnings during checkpatch run.
I can clean them if anyone insists.
I actually have a patch sitting somewhere doing just that. Might make sense if I
send it out for review and you rebase your patches on top of it.
drivers/hwmon/ntc_thermistor.c | 36
+-
include/linux/platform_data/ntc_thermistor.h |7 -
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index cfedbd3..1d31260 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -30,6 +30,11 @@
#include linux/platform_data/ntc_thermistor.h
+#include linux/iio/iio.h
+#include linux/iio/machine.h
+#include linux/iio/driver.h
+#include linux/iio/consumer.h
+
One problem with this change is that the driver now mandates the existence of
the IIO subsystem, even if not needed (ie if CONFIG_OF is not defined. We'll
have to limit that impact. I would suggest to keep all the ioo code in the
#ifdef CONFIG_OF block. also, you'll have to add a conditional dependency to
Kconfig.
#include linux/hwmon.h
#include linux/hwmon-sysfs.h
@@ -162,6 +167,28 @@ struct ntc_data {
char name[PLATFORM_NAME_SIZE];
};
+static int ntc_adc_read(struct ntc_thermistor_platform_data *pdata)
+{
Better name it ntc_adc_iio_read.
+ struct iio_channel *channel = pdata-chan;
+ unsigned int result;
+ int val, ret;
+
+ if (!channel)
+ return -EINVAL;
+
You should check this earlier (see below).
+ ret = iio_read_channel_raw(channel, val);
+ if (ret 0) {
+ pr_err(read channel() error: %d\n, ret);
+ return ret;
+ }
+
+ /* unit: mV */
+ result = pdata-pullup_uV * (s64) val;
+ result = 12;
+
+ return result;
+}
+
#ifdef CONFIG_OF
static void ntc_thermistor_parse_dt(struct ntc_thermistor_platform_data
*pdata,
struct device_node *np)
@@ -173,6 +200,8 @@ static void ntc_thermistor_parse_dt(struct
ntc_thermistor_platform_data *pdata,
pdata-connect = NTC_CONNECTED_POSITIVE;
else /* status change should be possible if not always on. */
pdata-connect = NTC_CONNECTED_GROUND;
+
+ pdata-read_uV = ntc_adc_read;
}
#else
static void
@@ -317,7 +346,7 @@ static int ntc_thermistor_get_ohm(struct ntc_data *data)
return data-pdata-read_ohm(data-pdev);
if (data-pdata-read_uV) {
- read_uV = data-pdata-read_uV(data-pdev);
+ read_uV = data-pdata-read_uV(data-pdata);
I am really not happy with this parameter change.
you should be able to get the pointer to pdata with
data = platform_get_drvdata(pdev);
pdata = data-pdata;
if (read_uV 0)
return read_uV;
return get_ohm_of_thermistor(data, read_uV);
@@ -417,6 +446,8 @@ static int ntc_thermistor_probe(struct platform_device
*pdev)
if (!data)
return -ENOMEM;
+ pdata-chan = iio_channel_get(pdev-dev, NULL);
+
I think this should be assigned together with read_uV in
ntc_thermistor_parse_dt,
and bail out if it returns an error. It does not make sense to instantiate
the driver otherwise if OF is used to construct pdata.
Also, iio_channel_get can return an error, so you have to check the returned
value for an error return with IS_ERR. This error can be EDEFER, so make
sure it is returned to the caller to cause the probe t be called again later.
data-dev = pdev-dev;
data-pdev = pdev;
data-pdata = pdata;
@@ -457,15 +488,18 @@ static int ntc_thermistor_probe(struct platform_device
*pdev)
return 0;
err_after_sysfs:
sysfs_remove_group(data-dev-kobj, ntc_attr_group);
+ iio_channel_release(pdata-chan);
That will crash nicely if pdata-chan is not set or has an error.
return ret;
}
static int ntc_thermistor_remove(struct platform_device *pdev)
{
struct ntc_data *data = platform_get_drvdata(pdev);
+ struct ntc_thermistor_platform_data *pdata = data-pdata;
hwmon_device_unregister(data-hwmon_dev);
sysfs_remove_group(data-dev-kobj, ntc_attr_group);
+
Re: [lm-sensors] [PATCH 1/2] hwmon: ntc: Add DT support to NTC thermistor driver
On Mon, Mar 11, 2013 at 05:21:44AM -0700, Guenter Roeck wrote:
On Mon, Mar 11, 2013 at 07:39:46AM +0530, Naveen Krishna Chatradhi wrote:
This patch adds the DT support to NTC driver to parse the
platform data.
Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
---
drivers/hwmon/ntc_thermistor.c | 93
1 file changed, 75 insertions(+), 18 deletions(-)
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
index d92b237..cfedbd3 100644
--- a/drivers/hwmon/ntc_thermistor.c
+++ b/drivers/hwmon/ntc_thermistor.c
@@ -26,6 +26,7 @@
#include linux/math64.h
#include linux/platform_device.h
#include linux/err.h
+#include linux/of.h
#include linux/platform_data/ntc_thermistor.h
@@ -37,6 +38,41 @@ struct ntc_compensation {
unsigned intohm;
};
+static const struct platform_device_id ntc_thermistor_id[] = {
+ { ncp15wb473, TYPE_NCPXXWB473 },
+ { ncp18wb473, TYPE_NCPXXWB473 },
+ { ncp21wb473, TYPE_NCPXXWB473 },
+ { ncp03wb473, TYPE_NCPXXWB473 },
+ { ncp15wl333, TYPE_NCPXXWL333 },
+ { },
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id ntc_match[] = {
+ { .compatible = ntc,ncp15wb473, .data = (void *)TYPE_NCPXXWB473 },
+ { .compatible = ntc,ncp18wb473, .data = (void *)TYPE_NCPXXWB473 },
+ { .compatible = ntc,ncp21wb473, .data = (void *)TYPE_NCPXXWB473 },
+ { .compatible = ntc,ncp03wb473, .data = (void *)TYPE_NCPXXWB473 },
+ { .compatible = ntc,ncp15wl333, .data = (void *)TYPE_NCPXXWL333 },
Better point to ntc_thermistor_id[TYPE_xxx]. See below.
+ { },
+};
+MODULE_DEVICE_TABLE(of, ntc_match);
+#endif
+
+/* Get NTC type either from device tree or platform data. */
+static enum ntc_thermistor_type thermistor_get_type
+ (struct platform_device *pdev)
+{
+ if (pdev-dev.of_node) {
+ const struct of_device_id *match;
+ match = of_match_node(of_match_ptr(ntc_match),
+ pdev-dev.of_node);
+ return (unsigned int)match-data;
+ }
+
+ return platform_get_device_id(pdev)-driver_data;
+}
+
/*
* A compensation table should be sorted by the values of .ohm
* in descending order.
@@ -126,6 +162,27 @@ struct ntc_data {
char name[PLATFORM_NAME_SIZE];
};
+#ifdef CONFIG_OF
Please merge the two CONFIG_OF blocks into one.
+static void ntc_thermistor_parse_dt(struct ntc_thermistor_platform_data
*pdata,
+ struct device_node *np)
+{
+ of_property_read_u32(np, pullup-uV, pdata-pullup_uV);
+ of_property_read_u32(np, pullup-ohm, pdata-pullup_ohm);
+ of_property_read_u32(np, pulldown-ohm, pdata-pulldown_ohm);
+ if (of_find_property(np, connected-positive, NULL))
+ pdata-connect = NTC_CONNECTED_POSITIVE;
+ else /* status change should be possible if not always on. */
+ pdata-connect = NTC_CONNECTED_GROUND;
+}
Forgot to mention: You'll have to add a devicetree bindings file describing the
bindings.
+#else
+static void
+static void ntc_thermistor_parse_dt(struct ntc_thermistor_platform_data
*pdata,
+ struct device_node *np)
+{
+ return;
Unnecessary return statement.
+}
+#endif
+
static inline u64 div64_u64_safe(u64 dividend, u64 divisor)
{
if (divisor == 0 dividend == 0)
@@ -313,9 +370,19 @@ static const struct attribute_group ntc_attr_group = {
static int ntc_thermistor_probe(struct platform_device *pdev)
{
struct ntc_data *data;
- struct ntc_thermistor_platform_data *pdata = pdev-dev.platform_data;
+ struct device_node *np = pdev-dev.of_node;
+ struct ntc_thermistor_platform_data *pdata = NULL;
Unnecessary change. It doesn't hurt if pdata is initialized to non-NULL, and
initializing it to NULL is unnecessary anyway since it is always assigned
below.
int ret = 0;
+ if (np) {
+ pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ ntc_thermistor_parse_dt(pdata, np);
This compiles, but the resulting code would not work as the necessary fields
in pdata are not all filled out. So I think it would be better to merge the
code
with the next patch, as both don't really work independently.
+ } else
+ pdata = pdev-dev.platform_data;
Not necessary if you keep above pre-initialization.
A better idea would be to move the devm_kzalloc into ntc_thermistor_parse_dt
and return a pointer to pdata from it. Then you would have
pdata = ntc_thermistor_parse_dt(np);
if (!pdata)
pdata = pdev-dev.platform_data;
The check for np == NULL could then also be in ntc_thermistor_parse_dt
Re: [PATCH v5] iio: Add OF support
On Fri, Feb 08, 2013 at 08:30:48AM +, Jonathan Cameron wrote:
On 07/02/13 17:09, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
Looks fine to me. Will give it a few more days to see what others
have to say.
Hi Jonathan,
Any further feedback on this ? If there is anything else I need to do
to get it accepted, please let me know.
Thanks,
Guenter
Thanks Guenter
---
v5:
- Updated examples in bindings.
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
undefined, and wrong return value.
- Initialize indio_dev-of_node in iio_device_register if the calling driver
neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type
instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other device
types) and make it global for that purpose. Check the OF node first, then
the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
an error if there is a problem with consistency or if the provider device
is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a
channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
not used in the code, and it is not entirely clear what it would be used
for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still
builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
.../devicetree/bindings/iio/iio-bindings.txt | 97 +++
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|8 +-
drivers/iio/inkern.c | 171
4 files changed, 275 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..1182845
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,97 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for
nodes
+ with a single IIO output and 1 for nodes with multiple
+ IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+adc: voltage-sensor@35 {
+compatible = maxim,max1139;
+reg = 0x35;
+#io-channel-cells = 1;
+};
+
+Example for a configuration with trigger:
+
+adc@35 {
+compatible = some-vendor,some-adc;
+reg = 0x35;
+
+adc1: iio-device@0 {
+#io-channel-cells = 1;
+/* other properties */
+};
+adc2: iio-device@1 {
+#io-channel-cells = 1;
+/* other properties */
+};
+};
+
+==IIO consumers==
+
+Required properties:
+io-channels:List of phandle and IIO specifier pairs, one pair
+for each IIO input to the device. Note: if the
+IIO provider specifies '0' for #io-channel-cells,
+then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+List of IIO input name strings sorted in the same
+order as the io-channels property
Re: [PATCH v5] iio: Add OF support
On Wed, Feb 20, 2013 at 11:38:22AM -0600, Rob Herring wrote:
On 02/07/2013 11:09 AM, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
v5:
- Updated examples in bindings.
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
undefined, and wrong return value.
- Initialize indio_dev-of_node in iio_device_register if the calling driver
neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type
instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other device
types) and make it global for that purpose. Check the OF node first, then
the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
an error if there is a problem with consistency or if the provider device
is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a
channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property
is
not used in the code, and it is not entirely clear what it would be used
for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still
builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of
device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
.../devicetree/bindings/iio/iio-bindings.txt | 97 +++
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|8 +-
drivers/iio/inkern.c | 171
4 files changed, 275 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..1182845
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,97 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
Bindings are an ABI. It should not be a WIP. What part is a WIP?
The text is copied from clock bindings. I do not claim to be better
than the clock subsystem is doing in respect to its bindings.
I'll be more than happy to take this text out if it is a source of contention,
even more so if that is what is holding up the patch. Jonathan ?
Thanks,
Guenter
Knowing nothing about IIO, the binding seems sane.
Rob
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for
nodes
+ with a single IIO output and 1 for nodes with multiple
+ IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+ adc: voltage-sensor@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+Example for a configuration with trigger:
+
+ adc@35 {
+ compatible = some-vendor,some-adc;
+ reg = 0x35;
+
+ adc1: iio-device@0 {
+ #io-channel-cells = 1;
+ /* other properties */
+ };
+ adc2: iio-device@1 {
+ #io-channel-cells = 1;
+ /* other properties */
+ };
+ };
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #io
Re: [PATCH v5] iio: Add OF support
On Wed, Feb 20, 2013 at 06:51:08PM +, Jonathan Cameron wrote:
Guenter Roeck li...@roeck-us.net wrote:
On Wed, Feb 20, 2013 at 11:38:22AM -0600, Rob Herring wrote:
On 02/07/2013 11:09 AM, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
v5:
- Updated examples in bindings.
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if
CONFIG_OF is
undefined, and wrong return value.
- Initialize indio_dev-of_node in iio_device_register if the
calling driver
neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device
type instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other
device
types) and make it global for that purpose. Check the OF node
first, then the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and
of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found,
or
an error if there is a problem with consistency or if the
provider device is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name()
returns a channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The
property is
not used in the code, and it is not entirely clear what it would
be used for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code
still builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead
of device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
.../devicetree/bindings/iio/iio-bindings.txt | 97
+++
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|8 +-
drivers/iio/inkern.c | 171
4 files changed, 275 insertions(+), 2 deletions(-)
create mode 100644
Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..1182845
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,97 @@
+This binding is a work-in-progress. It is derived from clock
bindings,
+and based on suggestions from Lars-Peter Clausen [1].
Bindings are an ABI. It should not be a WIP. What part is a WIP?
The text is copied from clock bindings. I do not claim to be better
than the clock subsystem is doing in respect to its bindings.
I'll be more than happy to take this text out if it is a source of
contention,
even more so if that is what is holding up the patch. Jonathan ?
I don't really care about the wording though can see why Rob asked!
We will probably have other stuff to add anyway.
Quite likely - for example, I did not add io-output-names since I did not
know what to do with it.
What do you want me to do ? Submit another version with the sentence deleted,
or keep it as-is ?
Thanks,
Guenter
Jonathan
Thanks,
Guenter
Knowing nothing about IIO, the binding seems sane.
Rob
+Sources of IIO channels can be represented by any node in the
device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by
the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically
0 for nodes
+ with a single IIO output and 1 for nodes with
multiple
+ IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+adc: voltage-sensor@35 {
+compatible = maxim,max1139;
+reg = 0x35;
+#io-channel-cells = 1;
+};
+
+Example for a configuration with trigger:
+
+adc@35
Re: [lm-sensors] [RFC PATCH 3/9] hwmon: (lm90) add support to handle irq
On Mon, Feb 18, 2013 at 07:30:25PM +0800, Wei Ni wrote:
Add support to handle irq. When the temperature touch the limit value, the
driver can handle the interrupt.
Signed-off-by: Alexandre Courbot acour...@nvidia.com
Signed-off-by: Wei Ni w...@nvidia.com
---
drivers/hwmon/lm90.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index caf01b0..80311ef 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -89,6 +89,8 @@
#include linux/err.h
#include linux/mutex.h
#include linux/sysfs.h
+#include linux/interrupt.h
+#include linux/of_irq.h
/*
* Addresses to scan
@@ -302,6 +304,7 @@ static const struct lm90_params lm90_params[] = {
struct lm90_data {
struct device *hwmon_dev;
struct mutex update_lock;
+ struct work_struct irq_work;
char valid; /* zero until following fields are valid */
unsigned long last_updated; /* in jiffies */
int kind;
@@ -1418,6 +1421,29 @@ static void lm90_init_client(struct i2c_client *client)
i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, config);
}
+static void lm90_alert(struct i2c_client *client, unsigned int flag);
+
+static void lm90_irq_work(struct work_struct *work)
+{
+ struct lm90_data *data = container_of(work, struct lm90_data,
+ irq_work);
+ struct i2c_client *client = to_i2c_client(data-hwmon_dev-parent);
+
+ lm90_alert(client, 0);
+
+ enable_irq(client-irq);
+}
+
+static irqreturn_t lm90_irq(int irq, void *dev_id)
+{
+ struct lm90_data *data = dev_id;
+
+ disable_irq_nosync(irq);
+ schedule_work(data-irq_work);
+
+ return IRQ_HANDLED;
+}
+
static int lm90_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -1494,6 +1520,17 @@ static int lm90_probe(struct i2c_client *client,
goto exit_remove_files;
}
+ if (client-irq = 0) {
+ INIT_WORK(data-irq_work, lm90_irq_work);
+ dev_dbg(dev, lm90 irq: %d\n, client-irq);
+ err = request_irq(client-irq, lm90_irq, IRQF_TRIGGER_LOW,
+lm90, data);
request_threaded_irq or even better devm_request_threaded_irq would be better
here.
+ if (err 0) {
+ dev_err(dev, cannot request interrupt\n);
+ goto exit_remove_files;
+ }
+ }
+
return 0;
exit_remove_files:
@@ -1507,6 +1544,7 @@ static int lm90_remove(struct i2c_client *client)
{
struct lm90_data *data = i2c_get_clientdata(client);
+ free_irq(client-irq, data);
hwmon_device_unregister(data-hwmon_dev);
lm90_remove_files(client, data);
lm90_restore_conf(client, data);
--
1.7.9.5
___
lm-sensors mailing list
lm-sens...@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [lm-sensors] [RFC PATCH 6/9] hwmon: (lm90) Register to the thermal framework
On Mon, Feb 18, 2013 at 07:30:28PM +0800, Wei Ni wrote:
Register the remote sensor to the thermal framework.
It can support to show the temperature and read/write threshold.
Signed-off-by: Wei Ni w...@nvidia.com
---
arch/arm/boot/dts/tegra30-cardhu.dtsi |1 +
drivers/hwmon/lm90.c | 182
-
2 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi
b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index 15ad1ad..3f6ab89 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi
@@ -279,6 +279,7 @@
reg = 0x4c;
interrupt-parent = gpio;
interrupts = 226 0x08; /* gpio PCC2 */
+ #sensor-cells = 1;
};
};
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index de5a476..0abdedc 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -91,6 +91,7 @@
#include linux/sysfs.h
#include linux/interrupt.h
#include linux/of_irq.h
+#include linux/thermal.h
/*
* Addresses to scan
@@ -182,6 +183,15 @@ enum chips { lm90, adm1032, lm99, lm86, max6657,
max6659, adt7461, max6680,
#define LM90_HAVE_BROKEN_ALERT (1 7) /* Broken alert
*/
/*
+ * Thermal framework
+ */
+enum lm90_thresholds {
+ LM90_LOW_THRESHOLDS = 0,/* threshold 0: low limits */
+ LM90_HIGH_THRESHOLDS, /* threshold 1: high limits */
+ LM90_NUM_THRESHOLDS
+};
+
+/*
* Driver data (common to all clients)
*/
@@ -377,6 +387,9 @@ struct lm90_data {
s16 temp11[TEMP11_REG_NUM];
u8 temp_hyst;
u16 alarms; /* bitvector (upper 8 bits for max6695/96) */
+
+ struct thermal_sensor *ts_remote;
+ struct thermal_sensor *ts_local;
};
/*
@@ -1493,12 +1506,151 @@ static irqreturn_t lm90_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static int lm90_read_remote_temp(struct thermal_sensor *ts, long *temp)
+{
+ struct i2c_client *client = ts-devdata;
+ struct device *dev = client-dev;
+
+ _show_temp11(dev, TEMP11_REMOTE_TEMP, (int *)temp);
+
+ return 0;
+}
+
+static int lm90_read_remote_threshold(struct thermal_sensor *ts, int
th_index,
+ long *val)
+{
+ struct i2c_client *client = ts-devdata;
+ struct device *dev = client-dev;
+ int index;
+
+ switch (th_index) {
+ case LM90_LOW_THRESHOLDS:
+ /* remote low limit */
+ index = TEMP11_REMOTE_LOW;
+ break;
+ case LM90_HIGH_THRESHOLDS:
+ /* remote high limit */
+ index = TEMP11_REMOTE_HIGH;
+ break;
+ default:
+ dev_err(dev, read remote threshold failed.\n);
+ return -EINVAL;
+ }
+
+ _show_temp11(dev, index, (int *)val);
Typecasting a pointer like this doesn't work. Try this on a big-endian system
with
sizeof(int) != sizeof(long). Note that you would not have the problem if you
would pass the value instead of the pointer.
+
+ return 0;
+}
+
+static int lm90_write_remote_threshold(struct thermal_sensor *ts, int
th_index,
+ long val)
+{
+ struct i2c_client *client = ts-devdata;
+ struct device *dev = client-dev;
+ int nr, index;
+
+ switch (th_index) {
+ case LM90_LOW_THRESHOLDS:
+ /* remote low limit */
+ nr = NR_CHAN_0_REMOTE_LOW;
+ index = TEMP11_REMOTE_LOW;
+ break;
+ case LM90_HIGH_THRESHOLDS:
+ /* remote high limit */
+ nr = NR_CHAN_0_REMOTE_HIGH;
+ index = TEMP11_REMOTE_HIGH;
+ break;
+ default:
+ dev_err(dev, write remote threshold failed.\n);
+ return -EINVAL;
+ }
+
+ _set_temp11(dev, nr, index, val);
+
+ return 0;
+}
+
+static struct thermal_sensor_ops remote_ops = {
+ .get_temp = lm90_read_remote_temp,
+ .get_threshold = lm90_read_remote_threshold,
+ .set_threshold = lm90_write_remote_threshold,
+};
+
+static int lm90_read_local_temp(struct thermal_sensor *ts, long *temp)
+{
+ struct i2c_client *client = ts-devdata;
+ struct device *dev = client-dev;
+
+ _show_temp11(dev, TEMP11_LOCAL_TEMP, (int *)temp);
+
Same here and everywhere else.
+ return 0;
+}
+
+static int lm90_read_local_threshold(struct thermal_sensor *ts, int th_index,
+ long *val)
+{
+ struct i2c_client *client = ts-devdata;
+ struct device *dev = client-dev;
+ int index;
+
+ switch (th_index) {
+ case LM90_LOW_THRESHOLDS:
+ /* local low limit */
+ index = TEMP8_LOCAL_LOW;
+ break;
+ case LM90_HIGH_THRESHOLDS:
+ /* local high limit */
+
[PATCH v5] iio: Add OF support
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
v5:
- Updated examples in bindings.
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
undefined, and wrong return value.
- Initialize indio_dev-of_node in iio_device_register if the calling driver
neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other device
types) and make it global for that purpose. Check the OF node first, then the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
an error if there is a problem with consistency or if the provider device is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
not used in the code, and it is not entirely clear what it would be used for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
.../devicetree/bindings/iio/iio-bindings.txt | 97 +++
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|8 +-
drivers/iio/inkern.c | 171
4 files changed, 275 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..1182845
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,97 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+ with a single IIO output and 1 for nodes with multiple
+ IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+ adc: voltage-sensor@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+Example for a configuration with trigger:
+
+ adc@35 {
+ compatible = some-vendor,some-adc;
+ reg = 0x35;
+
+ adc1: iio-device@0 {
+ #io-channel-cells = 1;
+ /* other properties */
+ };
+ adc2: iio-device@1 {
+ #io-channel-cells = 1;
+ /* other properties */
+ };
+ };
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #io-channel-cells,
+ then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can inherit named
+ IIO channels from this node. Useful for bus nodes to provide
+ and IIO channel to their children.
+
+For example:
+
+ device {
+ io-channels = adc 1, ref 0
[PATCH v4 2/2] iio: Add OF support
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
undefined, and wrong return value.
- Initialize indio_dev-of_node in iio_device_register if the calling driver
neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other device
types) and make it global for that purpose. Check the OF node first, then the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
an error if there is a problem with consistency or if the provider device is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
not used in the code, and it is not entirely clear what it would be used for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|8 +-
drivers/iio/inkern.c | 171
4 files changed, 268 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+ with a single IIO output and 1 for nodes with multiple
+ IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+ adc: voltage-sensor@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+Example for a configuration with trigger:
+
+ adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+
+ adc: iio-device {
+ #io-channel-cells = 1;
+ };
+ trigger: iio-trigger {
+ #io-channel-cells = 1;
+ };
+ };
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #io-channel-cells,
+ then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can inherit named
+ IIO channels from this node. Useful for bus nodes to provide
+ and IIO channel to their children.
+
+For example:
+
+ device {
+ io-channels = adc 1, ref 0;
+ io-channel-names = vcc, vdd;
+ };
+
+This represents a device with two IIO inputs, named vcc and vdd.
+The vcc channel is connected to output 1 of the adc device, and the
+vdd channel is connected
Re: [PATCH v4 2/2] iio: Add OF support
On Wed, Feb 06, 2013 at 07:37:37PM +, Jonathan Cameron wrote:
On 02/06/2013 06:29 PM, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
looks good to me. Couple of little queries inline.
---
v4:
- Fixed wrong parameter to dummy of_iio_channel_get_by_name if CONFIG_OF is
undefined, and wrong return value.
- Initialize indio_dev-of_node in iio_device_register if the calling driver
neglected to do it.
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type
instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other device
types) and make it global for that purpose. Check the OF node first, then
the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
an error if there is a problem with consistency or if the provider device
is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a
channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property
is
not used in the code, and it is not entirely clear what it would be used
for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still
builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of
device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|8 +-
drivers/iio/inkern.c | 171
4 files changed, 268 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for
nodes
+ with a single IIO output and 1 for nodes with multiple
+ IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+ adc: voltage-sensor@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+Example for a configuration with trigger:
+
+ adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+
+ adc: iio-device {
+ #io-channel-cells = 1;
+ };
+ trigger: iio-trigger {
+ #io-channel-cells = 1;
Why would the trigger have channel-cells?
So far we don't have any device tree stuff for the triggers.
This comes from one of the examples passed around earlier.
Presumably, when triggers are supported through devicetree, you would have more
than one per device, meaning you need the same logic as for devices.
Though on the other side that isn't supported yet - I don't have hardware to
test, so I can not implement - or, rather, test - devicetree based trigger
support.
I can take the entire thing out if you prefer - I don't really have a strong
opinion.
+ };
+ };
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #io
Re: [PATCH v4 2/2] iio: Add OF support
On Wed, Feb 06, 2013 at 07:37:37PM +, Jonathan Cameron wrote:
On 02/06/2013 06:29 PM, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
looks good to me. Couple of little queries inline.
[ ... ]
+
+Example for a configuration with trigger:
+
+ adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+
+ adc: iio-device {
+ #io-channel-cells = 1;
+ };
+ trigger: iio-trigger {
+ #io-channel-cells = 1;
Why would the trigger have channel-cells?
So far we don't have any device tree stuff for the triggers.
I thought about keeping the example and adding a note that trigger support is
not currently implemented, but a better idea is to replace it with a second iio
device. I'll do that unless someone objects.
adc@35 {
compatible = some-vendor,some-adc;
reg = 0x35;
adc1: iio-device@0 {
#io-channel-cells = 1;
/* other properties */
};
adc2: iio-device@1 {
#io-channel-cells = 1;
/* other properties */
};
};
+
+==Example==
+
+ adc: max1139@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+ ...
+
+ iio_hwmon {
+ compatible = iio-hwmon;
+ io-channels = adc 0, adc 1, adc 2,
+ adc 3, adc 4, adc 5,
+ adc 6, adc 7, adc 8,
+ adc 9, adc 10, adc 11;
+ io-channel-names = vcc, vdd, vref, 1.2V;
This example still seems a little odd. Why give one where there are
more channels than names?
I replaced the example with
iio_hwmon {
compatible = iio-hwmon;
io-channels = adc 0, adc 1, adc 2,
adc 3, adc 4, adc 5,
adc 6, adc 7, adc 8,
adc 9;
};
some_consumer {
compatible = some-consumer;
io-channels = adc 10, adc 11;
io-channel-names = adc1, adc2;
};
Guenter
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 2/2] iio: Add OF support
On Mon, Feb 04, 2013 at 12:26:06PM -0800, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
One open question is how to assign of_node to the iio device. We can either
do it
in each driver (which turns out to be a huge patchset), or add something like
the
following to iio_device_register.
if (!indio_dev-dev.of_node indio_dev-dev.parent)
indio_dev-dev.of_node = indio_dev-dev.parent-of_node;
[ ... ]
+
+#else /* CONFIG_OF */
+
+static inline struct iio_channel *
+of_iio_channel_get_by_name(struct device *dev, const char *name)
+{
+ return ERR_PTR(-ENOENT);
+}
+
The above should be
static inline struct iio_channel *
of_iio_channel_get_by_name(struct device_node *np, const char *name)
{
return NULL;
}
Will be fixed in the next version.
Guenter
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/4] iio: Add OF support
On Sun, Feb 03, 2013 at 08:22:13AM -0800, Guenter Roeck wrote: [ ... ] + + /* NULL terminated array to save passing size */ + chans = kzalloc(sizeof(*chans)*(nummaps + 1), GFP_KERNEL); I think using kcalloc makes sense here. that would leave chan-data uninitialized, and I would have to initialize it explicitly. also, if additional fields are ever added, we would risk having uninitialized fields. Using kzalloc avoids a potential future error case, so I would prefer to keep it. Please ignore this one. Looks like my brain was too flu-foggy to realize that kcalloc does clear the memory. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/4] iio: Add OF support
On Mon, Feb 04, 2013 at 12:14:52AM +0100, Tomasz Figa wrote:
On Sunday 03 of February 2013 19:55:47 Lars-Peter Clausen wrote:
On 02/03/2013 06:30 PM, Tomasz Figa wrote:
On Sunday 03 of February 2013 09:01:07 Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 12:52:40PM +0100, Tomasz Figa wrote:
On Sunday 03 of February 2013 12:29:23 Lars-Peter Clausen wrote:
On 02/03/2013 03:06 AM, Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 02:30:24AM +0100, Tomasz Figa wrote:
Hi Guenter,
Some comments inline.
On Saturday 02 of February 2013 16:59:40 Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document.
The
property is not used in the code, and it is not entirely clear
what
it
would be used for. If there is a need for it, we can add it back
in
later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the
code
still builds if it is not selected.
- Change iio_channel_get to take device pointer as argument
instead
of
device name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF
configurations.
.../devicetree/bindings/iio/iio-bindings.txt | 76
drivers/iio/inkern.c | 186
2 files changed, 262 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git
a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt new
file
mode
100644
index 000..58df5f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,76 @@
+This binding is a work-in-progress. It is derived from clock
bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the
device
+tree. Those nodes are designated as IIO providers. IIO
consumer
+nodes use a phandle and IIO specifier pair to connect IIO
provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined
by
the
+value of a #io-channel-cells property in the clock provider
node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier;
Typically 0
for nodes +with a single IIO output and 1 for nodes
with
multiple + IIO outputs.
+
+For example:
+
+adc: adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+#io-channel-cells = 1;
+};
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one
pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #clock-cells, then
+ only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers
drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can
inherit
named
+ IIO channels from this node. Useful for bus nodes
to
provide
+ and IIO channel to their children.
+
+For example:
+
+device {
+io-channels = adc 1, ref 0;
+io-channel-names = vcc, vdd;
+};
+
+This represents a device with two IIO inputs, named vcc and
vdd.
+The vcc channel is connected to output 1 of the adc device,
and
the
+vdd channel is connected to output 0 of the ref device.
+
+==Example==
+
+ adc: max1139@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+ ...
+
+ iio_hwmon {
+ compatible = iio-hwmon;
+ io-channels = adc 0, adc 1, adc 2,
+ adc 3, adc 4, adc 5,
+ adc 6, adc 7, adc 8,
+ adc 9, adc 10, adc 11;
+ io-channel-names = vcc, vdd, vref, 1.2V;
+ };
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index b289915..d48f2a8 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -10,6
Re: [PATCH v2 4/4] iio: Add OF support
On Mon, Feb 04, 2013 at 09:12:14AM -0800, Guenter Roeck wrote:
On Mon, Feb 04, 2013 at 12:14:52AM +0100, Tomasz Figa wrote:
On Sunday 03 of February 2013 19:55:47 Lars-Peter Clausen wrote:
On 02/03/2013 06:30 PM, Tomasz Figa wrote:
On Sunday 03 of February 2013 09:01:07 Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 12:52:40PM +0100, Tomasz Figa wrote:
On Sunday 03 of February 2013 12:29:23 Lars-Peter Clausen wrote:
On 02/03/2013 03:06 AM, Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 02:30:24AM +0100, Tomasz Figa wrote:
Hi Guenter,
Some comments inline.
On Saturday 02 of February 2013 16:59:40 Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document.
The
property is not used in the code, and it is not entirely clear
what
it
would be used for. If there is a need for it, we can add it back
in
later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the
code
still builds if it is not selected.
- Change iio_channel_get to take device pointer as argument
instead
of
device name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF
configurations.
.../devicetree/bindings/iio/iio-bindings.txt | 76
drivers/iio/inkern.c | 186
2 files changed, 262 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git
a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt new
file
mode
100644
index 000..58df5f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,76 @@
+This binding is a work-in-progress. It is derived from clock
bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the
device
+tree. Those nodes are designated as IIO providers. IIO
consumer
+nodes use a phandle and IIO specifier pair to connect IIO
provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined
by
the
+value of a #io-channel-cells property in the clock provider
node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier;
Typically 0
for nodes + with a single IIO output and 1 for
nodes
with
multiple + IIO outputs.
+
+For example:
+
+adc: adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+#io-channel-cells = 1;
+};
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one
pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #clock-cells, then
+ only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers
drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can
inherit
named
+ IIO channels from this node. Useful for bus nodes
to
provide
+ and IIO channel to their children.
+
+For example:
+
+device {
+io-channels = adc 1, ref 0;
+io-channel-names = vcc, vdd;
+};
+
+This represents a device with two IIO inputs, named vcc and
vdd.
+The vcc channel is connected to output 1 of the adc device,
and
the
+vdd channel is connected to output 0 of the ref device.
+
+==Example==
+
+ adc: max1139@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+ ...
+
+ iio_hwmon {
+ compatible = iio-hwmon;
+ io-channels = adc 0, adc 1, adc 2,
+ adc 3, adc 4, adc 5,
+ adc 6, adc 7, adc 8,
+ adc 9, adc 10, adc 11;
+ io-channel-names = vcc, vdd
Re: [PATCH v2 4/4] iio: Add OF support
On Mon, Feb 04, 2013 at 07:00:55PM +0100, Tomasz Figa wrote:
On Monday 04 of February 2013 09:51:34 Guenter Roeck wrote:
On Mon, Feb 04, 2013 at 09:12:14AM -0800, Guenter Roeck wrote:
On Mon, Feb 04, 2013 at 12:14:52AM +0100, Tomasz Figa wrote:
On Sunday 03 of February 2013 19:55:47 Lars-Peter Clausen wrote:
On 02/03/2013 06:30 PM, Tomasz Figa wrote:
On Sunday 03 of February 2013 09:01:07 Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 12:52:40PM +0100, Tomasz Figa wrote:
On Sunday 03 of February 2013 12:29:23 Lars-Peter Clausen
wrote:
On 02/03/2013 03:06 AM, Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 02:30:24AM +0100, Tomasz Figa wrote:
Hi Guenter,
Some comments inline.
On Saturday 02 of February 2013 16:59:40 Guenter Roeck
wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings
document.
The
property is not used in the code, and it is not entirely
clear
what
it
would be used for. If there is a need for it, we can add
it back
in
later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that
the
code
still builds if it is not selected.
- Change iio_channel_get to take device pointer as
argument
instead
of
device name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF
configurations.
.../devicetree/bindings/iio/iio-bindings.txt | 76
drivers/iio/inkern.c | 186
2 files changed, 262 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git
a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new
file
mode
100644
index 000..58df5f6
--- /dev/null
+++
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,76 @@
+This binding is a work-in-progress. It is derived from
clock
bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in
the
device
+tree. Those nodes are designated as IIO providers. IIO
consumer
+nodes use a phandle and IIO specifier pair to connect IIO
provider
+outputs to IIO inputs. Similar to the gpio specifiers,
an IIO
+specifier is an array of one or more cells identifying
the IIO
+output on a device. The length of an IIO specifier is
defined
by
the
+value of a #io-channel-cells property in the clock
provider
node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier;
Typically 0
for nodes + with a single IIO output and 1 for
nodes
with
multiple + IIO outputs.
+
+For example:
+
+adc: adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+#io-channel-cells = 1;
+};
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs,
one
pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #clock-cells,
then
+ only the phandle portion of the pair will
appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the
same
+ order as the io-channels property. Consumers
drivers
+ will use io-channel-names to match IIO input
names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can
inherit
named
+ IIO channels from this node. Useful for bus
nodes
to
provide
+ and IIO channel to their children.
+
+For example:
+
+device {
+io-channels = adc 1, ref 0;
+io-channel-names = vcc, vdd;
+};
+
+This represents a device with two IIO inputs, named vcc
and
vdd.
+The vcc channel is connected
[PATCH 1/2] iio: Update iio_channel_get API to use consumer device pointer as argument
For iio_channel_get to work with OF based configurations, it needs the
consumer device pointer instead of the consumer device name as argument.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/extcon/extcon-adc-jack.c|3 +--
drivers/iio/inkern.c| 11 ++-
drivers/power/generic-adc-battery.c |4 ++--
drivers/power/lp8788-charger.c |8
include/linux/iio/consumer.h|5 +++--
5 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index eda2a1a..d0233cd 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
;
data-num_conditions = i;
- data-chan = iio_channel_get(dev_name(pdev-dev),
- pdata-consumer_channel);
+ data-chan = iio_channel_get(pdev-dev, pdata-consumer_channel);
if (IS_ERR(data-chan)) {
err = PTR_ERR(data-chan);
goto out;
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c42aba6..b289915 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -93,7 +93,8 @@ static const struct iio_chan_spec
}
-struct iio_channel *iio_channel_get(const char *name, const char *channel_name)
+static struct iio_channel *iio_channel_get_sys(const char *name,
+ const char *channel_name)
{
struct iio_map_internal *c_i = NULL, *c = NULL;
struct iio_channel *channel;
@@ -144,6 +145,14 @@ error_no_mem:
iio_device_put(c-indio_dev);
return ERR_PTR(err);
}
+
+struct iio_channel *iio_channel_get(struct device *dev,
+ const char *channel_name)
+{
+ const char *name = dev ? dev_name(dev) : NULL;
+
+ return iio_channel_get_sys(name, channel_name);
+}
EXPORT_SYMBOL_GPL(iio_channel_get);
void iio_channel_release(struct iio_channel *channel)
diff --git a/drivers/power/generic-adc-battery.c
b/drivers/power/generic-adc-battery.c
index 32ce17e..42733c4 100644
--- a/drivers/power/generic-adc-battery.c
+++ b/drivers/power/generic-adc-battery.c
@@ -287,8 +287,8 @@ static int gab_probe(struct platform_device *pdev)
* based on the channel supported by consumer device.
*/
for (chan = 0; chan ARRAY_SIZE(gab_chan_name); chan++) {
- adc_bat-channel[chan] = iio_channel_get(dev_name(pdev-dev),
- gab_chan_name[chan]);
+ adc_bat-channel[chan] = iio_channel_get(pdev-dev,
+gab_chan_name[chan]);
if (IS_ERR(adc_bat-channel[chan])) {
ret = PTR_ERR(adc_bat-channel[chan]);
} else {
diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
index 22b6407c..2788908 100644
--- a/drivers/power/lp8788-charger.c
+++ b/drivers/power/lp8788-charger.c
@@ -580,7 +580,7 @@ static void lp8788_irq_unregister(struct platform_device
*pdev,
}
}
-static void lp8788_setup_adc_channel(const char *consumer_name,
+static void lp8788_setup_adc_channel(struct device *dev,
struct lp8788_charger *pchg)
{
struct lp8788_charger_platform_data *pdata = pchg-pdata;
@@ -590,11 +590,11 @@ static void lp8788_setup_adc_channel(const char
*consumer_name,
return;
/* ADC channel for battery voltage */
- chan = iio_channel_get(consumer_name, pdata-adc_vbatt);
+ chan = iio_channel_get(dev, pdata-adc_vbatt);
pchg-chan[LP8788_VBATT] = IS_ERR(chan) ? NULL : chan;
/* ADC channel for battery temperature */
- chan = iio_channel_get(consumer_name, pdata-adc_batt_temp);
+ chan = iio_channel_get(dev, pdata-adc_batt_temp);
pchg-chan[LP8788_BATT_TEMP] = IS_ERR(chan) ? NULL : chan;
}
@@ -704,7 +704,7 @@ static int lp8788_charger_probe(struct platform_device
*pdev)
if (ret)
return ret;
- lp8788_setup_adc_channel(pdev-name, pchg);
+ lp8788_setup_adc_channel(pdev-dev, pchg);
ret = lp8788_psy_register(pdev, pchg);
if (ret)
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index a85787a..833926c 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -31,14 +31,15 @@ struct iio_channel {
/**
* iio_channel_get() - get description of all that is needed to access channel.
- * @name: Unique name of the device as provided in the iio_map
+ * @dev: Pointer to consumer device. Device name must match
+ * the name of the device as provided in the iio_map
* with which the desired provider to consumer mapping
* was registered.
* @consumer_channel: Unique name
[PATCH v3 0/2] iio: Devicetree support
This patch set adds basic device tree support to the IIO subsystem. It is the result of discussions [1] and [2]. Patch 1 changes the first parameter to iio_channel_get() to be the pointer to the consumer device instead of the consumer device name. Patch 2 adds basic OF support to the IIO subsystem. Support is modeled after the clock subsystem. The patches are now based on the to togreg branch of iio kernel repository. Already applied patches are no longer part of the patch set. I increased the audience to all affected subsystems. Sorry for missing out power and extcon earlier, which are both affected by the first patch. Guenter --- [1] http://marc.info/?l=linux-iiom=135902119507483w=2 [2] http://marc.info/?l=lm-sensorsm=135375101529918w=1 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v3 2/2] iio: Add OF support
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
One open question is how to assign of_node to the iio device. We can either do
it
in each driver (which turns out to be a huge patchset), or add something like
the
following to iio_device_register.
if (!indio_dev-dev.of_node indio_dev-dev.parent)
indio_dev-dev.of_node = indio_dev-dev.parent-of_node;
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other device
types) and make it global for that purpose. Check the OF node first, then the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
an error if there is a problem with consistency or if the provider device is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
not used in the code, and it is not entirely clear what it would be used for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
.../devicetree/bindings/iio/iio-bindings.txt | 90 +++
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|4 +-
drivers/iio/inkern.c | 171
4 files changed, 264 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+ with a single IIO output and 1 for nodes with multiple
+ IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+ adc: voltage-sensor@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+Example for a configuration with trigger:
+
+ adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+
+ adc: iio-device {
+ #io-channel-cells = 1;
+ };
+ trigger: iio-trigger {
+ #io-channel-cells = 1;
+ };
+ };
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #io-channel-cells,
+ then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can inherit named
+ IIO channels from this node. Useful for bus nodes to provide
+ and IIO channel to their children.
+
+For example:
+
+ device {
+ io-channels = adc 1, ref 0;
+ io-channel-names
Re: [PATCH v3 2/2] iio: Add OF support
On Mon, Feb 04, 2013 at 12:26:06PM -0800, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
Note: One of the parameters to the dummy function of_iio_channel_get_by_name()
used when CONFIG_OF is undefined is wrong. I'll fix that in the next version of
the patch.
---
One open question is how to assign of_node to the iio device. We can either
do it
in each driver (which turns out to be a huge patchset), or add something like
the
following to iio_device_register.
if (!indio_dev-dev.of_node indio_dev-dev.parent)
indio_dev-dev.of_node = indio_dev-dev.parent-of_node;
v3:
- Cleaned up documentation (formatting, left-over clock references)
- Updated bindings description to permit sub-devices
- When searching for iio devices, use the pointer to the iio device type
instead
of strcmp. Rename iio_dev_type to iio_device_type (to match other device
types) and make it global for that purpose. Check the OF node first, then
the
device type, as the node is less likely to match.
- Move the common code in of_iio_channel_get and of_iio_channel_get_all to
__of_iio_channel_get.
- Return NULL from of_iio_channel_get_by_name if nothing is found, or
an error if there is a problem with consistency or if the provider device is
not yet available.
- In iio_channel_get, return if of_iio_channel_get_by_name() returns a channel
or an error, and continue otherwise.
v2:
- Rebased to iio/togreg
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property is
not used in the code, and it is not entirely clear what it would be used
for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still
builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations
- Use regulator to get vref for max1363 driver.
.../devicetree/bindings/iio/iio-bindings.txt | 90 +++
drivers/iio/iio_core.h |1 +
drivers/iio/industrialio-core.c|4 +-
drivers/iio/inkern.c | 171
4 files changed, 264 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..2475c2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,90 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the IIO provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for nodes
+with a single IIO output and 1 for nodes with multiple
+IIO outputs.
+
+Example for a simple configuration with no trigger:
+
+ adc: voltage-sensor@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+Example for a configuration with trigger:
+
+ adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+
+ adc: iio-device {
+ #io-channel-cells = 1;
+ };
+ trigger: iio-trigger {
+ #io-channel-cells = 1;
+ };
+ };
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #io-channel-cells,
+ then only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges
Re: [PATCH v2 4/4] iio: Add OF support
On Sun, Feb 03, 2013 at 03:17:57PM +0100, Lars-Peter Clausen wrote:
On 02/03/2013 01:59 AM, Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The property
is
not used in the code, and it is not entirely clear what it would be used
for.
If there is a need for it, we can add it back in later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still
builds
if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of
device
name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations.
From my point of view this looks good in general now, Just a few comments.
.../devicetree/bindings/iio/iio-bindings.txt | 76
drivers/iio/inkern.c | 186
2 files changed, 262 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..58df5f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,76 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the clock provider node.
+
Is the extra space at the begining of each sentence on purpose?
Copied from clock code, and used throughout documentation here or there.
Not my style, really, and I'll likely forget to do it if/when I add text myself,
so I'll change it.
[ ... ]
+ iio_hwmon {
+ compatible = iio-hwmon;
+ io-channels = adc 0, adc 1, adc 2,
+ adc 3, adc 4, adc 5,
+ adc 6, adc 7, adc 8,
+ adc 9, adc 10, adc 11;
I'm not sure how much sense this example makes, since you can only request
those channels which have a name.
It does - of_iio_channel_get_all() returns all channels even if there are no
names.
[...]
+
+static struct iio_channel *of_iio_channel_get_all(struct device *dev)
+{
+ struct iio_channel *chans;
+ int i, mapind, nummaps = 0;
+ int ret;
+
+ do {
+ ret = of_parse_phandle_with_args(dev-of_node,
+io-channels,
+#io-channel-cells,
+nummaps, NULL);
+ if (ret 0)
+ break;
+ } while (++nummaps);
+
+ if (nummaps == 0) /* no error, return NULL to search map table */
+ return NULL;
+
+ /* NULL terminated array to save passing size */
+ chans = kzalloc(sizeof(*chans)*(nummaps + 1), GFP_KERNEL);
I think using kcalloc makes sense here.
that would leave chan-data uninitialized, and I would have to initialize it
explicitly. also, if additional fields are ever added, we would risk having
uninitialized fields. Using kzalloc avoids a potential future error case, so I
would prefer to keep it.
+ if (chans == NULL) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ /* Search for OF matches */
+ for (mapind = 0; mapind nummaps; mapind++) {
+ struct device *idev;
+ struct iio_dev *indio_dev;
+ int channel;
+ struct of_phandle_args iiospec;
+
+ ret = of_parse_phandle_with_args(dev-of_node,
+io-channels,
+#io-channel-cells,
+mapind, iiospec);
+ if (ret)
+ goto error_free_chans;
+
+ idev = bus_find_device(iio_bus_type, NULL, iiospec.np,
+ iio_dev_node_match);
+ of_node_put(iiospec.np);
+ if (idev == NULL) {
+ ret = -EPROBE_DEFER;
+ goto error_free_chans;
+ }
+ indio_dev = dev_to_iio_dev(idev);
+ channel = iiospec.args_count ? iiospec.args[0] : 0
Re: [RFC 10/11] iio: Add OF support
On Sun, Feb 03, 2013 at 11:57:53AM +, Jonathan Cameron wrote:
On 02/03/2013 11:52 AM, Lars-Peter Clausen wrote:
On 02/03/2013 12:47 PM, Lars-Peter Clausen wrote:
On 02/03/2013 12:39 PM, Jonathan Cameron wrote:
On 02/02/2013 04:10 PM, Guenter Roeck wrote:
On Sat, Feb 02, 2013 at 10:29:02AM +, Jonathan Cameron wrote:
On 01/31/2013 09:43 PM, Guenter Roeck wrote:
Provide bindings, new API access functions, and parse OF data
during initialization.
Firstly thanks for working on this Guenter, it's been a big hole
for a while largely because non of our largest developers were
actually using development platforms with device tree support.
Given my knowledge of device tree is based on the odd article
and looking at similar sets of bindings this morning, my comments
are likely to be somewhat superficial and uninformed ;)
Mostly on this one I'll take a back seat and let those who
know this stuff better come to a consensus.
Jonathan
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
.../devicetree/bindings/iio/iio-bindings.txt | 97
drivers/iio/inkern.c | 241
include/linux/iio/consumer.h |8 +
3 files changed, 299 insertions(+), 47 deletions(-)
create mode 100644
Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..0f51c95
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,97 @@
+This binding is a work-in-progress, and are based on clock bindings
and
+suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one more more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the clock provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0
for nodes
+ with a single IIO output and 1 for nodes with
multiple
+ IIO outputs.
+
+Optional properties:
+io-channel-output-names:
+ Recommended to be a list of strings of IIO output
signal
+ names indexed by the first cell in the IIO
specifier.
+ However, the meaning of io-channel-output-names is
domain
+ specific to the IIO provider, and is only provided
to
+ encourage using the same meaning for the majority
of IIO
+ providers. This format may not work for IIO
providers
+ using a complex IIO specifier format. In those
cases it
+ is recommended to omit this property and create a
binding
+ specific names property.
+
+ IIO consumer nodes must never directly reference
+ the provider's io-channel-output-names property.
+
+For example:
+
+adc: adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+#io-channel-cells = 1;
+io-channel-output-names = adc1, adc2;
+};
+
+- this node defines a device with two named IIO outputs, the first
named
+ adc1 and the second named adc2. Consumer nodes always reference
+ IIO channels by index. The names should reflect the IIO output
signal
+ names for the device.
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #clock-cells, then
+ only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can inherit
named
+ IIO channels from this node. Useful for bus nodes to
provide
+ and IIO channel to their children.
+
+For example:
+
+device {
+io-channels = adc 1, ref 0;
+io-channel-names = vcc, vdd;
+};
+
+This represents a device with two IIO inputs, named vcc and vdd.
+The vcc
Re: [PATCH v2 4/4] iio: Add OF support
On Sun, Feb 03, 2013 at 12:29:23PM +0100, Lars-Peter Clausen wrote:
On 02/03/2013 03:06 AM, Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 02:30:24AM +0100, Tomasz Figa wrote:
Hi Guenter,
Some comments inline.
On Saturday 02 of February 2013 16:59:40 Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The
property is not used in the code, and it is not entirely clear what it
would be used for. If there is a need for it, we can add it back in
later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code still
builds if it is not selected.
- Change iio_channel_get to take device pointer as argument instead of
device name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations.
.../devicetree/bindings/iio/iio-bindings.txt | 76
drivers/iio/inkern.c | 186
2 files changed, 262 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt new file mode
100644
index 000..58df5f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,76 @@
+This binding is a work-in-progress. It is derived from clock bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the clock provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for
nodes + with a single IIO output and 1 for nodes with
multiple
+IIO outputs.
+
+For example:
+
+adc: adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+#io-channel-cells = 1;
+};
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #clock-cells, then
+ only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can inherit named
+ IIO channels from this node. Useful for bus nodes to provide
+ and IIO channel to their children.
+
+For example:
+
+device {
+io-channels = adc 1, ref 0;
+io-channel-names = vcc, vdd;
+};
+
+This represents a device with two IIO inputs, named vcc and vdd.
+The vcc channel is connected to output 1 of the adc device, and the
+vdd channel is connected to output 0 of the ref device.
+
+==Example==
+
+ adc: max1139@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+ ...
+
+ iio_hwmon {
+ compatible = iio-hwmon;
+ io-channels = adc 0, adc 1, adc 2,
+ adc 3, adc 4, adc 5,
+ adc 6, adc 7, adc 8,
+ adc 9, adc 10, adc 11;
+ io-channel-names = vcc, vdd, vref, 1.2V;
+ };
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index b289915..d48f2a8 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -10,6 +10,7 @@
#include linux/export.h
#include linux/slab.h
#include linux/mutex.h
+#include linux/of.h
#include linux/iio/iio.h
#include iio_core.h
@@ -92,6 +93,179 @@ static const struct iio_chan_spec
return chan;
}
+#ifdef CONFIG_OF
+
+static int iio_dev_node_match(struct device *dev, void *data)
+{
+ return !strcmp(dev-type-name, iio_device) dev-of_node ==
data;
Hmm, do you need to check type name here? One device node should rather
represent only one device, making node an unique identifier.
It this is meant to be a sanity check, it could
Re: [PATCH v2 4/4] iio: Add OF support
On Sun, Feb 03, 2013 at 12:52:40PM +0100, Tomasz Figa wrote:
On Sunday 03 of February 2013 12:29:23 Lars-Peter Clausen wrote:
On 02/03/2013 03:06 AM, Guenter Roeck wrote:
On Sun, Feb 03, 2013 at 02:30:24AM +0100, Tomasz Figa wrote:
Hi Guenter,
Some comments inline.
On Saturday 02 of February 2013 16:59:40 Guenter Roeck wrote:
Provide bindings and parse OF data during initialization.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
- Documentation update per feedback
- Dropped io-channel-output-names from the bindings document. The
property is not used in the code, and it is not entirely clear what
it
would be used for. If there is a need for it, we can add it back in
later on.
- Don't export OF specific API calls
- For OF support, no longer depend on iio_map
- Add #ifdef CONFIG_OF where appropriate, and ensure that the code
still builds if it is not selected.
- Change iio_channel_get to take device pointer as argument instead
of
device name. Retain old API as of_iio_channel_get_sys.
- iio_channel_get now works for both OF and non-OF configurations.
.../devicetree/bindings/iio/iio-bindings.txt | 76
drivers/iio/inkern.c | 186
2 files changed, 262 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt new file
mode
100644
index 000..58df5f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,76 @@
+This binding is a work-in-progress. It is derived from clock
bindings,
+and based on suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the
device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one or more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by
the
+value of a #io-channel-cells property in the clock provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0
for nodes +with a single IIO output and 1 for nodes with
multiple + IIO outputs.
+
+For example:
+
+adc: adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+#io-channel-cells = 1;
+};
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #clock-cells, then
+ only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can inherit
named
+ IIO channels from this node. Useful for bus nodes to
provide
+ and IIO channel to their children.
+
+For example:
+
+device {
+io-channels = adc 1, ref 0;
+io-channel-names = vcc, vdd;
+};
+
+This represents a device with two IIO inputs, named vcc and
vdd.
+The vcc channel is connected to output 1 of the adc device, and
the
+vdd channel is connected to output 0 of the ref device.
+
+==Example==
+
+ adc: max1139@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+ ...
+
+ iio_hwmon {
+ compatible = iio-hwmon;
+ io-channels = adc 0, adc 1, adc 2,
+ adc 3, adc 4, adc 5,
+ adc 6, adc 7, adc 8,
+ adc 9, adc 10, adc 11;
+ io-channel-names = vcc, vdd, vref, 1.2V;
+ };
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index b289915..d48f2a8 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -10,6 +10,7 @@
#include linux/export.h
#include linux/slab.h
#include linux/mutex.h
+#include linux/of.h
#include linux/iio/iio.h
#include iio_core.h
@@ -92,6 +93,179 @@ static const
Re: [PATCH 1/4] iio: max1363: Use devm_ functions whereever possible to allocate resources
On Sun, Feb 03, 2013 at 12:10:32PM +, Jonathan Cameron wrote: On 02/03/2013 12:59 AM, Guenter Roeck wrote: Signed-off-by: Guenter Roeck li...@roeck-us.net Applied to togreg branch of iio.git. Note I'll probably rebase the togreg branch if / when Greg has pulled last pull request (sent a few mins ago) so as to get a directly applied fix for this driver. No problem. There is some dead code in the max1363 driver. drivers/iio/adc/max1363.c:1492:12: warning: 'max1363_register_buffered_funcs_and_init' defined but not used [-Wunused-function] drivers/iio/adc/max1363.c:1527:13: warning: 'max1363_buffer_cleanup' defined but not used [-Wunused-function] I thought I had seen a note on the iio mailing list that you wanted to submit a patch for it, but I didn't see it yet. Or maybe my memory is wrong, or you were talking about something else. Want me to submit a patch (assuming the fix is simply to remove the above functions) ? Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/4] iio: max1363: Use devm_ functions whereever possible to allocate resources
On Sun, Feb 03, 2013 at 06:02:57PM +, Jonathan Cameron wrote: On 02/03/2013 05:18 PM, Guenter Roeck wrote: On Sun, Feb 03, 2013 at 12:10:32PM +, Jonathan Cameron wrote: On 02/03/2013 12:59 AM, Guenter Roeck wrote: Signed-off-by: Guenter Roeck li...@roeck-us.net Applied to togreg branch of iio.git. Note I'll probably rebase the togreg branch if / when Greg has pulled last pull request (sent a few mins ago) so as to get a directly applied fix for this driver. No problem. There is some dead code in the max1363 driver. drivers/iio/adc/max1363.c:1492:12: warning: 'max1363_register_buffered_funcs_and_init' defined but not used [-Wunused-function] drivers/iio/adc/max1363.c:1527:13: warning: 'max1363_buffer_cleanup' defined but not used [-Wunused-function] I thought I had seen a note on the iio mailing list that you wanted to submit a patch for it, Went out yesterday as [PATCH] iio:max1363 remove some functions left after merge but I didn't see it yet. Or maybe my memory is wrong, or you were talking about something else. It's gone to Greg who added it staging-next yesterday (rather than throw the iio tree). I actually intended to get his Ack on it (as the merge that caused it was I think his), but he applied it directly. Anyhow, that's the reason staging-next with that patch will differ from iio.git and hence why I'll rebase once the current pull request is dealt with. Want me to submit a patch (assuming the fix is simply to remove the above functions) ? All sorted if via an indirect route. Great, thanks. Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 10/11] iio: Add OF support
On Sat, Feb 02, 2013 at 10:29:02AM +, Jonathan Cameron wrote:
On 01/31/2013 09:43 PM, Guenter Roeck wrote:
Provide bindings, new API access functions, and parse OF data
during initialization.
Firstly thanks for working on this Guenter, it's been a big hole
for a while largely because non of our largest developers were
actually using development platforms with device tree support.
Given my knowledge of device tree is based on the odd article
and looking at similar sets of bindings this morning, my comments
are likely to be somewhat superficial and uninformed ;)
Mostly on this one I'll take a back seat and let those who
know this stuff better come to a consensus.
Jonathan
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
.../devicetree/bindings/iio/iio-bindings.txt | 97
drivers/iio/inkern.c | 241
include/linux/iio/consumer.h |8 +
3 files changed, 299 insertions(+), 47 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/iio-bindings.txt
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt
b/Documentation/devicetree/bindings/iio/iio-bindings.txt
new file mode 100644
index 000..0f51c95
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -0,0 +1,97 @@
+This binding is a work-in-progress, and are based on clock bindings and
+suggestions from Lars-Peter Clausen [1].
+
+Sources of IIO channels can be represented by any node in the device
+tree. Those nodes are designated as IIO providers. IIO consumer
+nodes use a phandle and IIO specifier pair to connect IIO provider
+outputs to IIO inputs. Similar to the gpio specifiers, an IIO
+specifier is an array of one more more cells identifying the IIO
+output on a device. The length of an IIO specifier is defined by the
+value of a #io-channel-cells property in the clock provider node.
+
+[1] http://marc.info/?l=linux-iiom=135902119507483w=2
+
+==IIO providers==
+
+Required properties:
+#io-channel-cells: Number of cells in an IIO specifier; Typically 0 for
nodes
+ with a single IIO output and 1 for nodes with multiple
+ IIO outputs.
+
+Optional properties:
+io-channel-output-names:
+ Recommended to be a list of strings of IIO output signal
+ names indexed by the first cell in the IIO specifier.
+ However, the meaning of io-channel-output-names is domain
+ specific to the IIO provider, and is only provided to
+ encourage using the same meaning for the majority of IIO
+ providers. This format may not work for IIO providers
+ using a complex IIO specifier format. In those cases it
+ is recommended to omit this property and create a binding
+ specific names property.
+
+ IIO consumer nodes must never directly reference
+ the provider's io-channel-output-names property.
+
+For example:
+
+adc: adc@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+#io-channel-cells = 1;
+io-channel-output-names = adc1, adc2;
+};
+
+- this node defines a device with two named IIO outputs, the first named
+ adc1 and the second named adc2. Consumer nodes always reference
+ IIO channels by index. The names should reflect the IIO output signal
+ names for the device.
+
+==IIO consumers==
+
+Required properties:
+io-channels: List of phandle and IIO specifier pairs, one pair
+ for each IIO input to the device. Note: if the
+ IIO provider specifies '0' for #clock-cells, then
+ only the phandle portion of the pair will appear.
+
+Optional properties:
+io-channel-names:
+ List of IIO input name strings sorted in the same
+ order as the io-channels property. Consumers drivers
+ will use io-channel-names to match IIO input names
+ with IIO specifiers.
+io-channel-ranges:
+ Empty property indicating that child nodes can inherit named
+ IIO channels from this node. Useful for bus nodes to provide
+ and IIO channel to their children.
+
+For example:
+
+device {
+io-channels = adc 1, ref 0;
+io-channel-names = vcc, vdd;
+};
+
+This represents a device with two IIO inputs, named vcc and vdd.
+The vcc channel is connected to output 1 of the adc device, and the
+vdd channel is connected to output 0 of the ref device.
+
+==Example==
+
+ adc: max1139@35 {
+ compatible = maxim,max1139;
+ reg = 0x35;
+ #io-channel-cells = 1;
+ };
+
+ ...
+
+ iio_hwmon {
+ compatible = iio-hwmon;
+ io-channels = adc 0, adc
Re: [RFC 11/11] iio/adc: (max1363) Add basic OF bindings and external vref support
On Sat, Feb 02, 2013 at 10:33:12AM +, Jonathan Cameron wrote: On 01/31/2013 09:43 PM, Guenter Roeck wrote: Signed-off-by: Guenter Roeck li...@roeck-us.net Mostly fine. Comments below are on the fact I'd prefer a reference voltage coming from a regulator than being a bit of platform data. --- Documentation/devicetree/bindings/iio/max1363.txt | 54 + drivers/iio/adc/max1363.c | 54 - 2 files changed, 95 insertions(+), 13 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/max1363.txt diff --git a/Documentation/devicetree/bindings/iio/max1363.txt b/Documentation/devicetree/bindings/iio/max1363.txt new file mode 100644 index 000..6d22861 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/max1363.txt @@ -0,0 +1,54 @@ +Device Tree bindings for MAX1363 and compatible ADC controllers + +This binding uses the common IIO binding[1]. + +[1] Documentation/devicetree/bindings/iio/iio-bindings.txt + +Required properties: + +- compatible, shall be one of the following: + maxim,max1361 + maxim,max1362 + maxim,max1363 + maxim,max1364 + maxim,max1036 + maxim,max1037 + maxim,max1038 + maxim,max1039 + maxim,max1136 + maxim,max1137 + maxim,max1138 + maxim,max1139 + maxim,max1236 + maxim,max1237 + maxim,max1238 + maxim,max1239 + maxim,max11600 + maxim,max11601 + maxim,max11602 + maxim,max11603 + maxim,max11604 + maxim,max11605 + maxim,max11606 + maxim,max11607 + maxim,max11608 + maxim,max11609 + maxim,max11610 + maxim,max11611 + maxim,max11612 + maxim,max11613 + maxim,max11614 + maxim,max11615 + maxim,max11616 + maxim,max11617 + +- reg: shall be the I2C device address + +Required properties for IIO bindings: +- #io-channel-cells: from common IIO bindings; shall be set to 1. + +Optional properties: +- vref: Reference voltage in mV. If the provided reference voltage matches + the internal reference voltage, the internal reference voltage is used. + Otherwise it is assumed that an external reference voltage is used, + and the chip is programmed accordingly. Why not use a regulator? It has a nice device tree map and if it's just a fixed voltage, we have the fixed regulator driver for them. This is pretty common throughout IIO (unsuprisingly) and we've been generally getting with platform data that does this in favour of regulators. Back when we started out, the regulators framework was new so providing an alternative was pretty much required. Now it's pretty universal. Makes sense' I'll look into it. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 10/11] iio: Add OF support
On Sat, Feb 02, 2013 at 03:37:46PM +0100, Lars-Peter Clausen wrote: On 02/01/2013 08:42 PM, Guenter Roeck wrote: On Fri, Feb 01, 2013 at 03:59:17PM +0100, Lars-Peter Clausen wrote: On 02/01/2013 03:33 PM, Guenter Roeck wrote: On Fri, Feb 01, 2013 at 12:58:02PM +0100, Lars-Peter Clausen wrote: 013 10:43 PM, Guenter Roeck wrote: Provide bindings, new API access functions, and parse OF data during initialization. Hi Guenter, Thanks for taking care of this. I'd prefer to have only one iio_get_channel which handles both the dt and the non-dt case. Otherwise we'll soon have constructs like if (dev-of_node) chan = of_iio_get_channel(dev-of_node, 1); else chan = iio_get_channel(dev_name(dev), vcc); Clock code has of_clk_get_by_name(node *, name), clk_get(dev, name), and of_clk_get(node *, index). Right now of_iio_get_channel matches of_clk_get, and iio_get_channel matches clk_get. Question is really how we want to API to look like. I am open to suggestions. I'm not necessarily against having a separate of_iio_get_channe function, but I'm not sure if we really need it, maybe use it internally as a helper and if we really need it in some driver make it public and export it. clk_get first calls of_clk_get_by_name, and only if didn't find a clk it falls back to the map based lookup. I think iio_get_channel should behave the similar. of_clk_get is kind of just a helper function for of_clk_get_by_name, not sure if we need it as a separate function in IIO. If we find out we need it we can probably still add it later. You are right. If I modify iio_get_channel to take the device pointer as argument, it should work for both OF and non-OF with the approach you outlined above. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[RFC v2 0/4] iio: Devicetree support
This patch set adds basic device tree support to the IIO subsystem. It is the result of the [1] and [2] discussions. The first two patches are actually updates to the MAX1363 driver. The first patch updates the driver to use devm_ functions, and the second patch adds support for an external reference voltage. Patch 3 changes the first parameter to iio_channel_get() to be the pointer to the consumer device instead of the consumer device name. Patch 4 adds basic OF support to the IIO subsystem. Support is modeled after the clock subsystem. The patches are now based on the to togreg branch of iio kernel repository. Several patches were already applied and are no longer part of the patch set. Guenter --- [1] http://marc.info/?l=linux-iiom=135902119507483w=2 [2] http://marc.info/?l=lm-sensorsm=135375101529918w=1 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 1/4] iio: max1363: Use devm_ functions whereever possible to allocate resources
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/iio/adc/max1363.c | 29 ++---
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
index ef868c9..1353fda 100644
--- a/drivers/iio/adc/max1363.c
+++ b/drivers/iio/adc/max1363.c
@@ -1410,8 +1410,9 @@ static int max1363_alloc_scan_masks(struct iio_dev
*indio_dev)
unsigned long *masks;
int i;
- masks = kzalloc(BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*sizeof(long)*
- (st-chip_info-num_modes + 1), GFP_KERNEL);
+ masks = devm_kzalloc(indio_dev-dev,
+ BITS_TO_LONGS(MAX1363_MAX_CHANNELS) * sizeof(long) *
+ (st-chip_info-num_modes + 1), GFP_KERNEL);
if (!masks)
return -ENOMEM;
@@ -1546,7 +1547,7 @@ static int max1363_probe(struct i2c_client *client,
st = iio_priv(indio_dev);
- st-reg = regulator_get(client-dev, vcc);
+ st-reg = devm_regulator_get(client-dev, vcc);
if (IS_ERR(st-reg)) {
ret = PTR_ERR(st-reg);
goto error_unregister_map;
@@ -1554,7 +1555,7 @@ static int max1363_probe(struct i2c_client *client,
ret = regulator_enable(st-reg);
if (ret)
- goto error_put_reg;
+ goto error_unregister_map;
/* this is only used for device removal purposes */
i2c_set_clientdata(client, indio_dev);
@@ -1575,15 +1576,15 @@ static int max1363_probe(struct i2c_client *client,
indio_dev-modes = INDIO_DIRECT_MODE;
ret = max1363_initial_setup(st);
if (ret 0)
- goto error_free_available_scan_masks;
+ goto error_disable_reg;
ret = iio_triggered_buffer_setup(indio_dev, NULL,
max1363_trigger_handler, max1363_buffered_setup_ops);
if (ret)
- goto error_free_available_scan_masks;
+ goto error_disable_reg;
if (client-irq) {
- ret = request_threaded_irq(st-client-irq,
+ ret = devm_request_threaded_irq(client-dev, st-client-irq,
NULL,
max1363_event_handler,
IRQF_TRIGGER_RISING | IRQF_ONESHOT,
@@ -1596,20 +1597,14 @@ static int max1363_probe(struct i2c_client *client,
ret = iio_device_register(indio_dev);
if (ret 0)
- goto error_free_irq;
+ goto error_uninit_buffer;
return 0;
-error_free_irq:
- if (client-irq)
- free_irq(st-client-irq, indio_dev);
+
error_uninit_buffer:
iio_triggered_buffer_cleanup(indio_dev);
-error_free_available_scan_masks:
- kfree(indio_dev-available_scan_masks);
error_disable_reg:
regulator_disable(st-reg);
-error_put_reg:
- regulator_put(st-reg);
error_unregister_map:
iio_map_array_unregister(indio_dev);
error_free_device:
@@ -1624,12 +1619,8 @@ static int max1363_remove(struct i2c_client *client)
struct max1363_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- if (client-irq)
- free_irq(st-client-irq, indio_dev);
iio_triggered_buffer_cleanup(indio_dev);
- kfree(indio_dev-available_scan_masks);
regulator_disable(st-reg);
- regulator_put(st-reg);
iio_map_array_unregister(indio_dev);
iio_device_free(indio_dev);
--
1.7.9.7
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v2 2/4] iio/adc: (max1363) Add support for external reference voltage
Implement external reference voltage as regulator named vref.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
v2: Use regulator API to specify vref instead of creating new devicetree
bindings.
Keep reference voltage internally in uV, as this is the scale provided
by the regulator subsystem. We need the value in uV anyway, so that does
make some sense.
drivers/iio/adc/max1363.c | 52 +
1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
index 1353fda..cac73d7 100644
--- a/drivers/iio/adc/max1363.c
+++ b/drivers/iio/adc/max1363.c
@@ -163,6 +163,8 @@ struct max1363_chip_info {
* @mask_low: bitmask for enabled low thresholds
* @thresh_high: high threshold values
* @thresh_low:low threshold values
+ * @vref: Reference voltage regulator
+ * @vref_uv: Actual (external or internal) reference voltage
*/
struct max1363_state {
struct i2c_client *client;
@@ -182,6 +184,8 @@ struct max1363_state {
/* 4x unipolar first then the fours bipolar ones */
s16 thresh_high[8];
s16 thresh_low[8];
+ struct regulator*vref;
+ u32 vref_uv;
};
#define MAX1363_MODE_SINGLE(_num, _mask) { \
@@ -393,6 +397,8 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
{
struct max1363_state *st = iio_priv(indio_dev);
int ret;
+ unsigned long scale_uv;
+
switch (m) {
case IIO_CHAN_INFO_RAW:
ret = max1363_read_single_chan(indio_dev, chan, val, m);
@@ -400,16 +406,10 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
return ret;
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
- if ((1 (st-chip_info-bits + 1))
- st-chip_info-int_vref_mv) {
- *val = 0;
- *val2 = 50;
- return IIO_VAL_INT_PLUS_MICRO;
- } else {
- *val = (st-chip_info-int_vref_mv)
-st-chip_info-bits;
- return IIO_VAL_INT;
- }
+ scale_uv = st-vref_uv st-chip_info-bits;
+ *val = scale_uv / 1000;
+ *val2 = (scale_uv % 1000) * 1000;
+ return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
@@ -1390,12 +1390,16 @@ static const struct max1363_chip_info
max1363_chip_info_tbl[] = {
static int max1363_initial_setup(struct max1363_state *st)
{
- st-setupbyte = MAX1363_SETUP_AIN3_IS_AIN3_REF_IS_VDD
- | MAX1363_SETUP_POWER_UP_INT_REF
- | MAX1363_SETUP_INT_CLOCK
+ st-setupbyte = MAX1363_SETUP_INT_CLOCK
| MAX1363_SETUP_UNIPOLAR
| MAX1363_SETUP_NORESET;
+ if (st-vref)
+ st-setupbyte |= MAX1363_SETUP_AIN3_IS_REF_EXT_TO_REF;
+ else
+ st-setupbyte |= MAX1363_SETUP_POWER_UP_INT_REF
+ | MAX1363_SETUP_AIN3_IS_AIN3_REF_IS_INT;
+
/* Set scan mode writes the config anyway so wait until then */
st-setupbyte = MAX1363_SETUP_BYTE(st-setupbyte);
st-current_mode = max1363_mode_table[st-chip_info-default_mode];
@@ -1533,6 +1537,7 @@ static int max1363_probe(struct i2c_client *client,
int ret;
struct max1363_state *st;
struct iio_dev *indio_dev;
+ struct regulator *vref;
indio_dev = iio_device_alloc(sizeof(struct max1363_state));
if (indio_dev == NULL) {
@@ -1563,6 +1568,23 @@ static int max1363_probe(struct i2c_client *client,
st-chip_info = max1363_chip_info_tbl[id-driver_data];
st-client = client;
+ st-vref_uv = st-chip_info-int_vref_mv * 1000;
+ vref = devm_regulator_get(client-dev, vref);
+ if (!IS_ERR(vref)) {
+ int vref_uv;
+
+ ret = regulator_enable(vref);
+ if (ret)
+ goto error_disable_reg;
+ st-vref = vref;
+ vref_uv = regulator_get_voltage(vref);
+ if (vref_uv = 0) {
+ ret = -EINVAL;
+ goto error_disable_reg;
+ }
+ st-vref_uv = vref_uv;
+ }
+
ret = max1363_alloc_scan_masks(indio_dev);
if (ret)
goto error_disable_reg;
@@ -1604,6 +1626,8 @@ static int max1363_probe(struct i2c_client *client,
error_uninit_buffer:
iio_triggered_buffer_cleanup(indio_dev);
error_disable_reg:
+ if (st-vref)
+ regulator_disable(st-vref);
regulator_disable(st-reg);
error_unregister_map:
iio_map_array_unregister(indio_dev);
@@ -1620,6 +1644,8
[PATCH RFC 3/4] iio: Update iio_channel_get API to use consumer device pointer as argument
For iio_channel_get to work with OF based configurations, it needs the
consumer device pointer instead of the consumer device name as argument.
Signed-off-by: Guenter Roeck li...@roeck-us.net
---
drivers/extcon/extcon-adc-jack.c|3 +--
drivers/iio/inkern.c| 11 ++-
drivers/power/generic-adc-battery.c |4 ++--
drivers/power/lp8788-charger.c |8
include/linux/iio/consumer.h|5 +++--
5 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index eda2a1a..d0233cd 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -135,8 +135,7 @@ static int adc_jack_probe(struct platform_device *pdev)
;
data-num_conditions = i;
- data-chan = iio_channel_get(dev_name(pdev-dev),
- pdata-consumer_channel);
+ data-chan = iio_channel_get(pdev-dev, pdata-consumer_channel);
if (IS_ERR(data-chan)) {
err = PTR_ERR(data-chan);
goto out;
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c42aba6..b289915 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -93,7 +93,8 @@ static const struct iio_chan_spec
}
-struct iio_channel *iio_channel_get(const char *name, const char *channel_name)
+static struct iio_channel *iio_channel_get_sys(const char *name,
+ const char *channel_name)
{
struct iio_map_internal *c_i = NULL, *c = NULL;
struct iio_channel *channel;
@@ -144,6 +145,14 @@ error_no_mem:
iio_device_put(c-indio_dev);
return ERR_PTR(err);
}
+
+struct iio_channel *iio_channel_get(struct device *dev,
+ const char *channel_name)
+{
+ const char *name = dev ? dev_name(dev) : NULL;
+
+ return iio_channel_get_sys(name, channel_name);
+}
EXPORT_SYMBOL_GPL(iio_channel_get);
void iio_channel_release(struct iio_channel *channel)
diff --git a/drivers/power/generic-adc-battery.c
b/drivers/power/generic-adc-battery.c
index 32ce17e..42733c4 100644
--- a/drivers/power/generic-adc-battery.c
+++ b/drivers/power/generic-adc-battery.c
@@ -287,8 +287,8 @@ static int gab_probe(struct platform_device *pdev)
* based on the channel supported by consumer device.
*/
for (chan = 0; chan ARRAY_SIZE(gab_chan_name); chan++) {
- adc_bat-channel[chan] = iio_channel_get(dev_name(pdev-dev),
- gab_chan_name[chan]);
+ adc_bat-channel[chan] = iio_channel_get(pdev-dev,
+gab_chan_name[chan]);
if (IS_ERR(adc_bat-channel[chan])) {
ret = PTR_ERR(adc_bat-channel[chan]);
} else {
diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
index 22b6407c..2788908 100644
--- a/drivers/power/lp8788-charger.c
+++ b/drivers/power/lp8788-charger.c
@@ -580,7 +580,7 @@ static void lp8788_irq_unregister(struct platform_device
*pdev,
}
}
-static void lp8788_setup_adc_channel(const char *consumer_name,
+static void lp8788_setup_adc_channel(struct device *dev,
struct lp8788_charger *pchg)
{
struct lp8788_charger_platform_data *pdata = pchg-pdata;
@@ -590,11 +590,11 @@ static void lp8788_setup_adc_channel(const char
*consumer_name,
return;
/* ADC channel for battery voltage */
- chan = iio_channel_get(consumer_name, pdata-adc_vbatt);
+ chan = iio_channel_get(dev, pdata-adc_vbatt);
pchg-chan[LP8788_VBATT] = IS_ERR(chan) ? NULL : chan;
/* ADC channel for battery temperature */
- chan = iio_channel_get(consumer_name, pdata-adc_batt_temp);
+ chan = iio_channel_get(dev, pdata-adc_batt_temp);
pchg-chan[LP8788_BATT_TEMP] = IS_ERR(chan) ? NULL : chan;
}
@@ -704,7 +704,7 @@ static int lp8788_charger_probe(struct platform_device
*pdev)
if (ret)
return ret;
- lp8788_setup_adc_channel(pdev-name, pchg);
+ lp8788_setup_adc_channel(pdev-dev, pchg);
ret = lp8788_psy_register(pdev, pchg);
if (ret)
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index a85787a..833926c 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -31,14 +31,15 @@ struct iio_channel {
/**
* iio_channel_get() - get description of all that is needed to access channel.
- * @name: Unique name of the device as provided in the iio_map
+ * @dev: Pointer to consumer device. Device name must match
+ * the name of the device as provided in the iio_map
* with which the desired provider to consumer mapping
* was registered.
* @consumer_channel: Unique name
