Hi,
On 18-08-19 18:19, Hans de Goede wrote:
Hi,
On 18-08-19 16:04, Andy Shevchenko wrote:
On Sun, Aug 18, 2019 at 03:38:17PM +0200, Hans de Goede wrote:
On 18-08-19 15:24, Andy Shevchenko wrote:
On Fri, Aug 16, 2019 at 11:18:22AM +0200, Hans de Goede wrote:
On 12-08-19 15:53, Linus Walleij wrote:
Anyways, this will need to be fixed before we can merge this.
It might affect the behaviour of pinctrl-baytrail as well.
Hans, do you have any Baytrail at hand to test latest linux-next, or take my
for-next branch from
git://git.kernel.org/pub/scm/linux/kernel/git/pinctrl/intel.git?
Given all the hw-enablement work I've done for BYT/CHT I have a whole
stack of Bay Trail devices. Is there anything specific you want me to
test? Or should I just take the first one of the stack which uses a
GPIO from the SoC as IRQ for something and then test that something?
From the thread I got that it should be one which uses GPIO for GPE.
Given that we have a fix there against misconfigured pins by firmware [1, 2],
which utilizes need_valid_mask, probably ASUS T100TA is a good candidate.
[1]: https://www.spinics.net/lists/linux-gpio/msg18842.html
[2]: commit 49c03096263871a68c9dea3e86b7d1e163d2fba8
Thanks for the context, so for testing I need a Bay Trail device which
uses a GPIO interrupt line with a "Interrupt" descriptor, like this:
Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusiv
{
0x00000045,
}
Instead of the more modern GpioInt descriptors. My Asus T100TA is in my
storage-bin at the localhackerspace ATM, but I have a T200TA here which
is the same wrt touchscreen IRQ routing.
I'm currently charging it because its battery was very dead TM, but in
the mean time I've been looking at the code and I can already tell
that a kernel with the 2e65e0fad935f578e998656d3e7be5a26e072b0e
("pinctrl: intel: baytrail: Pass irqchip when adding gpiochip")
is not going to boot, that commit moves the call to
byt_gpio_irq_init_hw() to before the call to devm_gpiochip_add_data().
devm_gpiochip_add_data() allocs gpio_chip.irq.valid_mask and
byt_gpio_irq_init_hw() does:
if (value & BYT_DIRECT_IRQ_EN) {
clear_bit(i, gc->irq.valid_mask);
Which means it will trigger a NULL deref after the
"pinctrl: intel: baytrail: Pass irqchip when adding gpiochip"
commit, sine now byt_gpio_irq_init_hw() runs before
devm_gpiochip_add_data().
Note that the gpio_chip structure already has a init_valid_mask
callback which runs after gpiochip_irqchip_init_valid_mask wich
allocs gpio_chip.irq.valid_mask, so we might use that, but:
That is intended to setup the valid_mask for the pins, not for
the IRQs, which would mean we are abusing it a bit and it runs
after gpiochip_add_irqchip(), which might be too late I guess.
So it looks like we need a gpio_chip.irq.init_valid_mask callback
to fix this ordering problem and until this is fixed we should revert
2e65e0fad935f578e998656d3e7be5a26e072b0e.
So thinking a bit more about this, I think a better fix would be to
export gpiochip_irqchip_init_valid_mask and make it proof against
multiple calls. Then drivers which need this before
devm_gpiochip_add_data() time can simply call it in advance.
Maybe (not sure if this is a good idea) we can eventually even
drop gpio_chip.irq.need_valid_mask and simply have all users of
the valid_mask explictly call gpiochip_irqchip_init_valid_mask.
If this second bit is a good idea depends on how much users there
are I guess and what the changes to those users would look like.
Regards,
Hans
