[RFC,v2,3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors
Commit Message
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
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.
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
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
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.
@@ -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