All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@codeaurora.org>
To: Stanimir Varbanov <svarbanov@mm-sol.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org,
	Bjorn Andersson <bjorn.andersson@sonymobile.com>
Subject: Re: [PATCH] pinctrl: qcom: enable generic support and input-enable pinctrl conf
Date: Wed, 28 Jan 2015 12:48:30 -0800	[thread overview]
Message-ID: <54C94B1E.7050304@codeaurora.org> (raw)
In-Reply-To: <54C7928F.9090407@mm-sol.com>

On 01/27/15 05:28, Stanimir Varbanov wrote:
> Hi Stephen,
>
> Thanks for the comments!
>
> On 01/27/2015 03:18 AM, Stephen Boyd wrote:
>> On 01/26/15 08:24, Stanimir Varbanov wrote:
>>
>>>  	return -ENOTSUPP;
>>>  }
>>>  
>>> @@ -276,9 +274,11 @@ static int msm_config_group_get(struct pinctrl_dev *pctldev,
>>>  		val = readl(pctrl->regs + g->io_reg);
>>>  		arg = !!(val & BIT(g->in_bit));
>>>  		break;
>>> +	case PIN_CONFIG_INPUT_ENABLE:
>>> +		val = readl(pctrl->regs + g->io_reg);
>>> +		arg = !!(val & BIT(g->in_bit));
>>> +		break;
>> This bit is used to read the value of the gpio. If the gpio is high,
>> this bit will read back as a 1. If the gpio is low, this bit will read
>> back as a 0. I don't see how this is related to input-enable? Doesn't
> Maybe I had to split on few patches. The change in .pin_config_group_get
> is related to debugfs dump. The change which adds input-enable support
> is in .pin_config_group_set.
>
>> input-enable mean we configure the pin to accept input? In that sense,
>> configuring the pin to accept input would sort of be like configuring
>> the pin for function "gpio" so that we can read this bit and see if the
>> pin is high or low, but I don't know if we care to support that. I think
>> we rely on users configuring the pin for the gpio function though.
> Why you do not care, because you think that tlmm doesn't support it or
> because it is a driver responsibility to set up gpio function plus
> input/output configuration?

Ah ok, I see that the driver clears the output enable bit for
gpio_direction_input(). Technically that isn't even required because
from what I can recall the output enable bit only controls if the OE bit
is driven out on the line. We can always read what's there by reading
the IN bit even when the output is enabled.

>
> I can imagine usecase where the driver implements an 'idle' pinctrl
> state which configure its pins to be gpio input enabled to save power
> assuming that the previous 'default' pinctrl state it was gpio output.
> In that case the driver doesn't need to call gpio_direction_input() for
> every gpio, only call pinctrl_select_state('idle').
>
> What's wrong with this?

Yes that's fine. It sounds like you want to disabled the output and the
way that works today is by switching the gpio direction to input. I just
got caught up because in my mind the input is always enabled because we
can always read what's on the pin. I guess output-disable ==
input-enable. Either way, the implementation doesn't look right for this
part where we want to see if the input is enabled. Looks like Bjorn has
the right idea, so let's see how v2 goes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

      parent reply	other threads:[~2015-01-28 20:48 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-26 16:24 [PATCH] pinctrl: qcom: enable generic support and input-enable pinctrl conf Stanimir Varbanov
2015-01-27  1:18 ` Stephen Boyd
2015-01-27  1:23   ` Stephen Boyd
2015-01-27 13:28     ` Stanimir Varbanov
2015-01-27 17:44     ` Bjorn
2015-01-27 13:28   ` Stanimir Varbanov
2015-01-27 18:05     ` Bjorn
2015-01-28 20:48     ` Stephen Boyd [this message]

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=54C94B1E.7050304@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=bjorn.andersson@sonymobile.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=svarbanov@mm-sol.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.