serial: qcom_geni_serial: Setup serial port after Deep sleep

While exiting deep sleep, serial port loses its configuration
hence it prints garbage characters on console.

Set serial port configuration while resume from deep sleep.

Signed-off-by: Mehul Raninga <quic_mraninga@quicinc.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 3 +++
 1 file changed, 3 insertions(+)

On Wed, May 31, 2023 at 01:06:22PM +0000, Mehul Raninga (Temp) (QUIC) wrote:
> Hello Andersson,
> Thanks for the review. Kindly find my reply inline below
> 
> > -----Original Message-----
> > From: Bjorn Andersson <andersson@kernel.org>
> > Sent: Tuesday, May 30, 2023 8:37 PM
> > To: Mehul Raninga (Temp) (QUIC) <quic_mraninga@quicinc.com>
> > Cc: agross@kernel.org; konrad.dybcio@linaro.org; gregkh@linuxfoundation.org;
> > jirislaby@kernel.org; linux-arm-msm@vger.kernel.org; linux-
> > serial@vger.kernel.org; linux-kernel@vger.kernel.org; Viken Dadhaniya (QUIC)
> > <quic_vdadhani@quicinc.com>; Visweswara Tanuku (QUIC)
> > <quic_vtanuku@quicinc.com>; Vijaya Krishna Nivarthi (Temp) (QUIC)
> > <quic_vnivarth@quicinc.com>
> > Subject: Re: [PATCH] serial: qcom_geni_serial: Setup serial port after Deep sleep
> > 
> > WARNING: This email originated from outside of Qualcomm. Please be wary of
> > any links or attachments, and do not enable macros.
> > 
> > On Tue, May 30, 2023 at 04:45:57PM +0530, Mehul Raninga wrote:
> > > While exiting deep sleep, serial port loses its configuration hence it
> > > prints garbage characters on console.
> > 
> > Presumably it lost its configuration sometime after suspend, rather than while
> > resuming the system?
> 
> I will reword commit message in V2 as below:
> Serial port lost its configuration sometime after suspend hence it
> prints garbage characters on console after resuming.

Please wrap your replies to 72 chars wide

You can probably be more specific than "sometime after suspend" :)

> > 
> > >
> > > Set serial port configuration while resume from deep sleep.
> > >
> > 
> > What happens if you do this unconditionally?
> 
> pm_suspend_via_firmware returns true indicating system is resuming
> from deepsleep. In case we are not resuming from deepsleep, this
> serial port setup is not required.
> > 
> > > Signed-off-by: Mehul Raninga <quic_mraninga@quicinc.com>
> > > ---
> > >  drivers/tty/serial/qcom_geni_serial.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/tty/serial/qcom_geni_serial.c
> > > b/drivers/tty/serial/qcom_geni_serial.c
> > > index 8582479f0211..c04b8fec30ba 100644
> > > --- a/drivers/tty/serial/qcom_geni_serial.c
> > > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > > @@ -20,6 +20,7 @@
> > >  #include <linux/serial.h>
> > >  #include <linux/serial_core.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/suspend.h>
> > >  #include <linux/tty.h>
> > >  #include <linux/tty_flip.h>
> > >  #include <dt-bindings/interconnect/qcom,icc.h>
> > > @@ -1737,6 +1738,8 @@ static int qcom_geni_serial_sys_resume(struct
> > device *dev)
> > >       if (uart_console(uport)) {
> > >               geni_icc_set_tag(&port->se, QCOM_ICC_TAG_ALWAYS);
> > >               geni_icc_set_bw(&port->se);
> > > +             if (pm_suspend_via_firmware())
> > 
> > I'm not familiar with this api, but aren't all our systems implementing firmware-
> > assisted suspend?
> 
> Not all the platform supports deep sleep hence to differentiate if
> resume is from deep sleep suspend or normal suspend, this api is
> required.

Can you point me to where this difference in flags is coming from in the
upstream kernel?

Thanks,
Bjorn

> > 
> > Regards,
> > Bjorn
> > 
> > > +                     qcom_geni_serial_port_setup(uport);
> > >       }
> > >       return ret;
> > >  }
> > > --
> > > 2.17.1
> > >

On Fri, Jun 02, 2023 at 06:39:02AM +0000, Mehul Raninga (Temp) (QUIC) wrote:
[..]
> > On Wed, May 31, 2023 at 01:06:22PM +0000, Mehul Raninga (Temp)
> > (QUIC) wrote:
[..]
> > > > > +             if (pm_suspend_via_firmware())
> > > >
> > > > I'm not familiar with this api, but aren't all our systems
> > > > implementing firmware- assisted suspend?
> > >
> > > Not all the platform supports deep sleep hence to differentiate if
> > > resume is from deep sleep suspend or normal suspend, this api is
> > > required.
> > 
> > Can you point me to where this difference in flags is coming from in the
> > upstream kernel?
> 
> In upstream kernel by default the flag is false, and for deepsleep
> support vendor kernel sets this flag true.

I am questioning whether this is correct, given that we indeed "suspend
with the help of firmware". But I might be misinterpreting what that
statement means.

Regards,
Bjorn