linux-kernel - RE: [PATCH] serial: qcom_geni_serial: Setup serial port after Deep sleep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SJ0PR02MB7742A7386189692ACACC0A3FE3489@SJ0PR02MB7742.namprd02.prod.outlook.com>
Date:   Wed, 31 May 2023 13:06:22 +0000
From:   "Mehul Raninga (Temp) (QUIC)" <quic_mraninga@...cinc.com>
To:     Bjorn Andersson <andersson@...nel.org>,
        "Mehul Raninga (Temp) (QUIC)" <quic_mraninga@...cinc.com>
CC:     "agross@...nel.org" <agross@...nel.org>,
        "konrad.dybcio@...aro.org" <konrad.dybcio@...aro.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "jirislaby@...nel.org" <jirislaby@...nel.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Viken Dadhaniya (QUIC)" <quic_vdadhani@...cinc.com>,
        "Visweswara Tanuku (QUIC)" <quic_vtanuku@...cinc.com>,
        "Vijaya Krishna Nivarthi (Temp) (QUIC)" <quic_vnivarth@...cinc.com>
Subject: RE: [PATCH] serial: qcom_geni_serial: Setup serial port after Deep
 sleep

Hello Andersson,
Thanks for the review. Kindly find my reply inline below

> -----Original Message-----
> From: Bjorn Andersson <andersson@...nel.org>
> Sent: Tuesday, May 30, 2023 8:37 PM
> To: Mehul Raninga (Temp) (QUIC) <quic_mraninga@...cinc.com>
> Cc: agross@...nel.org; konrad.dybcio@...aro.org; gregkh@...uxfoundation.org;
> jirislaby@...nel.org; linux-arm-msm@...r.kernel.org; linux-
> serial@...r.kernel.org; linux-kernel@...r.kernel.org; Viken Dadhaniya (QUIC)
> <quic_vdadhani@...cinc.com>; Visweswara Tanuku (QUIC)
> <quic_vtanuku@...cinc.com>; Vijaya Krishna Nivarthi (Temp) (QUIC)
> <quic_vnivarth@...cinc.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.
> 
> >
> > 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@...cinc.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.
> 
> Regards,
> Bjorn
> 
> > +                     qcom_geni_serial_port_setup(uport);
> >       }
> >       return ret;
> >  }
> > --
> > 2.17.1
> >