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

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: <b84f765d-4dfc-4182-8d82-365b3e04f973@quicinc.com>
Date: Mon, 14 Apr 2025 14:25:26 +0530
From: Praveen Talari <quic_ptalari@...cinc.com>
To: Jiri Slaby <jirislaby@...nel.org>,
        Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>,
        Rob Herring <robh@...nel.org>,
        "Krzysztof
 Kozlowski" <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        "Bjorn
 Andersson" <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd
	<sboyd@...nel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-serial@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-pm@...r.kernel.org>
CC: <psodagud@...cinc.com>, <djaggi@...cinc.com>, <quic_msavaliy@...cinc.com>,
        <quic_vtanuku@...cinc.com>, <quic_arandive@...cinc.com>,
        <quic_mnaresh@...cinc.com>, <quic_shazhuss@...cinc.com>
Subject: Re: [PATCH v1 6/9] serial: qcom-geni: move resource control logic to
 separate functions

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@...cinc.com>
>> ---
>>   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,