[v1,1/2] dt-bindings: i2c: add binding for i2c-hotplug-gpio

  Document device tree schema which describes hot-pluggable via GPIO
i2c bus.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 .../bindings/i2c/i2c-hotplug-gpio.yaml        | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
  

On 19/06/2023 16:36, Svyatoslav Ryhel wrote:
> Document device tree schema which describes hot-pluggable via GPIO
> i2c bus.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  .../bindings/i2c/i2c-hotplug-gpio.yaml        | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
> new file mode 100644
> index 000000000000..74544687a2b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/i2c-hotplug-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO detected hot-plugged I2C bus
> +
> +maintainers:
> +  - Michał Mirosław <mirq-linux@rere.qmqm.pl>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  Driver for hot-plugged I2C busses, where some devices on a bus
> +  are hot-pluggable and their presence is indicated by GPIO line.
> +
> +properties:
> +  $nodename:
> +    pattern: "^i2c-(.*)?"

Drop

> +
> +  compatible:
> +    items:
> +      - const: i2c-hotplug-gpio
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  interrupts-extended:
> +    minItems: 1
> +
> +  detect-gpios:
> +    maxItems: 1
> +
> +  i2c-parent:
> +    maxItems: 1

I don't understand this part. You built it as a complimentary device to
the I2C controller, but there is no such device as "hotplug I2C", right?
The GPIO is part of the controller and this is imaginary (virtual) device?

Otherwise, where does the "detect-gpios" go? To the SoC? Then it is not
a real device...

> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"

Use consistent quotes (' or ").

> +  - interrupts-extended
> +  - detect-gpios
> +  - i2c-parent
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    /*
> +     * Asus Transformers use I2C hotplug for attachable dock keyboard
> +     */
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c-dock {
> +        compatible = "i2c-hotplug-gpio";
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        interrupts-extended = <&gpio 164 IRQ_TYPE_EDGE_BOTH>;
> +        detect-gpios = <&gpio 164 1>;

You forgot define.

> +
> +        i2c-parent = <&{/i2c@7000c400}>;

Use normal phandles/labels like entire DTS, not full paths or node names.

> +    };
> +...

Best regards,
Krzysztof
  

пн, 19 черв. 2023 р. о 17:45 Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> пише:
>
> On 19/06/2023 16:36, Svyatoslav Ryhel wrote:
> > Document device tree schema which describes hot-pluggable via GPIO
> > i2c bus.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  .../bindings/i2c/i2c-hotplug-gpio.yaml        | 68 +++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
> > new file mode 100644
> > index 000000000000..74544687a2b8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
> > @@ -0,0 +1,68 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i2c/i2c-hotplug-gpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GPIO detected hot-plugged I2C bus
> > +
> > +maintainers:
> > +  - Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > +
> > +description: |
>
> Do not need '|' unless you need to preserve formatting.
>
> > +  Driver for hot-plugged I2C buses, where some devices on a bus
> > +  are hot-pluggable and their presence is indicated by GPIO line.
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^i2c-(.*)?"
>
> Drop
>
> > +
> > +  compatible:
> > +    items:
> > +      - const: i2c-hotplug-gpio
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> > +
> > +  interrupts-extended:
> > +    minItems: 1
> > +
> > +  detect-gpios:
> > +    maxItems: 1
> > +
> > +  i2c-parent:
> > +    maxItems: 1
>
> I don't understand this part. You built it as a complimentary device to
> the I2C controller, but there is no such device as "hotplug I2C", right?
> The GPIO is part of the controller and this is imaginary (virtual) device?
>
> Otherwise, where does the "detect-gpios" go? To the SoC? Then it is not
> a real device...
>

This is basically GPIO controlled i2c bus duplication. Transformer has
2 ECs, one for pad and one for dock. They both are present on the i2c
bus, but the dock is not always present. Its presence is determined by
a GPIO.

Once a dock is present, GPIO triggers bus duplication and all devices
described on that bus are probed, same when detaching the dock.
Detecting GPIO and interrupt GPIO is the same GPIO.

> > +
> > +required:
> > +  - compatible
> > +  - "#address-cells"
> > +  - "#size-cells"
>
> Use consistent quotes (' or ").
>
> > +  - interrupts-extended
> > +  - detect-gpios
> > +  - i2c-parent
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    /*
> > +     * Asus Transformers use I2C hotplug for attachable dock keyboard
> > +     */
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    i2c-dock {
> > +        compatible = "i2c-hotplug-gpio";
> > +
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        interrupts-extended = <&gpio 164 IRQ_TYPE_EDGE_BOTH>;
> > +        detect-gpios = <&gpio 164 1>;
>
> You forgot define.
>

Define GPIO name or high/low? May you specify?

Best regards,
Svyatoslav R.

> > +
> > +        i2c-parent = <&{/i2c@7000c400}>;
>
> Use normal phandles/labels like entire DTS, not full paths or node names.
>
> > +    };
> > +...
>
> Best regards,
> Krzysztof
>
  

On Mon, 19 Jun 2023 17:36:10 +0300, Svyatoslav Ryhel wrote:
> Document device tree schema which describes hot-pluggable via GPIO
> i2c bus.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  .../bindings/i2c/i2c-hotplug-gpio.yaml        | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
FATAL ERROR: Can't generate fixup for reference to path &{/i2c@7000c400}
make[1]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/i2c/i2c-hotplug-gpio.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1512: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230619143611.24482-2-clamor95@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.