All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.