linux-kernel - Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

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]

Date:	Fri, 29 Jul 2016 23:34:41 -0400
From:	Rich Felker <dalias@...c.org>
To:	Mark Brown <broonie@...nel.org>
Cc:	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-spi@...r.kernel.org, linux-sh@...r.kernel.org,
	Rob Herring <robh+dt@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Yoshinori Sato <ysato@...rs.sourceforge.jp>
Subject: Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

On Thu, Jul 28, 2016 at 08:51:25PM +0100, Mark Brown wrote:
> On Thu, Jul 28, 2016 at 03:40:45PM -0400, Rich Felker wrote:
> > On Thu, Jul 28, 2016 at 08:11:53PM +0100, Mark Brown wrote:
> 
> > > An architecture or SoC dependency with || COMPILE_TEST would be useful
> > > for avoiding cluttering Kconfig for other users.  Though as this is in a
> > > FPGA it's perhaps likely people will pick this up for other FPGAs so
> > > perhaps a comment to that effect if it seems likely.
> 
> > Unlike some of the other SoC hardware (interrupt controller) that's
> > more closely tied to the SH cpu trap behavior, the SPI master seems
> > like something that would be nice and easy to reuse elsewhere. I don't
> > feel strongly about it either way though; I can add the arch dep if
> > you want.
> 
> I guess it depends if anyone is actually doing that or not, if nobody is
> the dependency would be better.

OK, let's add the dependency for now with the intent to remove it
if/when there's a need.

> > > Why are you not using the clock API for this?  Just require a clock and
> > > use clk_get_rate() to find out what rate it is.
> 
> > I thought about that but I'm not familiar with it. I can try to figure
> > it out quickly and test that approach; don't see any reason it
> > shouldn't work. Would you insist on having full support for
> > enabling/disabling the clk when it's in use, or would you be happy
> > with treating it as a fixed clock that's always-on for now and
> > possibly extending it with more functionality later if there's ever
> > hardware where that's relevant/helpful?
> 
> It's fine to just enable it at startup and leave it on, though the
> runtime PM ops are trivial and you can set auto_runtime_pm to have the
> core do the gets and puts.

I was able to get it working via the clk api and I'll include support
for this in the next version of the patch, but to actually use it
depends on changing arch/sh to use the common clk framework; otherwise
there's no way to provide a suitable clk in the DT and have
[devm_]clk_get actually pick it up. Should I keep around the option of
using clock-frequency too? That would be most convenient.

I do have a pending patch from Sato-san to switch arch/sh over to CCF
but it's part of a series and I don't think it's ready to merge. I may
be able to merge just a minimal, safe subset that won't break legacy
non-DT configurations, though.

Rich