Re: [PATCH] pinctrl: qcom: sc7180: Add new qup functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/10/2019 11:14 PM, Doug Anderson wrote:
Hi,

On Tue, Dec 10, 2019 at 1:14 AM Rajendra Nayak <rnayak@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxx>
Signed-off-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx>
---
  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



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux