[v2,2/4] dt-bindings: pinctrl: qcom,tlmm-common: document i2c pull property

Commit Message

Abel Vesa Nov. 23, 2022, 3:19 p.m. UTC

From: Neil Armstrong <[e-mail]>

Document the new i2c_pull property introduced for SM8550 setting
an I2C specific pull mode on I2C able pins.

Signed-off-by: Neil Armstrong <[e-mail]>
Signed-off-by: Abel Vesa <[e-mail]>
---

Changes since v1:
 * Added my missing Signed-off-by tag
 * chaged i2c to I2C in qcom,i2c-pull description

 .../devicetree/bindings/pinctrl/qcom,tlmm-common.yaml          | 3 +++
 1 file changed, 3 insertions(+)

Comments

Krzysztof Kozlowski Nov. 24, 2022, 9:58 a.m. UTC | #1

On 23/11/2022 16:19, Abel Vesa wrote:
> From: Neil Armstrong <[e-mail]>
> 
> Document the new i2c_pull property introduced for SM8550 setting
> an I2C specific pull mode on I2C able pins.
> 
> Signed-off-by: Neil Armstrong <[e-mail]>
> Signed-off-by: Abel Vesa <[e-mail]>

Reviewed-by: Krzysztof Kozlowski <[e-mail]>

Best regards,
Krzysztof

Linus Walleij Nov. 24, 2022, 1:11 p.m. UTC | #2

On Wed, Nov 23, 2022 at 4:20 PM Abel Vesa <[e-mail]> wrote:

> From: Neil Armstrong <[e-mail]>
>
> Document the new i2c_pull property introduced for SM8550 setting
> an I2C specific pull mode on I2C able pins.
>
> Signed-off-by: Neil Armstrong <[e-mail]>
> Signed-off-by: Abel Vesa <[e-mail]>
(...)
> +      qcom,i2c-pull:
> +        type: boolean
> +        description: enable bias pull feature designed for I2C on pin

But what is this?

I2C buses are usually just plain old bias-high-impedance, high-z
or open drain, wire-or or whatever you want to call it.

But now there is some special i2c mode, huh?

The description is pretty much "it is what it is"... can we have
some explanation about what this means electrically speaking
and why you cannot use bias-high-impedance?

Yours,
Linus Walleij

Neil Armstrong Nov. 24, 2022, 1:24 p.m. UTC | #3

Hi Linus,

On 24/11/2022 14:11, Linus Walleij wrote:
> On Wed, Nov 23, 2022 at 4:20 PM Abel Vesa <[e-mail]> wrote:
> 
>> From: Neil Armstrong <[e-mail]>
>>
>> Document the new i2c_pull property introduced for SM8550 setting
>> an I2C specific pull mode on I2C able pins.
>>
>> Signed-off-by: Neil Armstrong <[e-mail]>
>> Signed-off-by: Abel Vesa <[e-mail]>
> (...)
>> +      qcom,i2c-pull:
>> +        type: boolean
>> +        description: enable bias pull feature designed for I2C on pin
> 
> But what is this?
> 
> I2C buses are usually just plain old bias-high-impedance, high-z
> or open drain, wire-or or whatever you want to call it.
> 
> But now there is some special i2c mode, huh?
> 
> The description is pretty much "it is what it is"... can we have
> some explanation about what this means electrically speaking
> and why you cannot use bias-high-impedance?

I'll try to get some more info, but so far I only found what I wrote in the bindings.

Neil

> 
> Yours,
> Linus Walleij

Linus Walleij Nov. 25, 2022, 9:47 a.m. UTC | #4

On Thu, Nov 24, 2022 at 2:24 PM Neil Armstrong
<[e-mail]> wrote:
> On 24/11/2022 14:11, Linus Walleij wrote:
> > On Wed, Nov 23, 2022 at 4:20 PM Abel Vesa <[e-mail]> wrote:
> >
> >> From: Neil Armstrong <[e-mail]>
> >>
> >> Document the new i2c_pull property introduced for SM8550 setting
> >> an I2C specific pull mode on I2C able pins.
> >>
> >> Signed-off-by: Neil Armstrong <[e-mail]>
> >> Signed-off-by: Abel Vesa <[e-mail]>
> > (...)
> >> +      qcom,i2c-pull:
> >> +        type: boolean
> >> +        description: enable bias pull feature designed for I2C on pin
> >
> > But what is this?
> >
> > I2C buses are usually just plain old bias-high-impedance, high-z
> > or open drain, wire-or or whatever you want to call it.
> >
> > But now there is some special i2c mode, huh?
> >
> > The description is pretty much "it is what it is"... can we have
> > some explanation about what this means electrically speaking
> > and why you cannot use bias-high-impedance?
>
> I'll try to get some more info, but so far I only found what I wrote in the bindings.

Björn: can you see if you can get some clarity about the i2c
bias thing?

Yours,
Linus Walleij

Neil Armstrong Nov. 25, 2022, 12:40 p.m. UTC | #5

On 25/11/2022 10:47, Linus Walleij wrote:
> On Thu, Nov 24, 2022 at 2:24 PM Neil Armstrong
> <[e-mail]> wrote:
>> On 24/11/2022 14:11, Linus Walleij wrote:
>>> On Wed, Nov 23, 2022 at 4:20 PM Abel Vesa <[e-mail]> wrote:
>>>
>>>> From: Neil Armstrong <[e-mail]>
>>>>
>>>> Document the new i2c_pull property introduced for SM8550 setting
>>>> an I2C specific pull mode on I2C able pins.
>>>>
>>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>>> Signed-off-by: Abel Vesa <[e-mail]>
>>> (...)
>>>> +      qcom,i2c-pull:
>>>> +        type: boolean
>>>> +        description: enable bias pull feature designed for I2C on pin
>>>
>>> But what is this?
>>>
>>> I2C buses are usually just plain old bias-high-impedance, high-z
>>> or open drain, wire-or or whatever you want to call it.
>>>
>>> But now there is some special i2c mode, huh?
>>>
>>> The description is pretty much "it is what it is"... can we have
>>> some explanation about what this means electrically speaking
>>> and why you cannot use bias-high-impedance?
>>
>> I'll try to get some more info, but so far I only found what I wrote in the bindings.
> 
> Björn: can you see if you can get some clarity about the i2c
> bias thing?

As I understood, it enables an "I2C resistor" on the pin, removing the need
of an external pull-up resistor on the line.

I assume the classical pull-up bias is not strong enough to replace an actual
resistor on the PCB.

Neil

> 
> Yours,
> Linus Walleij

Linus Walleij Nov. 26, 2022, 9:53 p.m. UTC | #6

On Fri, Nov 25, 2022 at 1:40 PM <[e-mail]> wrote:

> As I understood, it enables an "I2C resistor" on the pin, removing the need
> of an external pull-up resistor on the line.
>
> I assume the classical pull-up bias is not strong enough to replace an actual
> resistor on the PCB.

In that case I think this should be an argument to bias-pull-up like:

bias-pull-up = <360000>;

Nominally the pull up is in ohms:

  bias-pull-up:
    oneOf:
      - type: boolean
      - $ref: /schemas/types.yaml#/definitions/uint32
    description: pull up the pin. Takes as optional argument on hardware
      supporting it the pull strength in Ohm.

Then the driver can choose to shunt in this extra I2C resistance
from the resistance passed as argument. So no special property
is needed, provided you can get an idea about the resistance
provided here.

Yours,
Linus Walleij

Neil Armstrong Nov. 29, 2022, 8:15 a.m. UTC | #7

Hi Linus,

On 26/11/2022 22:53, Linus Walleij wrote:
> On Fri, Nov 25, 2022 at 1:40 PM <[e-mail]> wrote:
> 
>> As I understood, it enables an "I2C resistor" on the pin, removing the need
>> of an external pull-up resistor on the line.
>>
>> I assume the classical pull-up bias is not strong enough to replace an actual
>> resistor on the PCB.
> 
> In that case I think this should be an argument to bias-pull-up like:
> 
> bias-pull-up = <360000>;
> 
> Nominally the pull up is in ohms:
> 
>    bias-pull-up:
>      oneOf:
>        - type: boolean
>        - $ref: /schemas/types.yaml#/definitions/uint32
>      description: pull up the pin. Takes as optional argument on hardware
>        supporting it the pull strength in Ohm.
> 
> Then the driver can choose to shunt in this extra I2C resistance
> from the resistance passed as argument. So no special property
> is needed, provided you can get an idea about the resistance
> provided here.

I like this alternative, I'll try to figure out if we can find a value
to match against.

Thanks,
Neil

> 
> Yours,
> Linus Walleij

Bjorn Andersson Dec. 15, 2022, 8:49 p.m. UTC | #8

On Tue, Nov 29, 2022 at 09:15:02AM +0100, Neil Armstrong wrote:
> Hi Linus,
> 
> On 26/11/2022 22:53, Linus Walleij wrote:
> > On Fri, Nov 25, 2022 at 1:40 PM <[e-mail]> wrote:
> > 
> > > As I understood, it enables an "I2C resistor" on the pin, removing the need
> > > of an external pull-up resistor on the line.
> > > 
> > > I assume the classical pull-up bias is not strong enough to replace an actual
> > > resistor on the PCB.

That is correct.

> > 
> > In that case I think this should be an argument to bias-pull-up like:
> > 
> > bias-pull-up = <360000>;
> > 
> > Nominally the pull up is in ohms:
> > 
> >    bias-pull-up:
> >      oneOf:
> >        - type: boolean
> >        - $ref: /schemas/types.yaml#/definitions/uint32
> >      description: pull up the pin. Takes as optional argument on hardware
> >        supporting it the pull strength in Ohm.
> > 
> > Then the driver can choose to shunt in this extra I2C resistance
> > from the resistance passed as argument. So no special property
> > is needed, provided you can get an idea about the resistance
> > provided here.
> 
> I like this alternative, I'll try to figure out if we can find a value
> to match against.
> 

The typical value for this resistor is 2.2kOhm.

Regards,
Bjorn

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml
index e1354f0c64f8..9a809a69bd23 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml
@@ -77,6 +77,9 @@  $defs:
       input-enable: true
       output-high: true
       output-low: true
+      qcom,i2c-pull:
+        type: boolean
+        description: enable bias pull feature designed for I2C on pin
 
     allOf:
       - $ref: pincfg-node.yaml#