linux-kernel - Re: [linux-keystone] [PATCH v1 1/1] spi: davinci: add OF support for the spi controller

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] [day] [month] [year] [list]

Message-Id: <20121205234947.0EB3D3E0E22@localhost>
Date:	Wed, 05 Dec 2012 23:49:46 +0000
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Murali Karicheri <m-karicheri2@...com>, rob.herring@...xeda.com,
	rob@...dley.net, devicetree-discuss@...ts.ozlabs.org,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	spi-devel-general@...ts.sourceforge.net,
	davinci-linux-open-source@...ux.davincidsp.com,
	linux-keystone@...t.ti.com
Cc:	Murali Karicheri <m-karicheri2@...com>
Subject: Re: [linux-keystone] [PATCH v1 1/1] spi: davinci: add OF support for the spi controller

On Mon, 3 Dec 2012 13:38:38 -0500, Murali Karicheri <m-karicheri2@...com> wrote:
> This adds OF support to DaVinci SPI controller to configure platform
> data through device bindings. Also replaces clk_enable() with
> of clk_prepare_enable() as well as clk_disable() with
> clk_disable_unprepare().
> 

Hi Murali.

Thanks for the patch. Comments below...

> Signed-off-by: Murali Karicheri <m-karicheri2@...com>
> Reviewed-by : Grant Likely <grant.likely@...retlab.ca>
> ---
>  .../devicetree/bindings/spi/spi-davinci.txt        |   51 ++++++++++
>  drivers/spi/spi-davinci.c                          |  102 +++++++++++++++++---
>  2 files changed, 139 insertions(+), 14 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-davinci.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt b/Documentation/devicetree/bindings/spi/spi-davinci.txt
> new file mode 100644
> index 0000000..a2746c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
> @@ -0,0 +1,51 @@
> +Davinci SPI controller device bindings
> +
> +Required properties:
> +- #address-cells: number of cells required to define a chip select
> +	address on the SPI bus. Should be set to 1.
> +- #size-cells: should be zero.
> +- compatible:
> +	- "ti,davinci-spi-v1" for SPI version 1.0
> +	- "ti,davinci-spi-v2" for SPI version 2.0

It's better to use a specific chip name than some anonymous "v1" or "v2"
(unless you can point me at some hardware documentation that actually
uses those names).

> +- reg: Offset and length of SPI controller register space
> +- ti,davinci-spi-num-cs: Number of chip selects

Simply "num-cs" is sufficient here. There is now a common binding for
the number of chip select lines.

Otherwise the patch and the binding look fine to me.

g.
--
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/