From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756742AbdLPLkH (ORCPT ); Sat, 16 Dec 2017 06:40:07 -0500 Received: from smtp07.smtpout.orange.fr ([80.12.242.129]:35874 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752207AbdLPLkE (ORCPT ); Sat, 16 Dec 2017 06:40:04 -0500 X-ME-Helo: belgarion X-ME-Auth: amFyem1pay5yb2JlcnRAb3JhbmdlLmZy X-ME-Date: Sat, 16 Dec 2017 12:40:01 +0100 X-ME-IP: 86.201.132.185 From: Robert Jarzmik To: Daniel Thompson Cc: Lee Jones , Jingoo Han , Bartlomiej Zolnierkiewicz , linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/3] backlight: tdo24m: add the reset line gpio References: <20171013194250.9001-1-robert.jarzmik@free.fr> <20171013194250.9001-2-robert.jarzmik@free.fr> <20171215115309.ez7odo7sh5zz7vge@oak.lan> X-URL: http://belgarath.falguerolles.org/ Date: Sat, 16 Dec 2017 12:39:50 +0100 In-Reply-To: <20171215115309.ez7odo7sh5zz7vge@oak.lan> (Daniel Thompson's message of "Fri, 15 Dec 2017 11:53:09 +0000") Message-ID: <87shcaewah.fsf@belgarion.home> User-Agent: Gnus/5.130008 (Ma Gnus v0.8) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Daniel Thompson writes: > On Fri, Oct 13, 2017 at 09:42:48PM +0200, Robert Jarzmik wrote: >> The Toppoly panels have a global reset line. Add an optional gpio >> control for this line, for platforms which have the ability to drive it. >> >> Signed-off-by: Robert Jarzmik >> --- >> drivers/video/backlight/tdo24m.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/video/backlight/tdo24m.c b/drivers/video/backlight/tdo24m.c >> index e4bd63e9db6b..e4b2dfabf192 100644 >> --- a/drivers/video/backlight/tdo24m.c >> +++ b/drivers/video/backlight/tdo24m.c >> @@ -10,6 +10,7 @@ >> */ >> >> #include >> +#include >> #include >> #include >> #include >> @@ -32,6 +33,7 @@ struct tdo24m { >> struct spi_message msg; >> struct spi_transfer xfer; >> uint8_t *buf; >> + struct gpio_desc *xres; > > Why do we need a write-only variable? It's written to by devm_gpiod_get() below. >> + lcd->xres = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW); > > devm_gpiod_get_optional() would be better, if only for its commenting > value. Yes, you're perfectly right. > Also, this adds a new optional property, doesn't the devicetree binding > docs need to be update to match this? Ah yes, that too. I'll add it for v3. Cheers. -- Robert From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Jarzmik Date: Sat, 16 Dec 2017 11:39:50 +0000 Subject: Re: [PATCH v2 2/3] backlight: tdo24m: add the reset line gpio Message-Id: <87shcaewah.fsf@belgarion.home> List-Id: References: <20171013194250.9001-1-robert.jarzmik@free.fr> <20171013194250.9001-2-robert.jarzmik@free.fr> <20171215115309.ez7odo7sh5zz7vge@oak.lan> In-Reply-To: <20171215115309.ez7odo7sh5zz7vge@oak.lan> (Daniel Thompson's message of "Fri, 15 Dec 2017 11:53:09 +0000") MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Daniel Thompson Cc: Lee Jones , Jingoo Han , Bartlomiej Zolnierkiewicz , linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org Daniel Thompson writes: > On Fri, Oct 13, 2017 at 09:42:48PM +0200, Robert Jarzmik wrote: >> The Toppoly panels have a global reset line. Add an optional gpio >> control for this line, for platforms which have the ability to drive it. >> >> Signed-off-by: Robert Jarzmik >> --- >> drivers/video/backlight/tdo24m.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/video/backlight/tdo24m.c b/drivers/video/backlight/tdo24m.c >> index e4bd63e9db6b..e4b2dfabf192 100644 >> --- a/drivers/video/backlight/tdo24m.c >> +++ b/drivers/video/backlight/tdo24m.c >> @@ -10,6 +10,7 @@ >> */ >> >> #include >> +#include >> #include >> #include >> #include >> @@ -32,6 +33,7 @@ struct tdo24m { >> struct spi_message msg; >> struct spi_transfer xfer; >> uint8_t *buf; >> + struct gpio_desc *xres; > > Why do we need a write-only variable? It's written to by devm_gpiod_get() below. >> + lcd->xres = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW); > > devm_gpiod_get_optional() would be better, if only for its commenting > value. Yes, you're perfectly right. > Also, this adds a new optional property, doesn't the devicetree binding > docs need to be update to match this? Ah yes, that too. I'll add it for v3. Cheers. -- Robert