Re: [PATCH] dt-bindings: i2c: add bindings for nxp,pca9541

From: Guenter Roeck
Date: Mon Jun 27 2016 - 09:17:56 EST


On 06/27/2016 03:11 AM, Peter Rosin wrote:
Fill the gap for this pre-existing driver.

Signed-off-by: Peter Rosin <peda@xxxxxxxxxx>
---
.../devicetree/bindings/i2c/i2c-arb-pca9541.txt | 33 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-arb-pca9541.txt

Hi!

I'm wondering about this driver. It is not a trivial device, and yet it
has historically relied on the i2c core matching the chip w/o vendor
prefix. This is not ideal. But what to do about the driver implementing
this in terms of an i2c-mux, somthing which the chip is not; It is an
i2c arbitrator. It just happens to rely on the i2c mux core also handling
i2c gates and i2c arbitrators. But that seems like a Linux detail. So I
don't know what to do here?


The concept of arbitrators didn't exist when I wrote the driver. I would not
have a problem with renaming the file if that is what you are asking for.

That is, the patch - as is - describes something that would be trivial to
support today, but at the same time it seems to be too tied to Linux.

The problem is that the i2c@0 intermediate node is not really needed, but
at the same time removing it would cause a disruption for the driver since
it can't really use the i2c mux core if that node isn't there. I don't
see a simple way to fix that in the i2c mux core either (but admittedly
haven't given it too much thought).


The gpio arbitrator uses the same principle as well. Why not just leave it
alone ? Besides, I think it is a good idea to have it, since it groups
the i2c devices behind the chip together. I would not consider that to be
a Linuxism, but a design choice.

Guenter

Any suggestions?

Cheers,
Peter

PS. The driver source is in drivers/i2c/muxes/i2c-mux-pca9541.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-arb-pca9541.txt b/Documentation/devicetree/bindings/i2c/i2c-arb-pca9541.txt
new file mode 100644
index 000000000000..edbe84935906
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-arb-pca9541.txt
@@ -0,0 +1,33 @@
+* NXP PCA9541 I2C bus master selector
+
+Required Properties:
+
+ - compatible: Must be "nxp,pca9541"
+
+ - reg: The I2C address of the device.
+
+ The following required properties are defined externally:
+
+ - Standard I2C mux properties. See i2c-mux.txt in this directory.
+ - I2C child bus nodes. See i2c-mux.txt in this directory.
+
+
+Example:
+
+ i2c-arbitrator@74 {
+ compatible = "nxp,pca9541";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x74>;
+
+ i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ eeprom@54 {
+ compatible = "at,24c08";
+ reg = <0x54>;
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index e1b090f86e0d..3dd44d0d166c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5521,6 +5521,7 @@ S: Maintained
F: Documentation/i2c/i2c-topology
F: Documentation/i2c/muxes/
F: Documentation/devicetree/bindings/i2c/i2c-mux*
+F: Documentation/devicetree/bindings/i2c/i2c-arb*
F: drivers/i2c/i2c-mux.c
F: drivers/i2c/muxes/
F: include/linux/i2c-mux.h