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] [day] [month] [year] [list]

Message-ID: <0101016ef336d9c4-d9f221c4-bd59-41e0-a2ba-403880da148d-000000@us-west-2.amazonses.com>
Date:   Wed, 11 Dec 2019 04:28:08 +0000
From:   Rajendra Nayak <rnayak@...eaurora.org>
To:     Doug Anderson <dianders@...omium.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


On 12/10/2019 11:14 PM, Doug Anderson wrote:
> 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.

yes, I will include the bindings update patch when I repost.

> 
> 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.

Sure, will update the commit msg accordingly.

> 
>> @@ -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.

yes, I can confirm I do see QUP1_L2[1] and QUP1_L3[1] on Func2 for gpio 6/7
in my doc.

Thanks for the review.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation