All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laxman Dewangan <ldewangan@nvidia.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Grant Likely <grant.likely@secretlab.ca>,
	"linus.walleij@stericsson.com" <linus.walleij@stericsson.com>,
	"dunlap@xenotime.net" <dunlap@xenotime.net>,
	"lrg@ti.com" <lrg@ti.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH V1 2/3] Documentation: gpio: Add details of open-drain configuration
Date: Tue, 14 Feb 2012 14:29:32 +0530	[thread overview]
Message-ID: <4F3A2274.5080000@nvidia.com> (raw)
In-Reply-To: <20120213220656.GA11931@opensource.wolfsonmicro.com>

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.



  reply	other threads:[~2012-02-14  8:59 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-13  6:29 [PATCH V1 0/3] Support for open drain gpios in gpilib/fixed regulators Laxman Dewangan
2012-02-13  6:29 ` Laxman Dewangan
2012-02-13  6:29 ` [PATCH V1 1/3] gpio: gpiolib: Support for open drain gpios Laxman Dewangan
2012-02-13  6:29 ` [PATCH V1 2/3] Documentation: gpio: Add details of open-drain configuration Laxman Dewangan
2012-02-13 21:18   ` Grant Likely
2012-02-13 22:06     ` Mark Brown
2012-02-14  8:59       ` Laxman Dewangan [this message]
     [not found]     ` <20120213211809.GI11077-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2012-02-14  9:16       ` Laxman Dewangan
2012-02-14  9:16         ` Laxman Dewangan
2012-02-15 22:25     ` Linus Walleij
2012-02-16  8:28       ` Laxman Dewangan
2012-02-16 20:00         ` Linus Walleij
     [not found]           ` <CACRpkdaFyxU-gq5ETnNc4LonTo98PbUs09CmvsPUqzyt0Mg2Cg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-17 10:30             ` Laxman Dewangan
2012-02-17 10:30               ` Laxman Dewangan
     [not found] ` <1329114588-15430-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-02-13  6:29   ` [PATCH V1 3/3] regulator: fixed: Support for open-drain gpio Laxman Dewangan
2012-02-13  6:29     ` Laxman Dewangan
2012-02-13 20:02   ` [PATCH V1 0/3] Support for open drain gpios in gpilib/fixed regulators Linus Walleij
2012-02-13 20:02     ` Linus Walleij
2012-02-13 22:10     ` Mark Brown

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=4F3A2274.5080000@nvidia.com \
    --to=ldewangan@nvidia.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=dunlap@xenotime.net \
    --cc=grant.likely@secretlab.ca \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=lrg@ti.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.