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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160222110816.GD18327@sirena.org.uk>
Date:	Mon, 22 Feb 2016 20:08:16 +0900
From:	Mark Brown <broonie@...nel.org>
To:	Sergei Ianovich <ynvich@...il.com>
Cc:	linux-kernel@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@...r.kernel.org>,
	"open list:SPI SUBSYSTEM" <linux-spi@...r.kernel.org>
Subject: Re: [PATCH v1] spi: master driver to enable RTC on ICPDAS LP-8841

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.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)