linux-kernel - Re: [PATCH 1/4] ARM: dts: Add binding for Broadcom MSPI driver.

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: <5522D432.10604@broadcom.com>
Date:	Mon, 6 Apr 2015 11:45:06 -0700
From:	Jonathan Richardson <jonathar@...adcom.com>
To:	Florian Fainelli <f.fainelli@...il.com>
CC:	Mark Brown <broonie@...nel.org>, Dmitry Torokhov <dtor@...gle.com>,
	Anatol Pomazau <anatol@...gle.com>,
	Scott Branden <sbranden@...adcom.com>,
	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>,
	<linux-kernel@...r.kernel.org>, <linux-spi@...r.kernel.org>,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>,
	<devicetree@...r.kernel.org>, Rafal Milecki <zajec5@...il.com>,
	Brian Norris <computersforpeace@...il.com>,
	Kevin Cernekee <cernekee@...il.com>
Subject: Re: [PATCH 1/4] ARM: dts: Add binding for Broadcom MSPI driver.

On 15-04-04 12:17 PM, Florian Fainelli wrote:
> Le 02/04/2015 12:23, Jonathan Richardson a écrit :
>>
>> Signed-off-by: Jonathan Richardson <jonathar@...adcom.com>
>> ---
>>  .../devicetree/bindings/spi/brcm,mspi-spi.txt      |   38 ++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
>> new file mode 100644
>> index 0000000..16164e3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/brcm,mspi-spi.txt
>> @@ -0,0 +1,38 @@
>> +Broadcom MSPI controller
>> +
>> +Required properties:
>> +- compatible: Must be either "brcm,mspi" or "brcm,bcma-mspi". Use
>> +  "brcm,bcma-mspi" for controllers on a bcma bus and "brcm,mspi" otherwise.
> 
> We need a more specific compatible property here since there are at
> least 3 known SoCs families within Broadcom (Cygnus, BCM53xx, BCM7xxx)
> that use this controller, also older versions of the core did not have a
> revision register, yet they had an internal version numbering that we
> might want to reflect here. This does not need to be fixed immediately
> though, we can add compatible strings as we start adding support for
> older cores.
> 
>> +
>> +- reg:  Physical base address and length of the controller's registers.
>> +
>> +- interrupts: The interrupt id for the controller.
> 
> I think this should be two cells, on BCM7xxx chips there is a MSPI_DONE
> and a MSPI_ERROR interrupt bit, we typically only use the first one, but
> since we are describing the hardware here, we need to be exhaustive.

My mistake. Interrupts are not supported by this driver yet. I intend on
adding this later. I will remove this from the docs.

> 
>> +
>> +- #address-cells: should be 1.
>> +
>> +- #size-cells: should be 0.
>> +
>> +Optional properties:
>> +- clocks: The MSPI reference clock. If not provided then it is assumed a clock
>> +  is enabled by default and no control of clock-frequency (see below) is
>> +  possible.
>> +
>> +- clock-names: The name of the reference clock.
>> +
>> +- clock-frequency: Desired frequency of the clock. This will set the serial
>> +  clock baud rate (SPBR) based on the reference clock frequency. The frequency
>> +  of the SPBR is mspi_clk / (2 * SPBR) where SPBR is a value between 1-255
>> +  determined by the desired 'clock-frequency'. If not provided then the default
>> +  baud rate of the controller is used.
> 
> See my reply to the patch 4, that does not seem to match the
> "clock-frequency" vs. clock phandles practices in DT.

I could use a different property for it. I agree it's a bit weird since
we're not changing the frequency of the reference clock but the clock
derived from it for the baud rate of the controller. Does this warrant
adding a different property though?

> 
>> +
>> +Example:
>> +
>> +mspi@...47000 {
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	compatible = "brcm,mspi";
>> +	reg = <0x18047000 0x1000>;
>> +	clocks = <&axi41_clk>;
>> +	clock-names = "mspi_clk";
>> +	clock-frequency = <12500000>;
> 
> Since "interrupts" is a mandatory property you might want the example to
> show it for consistency.
> --
> Florian
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/