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

[Brandon Wyman]

On Mon, Jun 26, 2017 at 1:16 AM, Joel Stanley <joel at jms.id.au> wrote:
>
> 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.
Okay, I missed that. Will do.
>
> 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
> >