Re: [PATCH v1] spi: master driver to enable RTC on ICPDAS LP-8841

[Mark Brown]

On Mon, Feb 22, 2016 at 12:26:14PM +0300, Sergei Ianovich wrote:
> On Mon, 2016-02-22 at 12:10 +0900, Mark Brown wrote:
> > On Mon, Feb 22, 2016 at 04:47:06AM +0300, Sergei Ianovich wrote:

> > > +config SPI_LP8841_RTC
> > > +	tristate "ICP DAS LP-8841 SPI Controller for RTC"
> > > +	depends on OF && (MACH_PXA27X_DT || COMPILE_TEST)

> > Does this need a strict DT dependency or can it build without DT?

> The driver can only be useful on a single industrial PC. That PC
> requires DT, so I made this a strict dependency. The dependency can be
> removed by using ifdefs, but I see no point why.

It makes it easier for anyone doing generic work on the kernel to build
test, one could make exactly the same argument about the dependency on
the PXA27x machine.

> > > +/*
> > > + * REVISIT If there is support for SPI_3WIRE and SPI_LSB_FIRST in
> > > SPI
> > > + * GPIO driver, this SPI driver can be replaced by a simple GPIO
> > > driver
> > > + * providing 3 GPIO pins.
> > > + */

> > What's the advantage of not doing that?  Overall the driver looks
> > fairly
> > good but it does seem to just implement a straight bitbanging driver
> > with less flexibility.

> MicroWire (SPI_3WIRE) mode is slightly different from the modes
> implemented in spi-bitbang-txrx.h. I was getting junk from device in
> both Mode0 and Mode2 until I changed the implementation. The change is
> documented in the patch.

> There will also need to be changes in bitbang.c. SPI_LSB_FIRST will
> require a new flasg in txrx_word(). To keep overhead low will require
> to grow txrx_word[] from 4 to 16 or even 32. CPOL, CPHA, LSB_FIRST,
> 3_WIRE each requires an additional power of 2.

> While this change could be benefitial to both spi-gpio and spi-bitbang, 
> it is very big.

OK, the number of users is probably small enough that it's not worth
worryinng about duplication.

Attachment: signature.asc
Description: PGP signature