diff mbox series

[RFC,v2,3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors

Message ID 20230926-pxa-gpio-v2-3-984464d165dd@skole.hr
State New
Headers
Series ARM: pxa: GPIO descriptor conversions |

Commit Message

Duje Mihanović Sept. 26, 2023, 3:46 p.m. UTC 
  Sharp's Spitz board still uses the legacy GPIO interface for controlling
the power supply to its CF and SD card slots.

Convert it to use the GPIO descriptor interface.

Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
 arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

andy@kernel.org Sept. 26, 2023, 4:15 p.m. UTC | #1 
On Tue, Sep 26, 2023 at 05:46:24PM +0200, Duje Mihanović wrote:
> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> the power supply to its CF and SD card slots.
> 
> Convert it to use the GPIO descriptor interface.

...

> +	cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
> +	if (IS_ERR(cf_power)) {
> +		dev_err(&pxa_device_mci.dev,
> +				"failed to get power control GPIO with %ld\n",
> +				PTR_ERR(cf_power));
> +		return;
> +	}

> +	gpiod_put(cf_power);

Don't you want to use guarded gpiod_get()?
Okay, it seems not yet in the pending list, but we can survive without that.
Linus Walleij Sept. 26, 2023, 8:26 p.m. UTC | #2 
On Tue, Sep 26, 2023 at 5:46 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:

> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> the power supply to its CF and SD card slots.
>
> Convert it to use the GPIO descriptor interface.
>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Duje Mihanović Sept. 27, 2023, 12:56 p.m. UTC | #3 
On Tuesday, September 26, 2023 6:15:37 PM CEST Andy Shevchenko wrote:
> On Tue, Sep 26, 2023 at 05:46:24PM +0200, Duje Mihanović wrote:
> > Sharp's Spitz board still uses the legacy GPIO interface for controlling
> > the power supply to its CF and SD card slots.
> > 
> > Convert it to use the GPIO descriptor interface.
> 
> ...
> 
> > +	cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
> > +	if (IS_ERR(cf_power)) {
> > +		dev_err(&pxa_device_mci.dev,
> > +				"failed to get power control GPIO with 
%ld\n",
> > +				PTR_ERR(cf_power));
> > +		return;
> > +	}
> > 
> > +	gpiod_put(cf_power);
> 
> Don't you want to use guarded gpiod_get()?
> Okay, it seems not yet in the pending list, but we can survive without that.

Can you please elaborate? If I understand correctly, the if statement right 
after gpiod_get is a guard.

Regards,
Duje
andy@kernel.org Sept. 27, 2023, 1:46 p.m. UTC | #4 
On Wed, Sep 27, 2023 at 02:56:07PM +0200, Duje Mihanović wrote:
> On Tuesday, September 26, 2023 6:15:37 PM CEST Andy Shevchenko wrote:
> > On Tue, Sep 26, 2023 at 05:46:24PM +0200, Duje Mihanović wrote:

...

> > > +	cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
> > > +	if (IS_ERR(cf_power)) {
> > > +		dev_err(&pxa_device_mci.dev,
> > > +				"failed to get power control GPIO with 
> %ld\n",
> > > +				PTR_ERR(cf_power));
> > > +		return;
> > > +	}
> > > 
> > > +	gpiod_put(cf_power);
> > 
> > Don't you want to use guarded gpiod_get()?
> > Okay, it seems not yet in the pending list, but we can survive without that.
> 
> Can you please elaborate? If I understand correctly, the if statement right 
> after gpiod_get is a guard.

It's about RAII version of the gpiod_get(). It's quite a new thing
in the Linux kernel.
diff mbox series

Patch

diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 91c4b208973c..616305978727 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -133,6 +133,10 @@  static unsigned long spitz_pin_config[] __initdata = {
  * Scoop GPIO expander
  ******************************************************************************/
 #if defined(CONFIG_SHARP_SCOOP) || defined(CONFIG_SHARP_SCOOP_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_card_pwr_ctrl_gpio_table, "pxa2xx-mci.0",
+		"sharp-scoop", SPITZ_GPIO_CF_POWER, "cf_power",
+		GPIO_ACTIVE_HIGH);
+
 /* SCOOP Device #1 */
 static struct resource spitz_scoop_1_resources[] = {
 	[0] = {
@@ -190,6 +194,7 @@  struct platform_device spitz_scoop_2_device = {
 static void __init spitz_scoop_init(void)
 {
 	platform_device_register(&spitz_scoop_1_device);
+	gpiod_add_lookup_table(&spitz_card_pwr_ctrl_gpio_table);
 
 	/* Akita doesn't have the second SCOOP chip */
 	if (!machine_is_akita())
@@ -201,9 +206,18 @@  static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
 {
 	unsigned short cpr;
 	unsigned long flags;
+	struct gpio_desc *cf_power;
+
+	cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
+	if (IS_ERR(cf_power)) {
+		dev_err(&pxa_device_mci.dev,
+				"failed to get power control GPIO with %ld\n",
+				PTR_ERR(cf_power));
+		return;
+	}
 
 	if (new_cpr & 0x7) {
-		gpio_set_value(SPITZ_GPIO_CF_POWER, 1);
+		gpiod_direction_output(cf_power, 1);
 		mdelay(5);
 	}
 
@@ -222,8 +236,10 @@  static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
 
 	if (!(cpr & 0x7)) {
 		mdelay(1);
-		gpio_set_value(SPITZ_GPIO_CF_POWER, 0);
+		gpiod_direction_output(cf_power, 0);
 	}
+
+	gpiod_put(cf_power);
 }
 
 #else