Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v4 4/8] drivers:input:tsc2007: add iio interface to read external ADC input and temperature
From: H. Nikolaus Schaller @ 2016-10-23 19:11 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Mark Rutland, devicetree, linux-omap, Arnd Bergmann, kernel,
	Tony Lindgren, linux-kernel, Mark Brown, Dmitry Torokhov,
	Russell King, linux-iio, Sebastian Reichel,
	Javier Martinez Canillas, Rob Herring, Mika Penttilä,
	Benoît Cousson, linux-input, Michael Welling, letux-kernel,
	Andrew F. Davis, Igor Grinberg
In-Reply-To: <4b6773d9-bc33-7471-ee29-01f51c5cfce2@kernel.org>

Hi,

> Am 23.10.2016 um 21:00 schrieb Jonathan Cameron <jic23@kernel.org>:
> 
> On 23/10/16 19:34, H. Nikolaus Schaller wrote:
>> Hi Jonathan,
>> 
>>> Am 23.10.2016 um 11:57 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>>> 
>>> Hi,
>>> 
>>>>> +static int tsc2007_alloc(struct i2c_client *client, struct tsc2007 **ts,
>>>>> +                          struct input_dev **input_dev)
>>>>> +{
>>>>> +       int err;
>>>>> +       struct iio_dev *indio_dev;
>>>>> +
>>>>> +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*ts));
>>>> Instead of doing this to reduce the delta between versions make 
>>>> iio_priv a struct tsc2007 **
>>>> 
>>>> That is have a single pointer in there and do your allocation of struct
>>>> tsc2007 separately.
>>> 
>>> Sorry, but I think I do not completely understand what you mean here.
>>> 
>>> The problem is that we need to allocate some struct tsc2007 in both cases.
>>> But in one case managed directly by &client->dev and in the other managed
>>> indirectly. This is why I use the private area of struct iio_dev to store
>>> the full struct tsc2007 and not just a pointer.
>> 
>> Ok, I think I finally did understand how you mean this and have started to
>> implement something.
>> 
> oops. Didn't look on in my emails to get to this one!
>> The idea is to have one alloc function to return a struct tsc2007. This
>> can be part of the probe function, like it is in the unpatched driver.
>> 
>> In case of iio this struct tsc2007 is also allocated explicitly so that
>> a pointer can be stored in iio_priv.
>> 
>> This just means an additional iio_priv->ts = devm_kzalloc() in case of iio.
>> 
>> I have added that approach to my inlined patch and it seems to work (attached).
>> 
>> Sorry if I do not use the wording you would use and sometimes overlook
>> something you have said. I feel here like moving on thin ice and doing
>> guesswork about unspoken assumptions...
> That's fine.  Stuff that can appear obvious to one person is not
> necessarily obvious to another!
>> 
>>> 
>>>> 
>>>> Having doing that, you can have this CONFIG_IIO block as just
>>>> doing the iio stuff with the input elements pulled back into the main
>>>> probe function.
>>>> 
>>>> Then define something like
>>>> 
>>>> iio_configure (stubbed to nothing if no IIO)
>>>> and
>>>> iio_unconfigure (also stubbed to nothing if no IIO).
>> 
>> This seems to work (draft attached).
>> 
>>>> 
>>>> A couple of additions in the header
>> 
>> I think you mean tsc2007.h?
> Nope. A local header alongside the driver is what you want for this stuff.
> driver/input/tsc2007.h 

Ah, ok. This makes sense.

>> 
>> This currently contains only platform data and could IMHO be eliminated
>> if everything becomes DT.
>> 
>>>> to make it all work
>>>> (the struct tsc2007 and tsc2007_xfer() + a few of the
>>>> register defines..
>> 
>> Here it appears to me that I have to make a lot of so far private static
>> and even static inline functions public so that I can make them stubs and
>> call them from tsc2007_iio.c.
> There will be a few.
>> 
>> And for having proper parameter types I have to make most private structs
>> also public.
> Yes a few of those as well.
>> 
>> I really like the idea to have the optional iio feature in a separate source
>> file, but when really starting to write code, I get the impression that
>> it introduces more problems than it solves.
>> 
>> And I wonder a little why it is not done for #ifdef CONFIG_OF in tsc2007.c
>> as well. There are also two static function in some #ifdef #else # endif
>> and not going through stubs.
> Usually it is only done once a certain volume of code exists.
>> 
>> So is this intended to give up some static definitions?
> Yes, that happens the moment you have multiple source files.
> 
> Some losses but generally end up with clean code separation. Always a trade
> off unfortunately.  Pity we can't just insist IIO is available! Rather large
> to pull in for what is probable a niche use case.
> 
> Below is definitely heading in the right direction. I remember vaguely being
> convinced of the worth of doing this when optional code is involved!
> (was a good while ago now)

Ok. I think I can now integrate it and then post as PATCH v5 (together with
some other fixes for the patch set).

> 
> Jonathan
>> 
>> BR and thanks,
>> Nikolaus
>> 
>> diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
>> index 5e3c4bf..92da8f6 100644
>> --- a/drivers/input/touchscreen/tsc2007.c
>> +++ b/drivers/input/touchscreen/tsc2007.c
>> @@ -30,6 +30,7 @@
>> #include <linux/of.h>
>> #include <linux/of_gpio.h>
>> #include <linux/input/touchscreen.h>
>> +#include <linux/iio/iio.h>
> Should not need this after introducing the new file.  Will only be
> needed in the iio specific .c file.
>> 
>> #define TSC2007_MEASURE_TEMP0          (0x0 << 4)
>> #define TSC2007_MEASURE_AUX            (0x2 << 4)
>> @@ -98,6 +99,9 @@ struct tsc2007 {
> This will definitely need to go in the header though.
>> 
>>        int                     (*get_pendown_state)(struct device *);
>>        void                    (*clear_penirq)(void);
>> +
>> +       struct mutex            mlock;
>> +       void                    *private;
>> };
>> 
>> static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>> @@ -192,7 +196,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
>>        while (!ts->stopped && tsc2007_is_pen_down(ts)) {
>> 
>>                /* pen is down, continue with the measurement */
>> +
>> +               mutex_lock(&ts->mlock);
>>                tsc2007_read_values(ts, &tc);
>> +               mutex_unlock(&ts->mlock);
>> 
>>                rt = tsc2007_calculate_resistance(ts, &tc);
>> 
>> @@ -319,6 +326,157 @@ static void tsc2007_close(struct input_dev *input_dev)
>>        tsc2007_stop(ts);
>> }
>> 
>> +#ifdef CONFIG_IIO
>> +
>> +struct tsc2007_iio {
>> +       struct tsc2007 *ts;
> Spot on.
>> +};
>> +
>> +#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
>> +{ \
>> +       .datasheet_name = _name, \
>> +       .type = _type, \
>> +       .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
>> +                       BIT(_chan_info), \
>> +       .indexed = 1, \
>> +       .channel = _chan, \
>> +}
>> +
>> +static const struct iio_chan_spec tsc2007_iio_channel[] = {
>> +       TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> +       TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> +       TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> +       TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> +       TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>> +       TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */
>> +       TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),
>> +       TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
>> +       TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),
>> +};
>> +
>> +static int tsc2007_read_raw(struct iio_dev *indio_dev,
>> +       struct iio_chan_spec const *chan, int *val, int *val2, long mask)
>> +{
>> +       struct tsc2007_iio *iio = iio_priv(indio_dev);
>> +       struct tsc2007 *tsc = iio->ts;
>> +       int adc_chan = chan->channel;
>> +       int ret = 0;
>> +
>> +       if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
>> +               return -EINVAL;
>> +
>> +       if (mask != IIO_CHAN_INFO_RAW)
>> +               return -EINVAL;
>> +
>> +       mutex_lock(&tsc->mlock);
>> +
>> +       switch (chan->channel) {
>> +       case 0:
>> +               *val = tsc2007_xfer(tsc, READ_X);
>> +               break;
>> +       case 1:
>> +               *val = tsc2007_xfer(tsc, READ_Y);
>> +               break;
>> +       case 2:
>> +               *val = tsc2007_xfer(tsc, READ_Z1);
>> +               break;
>> +       case 3:
>> +               *val = tsc2007_xfer(tsc, READ_Z2);
>> +               break;
>> +       case 4:
>> +               *val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
>> +               break;
>> +       case 5: {
>> +               struct ts_event tc;
>> +
>> +               tc.x = tsc2007_xfer(tsc, READ_X);
>> +               tc.z1 = tsc2007_xfer(tsc, READ_Z1);
>> +               tc.z2 = tsc2007_xfer(tsc, READ_Z2);
>> +               *val = tsc2007_calculate_resistance(tsc, &tc);
>> +               break;
>> +       }
>> +       case 6:
>> +               *val = tsc2007_is_pen_down(tsc);
>> +               break;
>> +       case 7:
>> +               *val = tsc2007_xfer(tsc,
>> +                                   (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
>> +               break;
>> +       case 8:
>> +               *val = tsc2007_xfer(tsc,
>> +                                   (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
>> +               break;
>> +       }
>> +
>> +       /* Prepare for next touch reading - power down ADC, enable PENIRQ */
>> +       tsc2007_xfer(tsc, PWRDOWN);
>> +
>> +       mutex_unlock(&tsc->mlock);
>> +
>> +       ret = IIO_VAL_INT;
>> +
>> +       return ret;
>> +}
>> +
>> +static const struct iio_info tsc2007_iio_info = {
>> +       .read_raw = tsc2007_read_raw,
>> +       .driver_module = THIS_MODULE,
>> +};
>> +
>> +static inline int tsc2007_iio_configure(struct tsc2007 *ts)
>> +{
>> +       int err;
>> +       struct iio_dev *indio_dev;
>> +       struct tsc2007_iio *iio;
>> +
>> +       indio_dev = devm_iio_device_alloc(&ts->client->dev, sizeof(struct tsc2007_iio));
>> +       if (!indio_dev) {
>> +               dev_err(&ts->client->dev, "iio_device_alloc failed\n");
>> +               return -ENOMEM;
>> +       }
>> +
>> +       iio = iio_priv(indio_dev);
>> +       iio->ts = ts;
>> +       ts->private = (void *) indio_dev;
>> +
>> +       indio_dev->name = "tsc2007";
>> +       indio_dev->dev.parent = &ts->client->dev;
>> +       indio_dev->info = &tsc2007_iio_info;
>> +       indio_dev->modes = INDIO_DIRECT_MODE;
>> +       indio_dev->channels = tsc2007_iio_channel;
>> +       indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
>> +
>> +       err = iio_device_register(indio_dev);
>> +       if (err < 0) {
>> +               dev_err(&ts->client->dev, "iio_device_register() failed: %d\n",
>> +                       err);
>> +               return err;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static inline void tsc2007_iio_unconfigure(struct tsc2007 *ts)
>> +{
>> +       struct iio_dev *indio_dev = ts->private;
>> +
>> +       iio_device_unregister(indio_dev);
>> +}
>> +
>> +#else /* CONFIG_IIO */
>> +
>> +static inline int tsc2007_iio_configure(struct tsc2007 *ts)
>> +{
>> +       /* not needed */
>> +}
>> +
>> +static inline void tsc2007_iio_unconfigure(struct tsc2007 *ts)
>> +{
>> +       /* not needed */
>> +}
>> +
>> +#endif /* CONFIG_IIO */
>> +
>> #ifdef CONFIG_OF
>> static int tsc2007_get_pendown_state_gpio(struct device *dev)
>> {
>> @@ -472,7 +630,13 @@ static int tsc2007_probe(struct i2c_client *client,
>>        ts->client = client;
>>        ts->irq = client->irq;
>>        ts->input = input_dev;
>> +
>> +       err = tsc2007_iio_configure(ts);
>> +       if (err < 0)
>> +               return err;
>> +
>>        init_waitqueue_head(&ts->wait);
>> +       mutex_init(&ts->mlock);
>> 
>>        snprintf(ts->phys, sizeof(ts->phys),
>>                 "%s/input0", dev_name(&client->dev));
>> @@ -503,8 +667,10 @@ static int tsc2007_probe(struct i2c_client *client,
>>                                                          ts->fuzzz, 0);
>>        } else {
>>                err = tsc2007_probe_dt(client, ts);
>> -               if (err)
>> +               if (err) {
>> +                       tsc2007_iio_unconfigure(ts);
>>                        return err;
>> +               }
>>        }
>> 
>>        if (pdata) {
>> @@ -516,6 +682,7 @@ static int tsc2007_probe(struct i2c_client *client,
>>                                dev_err(&client->dev,
>>                                        "Failed to register exit_platform_hw action, %d\n",
>>                                        err);
>> +                               tsc2007_iio_unconfigure(ts);
>>                                return err;
>>                        }
>>                }
>> @@ -533,6 +700,7 @@ static int tsc2007_probe(struct i2c_client *client,
>>        if (err) {
>>                dev_err(&client->dev, "Failed to request irq %d: %d\n",
>>                        ts->irq, err);
>> +               tsc2007_iio_unconfigure(ts);
> Maybe need a common unwind and use a goto to get to it.

Yes, makes sense.

>>                return err;
>>        }
>> 
>> @@ -543,6 +711,7 @@ static int tsc2007_probe(struct i2c_client *client,
>>        if (err < 0) {
>>                dev_err(&client->dev,
>>                        "Failed to setup chip: %d\n", err);
>> +               tsc2007_iio_unconfigure(ts);
>>                return err;     /* usually, chip does not respond */
>>        }
>> 
>> @@ -550,12 +719,21 @@ static int tsc2007_probe(struct i2c_client *client,
>>        if (err) {
>>                dev_err(&client->dev,
>>                        "Failed to register input device: %d\n", err);
>> +               tsc2007_iio_unconfigure(ts);
>>                return err;
>>        }
>> 
>>        return 0;
>> }
>> 
>> +static int tsc2007_remove(struct i2c_client *client)
>> +{
>> +       struct tsc2007 *ts = i2c_get_clientdata(client);
>> +       tsc2007_iio_unconfigure(ts);
>> +       input_unregister_device(ts->input);
>> +       return 0;
>> +}
>> +
>> static const struct i2c_device_id tsc2007_idtable[] = {
>>        { "tsc2007", 0 },
>>        { }
>> @@ -578,6 +756,7 @@ static struct i2c_driver tsc2007_driver = {
>>        },
>>        .id_table       = tsc2007_idtable,
>>        .probe          = tsc2007_probe,
>> +       .remove         = tsc2007_remove,
>> };
>> 
>> module_i2c_driver(tsc2007_driver);--
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

BR and thanks,
Nikolaus

^ permalink raw reply

* [PATCH v4] media: et8ek8: add device tree binding documentation
From: Pavel Machek @ 2016-10-23 19:17 UTC (permalink / raw)
  To: ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
	sakari.ailus-X3B1VOXEql0, sre-DgEjT+Ai2ygdnm+yROfE0A,
	pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, mchehab-JPH+aEBZ4P+UEJcrhfAQsw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]


Add device tree binding documentation for toshiba et8ek8 sensor.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

---

diff from v3: explain what clock-frequency means


diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
new file mode 100644
index 0000000..54863cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
@@ -0,0 +1,51 @@
+Toshiba et8ek8 5MP sensor
+
+Toshiba et8ek8 5MP sensor is an image sensor found in Nokia N900 device
+
+More detailed documentation can be found in
+Documentation/devicetree/bindings/media/video-interfaces.txt .
+
+
+Mandatory properties
+--------------------
+
+- compatible: "toshiba,et8ek8"
+- reg: I2C address (0x3e, or an alternative address)
+- vana-supply: Analogue voltage supply (VANA), 2.8 volts
+- clocks: External clock to the sensor
+- clock-frequency: Frequency of the external clock to the sensor. Camera
+  driver will set this frequency on the external clock.
+- reset-gpios: XSHUTDOWN GPIO
+
+
+Endpoint node mandatory properties
+----------------------------------
+
+- remote-endpoint: A phandle to the bus receiver's endpoint node.
+
+Endpoint node optional properties
+----------------------------------
+
+- clock-lanes: <0>
+- data-lanes: <1..n>
+
+Example
+-------
+
+&i2c3 {
+	clock-frequency = <400000>;
+
+	cam1: camera@3e {
+		compatible = "toshiba,et8ek8";
+		reg = <0x3e>;
+		vana-supply = <&vaux4>;
+		clocks = <&isp 0>;
+		clock-frequency = <9600000>;
+		reset-gpio = <&gpio4 6 GPIO_ACTIVE_HIGH>; /* 102 */
+		port {
+			csi_cam1: endpoint {
+				remote-endpoint = <&csi_out1>;
+			};
+		};
+	};
+};

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply related

* Re: [PATCH v4] media: et8ek8: add device tree binding documentation
From: Sakari Ailus @ 2016-10-23 20:06 UTC (permalink / raw)
  To: Pavel Machek
  Cc: ivo.g.dimitrov.75, sre, pali.rohar, linux-media, robh+dt,
	pawel.moll, mark.rutland, ijc+devicetree, galak, mchehab,
	devicetree, linux-kernel
In-Reply-To: <20161023191706.GA25754@amd>

Hi Pavel,

On Sun, Oct 23, 2016 at 09:17:06PM +0200, Pavel Machek wrote:
> 
> Add device tree binding documentation for toshiba et8ek8 sensor.

How about the et8ek8 driver itself? And the N900 DT snippet? What's their
status?

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply

* Re: [PATCH 6/8] pinctrl: aspeed-g4: Capture SuperIO pinmux dependency
From: Linus Walleij @ 2016-10-23 22:09 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: Joel Stanley, Mark Rutland, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1477010011.8917.20.camel-zrmu5oMJ5Fs@public.gmane.org>

On Fri, Oct 21, 2016 at 2:33 AM, Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org> wrote:

>> Patch applied for v4.10.
>> (Tell me if I'm applying patches in wrong order or something, and
>> I hope this doesn't clash with the fixes.)
>
> Both this patch and 8/8 functionally depend on 5/8. I fetched the
> pinctrl tree to poke around but this patch didn't appear in any of the
> updated branches, so I'm not sure whether we have the right ordering.
> Without it we should hit build failures from missing macro definitions.
>
> Have you had a chance to look over patch 5/8? Joel wasn't keen on its
> current form, so I would appreciate your input.

Oops backed this patch out.

Will look at 5/8.

Appreciate if you repost the remaining patches in the series based on
v4.9-rc2 once it's out, and I'll rebase the pinctrl tree onto that.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes
From: Peter Rosin @ 2016-10-23 22:10 UTC (permalink / raw)
  To: Jonathan Cameron, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <74746223-a1aa-e9c6-622c-854526d0722d-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On 2016-10-23 11:33, Jonathan Cameron wrote:
> On 22/10/16 23:43, Peter Rosin wrote:
>> From: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>
>> A large number of attributes can only take a limited range of values.
>> Currently in IIO this is handled by directly registering additional
>> *_available attributes thus providing this information to userspace.
>>
>> It is desirable to provide this information via the core for much the same
>> reason this was done for the actual channel information attributes in the
>> first place.  If it isn't there, then it can only really be accessed from
>> userspace.  Other in kernel IIO consumers have no access to what valid
>> parameters are.
>>
>> Two forms are currently supported:
>> * list of values in one particular IIO_VAL_* format.
>> 	e.g. 1.300000 1.500000 1.730000
>> * range specification with a step size:
>> 	e.g. [1.000000 0.500000 2.500000]
>> 	equivalent to 1.000000 1.5000000 2.000000 2.500000
>>
>> An addition set of masks are used to allow different sharing rules for the
>> *_available attributes generated.
>>
>> This allows for example:
>>
>> in_accel_x_offset
>> in_accel_y_offset
>> in_accel_offset_available.
>>
>> We could have gone with having a specification for each and every
>> info_mask element but that would have meant changing the existing userspace
>> ABI.  This approach does not.
> I'm wondering what I meant by this... where does it cause use ABI issues?
> Do you have any idea?

Nope, sorry. My memory is probably just as blank as yours :-)

> I'd forgotten I'd even done it like this rather than just insisting on 
> available for everything (which is more logical to me as we should know
> the range of everything).
> 
> It's pretty low cost though and gives a way of adding this to drivers
> piecemeal which is probably a good idea!

Yes, that's always a good sign. Flag days are a pain.

>> Signed-off-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> [rather mindlessly forward ported from approx 3 years ago /peda]
> More importantly shepherding it through review!
> 
> Naturally it's perfect code so not comments inline ;)
> 
> Thanks again for picking this up!
> 
> So... I want to see lots of comments on this. If people are happy with
> it (unlikely ;) then please say so.  It's at least a bit controversial

Hmmm, maybe I should looking over the changes line-by-line, see inline
comments...

> and adds a 'lot' of new ABI.

I'd appreciate it if someone else wrote up the common stuff in the
testing/sysfs-bus/iio file. That file is a jungle to a newcomer...

> Jonathan
>> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>> ---
>>  drivers/iio/industrialio-core.c | 232 +++++++++++++++++++++++++++++++++++-----
>>  include/linux/iio/iio.h         |  11 ++
>>  include/linux/iio/types.h       |   5 +
>>  3 files changed, 223 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index fc340ed3dca1..93c69ebeda1e 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -575,51 +575,41 @@ int of_iio_read_mount_matrix(const struct device *dev,
>>  #endif
>>  EXPORT_SYMBOL(of_iio_read_mount_matrix);
>>  
>> -/**
>> - * iio_format_value() - Formats a IIO value into its string representation
>> - * @buf:	The buffer to which the formatted value gets written
>> - * @type:	One of the IIO_VAL_... constants. This decides how the val
>> - *		and val2 parameters are formatted.
>> - * @size:	Number of IIO value entries contained in vals
>> - * @vals:	Pointer to the values, exact meaning depends on the
>> - *		type parameter.
>> - *
>> - * Return: 0 by default, a negative number on failure or the
>> - *	   total number of characters written for a type that belongs
>> - *	   to the IIO_VAL_... constant.
>> - */
>> -ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
>> +static ssize_t __iio_format_value(char *buf, unsigned int type,
>> +				  int size, const int *vals)
>>  {
>>  	unsigned long long tmp;
>> +	int tmp0, tmp1;
>>  	bool scale_db = false;
>>  
>>  	switch (type) {
>>  	case IIO_VAL_INT:
>> -		return sprintf(buf, "%d\n", vals[0]);
>> +		return sprintf(buf, "%d", vals[0]);

This function was previously used to format one or a perhaps a few
values presumable at the start of a page. It doesn't bother to check
for buffer overflow. That is probably very bad now that it is used
to print arbitrary length lists of values...

I'll add a suggested fix in v3.

>>  	case IIO_VAL_INT_PLUS_MICRO_DB:
>>  		scale_db = true;
>>  	case IIO_VAL_INT_PLUS_MICRO:
>>  		if (vals[1] < 0)
>> -			return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
>> +			return sprintf(buf, "-%d.%06u%s", abs(vals[0]),
>>  				       -vals[1], scale_db ? " dB" : "");
>>  		else
>> -			return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
>> +			return sprintf(buf, "%d.%06u%s", vals[0], vals[1],
>>  				scale_db ? " dB" : "");
>>  	case IIO_VAL_INT_PLUS_NANO:
>>  		if (vals[1] < 0)
>> -			return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
>> +			return sprintf(buf, "-%d.%09u", abs(vals[0]),
>>  				       -vals[1]);
>>  		else
>> -			return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
>> +			return sprintf(buf, "%d.%09u", vals[0], vals[1]);
>>  	case IIO_VAL_FRACTIONAL:
>>  		tmp = div_s64((s64)vals[0] * 1000000000LL, vals[1]);
>> -		vals[0] = (int)div_s64_rem(tmp, 1000000000, &vals[1]);
>> -		return sprintf(buf, "%d.%09u\n", vals[0], abs(vals[1]));
>> +		tmp1 = vals[1];
>> +		tmp0 = (int)div_s64_rem(tmp, 1000000000, &tmp1);
>> +		return sprintf(buf, "%d.%09u", tmp0, abs(tmp1));
>>  	case IIO_VAL_FRACTIONAL_LOG2:
>>  		tmp = (s64)vals[0] * 1000000000LL >> vals[1];
>> -		vals[1] = do_div(tmp, 1000000000LL);
>> -		vals[0] = tmp;
>> -		return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
>> +		tmp1 = do_div(tmp, 1000000000LL);
>> +		tmp0 = tmp;
>> +		return sprintf(buf, "%d.%09u", tmp0, tmp1);
>>  	case IIO_VAL_INT_MULTIPLE:
>>  	{
>>  		int i;
>> @@ -628,13 +618,34 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
>>  		for (i = 0; i < size; ++i)
>>  			len += snprintf(&buf[len], PAGE_SIZE - len, "%d ",
>>  								vals[i]);

This seems like a dangerous thing to do, arg 2 of snprintf is size_t which
is unsigned, so if one call would have overflowed the buffer, the next
will see a negative available buffer, turned very large since unsigned.
*boom*

I'll add a suggested fix in v3.

>> -		len += snprintf(&buf[len], PAGE_SIZE - len, "\n");
>>  		return len;
>>  	}
>>  	default:
>>  		return 0;
>>  	}
>>  }
>> +
>> +/**
>> + * iio_format_value() - Formats a IIO value into its string representation
>> + * @buf:	The buffer to which the formatted value gets written
>> + * @type:	One of the IIO_VAL_... constants. This decides how the val
>> + *		and val2 parameters are formatted.
>> + * @size:	Number of IIO value entries contained in vals
>> + * @vals:	Pointer to the values, exact meaning depends on the
>> + *		type parameter.
>> + *
>> + * Return: 0 by default, a negative number on failure or the
>> + *	   total number of characters written for a type that belongs
>> + *	   to the IIO_VAL_... constant.
>> + */
>> +ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
>> +{
>> +	ssize_t len;
>> +
>> +	len = __iio_format_value(buf, type, size, vals);
>> +
>> +	return len + sprintf(buf + len, "\n");
>> +}
>>  EXPORT_SYMBOL_GPL(iio_format_value);
>>  
>>  static ssize_t iio_read_channel_info(struct device *dev,
>> @@ -662,6 +673,113 @@ static ssize_t iio_read_channel_info(struct device *dev,
>>  	return iio_format_value(buf, ret, val_len, vals);
>>  }
>>  
>> +static ssize_t iio_format_avail_list(char *buf, const int *vals,
>> +				     int type, int length)
>> +{
>> +	int i;
>> +	ssize_t len = 0;
>> +
>> +	switch (type) {
>> +	case IIO_VAL_INT:
>> +		for (i = 0; i < length; i++) {
>> +			len += __iio_format_value(buf + len, type, 1, &vals[i]);
>> +			if (i < length - 1)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"\n");
>> +		}
>> +		break;
>> +	default:
>> +		for (i = 0; i < length / 2; i++) {
>> +			len += __iio_format_value(buf + len,
>> +						  type,
>> +						  2,
>> +						  &vals[i * 2]);
>> +			if (i < length / 2 - 1)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"\n");
>> +		}
>> +	};
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t iio_format_avail_range(char *buf, const int *vals, int type)
>> +{
>> +	int i;
>> +	ssize_t len;
>> +
>> +	len = snprintf(buf, PAGE_SIZE, "[");
>> +	switch (type) {
>> +	case IIO_VAL_INT:
>> +		for (i = 0; i < 3; i++) {
>> +			len += __iio_format_value(buf + len, type, 1, &vals[i]);
>> +			if (i < 2)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"]\n");
>> +		}
>> +		break;
>> +	default:
>> +		for (i = 0; i < 3; i++) {
>> +			len += __iio_format_value(buf + len,
>> +						  type,
>> +						  2,
>> +						  &vals[i * 2]);
>> +			if (i < 2)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"]\n");
>> +		}
>> +	};
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t iio_read_channel_info_avail(struct device *dev,
>> +					   struct device_attribute *attr,
>> +					   char *buf)
>> +{
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +	const int *vals;
>> +	int ret;
>> +	int length;
>> +	int type;
>> +
>> +	ret = indio_dev->info->read_avail(indio_dev, this_attr->c,
>> +					  &vals, &type, &length,
>> +					  this_attr->address);
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +	switch (ret) {
>> +	case IIO_AVAIL_LIST:
>> +		return iio_format_avail_list(buf, vals, type, length);
>> +	case IIO_AVAIL_RANGE:
>> +		return iio_format_avail_range(buf, vals, type);
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>>  /**
>>   * iio_str_to_fixpoint() - Parse a fixed-point number from a string
>>   * @str: The string to parse
>> @@ -978,6 +1096,40 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
>>  	return attrcount;
>>  }
>>  
>> +static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
>> +					       struct iio_chan_spec const *chan,
>> +					       enum iio_shared_by shared_by,
>> +					       const long *infomask)
>> +{
>> +	int i, ret, attrcount = 0;
>> +	char *avail_postfix;
>> +
>> +	for_each_set_bit(i, infomask, sizeof(infomask) * 8) {
>> +		avail_postfix = kasprintf(GFP_KERNEL,
>> +					  "%s_available",
>> +					  iio_chan_info_postfix[i]);
>> +		if (!avail_postfix)
>> +			return -ENOMEM;
>> +
>> +		ret = __iio_add_chan_devattr(avail_postfix,
>> +					     chan,
>> +					     &iio_read_channel_info_avail,
>> +					     NULL,
>> +					     i,
>> +					     shared_by,
>> +					     &indio_dev->dev,
>> +					     &indio_dev->channel_attr_list);
>> +		kfree(avail_postfix);
>> +		if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
>> +			continue;
>> +		else if (ret < 0)
>> +			return ret;
>> +		attrcount++;
>> +	}
>> +
>> +	return attrcount;
>> +}
>> +
>>  static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  					struct iio_chan_spec const *chan)
>>  {
>> @@ -993,6 +1145,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SEPARATE,
>> +						  &chan->
>> +						  info_mask_separate_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>>  					    IIO_SHARED_BY_TYPE,
>>  					    &chan->info_mask_shared_by_type);
>> @@ -1000,6 +1160,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SHARED_BY_TYPE,
>> +						  &chan->
>> +						  info_mask_shared_by_type_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>>  					    IIO_SHARED_BY_DIR,
>>  					    &chan->info_mask_shared_by_dir);
>> @@ -1007,6 +1175,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SHARED_BY_DIR,
>> +						  &chan->info_mask_shared_by_dir_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>>  					    IIO_SHARED_BY_ALL,
>>  					    &chan->info_mask_shared_by_all);
>> @@ -1014,6 +1189,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SHARED_BY_ALL,
>> +						  &chan->info_mask_shared_by_all_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	if (chan->ext_info) {
>>  		unsigned int i = 0;
>>  		for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index b4a0679e4a49..c0f897084620 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -269,9 +269,13 @@ struct iio_chan_spec {
>>  		enum iio_endian endianness;
>>  	} scan_type;
>>  	long			info_mask_separate;
>> +	long			info_mask_separate_available;
>>  	long			info_mask_shared_by_type;
>> +	long			info_mask_shared_by_type_available;
>>  	long			info_mask_shared_by_dir;
>> +	long			info_mask_shared_by_dir_available;
>>  	long			info_mask_shared_by_all;
>> +	long			info_mask_shared_by_all_available;
>>  	const struct iio_event_spec *event_spec;
>>  	unsigned int		num_event_specs;
>>  	const struct iio_chan_spec_ext_info *ext_info;
>> @@ -397,6 +401,13 @@ struct iio_info {
>>  			int *val_len,
>>  			long mask);
>>  
>> +	int (*read_avail)(struct iio_dev *indio_dev,
>> +			  struct iio_chan_spec const *chan,
>> +			  const int **vals,
>> +			  int *type,
>> +			  int *length,
>> +			  long mask_el);
>> +
>>  	int (*write_raw)(struct iio_dev *indio_dev,
>>  			 struct iio_chan_spec const *chan,
>>  			 int val,
>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> index 32b579525004..2aa7b6384d64 100644
>> --- a/include/linux/iio/types.h
>> +++ b/include/linux/iio/types.h
>> @@ -29,4 +29,9 @@ enum iio_event_info {
>>  #define IIO_VAL_FRACTIONAL 10
>>  #define IIO_VAL_FRACTIONAL_LOG2 11
>>  
>> +enum iio_available_type {
>> +	IIO_AVAIL_LIST,
>> +	IIO_AVAIL_RANGE,
>> +};
>> +
>>  #endif /* _IIO_TYPES_H_ */
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 5/8] pinctrl: aspeed: Enable capture of off-SCU pinmux state
From: Linus Walleij @ 2016-10-23 22:20 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Andrew Jeffery, Mark Rutland, Rob Herring,
	linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	OpenBMC Maillist
In-Reply-To: <CACPK8Xc7y3GtcJCVYYs-JKTqBZvqVeZaz5MUk=UX151SX1xEFw@mail.gmail.com>

On Thu, Sep 29, 2016 at 8:45 AM, Joel Stanley <joel@jms.id.au> wrote:
> On Wed, Sep 28, 2016 at 12:20 AM, Andrew Jeffery <andrew@aj.id.au> wrote:

>> On the AST2400 and AST2500 a number of pins depend on state in one of
>> the SIO, LPC or GFX IP blocks, so add support to at least capture what
>> that state is. The pinctrl engine for the Aspeed SoCs doesn't try to
>> inspect or modify the state of the off-SCU IP blocks. Instead, it logs
>> the state requirement with the expectation that the platform
>> designer/maintainer arranges for the appropriate configuration to be
>> applied through the associated drivers.
(...)
>
> This is unfortunate.
>
> This patch kicks the can down the road, but doesn't solve the problem
> for a user who wants to configure some functionality that depends on
> the non-SCU bits. Because of this I'm not sure if we want to put it in
> the tree.
>
> However, I'm not sure what a proper solution would look like. Perhaps
> Linus can point out another SoC that has a similar problem?

If I could only understand it.

Don't you actually want to go and read a few registers on another
IO range?

What we generally do when pin control is spread out in a system is try
to consolidate it, meaning expose the necessary registers on the remote
end using syscon (drivers/mfd/syscon) so that we can easily get a handle
on these registers withe regmap MMIO.

Since regmap handles atomic access to the registers, that way we
protect from disasters and keep the state in the hardware.

I don't know if this is helpful. For a normal peripheral you may not want
to put a regmap over all its registers but you prefer to ioremap it, and then
we get the spaghetti of accessor functions to peek and poke into another
peripherals I/O space, and that is undesireable.

Maybe this is completely not understanding what you want to do, so
sorry, please elaborate. I am afraid the two of you are the only people on
the planet who actually understand what is going on here.

Also the hardware engineer who wrote the Aspeed pin controller, I would
like to read this persons design specification, I am pretty sure it is very
interesting.

>> -  * @reg: The register offset from base in bytes
>> +  * @reg: Split into three fields:
>> +  *       31:24: IP selector
>> +  *       23:12: Reserved
>> +  *       11:0: Register offset

That seems like unnecessary shoehorning. Is it reflecting the register layout
of the hardware or are you trying to save bits? For the latter, let it
go and instead
have one member for offset and one member for selector.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v3 0/8] IIO wrapper drivers, dpot-dac and envelope-detector
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi!

These two drivers share the fact that they wrap another iio channel,
and I use the first in combination with the second, which is why I'm
submitting them as a pair.

The first driver is a simple wrapper converting an iio dpot into an
iio dac. It only changes the unit and scale. It also does not add any
fancy iio buffer support that I don't need. I suppose that can be
added. By someone else :-)

The second driver (the envelope detector) is more involved. It also
explains why I need the dpot-dac driver. I wanted the envelope
detector to be generic and work with any dac, but I had a dpot...

But before those two new drivers, there is some infrastructure added
to provide available values for a channel.

Ok, so the devicetree compatible string for the envelope-detector gained
the "axentia,tse850-" prefix in v2, but at the same time the driver also
gained generality that in my mind puts it in about the same position as the
dpot-dac driver. The driver really has very little to do with the specifics
of the TSE-850. But at the same time there are more ways to build an
envelope detector compared to the obviousness of building a dac from a dpot...

One thing I still don't like is that the irq needs to be changed for cases
where it is only ever interesting with the 'invert' variant, and it may not
work to set up the irq the wrong way first. For the TSE-850, this does not
matter, since we have a mux on the envelope detector input and can make use
of both 'invert' and 'normal' for different signals (we could have gotten
by with only 'invert' since the only signals we are measuring that are
'normal' are also DC signals and can thus be detected both from below and
from above), but it is nice to have it both ways. The only way out of this
is a devicetree thing. I suppose it will have to be added by whomever needs
it whenever that is...

I also wonder if the "new" *_available ABI should perhaps be documented
for all variants directly in sysfs-bus-iio instead of doing it ina driver
specific maner that I did? But that can be fixed later by someone more
capable than me :-)

v2 -> v3
- add some missing @foo comments in iio.h in the initial forward ported patch
- killed some buffer overflow problems in the initial forward ported patch
- add inkern.c helpers for the new available attributes to avoid viral
  boilerplating in the future (also add support for finding max of
  IIO_AVAIL_LISTs of IIO_VAL_INTs)
- add ABI docs for new ABI of mcp4531 (out_resistance_raw_available)
  and an example in the commit message

dpot-dac:
- two more counts of s/assumed the that the/assumed that the/   *blush*
- add ABI docs for new ABI (out_voltageY_raw_available)

envelope detector:
- move device attributes 'compare_interval_ms' and 'invert' to extended
  channel attributes (out_altvoltage0_compare_interval and
  out_altvoltage0_invert) as they really are about the channel and not
  the device
- kill "dpot-dac,max-ohms = <100000>;" in the devicetree example


v1 -> v2
- provide out_resistanceX_raw_available channel attribute in mcp4531 dpot

dpot-dac:
- change Vref to vref
- the module will be called dpot-dac (in Kconfig help)
- removed a 'the'
- removed (s64) cast
- make the channel indexed, makes libiio find the channel (tested with 0.5)
- add a comment on how integer scale is converted to fractional scale
  and clarify the code a bit
- dig out max-ohms by looking at scale and maximum available raw value
  from the dpot channel and drop the 'dpot-dac,max-ohms' devicetree property
- provide out_voltageX_raw_available channel attribute

envelope-detector:
- change compatible from envelope-detector to axentia,tse850-envelope-detector
- remove envelope-detector,invert and envelope-detector,comp-interval-ms from
  devicetree and add them as iio device attributes instead
- make the channel indexed, makes libiio find the channel (tested with 0.5)
- reorder struct envelope to better indicate what is covered by read_lock
- add comment on interaction between envelope_detector_comp_latch (renamed
  from envelope_detector_latch) and envelope_detector_comp_isr (renamed
  from envelope_detector_isr)
- fixup a problem in envelope_detector_comp_latch where interrupts pending
  from when the interrupt has been disabled interferes with expected
  operation
- slight rewrite of the initial high/low assignments
- use a better name when requesting the interrupt
- dig out dac_max by looking at scale and maximum available raw value
  from the dac channel and drop the 'envelope-detector,dac-max' devicetree
  property

Cheers,
Peter

Jonathan Cameron (1):
  iio:core: add a callback to allow drivers to provide _available
    attributes

Peter Rosin (7):
  iio: inkern: add helpers to query available values from channels
  iio: mcp4531: provide range of available raw values
  dt-bindings: add axentia to vendor-prefixes
  dt-bindings: iio: document dpot-dac bindings
  iio: dpot-dac: DAC driver based on a digital potentiometer
  dt-bindings: iio: document envelope-detector bindings
  iio: envelope-detector: ADC driver based on a DAC and a comparator

 .../testing/sysfs-bus-iio-adc-envelope-detector    |  36 ++
 .../ABI/testing/sysfs-bus-iio-dac-dpot-dac         |   8 +
 .../testing/sysfs-bus-iio-potentiometer-mcp4531    |   8 +
 .../bindings/iio/adc/envelope-detector.txt         |  54 +++
 .../devicetree/bindings/iio/dac/dpot-dac.txt       |  41 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 MAINTAINERS                                        |  17 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/envelope-detector.c                | 422 +++++++++++++++++++++
 drivers/iio/dac/Kconfig                            |  10 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/dpot-dac.c                         | 267 +++++++++++++
 drivers/iio/industrialio-core.c                    | 259 +++++++++++--
 drivers/iio/inkern.c                               |  97 +++++
 drivers/iio/potentiometer/mcp4531.c                | 104 +++--
 include/linux/iio/consumer.h                       |  29 ++
 include/linux/iio/iio.h                            |  46 +++
 include/linux/iio/types.h                          |   5 +
 19 files changed, 1341 insertions(+), 75 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
 create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
 create mode 100644 Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
 create mode 100644 drivers/iio/adc/envelope-detector.c
 create mode 100644 drivers/iio/dac/dpot-dac.c

-- 
2.1.4

^ permalink raw reply

* [PATCH v3 1/8] iio:core: add a callback to allow drivers to provide _available attributes
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Jonathan Cameron, Peter Rosin, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477262381-7800-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

From: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

A large number of attributes can only take a limited range of values.
Currently in IIO this is handled by directly registering additional
*_available attributes thus providing this information to userspace.

It is desirable to provide this information via the core for much the same
reason this was done for the actual channel information attributes in the
first place.  If it isn't there, then it can only really be accessed from
userspace.  Other in kernel IIO consumers have no access to what valid
parameters are.

Two forms are currently supported:
* list of values in one particular IIO_VAL_* format.
	e.g. 1.300000 1.500000 1.730000
* range specification with a step size:
	e.g. [1.000000 0.500000 2.500000]
	equivalent to 1.000000 1.5000000 2.000000 2.500000

An addition set of masks are used to allow different sharing rules for the
*_available attributes generated.

This allows for example:

in_accel_x_offset
in_accel_y_offset
in_accel_offset_available.

We could have gone with having a specification for each and every
info_mask element but that would have meant changing the existing userspace
ABI.  This approach does not.

Signed-off-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
[forward ported, added some docs and fixed buffer overflows /peda]
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 drivers/iio/industrialio-core.c | 259 +++++++++++++++++++++++++++++++++++-----
 include/linux/iio/iio.h         |  29 +++++
 include/linux/iio/types.h       |   5 +
 3 files changed, 260 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index fc340ed3dca1..b35c61a31fa6 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -575,66 +575,82 @@ int of_iio_read_mount_matrix(const struct device *dev,
 #endif
 EXPORT_SYMBOL(of_iio_read_mount_matrix);
 
-/**
- * iio_format_value() - Formats a IIO value into its string representation
- * @buf:	The buffer to which the formatted value gets written
- * @type:	One of the IIO_VAL_... constants. This decides how the val
- *		and val2 parameters are formatted.
- * @size:	Number of IIO value entries contained in vals
- * @vals:	Pointer to the values, exact meaning depends on the
- *		type parameter.
- *
- * Return: 0 by default, a negative number on failure or the
- *	   total number of characters written for a type that belongs
- *	   to the IIO_VAL_... constant.
- */
-ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
+static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type,
+				  int size, const int *vals)
 {
 	unsigned long long tmp;
+	int tmp0, tmp1;
 	bool scale_db = false;
 
 	switch (type) {
 	case IIO_VAL_INT:
-		return sprintf(buf, "%d\n", vals[0]);
+		return snprintf(buf, len, "%d", vals[0]);
 	case IIO_VAL_INT_PLUS_MICRO_DB:
 		scale_db = true;
 	case IIO_VAL_INT_PLUS_MICRO:
 		if (vals[1] < 0)
-			return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
-				       -vals[1], scale_db ? " dB" : "");
+			return snprintf(buf, len, "-%d.%06u%s", abs(vals[0]),
+					-vals[1], scale_db ? " dB" : "");
 		else
-			return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
-				scale_db ? " dB" : "");
+			return snprintf(buf, len, "%d.%06u%s", vals[0], vals[1],
+					scale_db ? " dB" : "");
 	case IIO_VAL_INT_PLUS_NANO:
 		if (vals[1] < 0)
-			return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
-				       -vals[1]);
+			return snprintf(buf, len, "-%d.%09u", abs(vals[0]),
+					-vals[1]);
 		else
-			return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
+			return snprintf(buf, len, "%d.%09u", vals[0], vals[1]);
 	case IIO_VAL_FRACTIONAL:
 		tmp = div_s64((s64)vals[0] * 1000000000LL, vals[1]);
-		vals[0] = (int)div_s64_rem(tmp, 1000000000, &vals[1]);
-		return sprintf(buf, "%d.%09u\n", vals[0], abs(vals[1]));
+		tmp1 = vals[1];
+		tmp0 = (int)div_s64_rem(tmp, 1000000000, &tmp1);
+		return snprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
 	case IIO_VAL_FRACTIONAL_LOG2:
 		tmp = (s64)vals[0] * 1000000000LL >> vals[1];
-		vals[1] = do_div(tmp, 1000000000LL);
-		vals[0] = tmp;
-		return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
+		tmp1 = do_div(tmp, 1000000000LL);
+		tmp0 = tmp;
+		return snprintf(buf, len, "%d.%09u", tmp0, tmp1);
 	case IIO_VAL_INT_MULTIPLE:
 	{
 		int i;
-		int len = 0;
+		int l = 0;
 
-		for (i = 0; i < size; ++i)
-			len += snprintf(&buf[len], PAGE_SIZE - len, "%d ",
-								vals[i]);
-		len += snprintf(&buf[len], PAGE_SIZE - len, "\n");
-		return len;
+		for (i = 0; i < size; ++i) {
+			l += snprintf(&buf[l], len - l, "%d ", vals[i]);
+			if (l >= len)
+				break;
+		}
+		return l;
 	}
 	default:
 		return 0;
 	}
 }
+
+/**
+ * iio_format_value() - Formats a IIO value into its string representation
+ * @buf:	The buffer to which the formatted value gets written
+ *		which is assumed to be big enough (i.e. PAGE_SIZE).
+ * @type:	One of the IIO_VAL_... constants. This decides how the val
+ *		and val2 parameters are formatted.
+ * @size:	Number of IIO value entries contained in vals
+ * @vals:	Pointer to the values, exact meaning depends on the
+ *		type parameter.
+ *
+ * Return: 0 by default, a negative number on failure or the
+ *	   total number of characters written for a type that belongs
+ *	   to the IIO_VAL_... constant.
+ */
+ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
+{
+	ssize_t len;
+
+	len = __iio_format_value(buf, PAGE_SIZE, type, size, vals);
+	if (len >= PAGE_SIZE - 1)
+		return -EFBIG;
+
+	return len + sprintf(buf + len, "\n");
+}
 EXPORT_SYMBOL_GPL(iio_format_value);
 
 static ssize_t iio_read_channel_info(struct device *dev,
@@ -662,6 +678,119 @@ static ssize_t iio_read_channel_info(struct device *dev,
 	return iio_format_value(buf, ret, val_len, vals);
 }
 
+static ssize_t iio_format_avail_list(char *buf, const int *vals,
+				     int type, int length)
+{
+	int i;
+	ssize_t len = 0;
+
+	switch (type) {
+	case IIO_VAL_INT:
+		for (i = 0; i < length; i++) {
+			len += __iio_format_value(buf + len, PAGE_SIZE - len,
+						  type, 1, &vals[i]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < length - 1)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+		break;
+	default:
+		for (i = 0; i < length / 2; i++) {
+			len += __iio_format_value(buf + len, PAGE_SIZE - len,
+						  type, 2, &vals[i * 2]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < length / 2 - 1)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+	};
+
+	return len;
+}
+
+static ssize_t iio_format_avail_range(char *buf, const int *vals, int type)
+{
+	int i;
+	ssize_t len;
+
+	len = snprintf(buf, PAGE_SIZE, "[");
+	switch (type) {
+	case IIO_VAL_INT:
+		for (i = 0; i < 3; i++) {
+			len += __iio_format_value(buf + len, PAGE_SIZE - len,
+						  type, 1, &vals[i]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < 2)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"]\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+		break;
+	default:
+		for (i = 0; i < 3; i++) {
+			len += __iio_format_value(buf + len, PAGE_SIZE - len,
+						  type, 2, &vals[i * 2]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < 2)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"]\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+	};
+
+	return len;
+}
+
+static ssize_t iio_read_channel_info_avail(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	const int *vals;
+	int ret;
+	int length;
+	int type;
+
+	ret = indio_dev->info->read_avail(indio_dev, this_attr->c,
+					  &vals, &type, &length,
+					  this_attr->address);
+
+	if (ret < 0)
+		return ret;
+	switch (ret) {
+	case IIO_AVAIL_LIST:
+		return iio_format_avail_list(buf, vals, type, length);
+	case IIO_AVAIL_RANGE:
+		return iio_format_avail_range(buf, vals, type);
+	default:
+		return -EINVAL;
+	}
+}
+
 /**
  * iio_str_to_fixpoint() - Parse a fixed-point number from a string
  * @str: The string to parse
@@ -978,6 +1107,40 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
 	return attrcount;
 }
 
+static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
+					       struct iio_chan_spec const *chan,
+					       enum iio_shared_by shared_by,
+					       const long *infomask)
+{
+	int i, ret, attrcount = 0;
+	char *avail_postfix;
+
+	for_each_set_bit(i, infomask, sizeof(infomask) * 8) {
+		avail_postfix = kasprintf(GFP_KERNEL,
+					  "%s_available",
+					  iio_chan_info_postfix[i]);
+		if (!avail_postfix)
+			return -ENOMEM;
+
+		ret = __iio_add_chan_devattr(avail_postfix,
+					     chan,
+					     &iio_read_channel_info_avail,
+					     NULL,
+					     i,
+					     shared_by,
+					     &indio_dev->dev,
+					     &indio_dev->channel_attr_list);
+		kfree(avail_postfix);
+		if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
+			continue;
+		else if (ret < 0)
+			return ret;
+		attrcount++;
+	}
+
+	return attrcount;
+}
+
 static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 					struct iio_chan_spec const *chan)
 {
@@ -993,6 +1156,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SEPARATE,
+						  &chan->
+						  info_mask_separate_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	ret = iio_device_add_info_mask_type(indio_dev, chan,
 					    IIO_SHARED_BY_TYPE,
 					    &chan->info_mask_shared_by_type);
@@ -1000,6 +1171,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SHARED_BY_TYPE,
+						  &chan->
+						  info_mask_shared_by_type_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	ret = iio_device_add_info_mask_type(indio_dev, chan,
 					    IIO_SHARED_BY_DIR,
 					    &chan->info_mask_shared_by_dir);
@@ -1007,6 +1186,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SHARED_BY_DIR,
+						  &chan->info_mask_shared_by_dir_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	ret = iio_device_add_info_mask_type(indio_dev, chan,
 					    IIO_SHARED_BY_ALL,
 					    &chan->info_mask_shared_by_all);
@@ -1014,6 +1200,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SHARED_BY_ALL,
+						  &chan->info_mask_shared_by_all_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	if (chan->ext_info) {
 		unsigned int i = 0;
 		for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index b4a0679e4a49..45b781084a4b 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -225,12 +225,22 @@ struct iio_event_spec {
  *			endianness:	little or big endian
  * @info_mask_separate: What information is to be exported that is specific to
  *			this channel.
+ * @info_mask_separate_available: What availability information is to be
+ *			exported that is specific to this channel.
  * @info_mask_shared_by_type: What information is to be exported that is shared
  *			by all channels of the same type.
+ * @info_mask_shared_by_type_available: What availability information is to be
+ *			exported that is shared by all channels of the same
+ *			type.
  * @info_mask_shared_by_dir: What information is to be exported that is shared
  *			by all channels of the same direction.
+ * @info_mask_shared_by_dir_available: What availability information is to be
+ *			exported that is shared by all channels of the same
+ *			direction.
  * @info_mask_shared_by_all: What information is to be exported that is shared
  *			by all channels.
+ * @info_mask_shared_by_all_available: What availability information is to be
+ *			exported that is shared by all channels.
  * @event_spec:		Array of events which should be registered for this
  *			channel.
  * @num_event_specs:	Size of the event_spec array.
@@ -269,9 +279,13 @@ struct iio_chan_spec {
 		enum iio_endian endianness;
 	} scan_type;
 	long			info_mask_separate;
+	long			info_mask_separate_available;
 	long			info_mask_shared_by_type;
+	long			info_mask_shared_by_type_available;
 	long			info_mask_shared_by_dir;
+	long			info_mask_shared_by_dir_available;
 	long			info_mask_shared_by_all;
+	long			info_mask_shared_by_all_available;
 	const struct iio_event_spec *event_spec;
 	unsigned int		num_event_specs;
 	const struct iio_chan_spec_ext_info *ext_info;
@@ -349,6 +363,14 @@ struct iio_dev;
  *			max_len specifies maximum number of elements
  *			vals pointer can contain. val_len is used to return
  *			length of valid elements in vals.
+ * @read_avail:		function to return the available values from the device.
+ *			mask specifies which value. Note 0 means the available
+ *			values for the channel in question.  Return value
+ *			specifies if a IIO_AVAIL_LIST or a IIO_AVAIL_RANGE is
+ *			returned in vals. The type of the vals are returned in
+ *			type and the number of vals is returned in length. For
+ *			ranges, there are always three vals returned; min, step
+ *			and max. For lists, all possible values are enumerated.
  * @write_raw:		function to write a value to the device.
  *			Parameters are the same as for read_raw.
  * @write_raw_get_fmt:	callback function to query the expected
@@ -397,6 +419,13 @@ struct iio_info {
 			int *val_len,
 			long mask);
 
+	int (*read_avail)(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  const int **vals,
+			  int *type,
+			  int *length,
+			  long mask);
+
 	int (*write_raw)(struct iio_dev *indio_dev,
 			 struct iio_chan_spec const *chan,
 			 int val,
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 32b579525004..2aa7b6384d64 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -29,4 +29,9 @@ enum iio_event_info {
 #define IIO_VAL_FRACTIONAL 10
 #define IIO_VAL_FRACTIONAL_LOG2 11
 
+enum iio_available_type {
+	IIO_AVAIL_LIST,
+	IIO_AVAIL_RANGE,
+};
+
 #endif /* _IIO_TYPES_H_ */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 2/8] iio: inkern: add helpers to query available values from channels
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477262381-7800-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Specifically a helper for reading the available maximum raw value of a
channel and a helper for forwarding read_avail requests for raw values
from one iio driver to an iio channel that is consumed.

These rather specific helpers are in turn built with generic helpers
making it easy to build more helpers for available values as needed.

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 drivers/iio/inkern.c         | 97 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h | 29 +++++++++++++
 include/linux/iio/iio.h      | 17 ++++++++
 3 files changed, 143 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c4757e6367e7..74808f8a187a 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -703,6 +703,103 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val, int *val2)
 }
 EXPORT_SYMBOL_GPL(iio_read_channel_scale);
 
+static int iio_channel_read_avail(struct iio_channel *chan,
+				  const int **vals, int *type, int *length,
+				  enum iio_chan_info_enum info)
+{
+	if (!iio_channel_has_available(chan->channel, info))
+		return -EINVAL;
+
+	return chan->indio_dev->info->read_avail(chan->indio_dev, chan->channel,
+						 vals, type, length, info);
+}
+
+int iio_read_avail_channel_raw(struct iio_channel *chan,
+			       const int **vals, int *type, int *length)
+{
+	int ret;
+
+	mutex_lock(&chan->indio_dev->info_exist_lock);
+	if (!chan->indio_dev->info) {
+		ret = -ENODEV;
+		goto err_unlock;
+	}
+
+	ret = iio_channel_read_avail(chan,
+				     vals, type, length, IIO_CHAN_INFO_RAW);
+err_unlock:
+	mutex_unlock(&chan->indio_dev->info_exist_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_avail_channel_raw);
+
+static int iio_channel_read_max(struct iio_channel *chan,
+				int *val, int *val2, int *type,
+				enum iio_chan_info_enum info)
+{
+	int unused;
+	const int *vals;
+	int length;
+	int ret;
+
+	if (!val2)
+		val2 = &unused;
+
+	ret = iio_channel_read_avail(chan, &vals, type, &length, info);
+	switch (ret) {
+	case IIO_AVAIL_RANGE:
+		switch (*type) {
+		case IIO_VAL_INT:
+			*val = vals[2];
+			break;
+		default:
+			*val = vals[4];
+			*val2 = vals[5];
+		}
+		return 0;
+
+	case IIO_AVAIL_LIST:
+		if (length <= 0)
+			return -EINVAL;
+		switch (*type) {
+		case IIO_VAL_INT:
+			*val = vals[--length];
+			while (length) {
+				if (vals[--length] > *val)
+					*val = vals[length];
+			}
+			break;
+		default:
+			/* FIXME: learn about max for other iio values */
+			return -EINVAL;
+		}
+		return 0;
+
+	default:
+		return ret;
+	}
+}
+
+int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
+{
+	int ret;
+	int type;
+
+	mutex_lock(&chan->indio_dev->info_exist_lock);
+	if (!chan->indio_dev->info) {
+		ret = -ENODEV;
+		goto err_unlock;
+	}
+
+	ret = iio_channel_read_max(chan, val, NULL, &type, IIO_CHAN_INFO_RAW);
+err_unlock:
+	mutex_unlock(&chan->indio_dev->info_exist_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_max_channel_raw);
+
 int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
 {
 	int ret = 0;
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 9edccfba1ffb..baab5e45734f 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -226,6 +226,35 @@ int iio_read_channel_processed(struct iio_channel *chan, int *val);
 int iio_write_channel_raw(struct iio_channel *chan, int val);
 
 /**
+ * iio_read_max_channel_raw() - read maximum available raw value from a given
+ *				channel
+ * @chan:		The channel being queried.
+ * @val:		Value read back.
+ *
+ * Note raw reads from iio channels are in adc counts and hence
+ * scale will need to be applied if standard units are required.
+ */
+int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
+
+/**
+ * iio_read_avail_channel_raw() - read available raw values from a given channel
+ * @chan:		The channel being queried.
+ * @vals:		Available values read back.
+ * @type:		Type of available values in vals.
+ * @length:		Number of entries in vals.
+ *
+ * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
+ *
+ * For ranges, three vals are always returned; min, step and max.
+ * For lists, all the possible values are enumerated.
+ *
+ * Note raw available values from iio channels are in adc counts and
+ * hence scale will need to be applied if standard units are required.
+ */
+int iio_read_avail_channel_raw(struct iio_channel *chan,
+			       const int **vals, int *type, int *length);
+
+/**
  * iio_get_channel_type() - get the type of a channel
  * @channel:		The channel being queried.
  * @type:		The type of the channel.
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 45b781084a4b..e565701d13ce 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -315,6 +315,23 @@ static inline bool iio_channel_has_info(const struct iio_chan_spec *chan,
 		(chan->info_mask_shared_by_all & BIT(type));
 }
 
+/**
+ * iio_channel_has_available() - Checks if a channel has an available attribute
+ * @chan: The channel to be queried
+ * @type: Type of the available attribute to be checked
+ *
+ * Returns true if the channels supports reporting available values for the
+ * given attribute type, false otherwise.
+ */
+static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
+					     enum iio_chan_info_enum type)
+{
+	return (chan->info_mask_separate_available & BIT(type)) |
+		(chan->info_mask_shared_by_type_available & BIT(type)) |
+		(chan->info_mask_shared_by_dir_available & BIT(type)) |
+		(chan->info_mask_shared_by_all_available & BIT(type));
+}
+
 #define IIO_CHAN_SOFT_TIMESTAMP(_si) {					\
 	.type = IIO_TIMESTAMP,						\
 	.channel = -1,							\
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 3/8] iio: mcp4531: provide range of available raw values
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477262381-7800-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Example:

$ cat '/sys/bus/iio/devices/iio:device0/out_resistance_raw_available'
[0 1 256]

Meaning: min 0, step 1 and max 256.

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 .../testing/sysfs-bus-iio-potentiometer-mcp4531    |   8 ++
 MAINTAINERS                                        |   1 +
 drivers/iio/potentiometer/mcp4531.c                | 104 ++++++++++++---------
 3 files changed, 71 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531 b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
new file mode 100644
index 000000000000..2a91fbe394fc
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
@@ -0,0 +1,8 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_resistance_raw_available
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+Description:
+		The range of available values represented as the minimum value,
+		the step and the maximum value, all enclosed in square brackets.
+		Example: [0 1 256]
diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd38a7e0064..7c65585e1230 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7694,6 +7694,7 @@ MCP4531 MICROCHIP DIGITAL POTENTIOMETER DRIVER
 M:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
 L:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
 F:	drivers/iio/potentiometer/mcp4531.c
 
 MEASUREMENT COMPUTING CIO-DAC IIO DRIVER
diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
index 13b6ae2fcf7b..0d1bcf89ae17 100644
--- a/drivers/iio/potentiometer/mcp4531.c
+++ b/drivers/iio/potentiometer/mcp4531.c
@@ -38,7 +38,7 @@
 
 struct mcp4531_cfg {
 	int wipers;
-	int max_pos;
+	int avail[3];
 	int kohms;
 };
 
@@ -78,38 +78,38 @@ enum mcp4531_type {
 };
 
 static const struct mcp4531_cfg mcp4531_cfg[] = {
-	[MCP453x_502] = { .wipers = 1, .max_pos = 128, .kohms =   5, },
-	[MCP453x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
-	[MCP453x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
-	[MCP453x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
-	[MCP454x_502] = { .wipers = 1, .max_pos = 128, .kohms =   5, },
-	[MCP454x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
-	[MCP454x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
-	[MCP454x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
-	[MCP455x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
-	[MCP455x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
-	[MCP455x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
-	[MCP455x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
-	[MCP456x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
-	[MCP456x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
-	[MCP456x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
-	[MCP456x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
-	[MCP463x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
-	[MCP463x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
-	[MCP463x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
-	[MCP463x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
-	[MCP464x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
-	[MCP464x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
-	[MCP464x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
-	[MCP464x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
-	[MCP465x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
-	[MCP465x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
-	[MCP465x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
-	[MCP465x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
-	[MCP466x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
-	[MCP466x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
-	[MCP466x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
-	[MCP466x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
+	[MCP453x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP453x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP453x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP453x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP454x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP454x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP454x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP454x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP455x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP455x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP455x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP455x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
+	[MCP456x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP456x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP456x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP456x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
+	[MCP463x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP463x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP463x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP463x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP464x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP464x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP464x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP464x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP465x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP465x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP465x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP465x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
+	[MCP466x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP466x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP466x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP466x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
 };
 
 #define MCP4531_WRITE (0 << 2)
@@ -124,13 +124,14 @@ struct mcp4531_data {
 	const struct mcp4531_cfg *cfg;
 };
 
-#define MCP4531_CHANNEL(ch) {					\
-	.type = IIO_RESISTANCE,					\
-	.indexed = 1,						\
-	.output = 1,						\
-	.channel = (ch),					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+#define MCP4531_CHANNEL(ch) {						\
+	.type = IIO_RESISTANCE,						\
+	.indexed = 1,							\
+	.output = 1,							\
+	.channel = (ch),						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),	\
 }
 
 static const struct iio_chan_spec mcp4531_channels[] = {
@@ -156,13 +157,31 @@ static int mcp4531_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		*val = 1000 * data->cfg->kohms;
-		*val2 = data->cfg->max_pos;
+		*val2 = data->cfg->avail[2];
 		return IIO_VAL_FRACTIONAL;
 	}
 
 	return -EINVAL;
 }
 
+static int mcp4531_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct mcp4531_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*length = ARRAY_SIZE(data->cfg->avail);
+		*vals = data->cfg->avail;
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_RANGE;
+	}
+
+	return -EINVAL;
+}
+
 static int mcp4531_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
@@ -172,7 +191,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val > data->cfg->max_pos || val < 0)
+		if (val > data->cfg->avail[2] || val < 0)
 			return -EINVAL;
 		break;
 	default:
@@ -186,6 +205,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
 
 static const struct iio_info mcp4531_info = {
 	.read_raw = mcp4531_read_raw,
+	.read_avail = mcp4531_read_avail,
 	.write_raw = mcp4531_write_raw,
 	.driver_module = THIS_MODULE,
 };
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 4/8] dt-bindings: add axentia to vendor-prefixes
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477262381-7800-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index f0a48ea78659..a437120a7eee 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -39,6 +39,7 @@ auo	AU Optronics Corporation
 auvidea Auvidea GmbH
 avago	Avago Technologies
 avic	Shanghai AVIC Optoelectronics Co., Ltd.
+axentia	Axentia Technologies AB
 axis	Axis Communications AB
 boe	BOE Technology Group Co., Ltd.
 bosch	Bosch Sensortec GmbH
-- 
2.1.4

^ permalink raw reply related

* [PATCH v3 5/8] dt-bindings: iio: document dpot-dac bindings
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland, linux-iio,
	devicetree
In-Reply-To: <1477262381-7800-1-git-send-email-peda@axentia.se>

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/iio/dac/dpot-dac.txt       | 41 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++++
 2 files changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/dpot-dac.txt

diff --git a/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
new file mode 100644
index 000000000000..fdf47a01bfef
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
@@ -0,0 +1,41 @@
+Bindings for DAC emulation using a digital potentiometer
+
+It is assumed that the dpot is used as a voltage divider between the
+current dpot wiper setting and the maximum resistance of the dpot. The
+divided voltage is provided by a vref regulator.
+
+                  .------.
+   .-----------.  |      |
+   | vref      |--'    .---.
+   | regulator |--.    |   |
+   '-----------'  |    | d |
+                  |    | p |
+                  |    | o |  wiper
+                  |    | t |<---------+
+                  |    |   |
+                  |    '---'       dac output voltage
+                  |      |
+                  '------+------------+
+
+Required properties:
+- compatible: Should be "dpot-dac"
+- vref-supply: The regulator supplying the voltage divider.
+- io-channels: Channel node of the dpot to be used for the voltage division.
+- io-channel-names: Should be "dpot".
+
+Example:
+
+	&i2c {
+		dpot: mcp4651-503@28 {
+			compatible = "microchip,mcp4651-503";
+			reg = <0x28>;
+			#io-channel-cells = <1>;
+		};
+	};
+
+	dac {
+		compatible = "dpot-dac";
+		vref-supply = <&reg_3v3>;
+		io-channels = <&dpot 0>;
+		io-channel-names = "dpot";
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 7c65585e1230..6218010128dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6111,6 +6111,12 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	drivers/media/rc/iguanair.c
 
+IIO DIGITAL POTENTIOMETER DAC
+M:	Peter Rosin <peda@axentia.se>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
+
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
 R:	Hartmut Knaack <knaack.h@gmx.de>
-- 
2.1.4

^ permalink raw reply related

* [PATCH v3 6/8] iio: dpot-dac: DAC driver based on a digital potentiometer
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477262381-7800-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

It is assumed that the dpot is used as a voltage divider between the
current dpot wiper setting and the maximum resistance of the dpot. The
divided voltage is provided by a vref regulator.

                  .------.
   .-----------.  |      |
   | vref      |--'    .---.
   | regulator |--.    |   |
   '-----------'  |    | d |
                  |    | p |
                  |    | o |  wiper
                  |    | t |<---------+
                  |    |   |
                  |    '---'       dac output voltage
                  |      |
                  '------+------------+

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 .../ABI/testing/sysfs-bus-iio-dac-dpot-dac         |   8 +
 MAINTAINERS                                        |   2 +
 drivers/iio/dac/Kconfig                            |  10 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/dpot-dac.c                         | 267 +++++++++++++++++++++
 5 files changed, 288 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 create mode 100644 drivers/iio/dac/dpot-dac.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
new file mode 100644
index 000000000000..580e93f373f6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
@@ -0,0 +1,8 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw_available
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+Description:
+		The range of available values represented as the minimum value,
+		the step and the maximum value, all enclosed in square brackets.
+		Example: [0 1 256]
diff --git a/MAINTAINERS b/MAINTAINERS
index 6218010128dc..d7375f45ff0f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6115,7 +6115,9 @@ IIO DIGITAL POTENTIOMETER DAC
 M:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
 L:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
+F:	drivers/iio/dac/dpot-dac.c
 
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 120b24478469..d3084028905b 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -200,6 +200,16 @@ config AD8801
 	  To compile this driver as a module choose M here: the module will be called
 	  ad8801.
 
+config DPOT_DAC
+	tristate "DAC emulation using a DPOT"
+	depends on OF
+	help
+	  Say yes here to build support for DAC emulation using a digital
+	  potentiometer.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called dpot-dac.
+
 config LPC18XX_DAC
 	tristate "NXP LPC18xx DAC driver"
 	depends on ARCH_LPC18XX || COMPILE_TEST
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 27642bbf75f2..f01bf4a99867 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_AD7303) += ad7303.o
 obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_CIO_DAC) += cio-dac.o
+obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
 obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
 obj-$(CONFIG_M62332) += m62332.o
 obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/dpot-dac.c b/drivers/iio/dac/dpot-dac.c
new file mode 100644
index 000000000000..f227a211d34d
--- /dev/null
+++ b/drivers/iio/dac/dpot-dac.c
@@ -0,0 +1,267 @@
+/*
+ * IIO DAC emulation driver using a digital potentiometer
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * It is assumed that the dpot is used as a voltage divider between the
+ * current dpot wiper setting and the maximum resistance of the dpot. The
+ * divided voltage is provided by a vref regulator.
+ *
+ *                   .------.
+ *    .-----------.  |      |
+ *    | vref      |--'    .---.
+ *    | regulator |--.    |   |
+ *    '-----------'  |    | d |
+ *                   |    | p |
+ *                   |    | o |  wiper
+ *                   |    | t |<---------+
+ *                   |    |   |
+ *                   |    '---'       dac output voltage
+ *                   |      |
+ *                   '------+------------+
+ */
+
+#include <linux/err.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct dpot_dac {
+	struct regulator *vref;
+	struct iio_channel *dpot;
+	u32 max_ohms;
+};
+
+static const struct iio_chan_spec dpot_dac_iio_channel = {
+	.type = IIO_VOLTAGE,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+			    | BIT(IIO_CHAN_INFO_SCALE),
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
+	.output = 1,
+	.indexed = 1,
+};
+
+static int dpot_dac_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct dpot_dac *dac = iio_priv(indio_dev);
+	int ret;
+	unsigned long long tmp;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return iio_read_channel_raw(dac->dpot, val);
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = iio_read_channel_scale(dac->dpot, val, val2);
+		switch (ret) {
+		case IIO_VAL_FRACTIONAL_LOG2:
+			tmp = *val * 1000000000LL;
+			do_div(tmp, dac->max_ohms);
+			tmp *= regulator_get_voltage(dac->vref) / 1000;
+			do_div(tmp, 1000000000LL);
+			*val = tmp;
+			return ret;
+		case IIO_VAL_INT:
+			/*
+			 * Convert integer scale to fractional scale by
+			 * setting the denominator (val2) to one...
+			 */
+			*val2 = 1;
+			ret = IIO_VAL_FRACTIONAL;
+			/* ...and fall through. */
+		case IIO_VAL_FRACTIONAL:
+			*val *= regulator_get_voltage(dac->vref) / 1000;
+			*val2 *= dac->max_ohms;
+			break;
+		}
+
+		return ret;
+	}
+
+	return -EINVAL;
+}
+
+static int dpot_dac_read_avail(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       const int **vals, int *type, int *length,
+			       long mask)
+{
+	struct dpot_dac *dac = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return iio_read_avail_channel_raw(dac->dpot,
+						  vals, type, length);
+	}
+
+	return -EINVAL;
+}
+
+static int dpot_dac_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	struct dpot_dac *dac = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return iio_write_channel_raw(dac->dpot, val);
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info dpot_dac_info = {
+	.read_raw = dpot_dac_read_raw,
+	.read_avail = dpot_dac_read_avail,
+	.write_raw = dpot_dac_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int dpot_dac_channel_max_ohms(struct iio_dev *indio_dev)
+{
+	struct device *dev = &indio_dev->dev;
+	struct dpot_dac *dac = iio_priv(indio_dev);
+	unsigned long long tmp;
+	int ret;
+	int val;
+	int val2;
+	int max;
+
+	ret = iio_read_max_channel_raw(dac->dpot, &max);
+	if (ret < 0) {
+		dev_err(dev, "dpot does not indicate its raw maximum value\n");
+		return ret;
+	}
+
+	switch (iio_read_channel_scale(dac->dpot, &val, &val2)) {
+	case IIO_VAL_INT:
+		return max * val;
+	case IIO_VAL_FRACTIONAL:
+		tmp = (unsigned long long)max * val;
+		do_div(tmp, val2);
+		return tmp;
+	case IIO_VAL_FRACTIONAL_LOG2:
+		tmp = (s64)val * 1000000000LL * max >> val2;
+		do_div(tmp, 1000000000LL);
+		return tmp;
+	default:
+		dev_err(dev, "dpot has a scale that is too weird\n");
+	}
+
+	return -EINVAL;
+}
+
+static int dpot_dac_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iio_dev *indio_dev;
+	struct dpot_dac *dac;
+	enum iio_chan_type type;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*dac));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, indio_dev);
+	dac = iio_priv(indio_dev);
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &dpot_dac_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = &dpot_dac_iio_channel;
+	indio_dev->num_channels = 1;
+
+	dac->vref = devm_regulator_get(dev, "vref");
+	if (IS_ERR(dac->vref)) {
+		if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "failed to get vref regulator\n");
+		return PTR_ERR(dac->vref);
+	}
+
+	dac->dpot = devm_iio_channel_get(dev, "dpot");
+	if (IS_ERR(dac->dpot)) {
+		if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get dpot input channel\n");
+		return PTR_ERR(dac->dpot);
+	}
+
+	ret = iio_get_channel_type(dac->dpot, &type);
+	if (ret < 0)
+		return ret;
+
+	if (type != IIO_RESISTANCE) {
+		dev_err(dev, "dpot is of the wrong type\n");
+		return -EINVAL;
+	}
+
+	ret = dpot_dac_channel_max_ohms(indio_dev);
+	if (ret < 0)
+		return ret;
+	dac->max_ohms = ret;
+	dev_info(dev, "dpot max is %d\n", dac->max_ohms);
+
+	ret = regulator_enable(dac->vref);
+	if (ret) {
+		dev_err(dev, "failed to enable the vref regulator\n");
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "failed to register iio device\n");
+		goto disable_reg;
+	}
+
+	return 0;
+
+disable_reg:
+	regulator_disable(dac->vref);
+	return ret;
+}
+
+static int dpot_dac_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct dpot_dac *dac = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	regulator_disable(dac->vref);
+
+	return 0;
+}
+
+static const struct of_device_id dpot_dac_match[] = {
+	{ .compatible = "dpot-dac" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dpot_dac_match);
+
+static struct platform_driver dpot_dac_driver = {
+	.probe = dpot_dac_probe,
+	.remove = dpot_dac_remove,
+	.driver = {
+		.name = "iio-dpot-dac",
+		.of_match_table = dpot_dac_match,
+	},
+};
+module_platform_driver(dpot_dac_driver);
+
+MODULE_DESCRIPTION("DAC emulation driver using a digital potentiometer");
+MODULE_AUTHOR("Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 7/8] dt-bindings: iio: document envelope-detector bindings
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477262381-7800-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 .../bindings/iio/adc/envelope-detector.txt         | 54 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
new file mode 100644
index 000000000000..27544bdd4478
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
@@ -0,0 +1,54 @@
+Bindings for ADC envelope detector using a DAC and a comparator
+
+The DAC is used to find the peak level of an alternating voltage input
+signal by a binary search using the output of a comparator wired to
+an interrupt pin. Like so:
+                          _
+                         | \
+    input +------>-------|+ \
+                         |   \
+           .-------.     |    }---.
+           |       |     |   /    |
+           |    dac|-->--|- /     |
+           |       |     |_/      |
+           |       |              |
+           |       |              |
+           |    irq|------<-------'
+           |       |
+           '-------'
+
+Required properties:
+- compatible: Should be "axentia,tse850-envelope-detector"
+- io-channels: Channel node of the dac to be used for comparator input.
+- io-channel-names: Should be "dac".
+- interrupt specification for one client interrupt,
+  see ../../interrupt-controller/interrupts.txt for details.
+- interrupt-names: Should be "comp".
+
+Example:
+
+	&i2c {
+		dpot: mcp4651-104@28 {
+			compatible = "microchip,mcp4651-104";
+			reg = <0x28>;
+			#io-channel-cells = <1>;
+		};
+	};
+
+	dac: dac {
+		compatible = "dpot-dac";
+		vref-supply = <&reg_3v3>;
+		io-channels = <&dpot 0>;
+		io-channel-names = "dpot";
+		#io-channel-cells = <1>;
+	};
+
+	envelope-detector {
+		compatible = "axentia,tse850-envelope-detector";
+		io-channels = <&dac 0>;
+		io-channel-names = "dac";
+
+		interrupt-parent = <&gpio>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		interrupt-names = "comp";
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index d7375f45ff0f..fca35d16037d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6119,6 +6119,12 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
 F:	drivers/iio/dac/dpot-dac.c
 
+IIO ENVELOPE DETECTOR
+M:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+L:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
+
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
 R:	Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator
From: Peter Rosin @ 2016-10-23 22:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477262381-7800-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

The DAC is used to find the peak level of an alternating voltage input
signal by a binary search using the output of a comparator wired to
an interrupt pin. Like so:
                          _
                         | \
    input +------>-------|+ \
                         |   \
           .-------.     |    }---.
           |       |     |   /    |
           |    dac|-->--|- /     |
           |       |     |_/      |
           |       |              |
           |       |              |
           |    irq|------<-------'
           |       |
           '-------'

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 .../testing/sysfs-bus-iio-adc-envelope-detector    |  36 ++
 MAINTAINERS                                        |   2 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/envelope-detector.c                | 422 +++++++++++++++++++++
 5 files changed, 471 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 create mode 100644 drivers/iio/adc/envelope-detector.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
new file mode 100644
index 000000000000..2071f9bcfaa5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
@@ -0,0 +1,36 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_altvoltageY_invert
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+Description:
+		The DAC is used to find the peak level of an alternating
+		voltage input signal by a binary search using the output
+		of a comparator wired to an interrupt pin. Like so:
+		                           _
+		                          | \
+		     input +------>-------|+ \
+		                          |   \
+		            .-------.     |    }---.
+		            |       |     |   /    |
+		            |    dac|-->--|- /     |
+		            |       |     |_/      |
+		            |       |              |
+		            |       |              |
+		            |    irq|------<-------'
+		            |       |
+		            '-------'
+		The boolean invert attribute (0/1) should be set when the
+		input signal is centered around the maximum value of the
+		dac instead of zero. The envelope detector will search
+		from below in this case and will also invert the result.
+		The edge/level of the interrupt is also switched to its
+		opposite value.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_altvoltageY_compare_interval
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+Description:
+		Number of milliseconds to wait for the comparator in each
+		step of the binary search for the input peak level. Needs
+		to relate to the frequency of the input signal.
diff --git a/MAINTAINERS b/MAINTAINERS
index fca35d16037d..0cf3549e05e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6123,7 +6123,9 @@ IIO ENVELOPE DETECTOR
 M:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
 L:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
+F:	drivers/iio/adc/envelope-detector.c
 
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7edcf3238620..d5c4a95855c2 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -195,6 +195,16 @@ config DA9150_GPADC
 	  To compile this driver as a module, choose M here: the module will be
 	  called berlin2-adc.
 
+config ENVELOPE_DETECTOR
+	tristate "Envelope detector using a DAC and a comparator"
+	depends on OF
+	help
+	  Say yes here to build support for an envelope detector using a DAC
+	  and a comparator.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called iio-envelope-detector.
+
 config EXYNOS_ADC
 	tristate "Exynos ADC driver support"
 	depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || (OF && COMPILE_TEST)
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7a40c04c311f..0d773c6a0578 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
 obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
 obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
 obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
+obj-$(CONFIG_ENVELOPE_DETECTOR) += envelope-detector.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
 obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
 obj-$(CONFIG_HI8435) += hi8435.o
diff --git a/drivers/iio/adc/envelope-detector.c b/drivers/iio/adc/envelope-detector.c
new file mode 100644
index 000000000000..fef15c0d7c9c
--- /dev/null
+++ b/drivers/iio/adc/envelope-detector.c
@@ -0,0 +1,422 @@
+/*
+ * Driver for an envelope detector using a DAC and a comparator
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * The DAC is used to find the peak level of an alternating voltage input
+ * signal by a binary search using the output of a comparator wired to
+ * an interrupt pin. Like so:
+ *                           _
+ *                          | \
+ *     input +------>-------|+ \
+ *                          |   \
+ *            .-------.     |    }---.
+ *            |       |     |   /    |
+ *            |    dac|-->--|- /     |
+ *            |       |     |_/      |
+ *            |       |              |
+ *            |       |              |
+ *            |    irq|------<-------'
+ *            |       |
+ *            '-------'
+ */
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+struct envelope {
+	spinlock_t comp_lock; /* protects comp */
+	int comp;
+
+	struct mutex read_lock; /* protects everything else */
+
+	int comp_irq;
+	u32 comp_irq_trigger;
+	u32 comp_irq_trigger_inv;
+
+	struct iio_channel *dac;
+	struct delayed_work comp_timeout;
+
+	unsigned int comp_interval;
+	bool invert;
+	u32 dac_max;
+
+	int high;
+	int level;
+	int low;
+
+	struct completion done;
+};
+
+/*
+ * The envelope_detector_comp_latch function works together with the compare
+ * interrupt service routine below (envelope_detector_comp_isr) as a latch
+ * (one-bit memory) for if the interrupt has triggered since last calling
+ * this function.
+ * The ..._comp_isr function disables the interrupt so that the cpu does not
+ * need to service a possible interrupt flood from the comparator when no-one
+ * cares anyway, and this ..._comp_latch function reenables them again if
+ * needed.
+ */
+static int envelope_detector_comp_latch(struct envelope *env)
+{
+	int comp;
+
+	spin_lock_irq(&env->comp_lock);
+	comp = env->comp;
+	env->comp = 0;
+	spin_unlock_irq(&env->comp_lock);
+
+	if (!comp)
+		return 0;
+
+	/*
+	 * The irq was disabled, and is reenabled just now.
+	 * But there might have been a pending irq that
+	 * happened while the irq was disabled that fires
+	 * just as the irq is reenabled. That is not what
+	 * is desired.
+	 */
+	enable_irq(env->comp_irq);
+
+	/* So, synchronize this possibly pending irq... */
+	synchronize_irq(env->comp_irq);
+
+	/* ...and redo the whole dance. */
+	spin_lock_irq(&env->comp_lock);
+	comp = env->comp;
+	env->comp = 0;
+	spin_unlock_irq(&env->comp_lock);
+
+	if (comp)
+		enable_irq(env->comp_irq);
+
+	return 1;
+}
+
+static irqreturn_t envelope_detector_comp_isr(int irq, void *ctx)
+{
+	struct envelope *env = ctx;
+
+	spin_lock(&env->comp_lock);
+	env->comp = 1;
+	disable_irq_nosync(env->comp_irq);
+	spin_unlock(&env->comp_lock);
+
+	return IRQ_HANDLED;
+}
+
+static void envelope_detector_setup_compare(struct envelope *env)
+{
+	int ret;
+
+	/*
+	 * Do a binary search for the peak input level, and stop
+	 * when that level is "trapped" between two adjacent DAC
+	 * values.
+	 * When invert is active, use the midpoint floor so that
+	 * env->level ends up as env->low when the termination
+	 * criteria below is fulfilled, and use the midpoint
+	 * ceiling when invert is not active so that env->level
+	 * ends up as env->high in that case.
+	 */
+	env->level = (env->high + env->low + !env->invert) / 2;
+
+	if (env->high == env->low + 1) {
+		complete(&env->done);
+		return;
+	}
+
+	/* Set a "safe" DAC level (if there is such a thing)... */
+	ret = iio_write_channel_raw(env->dac, env->invert ? 0 : env->dac_max);
+	if (ret < 0)
+		goto err;
+
+	/* ...clear the comparison result... */
+	envelope_detector_comp_latch(env);
+
+	/* ...set the real DAC level... */
+	ret = iio_write_channel_raw(env->dac, env->level);
+	if (ret < 0)
+		goto err;
+
+	/* ...and wait for a bit to see if the latch catches anything. */
+	schedule_delayed_work(&env->comp_timeout,
+			      msecs_to_jiffies(env->comp_interval));
+	return;
+
+err:
+	env->level = ret;
+	complete(&env->done);
+}
+
+static void envelope_detector_timeout(struct work_struct *work)
+{
+	struct envelope *env = container_of(work, struct envelope,
+					    comp_timeout.work);
+
+	/* Adjust low/high depending on the latch content... */
+	if (!envelope_detector_comp_latch(env) ^ !env->invert)
+		env->low = env->level;
+	else
+		env->high = env->level;
+
+	/* ...and continue the search. */
+	envelope_detector_setup_compare(env);
+}
+
+static int envelope_detector_read_raw(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      int *val, int *val2, long mask)
+{
+	struct envelope *env = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		/*
+		 * When invert is active, start with high=max+1 and low=0
+		 * since we will end up with the low value when the
+		 * termination criteria is fulfilled (rounding down). And
+		 * start with high=max and low=-1 when invert is not active
+		 * since we will end up with the high value in that case.
+		 * This ensures that the returned value in both cases are
+		 * in the same range as the DAC and is a value that has not
+		 * triggered the comparator.
+		 */
+		mutex_lock(&env->read_lock);
+		env->high = env->dac_max + env->invert;
+		env->low = -1 + env->invert;
+		envelope_detector_setup_compare(env);
+		wait_for_completion(&env->done);
+		if (env->level < 0) {
+			ret = env->level;
+			goto err_unlock;
+		}
+		*val = env->invert ? env->dac_max - env->level : env->level;
+		mutex_unlock(&env->read_lock);
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		return iio_read_channel_scale(env->dac, val, val2);
+	}
+
+	return -EINVAL;
+
+err_unlock:
+	mutex_unlock(&env->read_lock);
+	return ret;
+}
+
+static ssize_t envelope_show_invert(struct iio_dev *indio_dev,
+				    uintptr_t private,
+				    struct iio_chan_spec const *ch, char *buf)
+{
+	struct envelope *env = iio_priv(indio_dev);
+
+	return sprintf(buf, "%u\n", env->invert);
+}
+
+static ssize_t envelope_store_invert(struct iio_dev *indio_dev,
+				     uintptr_t private,
+				     struct iio_chan_spec const *ch,
+				     const char *buf, size_t len)
+{
+	struct envelope *env = iio_priv(indio_dev);
+	unsigned long invert;
+	int ret;
+	u32 trigger;
+
+	ret = kstrtoul(buf, 0, &invert);
+	if (ret < 0)
+		return ret;
+	if (invert > 1)
+		return -EINVAL;
+
+	trigger = invert ? env->comp_irq_trigger_inv : env->comp_irq_trigger;
+
+	mutex_lock(&env->read_lock);
+	if (invert != env->invert)
+		ret = irq_set_irq_type(env->comp_irq, trigger);
+	if (!ret) {
+		env->invert = invert;
+		ret = len;
+	}
+	mutex_unlock(&env->read_lock);
+
+	return ret;
+}
+
+static ssize_t envelope_show_comp_interval(struct iio_dev *indio_dev,
+					   uintptr_t private,
+					   struct iio_chan_spec const *ch,
+					   char *buf)
+{
+	struct envelope *env = iio_priv(indio_dev);
+
+	return sprintf(buf, "%u\n", env->comp_interval);
+}
+
+static ssize_t envelope_store_comp_interval(struct iio_dev *indio_dev,
+					    uintptr_t private,
+					    struct iio_chan_spec const *ch,
+					    const char *buf, size_t len)
+{
+	struct envelope *env = iio_priv(indio_dev);
+	unsigned long interval;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &interval);
+	if (ret < 0)
+		return ret;
+	if (interval > 1000)
+		return -EINVAL;
+
+	mutex_lock(&env->read_lock);
+	env->comp_interval = interval;
+	mutex_unlock(&env->read_lock);
+
+	return len;
+}
+
+static const struct iio_chan_spec_ext_info envelope_detector_ext_info[] = {
+	{ .name = "invert",
+	  .read = envelope_show_invert,
+	  .write = envelope_store_invert, },
+	{ .name = "compare_interval",
+	  .read = envelope_show_comp_interval,
+	  .write = envelope_store_comp_interval, },
+	{ /* sentinel */ }
+};
+
+static const struct iio_chan_spec envelope_detector_iio_channel = {
+	.type = IIO_ALTVOLTAGE,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+			    | BIT(IIO_CHAN_INFO_SCALE),
+	.ext_info = envelope_detector_ext_info,
+	.indexed = 1,
+};
+
+static const struct iio_info envelope_detector_info = {
+	.read_raw = &envelope_detector_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int envelope_detector_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iio_dev *indio_dev;
+	struct envelope *env;
+	enum iio_chan_type type;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*env));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, indio_dev);
+	env = iio_priv(indio_dev);
+	env->comp_interval = 50; /* some sensible default? */
+
+	spin_lock_init(&env->comp_lock);
+	mutex_init(&env->read_lock);
+	init_completion(&env->done);
+	INIT_DELAYED_WORK(&env->comp_timeout, envelope_detector_timeout);
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->dev.of_node = dev->of_node;
+	indio_dev->info = &envelope_detector_info;
+	indio_dev->channels = &envelope_detector_iio_channel;
+	indio_dev->num_channels = 1;
+
+	env->dac = devm_iio_channel_get(dev, "dac");
+	if (IS_ERR(env->dac)) {
+		if (PTR_ERR(env->dac) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get dac input channel\n");
+		return PTR_ERR(env->dac);
+	}
+
+	env->comp_irq = platform_get_irq_byname(pdev, "comp");
+	if (env->comp_irq < 0) {
+		if (env->comp_irq != -EPROBE_DEFER)
+			dev_err(dev, "failed to get compare interrupt\n");
+		return env->comp_irq;
+	}
+
+	ret = devm_request_irq(dev, env->comp_irq, envelope_detector_comp_isr,
+			       0, "envelope-detector", env);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to request interrupt\n");
+		return ret;
+	}
+	env->comp_irq_trigger = irq_get_trigger_type(env->comp_irq);
+	if (env->comp_irq_trigger & IRQF_TRIGGER_RISING)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_FALLING;
+	if (env->comp_irq_trigger & IRQF_TRIGGER_FALLING)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_RISING;
+	if (env->comp_irq_trigger & IRQF_TRIGGER_HIGH)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_LOW;
+	if (env->comp_irq_trigger & IRQF_TRIGGER_LOW)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_HIGH;
+
+	ret = iio_get_channel_type(env->dac, &type);
+	if (ret < 0)
+		return ret;
+
+	if (type != IIO_VOLTAGE) {
+		dev_err(dev, "dac is of the wrong type\n");
+		return -EINVAL;
+	}
+
+	ret = iio_read_max_channel_raw(env->dac, &env->dac_max);
+	if (ret < 0) {
+		dev_err(dev, "dac does not indicate its raw maximum value\n");
+		return ret;
+	}
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id envelope_detector_match[] = {
+	{ .compatible = "axentia,tse850-envelope-detector", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, envelope_detector_match);
+
+static struct platform_driver envelope_detector_driver = {
+	.probe = envelope_detector_probe,
+	.driver = {
+		.name = "iio-envelope-detector",
+		.of_match_table = envelope_detector_match,
+	},
+};
+module_platform_driver(envelope_detector_driver);
+
+MODULE_DESCRIPTION("Envelope detector using a DAC and a comparator");
+MODULE_AUTHOR("Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH 1/2] pinctrl: stm32: remove dependency with interrupt controller
From: Linus Walleij @ 2016-10-23 23:55 UTC (permalink / raw)
  To: Alexandre TORGUE
  Cc: Maxime Coquelin, Patrice Chotard, Paul Gortmaker, Rob Herring,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <1476970012-7838-2-git-send-email-alexandre.torgue@st.com>

On Thu, Oct 20, 2016 at 3:26 PM, Alexandre TORGUE
<alexandre.torgue@st.com> wrote:

> This patch allows to probe stm32 pinctrl driver even if no interrupt
> controller is defined to manage gpio irqs.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

Patch applied for fixes.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 2/2] pinctrl: stm32: move gpio irqs binding to optional
From: Linus Walleij @ 2016-10-23 23:56 UTC (permalink / raw)
  To: Alexandre TORGUE
  Cc: Maxime Coquelin, Patrice Chotard, Paul Gortmaker, Rob Herring,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <1476970012-7838-3-git-send-email-alexandre.torgue@st.com>

On Thu, Oct 20, 2016 at 3:26 PM, Alexandre TORGUE
<alexandre.torgue@st.com> wrote:

> stm32 pinctrl driver could be probed even if no interrupt controller
> is defined to manage gpio irqs. Entries related to gpio irq management
> are moved to optional.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

Patch applied for fixes.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v3 2/6] pinctrl: sunxi: Support generic binding
From: Linus Walleij @ 2016-10-24  0:02 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Rob Herring
In-Reply-To: <519ec867509f8033d9235a61f7979bd698906ab5.1476971126.git-series.maxime.ripard@free-electrons.com>

On Thu, Oct 20, 2016 at 3:49 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> Our bindings are mostly irrelevant now that we have generic pinctrl
> bindings that cover exactly the same uses cases.
>
> Add support for the new ones, and obviously keep our old binding support in
> order to keep the ABI stable.
>
> Acked-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v3 3/6] dt-bindings: pinctrl: Deprecate sunxi pinctrl bindings
From: Linus Walleij @ 2016-10-24  0:03 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Rob Herring
In-Reply-To: <6aec73cd3b9d3dbf1085d042ec6c23f385a300de.1476971126.git-series.maxime.ripard@free-electrons.com>

On Thu, Oct 20, 2016 at 3:49 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> The generic pin configuration and multiplexing should be preferred now,
> even though we still support the old one.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Acked-by: Chen-Yu Tsai <wens@csie.org>
> Acked-by: Rob Herring <robh@kernel.org>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 5/8] pinctrl: aspeed: Enable capture of off-SCU pinmux state
From: Andrew Jeffery @ 2016-10-24  0:29 UTC (permalink / raw)
  To: Linus Walleij, Joel Stanley
  Cc: Mark Rutland, Rob Herring, linux-gpio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	OpenBMC Maillist
In-Reply-To: <CACRpkdazJzE=Oa-0-FEYTvUk=-kPLojHY3afZKX5cpmzrUZHGQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4416 bytes --]

On Mon, 2016-10-24 at 00:20 +0200, Linus Walleij wrote:
> On Thu, Sep 29, 2016 at 8:45 AM, Joel Stanley <joel@jms.id.au> wrote:
> > 
> > On Wed, Sep 28, 2016 at 12:20 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> > 
> > > 
> > > On the AST2400 and AST2500 a number of pins depend on state in one of
> > > the SIO, LPC or GFX IP blocks, so add support to at least capture what
> > > that state is. The pinctrl engine for the Aspeed SoCs doesn't try to
> > > inspect or modify the state of the off-SCU IP blocks. Instead, it logs
> > > the state requirement with the expectation that the platform
> > > designer/maintainer arranges for the appropriate configuration to be
> > > applied through the associated drivers.
> (...)
> > 
> > 
> > This is unfortunate.
> > 
> > This patch kicks the can down the road, but doesn't solve the problem
> > for a user who wants to configure some functionality that depends on
> > the non-SCU bits. Because of this I'm not sure if we want to put it in
> > the tree.
> > 
> > However, I'm not sure what a proper solution would look like. Perhaps
> > Linus can point out another SoC that has a similar problem?
> If I could only understand it.
> 
> Don't you actually want to go and read a few registers on another
> IO range?

Yes. I guess the hesitation was whether we should also write those
registers so we can apply the requested function.

> 
> What we generally do when pin control is spread out in a system is try
> to consolidate it, meaning expose the necessary registers on the remote
> end using syscon (drivers/mfd/syscon) so that we can easily get a handle
> on these registers withe regmap MMIO.
> 
> Since regmap handles atomic access to the registers, that way we
> protect from disasters and keep the state in the hardware.

This seems like the reasonable approach if we want to read/write those
registers. The affected IO ranges correspond to:

* SuperIO Controller (SIO)
* SOC Display Controller (GFX)
* LPC Controller (LPC)

SIO and LPC both perform a mishmash of functions and so likely would
use the mfd subsystem anyway. I don't yet have any arguments against
doing it for the GFX IO space. Joel?

> 
> I don't know if this is helpful. For a normal peripheral you may not want
> to put a regmap over all its registers but you prefer to ioremap it, and then
> we get the spaghetti of accessor functions to peek and poke into another
> peripherals I/O space, and that is undesireable.

I briefly experimented with the idea of accessing the other IO spaces
but it felt dirty precisely for what would have become accessor
spaghetti. So I wound up with the lame approach in this patch, which
just punts on the problem. I think the mfd/syscon approach would work
well though.

It looks like the rockchip pinctrl bindings are doing something along
the lines of what we need with the rockchip,pmu phandle property. Is it
acceptable to create three properties, a phandle to grab each regmap
for the IO spaces above?

> 
> Maybe this is completely not understanding what you want to do, so
> sorry, please elaborate. 

No, seems you have understood what we need to do.

> I am afraid the two of you are the only people on
> the planet who actually understand what is going on here.
> 
> Also the hardware engineer who wrote the Aspeed pin controller, I would
> like to read this persons design specification, I am pretty sure it is very
> interesting.

Well, presumably this engineer knows too :) And yes, I'd like to know
what constraints existed that forced this design as a solution. I'd
like my sanity back.

> 
> > 
> > > 
> > > -  * @reg: The register offset from base in bytes
> > > +  * @reg: Split into three fields:
> > > +  *       31:24: IP selector
> > > +  *       23:12: Reserved
> > > +  *       11:0: Register offset
> That seems like unnecessary shoehorning. Is it reflecting the register layout
> of the hardware or are you trying to save bits? For the latter, let it
> go and instead
> have one member for offset and one member for selector.

It doesn't represent the register layout. But saving bits also wasn't
the motivation, more avoiding a lot of code churn in the g4/g5 drivers
to populate a new member. Maybe that's a bad motivation. I'll have more
of a think about it.

Thanks for the feedback,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 6/8] pinctrl: aspeed-g4: Capture SuperIO pinmux dependency
From: Andrew Jeffery @ 2016-10-24  0:30 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Joel Stanley, Mark Rutland, Rob Herring,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <CACRpkdYZPcjGuRKVL6qwof1p7ZXT4EvwzAuz59oTgp9Z5Dzixw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

On Mon, 2016-10-24 at 00:09 +0200, Linus Walleij wrote:
> On Fri, Oct 21, 2016 at 2:33 AM, Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org> wrote:
> 
> > 
> > > 
> > > Patch applied for v4.10.
> > > (Tell me if I'm applying patches in wrong order or something, and
> > > I hope this doesn't clash with the fixes.)
> > Both this patch and 8/8 functionally depend on 5/8. I fetched the
> > pinctrl tree to poke around but this patch didn't appear in any of the
> > updated branches, so I'm not sure whether we have the right ordering.
> > Without it we should hit build failures from missing macro definitions.
> > 
> > Have you had a chance to look over patch 5/8? Joel wasn't keen on its
> > current form, so I would appreciate your input.
> Oops backed this patch out.
> 
> Will look at 5/8.
> 
> Appreciate if you repost the remaining patches in the series based on
> v4.9-rc2 once it's out, and I'll rebase the pinctrl tree onto that.
> 

Will do!

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH] gpio: mcp23s08: Add option to configure pullups.
From: Linus Walleij @ 2016-10-24  0:53 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-gpio@vger.kernel.org, Rob Herring
In-Reply-To: <1477062035-30825-1-git-send-email-enric.balletbo@collabora.com>

On Fri, Oct 21, 2016 at 5:00 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:

> Default is without pullups, but if property is specified in DT and the bit
> is set, set a pullup on GPIO-n.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

I don't generally like this.

In device tree it is the consumer that specifies how the line is used,
not the producer (gpiochip).

We currently only specify polarity, open drain and open source
for consumers though.

And the entire pin control system came into being *exactly* because
Grant didn't like me adding these things to the GPIO drivers.

So how many other things does the MCP support? Drive strength?
Schmitt trigger? Is there a datasheet?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [RFC PATCH 01/13] pinctrl: meson: Add GXL pinctrl definitions
From: Linus Walleij @ 2016-10-24  1:03 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Kevin Hilman, Carlo Caione,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	open list:ARM/Amlogic Meson...,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1477060838-14164-2-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

On Fri, Oct 21, 2016 at 4:40 PM, Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:

> Add support for the Amlogic Meson GXL SoC, this is a partially complete
> definition only based on the Amlogic Vendor tree.
>
> This definition differs a lot from the GXBB and needs a separate entry.
>
> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Looks good to me. Tell me when I may apply it, it looks orthogonal
to the rest of the patches.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH v2 2/6] arm64: dts: lg1312: DT fix s/#interrupts-cells/#interrupt-cells/
From: Chanho Min @ 2016-10-24  1:59 UTC (permalink / raw)
  To: 'Geert Uytterhoeven', 'Rob Herring',
	'Mark Rutland', 'Liam Girdwood',
	'Mark Brown', 'Patrice Chotard',
	'Michael Ellerman'
  Cc: devicetree, alsa-devel, linux-arm-kernel, kernel, linuxppc-dev,
	linux-kernel, 이건호
In-Reply-To: <1477039877-20227-3-git-send-email-geert+renesas@glider.be>

> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> v2:
>   - Add Acked-by,
>   - Rebased.
> ---

Acked-by: Chanho Min <chanho.min@lge.com>

^ permalink raw reply

* RE: [PATCH v2 3/6] arm64: dts: lg1313: DT fix s/#interrupts-cells/#interrupt-cells/
From: Chanho Min @ 2016-10-24  2:00 UTC (permalink / raw)
  To: 'Geert Uytterhoeven', 'Rob Herring',
	'Mark Rutland', 'Liam Girdwood',
	'Mark Brown', 'Patrice Chotard',
	'Michael Ellerman'
  Cc: devicetree, alsa-devel, linux-arm-kernel, kernel, linuxppc-dev,
	linux-kernel, 이건호
In-Reply-To: <1477039877-20227-4-git-send-email-geert+renesas@glider.be>

> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - New.
> ---

Acked-by: Chanho Min <chanho.min@lge.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox