> -----Original Message----- > From: Rob Herring [mailto:robh@xxxxxxxxxx] > Sent: Thursday, August 10, 2017 8:40 PM > To: Vadim Pasternak <vadimp@xxxxxxxxxxxx> > Cc: devicetree@xxxxxxxxxxxxxxx; lee.jones@xxxxxxxxxx; pavel@xxxxxx; > j.anaszewski@xxxxxxxxxxx; rpurdie@xxxxxxxxx; linux- > leds@xxxxxxxxxxxxxxx; jiri@xxxxxxxxxxx; jacek.anaszewski@xxxxxxxxx > Subject: Re: [patch v2 1/1 - resend] Documentation: dt-bindings: Add Device > Tree bindings for Mellanox programming devices > > On Thu, Aug 03, 2017 at 12:04:19PM +0000, Vadim Pasternak wrote: > > The mlxreg a multifunction device driver handling LEDs, events, > > exposing through sysfs reset signal, and reset causes info. These > > components share a common register space. > > "dt-bindings: mfd: ..." for the subject please. > Hi Rob, Thank you for review. OK, will change the subject. > > > > Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxxxx> > > --- > > .../devicetree/bindings/mfd/mellanox,mlxreg-core | 346 > +++++++++++++++++++++ > > 1 file changed, 346 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core > > > > diff --git > > a/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core > > b/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core > > new file mode 100644 > > index 0000000..a90933e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core > > @@ -0,0 +1,346 @@ > > +Mellanox programmable device control. > > +------------------------------------- > > +This binding defines the device control interface over I2C bus for > > +Mellanox BMC based switches. > > + > > +Required properties: > > +- compatible = "mellanox,mlxreg-i2c", > > + "mellanox,mlxreg-i2c-16", > > + "mellanox,mlxreg-core"; > > This is all one compatible entry? Why 3 compatibles? It could be "mellanox,mlxreg-i2c" or "mellanox,mlxreg-i2c-16" Is it not correct syntax? And I think I'll drop "mellanox,mlxreg-core". > > > +- #address-cells : must be 1; > > +- #size-cells : must be 0; > > +- reg : I2C address; > > + > > +Optional properties: > > +- interrupt-parent : phandle of parent interrupt controller; > > +- interrupts : interrupt line; > > +- deferred - I2C deferred bus phandle; > > + For example, for the case when device attached to I2C bus > number 4 > > + performs dynamic attachment of device to bus 12 upon some > event. > > + Due to this reason bus 4 is enforced to be activated after bus > > +12; > > I don't understand... For example I have CPLD device on i2c4, which handles the signals for ASIC device. I can connect this device to the bus at initialization time in case it's ready, or upon receiving the relevant signal when it's getting ready: &i2c4 { mlxcpld-swb-ctrl@72 { #address-cells = <1>; #size-cells = <0>; interrupt-parent = <&gpio>; interrupts = <ASPEED_GPIO(S, 1) 2>; compatible = "mellanox,mlxreg-i2c"; reg = <0x72>; deferred = <&i2c12>; cell = <0x40>; mask = <0x01>; asic { aggr_mask = <0x01>; regs = <0x50>; masks = <0x03>; phandles = <&asic_thermal>; }; .... }; The ASIC device is connected to i2c12: &i2c12 { status = "okay"; #address-cells = <1>; #size-cells = <0>; asic_thermal: mlxsw_minimal@48 { compatible = "mlxsw_minimal"; reg = <0x48>; status = "disabled"; cooling-phandle = <&cooling>; trips { /* trip type, temp in mcelsius, temp min, temp max */ trip@0 { trip = <0 75000 0 0>; }; trip@1 { trip = <2 85000 1 5>; }; trip@3 { trip = <2 105000 5 5>; }; }; }; }; So, in this case i2c4 should be activated after i2c12 and for this reason device mlxcpld-swb-ctrl@72 is deferred on activation of i2c12. > > > +- cell - top aggregation register offset; > > +- mask - top aggregation register mask; > > +- psu - power supply unit nodes: > > This section is "Optional Properties" and this is a sub-node. Each subnode > should have its own section. ACK > > > + - aggr_mask - effective bit in aggregation mask; > > Don't use '_' in node or property names. ACK > > > + - reg - register offset for all group members; > > + - mask - register mask; > > + - phandles - list of relevant device nodes; > > Other than reg, I don't understand what any of these are. > > From what I can tell you are describing things down to register bit level which > generally is too much detail in DT. How many variations of h/w do you have? > Do you really need to be able to define any possible combination or have > some finite set? We have about 10 different 1U systems with different numbers of FANs, with replicable or non-replaceable PSU. I am also planning to support director systems, which has different configurations - different number of leaf and spine cards and power supply and FAN units. This is the wide range of system from smallest with 16x100G ports with biggest with 800x200G ports. I really need flexibility in device description. In field mask I assign the affective mask for dynamic devices, f.e. example for system with 4 FANs mask will be 0x0f, with 6 0x3f and for director system it can be located in several registers with masks 0xff. Handles have the next purpose. For example for fan nodes I provide the list of EEPROM devices' handles, which should be connected/disconnected on insert/remove: fan { aggr_mask = <0x40>; reg = <0x88>; mask = <0x0f>; phandles = <&fan1_eeprom &fan2_eeprom &fan3_eeprom &fan4_eeprom>; }; And this EEPROM devices are connected to i2c switch device like below: &i2c6 { ... i2cswitch@71 { compatible = "nxp,pca9548"; #address-cells = <1>; #size-cells = <0>; reg = <0x71>; i2c@0 { #address-cells = <1>; #size-cells = <0>; reg = <0>; fan1_eeprom: at24c04@50 { compatible = "atmel,24c32"; reg = <0x50>; status = "disabled"; }; }; i2c@1 { #address-cells = <1>; #size-cells = <0>; reg = <1>; fan2_eeprom: at24c04@50 { compatible = "atmel,24c32"; reg = <0x50>; status = "disabled"; }; }; i2c@2 { #address-cells = <1>; #size-cells = <0>; reg = <2>; fan3_eeprom: at24c04@50 { compatible = "atmel,24c32"; reg = <0x50>; status = "disabled"; }; }; i2c@3 { #address-cells = <1>; #size-cells = <0>; reg = <3>; fan4_eeprom: at24c04@50 { compatible = "atmel,24c32"; reg = <0x50>; status = "disabled"; }; }; }; }; > > Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
