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. > > > > >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. > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html