* [PATCH v1] arm64: dts: qcom: msm8998: Add i2c5 pins @ 2019-04-25 16:06 Marc Gonzalez 2019-04-27 4:51 ` Bjorn Andersson 0 siblings, 1 reply; 5+ messages in thread From: Marc Gonzalez @ 2019-04-25 16:06 UTC (permalink / raw) To: Jeffrey Hugo, Bjorn Andersson, Andy Gross Cc: MSM, gpio, Linus Walleij, Rob Herring, Mark Rutland Downstream source: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi?h=LE.UM.1.3.r3.25#n165 Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> --- arch/arm64/boot/dts/qcom/msm8998-pins.dtsi | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi index 6db70acd38ee..d0a95c70d1e7 100644 --- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi @@ -2,6 +2,13 @@ /* Copyright (c) 2018, The Linux Foundation. All rights reserved. */ &tlmm { + i2c5_default: i2c5_default { + pins = "gpio87", "gpio88"; + function = "blsp_i2c5"; + drive-strength = <2>; + bias-disable; + }; + sdc2_clk_on: sdc2_clk_on { config { pins = "sdc2_clk"; -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1] arm64: dts: qcom: msm8998: Add i2c5 pins 2019-04-25 16:06 [PATCH v1] arm64: dts: qcom: msm8998: Add i2c5 pins Marc Gonzalez @ 2019-04-27 4:51 ` Bjorn Andersson 2019-04-29 8:38 ` Marc Gonzalez 0 siblings, 1 reply; 5+ messages in thread From: Bjorn Andersson @ 2019-04-27 4:51 UTC (permalink / raw) To: Marc Gonzalez Cc: Jeffrey Hugo, Andy Gross, MSM, gpio, Linus Walleij, Rob Herring, Mark Rutland On Thu 25 Apr 09:06 PDT 2019, Marc Gonzalez wrote: > Downstream source: > https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi?h=LE.UM.1.3.r3.25#n165 > > Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> > --- > arch/arm64/boot/dts/qcom/msm8998-pins.dtsi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > index 6db70acd38ee..d0a95c70d1e7 100644 > --- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > @@ -2,6 +2,13 @@ > /* Copyright (c) 2018, The Linux Foundation. All rights reserved. */ > > &tlmm { > + i2c5_default: i2c5_default { You need to reference this node for it to make a difference. Also the drive-strength and bias are board specific, so please move this to your board dts (and reference the node). Regards, Bjorn > + pins = "gpio87", "gpio88"; > + function = "blsp_i2c5"; > + drive-strength = <2>; > + bias-disable; > + }; > + > sdc2_clk_on: sdc2_clk_on { > config { > pins = "sdc2_clk"; > -- > 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] arm64: dts: qcom: msm8998: Add i2c5 pins 2019-04-27 4:51 ` Bjorn Andersson @ 2019-04-29 8:38 ` Marc Gonzalez 2019-05-02 15:12 ` Bjorn Andersson 0 siblings, 1 reply; 5+ messages in thread From: Marc Gonzalez @ 2019-04-29 8:38 UTC (permalink / raw) To: Bjorn Andersson, Linus Walleij Cc: Jeffrey Hugo, Andy Gross, MSM, gpio, Rob Herring, Mark Rutland On 27/04/2019 06:51, Bjorn Andersson wrote: > On Thu 25 Apr 09:06 PDT 2019, Marc Gonzalez wrote: > >> Downstream source: >> https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi?h=LE.UM.1.3.r3.25#n165 >> >> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> >> --- >> arch/arm64/boot/dts/qcom/msm8998-pins.dtsi | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi >> index 6db70acd38ee..d0a95c70d1e7 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi >> @@ -2,6 +2,13 @@ >> /* Copyright (c) 2018, The Linux Foundation. All rights reserved. */ >> >> &tlmm { >> + i2c5_default: i2c5_default { >> + pins = "gpio87", "gpio88"; >> + function = "blsp_i2c5"; >> + drive-strength = <2>; >> + bias-disable; >> + }; > > You need to reference this node for it to make a difference. Right. I do have a local board file referencing i2c5_default, which I plan to submit at some point. It contains: &blsp1_i2c5 { status = "ok"; clock-frequency = <100000>; pinctrl-names = "default"; pinctrl-0 = <&i2c5_default>; }; > Also the drive-strength and bias are board specific, so please move this > to your board dts (and reference the node). Wait... Are you saying there should be no drive-strength nor bias definitions inside msm8998-pins.dtsi? $ grep -c 'strength\|bias' arch/arm64/boot/dts/qcom/msm8998-pins.dtsi 18 Why are the SDHC pins different than the I2C pins? i2c5 is "tied" to gpio87 and gpio88. Could my board designer "reassign" these pins to a different HW block? Or is that immutable? Regards. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] arm64: dts: qcom: msm8998: Add i2c5 pins 2019-04-29 8:38 ` Marc Gonzalez @ 2019-05-02 15:12 ` Bjorn Andersson 2019-06-26 16:20 ` Marc Gonzalez 0 siblings, 1 reply; 5+ messages in thread From: Bjorn Andersson @ 2019-05-02 15:12 UTC (permalink / raw) To: Marc Gonzalez Cc: Linus Walleij, Jeffrey Hugo, Andy Gross, MSM, gpio, Rob Herring, Mark Rutland On Mon 29 Apr 01:38 PDT 2019, Marc Gonzalez wrote: > On 27/04/2019 06:51, Bjorn Andersson wrote: > > > On Thu 25 Apr 09:06 PDT 2019, Marc Gonzalez wrote: > > > >> Downstream source: > >> https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi?h=LE.UM.1.3.r3.25#n165 > >> > >> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> > >> --- > >> arch/arm64/boot/dts/qcom/msm8998-pins.dtsi | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > >> index 6db70acd38ee..d0a95c70d1e7 100644 > >> --- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > >> +++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > >> @@ -2,6 +2,13 @@ > >> /* Copyright (c) 2018, The Linux Foundation. All rights reserved. */ > >> > >> &tlmm { > >> + i2c5_default: i2c5_default { > >> + pins = "gpio87", "gpio88"; > >> + function = "blsp_i2c5"; > >> + drive-strength = <2>; > >> + bias-disable; > >> + }; > > > > You need to reference this node for it to make a difference. > > Right. I do have a local board file referencing i2c5_default, which I plan > to submit at some point. It contains: > > &blsp1_i2c5 { > status = "ok"; > clock-frequency = <100000>; > pinctrl-names = "default"; > pinctrl-0 = <&i2c5_default>; > }; > > > Also the drive-strength and bias are board specific, so please move this > > to your board dts (and reference the node). > > Wait... Are you saying there should be no drive-strength nor bias definitions > inside msm8998-pins.dtsi? > > $ grep -c 'strength\|bias' arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > 18 > > Why are the SDHC pins different than the I2C pins? > > i2c5 is "tied" to gpio87 and gpio88. Could my board designer "reassign" > these pins to a different HW block? Or is that immutable? > Right, so it makes a lot of sense to have a node in msm8998.dtsi that says that if i2c5 is probed then the associated pinmux should be set up. But the pinconf (drive-strenght, internal vs external bias) are board specific, so this part better go in the board.dts. On sdm845 we put a node with pinmux in the platform.dtsi and then in the board we extend this node with the electrical properties of the board. This works out pretty well, but we haven't gone back and updated the older platforms/boards yet. Regards, Bjorn ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] arm64: dts: qcom: msm8998: Add i2c5 pins 2019-05-02 15:12 ` Bjorn Andersson @ 2019-06-26 16:20 ` Marc Gonzalez 0 siblings, 0 replies; 5+ messages in thread From: Marc Gonzalez @ 2019-06-26 16:20 UTC (permalink / raw) To: Bjorn Andersson Cc: Linus Walleij, Jeffrey Hugo, Andy Gross, MSM, gpio, Rob Herring, Mark Rutland, I2C On 02/05/2019 17:12, Bjorn Andersson wrote: > On Mon 29 Apr 01:38 PDT 2019, Marc Gonzalez wrote: > >> On 27/04/2019 06:51, Bjorn Andersson wrote: >> >>> On Thu 25 Apr 09:06 PDT 2019, Marc Gonzalez wrote: >>> >>>> Downstream source: >>>> https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi?h=LE.UM.1.3.r3.25#n165 >>>> >>>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr> >>>> --- >>>> arch/arm64/boot/dts/qcom/msm8998-pins.dtsi | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi >>>> index 6db70acd38ee..d0a95c70d1e7 100644 >>>> --- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi >>>> +++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi >>>> @@ -2,6 +2,13 @@ >>>> /* Copyright (c) 2018, The Linux Foundation. All rights reserved. */ >>>> >>>> &tlmm { >>>> + i2c5_default: i2c5_default { >>>> + pins = "gpio87", "gpio88"; >>>> + function = "blsp_i2c5"; >>>> + drive-strength = <2>; >>>> + bias-disable; >>>> + }; >>> >>> You need to reference this node for it to make a difference. >> >> Right. I do have a local board file referencing i2c5_default, which I plan >> to submit at some point. It contains: >> >> &blsp1_i2c5 { >> status = "ok"; >> clock-frequency = <100000>; >> pinctrl-names = "default"; >> pinctrl-0 = <&i2c5_default>; >> }; >> >>> Also the drive-strength and bias are board specific, so please move this >>> to your board dts (and reference the node). >> >> Wait... Are you saying there should be no drive-strength nor bias definitions >> inside msm8998-pins.dtsi? >> >> $ grep -c 'strength\|bias' arch/arm64/boot/dts/qcom/msm8998-pins.dtsi >> 18 >> >> Why are the SDHC pins different than the I2C pins? >> >> i2c5 is "tied" to gpio87 and gpio88. Could my board designer "reassign" >> these pins to a different HW block? Or is that immutable? >> > > Right, so it makes a lot of sense to have a node in msm8998.dtsi that > says that if i2c5 is probed then the associated pinmux should be set up. > > But the pinconf (drive-strenght, internal vs external bias) are board > specific, so this part better go in the board.dts. > > > On sdm845 we put a node with pinmux in the platform.dtsi and then in the > board we extend this node with the electrical properties of the board. > This works out pretty well, but we haven't gone back and updated the > older platforms/boards yet. Wow, I had completely lost track of this thread... OK, I think what you had in mind is the following: (Please confirm before I spin a v2) diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi index f09f3e03f708..9cd1f96dc3c8 100644 --- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi @@ -27,6 +27,18 @@ status = "okay"; }; +&blsp1_i2c5 { + status = "ok"; + clock-frequency = <100000>; /*** NOT SURE... This depends on which devices are on the I2C bus? ***/ + pinctrl-names = "default"; + pinctrl-0 = <&i2c5_default>; +}; + +&i2c5_default { + drive-strength = <2>; + bias-disable; +}; + &qusb2phy { status = "okay"; diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi index 6db70acd38ee..dad175a52d03 100644 --- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi @@ -2,6 +2,11 @@ /* Copyright (c) 2018, The Linux Foundation. All rights reserved. */ &tlmm { + i2c5_default: i2c5-default { + pins = "gpio87", "gpio88"; + function = "blsp_i2c5"; + }; + sdc2_clk_on: sdc2_clk_on { config { pins = "sdc2_clk"; Well, except that there don't seem to be any devices on the i2c5 bus on the mediabox... # i2cdetect -r 0 i2cdetect: WARNING! This program can confuse your I2C bus Continue? [y/N] y 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- -- -- -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- But there are on several on my batfish board: # i2cdetect -r 0 i2cdetect: WARNING! This program can confuse your I2C bus Continue? [y/N] y 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- -- -- -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- -- 44 -- -- 47 -- -- -- -- -- -- -- -- 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- 68 -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- Can I submit the arch/arm64/boot/dts/qcom/msm8998-pins.dtsi alone? Regards. ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-26 16:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-25 16:06 [PATCH v1] arm64: dts: qcom: msm8998: Add i2c5 pins Marc Gonzalez 2019-04-27 4:51 ` Bjorn Andersson 2019-04-29 8:38 ` Marc Gonzalez 2019-05-02 15:12 ` Bjorn Andersson 2019-06-26 16:20 ` Marc Gonzalez
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.