lkml.org 
[lkml]   [2023]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] serial: qcom_geni_serial: Setup serial port after Deep sleep
    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
    > > >

    \
     
     \ /
      Last update: 2023-06-01 06:04    [W:3.269 / U:0.512 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site