Re: [PATCH 2/2] dt-bindings: i2c: arb-gpio-challange: convert to DT schema

[Peter Rosin]

Hi!

2023-07-22 at 11:57, Krzysztof Kozlowski wrote:
> Convert the bindings for GPIO-based I2C Arbitration Using a Challenge &
> Response Mechanism to DT schema.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Acked-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> 
> ---
> 
> The text of original bindings was written by Doug, so please kindly ack
> if you agree to relicense it from GPL-2 to (GPL-2.0-only OR
> BSD-2-Clause).
> 
> Changes in v2:
> 1. Drop i2c-controller $ref in top-level part, because only children are
>    I2C controllers.
> 2. Add Ack.
> ---
>  .../bindings/i2c/i2c-arb-gpio-challenge.txt   |  82 -----------
>  .../bindings/i2c/i2c-arb-gpio-challenge.yaml  | 135 ++++++++++++++++++
>  .../devicetree/bindings/i2c/i2c-arb.txt       |  35 -----
>  3 files changed, 135 insertions(+), 117 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-arb.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt b/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
> deleted file mode 100644
> index 548a73cde796..000000000000
> --- a/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
> +++ /dev/null
> @@ -1,82 +0,0 @@
> -GPIO-based I2C Arbitration Using a Challenge & Response Mechanism
> -=================================================================
> -This uses GPIO lines and a challenge & response mechanism to arbitrate who is
> -the master of an I2C bus in a multimaster situation.
> -
> -In many cases using GPIOs to arbitrate is not needed and a design can use
> -the standard I2C multi-master rules.  Using GPIOs is generally useful in
> -the case where there is a device on the bus that has errata and/or bugs
> -that makes standard multimaster mode not feasible.
> -
> -Note that this scheme works well enough but has some downsides:
> -* It is nonstandard (not using standard I2C multimaster)
> -* Having two masters on a bus in general makes it relatively hard to debug
> -  problems (hard to tell if i2c issues were caused by one master, another, or
> -  some device on the bus).
> -
> -
> -Algorithm:
> -
> -All masters on the bus have a 'bus claim' line which is an output that the
> -others can see. These are all active low with pull-ups enabled.  We'll
> -describe these lines as:
> -
> -- OUR_CLAIM: output from us signaling to other hosts that we want the bus
> -- THEIR_CLAIMS: output from others signaling that they want the bus
> -
> -The basic algorithm is to assert your line when you want the bus, then make
> -sure that the other side doesn't want it also.  A detailed explanation is best
> -done with an example.
> -
> -Let's say we want to claim the bus.  We:
> -1. Assert OUR_CLAIM.
> -2. Waits a little bit for the other sides to notice (slew time, say 10
> -   microseconds).
> -3. Check THEIR_CLAIMS.  If none are asserted then the we have the bus and we are
> -   done.
> -4. Otherwise, wait for a few milliseconds and see if THEIR_CLAIMS are released.
> -5. If not, back off, release the claim and wait for a few more milliseconds.
> -6. Go back to 1 (until retry time has expired).
> -
> -
> -Required properties:
> -- compatible: i2c-arb-gpio-challenge
> -- our-claim-gpio: The GPIO that we use to claim the bus.
> -- their-claim-gpios: The GPIOs that the other sides use to claim the bus.
> -  Note that some implementations may only support a single other master.
> -- I2C arbitration bus node. See i2c-arb.txt in this directory.
> -
> -Optional properties:
> -- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 us.
> -- wait-retry-us: we'll attempt another claim after this many microseconds.
> -    Default is 3000 us.
> -- wait-free-us: we'll give up after this many microseconds. Default is 50000 us.
> -
> -
> -Example:
> -	i2c@12ca0000 {
> -		compatible = "acme,some-i2c-device";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -	};
> -
> -	i2c-arbitrator {
> -		compatible = "i2c-arb-gpio-challenge";
> -
> -		i2c-parent = <&{/i2c@12CA0000}>;
> -
> -		our-claim-gpio = <&gpf0 3 1>;
> -		their-claim-gpios = <&gpe0 4 1>;
> -		slew-delay-us = <10>;
> -		wait-retry-us = <3000>;
> -		wait-free-us = <50000>;
> -
> -		i2c-arb {
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -
> -			i2c@52 {
> -				// Normal I2C device
> -			};
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.yaml b/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.yaml
> new file mode 100644
> index 000000000000..5bf6ce14c2dc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.yaml
> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/i2c-arb-gpio-challenge.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO-based I2C Arbitration Using a Challenge & Response Mechanism
> +
> +maintainers:
> +  - Doug Anderson <dianders@xxxxxxxxxxxx>
> +  - Peter Rosin <peda@xxxxxxxxxx>
> +
> +description: |
> +  This uses GPIO lines and a challenge & response mechanism to arbitrate who is
> +  the master of an I2C bus in a multimaster situation.
> +
> +  In many cases using GPIOs to arbitrate is not needed and a design can use the
> +  standard I2C multi-master rules.  Using GPIOs is generally useful in the case
> +  where there is a device on the bus that has errata and/or bugs that makes
> +  standard multimaster mode not feasible.
> +
> +  Note that this scheme works well enough but has some downsides:
> +   * It is nonstandard (not using standard I2C multimaster)
> +   * Having two masters on a bus in general makes it relatively hard to debug
> +     problems (hard to tell if i2c issues were caused by one master, another,
> +     or some device on the bus).
> +
> +  Algorithm:
> +  All masters on the bus have a 'bus claim' line which is an output that the
> +  others can see. These are all active low with pull-ups enabled.  We'll
> +  describe these lines as:
> +   * OUR_CLAIM: output from us signaling to other hosts that we want the bus
> +   * THEIR_CLAIMS: output from others signaling that they want the bus
> +
> +  The basic algorithm is to assert your line when you want the bus, then make
> +  sure that the other side doesn't want it also.  A detailed explanation is
> +  best done with an example.
> +
> +  Let's say we want to claim the bus.  We:
> +  1. Assert OUR_CLAIM.
> +  2. Waits a little bit for the other sides to notice (slew time, say 10
> +     microseconds).
> +  3. Check THEIR_CLAIMS.  If none are asserted then the we have the bus and we
> +     are done.
> +  4. Otherwise, wait for a few milliseconds and see if THEIR_CLAIMS are released.
> +  5. If not, back off, release the claim and wait for a few more milliseconds.
> +  6. Go back to 1 (until retry time has expired).
> +
> +properties:
> +  compatible:
> +    const: i2c-arb-gpio-challenge
> +
> +  i2c-parent:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      The I2C bus that this multiplexer's master-side port is connected to.
> +
> +  our-claim-gpios:
> +    maxItems: 1
> +    description:
> +      The GPIO that we use to claim the bus.
> +
> +  slew-delay-us:
> +    default: 10
> +    description:
> +      Time to wait for a GPIO to go high.
> +
> +  their-claim-gpios:
> +    minItems: 1
> +    maxItems: 2

I don't think there should be a max here? There is no reason for not
supporting more than 2 other masters. True, the current Linux driver
happens to only support 1 other master. So if there should be a max,
I guess it should be 1? But that feels like an implementation
detail of the driver. The Linux driver bails out with an error if
there are more than one other master, it's the only thing it does
with that 2nd 'their-claim-goios' ref.

Cheers,
Peter

> +    description:
> +      The GPIOs that the other sides use to claim the bus.  Note that some
> +      implementations may only support a single other master.
> +
> +  wait-free-us:
> +    default: 50000
> +    description:
> +      We'll give up after this many microseconds.
> +
> +  wait-retry-us:
> +    default: 3000
> +    description:
> +      We'll attempt another claim after this many microseconds.
> +
> +  i2c-arb:
> +    type: object
> +    $ref: /schemas/i2c/i2c-controller.yaml
> +    unevaluatedProperties: false
> +    description:
> +      I2C arbitration bus node.
> +
> +required:
> +  - compatible
> +  - i2c-arb
> +  - our-claim-gpios
> +  - their-claim-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c-arbitrator {
> +        compatible = "i2c-arb-gpio-challenge";
> +        i2c-parent = <&i2c_4>;
> +
> +        our-claim-gpios = <&gpf0 3 GPIO_ACTIVE_LOW>;
> +        their-claim-gpios = <&gpe0 4 GPIO_ACTIVE_LOW>;
> +        slew-delay-us = <10>;
> +        wait-retry-us = <3000>;
> +        wait-free-us = <50000>;
> +
> +        i2c-arb {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            sbs-battery@b {
> +                compatible = "sbs,sbs-battery";
> +                reg = <0xb>;
> +                sbs,poll-retry-count = <1>;
> +            };
> +
> +            embedded-controller@1e {
> +                compatible = "google,cros-ec-i2c";
> +                reg = <0x1e>;
> +                interrupts = <6 IRQ_TYPE_LEVEL_HIGH>;
> +                interrupt-parent = <&gpx1>;
> +                pinctrl-names = "default";
> +                pinctrl-0 = <&ec_irq>;
> +                wakeup-source;
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-arb.txt b/Documentation/devicetree/bindings/i2c/i2c-arb.txt
> deleted file mode 100644
> index 59abf9277bdc..000000000000
> --- a/Documentation/devicetree/bindings/i2c/i2c-arb.txt
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -Common i2c arbitration bus properties.
> -
> -- i2c-arb child node
> -
> -Required properties for the i2c-arb child node:
> -- #address-cells = <1>;
> -- #size-cells = <0>;
> -
> -Optional properties for i2c-arb child node:
> -- Child nodes conforming to i2c bus binding
> -
> -
> -Example :
> -
> -	/*
> -	   An NXP pca9541 I2C bus master selector at address 0x74
> -	   with a NXP pca8574 GPIO expander attached.
> -	 */
> -
> -	arb@74 {
> -		compatible = "nxp,pca9541";
> -		reg = <0x74>;
> -
> -		i2c-arb {
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -
> -			gpio@38 {
> -				compatible = "nxp,pca8574";
> -				reg = <0x38>;
> -				#gpio-cells = <2>;
> -				gpio-controller;
> -			};
> -		};
> -	};