Re: [PATCH 1/2] dt-bindings: i2c: add GPIO buses sharing one SCL line

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: i2c: redundant i2c-gpio-shared-scl binding duplicates i2c-gpio
--

commit ee80c5111c8336e97eace1faedb4188f4c454089
Author: Ross Cawston <ross@r-sc.ca>

dt-bindings: i2c: add GPIO buses sharing one SCL line

This commit adds a Device Tree binding for bit-banged I2C buses that share
a single SCL GPIO line across child bus nodes, each with its own SDA GPIO
line.

> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio-shared-scl.yaml b/Documentation/devicetree/bindings/i2c/i2c-gpio-shared-scl.yaml
> new file mode 100644
> index 0000000000000..e725120973ad2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-gpio-shared-scl.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/i2c-gpio-shared-scl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bit-banged I2C buses sharing one SCL line
> +
> +maintainers:
> +  - Ross Cawston <ross@r-sc.ca>
> +
> +description: |
> +  Several I2C buses bit-banged on GPIOs, with one SCL line common to all of
> +  them and one SDA line per bus. Each child node is a separate I2C bus.
> +  Transfers are serialized, so only one bus clocks SCL at a time.
> +
> +properties:
> +  compatible:
> +    const: i2c-gpio-shared-scl
> +
> +  scl-gpios:
> +    description: The shared SCL line, usually open drain.
> +    maxItems: 1
> +
> +  i2c-gpio,delay-us:
> +    description: Half the clock period in microseconds, as for i2c-gpio.
> +    default: 5
> +
> +  i2c-gpio,timeout-ms:
> +    description: Clock stretching timeout in milliseconds, as for i2c-gpio.
> +    default: 100
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^i2c@[0-9a-f]+$":
> +    $ref: /schemas/i2c/i2c-controller.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: Index of the bus, for naming only.
> +        maxItems: 1
> +
> +      sda-gpios:
> +        description: The SDA line of this bus, usually open drain.
> +        maxItems: 1
> +
> +    required:
> +      - reg
> +      - sda-gpios

[Severity: Medium]
Is a separate i2c-gpio-shared-scl binding needed here? The existing
i2c-gpio binding in Documentation/devicetree/bindings/i2c/i2c-gpio.yaml and
the driver in drivers/i2c/busses/i2c-gpio.c already support multiple buses
sharing a single SCL GPIO line (added in commit 645c3d7d07fa ("i2c:
i2c-gpio: Enhance driver for buses with shared SCL") for Realtek switches).

In drivers/i2c/busses/i2c-gpio.c, i2c_gpio_create_scl() and
i2c_gpio_lookup_scl() match the scl-gpios phandle and specifier cells across
standard i2c-gpio nodes:

drivers/i2c/busses/i2c-gpio.c:i2c_gpio_create_scl() {
	if (fwnode) {
		ret = fwnode_property_get_reference_args(fwnode, "scl-gpios",
							 "#gpio-cells", 0, 0, &args);
		if (ret)
			/* try the ancient way */
			ret = fwnode_property_get_reference_args(fwnode, "gpios",
								 "#gpio-cells", 0, 1, &args);

		if (!ret) {
			if (args.nargs >= 1)
				sharable = true;
			else
				fwnode_handle_put(args.fwnode);
		}
	}
}

That existing mechanism refcounts the shared struct gpio_desc and serializes
transfers across all adapters sharing the SCL line via i2c_gpio_lock_ops and
scl->lock, while preserving standard per-bus i2c-gpio properties (such as
per-bus i2c-gpio,delay-us, i2c-gpio,timeout-ms, i2c-gpio,sda-output-only,
and i2c-gpio,scl-output-only).

Also, defining a virtual parent container node and child reg properties
described as "Index of the bus, for naming only" runs counter to
Documentation/devicetree/bindings/writing-bindings.rst regarding driver-only
container nodes and arbitrary software instance indices. Could standard
i2c-gpio nodes referencing the same scl-gpios phandle be used instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018194315.945-1-ross@r-sc.ca?part=1