| Message ID | 20231218072428.1802969-3-sumit.garg@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add SE HMBSC board support | expand |
I guess these GPIOs are RTS/CTS? Missing description... Given that you just set them to their respective values, you should be able to configure these using pinctrl with this patch [1], so we can avoid adding these non-standard bindings. I tested your next patch, setting "MSM_UART_MR1_RX_RDY_CTL" in uart_dm_init() on db410c and db820c and I don't notice any issues with setting that bit unconditionally. If it's definitely needed for your board then I'd be ok with a patch to always set it with a comment offering some context. [1]: https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-10-b6dd9704219e@linaro.org/ On 18/12/2023 07:24, Sumit Garg wrote: > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > --- > drivers/serial/serial_msm.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c > index a22623c316e..43e58595dc2 100644 > --- a/drivers/serial/serial_msm.c > +++ b/drivers/serial/serial_msm.c > @@ -16,6 +16,7 @@ > #include <serial.h> > #include <watchdog.h> > #include <asm/global_data.h> > +#include <asm/gpio.h> > #include <asm/io.h> > #include <linux/compiler.h> > #include <linux/delay.h> > @@ -210,6 +211,7 @@ static int msm_serial_probe(struct udevice *dev) > { > int ret; > struct msm_serial_data *priv = dev_get_priv(dev); > + struct gpio_desc rs232_0, rs232_1; > > /* No need to reinitialize the UART after relocation */ > if (gd->flags & GD_FLG_RELOC) > @@ -219,6 +221,11 @@ static int msm_serial_probe(struct udevice *dev) > if (ret) > return ret; > > + if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT)) > + dm_gpio_set_value(&rs232_0, 1); > + if (!gpio_request_by_name(dev, "gpios", 1, &rs232_1, GPIOD_IS_OUT)) > + dm_gpio_set_value(&rs232_1, 0); > + > pinctrl_select_state(dev, "uart"); > uart_dm_init(priv); >
On Tue, 19 Dec 2023 at 22:17, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > I guess these GPIOs are RTS/CTS? Missing description... No, these GPIOs (99 and 100) on HMIBSC board rather act as a mux to control UART mode out of rs232/422/485. I will add a corresponding description. > > Given that you just set them to their respective values, you should be > able to configure these using pinctrl with this patch [1], so we can > avoid adding these non-standard bindings. Okay I will try to use them once we resolve the conflicts discussed in the other thread. > > I tested your next patch, setting "MSM_UART_MR1_RX_RDY_CTL" in > uart_dm_init() on db410c and db820c and I don't notice any issues with > setting that bit unconditionally. If it's definitely needed for your > board then I'd be ok with a patch to always set it with a comment > offering some context. Yeah it's needed for RS232 mode to operate correctly. I will make it unconditional then. -Sumit > > [1]: > https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-10-b6dd9704219e@linaro.org/ > > On 18/12/2023 07:24, Sumit Garg wrote: > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > --- > > drivers/serial/serial_msm.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c > > index a22623c316e..43e58595dc2 100644 > > --- a/drivers/serial/serial_msm.c > > +++ b/drivers/serial/serial_msm.c > > @@ -16,6 +16,7 @@ > > #include <serial.h> > > #include <watchdog.h> > > #include <asm/global_data.h> > > +#include <asm/gpio.h> > > #include <asm/io.h> > > #include <linux/compiler.h> > > #include <linux/delay.h> > > @@ -210,6 +211,7 @@ static int msm_serial_probe(struct udevice *dev) > > { > > int ret; > > struct msm_serial_data *priv = dev_get_priv(dev); > > + struct gpio_desc rs232_0, rs232_1; > > > > /* No need to reinitialize the UART after relocation */ > > if (gd->flags & GD_FLG_RELOC) > > @@ -219,6 +221,11 @@ static int msm_serial_probe(struct udevice *dev) > > if (ret) > > return ret; > > > > + if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT)) > > + dm_gpio_set_value(&rs232_0, 1); > > + if (!gpio_request_by_name(dev, "gpios", 1, &rs232_1, GPIOD_IS_OUT)) > > + dm_gpio_set_value(&rs232_1, 0); > > + > > pinctrl_select_state(dev, "uart"); > > uart_dm_init(priv); > > > > -- > // Caleb (they/them)
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c index a22623c316e..43e58595dc2 100644 --- a/drivers/serial/serial_msm.c +++ b/drivers/serial/serial_msm.c @@ -16,6 +16,7 @@ #include <serial.h> #include <watchdog.h> #include <asm/global_data.h> +#include <asm/gpio.h> #include <asm/io.h> #include <linux/compiler.h> #include <linux/delay.h> @@ -210,6 +211,7 @@ static int msm_serial_probe(struct udevice *dev) { int ret; struct msm_serial_data *priv = dev_get_priv(dev); + struct gpio_desc rs232_0, rs232_1; /* No need to reinitialize the UART after relocation */ if (gd->flags & GD_FLG_RELOC) @@ -219,6 +221,11 @@ static int msm_serial_probe(struct udevice *dev) if (ret) return ret; + if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT)) + dm_gpio_set_value(&rs232_0, 1); + if (!gpio_request_by_name(dev, "gpios", 1, &rs232_1, GPIOD_IS_OUT)) + dm_gpio_set_value(&rs232_1, 0); + pinctrl_select_state(dev, "uart"); uart_dm_init(priv);
Signed-off-by: Sumit Garg <sumit.garg@linaro.org> --- drivers/serial/serial_msm.c | 7 +++++++ 1 file changed, 7 insertions(+)