On 2018-08-10 21:43, Mark Brown wrote:
On Fri, Aug 10, 2018 at 08:40:17AM -0700, Doug Anderson wrote:
On Fri, Aug 10, 2018 at 3:52 AM, Mark Brown <broonie@xxxxxxxxxx>
wrote:
> This is more about matching the data rate between the two drivers - the
> clock framework could (and possibly should) reasonably return an error
> here, we're trying to ensure that drivers and controllers work well
> together here.
The clock framework should be able to accomplish what you want. If
you just request the rate it will do its best to make the rate
requested. If we want to see what clock would be set before setting
The request could be massively off the deliverable rate - 50% or more.
is "close enough" in this case? I haven't gone and dug, but I've
always seen people only specify a max rate for SPI. ...so as long as
the clock framework gives us something <= the clock we requested then
we should be OK, right? If / when this becomes a problem then we
should add a min/max to "struct spi_transfer", no?
On the other hand if I ask for my audio to be played at 44.1kHz and the
clock framework says "yes, sure" and gives me 8kHz then the user
experience will be poor.
Note that there are also clk_set_rate_range(), clk_set_min_rate(), and
clk_set_max_rate() though I'm told those might be a bit quirky.
They're certainly not widely used at any rate.
...but maybe we don't need to argue about this anyway since IMHO we
should just use the clk framework to figure out our maximum speed.
It looks like that's true in this case.
>> 3. If you really truly need code in the SPI driver then make sure you
>> include a compatible string for the SoC and have a table in the driver
>> that's found with of_device_get_match_data(). AKA:
>> compatible = "qcom,geni-spi-sdm845", "qcom,geni-spi";
> A controller driver really shouldn't need to be open coding anything.
It wouldn't be open-coding, it would be a different way of specifying
things. In my understanding it's always a judgement call about how
If you're saying we need clock rate selection logic (which is what it
sounds like) rather than data then that seems like a problem.
Here are my couple of cents:
SPI controller maximum frequency can be lesser than or equal to Clock
framework's maximum
frequency, so should not rely on the Clock framework.
SPI controller maximum frequency should not be derived from the SPI
slave maximum frequency.
Consider the case where N number of slaves connected to SPI controller
then there will
be N number of slave maximum frequency.
Also it does not look meaningful in communicating the SPI slave maximum
frequency
as SPI controller maximum frequency to the SPI core framework.
Hence relying on SPI slave maximum frequency for SPI controller maximum
frequency can be eliminated.
Now the need is, how to communicate the SPI controller maximum frequency
to SPI core framework?
Is it by DTSI entry or hardcoding in the SPI controller driver?
My stand is for providing the DTSI entry.
Why because, this keeps SPI controller driver generic across the boards
and portable.
Also it is not against to Device tree usage because maximum frequency
is describing the property of the hardware.
Please add your points.
--Dilip
