Re: [PATCH v1 6/9] serial: qcom-geni: move resource control logic to separate functions

[Praveen Talari]

Hi

On 4/14/2025 1:29 PM, Jiri Slaby wrote:
> On 10. 04. 25, 19:40, Praveen Talari wrote:
> > Supports use in PM system/runtime frameworks, helping to
> > distinguish new resource control mechanisms and facilitate
> > future modifications within the new API.
> >
> > The code that handles the actual enable or disable of resources
> > like clock and ICC paths to a separate function
> > (geni_serial_resources_on() and geni_serial_resources_off()) which
> > enhances code readability.
> >
> > Signed-off-by: Praveen Talari <quic_ptalari@xxxxxxxxxxx>
> > ---
> >   drivers/tty/serial/qcom_geni_serial.c | 53 +++++++++++++++++++++------
> >   1 file changed, 42 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c 
> > b/drivers/tty/serial/qcom_geni_serial.c
> > index 889ce8961e0a..e341f5090ecc 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -1572,6 +1572,42 @@ static struct uart_driver 
> > qcom_geni_uart_driver = {
> >       .nr =  GENI_UART_PORTS,
> >   };
> >   +static int geni_serial_resources_off(struct uart_port *uport)
> > +{
> > +    struct qcom_geni_serial_port *port = to_dev_port(uport);
> > +    int ret;
> > +
> > +    dev_pm_opp_set_rate(uport->dev, 0);
> > +    ret = geni_se_resources_off(&port->se);
> > +    if (ret)
> > +        return ret;
> > +
> > +    geni_icc_disable(&port->se);
> > +
> > +    return ret;
>
> This is a bit confusing (needs context). return 0 directly.
here "ret" is also pointing to 0. Why can't we use "ret" directly 
instead of 0.
> > +}
> > +
> > +static int geni_serial_resources_on(struct uart_port *uport)
> > +{
> > +    struct qcom_geni_serial_port *port = to_dev_port(uport);
> > +    int ret;
> > +
> > +    ret = geni_icc_enable(&port->se);
> > +    if (ret)
> > +        return ret;
> > +
> > +    ret = geni_se_resources_on(&port->se);
> > +    if (ret) {
> > +        geni_icc_disable(&port->se);
> > +        return ret;
> > +    }
> > +
> > +    if (port->clk_rate)
> > +        dev_pm_opp_set_rate(uport->dev, port->clk_rate);
> > +
> > +    return ret;
>
> Same here.
>
> thanks,