linux-kernel - RE: [V2] tty: serial: qcom-geni-serial: Remove uart frequency table. Instead, find suitable frequency with call to clk_round

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] [day] [month] [year] [list]
Message-ID: <BL0PR02MB45645D04D0EFCAC7CF0CF4A1FADC9@BL0PR02MB4564.namprd02.prod.outlook.com>
Date:   Tue, 31 May 2022 18:17:40 +0000
From:   "Vijaya Krishna Nivarthi (Temp) (QUIC)" <quic_vnivarth@...cinc.com>
To:     "Vijaya Krishna Nivarthi (Temp) (QUIC)" <quic_vnivarth@...cinc.com>,
        Doug Anderson <dianders@...omium.org>
CC:     Andy Gross <agross@...nel.org>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "Mukesh Savaliya (QUIC)" <quic_msavaliy@...cinc.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        Stephen Boyd <swboyd@...omium.org>,
        "Satya Priya Kakitapalli (Temp) (QUIC)" <quic_c_skakit@...cinc.com>
Subject: RE: [V2] tty: serial: qcom-geni-serial: Remove uart frequency table.
 Instead, find suitable frequency with call to clk_round_rate.

For sake of smoother upstreaming, I will be implementing feedback in incremental patches.
Uploading first one today.
Thank you.


-----Original Message-----
From: Vijaya Krishna Nivarthi (Temp) (QUIC) 
Sent: Friday, May 27, 2022 6:36 PM
To: Doug Anderson <dianders@...omium.org>; Vijaya Krishna Nivarthi (Temp) (QUIC) <quic_vnivarth@...cinc.com>
Cc: Andy Gross <agross@...nel.org>; bjorn.andersson@...aro.org; Greg Kroah-Hartman <gregkh@...uxfoundation.org>; Jiri Slaby <jirislaby@...nel.org>; linux-arm-msm <linux-arm-msm@...r.kernel.org>; linux-serial@...r.kernel.org; LKML <linux-kernel@...r.kernel.org>; Mukesh Savaliya (QUIC) <quic_msavaliy@...cinc.com>; Matthias Kaehlcke <mka@...omium.org>; Stephen Boyd <swboyd@...omium.org>; Satya Priya Kakitapalli (Temp) (QUIC) <quic_c_skakit@...cinc.com>
Subject: RE: [V2] tty: serial: qcom-geni-serial: Remove uart frequency table. Instead, find suitable frequency with call to clk_round_rate.

Thanks a lot for very useful feedback.
I see that the patch has landed, will upload another incorporating these suggestions.
-Vijay/


-----Original Message-----
From: Doug Anderson <dianders@...omium.org>
Sent: Thursday, May 26, 2022 10:38 PM
To: Vijaya Krishna Nivarthi (Temp) (QUIC) <quic_vnivarth@...cinc.com>
Cc: Andy Gross <agross@...nel.org>; bjorn.andersson@...aro.org; Greg Kroah-Hartman <gregkh@...uxfoundation.org>; Jiri Slaby <jirislaby@...nel.org>; linux-arm-msm <linux-arm-msm@...r.kernel.org>; linux-serial@...r.kernel.org; LKML <linux-kernel@...r.kernel.org>; Mukesh Savaliya (QUIC) <quic_msavaliy@...cinc.com>; Matthias Kaehlcke <mka@...omium.org>; Stephen Boyd <swboyd@...omium.org>; Satya Priya Kakitapalli (Temp) (QUIC) <quic_c_skakit@...cinc.com>
Subject: Re: [V2] tty: serial: qcom-geni-serial: Remove uart frequency table. Instead, find suitable frequency with call to clk_round_rate.

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

Hi,

On Mon, May 16, 2022 at 3:38 AM Vijaya Krishna Nivarthi <quic_vnivarth@...cinc.com> wrote:
>
> Replace the UART frequency table 'root_freq[]' with logic around
> clk_round_rate() so that SoC details like the available clk 
> frequencies can change and this driver still works. This reduces tight 
> coupling between this UART driver and the SoC clk driver because we no 
> longer have to update the 'root_freq[]' array for new SoCs. Instead 
> the driver determines the available frequencies at runtime.
>
> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@...cinc.com>
> ---
> v2: loops through clk dividers to zero-in quickly
> v1: intial patch looped through available clk frequencies
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 56
> ++++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c
> b/drivers/tty/serial/qcom_geni_serial.c
> index f496102..4733a23 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -149,12 +149,6 @@ static unsigned int 
> qcom_geni_serial_tx_empty(struct uart_port *port);  static void 
> qcom_geni_serial_stop_rx(struct uart_port *uport);  static void 
> qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop);
>
> -static const unsigned long root_freq[] = {7372800, 14745600, 19200000, 29491200,
> -                                       32000000, 48000000, 51200000, 64000000,
> -                                       80000000, 96000000, 100000000,
> -                                       102400000, 112000000, 120000000,
> -                                       128000000};
> -
>  #define to_dev_port(ptr, member) \
>                 container_of(ptr, struct qcom_geni_serial_port,
> member)
>
> @@ -946,25 +940,43 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
>         return 0;
>  }
>
> -static unsigned long get_clk_cfg(unsigned long clk_freq) -{
> -       int i;
> -
> -       for (i = 0; i < ARRAY_SIZE(root_freq); i++) {
> -               if (!(root_freq[i] % clk_freq))
> -                       return root_freq[i];
> -       }
> -       return 0;
> -}
> -
> -static unsigned long get_clk_div_rate(unsigned int baud,
> +static unsigned long get_clk_div_rate(struct clk *clk, unsigned int 
> +baud,
>                         unsigned int sampling_rate, unsigned int
> *clk_div)  {
>         unsigned long ser_clk;
>         unsigned long desired_clk;
> +       unsigned long freq, prev;
> +       unsigned long div, maxdiv;
> +       int64_t mult;

Why is "mult" signed? Shouldn't it be type "u64" or something?