On 01.08.19 11:55, Uwe Kleine-König wrote: > On Thu, Aug 01, 2019 at 09:28:33AM +0000, Schrempf Frieder wrote: >> Hi Uwe, >> >> On 01.08.19 10:48, Uwe Kleine-König wrote: >>> On Thu, Aug 01, 2019 at 08:18:05AM +0000, Schrempf Frieder wrote: >>>> From: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx> >>>> >>>> If CONFIG_GPIOLIB is not enabled, mctrl_gpio_init() will return >>>> -ENOSYS and cause the probing of the imx UART to fail. As the >>>> GPIOs are optional, we should continue probing in this case. >>> >>> Is this really still the case? On which version did you hit this >>> problem? >> >> Yes, I think it is. I used v5.2.5, that already has d99482673f95. >> >>> >>> I would expect that is gone with >>> d99482673f950817b30caf3fcdfb31179b050ce1 if not earlier. >> >> I think this is a different problem. If CONFIG_GPIOLIB is disabled, >> mctrl_gpio_init() returns -ENOSYS unconditionally here: [1]. The >> existing patch (d99482673f95) seems to handle the case when >> CONFIG_GPIOLIB is enabled, but no or not all GPIOs are given in the dtb. > > Ah, I see. > > I don't think we should handle this on a per-driver basis. So my > suggestion is to drop the dummy implementation for mctrl_gpio if GPIOLIB > is disabled. Then the behaviour should be consistant with the gpio stuff > returning NULL in this case. (Or alternatively adapt the dummy > implementation to shortcut and behave identically.) I get your point, but it seems a bit strange to go down all the way from the driver to mctrl_gpio_init_noauto() and into the loop just to return an empty struct mctrl_gpios to the driver that will be looped over again on each call of mctrl_gpio_*(). So I would rather go with a variation of your second proposal and keep the dummy implementation, but let it return NULL instead of an error pointer, as all the mctrl_gpio_*() functions already seem to have a check for gpios == NULL. What do you think? > > (Having said that I don't like gpiolib's behaviour of returning NULL for > the optional calls if it's disabled, but having mctrl_gpio behave > differently is worse.) > >> The sh-sci.c driver has a similar check to skip this case: [2]. > > This should than be dropped, too. > > Best regards > Uwe > >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/serial_mctrl_gpio.h#n121 >> [2] >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/sh-sci.c#n3290 >
