[PATCH linux dev-4.10] Add fan card PCA9552 to Witherspoon devicetree

[Joel Stanley]

Hello Brandon,

On Sat, Jun 24, 2017 at 9:26 AM, Brandon Wyman <bjwyman at gmail.com> wrote:
> The Witherspoon OpenBMC system has a PCA9552 chip on the fan card that
> is used for fan fault and identify LED control. Adding this chip and the
> LED lines to the device tree.

Great first submission. One issue I found, please see below.

>
> Signed-off-by: Brandon Wyman <bjwyman at gmail.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 47 ++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index 31315d0..e5e3b37 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -203,6 +203,53 @@
>                 compatible = "infineon,dps310";
>                 reg = <0x76>;
>         };
> +
> +       pca0: pca9552 at 60 {
> +               compatible = "nxp,pca9552";
> +               reg = <0x60>;

You've missed out the #address-cells and #size-cells properties here.
Without these you would have seen this when you compiled the patch:

  DTC     arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dtb
Warning (reg_format): "reg" property in
/ahb/apb/i2c at 1e78a000/i2c-bus at 100/pca9552 at 60/led at 0 has invalid length
(4 bytes) (#address-cells == 2, #size-cells == 1)
Warning (reg_format): "reg" property in
/ahb/apb/i2c at 1e78a000/i2c-bus at 100/pca9552 at 60/led at 1 has invalid length
(4 bytes) (#address-cells == 2, #size-cells == 1)
Warning (avoid_default_addr_size): Relying on default #address-cells
value for /ahb/apb/i2c at 1e78a000/i2c-bus at 100/pca9552 at 60/led at 0

If you go back and check the bindings document, you will notice that
it specifies these properties as required:

- #address-cells: must be 1
- #size-cells: must be 0

https://github.com/openbmc/linux/blob/95dffc96f1f1271c60f180bf736e18278f905a88/Documentation/devicetree/bindings/leds/leds-pca955x.txt#L13

Please send a v2 with this fixed.

Cheers,

Joel


> +               fan0: led at 0 {
> +                       label = "fan0:yellow";
> +                       default-state = "keep";
> +                       reg = <0>;
> +                       type = <PCA955X_TYPE_LED>;
> +               };
> +               fan1: led at 1 {
> +                       label = "fan1:yellow";
> +                       default-state = "keep";
> +                       reg = <1>;
> +                       type = <PCA955X_TYPE_LED>;
> +               };
> +               fan2: led at 2 {
> +                       label = "fan2:yellow";
> +                       default-state = "keep";
> +                       reg = <2>;
> +                       type = <PCA955X_TYPE_LED>;
> +               };
> +               fan3: led at 3 {
> +                       label = "fan3:yellow";
> +                       default-state = "keep";
> +                       reg = <3>;
> +                       type = <PCA955X_TYPE_LED>;
> +               };
> +               led at 13 {
> +                       label = "yellow";
> +                       default-state = "keep";
> +                       reg = <13>;
> +                       type = <PCA955X_TYPE_LED>;
> +               };
> +               led at 14 {
> +                       label = "green";
> +                       default-state = "keep";
> +                       reg = <14>;
> +                       type = <PCA955X_TYPE_LED>;
> +               };
> +               led at 15 {
> +                       label = "blue";
> +                       default-state = "keep";
> +                       reg = <15>;
> +                       type = <PCA955X_TYPE_LED>;
> +               };
> +       };
>  };
>
>  &i2c4 {
> --
> 1.8.2.2
>