| Message ID | 20221212182314.1902632-4-bmasney@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: sc8280xp: add i2c and spi nodes | expand |
On 12/12/2022 11:53 PM, Brian Masney wrote: > Add the necessary nodes in order to get qup1_i2c15 and qup2_i2c18 > functioning on the automotive board and exposed to userspace. > > This work was derived from various patches that Qualcomm delivered > to Red Hat in a downstream kernel. This change was validated by using > i2c-tools 4.3.3 on CentOS Stream 9: > > [root@localhost ~]# i2cdetect -l > i2c-15 i2c Geni-I2C I2C adapter > i2c-18 i2c Geni-I2C I2C adapter > > [root@localhost ~]# i2cdetect -a -y 15 > Warning: Can't use SMBus Quick Write command, will skip some addresses > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: > 10: > 20: > 30: -- -- -- -- -- -- -- -- > 40: > 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 60: > 70: > > Bus 18 has the same output. I validated that we get the same output on > the downstream kernel. > > Signed-off-by: Brian Masney <bmasney@redhat.com> > --- > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 46 +++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > index d70859803fbd..6dc3f3ff8ece 100644 > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > @@ -17,6 +17,8 @@ / { > compatible = "qcom,sa8540p-ride", "qcom,sa8540p"; > > aliases { > + i2c15 = &qup1_i2c15; > + i2c18 = &qup2_i2c18; I was listing out all the i2c devices to be enabled apart from above and below are applicable for Qdrive3 as well: i2c0 980000 i2c1 984000 i2c12 0x00A90000 -Shazad > serial0 = &qup2_uart17; > }; > > @@ -188,10 +190,28 @@ &pcie3a_phy { > status = "okay"; > }; > > +&qup1 { > + status = "okay"; > +}; > + > +&qup1_i2c15 { > + pinctrl-names = "default"; > + pinctrl-0 = <&qup1_i2c15_default>; > + > + status = "okay"; > +}; > + > &qup2 { > status = "okay"; > }; > > +&qup2_i2c18 { > + pinctrl-names = "default"; > + pinctrl-0 = <&qup2_i2c18_default>; > + > + status = "okay"; > +}; > + > &qup2_uart17 { > compatible = "qcom,geni-debug-uart"; > status = "okay"; > @@ -313,4 +333,30 @@ wake-pins { > bias-pull-up; > }; > }; > + > + qup1_i2c15_default: qup1-i2c15-state { > + mux-pins { > + pins = "gpio36", "gpio37"; > + function = "qup15"; > + }; > + > + config-pins { > + pins = "gpio36", "gpio37"; > + drive-strength = <0x02>; > + bias-pull-up; > + }; > + }; > + > + qup2_i2c18_default: qup2-i2c18-state { > + mux-pins { > + pins = "gpio66", "gpio67"; > + function = "qup18"; > + }; > + > + config-pins { > + pins = "gpio66", "gpio67"; > + drive-strength = <0x02>; > + bias-pull-up; > + }; > + }; > };
On Tue, Dec 13, 2022 at 03:48:27PM +0100, Konrad Dybcio wrote: > > + qup1_i2c15_default: qup1-i2c15-state { > > + mux-pins { > > + pins = "gpio36", "gpio37"; > > + function = "qup15"; > > + }; > > + > > + config-pins { > > + pins = "gpio36", "gpio37"; > > + drive-strength = <0x02>; > > + bias-pull-up; > > + }; > > + }; > > You can drop mux/config-pins and have the pin properties live directly > under the qup1-i2cN-state node. Hi Konrad (and Shazad below), I need to enable 5 i2c buses (0, 1, 12, 15, 18) on this board. I tried the following combinations with the pin mapping configuration and the only one that seems to work reliably for me is what I originally had. With the following, only 2 out of the 5 buses are detected. There's no i2c mesages in dmesg. i2c0_default: i2c0-default-state { pins = "gpio135", "gpio136"; function = "qup15"; }; Next, I added a drive-strength and bias-pull-up. All 5 buses are detected. One bus throws read errors when I probe it with i2cdetect, two others 'i2cdetect -a -y $BUSNUM' takes ~5 seconds to run, and the remaining two are fast. i2c0_default: i2c0-default-state { pins = "gpio135", "gpio136"; function = "qup15"; drive-strength = <2>; bias-pull-up; }; This is the style where i2cdetect seems to be happy for all 5 buses and is fast: i2c0_default: i2c0-default-state { mux-pins { pins = "gpio135", "gpio136"; function = "qup0"; }; config-pins { pins = "gpio135", "gpio136"; drive-strength = <2>; bias-pull-up; }; }; Shazad: 'i2cdetect -a -y $BUSNUM) shows that all 5 buses have the same addresses listening. Is that expected? That seems a bit odd to me. [root@localhost ~]# i2cdetect -a -y 0 Warning: Can't use SMBus Quick Write command, will skip some addresses 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: 10: 20: 30: -- -- -- -- -- -- -- -- 40: 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: 70: I triple checked that I have the QUP pins defined correctly for the 5 buses. I checked them against what's in the downstream kernel and I also checked them against what's in upstream's drivers/pinctrl/qcom/pinctrl-sc8280xp.c. This is the pin mapping that I have: i2c0: gpio135, gpio136 i2c1: gpio158, gpio159 i2c12: gpio0, gpio1 i2c15: gpio36, gpio37 i2c18: gpio66, gpio67 Brian
On 12/12/2022 19:23, Brian Masney wrote: > Add the necessary nodes in order to get qup1_i2c15 and qup2_i2c18 > functioning on the automotive board and exposed to userspace. > > This work was derived from various patches that Qualcomm delivered > to Red Hat in a downstream kernel. This change was validated by using > i2c-tools 4.3.3 on CentOS Stream 9: > > [root@localhost ~]# i2cdetect -l > i2c-15 i2c Geni-I2C I2C adapter > i2c-18 i2c Geni-I2C I2C adapter > > [root@localhost ~]# i2cdetect -a -y 15 > Warning: Can't use SMBus Quick Write command, will skip some addresses > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: > 10: > 20: > 30: -- -- -- -- -- -- -- -- > 40: > 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 60: > 70: > > Bus 18 has the same output. I validated that we get the same output on > the downstream kernel. > > Signed-off-by: Brian Masney <bmasney@redhat.com> > --- > arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 46 +++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > index d70859803fbd..6dc3f3ff8ece 100644 > --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts > @@ -17,6 +17,8 @@ / { > compatible = "qcom,sa8540p-ride", "qcom,sa8540p"; > > aliases { > + i2c15 = &qup1_i2c15; > + i2c18 = &qup2_i2c18; > serial0 = &qup2_uart17; > }; > > @@ -188,10 +190,28 @@ &pcie3a_phy { > status = "okay"; > }; > > +&qup1 { > + status = "okay"; > +}; > + > +&qup1_i2c15 { > + pinctrl-names = "default"; > + pinctrl-0 = <&qup1_i2c15_default>; > + > + status = "okay"; > +}; > + > &qup2 { > status = "okay"; > }; > > +&qup2_i2c18 { > + pinctrl-names = "default"; > + pinctrl-0 = <&qup2_i2c18_default>; > + > + status = "okay"; > +}; > + > &qup2_uart17 { > compatible = "qcom,geni-debug-uart"; > status = "okay"; > @@ -313,4 +333,30 @@ wake-pins { > bias-pull-up; > }; > }; > + > + qup1_i2c15_default: qup1-i2c15-state { > + mux-pins { > + pins = "gpio36", "gpio37"; > + function = "qup15"; > + }; > + > + config-pins { > + pins = "gpio36", "gpio37"; > + drive-strength = <0x02>; Except the problem pointed out by Konrad (we do not have separate mux and config pins anymore), this is not a hex, it's mA. Best regards, Krzysztof
On 14/12/2022 13:30, Brian Masney wrote: > On Tue, Dec 13, 2022 at 03:48:27PM +0100, Konrad Dybcio wrote: >>> + qup1_i2c15_default: qup1-i2c15-state { >>> + mux-pins { >>> + pins = "gpio36", "gpio37"; >>> + function = "qup15"; >>> + }; >>> + >>> + config-pins { >>> + pins = "gpio36", "gpio37"; >>> + drive-strength = <0x02>; >>> + bias-pull-up; >>> + }; >>> + }; >> >> You can drop mux/config-pins and have the pin properties live directly >> under the qup1-i2cN-state node. > > Hi Konrad (and Shazad below), > > I need to enable 5 i2c buses (0, 1, 12, 15, 18) on this board. I tried > the following combinations with the pin mapping configuration and the > only one that seems to work reliably for me is what I originally had. > > With the following, only 2 out of the 5 buses are detected. There's no > i2c mesages in dmesg. > > i2c0_default: i2c0-default-state { > pins = "gpio135", "gpio136"; > function = "qup15"; > }; > > Next, I added a drive-strength and bias-pull-up. All 5 buses are > detected. One bus throws read errors when I probe it with i2cdetect, two > others 'i2cdetect -a -y $BUSNUM' takes ~5 seconds to run, and the > remaining two are fast. > > i2c0_default: i2c0-default-state { > pins = "gpio135", "gpio136"; > function = "qup15"; > drive-strength = <2>; > bias-pull-up; > }; > > This is the style where i2cdetect seems to be happy for all 5 buses and > is fast: > > i2c0_default: i2c0-default-state { > mux-pins { > pins = "gpio135", "gpio136"; > function = "qup0"; > }; > > config-pins { > pins = "gpio135", "gpio136"; > drive-strength = <2>; > bias-pull-up; > }; > }; > > > Shazad: 'i2cdetect -a -y $BUSNUM) shows that all 5 buses have the same > addresses listening. Is that expected? That seems a bit odd to me. > > [root@localhost ~]# i2cdetect -a -y 0 > Warning: Can't use SMBus Quick Write command, will skip some addresses > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: > 10: > 20: > 30: -- -- -- -- -- -- -- -- > 40: > 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 60: > 70: > > I triple checked that I have the QUP pins defined correctly for the 5 > buses. I checked them against what's in the downstream kernel and I also > checked them against what's in upstream's > drivers/pinctrl/qcom/pinctrl-sc8280xp.c. This is the pin mapping that I What's the base of this kernel? Are you sure you have d21f4b7ffc22? Best regards, Krzysztof
On 14.12.2022 13:30, Brian Masney wrote: > On Tue, Dec 13, 2022 at 03:48:27PM +0100, Konrad Dybcio wrote: >>> + qup1_i2c15_default: qup1-i2c15-state { >>> + mux-pins { >>> + pins = "gpio36", "gpio37"; >>> + function = "qup15"; >>> + }; >>> + >>> + config-pins { >>> + pins = "gpio36", "gpio37"; >>> + drive-strength = <0x02>; >>> + bias-pull-up; >>> + }; >>> + }; >> >> You can drop mux/config-pins and have the pin properties live directly >> under the qup1-i2cN-state node. > > Hi Konrad (and Shazad below), > > I need to enable 5 i2c buses (0, 1, 12, 15, 18) on this board. I tried > the following combinations with the pin mapping configuration and the > only one that seems to work reliably for me is what I originally had. > > With the following, only 2 out of the 5 buses are detected. There's no > i2c mesages in dmesg. > > i2c0_default: i2c0-default-state { > pins = "gpio135", "gpio136"; > function = "qup15"; > }; > > Next, I added a drive-strength and bias-pull-up. All 5 buses are > detected. One bus throws read errors when I probe it with i2cdetect, two > others 'i2cdetect -a -y $BUSNUM' takes ~5 seconds to run, and the > remaining two are fast. > > i2c0_default: i2c0-default-state { > pins = "gpio135", "gpio136"; > function = "qup15"; > drive-strength = <2>; > bias-pull-up; > }; > > This is the style where i2cdetect seems to be happy for all 5 buses and > is fast: > > i2c0_default: i2c0-default-state { > mux-pins { > pins = "gpio135", "gpio136"; > function = "qup0"; > }; > > config-pins { > pins = "gpio135", "gpio136"; > drive-strength = <2>; > bias-pull-up; > }; > }; Unless you made a typo somewhere, I genuinely have no explanation for this.. Konrad > > > Shazad: 'i2cdetect -a -y $BUSNUM) shows that all 5 buses have the same > addresses listening. Is that expected? That seems a bit odd to me. > > [root@localhost ~]# i2cdetect -a -y 0 > Warning: Can't use SMBus Quick Write command, will skip some addresses > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: > 10: > 20: > 30: -- -- -- -- -- -- -- -- > 40: > 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 60: > 70: > > I triple checked that I have the QUP pins defined correctly for the 5 > buses. I checked them against what's in the downstream kernel and I also > checked them against what's in upstream's > drivers/pinctrl/qcom/pinctrl-sc8280xp.c. This is the pin mapping that I > have: > > i2c0: gpio135, gpio136 > i2c1: gpio158, gpio159 > i2c12: gpio0, gpio1 > i2c15: gpio36, gpio37 > i2c18: gpio66, gpio67 > > Brian >
On Wed, Dec 14, 2022 at 01:52:17PM +0100, Krzysztof Kozlowski wrote: > On 14/12/2022 13:30, Brian Masney wrote: > > I triple checked that I have the QUP pins defined correctly for the 5 > > buses. I checked them against what's in the downstream kernel and I also > > checked them against what's in upstream's > > drivers/pinctrl/qcom/pinctrl-sc8280xp.c. This is the pin mapping that I > > What's the base of this kernel? Are you sure you have d21f4b7ffc22? I'm based on top of linux-next-20221208 with no other changes. I have that commit. commit d21f4b7ffc22c009da925046b69b15af08de9d75 Author: Douglas Anderson <dianders@chromium.org> Date: Fri Oct 14 10:33:18 2022 -0700 pinctrl: qcom: Avoid glitching lines when we first mux to output On Wed, Dec 14, 2022 at 01:53:38PM +0100, Konrad Dybcio wrote: > > This is the style where i2cdetect seems to be happy for all 5 buses and > > is fast: > > > > i2c0_default: i2c0-default-state { > > mux-pins { > > pins = "gpio135", "gpio136"; > > function = "qup0"; > > }; > > > > config-pins { > > pins = "gpio135", "gpio136"; > > drive-strength = <2>; > > bias-pull-up; > > }; > > }; > Unless you made a typo somewhere, I genuinely have no explanation for this.. I have my unpublished v2 patch set committed to my tree and a clean tree according to git. I started with the state that I have quoted above. As I did the various tests I described in my last email, I would do a 'git diff' just to be sure that I didn't have any typos. I'll wait to hear from Shazad about whether or not the output that I got from i2cdetect is supposed to be the same for those 5 buses. Brian
On 12/14/2022 6:00 PM, Brian Masney wrote: > On Tue, Dec 13, 2022 at 03:48:27PM +0100, Konrad Dybcio wrote: >>> + qup1_i2c15_default: qup1-i2c15-state { >>> + mux-pins { >>> + pins = "gpio36", "gpio37"; >>> + function = "qup15"; >>> + }; >>> + >>> + config-pins { >>> + pins = "gpio36", "gpio37"; >>> + drive-strength = <0x02>; >>> + bias-pull-up; >>> + }; >>> + }; >> >> You can drop mux/config-pins and have the pin properties live directly >> under the qup1-i2cN-state node. > > Hi Konrad (and Shazad below), > > I need to enable 5 i2c buses (0, 1, 12, 15, 18) on this board. I tried > the following combinations with the pin mapping configuration and the > only one that seems to work reliably for me is what I originally had. > > With the following, only 2 out of the 5 buses are detected. There's no > i2c mesages in dmesg. > > i2c0_default: i2c0-default-state { > pins = "gpio135", "gpio136"; > function = "qup15"; > }; > > Next, I added a drive-strength and bias-pull-up. All 5 buses are > detected. One bus throws read errors when I probe it with i2cdetect, two > others 'i2cdetect -a -y $BUSNUM' takes ~5 seconds to run, and the This I have also observed on downstream as well, where scanning all addresses takes some amount of time near to 5-6 seconds. > remaining two are fast. > > i2c0_default: i2c0-default-state { > pins = "gpio135", "gpio136"; > function = "qup15"; > drive-strength = <2>; > bias-pull-up; > }; > This is the default config we should use. > This is the style where i2cdetect seems to be happy for all 5 buses and > is fast: > > i2c0_default: i2c0-default-state { > mux-pins { > pins = "gpio135", "gpio136"; > function = "qup0"; > }; > > config-pins { > pins = "gpio135", "gpio136"; > drive-strength = <2>; > bias-pull-up; > }; > }; > > > Shazad: 'i2cdetect -a -y $BUSNUM) shows that all 5 buses have the same > addresses listening. Is that expected? That seems a bit odd to me. > Brian, even I haven't checked with all enabled, let me check this on other projects and with downstream as well and get back to you. -Shazad > [root@localhost ~]# i2cdetect -a -y 0 > Warning: Can't use SMBus Quick Write command, will skip some addresses > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: > 10: > 20: > 30: -- -- -- -- -- -- -- -- > 40: > 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 60: > 70: > > I triple checked that I have the QUP pins defined correctly for the 5 > buses. I checked them against what's in the downstream kernel and I also > checked them against what's in upstream's > drivers/pinctrl/qcom/pinctrl-sc8280xp.c. This is the pin mapping that I > have: > > i2c0: gpio135, gpio136 > i2c1: gpio158, gpio159 > i2c12: gpio0, gpio1 > i2c15: gpio36, gpio37 > i2c18: gpio66, gpio67 > > Brian >
On Wed, Dec 14, 2022 at 09:06:48PM +0530, Shazad Hussain wrote: > > i2c0_default: i2c0-default-state { > > pins = "gpio135", "gpio136"; > > function = "qup15"; > > drive-strength = <2>; > > bias-pull-up; > > }; > > > > This is the default config we should use. OK, I'll stick with this config. > > Shazad: 'i2cdetect -a -y $BUSNUM) shows that all 5 buses have the same > > addresses listening. Is that expected? That seems a bit odd to me. > > > > Brian, even I haven't checked with all enabled, let me check this on other > projects and with downstream as well and get back to you. I'll post my v2 in a little bit so that you can test it. Brian
diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts index d70859803fbd..6dc3f3ff8ece 100644 --- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts +++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts @@ -17,6 +17,8 @@ / { compatible = "qcom,sa8540p-ride", "qcom,sa8540p"; aliases { + i2c15 = &qup1_i2c15; + i2c18 = &qup2_i2c18; serial0 = &qup2_uart17; }; @@ -188,10 +190,28 @@ &pcie3a_phy { status = "okay"; }; +&qup1 { + status = "okay"; +}; + +&qup1_i2c15 { + pinctrl-names = "default"; + pinctrl-0 = <&qup1_i2c15_default>; + + status = "okay"; +}; + &qup2 { status = "okay"; }; +&qup2_i2c18 { + pinctrl-names = "default"; + pinctrl-0 = <&qup2_i2c18_default>; + + status = "okay"; +}; + &qup2_uart17 { compatible = "qcom,geni-debug-uart"; status = "okay"; @@ -313,4 +333,30 @@ wake-pins { bias-pull-up; }; }; + + qup1_i2c15_default: qup1-i2c15-state { + mux-pins { + pins = "gpio36", "gpio37"; + function = "qup15"; + }; + + config-pins { + pins = "gpio36", "gpio37"; + drive-strength = <0x02>; + bias-pull-up; + }; + }; + + qup2_i2c18_default: qup2-i2c18-state { + mux-pins { + pins = "gpio66", "gpio67"; + function = "qup18"; + }; + + config-pins { + pins = "gpio66", "gpio67"; + drive-strength = <0x02>; + bias-pull-up; + }; + }; };
Add the necessary nodes in order to get qup1_i2c15 and qup2_i2c18 functioning on the automotive board and exposed to userspace. This work was derived from various patches that Qualcomm delivered to Red Hat in a downstream kernel. This change was validated by using i2c-tools 4.3.3 on CentOS Stream 9: [root@localhost ~]# i2cdetect -l i2c-15 i2c Geni-I2C I2C adapter i2c-18 i2c Geni-I2C I2C adapter [root@localhost ~]# i2cdetect -a -y 15 Warning: Can't use SMBus Quick Write command, will skip some addresses 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: 10: 20: 30: -- -- -- -- -- -- -- -- 40: 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: 70: Bus 18 has the same output. I validated that we get the same output on the downstream kernel. Signed-off-by: Brian Masney <bmasney@redhat.com> --- arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+)