ARM: pxa: Fix gpio numbers for backlight controls
Subsystems (3)
PATCH 1/1 ARM: pxa: Fix gpio numbers for backlight controls
With recent stable kernels on my Zaurus Akita I am unable to change the backlight brightness, as the necessary files are not in the sys filesystem, and at startup I get the following errors:
corgi-lcd spi2.1: requested GPIO 0 (207) is out of range [0..120] for chip gpio-pxa
corgi-lcd: probe of spi2.1 failed with error -22
Looking at recent changes to the Corgi backlight driver, I found that commit ee0c8e494cc3 ("backlight: corgi: Convert to use GPIO descriptors") replaced the use of the deprecated integer-based GPIO interface, but didn't convert the GPIO numbers in the platform header to match this. I think this is what causes the error, as the GPIOs for the backlight are part of an expander chip with a GPIO base number larger than the number of GPIOs for the gpio-pxa device.
This patch fixes things by updating the GPIO numbers for the backlight, simply by dropping the GPIO base. It also updates the GPIO lookups to look in the table for the appropriate expander. For the Akita platform the expander with i2c address 0-0018 is used, for Spitz and Borzoi the second Sharp SCOOP expander is used.
I've tested this patch with real Akita and Spitz handhelds, as well as with QEMU.
--- arch/arm/mach-pxa/spitz.c | 8 ++++---- arch/arm/mach-pxa/spitz.h | 34 +++++++++++++++++----------------- 2 files changed, 21 insertions(+), 21 deletions(-)
arch/arm/mach-pxa/spitz.c +4 -4
@@ -524,9 +524,9 @@ static void spitz_bl_kick_battery(void)static struct gpiod_lookup_table spitz_lcdcon_gpio_table = { .dev_id ="spi2.1" , .table = { -GPIO_LOOKUP ("gpio-pxa" ,SPITZ_GPIO_BACKLIGHT_CONT , +GPIO_LOOKUP ("sharp-scoop.1" ,SPITZ_GPIO_BACKLIGHT_CONT ,"BL_CONT" ,GPIO_ACTIVE_LOW ), -GPIO_LOOKUP ("gpio-pxa" ,SPITZ_GPIO_BACKLIGHT_ON , +GPIO_LOOKUP ("sharp-scoop.1" ,SPITZ_GPIO_BACKLIGHT_ON ,"BL_ON" ,GPIO_ACTIVE_HIGH ), { }, },
@@ -535,9 +535,9 @@ static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {static struct gpiod_lookup_table akita_lcdcon_gpio_table = { .dev_id ="spi2.1" , .table = { -GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_CONT , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_CONT ,"BL_CONT" ,GPIO_ACTIVE_LOW ), -GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_ON , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_ON ,"BL_ON" ,GPIO_ACTIVE_HIGH ), { }, },
arch/arm/mach-pxa/spitz.h +17 -17
@@ -137,26 +137,26 @@#define SPITZ_SCP2_SUS_SET (SPITZ_SCP2_IR_ON | SPITZ_SCP2_RESERVED_1)#define SPITZ_SCP2_GPIO_BASE (PXA_NR_BUILTIN_GPIO + 12) -#define SPITZ_GPIO_IR_ON (SPITZ_SCP2_GPIO_BASE + 0) -#define SPITZ_GPIO_AKIN_PULLUP (SPITZ_SCP2_GPIO_BASE + 1) -#define SPITZ_GPIO_RESERVED_1 (SPITZ_SCP2_GPIO_BASE + 2) -#define SPITZ_GPIO_RESERVED_2 (SPITZ_SCP2_GPIO_BASE + 3) -#define SPITZ_GPIO_RESERVED_3 (SPITZ_SCP2_GPIO_BASE + 4) -#define SPITZ_GPIO_RESERVED_4 (SPITZ_SCP2_GPIO_BASE + 5) -#define SPITZ_GPIO_BACKLIGHT_CONT (SPITZ_SCP2_GPIO_BASE + 6) -#define SPITZ_GPIO_BACKLIGHT_ON (SPITZ_SCP2_GPIO_BASE + 7) -#define SPITZ_GPIO_MIC_BIAS (SPITZ_SCP2_GPIO_BASE + 8) +#define SPITZ_GPIO_IR_ON 0 +#define SPITZ_GPIO_AKIN_PULLUP 1 +#define SPITZ_GPIO_RESERVED_1 2 +#define SPITZ_GPIO_RESERVED_2 3 +#define SPITZ_GPIO_RESERVED_3 4 +#define SPITZ_GPIO_RESERVED_4 5 +#define SPITZ_GPIO_BACKLIGHT_CONT 6 +#define SPITZ_GPIO_BACKLIGHT_ON 7 +#define SPITZ_GPIO_MIC_BIAS 8/* Akita IO Expander GPIOs */ #define AKITA_IOEXP_GPIO_BASE (PXA_NR_BUILTIN_GPIO + 12) -#define AKITA_GPIO_RESERVED_0 (AKITA_IOEXP_GPIO_BASE + 0) -#define AKITA_GPIO_RESERVED_1 (AKITA_IOEXP_GPIO_BASE + 1) -#define AKITA_GPIO_MIC_BIAS (AKITA_IOEXP_GPIO_BASE + 2) -#define AKITA_GPIO_BACKLIGHT_ON (AKITA_IOEXP_GPIO_BASE + 3) -#define AKITA_GPIO_BACKLIGHT_CONT (AKITA_IOEXP_GPIO_BASE + 4) -#define AKITA_GPIO_AKIN_PULLUP (AKITA_IOEXP_GPIO_BASE + 5) -#define AKITA_GPIO_IR_ON (AKITA_IOEXP_GPIO_BASE + 6) -#define AKITA_GPIO_RESERVED_7 (AKITA_IOEXP_GPIO_BASE + 7) +#define AKITA_GPIO_RESERVED_0 0 +#define AKITA_GPIO_RESERVED_1 1 +#define AKITA_GPIO_MIC_BIAS 2 +#define AKITA_GPIO_BACKLIGHT_ON 3 +#define AKITA_GPIO_BACKLIGHT_CONT 4 +#define AKITA_GPIO_AKIN_PULLUP 5 +#define AKITA_GPIO_IR_ON 6 +#define AKITA_GPIO_RESERVED_7 7/* Spitz IRQ Definitions */
Signature · 1 line
2.36.1
Quoted 17 lines · Laurence de Bruxelles
With recent stable kernels on my Zaurus Akita I am unable to change the backlight brightness, as the necessary files are not in the sys filesystem, and at startup I get the following errors:
corgi-lcd spi2.1: requested GPIO 0 (207) is out of range [0..120] for chip gpio-pxa
corgi-lcd: probe of spi2.1 failed with error -22Looking at recent changes to the Corgi backlight driver, I found that commit
ee0c8e494cc3("backlight: corgi: Convert to use GPIO descriptors") replaced the use of the deprecated integer-based GPIO interface, but didn't convert the GPIO numbers in the platform header to match this. I think this is what causes the error, as the GPIOs for the backlight are part of an expander chip with a GPIO base number larger than the number of GPIOs for the gpio-pxa device.
Quoted 8 lines
This patch fixes things by updating the GPIO numbers for the backlight, simply by dropping the GPIO base. It also updates the GPIO lookups to look in the table for the appropriate expander. For the Akita platform the expander with i2c address 0-0018 is used, for Spitz and Borzoi the second Sharp SCOOP expander is used.
I've tested this patch with real Akita and Spitz handhelds, as well as with QEMU.
Thank you for the patch! Since I recently worked on those files, I tried to have a closer look myself
arch/arm/mach-pxa/spitz.c static void spitz_bl_kick_battery(void) · line 532
Quoted 12 lines
arch/arm/mach-pxa/spitz.c +2
-2@@ -524,9 +524,9 @@ static void spitz_bl_kick_battery(void)static struct gpiod_lookup_table spitz_lcdcon_gpio_table = { .dev_id ="spi2.1" , .table = { -GPIO_LOOKUP ("gpio-pxa" ,SPITZ_GPIO_BACKLIGHT_CONT , +GPIO_LOOKUP ("sharp-scoop.1" ,SPITZ_GPIO_BACKLIGHT_CONT ,"BL_CONT" ,GPIO_ACTIVE_LOW ), -GPIO_LOOKUP ("gpio-pxa" ,SPITZ_GPIO_BACKLIGHT_ON , +GPIO_LOOKUP ("sharp-scoop.1" ,SPITZ_GPIO_BACKLIGHT_ON ,"BL_ON" ,GPIO_ACTIVE_HIGH ), { }, },
This part looks fine.
arch/arm/mach-pxa/spitz.c static struct gpiod_lookup_table spitz_lcdcon_gpio_table = { · line 543
Quoted 12 lines
arch/arm/mach-pxa/spitz.c +2
-2@@ -535,9 +535,9 @@ static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {static struct gpiod_lookup_table akita_lcdcon_gpio_table = { .dev_id ="spi2.1" , .table = { -GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_CONT , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_CONT ,"BL_CONT" ,GPIO_ACTIVE_LOW ), -GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_ON , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_ON ,"BL_ON" ,GPIO_ACTIVE_HIGH ), { }, },
I assume this is correct as well, but I can't see where you got the "0-0018" name from, or if we need to change others the same way.
Quoted 16 lines
arch/arm/mach-pxa/spitz.h +0
-8@@ -137,26 +137,26 @@#define SPITZ_SCP2_SUS_SET (SPITZ_SCP2_IR_ON | SPITZ_SCP2_RESERVED_1)#define SPITZ_SCP2_GPIO_BASE (PXA_NR_BUILTIN_GPIO + 12) -#define SPITZ_GPIO_IR_ON (SPITZ_SCP2_GPIO_BASE + 0) -#define SPITZ_GPIO_AKIN_PULLUP (SPITZ_SCP2_GPIO_BASE + 1) -#define SPITZ_GPIO_RESERVED_1 (SPITZ_SCP2_GPIO_BASE + 2) -#define SPITZ_GPIO_RESERVED_2 (SPITZ_SCP2_GPIO_BASE + 3) -#define SPITZ_GPIO_RESERVED_3 (SPITZ_SCP2_GPIO_BASE + 4) -#define SPITZ_GPIO_RESERVED_4 (SPITZ_SCP2_GPIO_BASE + 5) -#define SPITZ_GPIO_BACKLIGHT_CONT (SPITZ_SCP2_GPIO_BASE + 6) -#define SPITZ_GPIO_BACKLIGHT_ON (SPITZ_SCP2_GPIO_BASE + 7)
arch/arm/mach-pxa/spitz.h line 148
Quoted 10 lines
arch/arm/mach-pxa/spitz.h +9
-1-#define SPITZ_GPIO_MIC_BIAS (SPITZ_SCP2_GPIO_BASE + 8) +#define SPITZ_GPIO_IR_ON 0 +#define SPITZ_GPIO_AKIN_PULLUP 1 +#define SPITZ_GPIO_RESERVED_1 2 +#define SPITZ_GPIO_RESERVED_2 3 +#define SPITZ_GPIO_RESERVED_3 4 +#define SPITZ_GPIO_RESERVED_4 5 +#define SPITZ_GPIO_BACKLIGHT_CONT 6 +#define SPITZ_GPIO_BACKLIGHT_ON 7 +#define SPITZ_GPIO_MIC_BIAS 8
This probably breaks the audio support, at least the SPITZ_GPIO_MIC_BIAS one:
arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_L - SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_R - SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.1",SPITZ_GPIO_MIC_BIAS - SPITZ_SCP2_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_L - SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_R - SPITZ_SCP_GPIO_BASE,
I think we'd probably want to handle the SCP and SCP2 GPIOs the same way, either adding/subtracting the GPIO_BASE everywhere, or dropping that.
Quoted 8 lines
arch/arm/mach-pxa/spitz.h +0
-6/* Akita IO Expander GPIOs */ #define AKITA_IOEXP_GPIO_BASE (PXA_NR_BUILTIN_GPIO + 12) -#define AKITA_GPIO_RESERVED_0 (AKITA_IOEXP_GPIO_BASE + 0) -#define AKITA_GPIO_RESERVED_1 (AKITA_IOEXP_GPIO_BASE + 1) -#define AKITA_GPIO_MIC_BIAS (AKITA_IOEXP_GPIO_BASE + 2) -#define AKITA_GPIO_BACKLIGHT_ON (AKITA_IOEXP_GPIO_BASE + 3) -#define AKITA_GPIO_BACKLIGHT_CONT (AKITA_IOEXP_GPIO_BASE + 4) -#define AKITA_GPIO_AKIN_PULLUP (AKITA_IOEXP_GPIO_BASE + 5)
arch/arm/mach-pxa/spitz.h line 159
Quoted 10 lines
arch/arm/mach-pxa/spitz.h +8
-2-#define AKITA_GPIO_IR_ON (AKITA_IOEXP_GPIO_BASE + 6) -#define AKITA_GPIO_RESERVED_7 (AKITA_IOEXP_GPIO_BASE + 7) +#define AKITA_GPIO_RESERVED_0 0 +#define AKITA_GPIO_RESERVED_1 1 +#define AKITA_GPIO_MIC_BIAS 2 +#define AKITA_GPIO_BACKLIGHT_ON 3 +#define AKITA_GPIO_BACKLIGHT_CONT 4 +#define AKITA_GPIO_AKIN_PULLUP 5 +#define AKITA_GPIO_IR_ON 6 +#define AKITA_GPIO_RESERVED_7 7
I see that this line
arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("i2c-max7310",AKITA_GPIO_MIC_BIAS - AKITA_IOEXP_GPIO_BASE,
is now wrong, and presumably the "i2c-max7310" string also needs to be changed to "0-0018" now.
Arnd
Quoted 3 lines · Arnd Bergmann
Thank you for the patch! Since I recently worked on those files, I tried to have a closer look myself
Thanks for the comments! Sorry for the late reply, I got stuck in a rabbit hole trying to understand some things.
Quoted 16 lines
arch/arm/mach-pxa/spitz.c static struct gpiod_lookup_table spitz_lcdcon_gpio_table = { · line 543
Quoted 13 lines
arch/arm/mach-pxa/spitz.c +2
-2@@ -535,9 +535,9 @@ static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {static struct gpiod_lookup_table akita_lcdcon_gpio_table = { .dev_id ="spi2.1" , .table = { -GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_CONT , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_CONT ,"BL_CONT" ,GPIO_ACTIVE_LOW ), -GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_ON , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_ON ,"BL_ON" ,GPIO_ACTIVE_HIGH ), { }, },I assume this is correct as well, but I can't see where you got the "0-0018" name from, or if we need to change others the same way.
Good question. I spent a while trying to figure this out at the time, in the end I just plugged in the i2c address I found from looking in sysfs, and it worked. It does seem a bit opaque, but as far as I can tell from reading the GPIO API manual and the i2c GPIO driver that's what it is supposed to be:
key is ... the label of the gpiod_chip instance providing the GPIO
(from https://www.kernel.org/doc/html/latest/driver-api/gpio/board.html).
Quoted 42 lines
Quoted 17 lines
arch/arm/mach-pxa/spitz.h +0
-8@@ -137,26 +137,26 @@#define SPITZ_SCP2_SUS_SET (SPITZ_SCP2_IR_ON | SPITZ_SCP2_RESERVED_1)#define SPITZ_SCP2_GPIO_BASE (PXA_NR_BUILTIN_GPIO + 12) -#define SPITZ_GPIO_IR_ON (SPITZ_SCP2_GPIO_BASE + 0) -#define SPITZ_GPIO_AKIN_PULLUP (SPITZ_SCP2_GPIO_BASE + 1) -#define SPITZ_GPIO_RESERVED_1 (SPITZ_SCP2_GPIO_BASE + 2) -#define SPITZ_GPIO_RESERVED_2 (SPITZ_SCP2_GPIO_BASE + 3) -#define SPITZ_GPIO_RESERVED_3 (SPITZ_SCP2_GPIO_BASE + 4) -#define SPITZ_GPIO_RESERVED_4 (SPITZ_SCP2_GPIO_BASE + 5) -#define SPITZ_GPIO_BACKLIGHT_CONT (SPITZ_SCP2_GPIO_BASE + 6) -#define SPITZ_GPIO_BACKLIGHT_ON (SPITZ_SCP2_GPIO_BASE + 7)arch/arm/mach-pxa/spitz.h line 148
Quoted 10 lines
arch/arm/mach-pxa/spitz.h +9
-1-#define SPITZ_GPIO_MIC_BIAS (SPITZ_SCP2_GPIO_BASE + 8) +#define SPITZ_GPIO_IR_ON 0 +#define SPITZ_GPIO_AKIN_PULLUP 1 +#define SPITZ_GPIO_RESERVED_1 2 +#define SPITZ_GPIO_RESERVED_2 3 +#define SPITZ_GPIO_RESERVED_3 4 +#define SPITZ_GPIO_RESERVED_4 5 +#define SPITZ_GPIO_BACKLIGHT_CONT 6 +#define SPITZ_GPIO_BACKLIGHT_ON 7 +#define SPITZ_GPIO_MIC_BIAS 8This probably breaks the audio support, at least the
SPITZ_GPIO_MIC_BIASone:
arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_L-SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_R-SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.1",SPITZ_GPIO_MIC_BIAS-SPITZ_SCP2_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_L-SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_R-SPITZ_SCP_GPIO_BASE,
Quoted 3 lines
I think we'd probably want to handle the SCP and SCP2 GPIOs the same way, either adding/subtracting the
GPIO_BASEeverywhere, or dropping that.
Good catch, I'm not sure how I missed your patch. I think having the GPIO_BASE included in the index is part of the legacy API, or that is what the API docs suggest, so we shouldn't need to add it or subtract it anywhere.
I'd like to make that change, butg I'm not sure whether to submit that as another patch in a new V3 series, or as a separate patch, or just roll it into one patch... I'll probably make a V3 patch series unless you suggest otherwise.
Quoted 21 lines
Quoted 8 lines
arch/arm/mach-pxa/spitz.h +0
-6/* Akita IO Expander GPIOs */ #define AKITA_IOEXP_GPIO_BASE (PXA_NR_BUILTIN_GPIO + 12) -#define AKITA_GPIO_RESERVED_0 (AKITA_IOEXP_GPIO_BASE + 0) -#define AKITA_GPIO_RESERVED_1 (AKITA_IOEXP_GPIO_BASE + 1) -#define AKITA_GPIO_MIC_BIAS (AKITA_IOEXP_GPIO_BASE + 2) -#define AKITA_GPIO_BACKLIGHT_ON (AKITA_IOEXP_GPIO_BASE + 3) -#define AKITA_GPIO_BACKLIGHT_CONT (AKITA_IOEXP_GPIO_BASE + 4) -#define AKITA_GPIO_AKIN_PULLUP (AKITA_IOEXP_GPIO_BASE + 5)arch/arm/mach-pxa/spitz.h line 159
Quoted 10 lines
arch/arm/mach-pxa/spitz.h +8
-2-#define AKITA_GPIO_IR_ON (AKITA_IOEXP_GPIO_BASE + 6) -#define AKITA_GPIO_RESERVED_7 (AKITA_IOEXP_GPIO_BASE + 7) +#define AKITA_GPIO_RESERVED_0 0 +#define AKITA_GPIO_RESERVED_1 1 +#define AKITA_GPIO_MIC_BIAS 2 +#define AKITA_GPIO_BACKLIGHT_ON 3 +#define AKITA_GPIO_BACKLIGHT_CONT 4 +#define AKITA_GPIO_AKIN_PULLUP 5 +#define AKITA_GPIO_IR_ON 6 +#define AKITA_GPIO_RESERVED_7 7I see that this line
Quoted 6 lines
arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("i2c-max7310",AKITA_GPIO_MIC_BIAS-AKITA_IOEXP_GPIO_BASE,is now wrong, and presumably the "i2c-max7310" string also needs to be changed to "0-0018" now.
Yes, I think so, when I run the kernel with this change with akita on QEMU I get the error
spitz-audio spitz-audio: cannot find GPIO chip i2c-max7310, deferring
It's a shame, because it is more readable I think. I will change that in the followup patch I mentioned.
But to be honest I'm not fully able to test any changes to the audio support for this hardware. I spent a little while trying to get audio working on my device with a version of kernel 5.15, which doesn't have your audio patch, but it was broken. I reverted a change to the pxa audio driver to fix a DMA preallocation issue, but it was still broken. So I don't have a working audio setup just yet.
Quoted 23 lines · Laurence de Bruxelles
Quoted 7 lines · Arnd Bergmann
Quoted 7 lines
arch/arm/mach-pxa/spitz.c +1
-1@@ -535,9 +535,9 @@ static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {static struct gpiod_lookup_table akita_lcdcon_gpio_table = { .dev_id ="spi2.1" , .table = { -GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_CONT , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_CONT ,"BL_CONT" ,GPIO_ACTIVE_LOW ),Quoted 8 lines
arch/arm/mach-pxa/spitz.c static struct gpiod_lookup_table spitz_lcdcon_gpio_table = { · line 543
Quoted 5 lines
arch/arm/mach-pxa/spitz.c +1
-1-GPIO_LOOKUP ("gpio-pxa" ,AKITA_GPIO_BACKLIGHT_ON , +GPIO_LOOKUP ("0-0018" ,AKITA_GPIO_BACKLIGHT_ON , "BL_ON", GPIO_ACTIVE_HIGH), { }, },I assume this is correct as well, but I can't see where you got the "0-0018" name from, or if we need to change others the same way.
Good question. I spent a while trying to figure this out at the time, in the end I just plugged in the i2c address I found from looking in sysfs, and it worked. It does seem a bit opaque, but as far as I can tell from reading the GPIO API manual and the i2c GPIO driver that's what it is supposed to be:
Quoted 3 lines
key is ... the label of the
gpiod_chipinstance providing the GPIO(from https://www.kernel.org/doc/html/latest/driver-api/gpio/board.html).
It clearly has to be the label that is associated with the i2c-gpio driver, I'm just surprised that this label is just the i2c address and nor something more descriptive. I see that pca953x_setup_gpio() sets the label as
gc->label = dev_name(&chip->client->dev);
which may or may not be the right thing to do here. Maybe Linus has a suggestion if this is expected or the name can be set to something else instead.
Quoted 20 lines
Quoted 11 lines
arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_L-SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_R-SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.1",SPITZ_GPIO_MIC_BIAS-SPITZ_SCP2_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_L-SPITZ_SCP_GPIO_BASE,arch/arm/mach-pxa/spitz.c: GPIO_LOOKUP("sharp-scoop.0",SPITZ_GPIO_MUTE_R-SPITZ_SCP_GPIO_BASE,Quoted 3 lines
I think we'd probably want to handle the SCP and SCP2 GPIOs the same way, either adding/subtracting the
GPIO_BASEeverywhere, or dropping that.Good catch, I'm not sure how I missed your patch. I think having the
GPIO_BASEincluded in the index is part of the legacy API, or that is what the API docs suggest, so we shouldn't need to add it or subtract it anywhere.
Quoted 4 lines
I'd like to make that change, butg I'm not sure whether to submit that as another patch in a new V3 series, or as a separate patch, or just roll it into one patch... I'll probably make a V3 patch series unless you suggest otherwise.
I think it's fine either way. As a rule, I try to decide based on the changelog
text: if I end up describing two different things in a commit log, then it's time to split it up, but if I find a coherent way to describe the change then it can stay as a single patch. If you end up splitting it, I would recommend to keep changes across multiple boards together when they do the same thing, but split it into logical changes that build on top of one another.
Quoted 6 lines
But to be honest I'm not fully able to test any changes to the audio support for this hardware. I spent a little while trying to get audio working on my device with a version of kernel 5.15, which doesn't have your audio patch, but it was broken. I reverted a change to the pxa audio driver to fix a DMA preallocation issue, but it was still broken. So I don't have a working audio setup just yet.
Ok, I see. Don't spend too much time on that yet then. It should be possible to figure out what broke it using git-bisect, but you can do that once you work on the DT conversion.
Arnd