Re: [PATCH v2] iio: dac: Add support for the AD5592R/AD5593R ADCs/DACs

[Michael Hennerich <michael.hennerich@analog.com>]

On 03/05/2016 06:58 PM, Jonathan Cameron wrote:
> On 02/03/16 16:21, Michael Hennerich wrote:
>> On 02/27/2016 06:50 PM, Jonathan Cameron wrote:
>>> On 25/02/16 13:36, michael.hennerich@analog.com wrote:
>>>> From: Paul Cercueil <paul.cercueil@analog.com>
>>>>
>>>> This patch adds support for the AD5592R (spi) and AD5593R (i2c)
>>>> ADC/DAC devices.
>>>>
>>>> Signed-off-by: Paul Cercueil <paul.cercueil@analog.com>
>>>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>>>>
>>> A few bits inline.
>>>
>>> I'll need a gpio review on this (looks fine to me but it does contain
>>> a gpiochip driver.) Not to mention the question of whether they will
>>> be happy with a gpio chip hiding in iio (rather than via an mfd with a
>>> separate driver - which feels like overkill here).
>>>
>>> The big question to my mind is whether we can take the view this won't
>>> be the last multipurpose chip we will see so do we need to sort the
>>> binding out to make it generic?  It'll be a bit of a pain for you
>>> but I think we can do it fairly easily.
>>> (either way I'll also need a device tree ack on this one!)
>>>
>>> So then we get into the question of the best way of doing the bindings.
>>> The gpio approach seems a little limiting for things as flexible as
>>> this but we should certainly be using their macros where relevant.
>>
>> Hi Jonathan,
>>
>> Thanks for the review.
>>
>> The problem is see is that using GPIOF_OPEN_DRAIN, will simulate OPEN DRAIN behaviour only, by configuring the device for input when outputing logic high.
>>
> Is it actually documented as such anywhere? (that's hideous!)


In fact it is -
www.kernel.org/doc/Documentation/gpio/gpio-legacy.txt

"When setting the flag as GPIOF_OPEN_DRAIN then it will assume that pins is
open drain type. Such pins will not be driven to 1 in output mode. It is
require to connect pull-up on such pins. By enabling this flag, gpio lib 
will
make the direction to input when it is asked to set value of 1 in output 
mode
to make the pin HIGH. The pin is make to LOW by driving value 0 in 
output mode."

And that's exactly what the gpiolib code does.

-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif