Re: spi_mpc8xxx.c: chip select polarity problem

[Torsten Fleischer <to-fleischer@t-online.de>]

On Wed, Dec 9, 2009 at 18:46:51 Grant Likely wrote:
[...]
> > +               ret = of_get_spi_cs_active_state(np, i, &astate);
> > +               if (ret) {
> > +                       dev_err(dev, "can't get cs active state of device
> > " +                               "#%d: %d\n", i, ret);
> > +                       goto err_loop;
> > +               }
> 
> This is a bit heavy handed in that it expects the device tree to be
> fully populated with all SPI devices which isn't always a given.  For
> example a board that has some unpopulated SPI devices could have some
> gaps in the GPIO CS layout.  If a node can't be found, then just
> ignore it silently and move on to the next.  I'd do something like
> this:
> 
> +               astate = of_get_spi_cs_active_state(np, i);

What should be returned if the node can't be found, 'true' or 'false? 

Maybe its better to do the following:

+               ret = of_get_spi_cs_active_state(np, i, &astate);
+               if (ret) {
+                       /* Device node not found */
+                       continue;
+               }

>                ret = gpio_direction_output(pinfo->gpios[i],
> -                                           pinfo->alow_flags[i]);
> +                                           pinfo->alow_flags[i] ^ !astate);
> 
> BTW, why the xor?  The usage is non-obvious enough that I'd like to
> see a comment describing the use case.
 
If I understand it right, the alow_flags describe the wiring. If set to 0 the 
wiring is non-inverted, if set to 1 its inverted respectively. To take this 
into account the active state has to be xor'd with the appropriate alow_flag.