[BUG] keyboard doesn't work on Olimex teres-I notebook
[ Please CC to me, I'm not subscribed to mailing list ]
Hi,
On u-boot release 2022.04 keyboard doesn't work in u-boot on Olimex TERES-I notebook.
With git bisect I found that commit 35ae126c16a6a9149edc6638faaa247f67b8a400 is introduced this. Reverting this commit solved problem and in my test keyboard now works.
On Tue, 19 Apr 2022 14:27:24 +0200 Milan P. Stanić mps@arvanta.net wrote:
Hi Milan,
[ Please CC to me, I'm not subscribed to mailing list ]
Hi,
On u-boot release 2022.04 keyboard doesn't work in u-boot on Olimex TERES-I notebook.
Thanks for the report, and sorry for the delay!
With git bisect I found that commit 35ae126c16a6a9149edc6638faaa247f67b8a400 is introduced this. Reverting this commit solved problem and in my test keyboard now works.
Unfortunately just reverting this commit is not really the way forward, but I had a closer look and few printf's later found the problem: The USB PHY driver uses the legacy GPIO interface, which is now wrapped in gpio-uclass.c, to call the actual DM functions. The commit that you bisected to implements the recommended .set_flags ops member, which relies on the GPIO being properly configured before. Unfortunately those compat wrappers seem to be broken, and not using .set_flags just papered over this problem before: - A call to gpio_direction_output() fills the DM struct gpio_desc *locally*, on the stack, so any setup is lost when the function returns. The GPIO hardware is still configured as an output, but U-Boot doesn't remember this. - A call to gpio_direction_output() in the legacy interface is supposed to also set the output value, but the compat wrapper does not implement this. - When .set_flags is implemented in a GPIO driver implementation, the DM GPIO interface *requires* GPIOs to be configured before, but since this information is lot, a call to gpio_set_value() does nothing.
So while those issues looks like they demand fixing, in the generic compat GPIO wrappers, I decided to upgrade the sunxi USB PHY driver to just use the DM GPIO interface directly. Eventually this driver should drop the U-Boot configured GPIOs, and use proper DT methods, but this is a bigger change I'd rather not introduce that late in the development cycle. So for now I sent this patch, that should fix the issue: https://lore.kernel.org/u-boot/20220608000604.3357-1-andre.przywara@arm.com/
If you could please test this patch, then reply to that patch email, for instance with a Tested-by:, this would be greatly appreciated!
Cheers, Andre
On Wed, 8 Jun 2022 01:30:42 +0100 Andre Przywara andre.przywara@arm.com wrote:
Hi,
On Tue, 19 Apr 2022 14:27:24 +0200 Milan P. Stanić mps@arvanta.net wrote:
Hi Milan,
[ Please CC to me, I'm not subscribed to mailing list ]
Hi,
On u-boot release 2022.04 keyboard doesn't work in u-boot on Olimex TERES-I notebook.
Thanks for the report, and sorry for the delay!
With git bisect I found that commit 35ae126c16a6a9149edc6638faaa247f67b8a400 is introduced this. Reverting this commit solved problem and in my test keyboard now works.
Unfortunately just reverting this commit is not really the way forward, but I had a closer look and few printf's later found the problem: The USB PHY driver uses the legacy GPIO interface, which is now wrapped in gpio-uclass.c, to call the actual DM functions. The commit that you bisected to implements the recommended .set_flags ops member, which relies on the GPIO being properly configured before. Unfortunately those compat wrappers seem to be broken, and not using .set_flags just papered over this problem before:
- A call to gpio_direction_output() fills the DM struct gpio_desc *locally*, on the stack, so any setup is lost when the function returns. The GPIO hardware is still configured as an output, but U-Boot doesn't remember this.
- A call to gpio_direction_output() in the legacy interface is supposed to also set the output value, but the compat wrapper does not implement this.
Scratch that part: that works, as the h/w routines are called inside dm_gpio_clrset_flags(), where we set the correct flags in our (temporary) struct gpio_desc, before calling the .set_flags implementation. Got tricked by the rather innocent function name.
But still calling gpio_direction_output(), then later gpio_set_value() is broken. I guess we should fix it, maybe by forcing GPIOD_IS_OUT in the desc, before calling dm_gpio_set_value()?
Cheers, Andre
- When .set_flags is implemented in a GPIO driver implementation, the DM GPIO interface *requires* GPIOs to be configured before, but since this information is lot, a call to gpio_set_value() does nothing.
So while those issues looks like they demand fixing, in the generic compat GPIO wrappers, I decided to upgrade the sunxi USB PHY driver to just use the DM GPIO interface directly. Eventually this driver should drop the U-Boot configured GPIOs, and use proper DT methods, but this is a bigger change I'd rather not introduce that late in the development cycle. So for now I sent this patch, that should fix the issue: https://lore.kernel.org/u-boot/20220608000604.3357-1-andre.przywara@arm.com/
If you could please test this patch, then reply to that patch email, for instance with a Tested-by:, this would be greatly appreciated!
Cheers, Andre
participants (2)
-
Andre Przywara -
Milan P. Stanić