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:	Tue, 28 May 2013 11:09:41 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Gabriel Fernandez <gabriel.fernandez.st@...il.com>
Cc:	linux-kernel@...r.kernel.org,
	Linus Walleij <linus.walleij@...aro.org>,
	Olivier Clergeaud <olivier.clergeaud@...com>,
	Gabriel Fernandez <gabriel.fernandez@...com>
Subject: Re: [PATCH 2/3] ARM: u8540: Add Pinctrl Device Tree settings for
 uart0, uart2

On Mon, 27 May 2013, Gabriel Fernandez wrote:

> From: Gabriel Fernandez <gabriel.fernandez@...com>
> 
> This patch adds pinctrl device tree settings for uart0 and uart2
> for ccu8540 board.
> 
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@...com>
> ---
>  arch/arm/boot/dts/ccu8540-pinctrl.dtsi     | 77 ++++++++++++++++++++++++
>  arch/arm/boot/dts/ccu8540.dts              |  7 +++
>  arch/arm/boot/dts/dbx5x0.dtsi              |  2 +-
>  arch/arm/boot/dts/ste-nomadik-pinctrl.dtsi | 95 ++++++++++++++++++++++++++++++

This is starting to get a bit confusing. How intrusive would it be to
place the ccu8540-pinctrl information inside ccu8540.dts instead of
breaking it out into different files and convoluting the issue?

Also, please correct me if I'm wrong, but isn't what we call the
Nomadik Pinctrl/GPIO really the same as DBX500 Pinctrl/GPIO? I wonder
if this would be a better naming convention?

Linus, what do you think?

>  include/dt-bindings/pinctrl/nomadik.h      | 36 +++++++++++
>  5 files changed, 216 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/ccu8540-pinctrl.dtsi
>  create mode 100644 arch/arm/boot/dts/ste-nomadik-pinctrl.dtsi
>  create mode 100644 include/dt-bindings/pinctrl/nomadik.h
> 
> diff --git a/arch/arm/boot/dts/ccu8540-pinctrl.dtsi b/arch/arm/boot/dts/ccu8540-pinctrl.dtsi
> new file mode 100644
> index 0000000..26e718b
> --- /dev/null
> +++ b/arch/arm/boot/dts/ccu8540-pinctrl.dtsi
> @@ -0,0 +1,77 @@
> +/*
> + * Copyright 2012 ST-Ericsson
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +#include "ste-nomadik-pinctrl.dtsi"
> +
> +&pinctrl_dbx500 {

What does the '&' do?

> +	uart0 {
> +		uart0_default_mux: uart0_mux {
> +			default_mux {
> +				ste,function = "u0";
> +				ste,pins = "u0_a_1";
> +			};
> +		};
> +
> +		uart0_default_mode: uart0_default {
> +			default_cfg1 {
> +				ste,pins = "GPIO0", "GPIO2";
> +				ste,config = <&in_pu>;
> +			};
> +
> +			default_cfg2 {
> +				ste,pins = "GPIO1", "GPIO3";
> +				ste,config = <&out_hi>;
> +			};
> +		};
> +
> +		uart0_sleep_mode: uart0_sleep {
> +			sleep_cfg1 {
> +				ste,pins = "GPIO0", "GPIO2";
> +				ste,config = <&slpm_in_pu>;
> +			};
> +
> +			sleep_cfg2 {
> +				ste,pins = "GPIO1", "GPIO3";
> +				ste,config = <&slpm_out_hi>;
> +			};
> +		};
> +	};
> +
> +	uart2 {
> +		uart2_default_mode: uart2_default {
> +			default_mux {
> +				ste,function = "u2";
> +				ste,pins = "u2txrx_a_1";
> +			};
> +
> +			default_cfg1 {
> +				ste,pins = "GPIO120";
> +				ste,config = <&in_pu>;
> +			};
> +
> +			default_cfg2 {
> +				ste,pins = "GPIO121";
> +				ste,config = <&out_hi>;
> +			};
> +		};
> +
> +		uart2_sleep_mode: uart2_sleep {
> +			sleep_cfg1 {
> +				ste,pins = "GPIO120";
> +				ste,config = <&slpm_in_pu>;
> +			};
> +
> +			sleep_cfg2 {
> +				ste,pins = "GPIO121";
> +				ste,config = <&slpm_out_hi>;
> +			};
> +		};
> +	};
> +};
> diff --git a/arch/arm/boot/dts/ccu8540.dts b/arch/arm/boot/dts/ccu8540.dts
> index 5de9e1e..4f93795 100644
> --- a/arch/arm/boot/dts/ccu8540.dts
> +++ b/arch/arm/boot/dts/ccu8540.dts
> @@ -11,6 +11,7 @@
>  
>  /dts-v1/;
>  #include "dbx5x0.dtsi"
> +#include "ccu8540-pinctrl.dtsi"
>  
>  / {
>  	model = "ST-Ericsson U8540 platform with Device Tree";
> @@ -27,6 +28,9 @@
>  		};
>  
>  		uart@...20000 {
> +			pinctrl-names = "default", "sleep";
> +			pinctrl-0 = <&uart0_default_mux>, <&uart0_default_mode>;
> +			pinctrl-1 = <&uart0_sleep_mode>;
>  			status = "okay";
>  		};
>  
> @@ -35,6 +39,9 @@
>  		};
>  
>  		uart@...07000 {
> +			pinctrl-names = "default", "sleep";
> +			pinctrl-0 = <&uart2_default_mode>;
> +			pinctrl-1 = <&uart2_sleep_mode>;
>  			status = "okay";
>  		};
>  	};
> diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
> index f85ff85..7507148 100644
> --- a/arch/arm/boot/dts/dbx5x0.dtsi
> +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> @@ -170,7 +170,7 @@
>  			gpio-bank = <8>;
>  		};
>  
> -		pinctrl {
> +		pinctrl_dbx500: pinctrl {
>  			compatible = "stericsson,nmk-pinctrl";
>  			prcm = <&prcmu>;
>  		};
> diff --git a/arch/arm/boot/dts/ste-nomadik-pinctrl.dtsi b/arch/arm/boot/dts/ste-nomadik-pinctrl.dtsi
> new file mode 100644
> index 0000000..efddee9
> --- /dev/null
> +++ b/arch/arm/boot/dts/ste-nomadik-pinctrl.dtsi
> @@ -0,0 +1,95 @@
> +/*
> + * Copyright 2012 ST-Ericsson
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +#include <dt-bindings/pinctrl/nomadik.h>
> +
> +/ {
> +	in_nopull: in_nopull {
> +		ste,input = <INPUT_NOPULL>;
> +	};
> +
> +	in_pu: input_pull_up {
> +		ste,input = <INPUT_PULLUP>;
> +	};
> +
> +	in_pd: input_pull_down {
> +		ste,input = <INPUT_PULLDOWN>;
> +	};
> +
> +	out_hi: output_high {
> +		ste,output = <OUTPUT_HIGH>;
> +	};
> +
> +	out_lo: output_low {
> +		ste,output = <OUTPUT_LOW>;
> +	};
> +
> +	gpio_out_lo: gpio_output_low {
> +		ste,gpio = <GPIOMODE_ENABLED>;
> +		ste,output = <OUTPUT_LOW>;
> +	};
> +
> +	slpm_in_pu: slpm_in_pu {
> +		ste,sleep = <SLPM_ENABLED>;
> +		ste,sleep-input = <SLPM_INPUT_PULLUP>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +	};
> +
> +	slpm_in_wkup_pdis: slpm_in_wkup_pdis {
> +		ste,sleep = <SLPM_ENABLED>;
> +		ste,sleep-input = <SLPM_DIR_INPUT>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +		ste,sleep-pull-disable = <SLPM_PDIS_DISABLED>;
> +	};
> +
> +	slpm_out_lo: slpm_out_lo {
> +		ste,sleep = <SLPM_ENABLED>;
> +		ste,sleep-output = <SLPM_OUTPUT_LOW>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +	};
> +
> +	slpm_out_hi: slpm_out_hi {
> +		ste,sleep = <SLPM_ENABLED>;
> +		ste,sleep-output = <SLPM_OUTPUT_HIGH>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +	};
> +
> +	slpm_out_hi_wkup_pdis: slpm_out_hi_wkup_pdis {
> +		ste,sleep = <SLPM_ENABLED>;
> +		ste,sleep-output = <SLPM_OUTPUT_HIGH>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +		ste,sleep-pull-disable = <SLPM_PDIS_DISABLED>;
> +	};
> +
> +	slpm_out_wkup_pdis: slpm_out_wkup_pdis {
> +		ste,sleep = <SLPM_ENABLED>;
> +		ste,sleep-output = <SLPM_DIR_OUTPUT>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +		ste,sleep-pull-disable = <SLPM_PDIS_DISABLED>;
> +	};
> +
> +	in_wkup_pdis: in_wkup_pdis {
> +		ste,sleep-input = <SLPM_DIR_INPUT>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +		ste,sleep-pull-disable = <SLPM_PDIS_DISABLED>;
> +	};
> +
> +	out_hi_wkup_pdis: out_hi_wkup_pdis {
> +		ste,sleep-output = <SLPM_OUTPUT_HIGH>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +		ste,sleep-pull-disable = <SLPM_PDIS_DISABLED>;
> +	};
> +
> +	out_wkup_pdis: out_wkup_pdis {
> +		ste,sleep-output = <SLPM_DIR_OUTPUT>;
> +		ste,sleep-wakeup = <SLPM_WAKEUP_ENABLE>;
> +		ste,sleep-pull-disable = <SLPM_PDIS_DISABLED>;
> +	};
> +};

Nodes look pretty good, except shouldn't 'ste' really be 'stericsson'?

Yes, it's not as succinct, but it is the standard.

> diff --git a/include/dt-bindings/pinctrl/nomadik.h b/include/dt-bindings/pinctrl/nomadik.h
> new file mode 100644
> index 0000000..638fb32
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/nomadik.h
> @@ -0,0 +1,36 @@
> +/*
> + * nomadik.h
> + *
> + * Copyright (C) ST-Ericsson SA 2013
> + * Author: Gabriel Fernandez <gabriel.fernandez@...com> for ST-Ericsson.
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#define INPUT_NOPULL		0
> +#define INPUT_PULLUP		1
> +#define INPUT_PULLDOWN		2
> +
> +#define OUTPUT_LOW		0
> +#define OUTPUT_HIGH		1
> +#define DIR_OUTPUT		2
> +
> +#define SLPM_DISABLED		0
> +#define SLPM_ENABLED		1
> +
> +#define SLPM_INPUT_NOPULL	0
> +#define SLPM_INPUT_PULLUP	1
> +#define SLPM_INPUT_PULLDOWN	2
> +#define SLPM_DIR_INPUT		3
> +
> +#define SLPM_OUTPUT_LOW		0
> +#define SLPM_OUTPUT_HIGH	1
> +#define SLPM_DIR_OUTPUT		2
> +
> +#define SLPM_WAKEUP_DISABLE	0
> +#define SLPM_WAKEUP_ENABLE	1
> +
> +#define GPIOMODE_DISABLED	0
> +#define GPIOMODE_ENABLED	1
> +
> +#define SLPM_PDIS_DISABLED	0
> +#define SLPM_PDIS_ENABLED	1

Some stray tabbing above.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ