On Tue, Oct 01, 2024 at 07:20:36PM GMT, Mukesh Kumar Savaliya wrote:
> Thanks Johan for the fixes.
>
> On 10/1/2024 6:20 PM, Johan Hovold wrote:
> > The receiver should not be enabled until the port is opened so drop the
> > bogus call to start rx from the setup code which is shared with the
> > console implementation.
> >
> > This was added for some confused implementation of hibernation support,
> > but the receiver must not be started unconditionally as the port may not
> > have been open when hibernating the system.
> >
> > Fixes: 35781d8356a2 ("tty: serial: qcom-geni-serial: Add support for Hibernation feature")
> > Cc:stable@xxxxxxxxxxxxxxx # 6.2
> > Cc: Aniket Randive<quic_arandive@xxxxxxxxxxx>
> > Signed-off-by: Johan Hovold<johan+linaro@xxxxxxxxxx>
> > ---
> > drivers/tty/serial/qcom_geni_serial.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 6f0db310cf69..9ea6bd09e665 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -1152,7 +1152,6 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
> > false, true, true);
> > geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
> > geni_se_select_mode(&port->se, port->dev_data->mode);
> > - qcom_geni_serial_start_rx(uport);
> Does it mean hibernation will break now ? Not sure if its tested with
> hibernation. I can see this call was added to port_setup specifically for
> hibernation but now after removing it, where is it getting fixed ?
Can you explain how you're testing hibernation and on which platform
this is done? I'd like to add this to my set of tests, but last time I
tested I couldn't find a platform where we survived the restore
processes (it's been a while though).
> I think RX will not be initialized after hibernation.
qcom_geni_serial_port_setup() is invoked in multiple places, how come
we don't perform this hibernation-specific operation in
qcom_geni_serial_sys_hib_resume()? (And why is it called hib_resume when
the kernel nomenclature for what it does is "restore"?)
Regards,
Bjorn
> > port->setup = true;
> > return 0;
> > -- 2.45.2
