linux-kernel - Re: [PATCH 2/2] V3 ARM: mvebu: Added dts defintion for Lenovo Iomega ix4-300d NAS

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: <C2BE2F4B-1045-484F-9E12-A757EE3CD1F6@perenite.com>
Date:	Mon, 28 Jul 2014 01:35:35 +0200
From:	Benoit Masson <benoitm@...enite.com>
To:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Cc:	Benoit Masson <yahoo@...enite.com>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Russell King <linux@....linux.org.uk>,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, gregory.clement@...e-electrons.com,
	andrew@...n.ch, sebastian.hesselbarth@...glemail.com,
	jason@...edaemon.net
Subject: Re: [PATCH 2/2] V3 ARM: mvebu: Added dts defintion for Lenovo Iomega ix4-300d NAS

 
Le 27 juil. 2014 à 22:32, Thomas Petazzoni <thomas.petazzoni@...e-electrons.com> a écrit :

> Dear Benoit Masson,
> 

Dear Thomas,

> On Sat, 26 Jul 2014 16:48:10 -0700, Benoit Masson wrote:
> 
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +		pinctrl-0 = <&power_button_pin &reset_button_pin
>> +			&select_button_pin &scroll_button_pin>;
>> +		pinctrl-names = "default";
>> +
>> +		power-button {
>> +			label = "Power Button";
>> +			linux,code = <KEY_POWER>;
>> +			gpios = <&gpio1 12 GPIO_ACTIVE_HIGH>;
>> +		};
>> +
>> +		reset-button {
>> +			label = "Reset Button";
>> +			linux,code = <KEY_RESTART>;
>> +			gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
>> +		};
>> +
>> +		select-button {
>> +			label = "Select Button";
>> +			linux,code = <BTN_SELECT>;
>> +			gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>> +		};
>> +
>> +		scroll-button {
>> +			label = "Scroll Button";
>> +			linux,code = <KEY_SCROLLDOWN>;
>> +			gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
> 
> So you're using the GPIO_ACTIVE_<foo> defines here.
> 
>> +		};
>> +	};
>> +
>> +	spi3 {
>> +		compatible = "spi-gpio";
>> +		status = "okay";
>> +		gpio-sck = <&gpio0 25 0>;
>> +		gpio-mosi = <&gpio1 15 0>; /*gpio 47*/
>> +		cs-gpios = <&gpio0 27 0 >;
> 
> But not here. Any reason?

Indeed I think Jason made this comment already and I seem to have missed -> Corrected in V4

> 
> Also, nitpick: unneeded space in <&gpio0 27 0 > (i.e the space right
> before the closing >).
> 

-> Corrected in V4

>> +		num-chipselects = <1>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		gpio2: gpio2@0 {
>> +			compatible = "fairchild,74hc595";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			reg = <0>;
>> +			registers-number = <2>;
>> +			spi-max-frequency = <100000>;
>> +		};
> 
> I'm not sure "gpio2" is a good name, as we might think of it as being
> the third GPIO controller internal to the SoC.
> 

Renamed to gpio_spi, tested ok  -> Corrected in V4

> Thanks,
> 

You're welcome, patch issued as V4 just now.

> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android 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/