Hello,
Small series to convert some of the board files in the mach-pxa directory
to use the new GPIO descriptor interface.
Most notably, the am200epd, am300epd and Spitz matrix keypad among
others are not converted in this series.
Signed-off-by: Duje Mihanović <[email protected]>
---
Changes in v3:
- Address maintainer comments:
- Use GPIO_LOOKUP_IDX for LEDs
- Drop unnecessary NULL assignments
- Don't give up on *all* SPI devices if hsync cannot be set up
- Add Linus' Acked-by
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Address maintainer comments:
- Change mentions of function to function()
- Drop cast in OHCI driver dev_warn() call
- Use %pe in OHCI and reset drivers
- Use GPIO _optional() API in OHCI driver
- Drop unnecessary not-null check in OHCI driver
- Use pr_err() instead of printk() in reset driver
- Rebase on v6.6-rc3
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Duje Mihanović (6):
ARM: pxa: Convert Spitz OHCI to GPIO descriptors
ARM: pxa: Convert Spitz LEDs to GPIO descriptors
ARM: pxa: Convert Spitz CF power control to GPIO descriptors
ARM: pxa: Convert reset driver to GPIO descriptors
ARM: pxa: Convert Spitz hsync to GPIO descriptors
ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
arch/arm/mach-pxa/gumstix.c | 24 +++++++-------
arch/arm/mach-pxa/reset.c | 39 ++++++++---------------
arch/arm/mach-pxa/reset.h | 3 +-
arch/arm/mach-pxa/spitz.c | 72 +++++++++++++++++++++++++++++++++---------
drivers/usb/host/ohci-pxa27x.c | 7 ++++
5 files changed, 91 insertions(+), 54 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230807-pxa-gpio-3ce25d574814
Best regards,
--
Duje Mihanović <[email protected]>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
a GPIO pin related to the USB host controller.
Convert this function to use the new GPIO descriptor interface.
Signed-off-by: Duje Mihanović <[email protected]>
---
arch/arm/mach-pxa/spitz.c | 13 ++++++-------
drivers/usb/host/ohci-pxa27x.c | 7 +++++++
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index cc691b199429..535e2b2e997b 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -649,23 +649,22 @@ static inline void spitz_mmc_init(void) {}
* USB Host
******************************************************************************/
#if defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_usb_host_gpio_table, "pxa27x-ohci", "gpio-pxa",
+ SPITZ_GPIO_USB_HOST, "usb-host", GPIO_ACTIVE_LOW);
+
static int spitz_ohci_init(struct device *dev)
{
- int err;
-
- err = gpio_request(SPITZ_GPIO_USB_HOST, "USB_HOST");
- if (err)
- return err;
+ gpiod_add_lookup_table(&spitz_usb_host_gpio_table);
/* Only Port 2 is connected, setup USB Port 2 Output Control Register */
UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
- return gpio_direction_output(SPITZ_GPIO_USB_HOST, 1);
+ return 0;
}
static void spitz_ohci_exit(struct device *dev)
{
- gpio_free(SPITZ_GPIO_USB_HOST);
+ gpiod_remove_lookup_table(&spitz_usb_host_gpio_table);
}
static struct pxaohci_platform_data spitz_ohci_platform_data = {
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 357d9aee38a3..b70d452ca7c2 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -121,6 +121,7 @@ struct pxa27x_ohci {
void __iomem *mmio_base;
struct regulator *vbus[3];
bool vbus_enabled[3];
+ struct gpio_desc *usb_host;
};
#define to_pxa27x_ohci(hcd) (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
@@ -447,6 +448,10 @@ static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
pxa_ohci = to_pxa27x_ohci(hcd);
pxa_ohci->clk = usb_clk;
pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
+ pxa_ohci->usb_host = gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
+ if (IS_ERR(pxa_ohci->usb_host))
+ dev_warn(&pdev->dev, "failed to get USB host GPIO with %pe\n",
+ pxa_ohci->usb_host);
for (i = 0; i < 3; ++i) {
char name[6];
@@ -512,6 +517,8 @@ static void ohci_hcd_pxa27x_remove(struct platform_device *pdev)
for (i = 0; i < 3; ++i)
pxa27x_ohci_set_vbus_power(pxa_ohci, i, false);
+ gpiod_put(pxa_ohci->usb_host);
+
usb_put_hcd(hcd);
}
--
2.42.0
Sharp's Spitz still uses the legacy GPIO interface in its
wait_for_hsync() function.
Convert it to use the GPIO descriptor interface.
Signed-off-by: Duje Mihanović <[email protected]>
---
arch/arm/mach-pxa/spitz.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index c789eeaf3c2c..25878daec986 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -520,12 +520,14 @@ static inline void spitz_leds_init(void) {}
* SSP Devices
******************************************************************************/
#if defined(CONFIG_SPI_PXA2XX) || defined(CONFIG_SPI_PXA2XX_MODULE)
+static struct gpio_desc *hsync;
+
static void spitz_ads7846_wait_for_hsync(void)
{
- while (gpio_get_value(SPITZ_GPIO_HSYNC))
+ while (gpiod_get_value(hsync))
cpu_relax();
- while (!gpio_get_value(SPITZ_GPIO_HSYNC))
+ while (!gpiod_get_value(hsync))
cpu_relax();
}
@@ -543,6 +545,8 @@ static struct gpiod_lookup_table spitz_ads7846_gpio_table = {
.table = {
GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_TP_INT,
"pendown", GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_HSYNC,
+ "hsync", GPIO_ACTIVE_LOW),
{ }
},
};
@@ -622,8 +626,13 @@ static void __init spitz_spi_init(void)
gpiod_add_lookup_table(&spitz_ads7846_gpio_table);
gpiod_add_lookup_table(&spitz_spi_gpio_table);
+ hsync = gpiod_get(NULL, "hsync", GPIOD_IN);
pxa2xx_set_spi_info(2, &spitz_spi_info);
- spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices));
+ if (IS_ERR(hsync)) {
+ pr_err("Failed to get hsync GPIO: %ld\n", PTR_ERR(hsync));
+ spi_register_board_info(ARRAY_AND_SIZE(&spitz_spi_devices[1]));
+ } else
+ spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices));
}
#else
static inline void spitz_spi_init(void) {}
--
2.42.0
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.
Acked-by: Linus Walleij <[email protected]>
Signed-off-by: Duje Mihanović <[email protected]>
---
arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 6aa4a3a9f7aa..59a4a439e3d2 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
--
2.42.0
Sharp's Spitz board still uses the legacy GPIO interface for configuring
its two onboard LEDs.
Convert them to use the GPIO descriptor interface.
Signed-off-by: Duje Mihanović <[email protected]>
---
arch/arm/mach-pxa/spitz.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 535e2b2e997b..6aa4a3a9f7aa 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -452,16 +452,25 @@ static inline void spitz_keys_init(void) {}
* LEDs
******************************************************************************/
#if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
+static struct gpiod_lookup_table spitz_led_gpio_table = {
+ .dev_id = "leds-gpio",
+ .table = {
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
+ GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
+ GPIO_ACTIVE_HIGH),
+ { }
+ }
+};
+
static struct gpio_led spitz_gpio_leds[] = {
{
.name = "spitz:amber:charge",
.default_trigger = "sharpsl-charge",
- .gpio = SPITZ_GPIO_LED_ORANGE,
},
{
.name = "spitz:green:hddactivity",
.default_trigger = "disk-activity",
- .gpio = SPITZ_GPIO_LED_GREEN,
},
};
@@ -480,6 +489,11 @@ static struct platform_device spitz_led_device = {
static void __init spitz_leds_init(void)
{
+ gpiod_add_lookup_table(&spitz_led_gpio_table);
+ spitz_gpio_leds[0].gpiod = gpiod_get_index(&spitz_led_device.dev,
+ NULL, 0, GPIOD_ASIS);
+ spitz_gpio_leds[1].gpiod = gpiod_get_index(&spitz_led_device.dev,
+ NULL, 1, GPIOD_ASIS);
platform_device_register(&spitz_led_device);
}
#else
--
2.42.0
Hi Duje,
thanks for your patch!
On Fri, Sep 29, 2023 at 3:15 PM Duje Mihanović <[email protected]> wrote:
> Sharp's Spitz still uses the legacy GPIO interface in its
> wait_for_hsync() function.
>
> Convert it to use the GPIO descriptor interface.
>
> Signed-off-by: Duje Mihanović <[email protected]>
Overall this looks fine, but can't help but notice:
> static void spitz_ads7846_wait_for_hsync(void)
> {
> - while (gpio_get_value(SPITZ_GPIO_HSYNC))
> + while (gpiod_get_value(hsync))
> cpu_relax();
Waits while the line is high...
> - while (!gpio_get_value(SPITZ_GPIO_HSYNC))
> + while (!gpiod_get_value(hsync))
> cpu_relax();
Then as it goes low, waits for it to go high again.
So the hsync signal is *active* when it is *low*.
> @@ -543,6 +545,8 @@ static struct gpiod_lookup_table spitz_ads7846_gpio_table = {
> .table = {
> GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_TP_INT,
> "pendown", GPIO_ACTIVE_LOW),
> + GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_HSYNC,
> + "hsync", GPIO_ACTIVE_LOW),
Which is what you appropriately flag it for.
BUT: the signal is now inverted in gpiolib, so the
spitz_ads7846_wait_for_hsync() loops needs to be rewritten
inverted, because the value from gpiod_get_value() now means
"asserted" if high.
/* Wait while de-asserted */
while (!gpiod_get_value(hsync))
cpu_relax();
/* Wait while asserted */
while (gpiod_get_value(hsync))
cpu_relax();
return;
Right?
> @@ -622,8 +626,13 @@ static void __init spitz_spi_init(void)
>
> gpiod_add_lookup_table(&spitz_ads7846_gpio_table);
> gpiod_add_lookup_table(&spitz_spi_gpio_table);
> + hsync = gpiod_get(NULL, "hsync", GPIOD_IN);
You are getting the gpiod from device NULL which is probably correct
when you do this in the board file.
But the spitz_ads7846_gpio_table where you put the descriptor
is associated with the ads7846 device so this will not work.
You either have to add a one-gpio table just for this, or (better)
move the whole spitz_ads7846_wait_for_hsync() down into the
touchscreen driver instead, so the device driver can (optionally) obtain
this gpio and deal with it. Which is easy because:
[linus@Fecusia linux-nomadik]$ git grep ads7846_platform_data
Documentation/spi/spi-summary.rst: static struct
ads7846_platform_data ads_info = {
arch/arm/mach-pxa/spitz.c:static struct ads7846_platform_data
spitz_ads7846_info = {
arch/mips/alchemy/devboards/db1000.c:static struct
ads7846_platform_data db1100_touch_pd = {
arch/powerpc/platforms/512x/pdm360ng.c:static struct
ads7846_platform_data pdm360ng_ads7846_pdata = {
Only three users in the kernel, and sptiz is the only one using the
void (*wait_for_sync)(void) callback in ads7846_platform_data!
So delete that callback from ads7846_platform_data in
include/linux/spi/ads7846.h
and augment drivers/input/touchscreen/ads7846.c to get the
GPIO optionally with gpiod_get_optional() from the device,
then copy the code from spitz_ads7846_wait_for_hsync() right
into the driver and make sure it gets called if and only
if the "hsync" gpio exists.
Yours,
Linus Walleij
Hi Duje,
thanks for your patch!
On Fri, Sep 29, 2023 at 3:15 PM Duje Mihanović <[email protected]> wrote:
> Sharp's Spitz board still uses the legacy GPIO interface for configuring
> its two onboard LEDs.
>
> Convert them to use the GPIO descriptor interface.
>
> Signed-off-by: Duje Mihanović <[email protected]>
(...)
> + .table = {
> + GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
> + GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
> + GPIO_ACTIVE_HIGH),
This looks right!
> + gpiod_add_lookup_table(&spitz_led_gpio_table);
> + spitz_gpio_leds[0].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 0, GPIOD_ASIS);
> + spitz_gpio_leds[1].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 1, GPIOD_ASIS);
> platform_device_register(&spitz_led_device);
I missed this before, sorry.
This will probably not work. You need to register the spitz_led_device
first, then
you can get the gpiod:s.
The lookup will use the device name to locate the device, and if the device
isn't there it can't be found.
Yours,
Linus Walleij
On Fri, Sep 29, 2023 at 3:15 PM Duje Mihanović <[email protected]> wrote:
> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> a GPIO pin related to the USB host controller.
>
> Convert this function to use the new GPIO descriptor interface.
>
> Signed-off-by: Duje Mihanović <[email protected]>
Looks good to me!
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij