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.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
