On Mon, Feb 06, 2023 at 11:35:32AM +0000, Daniel Thompson wrote:
> On Tue, Jan 31, 2023 at 02:57:06PM -0800, Dmitry Torokhov wrote:
> > Switch the driver from legacy gpio API that is deprecated to the newer
> > gpiod API that respects line polarities described in ACPI/DT.
> >
> > This makes driver use standard property name for the reset gpio
> > ("reset-gpios" vs "gpios-reset"), however there is a quirk in gpiolib
> > to also recognize the legacy name and keep compatibility with older
> > DTSes.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > ---
> >
> > All preparation gpiolib work to handle legacy names and polarity quirks
> > has landed in mainline...
> >
> > drivers/video/backlight/hx8357.c | 82 ++++++++++++++------------------
> > 1 file changed, 37 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
> > index 9b50bc96e00f..a93e14adb846 100644
> > --- a/drivers/video/backlight/hx8357.c
> > +++ b/drivers/video/backlight/hx8357.c
> > [snip]
> > - if (of_find_property(spi->dev.of_node, "im-gpios", NULL)) {
> > - lcd->use_im_pins = 1;
> > -
> > - for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
> > - lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
> > - "im-gpios", i);
> > - if (lcd->im_pins[i] == -EPROBE_DEFER) {
> > - dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
> > - return -EPROBE_DEFER;
> > - }
> > - if (!gpio_is_valid(lcd->im_pins[i])) {
> > - dev_err(&spi->dev, "Missing dt property: im-gpios\n");
> > - return -EINVAL;
> > + gpiod_set_consumer_name(lcd->reset, "hx8357-reset");
> > +
> > + for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
> > + lcd->im_pins[i] = devm_gpiod_get_index(&spi->dev,
> > + "im", i, GPIOD_OUT_LOW);
> > + ret = PTR_ERR_OR_ZERO(lcd->im_pins[i]);
> > + if (ret) {
> > + if (ret == -ENOENT) {
> > + if (i == 0)
> > + break;
> > + dev_err(&spi->dev, "Missing im gpios[%d]\n", i);
> > + ret = -EINVAL;
> > + } if (ret == -EPROBE_DEFER) {
I see I miss "else" here...
> > + dev_info(&spi->dev, "im gpio[%d] is not here yet, deferring the probe\n",
> > + i);
> > + } else {
> > + dev_err(&spi->dev, "failed to request im gpio[%d]: %d\n",
> > + i, ret);
> > }
>
> These last two clauses should be updated to return dev_err_probe(...)
> instead.
>
> With that change:
> Reviewed-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
So you want to actually suppress the deferral message unless debug
printks are enabled? So you want this to read:
if (ret) {
if (ret == -ENOENT) {
if (i == 0)
break;
dev_err(&spi->dev, "Missing im gpios[%d]\n", i);
return -EINVAL;
}
return dev_err_probe(&spi->dev, ret,
"failed to request im gpio[%d]\n", i);
}
Did I get it right?
Thanks.
--
Dmitry
