Re: [PATCH] spi: spi-geni-qcom: Add SPI driver support for GENI based QUP

[Trent Piepho <tpiepho@impinj.com>]

On Thu, 2018-08-09 at 12:37 -0700, Doug Anderson wrote:
> On Thu, Aug 9, 2018 at 11:24 AM, Trent Piepho <tpiepho@impinj.com> wrote:
> > 
> I think we're in agreement but perhaps there's a miscommunication here?
> 
> I'm saying that we _shouldn't_ put the max-speed of the master in the
> device tree.  The max speed for the IP block ought to be in code
> either in the clock driver or in the SPI driver based on the
> compatible string.

Yes, totally agree.

Usually the clock framework provides a clock to the SPI IP block at
some rate, or range of rates.  Based on that clock, the SPI IP block
has a max SPI clock, and the driver can calculate it.

We do have a concept of a SPI master max clock, but it's never needed
to be specified in the DT.

Consider userspace access via spidev.  The transfer asks for 100 MHz,
but the spi master driver can not possibly program that speed into
whatever register(s) control the spi clock.  The SPI core handles that
case of limiting a transfer to the master's max.

> ...as one argument _against_ putting a max-speed for the master in the
> device tree I'm saying that we can already specify a max-speed of each
> slave in the device tree.  The max speed of the slave should take into
> account whatever factors are specific to this board including trace
> lengths, noise, etc.  If we somehow did need to get a max speed in the
> device tree it seems like we could just adjust the max speed for the
> slave to be something lower.  In other words if you know a board has
> an sdm845 on it and you know that the SPI clock on sdm845 can't go
> over 50 MHz then it would make no sense to suggest that we should run
> the SPI clock for a device at 100 MHz.

What you might do is determine the slave can run at 100 MHz, in some
cases.  The board, traces, master, etc. can do it, in some cases.  But
it's also possible to change the clocking design or settings at a
higher level, which trickles down to the SPI master IP block having a
lower clock.  This is handled by the master max speed being less then
the slave's max speed.

The master max wouldn't be in the DT, since it's probably determined at
run time based on input clock to the master.  Based on the SoC core
clock, voltage, power mode, etc.

So you might have a DT with a slave at 100 MHz, even if in some cases
the master's max is less than 100 MHz.  It's ok to depend on the master
to be the limiting factor.