Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1671632 > unrolled thread

[PATCH v4 1/3] pinctrl: Add sleep related configuration

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 1/3] pinctrl: Add sleep related configuration Baolin Wang <baolin.wang@spreadtrum.com> - 2017-06-21 14:00 +0200
    [PATCH v4 2/3] dt-bindings: pinctrl: Add Spreadtrum SC9860 pin controller Baolin Wang <baolin.wang@spreadtrum.com> - 2017-06-21 14:10 +0200
    Re: [PATCH v4 1/3] pinctrl: Add sleep related configuration Rob Herring <robh@kernel.org> - 2017-06-26 18:20 +0200
      Re: [PATCH v4 1/3] pinctrl: Add sleep related configuration Baolin Wang <baolin.wang@spreadtrum.com> - 2017-06-27 10:30 +0200
        Re: [PATCH v4 1/3] pinctrl: Add sleep related configuration Baolin Wang <baolin.wang@spreadtrum.com> - 2017-06-27 14:20 +0200

#1671632 — [PATCH v4 1/3] pinctrl: Add sleep related configuration

In some scenarios, we should set some pins as input/output/pullup/pulldown
when the specified system goes into deep sleep mode, then when the system
goes into deep sleep mode, these pins will be set automatically by hardware.

Usually we can set the "sleep" state to set sleep related config, but one SoC
usually has not only one system (especially for mobile SoC), some systems on
the SoC which did not run linux kernel, they can not select the "sleep" state
when they go into deep sleep mode.

Thus we introduce some sleep related config into pinconf-generic for users to
configure.

Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com>
---
 - Add this patch since v4.
---
 .../bindings/pinctrl/pinctrl-bindings.txt          |   12 ++++++++++++
 drivers/pinctrl/pinconf-generic.c                  |   10 ++++++++++
 include/linux/pinctrl/pinconf-generic.h            |   14 ++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index bf3f7b0..e098059 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -236,6 +236,18 @@ low-power-enable	- enable low power mode
 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
+sleep-bias-pull-up	- pull up the pin when the specified system goes into
+			  deep sleep mode
+sleep-bias-pull-down	- pull down the pin when the specified system goes into
+			  deep sleep mode
+sleep-input-enable	- enable input on pin when the specified system goes
+			  into deep sleep mode (no effect on output)
+sleep-intput-disable	- disable input on pin when the specified system goes
+			  into deep sleep mode (no effect on output)
+sleep-output-low	- set the pin to output mode with low level when the
+			  specified system goes into deep sleep mode
+sleep-output-high	- set the pin to output mode with high level when the
+			  specified system goes into deep sleep mode
 slew-rate		- set the slew rate
 
 For example:
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index ce3335a..2847026 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -46,6 +46,10 @@
 	PCONFDUMP(PIN_CONFIG_LOW_POWER_MODE, "pin low power", "mode", true),
 	PCONFDUMP(PIN_CONFIG_OUTPUT, "pin output", "level", true),
 	PCONFDUMP(PIN_CONFIG_POWER_SOURCE, "pin power source", "selector", true),
+	PCONFDUMP(PIN_CONFIG_SLEEP_BIAS_PULL_DOWN, "sleep input bias pull down", NULL, false),
+	PCONFDUMP(PIN_CONFIG_SLEEP_BIAS_PULL_UP, "sleep input bias pull up", NULL, false),
+	PCONFDUMP(PIN_CONFIG_SLEEP_INPUT_ENABLE, "sleep input enabled", NULL, false),
+	PCONFDUMP(PIN_CONFIG_SLEEP_OUTPUT, "sleep pin output", NULL, true),
 	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
 };
 
@@ -175,6 +179,12 @@ void pinconf_generic_dump_config(struct pinctrl_dev *pctldev,
 	{ "output-high", PIN_CONFIG_OUTPUT, 1, },
 	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
 	{ "power-source", PIN_CONFIG_POWER_SOURCE, 0 },
+	{ "sleep-bias-pull-down", PIN_CONFIG_SLEEP_BIAS_PULL_DOWN, 1 },
+	{ "sleep-bias-pull-up", PIN_CONFIG_SLEEP_BIAS_PULL_UP, 1 },
+	{ "sleep-input-disable", PIN_CONFIG_SLEEP_INPUT_ENABLE, 0 },
+	{ "sleep-input-enable", PIN_CONFIG_SLEEP_INPUT_ENABLE, 1 },
+	{ "sleep-output-high", PIN_CONFIG_SLEEP_OUTPUT, 1 },
+	{ "sleep-output-low", PIN_CONFIG_SLEEP_OUTPUT, 0 },
 	{ "slew-rate", PIN_CONFIG_SLEW_RATE, 0 },
 };
 
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index 7620eb1..3deb6bc 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -80,6 +80,16 @@
  * @PIN_CONFIG_POWER_SOURCE: if the pin can select between different power
  *	supplies, the argument to this parameter (on a custom format) tells
  *	the driver which alternative power source to use.
+ * @PIN_CONFIG_SLEEP_BIAS_PULL_DOWN: the pin will be pulled down when the
+ *	specified system goes into deep sleep mode.
+ * @PIN_CONFIG_SLEEP_BIAS_PULL_UP: the pin will be pulled up when the specified
+ *	system goes into deep sleep mode.
+ * @PIN_CONFIG_SLEEP_INPUT_ENABLE: enable the pin's input when the specified
+ *	system goes into deep sleep mode.  Note that this does not affect the
+ *	pin's ability to drive output.  1 enables input, 0 disables input.
+ * @PIN_CONFIG_SLEEP_OUTPUT: this will configure the pin as an output when the
+ *	specified system goes into deep sleep mode. Use argument 1 to indicate
+ *	high level, argument 0 to indicate low level.
  * @PIN_CONFIG_SLEW_RATE: if the pin can select slew rate, the argument to
  *	this parameter (on a custom format) tells the driver which alternative
  *	slew rate to use.
@@ -107,6 +117,10 @@ enum pin_config_param {
 	PIN_CONFIG_LOW_POWER_MODE,
 	PIN_CONFIG_OUTPUT,
 	PIN_CONFIG_POWER_SOURCE,
+	PIN_CONFIG_SLEEP_BIAS_PULL_DOWN,
+	PIN_CONFIG_SLEEP_BIAS_PULL_UP,
+	PIN_CONFIG_SLEEP_INPUT_ENABLE,
+	PIN_CONFIG_SLEEP_OUTPUT,
 	PIN_CONFIG_SLEW_RATE,
 	PIN_CONFIG_END = 0x7F,
 	PIN_CONFIG_MAX = 0xFF,
-- 
1.7.9.5

[toc] | [next] | [standalone]

#1671635 — [PATCH v4 2/3] dt-bindings: pinctrl: Add Spreadtrum SC9860 pin controller

This patch adds the binding documentation for Spreadtrum SC9860 pin
controller device.

Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com>
---
Changes since v3:
 - Use generic "bias-pull-up" instead of "sprd,pull-up".
 - Change subject name.
 - Use generic sleep related config.
 - Add more explanation for sleep related configuration and sleep mode.

Changes since v2:
 - No updates.

Changes since v1:
 - Remove magic numbers and get to use the standard bindings.
 - Fix some typos.
---
 .../devicetree/bindings/pinctrl/sprd,pinctrl.txt   |   90 ++++++++++++++++++++
 .../bindings/pinctrl/sprd,sc9860-pinctrl.txt       |   69 +++++++++++++++
 2 files changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt
 create mode 100644 Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt
new file mode 100644
index 0000000..e33fb46
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/sprd,pinctrl.txt
@@ -0,0 +1,90 @@
+* Spreadtrum Pin Controller
+
+The Spreadtrum pin controller are organized in 3 blocks (types).
+
+The first block comprises some global control registers, and each
+register contains several bit fields with one bit or several bits
+to configure for some global common configuration, such as domain
+pad driving level, system control select and so on ("domain pad
+driving level": One pin can output 3.0v or 1.8v, depending on the
+related domain pad driving selection, if the related domain pad
+slect 3.0v, then the pin can output 3.0v. "system control" is used
+to choose one function (like: UART0) for which system, since we
+have several systems (AP/CP/CM4) on one SoC.).
+
+There are too much various configuration that we can not list all
+of them, so we can not make every Spreadtrum-special configuration
+as one generic configuration, and maybe it will add more strange
+global configuration in future. Then we add one "sprd,control" to
+set these various global control configuration, and we need use
+magic number for this property.
+
+Moreover we recognise every fields comprising one bit or several
+bits in one global control register as one pin, thus we should
+record every pin's bit offset, bit width and register offset to
+configure this field (pin).
+
+The second block comprises some common registers which have unified
+register definition, and each register described one pin is used
+to configure the pin sleep mode, function select and sleep related
+configuration.
+
+Now we have 4 systems for sleep mode on SC9860 SoC: AP system,
+PUBCP system, TGLDSP system and AGDSP system. And the pin sleep
+related configuration are:
+- sleep-input-enable
+- sleep-input-disable
+- sleep-output-high
+- sleep-output-low
+- sleep-bias-pull-up
+- sleep-bias-pull-down
+
+In some situation we need set the pin sleep mode and pin sleep related
+configuration, to set the pin sleep related configuration automatically
+by hardware when the system specified by sleep mode goes into deep
+sleep mode. For example, if we set the pin sleep mode as PUBCP_SLEEP
+and set the pin sleep related configuration as "sleep-input-enable",
+which means when PUBCP system goes into deep sleep mode, this pin will
+be set input enable automatically.
+
+Moreover we can not use the "sleep" state, since some systems (like:
+PUBCP system) do not run linux kernel OS (only AP system run linux
+kernel on SC9860 platform), then we can not select "sleep" state
+when the PUBCP system goes into deep sleep mode. Thus we introduce
+"sprd,sleep-mode" property to set pin sleep mode.
+
+The last block comprises some misc registers which also have unified
+register definition, and each register described one pin is used to
+configure drive strength, pull up/down and so on. Especially for pull
+up, we have two kind pull up resistor: 20K and 4.7K.
+
+Required properties for Spreadtrum pin controller:
+- compatible: "sprd,<soc>-pinctrl"
+  Please refer to each sprd,<soc>-pinctrl.txt binding doc for supported SoCs.
+- reg: The register address of pin controller device.
+- pins : An array of pin names.
+
+Optional properties:
+- function: Specified the function name.
+- drive-strength: Drive strength in mA.
+- input-schmitt-disable: Enable schmitt-trigger mode.
+- input-schmitt-enable: Disable schmitt-trigger mode.
+- bias-disable: Disable pin bias.
+- bias-pull-down: Pull down on pin.
+- bias-pull-up: Pull up on pin.
+- sleep-input-enable: Input enable when the system specified by sleep
+		      mode goes into deep sleep mode.
+- sleep-input-disable: Input disable when the system specified by sleep
+		       mode goes into deep sleep mode.
+- sleep-output-high: Output high enable when the system specified by
+		     sleep mode goes into deep sleep mode.
+- sleep-output-low: Output low enable when the system specified by sleep
+		    mode goes into deep sleep mode.
+- sleep-bias-pull-up: Pull up enable when the system specified by sleep
+		      mode goes into deep sleep mode.
+- sleep-bias-pull-down: Pull down enable when the system specified by
+			sleep mode goes into deep sleep mode.
+- sprd,control: Control values referring to databook for global control pins.
+- sprd,sleep-mode: Sleep mode selection.
+
+Please refer to each sprd,<soc>-pinctrl.txt binding doc for supported values.
diff --git a/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt
new file mode 100644
index 0000000..067b7e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/sprd,sc9860-pinctrl.txt
@@ -0,0 +1,69 @@
+* Spreadtrum SC9860 Pin Controller
+
+Please refer to sprd,pinctrl.txt in this directory for common binding part
+and usage.
+
+Required properties:
+- compatible: Must be "sprd,sc9860-pinctrl".
+- reg: The register address of pin controller device.
+- pins : An array of strings, each string containing the name of a pin.
+
+Optional properties:
+- function: A string containing the name of the function, values must be
+  one of: "func1", "func2", "func3" and "func4".
+- drive-strength: Drive strength in mA. Supported values: 2, 4, 6, 8, 10,
+  12, 14, 16, 20, 21, 24, 25, 27, 29, 31 and 33.
+- input-schmitt-disable: Enable schmitt-trigger mode.
+- input-schmitt-enable: Disable schmitt-trigger mode.
+- bias-disable: Disable pin bias.
+- bias-pull-down: Pull down on pin.
+- bias-pull-up: Pull up on pin. Supported values: 20000 for pull-up resistor
+  is 20K and 4700 for pull-up resistor is 4.7K.
+- sleep-input-enable: Input enable when the system specified by sleep mode
+  goes into deep sleep mode.
+- sleep-input-disable: Input disable when the system specified by sleep mode
+  goes into deep sleep mode.
+- sleep-output-high: Output high enable when the system specified by sleep
+  mode goes into deep sleep mode.
+- sleep-output-low: Output low enable when the system specified by sleep mode
+  goes into deep sleep mode.
+- sleep-bias-pull-up: Pull up enable when the system specified by sleep mode
+  goes into deep sleep mode.
+- sleep-bias-pull-down: Pull down enable when the system specified by sleep
+  mode goes into deep sleep mode.
+- sprd,control: Control values referring to databook for global control pins.
+- sprd,sleep-mode: Choose the pin sleep mode, and supported values are:
+  AP_SLEEP, PUBCP_SLEEP, TGLDSP_SLEEP and AGDSP_SLEEP.
+
+Pin sleep mode definition:
+enum pin_sleep_mode {
+	AP_SLEEP = BIT(0),
+	PUBCP_SLEEP = BIT(1),
+	TGLDSP_SLEEP = BIT(2),
+	AGDSP_SLEEP = BIT(3),
+};
+
+Example:
+pin_controller: pinctrl@402a0000 {
+	compatible = "sprd,sc9860-pinctrl";
+	reg = <0x402a0000 0x10000>;
+
+	grp1: sd0 {
+		pins = "SC9860_VIO_SD2_IRTE", "SC9860_VIO_SD0_IRTE";
+		sprd,control = <0x1>;
+	};
+
+	grp2: rfctl_33 {
+		pins = "SC9860_RFCTL33";
+		function = "func2";
+		sprd,sleep-mode = <AP_SLEEP | PUBCP_SLEEP>;
+		sleep-output-low;
+	};
+
+	grp3: rfctl_misc_20 {
+		pins = "SC9860_RFCTL20_MISC";
+		drive-strength = <10>;
+		bias-pull-up = <4700>;
+		sleep-bias-pull-up;
+	};
+};
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1674923

On Wed, Jun 21, 2017 at 07:55:37PM +0800, Baolin Wang wrote:
> In some scenarios, we should set some pins as input/output/pullup/pulldown
> when the specified system goes into deep sleep mode, then when the system
> goes into deep sleep mode, these pins will be set automatically by hardware.
> 
> Usually we can set the "sleep" state to set sleep related config, but one SoC
> usually has not only one system (especially for mobile SoC), some systems on
> the SoC which did not run linux kernel, they can not select the "sleep" state
> when they go into deep sleep mode.

The wording here is not very clear. I think what you mean is some pins 
are not controlled by any specific driver in the OS, but need to be 
controlled when entering sleep mode.

> Thus we introduce some sleep related config into pinconf-generic for users to
> configure.
> 
> Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com>
> ---
>  - Add this patch since v4.
> ---
>  .../bindings/pinctrl/pinctrl-bindings.txt          |   12 ++++++++++++
>  drivers/pinctrl/pinconf-generic.c                  |   10 ++++++++++
>  include/linux/pinctrl/pinconf-generic.h            |   14 ++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> index bf3f7b0..e098059 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> @@ -236,6 +236,18 @@ low-power-enable	- enable low power mode
>  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
> +sleep-bias-pull-up	- pull up the pin when the specified system goes into
> +			  deep sleep mode
> +sleep-bias-pull-down	- pull down the pin when the specified system goes into
> +			  deep sleep mode
> +sleep-input-enable	- enable input on pin when the specified system goes
> +			  into deep sleep mode (no effect on output)
> +sleep-intput-disable	- disable input on pin when the specified system goes
> +			  into deep sleep mode (no effect on output)
> +sleep-output-low	- set the pin to output mode with low level when the
> +			  specified system goes into deep sleep mode
> +sleep-output-high	- set the pin to output mode with high level when the
> +			  specified system goes into deep sleep mode
>  slew-rate		- set the slew rate

I don't really like having 2 ways to define pin setup and this doesn't 
scale if I need to define 3 states. Couldn't we create pin state 
definitions and have a pinctrl-n property within the pin controller 
node to handle all the unhandled pins?

Rob

[toc] | [prev] | [next] | [standalone]

#1675409

On 一,  6月 26, 2017 at 11:13:48上午 -0500, Rob Herring wrote:
> On Wed, Jun 21, 2017 at 07:55:37PM +0800, Baolin Wang wrote:
> > In some scenarios, we should set some pins as input/output/pullup/pulldown
> > when the specified system goes into deep sleep mode, then when the system
> > goes into deep sleep mode, these pins will be set automatically by hardware.
> > 
> > Usually we can set the "sleep" state to set sleep related config, but one SoC
> > usually has not only one system (especially for mobile SoC), some systems on
> > the SoC which did not run linux kernel, they can not select the "sleep" state
> > when they go into deep sleep mode.
> 
> The wording here is not very clear. I think what you mean is some pins 
> are not controlled by any specific driver in the OS, but need to be 
> controlled when entering sleep mode.

Yes, that is what I meaning, sorry for confusing.

> 
> > Thus we introduce some sleep related config into pinconf-generic for users to
> > configure.
> > 
> > Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com>
> > ---
> >  - Add this patch since v4.
> > ---
> >  .../bindings/pinctrl/pinctrl-bindings.txt          |   12 ++++++++++++
> >  drivers/pinctrl/pinconf-generic.c                  |   10 ++++++++++
> >  include/linux/pinctrl/pinconf-generic.h            |   14 ++++++++++++++
> >  3 files changed, 36 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > index bf3f7b0..e098059 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > @@ -236,6 +236,18 @@ low-power-enable	- enable low power mode
> >  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
> > +sleep-bias-pull-up	- pull up the pin when the specified system goes into
> > +			  deep sleep mode
> > +sleep-bias-pull-down	- pull down the pin when the specified system goes into
> > +			  deep sleep mode
> > +sleep-input-enable	- enable input on pin when the specified system goes
> > +			  into deep sleep mode (no effect on output)
> > +sleep-intput-disable	- disable input on pin when the specified system goes
> > +			  into deep sleep mode (no effect on output)
> > +sleep-output-low	- set the pin to output mode with low level when the
> > +			  specified system goes into deep sleep mode
> > +sleep-output-high	- set the pin to output mode with high level when the
> > +			  specified system goes into deep sleep mode
> >  slew-rate		- set the slew rate
> 
> I don't really like having 2 ways to define pin setup and this doesn't 
> scale if I need to define 3 states. Couldn't we create pin state 
> definitions and have a pinctrl-n property within the pin controller 
> node to handle all the unhandled pins?

As LinusW also suggest we can create one "sleep" state and program them
into registers at early point (like: after probing pinctrl driver). So I
think I can introduce one called "early-sleep" state which need select it
after initializing pinctrl driver.

> 
> Rob

[toc] | [prev] | [next] | [standalone]

#1675611

Hi,

On 二,  6月 27, 2017 at 04:21:32下午 +0800, Baolin Wang wrote:
> On 一,  6月 26, 2017 at 11:13:48上午 -0500, Rob Herring wrote:
> > On Wed, Jun 21, 2017 at 07:55:37PM +0800, Baolin Wang wrote:
> > > In some scenarios, we should set some pins as input/output/pullup/pulldown
> > > when the specified system goes into deep sleep mode, then when the system
> > > goes into deep sleep mode, these pins will be set automatically by hardware.
> > > 
> > > Usually we can set the "sleep" state to set sleep related config, but one SoC
> > > usually has not only one system (especially for mobile SoC), some systems on
> > > the SoC which did not run linux kernel, they can not select the "sleep" state
> > > when they go into deep sleep mode.
> > 
> > The wording here is not very clear. I think what you mean is some pins 
> > are not controlled by any specific driver in the OS, but need to be 
> > controlled when entering sleep mode.
> 
> Yes, that is what I meaning, sorry for confusing.
> 
> > 
> > > Thus we introduce some sleep related config into pinconf-generic for users to
> > > configure.
> > > 
> > > Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com>
> > > ---
> > >  - Add this patch since v4.
> > > ---
> > >  .../bindings/pinctrl/pinctrl-bindings.txt          |   12 ++++++++++++
> > >  drivers/pinctrl/pinconf-generic.c                  |   10 ++++++++++
> > >  include/linux/pinctrl/pinconf-generic.h            |   14 ++++++++++++++
> > >  3 files changed, 36 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > > index bf3f7b0..e098059 100644
> > > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > > @@ -236,6 +236,18 @@ low-power-enable	- enable low power mode
> > >  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
> > > +sleep-bias-pull-up	- pull up the pin when the specified system goes into
> > > +			  deep sleep mode
> > > +sleep-bias-pull-down	- pull down the pin when the specified system goes into
> > > +			  deep sleep mode
> > > +sleep-input-enable	- enable input on pin when the specified system goes
> > > +			  into deep sleep mode (no effect on output)
> > > +sleep-intput-disable	- disable input on pin when the specified system goes
> > > +			  into deep sleep mode (no effect on output)
> > > +sleep-output-low	- set the pin to output mode with low level when the
> > > +			  specified system goes into deep sleep mode
> > > +sleep-output-high	- set the pin to output mode with high level when the
> > > +			  specified system goes into deep sleep mode
> > >  slew-rate		- set the slew rate
> > 
> > I don't really like having 2 ways to define pin setup and this doesn't 
> > scale if I need to define 3 states. Couldn't we create pin state 
> > definitions and have a pinctrl-n property within the pin controller 
> > node to handle all the unhandled pins?
> 
> As LinusW also suggest we can create one "sleep" state and program them
> into registers at early point (like: after probing pinctrl driver). So I
> think I can introduce one called "early-sleep" state which need select it
> after initializing pinctrl driver.

After more investigation, I do not think it is a good solution to create one
state containing sleep related configs and select it at early point.

First these sleep related configs are pins' attributes, they should be set
depanding on users situation, which means the state containing sleep related
configs should be selected by users. We can not create one "sleep-xxx" state
containing all pins' sleep related configs, and select it when initializing
pinctrl driver.

Second if we create one "sleep-xxx" state containing sleep related configs,
which means we should set one pin's configuration in 2 places.

If we introduce "sleep-input-enable" config, we can set the pin's config
as below:
vio_sd0_ms_3: regctrl3 {
	pins = "SC9860_RFCTL30", "SC9860_RFCTL31", "SC9860_RFCTL32";
	function = "func1";
	sprd,sleep-mode = <0x3>;
	sleep-input-enable;
};

But If we create one extra "sleep-xxx" state for sleep-related configs,
it will be like:
grp1: regctrl3 {
	pins = "SC9860_RFCTL30", "SC9860_RFCTL31";
	function = "func1";
	sprd,sleep-mode = <0x3>;
};

sleep-input: input_grp {
	pins = "SC9860_RFCTL30", "SC9860_RFCTL31", "SC9860_RFCTL32";
	input-enable;
};

pinctrl-names = "sleep-input";
pinctrl-0 = <&sleep-input>;

"sleep-input" state will be selected when initializing pinctrl driver, "grp1"
will be selected by user to set other pin configuration.

Then we need config "SC9860_RFCTL30" pin in 2 different places, which is
more inconvenient for users.

Accoring to above explanation, I think we should introduce these standard
sleep related configs for users, but if you still have strong objection for
it, I think I should introduce some SoC-specific attributes (something like
"sprd,sleep-input") for our driver. LinusW and Rob, do you have any good
suggestion? Thanks.

> 
> > 
> > Rob

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web