linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Jonathan Cameron <jic23@kernel.org>,
	Jonathan Cameron <jic23@jic23.retrosnub.co.uk>,
	linux-iio@vger.kernel.org, Jean Delvare <khali@linux-fr.org>,
	lm-sensors@lm-sensors.org
Subject: Re: MAX1363 IIO driver questions
Date: Tue, 29 Jan 2013 21:10:25 +0100	[thread overview]
Message-ID: <51082CB1.1050401@metafoo.de> (raw)
In-Reply-To: <20130129200055.GA18207@roeck-us.net>

On 01/29/2013 09:00 PM, Guenter Roeck wrote:
> On Sat, Jan 26, 2013 at 11:25:35AM +0000, Jonathan Cameron wrote:
>> On 01/18/2013 10:09 PM, Guenter Roeck wrote:
>>> On Thu, Jan 17, 2013 at 09:03:58PM +0000, Jonathan Cameron wrote:
>>>>
>>>> Good point. New use case for us so suggestions on how to do the association cleanly would be most welcome. Is there anything similar out there? We could add a per iio device sysfs interface to add additional mappings but it is a little uggly...
>>>>
>>>
>>> Best idea I can come up with is to disconnect iio_hwmon from the requirement to
>>> instantiate it explicitly. There might be two sysfs entries - one to
>>> attach it to a specific iio device, and one to attach it to individual channels
>>> of an iio device. Similar like the new_device interface on i2c adapters, and
>>> along the line of
>>>
>>> echo max1139 something > /sys/module/iio_hwmon/something_else
>>
>> We'll have to have something more specific or the common case of more than
>> one instance of an adc will cause trouble.  Obviously this doesn't matter
>> doing by adding the map from the IIO side.
>>
>>>
>>> and/or
>>>
>>> echo max1139 something channel > /sys/module/iio_hwmon/something_else
>>>
>>> ie sysfs attributes associated with iio_hwmon, not with the iio device itself.
>>>
>> This will play havock with the way the internal mappings work.  Originally
>> we had it mapped from both sides by name (e.g. the map wasn't in any way
>> handled by either driver) but that got an awful lot of flack and really
>> wasn't considered acceptable.  The current version of treating it much like
>> regulators etc is much cleaner.
>>
> 
> I think I am giving up on testing the code on a non-embedded system;
> I would need/use manual instantiation only for testing, and it seems too
> difficult to implement and not really worth it. I'll focus on getting it
> to work with OF.
> 
> The current approach, with iio_hwmon requesting its assigned mappings through 
> io_channel_get_all(), does not work well for me for a number of reasons.
> 
> First, it is difficult to associate device references in OF with actual device
> names. I don't know if you have tried, but while a reference to &iio_hwmon can
> uniquely identify the device name for an OF entry such as
> 
> 	iio_hwmon: iio_hwmon@0 {
> 		compatible = "iio-hwmon";
> 	};
> 
> it is difficult to predict how the actual iio_hwmon device name looks like.
> Amongst others, it depends if there are additional attributes such as "reg = <>",
> on the value of "@x" (if specified) and other attributes I have not really tracked
> down yet. In other words, when I tried to create a device named "iio_hwmon.0",
> I managed to get all kinds of device names except for "iio_hwmon.0".
> 
> Also, the iio_hwmon driver does not know which consumers are assigned to it.
> If it is instantiated before the ADC driver (which happens all the time for me,
> as iio_hwmon does not have to wait for the i2c bus adapter), its call to
> iio_channel_get_all() returns nothing. Even if it does return some mappings,
> there is no guarantee that the mappings are complete (eg if an instance of
> iio_hwmon is mapped to ADC channels from multiple chips).
> 
> Other subsystems solve that problem by having the consumer request the resources
> it needs. The leds-gpio driver is an excellent example: It knows from its OF data
> which gpio pins it needs and requests those. If the pins are not available,
> it gets an -EPROBE_DEFER error from the gpio subsystem, and simply defers
> its probe until the missing pins are available.
> 
> The question for me is really if it would be possible to implement the same
> approach for the iio subsystem. I would then specify something like
> 
> 	max1139: max1139@35 {
>         	compatible = "maxim,max1139";
> 		reg = <0x35>;
> 	};
> 
> ...
> 	iio_hwmon {
> 		compatible = "iio-hwmon";
> 
> 		in0 {
> 			label = "vin";
> 			iio-map = { &max1139 0 }; /* adc channel 0 */
> 		};
> 		in1 {
> 			label = "vout";
> 			iio-map = { &max1139 1 }; /* adc channel 1 */
> 		};
> 		...
> 	};
> 
> which would then map into in0/in1 hwmon attributes (with optional "vin" and
> "vout" labels if specified).
> 
> Problem here is that io_channel_get() currently does not use the provider name
> as argument to find the resource. Instead, it uses consumer_dev_name and/or
> consumer_channel. I am not sure how to solve that problem. It would be much more
> helpful if the provider would not be tied to the consumer from provider side,
> but from consumer side, and the mapping would be based on provider device and
> index (or something else such as ADC channel name if that is preferred).
> 
> Would this kind of solution be acceptable for the iio maintainers ? Is it
> even possible, given that the provider has to currently provide the mapping
> to its intended consumers using iio_map_array_register() ?

Hi Guenter,

I wrote in another mail a few days ago, how I think dt bindings for IIO could
be implemented. The basic idea was to simply use bindings very similar to what
the clk API uses, since its provider/consumer structure actually matches what
we do in IIO pretty good.

The full mail can be found here:
http://marc.info/?l=linux-iio&m=135902119507483&w=2

- Lars

  reply	other threads:[~2013-01-29 20:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-24  5:27 MAX1363 IIO driver questions Guenter Roeck
2012-11-24 10:18 ` Jonathan Cameron
2012-11-24 11:12   ` Guenter Roeck
2013-01-16  5:49   ` Guenter Roeck
2013-01-16  8:41     ` Jonathan Cameron
     [not found]       ` <20130117204549.GB22045@roeck-us.net>
2013-01-17 21:03         ` Jonathan Cameron
2013-01-17 21:37           ` Userspace IIO map instantiation [was MAX1363 IIO driver questions] Jonathan Cameron
2013-01-18 22:09           ` MAX1363 IIO driver questions Guenter Roeck
2013-01-26 11:25             ` Jonathan Cameron
2013-01-29 20:00               ` Guenter Roeck
2013-01-29 20:10                 ` Lars-Peter Clausen [this message]
2013-01-29 21:05                   ` Guenter Roeck
2013-01-29 21:21                     ` Lars-Peter Clausen
2013-01-29 21:46                       ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51082CB1.1050401@metafoo.de \
    --to=lars@metafoo.de \
    --cc=jic23@jic23.retrosnub.co.uk \
    --cc=jic23@kernel.org \
    --cc=khali@linux-fr.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lm-sensors@lm-sensors.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).