linux-kernel - Re: [PATCH] pinctrl: qcom: sc7180: Add new qup 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: <CAD=FV=XAhSWOOPrHpuhjObi0UpKaeaD4TX1yhTh64cy39E1wyQ@mail.gmail.com>
Date:   Tue, 10 Dec 2019 09:44:55 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Rajendra Nayak <rnayak@...eaurora.org>
Cc:     LinusW <linus.walleij@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Stephen Boyd <swboyd@...omium.org>
Subject: Re: [PATCH] pinctrl: qcom: sc7180: Add new qup functions

Hi,

On Tue, Dec 10, 2019 at 1:14 AM Rajendra Nayak <rnayak@...eaurora.org> wrote:
>
> on sc7180 we have cases where multiple functions from the same
> qup instance share the same pin. This is true for qup02/04/11 and qup13.
> Add new function names to distinguish which qup function to use.
>
> Reported-by: Stephen Boyd <swboyd@...omium.org>
> Signed-off-by: Rajendra Nayak <rnayak@...eaurora.org>
> ---
>  drivers/pinctrl/qcom/pinctrl-sc7180.c | 60 +++++++++++++++++++++++------------
>  1 file changed, 40 insertions(+), 20 deletions(-)

Two overall issues:

1. I think you also need to update the device tree bindings, so this
should be a 2-patch series.  Those list all possible values for
"function" so you need to update.

2. It would be nice if you mentioned in the commit message that this
will break i2c usage on these QUPs for anyone using old device tree
files.  That shouldn't be a problem (AKA no need to provide backward
compatibility) since I think the main sc7180.dtsi hasn't landed
anywhere yet, but if anyone pulled an early patch from the list it
would be good to give them a heads up in your commit message.

> @@ -976,8 +996,8 @@ static const struct msm_pingroup sc7180_groups[] = {
>         [3] = PINGROUP(3, SOUTH, qup01, sp_cmu, dbg_out, qdss_cti, _, _, _, _, _),
>         [4] = PINGROUP(4, NORTH, sdc1_tb, _, qdss_cti, _, _, _, _, _, _),
>         [5] = PINGROUP(5, NORTH, sdc2_tb, _, _, _, _, _, _, _, _),
> -       [6] = PINGROUP(6, NORTH, qup11, qup11, _, _, _, _, _, _, _),
> -       [7] = PINGROUP(7, NORTH, qup11, qup11, ddr_bist, _, _, _, _, _, _),
> +       [6] = PINGROUP(6, NORTH, qup11_i2c, qup11_uart, _, _, _, _, _, _, _),
> +       [7] = PINGROUP(7, NORTH, qup11_i2c, qup11_uart, ddr_bist, _, _, _, _, _, _),

You probably have a more complete document than I have.  ...but the
one I'm looking at shows that for pins 6/7 only i2c is available, not
UART.  Said another way: I see QUP1_L0[1] and QUP1_L1[1] on Func1 but
I don't see anything on Func2.  Of course, my document also doesn't
include "ddr_bist", so maybe it's just incomplete.

-Doug