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

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: <CAD=FV=X-fGukBpSOYCO=rv94_3gorL=9qmttsvZk21=m6mwUgA@mail.gmail.com>
Date:   Thu, 9 Aug 2018 11:03:55 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Dilip Kota <dkota@...eaurora.org>
Cc:     Stephen Boyd <swboyd@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>,
        linux-spi <linux-spi@...r.kernel.org>,
        Sagar Dharia <sdharia@...eaurora.org>,
        Karthikeyan Ramasubramanian <kramasub@...eaurora.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "Mahadevan, Girish" <girishm@...eaurora.org>
Subject: Re: [PATCH] spi: spi-geni-qcom: Add SPI driver support for GENI based QUP

Hi,

On Fri, Aug 3, 2018 at 5:18 AM,  <dkota@...eaurora.org> wrote:
>>> +       if (of_property_read_u32(pdev->dev.of_node, "spi-max-frequency",
>>> +                               &spi->max_speed_hz)) {
>
>
>> Why does this need to come from DT?
>
>
> This is required to set the SPI controller max frequency.
> As it is specific to the controller, so looks meaningful to specify it in
> dtsi.
> Also, spi core framework will set the transfer speed to controller max
> frequency
> if transfer frequency is greater than controller max frequency.
> Please mention if you have a other opinion.

Here are my thoughts:

1. It sure seems like the clock framework could be enforcing the max
speed here.  SPI can just ask for the speed and the clock framework
will pick the highest speed it can if you ask for one too high.  Isn't
that the whole point of the "struct freq_tbl" in the clock driver?

2. The device tree writer already provides a max clock speed for each
SPI slave in the device tree.  ...shouldn't the device tree writer
already be taking into account the max of the SPI port when setting
this value?

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";

...as per #1 and #2 I don't think this is useful

-Doug