Re: [PATCH V1 2/3] Documentation: gpio: Add details of open-drain configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 14 February 2012 03:36 AM, Mark Brown wrote:
* PGP Signed by an unknown key

Implicitly, the gpio api already supports open-drain and several drivers
make use of it.  Instead of being a separate flag; users who need open
drain will set the pin to input.  For example, see the i2c-gpio driver.
I'm not convinced this is needed; but my opinion could be swayed.
The actual idea is to provide support for doing the switch to input to
drivers that just want to set a logic level and don't (at their level)
care one way or another if it's a CMOS or open drain output but instead
leaves it up to board configuration which mode is used.  Laxman posted a
patch for doing this to a regulator driver but looking at it the code
for emulating open drain while also maintaining support for regular CMOS
is fiddly enough that it seemed like it should be factored out of the
individual drivers.


Implementing open-drain handling in every gpio client (like i2c-gpio/regulator/fixed.c) is just duplicating the code everywhere. Also it leaves to have such implementation buggy which is in following piece of code (taken from i2c-gpio.c)

        if (pdata->sda_is_open_drain) {
                gpio_direction_output(pdata->sda_pin, 1);
                bit_data->setsda = i2c_gpio_setsda_val;
        } else {
                gpio_direction_input(pdata->sda_pin);
                bit_data->setsda = i2c_gpio_setsda_dir;
        }

Header says
 * @sda_is_open_drain: SDA is configured as open drain, i.e. the pin
 *      isn't actively driven high when setting the output value high.
 *      gpio_get_value() must return the actual pin state even if the
 *      pin is configured as an output.

And hence the check should be
if (!pdata->sda_is_open_drain) {
:::::
}


All these can be avoided by supporting open-drain flag and handling in gpio library.

Also, you should include a patch to make i2c-gpio.c use this new
functionality as a proof-of-concept.  You may not be able to test it,
but it will make it a lot easier to justify merging by showing how it
cleans up a gpio user.

I can do the cleanup in i2c-gpio also which will reduce lots of code if this change in gpiolib is acceptable and fine with everyone. Let me know if patch regulator/fixed.c V1 is sufficient or not to justify. I will create the patch for i2c-gpio also.


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


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux