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:

quoted

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@gmail.com>
---

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...

quoted

+				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 <redacted>

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