| Message ID | 20211001093711.17450-1-fercerpav@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Adrian Schmutzler |
| Headers | show |
| Series | [v3] realtek: ensure output drivers are enabled in RTL8231 | expand |
> -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Paul Fertser > Sent: Freitag, 1. Oktober 2021 11:37 > To: openwrt-devel@lists.openwrt.org > Cc: Sander Vanheule <sander@svanheule.net>; Paul Fertser > <fercerpav@gmail.com> > Subject: [PATCH v3] realtek: ensure output drivers are enabled in RTL8231 > > The bootloader can leave the GPIO expander in a state which doesn't have > output drivers enabled so GPIOs will properly work for input but output > operations will have no effect. > > To avoid disrupting the boot in case the bootloader left direction and data > registers in an inconsistent state (e.g. pulling SoC's reset to 0) reconfigure > everything as input. > > Reviewed-by: Sander Vanheule <sander@svanheule.net> > Signed-off-by: Paul Fertser <fercerpav@gmail.com> I added Fixes: 16ae56b4f9ec ("realtek: fix RTL8231 gpio expander for high GPIOs") and will merge this later tonight. Best Adrian > --- > > Notes: > Changes from v2: > - Fix 5.10 as well > > Changes from v1: > - Fix comment to mention we affect two extra GPIOs > - Remove unused "v" declaration. > > .../realtek/files-5.10/drivers/gpio/gpio-rtl8231.c | 12 +++++++----- > .../realtek/files-5.4/drivers/gpio/gpio-rtl8231.c | 12 +++++++----- > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c > b/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c > index a8ffcdc31368..f4f5621e0c1b 100644 > --- a/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c > +++ b/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c > @@ -239,8 +239,6 @@ void rtl8231_gpio_set(struct gpio_chip *gc, unsigned > int offset, int value) > > int rtl8231_init(struct rtl8231_gpios *gpios) { > - u32 v; > - > pr_info("%s called, MDIO bus ID: %d\n", __func__, gpios- > >smi_bus_id); > > gpios->reg_cached = 0; > @@ -254,11 +252,15 @@ int rtl8231_init(struct rtl8231_gpios *gpios) > sw_w32_mask(3, 1, RTL838X_DMY_REG5); > } > > - /* Select GPIO functionality for pins 0-34 */ > + /* Select GPIO functionality and force input direction for pins 0-36 > +*/ > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(0), 0xffff); > + rtl8231_write(gpios, RTL8231_GPIO_DIR(0), 0xffff); > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(16), 0xffff); > - v = rtl8231_read(gpios, RTL8231_GPIO_PIN_SEL(32)); > - rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), v | 0x7); > + rtl8231_write(gpios, RTL8231_GPIO_DIR(16), 0xffff); > + rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 0x03ff); > + > + /* Set LED_Start to enable drivers for output mode */ > + rtl8231_write(gpios, RTL8231_LED_FUNC0, 1 << 1); > > return 0; > } > 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 a8ffcdc31368..f4f5621e0c1b 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 > @@ -239,8 +239,6 @@ void rtl8231_gpio_set(struct gpio_chip *gc, unsigned > int offset, int value) > > int rtl8231_init(struct rtl8231_gpios *gpios) { > - u32 v; > - > pr_info("%s called, MDIO bus ID: %d\n", __func__, gpios- > >smi_bus_id); > > gpios->reg_cached = 0; > @@ -254,11 +252,15 @@ int rtl8231_init(struct rtl8231_gpios *gpios) > sw_w32_mask(3, 1, RTL838X_DMY_REG5); > } > > - /* Select GPIO functionality for pins 0-34 */ > + /* Select GPIO functionality and force input direction for pins 0-36 > +*/ > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(0), 0xffff); > + rtl8231_write(gpios, RTL8231_GPIO_DIR(0), 0xffff); > rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(16), 0xffff); > - v = rtl8231_read(gpios, RTL8231_GPIO_PIN_SEL(32)); > - rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), v | 0x7); > + rtl8231_write(gpios, RTL8231_GPIO_DIR(16), 0xffff); > + rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 0x03ff); > + > + /* Set LED_Start to enable drivers for output mode */ > + rtl8231_write(gpios, RTL8231_LED_FUNC0, 1 << 1); > > return 0; > } > -- > 2.17.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Hello Adrian, Thank you for taking care about this. One note below. On Sat, Oct 02, 2021 at 06:37:22PM +0200, Adrian Schmutzler wrote: > > The bootloader can leave the GPIO expander in a state which doesn't have > > output drivers enabled so GPIOs will properly work for input but output > > operations will have no effect. ... > > Reviewed-by: Sander Vanheule <sander@svanheule.net> > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > > I added > > Fixes: 16ae56b4f9ec ("realtek: fix RTL8231 gpio expander for high GPIOs") Even though the patch changes the code that was introduced before with the patch you mention it's not fixing it. Commit 16ae56b4f9ec was fixing another bug (working with high GPIOs) and it was consistent with the existing code (that wasn't changing input/output state on init). However, that clearly leads to GPIOs not being able to work for output at least on some of the supported targets so if anything it should have Fixes: 2b88563ee5aa ("realtek: update the tree to the latest refactored version")
Hi, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Paul Fertser > Sent: Samstag, 2. Oktober 2021 18:51 > To: Adrian Schmutzler <mail@adrianschmutzler.de> > Cc: openwrt-devel@lists.openwrt.org; 'Sander Vanheule' > <sander@svanheule.net> > Subject: Re: [PATCH v3] realtek: ensure output drivers are enabled in > RTL8231 > > Hello Adrian, > > Thank you for taking care about this. One note below. > > On Sat, Oct 02, 2021 at 06:37:22PM +0200, Adrian Schmutzler wrote: > > > The bootloader can leave the GPIO expander in a state which doesn't > > > have output drivers enabled so GPIOs will properly work for input > > > but output operations will have no effect. > ... > > > Reviewed-by: Sander Vanheule <sander@svanheule.net> > > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > > > > I added > > > > Fixes: 16ae56b4f9ec ("realtek: fix RTL8231 gpio expander for high > > GPIOs") > > Even though the patch changes the code that was introduced before with > the patch you mention it's not fixing it. Commit 16ae56b4f9ec was fixing > another bug (working with high GPIOs) and it was consistent with the > existing code (that wasn't changing input/output state on init). However, > that clearly leads to GPIOs not being able to work for output at least on some > of the supported targets so if anything it should have > > Fixes: 2b88563ee5aa ("realtek: update the tree to the latest refactored > version") Thanks for the explanation. In this case, I will simply drop my Fixes: again and use the patch as you provided it. Best Adrian > > -- > Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! > mailto:fercerpav@gmail.com > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff --git a/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c b/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c index a8ffcdc31368..f4f5621e0c1b 100644 --- a/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c +++ b/target/linux/realtek/files-5.10/drivers/gpio/gpio-rtl8231.c @@ -239,8 +239,6 @@ void rtl8231_gpio_set(struct gpio_chip *gc, unsigned int offset, int value) int rtl8231_init(struct rtl8231_gpios *gpios) { - u32 v; - pr_info("%s called, MDIO bus ID: %d\n", __func__, gpios->smi_bus_id); gpios->reg_cached = 0; @@ -254,11 +252,15 @@ int rtl8231_init(struct rtl8231_gpios *gpios) sw_w32_mask(3, 1, RTL838X_DMY_REG5); } - /* Select GPIO functionality for pins 0-34 */ + /* Select GPIO functionality and force input direction for pins 0-36 */ rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(0), 0xffff); + rtl8231_write(gpios, RTL8231_GPIO_DIR(0), 0xffff); rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(16), 0xffff); - v = rtl8231_read(gpios, RTL8231_GPIO_PIN_SEL(32)); - rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), v | 0x7); + rtl8231_write(gpios, RTL8231_GPIO_DIR(16), 0xffff); + rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 0x03ff); + + /* Set LED_Start to enable drivers for output mode */ + rtl8231_write(gpios, RTL8231_LED_FUNC0, 1 << 1); return 0; } 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 a8ffcdc31368..f4f5621e0c1b 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 @@ -239,8 +239,6 @@ void rtl8231_gpio_set(struct gpio_chip *gc, unsigned int offset, int value) int rtl8231_init(struct rtl8231_gpios *gpios) { - u32 v; - pr_info("%s called, MDIO bus ID: %d\n", __func__, gpios->smi_bus_id); gpios->reg_cached = 0; @@ -254,11 +252,15 @@ int rtl8231_init(struct rtl8231_gpios *gpios) sw_w32_mask(3, 1, RTL838X_DMY_REG5); } - /* Select GPIO functionality for pins 0-34 */ + /* Select GPIO functionality and force input direction for pins 0-36 */ rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(0), 0xffff); + rtl8231_write(gpios, RTL8231_GPIO_DIR(0), 0xffff); rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(16), 0xffff); - v = rtl8231_read(gpios, RTL8231_GPIO_PIN_SEL(32)); - rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), v | 0x7); + rtl8231_write(gpios, RTL8231_GPIO_DIR(16), 0xffff); + rtl8231_write(gpios, RTL8231_GPIO_PIN_SEL(32), 0x03ff); + + /* Set LED_Start to enable drivers for output mode */ + rtl8231_write(gpios, RTL8231_LED_FUNC0, 1 << 1); return 0; }