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

[Florian Fainelli <f.fainelli@gmail.com>]

Le 02/04/2015 12:23, Jonathan Richardson a écrit :
> 
> Signed-off-by: Jonathan Richardson <jonathar@broadcom.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.

> +
> +- #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.

> +
> +Example:
> +
> +mspi@18047000 {
> +	#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