From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Fenglin Wu <fenglinw@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
Linus Walleij <linus.walleij@linaro.org>,
linux-gpio@vger.kernel.org, collinsd@codeaurora.org,
aghayal@codeaurora.org, wruan@codeaurora.org,
subbaram@codeaurora.org, kgunda@codeaurora.org
Subject: Re: [PATCH V1] pinctrl: qcom: spmi-gpio: Update GPIO EN_CTL when setting pin config
Date: Tue, 10 Oct 2017 22:48:52 -0700 [thread overview]
Message-ID: <20171011054852.GH1165@minitux> (raw)
In-Reply-To: <35aec5d7-8808-9930-8d6d-e0399df12a9e@codeaurora.org>
On Mon 09 Oct 17:17 PDT 2017, Fenglin Wu wrote:
> On 10/9/2017 1:56 PM, Bjorn Andersson wrote:
> > On Sun 08 Oct 22:34 PDT 2017, Fenglin Wu wrote:
> >
> > > On 10/6/2017 12:27 AM, Bjorn Andersson wrote:
[..]
> > > > But I spotted another issue while reviewing this; currently the initial
> > > > state of is_enabled is unconditionally set to enabled in
> > > > pmic_gpio_populate(), so reading the initial pinconf or configuring a
> > > > pinmux before setting a pinconf will operate on the potentially wrong
> > > > information.
> > > >
> > > > So I think the initial value should be read out from REG_EN_CTL rather
> > > > than being just "true".
> > > >
> > > > Can you please either submit another patch for this?
> > >
> > > Hmm, considering a GPIO which is disabled by default in hardware
> > > setting, what's its expected state if we only define "function" for it?
> > > I was thinking we need to enable it once it has any setting in pinmux or
> > > pinconf. If you think that we need to keep its original state until we
> > > set pinconf for it, yes, I can submit a change to address this.
> > >
> >
> > Are there valid cases where only function should be selected and no
> > other configuration is used? If so it makes sense to make
> > pmic_gpio_set_mux() enable the block.
> >
> >
> > Regardless of this, if there are disabled pins that are not mentioned in
> > DT they will still appear as enabled in the debugfs interface; and this
> > I consider an error worth fixing.
> How about we do both: read the HW initial state in pmic_gpio_populate(),
> and also enable the GPIO block in pmic_gpio_set_mux()?
>
That sounds good.
Please do this as two separate patches, with the commit message clearly
describing a case where the pinconf does not affect the function of the
pin, so a pinmux is the only thing needed.
Regards,
Bjorn
next prev parent reply other threads:[~2017-10-11 5:48 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-12 0:32 [PATCH V1] pinctrl: qcom: spmi-gpio: Update GPIO EN_CTL when setting pin config fenglinw
2017-09-19 8:20 ` Linus Walleij
2017-10-05 16:27 ` Bjorn Andersson
2017-10-09 5:34 ` Fenglin Wu
2017-10-09 5:56 ` Bjorn Andersson
2017-10-10 0:17 ` Fenglin Wu
2017-10-11 5:48 ` Bjorn Andersson [this message]
2017-10-11 8:24 ` Linus Walleij
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=20171011054852.GH1165@minitux \
--to=bjorn.andersson@linaro.org \
--cc=aghayal@codeaurora.org \
--cc=collinsd@codeaurora.org \
--cc=fenglinw@codeaurora.org \
--cc=kgunda@codeaurora.org \
--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=subbaram@codeaurora.org \
--cc=wruan@codeaurora.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).