| Message ID | 20210917131121.14057-1-fercerpav@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | realtek: fix RTL8231 gpio expander for high GPIOs | expand |
Hi Paul, Thanks for the patch. One comment below. On Fri, 2021-09-17 at 16:11 +0300, Paul Fertser wrote: > GPIOs > 31 require special handling. This patch fixes both the > initialisation (defaulting to input) and direction get/set operations. > > Runtime-tested on D-Link DGS-1210-10P-R1 which has "reset" button on > GPIO[33]. > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > --- > .../realtek/files-5.4/drivers/gpio/gpio-rtl8231.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c > b/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c > index 031f60f5307c..ddb8894663ec 100644 > --- a/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c > +++ b/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c > @@ -253,9 +252,10 @@ int rtl8231_init(struct rtl8231_gpios *gpios) > sw_w32_mask(3, 1, RTL838X_DMY_REG5); > } > > - /*Select GPIO functionality for pins 0-15, 16-31 and 32-37 */ > + /*Select GPIO functionality for pins 0-15, 16-31 and 32-34 */ > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(0), 0xffff); > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(16), 0xffff); > + rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 7 | (7 << 5)); > Pins 35-36 cannot serve as LED output, but their GPIO direction can still be changed. Since the select bit for pins 35-36 should always read '1' (the GPIO pin function), this should probably be: rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 0x1f | (0x1f << 5)); Best, Sander
Hi Sander, Thank you for review, it made me think about consistency. On Mon, Sep 20, 2021 at 05:45:06PM +0200, Sander Vanheule wrote: > On Fri, 2021-09-17 at 16:11 +0300, Paul Fertser wrote: > > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(0), 0xffff); > > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(16), 0xffff); > > + rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 7 | (7 << 5)); > > Pins 35-36 cannot serve as LED output, but their GPIO direction can still be changed. > Since the select bit for pins 35-36 should always read '1' (the GPIO pin function), this > should probably be: > > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 0x1f | (0x1f << 5)); So we are not changing the direction for GPIOs 0-31 and it's probably a good thing since probably the bootloader has already configured them in some desired way. Guess the same idea should be applied to high GPIOs too, please see my v2.
diff --git a/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c b/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c index 031f60f5307c..ddb8894663ec 100644 --- a/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c +++ b/target/linux/realtek/files-5.4/drivers/gpio/gpio-rtl8231.c @@ -106,12 +106,11 @@ static int rtl8231_pin_dir(struct rtl8231_gpios *gpios, u32 gpio, u32 dir) u32 v; int pin_sel_addr = RTL8231_GPIO_PIN_SEL(gpio); int pin_dir_addr = RTL8231_GPIO_DIR(gpio); - int pin = gpio % 16; - int dpin = pin; + int dpin = gpio % 16; if (gpio > 31) { pr_debug("WARNING: HIGH pin\n"); - dpin = pin << 5; + dpin += 5; pin_dir_addr = pin_sel_addr; } @@ -140,7 +139,7 @@ static int rtl8231_pin_dir_get(struct rtl8231_gpios *gpios, u32 gpio, u32 *dir) if (gpio > 31) { pin_dir_addr = RTL8231_GPIO_PIN_SEL(gpio); - pin = pin << 5; + pin += 5; } v = rtl8231_read(gpios, pin_dir_addr); @@ -253,9 +252,10 @@ int rtl8231_init(struct rtl8231_gpios *gpios) sw_w32_mask(3, 1, RTL838X_DMY_REG5); } - /*Select GPIO functionality for pins 0-15, 16-31 and 32-37 */ + /*Select GPIO functionality for pins 0-15, 16-31 and 32-34 */ rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(0), 0xffff); rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(16), 0xffff); + rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 7 | (7 << 5)); return 0; }
GPIOs > 31 require special handling. This patch fixes both the initialisation (defaulting to input) and direction get/set operations. Runtime-tested on D-Link DGS-1210-10P-R1 which has "reset" button on GPIO[33]. Signed-off-by: Paul Fertser <fercerpav@gmail.com> --- .../realtek/files-5.4/drivers/gpio/gpio-rtl8231.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)