linux-kernel - Re: [PATCH v4 3/5] dt/bindings: qcom

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: <20151216073306.302f3614@bbrezillon>
Date:	Wed, 16 Dec 2015 07:33:06 +0100
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Archit Taneja <architt@...eaurora.org>
Cc:	linux-mtd@...ts.infradead.org, dehrenberg@...gle.com,
	cernekee@...il.com, computersforpeace@...il.com,
	sboyd@...eaurora.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	agross@...eaurora.org
Subject: Re: [PATCH v4 3/5] dt/bindings: qcom_nandc: Add DT bindings

Hi Archit,

Sorry for the late review, but there are a few things I think should be
addressed.

On Wed, 19 Aug 2015 10:19:04 +0530
Archit Taneja <architt@...eaurora.org> wrote:

> Add DT bindings document for the Qualcomm NAND controller driver.
> 
> Cc: devicetree@...r.kernel.org
> 
> v4:
> - No changes
> 
> v3:
> - Don't use '0x' when specifying nand controller address space
> - Add optional property for on-flash bbt usage
> 
> Acked-by: Andy Gross <agross@...eaurora.org>
> Signed-off-by: Archit Taneja <architt@...eaurora.org>
> ---
>  .../devicetree/bindings/mtd/qcom_nandc.txt         | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> new file mode 100644
> index 0000000..1de4643
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> @@ -0,0 +1,49 @@
> +* Qualcomm NAND controller
> +
> +Required properties:
> +- compatible:		should be "qcom,ebi2-nand" for IPQ806x
> +- reg:			MMIO address range
> +- clocks:		must contain core clock and always on clock
> +- clock-names:		must contain "core" for the core clock and "aon" for the
> +			always on clock
> +- dmas:			DMA specifier, consisting of a phandle to the ADM DMA
> +			controller node and the channel number to be used for
> +			NAND. Refer to dma.txt and qcom_adm.txt for more details
> +- dma-names:		must be "rxtx"
> +- qcom,cmd-crci:	must contain the ADM command type CRCI block instance
> +			number specified for the NAND controller on the given
> +			platform
> +- qcom,data-crci:	must contain the ADM data type CRCI block instance
> +			number specified for the NAND controller on the given
> +			platform
> +
> +Optional properties:
> +- nand-bus-width:	bus width. Must be 8 or 16. If not present, 8 is chosen
> +			as default
> +
> +- nand-ecc-strength:	number of bits to correct per ECC step. Must be 4 or 8
> +			bits. If not present, 4 is chosen as default
> +- nand-on-flash-bbt:	Create/use on-flash bad block table
> +
> +The device tree may optionally contain sub-nodes describing partitions of the
> +address space. See partition.txt for more detail.
> +
> +Example:
> +
> +nand@...00000 {
> +	compatible = "qcom,ebi2-nandc";
> +	reg = <0x1ac00000 0x800>;
> +
> +	clocks = <&gcc EBI2_CLK>,
> +		 <&gcc EBI2_AON_CLK>;
> +	clock-names = "core", "aon";
> +
> +	dmas = <&adm_dma 3>;
> +	dma-names = "rxtx";
> +	qcom,cmd-crci = <15>;
> +	qcom,data-crci = <3>;
> +
> +	partition@0 {
> +	...
> +	};
> +};


According to the registers layout defined in your driver, your NAND
controller can address multiple chips (NAND_DEV_SEL register). Since DT
bindings are supposed to be as stable as possible, I would recommend
separating the NAND controller and NAND chip declaration (as done here
[1] and here [2]).

Best Regards,

Boris

[1]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
[2]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/sunxi-nand.txt


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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/