> > +
> > +Optional properties:
> > +- interrupt-parent : phandle of parent interrupt controller;
> > +- interrupts : interrupt line;
> > +- deferred : I2C deferred bus phandle;
> > + I2C bus activation order enforce for the cases when hot-plug
> > + devices are attached to I2C bus, which is initialized after the
> > + I2C bus, where programmable device is attached;
>
> Dependencies aren't usually expressed in DT.
>
> Why can't this be done pragmatically?
>
> Usually we try to interrogate a device that we depend on and if it's not
> present, we return -EPROBE_DEFER and try again latter.
This particular device table is for BMC system with Aspeed2500 SoC. It has
14 i2c busses. I have the cases, like for example the following:
programmable device (in this case this is Lattice CPLD) is located on bus 4
and through this device presence of FANs drawers are detected. For which
presented unit EEPROM driver should be connected. And this EEPROMs
are connected to i2c switch, which is located at bus 6.
So I want to enforce initialization order. And in mlxreg-i2c driver,
which I removed from the last patch I do it as:
child = of_parse_phandle(np, "deferred", 0);
if (child) {
adapter = of_find_i2c_adapter_by_node(child);
of_node_put(child);
if (!adapter)
return -EPROBE_DEFER;
}
>
> > +- hotplug-spec <aggr mask>:
> > + - aggr : interrupt top aggregation register offset;
> > + - mask : interrupt top aggregation register mask;
>
> What is this? Are you describing register layout in DT?
>
> You should use the 'reg' property to describe registers.
>
> Bit masks are usually defined in C.
This driver is for programmable devices.
I have to support about 12-15 1U systems and in the close future
several modular systems. This systems can have different number of
replaceable units, different LEDs, resets, selects, etc.
Register map is different for the different systems. It could be slight
Difference (some bits in several registers), and could be a big enough
difference.
Programmable devices could be connected to i2c bus, to LPC bus, to
SPI bus.
My intention was to have a driver which doesn't depend on register
map layout and on physical bus.
In case I'll have bits defined in C, it would be necessary to modify the
In the future the driver code for each new system.
So, yes, I define several critical group of registers in DTS. And I thought
about more or less the same level of register description trough APCI
(I need to support ARM and x86 arch).
>
> > +Optional nodes and their properties:
> > + - psu : power supply unit nodes:
> > + Required properties:
> > + - hotplug-spec <aggr reg mask>:
> > + - aggr : effective bit in aggregation interrupt mask;
> > + - reg : interrupt status register offset for all group members;
> > + - mask : interrupt register mask;
> > + - hotplugs - list of relevant dynamic device nodes;
>
> What is a dynamic device node?
Replaceable units, like PSU, FAN, for which on interrupt I connect/
disconnect f.e. EEPROM.
Power cables, for which on cable in/out I connect/disconnect PSU
controller driver.
Network switch device, CPU, for which on up/down I connect/disconnect
hwmon drivers (this is BMC side).
I also provide support on some special mux selection to open access to
remote flashes (BIOS, shared NIC, etc), for the maintenance access.
This is the reason I am doing node enabling and disabling on the fly.
>
> I think I saw the code for this? Are you attempting to 'enable' and 'disable'
> nodes on-the-fly? That's not really what the status property is for. Usually
> we disable nodes in DTS(I) files which cover many devices, then enable them
> in more specific device files if they are located on that device.
>
> They are not to be used as a flag you just toggle on and off. That's a hack.
>
> > + - fan : fan unit nodes:
> > + Required properties:
> > + - hotplug-spec <aggr reg mask>:
> > + - aggr : effective bit in aggregation interrupt mask;
> > + - reg : interrupt status register offset for all group members;
> > + - mask : interrupt register mask;
> > + - hotplugs - list of relevant dynamic device nodes;
> > + - pwr : power cable nodes:
> > + Required properties:
> > + - hotplug-spec <aggr reg mask>:
> > + - aggr : effective bit in aggregation interrupt mask;
> > + - reg : interrupt status register offset for all group members;
> > + - mask : interrupt register mask;
> > + - hotplugs : list of relevant dynamic device nodes;
> > + - host : host side nodes (CPU host side for BMC):
> > + Required properties:
> > + - hotplug-spec <aggr reg mask>:
> > + - aggr : effective bit in aggregation interrupt mask;
> > + - reg : interrupt status register offset for all group members;
> > + - mask : interrupt register mask;
> > + - hotplugs : list of relevant dynamic device nodes;
> > + - asic : asic nodes:
> > + Required properties:
> > + - hotplug-spec <aggr reg mask>:
> > + - aggr : effective bit in aggregation interrupt mask;
> > + - reg : interrupt status register offsets array per group
> member;
> > + - mask : interrupt register mask;
> > + - hotplugs : list of relevant device nodes;
> > + - reset : reset nodes for system reset operations:
> > + Required properties:
> > + - reset control subnodes:
> > + Required properties:
> > + - attr-spec <reg mask>:
>
> This property is not defined, but again, 'reg' is the correct property to use.
> Masks are usually defined in C code.
>
> > + - reg : register offset;
> > + - mask : attribute mask;
> > + - cause - reset cause nodes for reading system reset cause:
> > + Required properties:
> > + - cause info subnodes:
> > + Required properties:
> > + - attr-spec <reg mask>:
> > + - reg : register offset;
> > + - mask : attribute mask;
> > + - mux - mux nodes:
> > + Required properties:
> > + - mux control subnodes for hardware select operations:
> > + Required properties:
> > + - attr-spec <reg mask bit>:
> > + - reg : register offset;
> > + - mask : attribute mask;
> > + - bit : effective bit;
> > + Optional property:
> > + - hotplugs : dynamic device node selected by mux;
> > + - gprw - general purpose read-write register nodes:
> > + Required properties:
> > + - general purpose read-write register subnodes:
> > + Required properties:
> > + - attr-spec <reg mask>:
> > + - reg : register offset;
> > + - mask : attribute mask;
> > + - gpro - general purpose read only register nodes:
> > + Required properties:
> > + - general purpose read only register subnodes:
> > + Required properties:
> > + - attr-spec <reg mask>:
> > + - reg : register offset.
> > + - mask : attribute mask.
> > + - led - led nodes for led operations control:
> > + Required properties:
> > + - led control nodes:
> > + Required properties:
> > + - attr-spec <reg mask>:
> > + - reg : register offset;
> > + - mask : attribute mask;
> > +
> > +Example:
> > + mlxcpld-mng-ctrl@71 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + interrupt-parent = <&gpio>;
> > + interrupts = <ASPEED_GPIO(S, 1) 2>;
> > + compatible = "mellanox,mlxcpld-ctrl-i2c";
> > + reg = <0x71>;
> > + deferred = <&i2c6>;
> > + hotplug-spec = <0x3a 0x4c>;
> > +
> > + pwr: {
> > + hotplug-spec = <0x08 0x64 0x03>;
> > + hotplugs = <&psu1_ctrl &psu2_ctrl>;
> > + };
> > +
> > + mux {
> > + spi_burn_bios_ci {
> > + attr-spec = <0x32 0xfe 0x00>;
> > + hotplugs = <&spi2>;
> > + };
> > + };
> > +
> > + led {
> > + status-green {
> > + attr-spec = <0x20 0xf0>;
> > + };
> > + status-red {
> > + attr-spec = <0x20 0xf0>;
> > + };
> > + };
> > +
> > + reset {
> > + sys_power_cycle {
> > + attr-spec = <0x30 0xfb>;
> > + };
> > + };
> > +
> > + cause {
> > + ac_power_cycle {
> > + attr-spec = <0x1d 0xfe>;
> > + };
> > + sys_pwr_cycle: {
> > + attr-spec = <0x1e 0xfb>;
> > + };
> > + };
> > +
> > + gpro {
> > + cpld_mng_version {
> > + attr-spec <0x00 0xff 0xff>;
> > + };
> > + };
> > +
> > + gprw {
> > + sw_reset_cause {
> > + attr-spec = <0x37 0x0f 0x0f>;
> > + };
> > + };
> > + };
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source
> software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
