linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Timur Tabi <timur@codeaurora.org>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Andy Gross <andy.gross@linaro.org>,
	David Brown <david.brown@linaro.org>,
	anjiandi@codeaurora.org,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH 0/2] [v5] pinctrl: qcom: add support for sparse GPIOs
Date: Wed, 4 Oct 2017 17:41:27 -0500	[thread overview]
Message-ID: <f8c20994-230e-0028-c46a-d001e59223ce@codeaurora.org> (raw)
In-Reply-To: <20171004215023.GA457@codeaurora.org>

On 10/04/2017 04:50 PM, Stephen Boyd wrote:

>> At the time I wrote that patch, the ACPI tables exposed all of the
>> GPIOs, even the ones it didn't care about.  The new ACPI tables list
>> only specific GPIOs, and so we no longer need to blindly read the
>> direction of all GPIOs.
>>
> 
> Do you avoid this problem on new ACPI tables because only pins
> that are able to be read are exposed?

Yes.  A recent firmware update enabled the "XPU" block which is being 
programmed with a select subset of individual GPIOs.  On our silicon, 
each TLMM GPIO is in a separate 64k page, and so the XPU can block any 
individual GPIO.  Any attempt to touch those registers causes an XPU 
violation which takes the whole system down.

>>> This is basically a revert of commit 72d320006177 ("gpio: set up
>>> initial state from .get_direction()").
>>
>> I would be in favor of either reverting that patch, or moving the
>> code into gpiochip_add_pin_range().
> 
> If it's in gpiochip_add_pin_range() would we still read the
> hardware when creating the pin ranges?

I presume so.  The idea is that pinctrl-qdf2xxx/pinctrl-msm only submit 
pin ranges that are present in the ACPI tables.

 > I don't want to have to> describe pin ranges of "valid" pins that 
won't cause the system
> to blow up if we touch them, because those pins are never used by
> Linux so reading them is not useful.

Well, that's exactly what I'm trying to do with current patch set :-) 
It seems the most logical approach to me.  I don't understand the 
dislike for it.  What else are pin ranges for, other than to specify 
ranges of pins that can be accessed?

Another alternative was to enumerate all of the GPIOs starting from 0. 
So the first GPIO in ACPI would be gpio0, regardless of what gpio number 
it actually was.  E.g. GPIO 37 would appear as gpio0, GPIO 38 would 
appear as gpio1, and so on.  That also worked, but it meant that 
customers would need to figure out which GPIO that "gpio0" actually 
pointed to.  That was not acceptable, so I dropped it.

I'm at a loss on how else to do it.  I think a gpio_chip.available 
callback is far less elegant than define pin ranges.  There is no chance 
that unavailable GPIOs can be accessed because the physical addresses 
are not in the msm_pingroup array.  That is, groups[0].ctrl_reg == 0, 
not 0xFF02010000.


-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

  reply	other threads:[~2017-10-04 22:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-07 15:33 [PATCH 0/2] [v5] pinctrl: qcom: add support for sparse GPIOs Timur Tabi
2017-09-07 15:33 ` [PATCH 1/2] [v5] pinctrl: qcom: disable GPIO groups with no pins Timur Tabi
2017-10-02 17:44   ` Bjorn Andersson
2017-10-02 20:47     ` Timur Tabi
2017-10-07 11:07       ` Linus Walleij
2017-10-13 23:35         ` Timur Tabi
2017-10-19 22:44         ` Timur Tabi
2017-10-16  8:01   ` Thierry Reding
2017-10-16 13:52     ` Timur Tabi
2017-09-07 15:33 ` [PATCH 2/2] [v3] pinctrl: qcom: qdf2xxx: add support for new ACPI HID QCOM8002 Timur Tabi
2017-09-08 12:50 ` [PATCH 0/2] [v5] pinctrl: qcom: add support for sparse GPIOs Linus Walleij
2017-09-13 17:09   ` Timur Tabi
2017-09-19  7:04 ` Stephen Boyd
2017-09-19  8:15   ` Linus Walleij
2017-09-19 12:32     ` Timur Tabi
2017-09-20 11:43       ` Linus Walleij
2017-09-20 13:04         ` Timur Tabi
2017-09-21 12:08           ` Linus Walleij
2017-09-21 12:12             ` Timur Tabi
2017-09-22 13:29               ` Linus Walleij
2017-09-22 13:37                 ` Timur Tabi
2017-10-03 22:03                   ` Stephen Boyd
2017-10-03 22:12                     ` Timur Tabi
2017-10-04 21:50                       ` Stephen Boyd
2017-10-04 22:41                         ` Timur Tabi [this message]
2017-10-05 21:30                           ` Stephen Boyd
2017-10-11  7:51                     ` Linus Walleij
2017-10-12  7:39                       ` Stephen Boyd
2017-10-14 22:43                         ` Linus Walleij
2017-10-16 13:42                           ` Timur Tabi
2017-10-13 23:26                     ` Timur Tabi
2017-10-15 20:18                       ` Thierry Reding
2017-10-15 21:09                         ` Timur Tabi
2017-10-02 16:02                 ` Timur Tabi

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=f8c20994-230e-0028-c46a-d001e59223ce@codeaurora.org \
    --to=timur@codeaurora.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy.gross@linaro.org \
    --cc=anjiandi@codeaurora.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=david.brown@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=sboyd@codeaurora.org \
    --cc=thierry.reding@gmail.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 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).