On 10/04, Timur Tabi wrote:

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

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.

Yes it's the same sort of design with the hardware I have too.

quoted

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.

Ok.

quoted

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?

I have no idea. To describe non-contiguous pin ranges? Linus?

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.

Agreed.

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.

Yes, thinking more about it I don't want an available callback
either. It will add burden on DT platforms where we have to
describe per-firmware pin ranges just because gpiolib is reading
the direction of gpios we don't use.

Instead, I'd prefer we delay reading the direction until a
consumer requests the gpio, this way we don't touch the hardware
unless a consumer wants to. That seems simpler and doesn't
require anything special from the driver.

Don't get me wrong, I'm willing to describe with DT/ACPI which
pins are available if we have a need for it, but so far I don't
see the requirement and I'm a lazy person so I like avoiding more
work.

Does my patch fail on your platform for some reason? I can only
guess that somewhere we don't request the gpio before using it
and then you don't see the proper direction.

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