[PATCH V2 1/2] spi: dual and quad support(device tree)

[Gupta, Pekon]

> 
> Hi, Pekon
> 
> What about the comment below, do I still need to check the mode before
> setting.
> 
I would suggest so :-)
 (a) 'mode == CPHA, CPOL'
(b) 'mode==SPI_TX_QUAD | SPI_TX_DUAL'

Though I would have suggested to add compatibility checks for both (a)
and (b), so that all mis-matches are identified during device probe.
But, as you patch newly adds (b), it would look complete if it adds
compatibility for at-least (b).

But if you are not keen, its okay. For your current patch..

Acked-by: Pekon Gupta <pekon at ti.com>


with regards, pekon

> >> You missed out comments provided earlier..
> >> http://lists.infradead.org/pipermail/linux-mtd/2013-August/048374.html
> >>
> >> Remember .. this compatibility check is between spi_device->mode
> >> v/s spi_master->mode_bits. This is different from ur V3 patch
> >> which checks for spi_transfer->tx_nbits v/s spi_device->mode.
> >>
> >> + if ((xfer->tx_nbits == SPI_NBITS_DUAL) &&
> >> + !(spi->mode & (SPI_TX_DUAL | SPI_TX_QUAD)))
> >> + return -EINVAL;
> >> + if ((xfer->tx_nbits == SPI_NBITS_QUAD) &&
> >> + !(spi->mode & SPI_TX_QUAD))
> >> + return -EINVAL;
> >>
> >>
> > Sorry for my mis-understanding. But here I want to regard DUAL / QUAD
> > mode just as other mode, such as CPHA, CPOL and so on. To other mode,
> > spi framework didn't do such check, example:
> > ---------------------------------------------------------------------------------
> > /* Mode (clock phase/polarity/etc.) */
> > if (of_find_property(nc, "spi-cpha", NULL))
> >     spi->mode |= SPI_CPHA;
> > if (of_find_property(nc, "spi-cpol", NULL))
> >     spi->mode |= SPI_CPOL;
> > ---------------------------------------------------------------------------------
> >
> > Spi framework do this check in spi_setup as follow:
> > ---------------------------------------------------------------------------------
> > bad_bits = spi->mode & ~spi->master->mode_bits;
> > if (bad_bits) {
> >     dev_err(&spi->dev, "setup: unsupported mode bits %x\n",
> >           bad_bits);
> >     return -EINVAL;
> > }
> > ---------------------------------------------------------------------------------
> > So to keep that consistant, I don't think it is necessary to add the
> > check when setting the mode.
> >
> > Best regards