diff mbox

[RFC,v2,3/8] pinctrl: pinconf-generic: Add parameter 'IO standard'

Message ID 1413479495-14206-4-git-send-email-soren.brinkmann@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Soren Brinkmann Oct. 16, 2014, 5:11 p.m. UTC
For HW that can select the IO standard for pins, add the infrastructure
in pinconf generic to parse this parameter.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt | 3 +++
 drivers/pinctrl/pinconf-generic.c                              | 2 ++
 include/linux/pinctrl/pinconf-generic.h                        | 4 ++++
 3 files changed, 9 insertions(+)

Comments

Linus Walleij Oct. 28, 2014, 2:59 p.m. UTC | #1
On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
<soren.brinkmann@xilinx.com> wrote:

> For HW that can select the IO standard for pins, add the infrastructure
> in pinconf generic to parse this parameter.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

Nothing in this patch actually explain what an "IO standard" is,
which is paramount if it should be a generic property.

And if it should be generic, I really want to know why this is a
thing not overlapping with existing generic configs, and exactly
what it is and that it is probable other silicon will have it too.
And we need to discuss if it's the right name.

Looking at what you actually set up, it looks very much like this
is actually PIN_CONFIG_DRIVE_STRENGTH.

Yours,
Linus Walleij
Soren Brinkmann Oct. 28, 2014, 4:07 p.m. UTC | #2
On Tue, 2014-10-28 at 03:59PM +0100, Linus Walleij wrote:
> On Thu, Oct 16, 2014 at 7:11 PM, Soren Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> 
> > For HW that can select the IO standard for pins, add the infrastructure
> > in pinconf generic to parse this parameter.
> >
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> 
> Nothing in this patch actually explain what an "IO standard" is,
> which is paramount if it should be a generic property.
> 
> And if it should be generic, I really want to know why this is a
> thing not overlapping with existing generic configs, and exactly
> what it is and that it is probable other silicon will have it too.
> And we need to discuss if it's the right name.
> 
> Looking at what you actually set up, it looks very much like this
> is actually PIN_CONFIG_DRIVE_STRENGTH.

I was thinking about that, but:
drive strength is well defined saying you choose the drive strength in
mA (or some other variation of Amperes). That is not (directly) what you
can set in Zynq. In Zynq the config allows selecting IO-standards like
LVCMOS(1.8|2.5|3.3) or HSTL.
So, this might also map to a certain drive-strength, but is not really
mapping well to Zynq pin configuration options.
Hence, I thought introducing this new property which allows a more
flexible interpretation of the argument would be a better way and might
also help other pinctrl cases (though this is speculative).

	Thanks,
	Sören
Linus Walleij Oct. 31, 2014, 8:19 a.m. UTC | #3
On Tue, Oct 28, 2014 at 5:07 PM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:

> So, this might also map to a certain drive-strength, but is not really
> mapping well to Zynq pin configuration options.
> Hence, I thought introducing this new property which allows a more
> flexible interpretation of the argument would be a better way and might
> also help other pinctrl cases (though this is speculative).

Maybe a new property is needed, but it doesn't seem generic enough.

It may be that you need to have a combination of generic and custom
config options (which is fine).

It may need some tweaking of the generic DT config parser to allow
custom attributes but that's needed one of these days anyway.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index 98eb94d91a1c..862c4fe17d04 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -184,6 +184,7 @@  low-power-disable	- disable low power mode
 output-low		- set the pin to output mode with low level
 output-high		- set the pin to output mode with high level
 slew-rate		- set the slew rate
+io-standard		- set the IO standard
 
 For example:
 
@@ -215,5 +216,7 @@  arguments are described below.
 - input-debounce takes the debounce time in usec as argument
   or 0 to disable debouncing
 
+- io-standard takes a driver specific argument to select an IO standard
+
 More in-depth documentation on these parameters can be found in
 <include/linux/pinctrl/pinconfig-generic.h>
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index e28ef957ca2d..17ac8f00e16b 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -58,6 +58,7 @@  static struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
 	PCONFDUMP(PIN_CONFIG_LOW_POWER_MODE, "pin low power", "mode", true),
 	PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level", true),
+	PCONFDUMP(PIN_CONFIG_IOSTANDARD, "io standard", NULL, true),
 };
 
 void pinconf_generic_dump_pin(struct pinctrl_dev *pctldev,
@@ -181,6 +182,7 @@  static struct pinconf_generic_dt_params dt_params[] = {
 	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
 	{ "output-high", PIN_CONFIG_OUTPUT, 1, },
 	{ "slew-rate", PIN_CONFIG_SLEW_RATE, 0},
+	{ "io-standard", PIN_CONFIG_IOSTANDARD, 0},
 };
 
 /**
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index d578a60eff23..dd1d3251fb93 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -92,6 +92,9 @@ 
  * @PIN_CONFIG_END: this is the last enumerator for pin configurations, if
  *	you need to pass in custom configurations to the pin controller, use
  *	PIN_CONFIG_END+1 as the base offset.
+ * @PIN_CONFIG_IOSTANDARD: if the pin can select an IO standard, the argument to
+ *	this parameter (on a custom format) tells the driver which alternative
+ *	IO standard to use.
  */
 enum pin_config_param {
 	PIN_CONFIG_BIAS_DISABLE,
@@ -112,6 +115,7 @@  enum pin_config_param {
 	PIN_CONFIG_SLEW_RATE,
 	PIN_CONFIG_LOW_POWER_MODE,
 	PIN_CONFIG_OUTPUT,
+	PIN_CONFIG_IOSTANDARD,
 	PIN_CONFIG_END = 0x7FFF,
 };