On 11/7/19 10:09 AM, Linus Walleij wrote:
On Tue, Nov 5, 2019 at 4:14 PM Amelie DELAUNAY <amelie.delaunay@xxxxxx> wrote:
On 11/5/19 3:32 PM, Linus Walleij wrote:
On Mon, Nov 4, 2019 at 11:09 AM Amelie Delaunay <amelie.delaunay@xxxxxx>
wrote:
With stmfx_pinctrl_gpio_init_valid_mask callback, gpio_valid_mask was used
to initialize gpiochip valid_mask for gpiolib. But gpio_valid_mask was not
yet initialized. gpio_valid_mask required gpio-ranges to be registered,
this is the case after gpiochip_add_data call. But init_valid_mask
callback is also called under gpiochip_add_data. gpio_valid_mask
initialization cannot be moved before gpiochip_add_data because
gpio-ranges are not registered.
Sorry but this doesn't add up, look at this call graph:
gpiochip_add_data()
gpiochip_add_data_with_key()
gpiochip_alloc_valid_mask()
of_gpiochip_add()
of_gpiochip_add_pin_range()
gpiochip_init_valid_mask()
So the .initi_valid_mask() is clearly called *after*
of_gpiochip_add_pin_range() so this cannot be the real reason,
provided that the ranges come from the device tree. AFAICT that
is the case with the stmfx.
Can you check and see if the problem is something else?
stmfx_pinctrl_gpio_init_valid_mask uses pctl->gpio_valid_mask to
initialize gpiochip valid_mask.
pctl->gpio_valid_mask is initialized in
stmfx_pinctrl_gpio_function_enable depending on gpio ranges.
stmfx_pinctrl_gpio_function_enable is called after gpiochip_add_data
because it requires gpio ranges to be registered.
So, in stmfx driver the call graph is
stmfx_pinctrl_probe
gpiochip_add_data()
gpiochip_add_data_with_key()
gpiochip_alloc_valid_mask()
of_gpiochip_add()
of_gpiochip_add_pin_range()
gpiochip_init_valid_mask()
stmfx_pinctrl_gpio_init_valid_mask (but pctl->gpio_valid_mask
is not yet initialized so gpiochip valid_mask is wrong)
stmfx_pinctrl_gpio_function_enable (pctl->gpio_valid_mask is going to
be initialized thanks to gpio ranges)
When consumer tries to take a pin (it is the case for the joystick on
stm32mp157c-ev1), it gets the following issue:
[ 3.347391] irq: :soc:i2c@40013000:stmfx@42:stmfx-pin-controller
didn't like hwirq-0x0 to VIRQ92 mapping (rc=-6)
[ 3.356418] irq: :soc:i2c@40013000:stmfx@42:stmfx-pin-controller
didn't like hwirq-0x1 to VIRQ92 mapping (rc=-6)
[ 3.366512] irq: :soc:i2c@40013000:stmfx@42:stmfx-pin-controller
didn't like hwirq-0x2 to VIRQ92 mapping (rc=-6)
[ 3.376671] irq: :soc:i2c@40013000:stmfx@42:stmfx-pin-controller
didn't like hwirq-0x3 to VIRQ92 mapping (rc=-6)
[ 3.387169] irq: :soc:i2c@40013000:stmfx@42:stmfx-pin-controller
didn't like hwirq-0x4 to VIRQ92 mapping (rc=-6)
[ 3.397065] gpio-keys joystick: Found button without gpio or irq
[ 3.403041] gpio-keys: probe of joystick failed with error -22
I can reword the commit message to make it clearer.
No need I understand it now, thanks for explaining!
We need to populate the valid mask some other way if you
want to safeguard this, I don't know if the existing
gpio-reserved-ranges would work? But it feels a bit unsafe
if you actually determine this some other way.
Before this patch, I made a "draft" version using the
gpio-reserved-ranges property but then I had to use
gpiochip_line_is_valid in pinconf_get/_set/_dbg_show in addition of
pinctrl_find_gpio_range_from_pin... With an update of the bindings for
optional property gpio-reserved-ranges.
I was not really fond of this solution, it sounded redundant.
Thanks for applying the patch.
Regards,
Amelie
