linux-kernel - Re: [PATCH 3/3] Addition of binding for firmware signals on peach-pi

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: <565E327E.90404@samsung.com>
Date:	Wed, 02 Dec 2015 08:51:26 +0900
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Martyn Welch <martyn.welch@...labora.co.uk>
Cc:	Olof Johansson <olof@...om.net>, linux-kernel@...r.kernel.org,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Russell King <linux@....linux.org.uk>,
	Kukjin Kim <kgene@...nel.org>, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org
Subject: Re: [PATCH 3/3] Addition of binding for firmware signals on peach-pi

On 02.12.2015 04:12, Martyn Welch wrote:
> The peach pi has a GPIO connected to the firmware write protect, developer
> mode and recovery mode lines. This patch adds the required nodes to the
> device tree to configure the pinmuxing and allow these to be read from
> user space.
> 
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: Pawel Moll <pawel.moll@....com>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Ian Campbell <ijc+devicetree@...lion.org.uk>
> Cc: Kumar Gala <galak@...eaurora.org>
> Cc: Russell King <linux@....linux.org.uk>
> Cc: Kukjin Kim <kgene@...nel.org>
> Cc: Krzysztof Kozlowski <k.kozlowski@...sung.com>
> Cc: devicetree@...r.kernel.org
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-samsung-soc@...r.kernel.org
> Signed-off-by: Martyn Welch <martyn.welch@...labora.co.uk>
> ---
>  arch/arm/boot/dts/exynos5800-peach-pi.dts | 40 +++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)

Hi,

Thanks for the patch.

Few points from my side:
1. Please add a prefix to the subject: "ARM: dts:".

2. There is no need of such huge CC-list in the body of commit. This
CC-list comes from get_maintainer so there is no benefit of duplicating
it here. The CC is usually used to notify other people who might be
interested but get_maintainer does not point them.

3. I received only this third patch. I did not receive cover letter
explaining possible dependencies so I am not sure how to deal with the
patch. It looks like there are no dependencies... but maybe there are?
Is this is a new binding or no? Please provide a cover letter (if it
exists already be sure to send it to all interested parties) or send
entire patchset so the big picture could be seen.

The patch itself looks good but I'll wait with a review tag for #3.

Best regards,
Krzysztof


> 
> diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> index 49a4f43..485c18f 100644
> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> @@ -53,6 +53,25 @@
>  		};
>  	};
>  
> +	chromeos-firmware {
> +		compatible = "google,gpio-firmware";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wp_gpio &dev_mode &rec_mode>;
> +
> +		write-protect {
> +			gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		developer-switch {
> +			gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		recovery-switch {
> +			gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
>  	gpio-keys {
>  		compatible = "gpio-keys";
>  
> @@ -731,6 +750,13 @@
>  		samsung,pin-val = <0>;
>  	};
>  
> +	rec_mode: rec-mode {
> +		samsung,pins = "gpx0-7";
> +		samsung,pin-function = <0>;
> +		samsung,pin-pud = <0>;
> +		samsung,pin-drv = <0>;
> +	};
> +
>  	tpm_irq: tpm-irq {
>  		samsung,pins = "gpx1-0";
>  		samsung,pin-function = <0>;
> @@ -752,6 +778,13 @@
>  		samsung,pin-drv = <0>;
>  	};
>  
> +	dev_mode: dev-mode {
> +		samsung,pins = "gpx1-3";
> +		samsung,pin-function = <0>;
> +		samsung,pin-pud = <3>;
> +		samsung,pin-drv = <0>;
> +	};
> +
>  	ec_irq: ec-irq {
>  		samsung,pins = "gpx1-5";
>  		samsung,pin-function = <0>;
> @@ -773,6 +806,13 @@
>  		samsung,pin-drv = <0>;
>  	};
>  
> +	wp_gpio: wp_gpio {
> +		samsung,pins = "gpx3-0";
> +		samsung,pin-function = <0>;
> +		samsung,pin-pud = <0>;
> +		samsung,pin-drv = <0>;
> +	};
> +
>  	max77802_irq: max77802-irq {
>  		samsung,pins = "gpx3-1";
>  		samsung,pin-function = <0>;
> 

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