linux-kernel - Re: [PATCH 5/7] i2c: Add device tree support to i2c-pnx.c

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]

Date:	Mon, 2 Apr 2012 13:17:23 +0000
From:	Arnd Bergmann <arnd@...db.de>
To:	Roland Stigge <stigge@...com.de>
Cc:	arm@...nel.org, linux-arm-kernel@...ts.infradead.org,
	linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
	w.sang@...gutronix.de, srinivas.bakki@....com, kevin.wells@....com,
	gregkh@...uxfoundation.org, netdev@...r.kernel.org,
	rtc-linux@...glegroups.com, a.zummo@...ertech.it,
	linux-watchdog@...r.kernel.org, wim@...ana.be
Subject: Re: [PATCH 5/7] i2c: Add device tree support to i2c-pnx.c

On Monday 02 April 2012, Roland Stigge wrote:

> --- /dev/null
> +++ linux-2.6/Documentation/devicetree/bindings/i2c/pnx.txt
> @@ -0,0 +1,36 @@
> +* NXP PNX I2C Controller
> +
> +Required properties:
> +
> + - reg: Offset and length of the register set for the device
> + - compatible: should be "nxp,pnx-i2c"
> + - interrupts: <a b> where a is the interrupt number and b is a
> +   field that represents an encoding of the sense and level
> +   information for the interrupt

The encoding of the interrupt is specific to the controller and does
not belong here. Just write that there is one interrupt line to be
configure.

> + - interrupt-parent: the phandle for the interrupt controller that
> +   services interrupts for this device.

interrupt-parent should be optional, it can be set in the (grand-)parent
device on systems where all devices use the same controller.

You should also require

#address-cells = <1>;
#size-cells = <0>;

To create an address space for i2c slave device numbers.
> +Examples:
> +
> +	i2c1: i2c@...A0000 {
> +		compatible = "nxp,pnx-i2c";
> +		reg = <0x400A0000 0x100>;
> +		interrupt-parent = <&mic>;
> +		interrupts = <51 0>;
> +	};
> +
> +	i2c2: i2c@...A8000 {
> +		compatible = "nxp,pnx-i2c";
> +		reg = <0x400A8000 0x100>;

Best use all lowercase letters in the addresses.

> +		interrupt-parent = <&mic>;
> +		interrupts = <50 0>;
> +		clock-frequency = <0x186a0>;
> +		pnx,timeout = <0x64>;
> +		slave-addr = <0x11>;
> +	};

>  static struct platform_driver i2c_pnx_driver = {
>  	.driver = {
>  		.name = "pnx-i2c",
>  		.owner = THIS_MODULE,
> +#ifdef CONFIG_OF
> +		.of_match_table = i2c_pnx_of_match,
> +#endif
>  	},
>  	.probe = i2c_pnx_probe,
>  	.remove = __devexit_p(i2c_pnx_remove),

of_match_ptr() instead of this #ifdef, please.

	Arnd
--
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/