[PATCH] dt: pinctrl: Document device tree binding

[PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

The core pin controller bindings define:
* The fact that pin controllers expose pin configurations as nodes in
  device tree.
* That the bindings for those pin configuration nodes is defined by the
  individual pin controller drivers.
* A standardized set of properties for client devices to define numbered
  or named pin configuration states, each referring to some number of the
  afore-mentioned pin configuration nodes.
* That the bindings for the client devices determines the set of numbered
  or named states that must exist.

Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
---
 .../bindings/pinctrl/pinctrl-bindings.txt          |  118 ++++++++++++++++++++
 1 files changed, 118 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
new file mode 100644
index 0000000..cce9f01
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -0,0 +1,118 @@
+== Introduction ==
+
+Hardware modules that control pin multiplexing or configuration parameters
+such as pull-up/down, tri-state, drive-strength etc are designated as pin
+controllers. Each pin controller must be represented as a node in device tree,
+just like any other hardware module.
+
+Hardware modules whose signals are affected by pin configuration are
+designated client devices. Again, each client device must be represented as a
+node in device tree, just like any other hardware module.
+
+For a client device to operate correctly, certain pin controllers must
+set up certain specific pin configurations. Some client devices need a
+single static pin configuration, e.g. set up during initialization. Others
+need to reconfigure pins at run-time, for example to tri-state pins when the
+device is inactive. Hence, each client device can define a set of named
+states. The number and names of those states is defined by the client device's
+own binding.
+
+The common pinctrl bindings defined in this file provide an infra-structure
+for client device device tree nodes to map those state names to the pin
+configuration used by those states.
+
+Note that pin controllers themselves may also be client devices of themselves.
+For example, a pin controller may set up its own "active" state when the
+driver loads. This would allow representing a board's static pin configuration
+in a single place, rather than splitting it across multiple client device
+nodes. The decision to do this or not somewhat rests with the author of
+individual board device tree files, and any requirements imposed by the
+bindings for the individual client devices in use by that board, i.e. whether
+they require certain specific named states for dynamic pin configuration.
+
+== Pinctrl client devices ==
+
+For each client device individually, every pin state is assigned an integer
+ID. These numbers start at 0, and are contiguous. For each state ID, a unique
+property exists to define the pin configuration. Each state may also be
+assigned a name. When names are used, another property exists to map from
+those names to the integer IDs.
+
+Each client device's own binding determines the set of states the must be
+defined in its device tree node, and whether to define the set of state
+IDs that must be provided, or whether to define the set of state names that
+must be provided.
+
+Required properties:
+pinctrl-0:	List of phandles, each pointing at a pin configuration
+		node. These referenced pin configuration nodes must be child
+		nodes of the pin controller that they configure. Multiple
+		entries may exist in this list so that multiple pin
+		controllers may be configured, or so that a state may be built
+		from multiple nodes for a single pin controller, each
+		contributing part of the overall configuration. See the next
+		section of this document for details of the format of these
+		pin configuration nodes.
+
+		In some cases, it may be useful to define a state, but for it
+		to be empty. This may be required when a common IP block is
+		used in an SoC either without a pin controller, or where the
+		pin controller does not affect the HW module in question. If
+		the binding for that IP block requires certain pin states to
+		exist, they must still be defined, but may be left empty.
+
+Optional properties:
+pinctrl-1:	List of phandles, each pointing at a pin configuration
+		node within a pin controller.
+...
+pinctrl-n:	List of phandles, each pointing at a pin configuration
+		node within a pin controller.
+pinctrl-names:	The list of names to assign states. List entry 0 defines the
+		name for integer state ID 0, list entry 1 for state ID 1, and
+		so on.
+
+For example:
+
+	/* For a client device requiring named states */
+	device {
+		pinctrl-names = "active", "idle";
+		pinctrl-0 = <&state_0_node_a>;
+		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
+	};
+
+	/* For the same device if using state IDs */
+	device {
+		pinctrl-0 = <&state_0_node_a>;
+		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
+	};
+
+== Pin controller devices ==
+
+Pin controller devices should contain the pin configuration nodes that client
+devices reference.
+
+For example:
+
+	pincontroller {
+		... /* Standard DT properties for the device itself elided */
+
+		state_0_node_a {
+			...
+		};
+		state_1_node_a {
+			...
+		};
+		state_1_node_b {
+			...
+		};
+	}
+
+The contents of each of those pin configuration child nodes is defined
+entirely by the binding for the individual pin controller device. There
+exists no common standard for this content.
+
+The pin configuration nodes need not be direct children of the pin controller
+device; they may be grand-children for example. Whether this is legal, and
+whether there is any interaction between the child and intermediate parent
+nodes, is again defined entirely by the binding for the individual pin
+controller device.
-- 
1.7.0.4

Re: [PATCH] dt: pinctrl: Document device tree binding

[Randy Dunlap]

On 03/09/2012 10:14 AM, Stephen Warren wrote:

> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
>  .../bindings/pinctrl/pinctrl-bindings.txt          |  118 ++++++++++++++++++++
>  1 files changed, 118 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> new file mode 100644
> index 0000000..cce9f01
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> @@ -0,0 +1,118 @@
> +== Introduction ==
> +
> +Hardware modules that control pin multiplexing or configuration parameters
> +such as pull-up/down, tri-state, drive-strength etc are designated as pin
> +controllers. Each pin controller must be represented as a node in device tree,
> +just like any other hardware module.
> +
> +Hardware modules whose signals are affected by pin configuration are
> +designated client devices. Again, each client device must be represented as a
> +node in device tree, just like any other hardware module.
> +
> +For a client device to operate correctly, certain pin controllers must
> +set up certain specific pin configurations. Some client devices need a
> +single static pin configuration, e.g. set up during initialization. Others
> +need to reconfigure pins at run-time, for example to tri-state pins when the
> +device is inactive. Hence, each client device can define a set of named
> +states. The number and names of those states is defined by the client device's
> +own binding.
> +
> +The common pinctrl bindings defined in this file provide an infra-structure


                                                               infrastructure

> +for client device device tree nodes to map those state names to the pin


              ^^^^^^^^^^^^^ on purpose?

> +configuration used by those states.
> +
> +Note that pin controllers themselves may also be client devices of themselves.
> +For example, a pin controller may set up its own "active" state when the
> +driver loads. This would allow representing a board's static pin configuration
> +in a single place, rather than splitting it across multiple client device
> +nodes. The decision to do this or not somewhat rests with the author of
> +individual board device tree files, and any requirements imposed by the
> +bindings for the individual client devices in use by that board, i.e. whether
> +they require certain specific named states for dynamic pin configuration.
> +
> +== Pinctrl client devices ==
> +
> +For each client device individually, every pin state is assigned an integer
> +ID. These numbers start at 0, and are contiguous. For each state ID, a unique
> +property exists to define the pin configuration. Each state may also be
> +assigned a name. When names are used, another property exists to map from
> +those names to the integer IDs.
> +
> +Each client device's own binding determines the set of states the must be
> +defined in its device tree node, and whether to define the set of state
> +IDs that must be provided, or whether to define the set of state names that
> +must be provided.
> +
> +Required properties:
> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> +		node. These referenced pin configuration nodes must be child
> +		nodes of the pin controller that they configure. Multiple
> +		entries may exist in this list so that multiple pin
> +		controllers may be configured, or so that a state may be built
> +		from multiple nodes for a single pin controller, each
> +		contributing part of the overall configuration. See the next
> +		section of this document for details of the format of these
> +		pin configuration nodes.
> +
> +		In some cases, it may be useful to define a state, but for it
> +		to be empty. This may be required when a common IP block is
> +		used in an SoC either without a pin controller, or where the
> +		pin controller does not affect the HW module in question. If
> +		the binding for that IP block requires certain pin states to
> +		exist, they must still be defined, but may be left empty.
> +
> +Optional properties:
> +pinctrl-1:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +...
> +pinctrl-n:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +pinctrl-names:	The list of names to assign states. List entry 0 defines the
> +		name for integer state ID 0, list entry 1 for state ID 1, and
> +		so on.
> +
> +For example:
> +
> +	/* For a client device requiring named states */
> +	device {
> +		pinctrl-names = "active", "idle";
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +	/* For the same device if using state IDs */
> +	device {
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +== Pin controller devices ==
> +
> +Pin controller devices should contain the pin configuration nodes that client
> +devices reference.
> +
> +For example:
> +
> +	pincontroller {
> +		... /* Standard DT properties for the device itself elided */
> +
> +		state_0_node_a {
> +			...
> +		};
> +		state_1_node_a {
> +			...
> +		};
> +		state_1_node_b {
> +			...
> +		};
> +	}
> +
> +The contents of each of those pin configuration child nodes is defined
> +entirely by the binding for the individual pin controller device. There
> +exists no common standard for this content.
> +
> +The pin configuration nodes need not be direct children of the pin controller
> +device; they may be grand-children for example. Whether this is legal, and


                       grandchildren, for example.

> +whether there is any interaction between the child and intermediate parent
> +nodes, is again defined entirely by the binding for the individual pin
> +controller device.



-- 
~Randy

Re: [PATCH] dt: pinctrl: Document device tree binding

[Randy Dunlap]

On 03/09/2012 10:14 AM, Stephen Warren wrote:

> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> ---
>  .../bindings/pinctrl/pinctrl-bindings.txt          |  118 ++++++++++++++++++++
>  1 files changed, 118 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> new file mode 100644
> index 0000000..cce9f01
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> @@ -0,0 +1,118 @@
> +== Introduction ==
> +
> +Hardware modules that control pin multiplexing or configuration parameters
> +such as pull-up/down, tri-state, drive-strength etc are designated as pin
> +controllers. Each pin controller must be represented as a node in device tree,
> +just like any other hardware module.
> +
> +Hardware modules whose signals are affected by pin configuration are
> +designated client devices. Again, each client device must be represented as a
> +node in device tree, just like any other hardware module.
> +
> +For a client device to operate correctly, certain pin controllers must
> +set up certain specific pin configurations. Some client devices need a
> +single static pin configuration, e.g. set up during initialization. Others
> +need to reconfigure pins at run-time, for example to tri-state pins when the
> +device is inactive. Hence, each client device can define a set of named
> +states. The number and names of those states is defined by the client device's
> +own binding.
> +
> +The common pinctrl bindings defined in this file provide an infra-structure


                                                               infrastructure

> +for client device device tree nodes to map those state names to the pin


              ^^^^^^^^^^^^^ on purpose?

> +configuration used by those states.
> +
> +Note that pin controllers themselves may also be client devices of themselves.
> +For example, a pin controller may set up its own "active" state when the
> +driver loads. This would allow representing a board's static pin configuration
> +in a single place, rather than splitting it across multiple client device
> +nodes. The decision to do this or not somewhat rests with the author of
> +individual board device tree files, and any requirements imposed by the
> +bindings for the individual client devices in use by that board, i.e. whether
> +they require certain specific named states for dynamic pin configuration.
> +
> +== Pinctrl client devices ==
> +
> +For each client device individually, every pin state is assigned an integer
> +ID. These numbers start at 0, and are contiguous. For each state ID, a unique
> +property exists to define the pin configuration. Each state may also be
> +assigned a name. When names are used, another property exists to map from
> +those names to the integer IDs.
> +
> +Each client device's own binding determines the set of states the must be
> +defined in its device tree node, and whether to define the set of state
> +IDs that must be provided, or whether to define the set of state names that
> +must be provided.
> +
> +Required properties:
> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> +		node. These referenced pin configuration nodes must be child
> +		nodes of the pin controller that they configure. Multiple
> +		entries may exist in this list so that multiple pin
> +		controllers may be configured, or so that a state may be built
> +		from multiple nodes for a single pin controller, each
> +		contributing part of the overall configuration. See the next
> +		section of this document for details of the format of these
> +		pin configuration nodes.
> +
> +		In some cases, it may be useful to define a state, but for it
> +		to be empty. This may be required when a common IP block is
> +		used in an SoC either without a pin controller, or where the
> +		pin controller does not affect the HW module in question. If
> +		the binding for that IP block requires certain pin states to
> +		exist, they must still be defined, but may be left empty.
> +
> +Optional properties:
> +pinctrl-1:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +...
> +pinctrl-n:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +pinctrl-names:	The list of names to assign states. List entry 0 defines the
> +		name for integer state ID 0, list entry 1 for state ID 1, and
> +		so on.
> +
> +For example:
> +
> +	/* For a client device requiring named states */
> +	device {
> +		pinctrl-names = "active", "idle";
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +	/* For the same device if using state IDs */
> +	device {
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +== Pin controller devices ==
> +
> +Pin controller devices should contain the pin configuration nodes that client
> +devices reference.
> +
> +For example:
> +
> +	pincontroller {
> +		... /* Standard DT properties for the device itself elided */
> +
> +		state_0_node_a {
> +			...
> +		};
> +		state_1_node_a {
> +			...
> +		};
> +		state_1_node_b {
> +			...
> +		};
> +	}
> +
> +The contents of each of those pin configuration child nodes is defined
> +entirely by the binding for the individual pin controller device. There
> +exists no common standard for this content.
> +
> +The pin configuration nodes need not be direct children of the pin controller
> +device; they may be grand-children for example. Whether this is legal, and


                       grandchildren, for example.

> +whether there is any interaction between the child and intermediate parent
> +nodes, is again defined entirely by the binding for the individual pin
> +controller device.



-- 
~Randy

Re: [PATCH] dt: pinctrl: Document device tree binding

[Shawn Guo]

On Fri, Mar 09, 2012 at 11:14:33AM -0700, Stephen Warren wrote:
> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

Re: [PATCH] dt: pinctrl: Document device tree binding

[Dong Aisheng]

On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>

It's exactly what we discussed in Linaro Connect.
Overally it looks good to me.
Only one small question below:

> ---
>  .../bindings/pinctrl/pinctrl-bindings.txt          |  118 ++++++++++++++++++++
>  1 files changed, 118 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> new file mode 100644
> index 0000000..cce9f01
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> @@ -0,0 +1,118 @@
> +== Introduction ==
> +
> +Hardware modules that control pin multiplexing or configuration parameters
> +such as pull-up/down, tri-state, drive-strength etc are designated as pin
> +controllers. Each pin controller must be represented as a node in device tree,
> +just like any other hardware module.
> +
> +Hardware modules whose signals are affected by pin configuration are
> +designated client devices. Again, each client device must be represented as a
> +node in device tree, just like any other hardware module.
> +
> +For a client device to operate correctly, certain pin controllers must
> +set up certain specific pin configurations. Some client devices need a
> +single static pin configuration, e.g. set up during initialization. Others
> +need to reconfigure pins at run-time, for example to tri-state pins when the
> +device is inactive. Hence, each client device can define a set of named
> +states. The number and names of those states is defined by the client device's
> +own binding.
> +
> +The common pinctrl bindings defined in this file provide an infra-structure
> +for client device device tree nodes to map those state names to the pin
> +configuration used by those states.
> +
> +Note that pin controllers themselves may also be client devices of themselves.
> +For example, a pin controller may set up its own "active" state when the
> +driver loads. This would allow representing a board's static pin configuration
> +in a single place, rather than splitting it across multiple client device
> +nodes. The decision to do this or not somewhat rests with the author of
> +individual board device tree files, and any requirements imposed by the
> +bindings for the individual client devices in use by that board, i.e. whether
> +they require certain specific named states for dynamic pin configuration.
> +
> +== Pinctrl client devices ==
> +
> +For each client device individually, every pin state is assigned an integer
> +ID. These numbers start at 0, and are contiguous. For each state ID, a unique
> +property exists to define the pin configuration. Each state may also be
> +assigned a name. When names are used, another property exists to map from
> +those names to the integer IDs.
> +
> +Each client device's own binding determines the set of states the must be
> +defined in its device tree node, and whether to define the set of state
> +IDs that must be provided, or whether to define the set of state names that
> +must be provided.
> +
> +Required properties:
> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> +		node. These referenced pin configuration nodes must be child
> +		nodes of the pin controller that they configure. Multiple
> +		entries may exist in this list so that multiple pin
> +		controllers may be configured, or so that a state may be built
> +		from multiple nodes for a single pin controller, each
> +		contributing part of the overall configuration. See the next
> +		section of this document for details of the format of these
> +		pin configuration nodes.
> +
> +		In some cases, it may be useful to define a state, but for it
> +		to be empty. This may be required when a common IP block is
> +		used in an SoC either without a pin controller, or where the
> +		pin controller does not affect the HW module in question. If
> +		the binding for that IP block requires certain pin states to
> +		exist, they must still be defined, but may be left empty.
> +
It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
before to address the issues that the shared IP block may need or not need
pinctrl configuration on different platforms(correct me if wrong).

Then, there may be cases like below which may look a bit confusing
to people.
device {
	pinctrl-names = "active", "idle";
	pinctrl-0;
	pinctrl-1;
};

I'm wondering if we can let each individual driver to handle this special case?
Like checking device id then make decision whether call pinctrl_* APIs.
Then we can just do not define those properties for devices who
do not need pin configurations.

Regards
Dong Aisheng

> +Optional properties:
> +pinctrl-1:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +...
> +pinctrl-n:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +pinctrl-names:	The list of names to assign states. List entry 0 defines the
> +		name for integer state ID 0, list entry 1 for state ID 1, and
> +		so on.
> +
> +For example:
> +
> +	/* For a client device requiring named states */
> +	device {
> +		pinctrl-names = "active", "idle";
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +	/* For the same device if using state IDs */
> +	device {
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +== Pin controller devices ==
> +
> +Pin controller devices should contain the pin configuration nodes that client
> +devices reference.
> +
> +For example:
> +
> +	pincontroller {
> +		... /* Standard DT properties for the device itself elided */
> +
> +		state_0_node_a {
> +			...
> +		};
> +		state_1_node_a {
> +			...
> +		};
> +		state_1_node_b {
> +			...
> +		};
> +	}
> +
> +The contents of each of those pin configuration child nodes is defined
> +entirely by the binding for the individual pin controller device. There
> +exists no common standard for this content.
> +
> +The pin configuration nodes need not be direct children of the pin controller
> +device; they may be grand-children for example. Whether this is legal, and
> +whether there is any interaction between the child and intermediate parent
> +nodes, is again defined entirely by the binding for the individual pin
> +controller device.
> -- 
> 1.7.0.4
> 
> 

Re: [PATCH] dt: pinctrl: Document device tree binding

[Dong Aisheng]

On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>

It's exactly what we discussed in Linaro Connect.
Overally it looks good to me.
Only one small question below:

> ---
>  .../bindings/pinctrl/pinctrl-bindings.txt          |  118 ++++++++++++++++++++
>  1 files changed, 118 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> new file mode 100644
> index 0000000..cce9f01
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> @@ -0,0 +1,118 @@
> +== Introduction ==
> +
> +Hardware modules that control pin multiplexing or configuration parameters
> +such as pull-up/down, tri-state, drive-strength etc are designated as pin
> +controllers. Each pin controller must be represented as a node in device tree,
> +just like any other hardware module.
> +
> +Hardware modules whose signals are affected by pin configuration are
> +designated client devices. Again, each client device must be represented as a
> +node in device tree, just like any other hardware module.
> +
> +For a client device to operate correctly, certain pin controllers must
> +set up certain specific pin configurations. Some client devices need a
> +single static pin configuration, e.g. set up during initialization. Others
> +need to reconfigure pins at run-time, for example to tri-state pins when the
> +device is inactive. Hence, each client device can define a set of named
> +states. The number and names of those states is defined by the client device's
> +own binding.
> +
> +The common pinctrl bindings defined in this file provide an infra-structure
> +for client device device tree nodes to map those state names to the pin
> +configuration used by those states.
> +
> +Note that pin controllers themselves may also be client devices of themselves.
> +For example, a pin controller may set up its own "active" state when the
> +driver loads. This would allow representing a board's static pin configuration
> +in a single place, rather than splitting it across multiple client device
> +nodes. The decision to do this or not somewhat rests with the author of
> +individual board device tree files, and any requirements imposed by the
> +bindings for the individual client devices in use by that board, i.e. whether
> +they require certain specific named states for dynamic pin configuration.
> +
> +== Pinctrl client devices ==
> +
> +For each client device individually, every pin state is assigned an integer
> +ID. These numbers start at 0, and are contiguous. For each state ID, a unique
> +property exists to define the pin configuration. Each state may also be
> +assigned a name. When names are used, another property exists to map from
> +those names to the integer IDs.
> +
> +Each client device's own binding determines the set of states the must be
> +defined in its device tree node, and whether to define the set of state
> +IDs that must be provided, or whether to define the set of state names that
> +must be provided.
> +
> +Required properties:
> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> +		node. These referenced pin configuration nodes must be child
> +		nodes of the pin controller that they configure. Multiple
> +		entries may exist in this list so that multiple pin
> +		controllers may be configured, or so that a state may be built
> +		from multiple nodes for a single pin controller, each
> +		contributing part of the overall configuration. See the next
> +		section of this document for details of the format of these
> +		pin configuration nodes.
> +
> +		In some cases, it may be useful to define a state, but for it
> +		to be empty. This may be required when a common IP block is
> +		used in an SoC either without a pin controller, or where the
> +		pin controller does not affect the HW module in question. If
> +		the binding for that IP block requires certain pin states to
> +		exist, they must still be defined, but may be left empty.
> +
It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
before to address the issues that the shared IP block may need or not need
pinctrl configuration on different platforms(correct me if wrong).

Then, there may be cases like below which may look a bit confusing
to people.
device {
	pinctrl-names = "active", "idle";
	pinctrl-0;
	pinctrl-1;
};

I'm wondering if we can let each individual driver to handle this special case?
Like checking device id then make decision whether call pinctrl_* APIs.
Then we can just do not define those properties for devices who
do not need pin configurations.

Regards
Dong Aisheng

> +Optional properties:
> +pinctrl-1:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +...
> +pinctrl-n:	List of phandles, each pointing at a pin configuration
> +		node within a pin controller.
> +pinctrl-names:	The list of names to assign states. List entry 0 defines the
> +		name for integer state ID 0, list entry 1 for state ID 1, and
> +		so on.
> +
> +For example:
> +
> +	/* For a client device requiring named states */
> +	device {
> +		pinctrl-names = "active", "idle";
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +	/* For the same device if using state IDs */
> +	device {
> +		pinctrl-0 = <&state_0_node_a>;
> +		pinctrl-1 = <&state_1_node_a &state_1_node_b>;
> +	};
> +
> +== Pin controller devices ==
> +
> +Pin controller devices should contain the pin configuration nodes that client
> +devices reference.
> +
> +For example:
> +
> +	pincontroller {
> +		... /* Standard DT properties for the device itself elided */
> +
> +		state_0_node_a {
> +			...
> +		};
> +		state_1_node_a {
> +			...
> +		};
> +		state_1_node_b {
> +			...
> +		};
> +	}
> +
> +The contents of each of those pin configuration child nodes is defined
> +entirely by the binding for the individual pin controller device. There
> +exists no common standard for this content.
> +
> +The pin configuration nodes need not be direct children of the pin controller
> +device; they may be grand-children for example. Whether this is legal, and
> +whether there is any interaction between the child and intermediate parent
> +nodes, is again defined entirely by the binding for the individual pin
> +controller device.
> -- 
> 1.7.0.4
> 
> 

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/12/2012 08:34 AM, Dong Aisheng wrote:
> On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
>> The core pin controller bindings define:
>> * The fact that pin controllers expose pin configurations as nodes in
>>   device tree.
>> * That the bindings for those pin configuration nodes is defined by the
>>   individual pin controller drivers.
>> * A standardized set of properties for client devices to define numbered
>>   or named pin configuration states, each referring to some number of the
>>   afore-mentioned pin configuration nodes.
>> * That the bindings for the client devices determines the set of numbered
>>   or named states that must exist.

>> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

>> +Required properties:
>> +pinctrl-0:	List of phandles, each pointing at a pin configuration
>> +		node. These referenced pin configuration nodes must be child
>> +		nodes of the pin controller that they configure. Multiple
>> +		entries may exist in this list so that multiple pin
>> +		controllers may be configured, or so that a state may be built
>> +		from multiple nodes for a single pin controller, each
>> +		contributing part of the overall configuration. See the next
>> +		section of this document for details of the format of these
>> +		pin configuration nodes.
>> +
>> +		In some cases, it may be useful to define a state, but for it
>> +		to be empty. This may be required when a common IP block is
>> +		used in an SoC either without a pin controller, or where the
>> +		pin controller does not affect the HW module in question. If
>> +		the binding for that IP block requires certain pin states to
>> +		exist, they must still be defined, but may be left empty.
>> +
>
> It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
> before to address the issues that the shared IP block may need or not need
> pinctrl configuration on different platforms(correct me if wrong).

Yes, it's to generate the dummy states.

> Then, there may be cases like below which may look a bit confusing
> to people.
> device {
> 	pinctrl-names = "active", "idle";
> 	pinctrl-0;
> 	pinctrl-1;
> };

I'd personally expect the syntax to look like:

device {
	pinctrl-names = "active", "idle";
	pinctrl-0 = <>;
	pinctrl-1 = <>;
};

which has an explicitly empty value. Admittedly, these would both
compile down to the exact same thing in the DTB, but I think the
interpretation of the above is pretty readable.

> I'm wondering if we can let each individual driver to handle this special case?
> Like checking device id then make decision whether call pinctrl_* APIs.
> Then we can just do not define those properties for devices who
> do not need pin configurations.

The individual client drivers certainly could work that way.

However, the disadvantage is that the client driver then needs explicit
code to deal with this case, and this needs to be triggered by using a
different compatible flag (or perhaps some other explicit property).
You'd have to write this code over and over for each individual driver.

That also means that if you were the first user of an IP block in a
system which didn't need pin muxing for it, you'd have to modify the
kernel to support pinctrl being optional before you could use that device.

If the pinctrl subsystem itself hides this from the client driver, then
you'd never need to add any code to any driver to support this case, and
all you'd need to do is write a few lines of device tree to use the
driver; no code changes.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/12/2012 08:34 AM, Dong Aisheng wrote:
> On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
>> The core pin controller bindings define:
>> * The fact that pin controllers expose pin configurations as nodes in
>>   device tree.
>> * That the bindings for those pin configuration nodes is defined by the
>>   individual pin controller drivers.
>> * A standardized set of properties for client devices to define numbered
>>   or named pin configuration states, each referring to some number of the
>>   afore-mentioned pin configuration nodes.
>> * That the bindings for the client devices determines the set of numbered
>>   or named states that must exist.

>> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

>> +Required properties:
>> +pinctrl-0:	List of phandles, each pointing at a pin configuration
>> +		node. These referenced pin configuration nodes must be child
>> +		nodes of the pin controller that they configure. Multiple
>> +		entries may exist in this list so that multiple pin
>> +		controllers may be configured, or so that a state may be built
>> +		from multiple nodes for a single pin controller, each
>> +		contributing part of the overall configuration. See the next
>> +		section of this document for details of the format of these
>> +		pin configuration nodes.
>> +
>> +		In some cases, it may be useful to define a state, but for it
>> +		to be empty. This may be required when a common IP block is
>> +		used in an SoC either without a pin controller, or where the
>> +		pin controller does not affect the HW module in question. If
>> +		the binding for that IP block requires certain pin states to
>> +		exist, they must still be defined, but may be left empty.
>> +
>
> It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
> before to address the issues that the shared IP block may need or not need
> pinctrl configuration on different platforms(correct me if wrong).

Yes, it's to generate the dummy states.

> Then, there may be cases like below which may look a bit confusing
> to people.
> device {
> 	pinctrl-names = "active", "idle";
> 	pinctrl-0;
> 	pinctrl-1;
> };

I'd personally expect the syntax to look like:

device {
	pinctrl-names = "active", "idle";
	pinctrl-0 = <>;
	pinctrl-1 = <>;
};

which has an explicitly empty value. Admittedly, these would both
compile down to the exact same thing in the DTB, but I think the
interpretation of the above is pretty readable.

> I'm wondering if we can let each individual driver to handle this special case?
> Like checking device id then make decision whether call pinctrl_* APIs.
> Then we can just do not define those properties for devices who
> do not need pin configurations.

The individual client drivers certainly could work that way.

However, the disadvantage is that the client driver then needs explicit
code to deal with this case, and this needs to be triggered by using a
different compatible flag (or perhaps some other explicit property).
You'd have to write this code over and over for each individual driver.

That also means that if you were the first user of an IP block in a
system which didn't need pin muxing for it, you'd have to modify the
kernel to support pinctrl being optional before you could use that device.

If the pinctrl subsystem itself hides this from the client driver, then
you'd never need to add any code to any driver to support this case, and
all you'd need to do is write a few lines of device tree to use the
driver; no code changes.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Dong Aisheng]

On Tue, Mar 13, 2012 at 01:16:19AM +0800, Stephen Warren wrote:
> On 03/12/2012 08:34 AM, Dong Aisheng wrote:
> > On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
> >> The core pin controller bindings define:
> >> * The fact that pin controllers expose pin configurations as nodes in
> >>   device tree.
> >> * That the bindings for those pin configuration nodes is defined by the
> >>   individual pin controller drivers.
> >> * A standardized set of properties for client devices to define numbered
> >>   or named pin configuration states, each referring to some number of the
> >>   afore-mentioned pin configuration nodes.
> >> * That the bindings for the client devices determines the set of numbered
> >>   or named states that must exist.
> 
> >> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> 
> >> +Required properties:
> >> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> >> +		node. These referenced pin configuration nodes must be child
> >> +		nodes of the pin controller that they configure. Multiple
> >> +		entries may exist in this list so that multiple pin
> >> +		controllers may be configured, or so that a state may be built
> >> +		from multiple nodes for a single pin controller, each
> >> +		contributing part of the overall configuration. See the next
> >> +		section of this document for details of the format of these
> >> +		pin configuration nodes.
> >> +
> >> +		In some cases, it may be useful to define a state, but for it
> >> +		to be empty. This may be required when a common IP block is
> >> +		used in an SoC either without a pin controller, or where the
> >> +		pin controller does not affect the HW module in question. If
> >> +		the binding for that IP block requires certain pin states to
> >> +		exist, they must still be defined, but may be left empty.
> >> +
> >
> > It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
> > before to address the issues that the shared IP block may need or not need
> > pinctrl configuration on different platforms(correct me if wrong).
> 
> Yes, it's to generate the dummy states.
> 
> > Then, there may be cases like below which may look a bit confusing
> > to people.
> > device {
> > 	pinctrl-names = "active", "idle";
> > 	pinctrl-0;
> > 	pinctrl-1;
> > };
> 
> I'd personally expect the syntax to look like:
> 
> device {
> 	pinctrl-names = "active", "idle";
> 	pinctrl-0 = <>;
> 	pinctrl-1 = <>;
> };
> 
> which has an explicitly empty value. Admittedly, these would both
> compile down to the exact same thing in the DTB, but I think the
> interpretation of the above is pretty readable.
> 
> > I'm wondering if we can let each individual driver to handle this special case?
> > Like checking device id then make decision whether call pinctrl_* APIs.
> > Then we can just do not define those properties for devices who
> > do not need pin configurations.
> 
> The individual client drivers certainly could work that way.
> 
> However, the disadvantage is that the client driver then needs explicit
> code to deal with this case, and this needs to be triggered by using a
Since this is purely specific to IP block(e.g. the driver knows this ip used
in which platform does not need pin configuration), so i guess it's natural
that the driver can also handle it instead of device tree, just like
a lot of existing drivers in kernel do similar things for tricks
on different SoCs.

> different compatible flag (or perhaps some other explicit property).
> You'd have to write this code over and over for each individual driver.
> 
I still do not understand why need a more special compatible flag?
My understanding is that in device driver side, they can do:
if (is_soc_a || is_soc_b) {
	pinctrl_get(dev, "default");
	....
} else if (is_soc_c) {
	/* do nothing */
}
I can't see why we still need a special compatible flag to tell driver.
Just do not define pinctrl-* properties for that devices in device tree.
Did i understand wrong?

> That also means that if you were the first user of an IP block in a
> system which didn't need pin muxing for it, you'd have to modify the
> kernel to support pinctrl being optional before you could use that device.
> 
Why need modify the kernel?
Assuming above example.
I'm a bit confused.

> If the pinctrl subsystem itself hides this from the client driver, then
> you'd never need to add any code to any driver to support this case, and
> all you'd need to do is write a few lines of device tree to use the
> driver; no code changes.
> 
Yes, that is the benefit.
My only concern is that if this may make people confuse when see
such code in device tree since we,i guess, still do not have such examples
in device tree. And i'm afraid this is a bit not HW oriented.
device {
	pinctrl-names = "active", "idle";
	pinctrl-0 = <>;
	pinctrl-1 = <>;
};
So i'm asking if we do it in driver.
Maybe device tree people can give some comments.

Regards
Dong Aisheng

Re: [PATCH] dt: pinctrl: Document device tree binding

[Dong Aisheng]

On Tue, Mar 13, 2012 at 01:16:19AM +0800, Stephen Warren wrote:
> On 03/12/2012 08:34 AM, Dong Aisheng wrote:
> > On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
> >> The core pin controller bindings define:
> >> * The fact that pin controllers expose pin configurations as nodes in
> >>   device tree.
> >> * That the bindings for those pin configuration nodes is defined by the
> >>   individual pin controller drivers.
> >> * A standardized set of properties for client devices to define numbered
> >>   or named pin configuration states, each referring to some number of the
> >>   afore-mentioned pin configuration nodes.
> >> * That the bindings for the client devices determines the set of numbered
> >>   or named states that must exist.
> 
> >> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> 
> >> +Required properties:
> >> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> >> +		node. These referenced pin configuration nodes must be child
> >> +		nodes of the pin controller that they configure. Multiple
> >> +		entries may exist in this list so that multiple pin
> >> +		controllers may be configured, or so that a state may be built
> >> +		from multiple nodes for a single pin controller, each
> >> +		contributing part of the overall configuration. See the next
> >> +		section of this document for details of the format of these
> >> +		pin configuration nodes.
> >> +
> >> +		In some cases, it may be useful to define a state, but for it
> >> +		to be empty. This may be required when a common IP block is
> >> +		used in an SoC either without a pin controller, or where the
> >> +		pin controller does not affect the HW module in question. If
> >> +		the binding for that IP block requires certain pin states to
> >> +		exist, they must still be defined, but may be left empty.
> >> +
> >
> > It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
> > before to address the issues that the shared IP block may need or not need
> > pinctrl configuration on different platforms(correct me if wrong).
> 
> Yes, it's to generate the dummy states.
> 
> > Then, there may be cases like below which may look a bit confusing
> > to people.
> > device {
> > 	pinctrl-names = "active", "idle";
> > 	pinctrl-0;
> > 	pinctrl-1;
> > };
> 
> I'd personally expect the syntax to look like:
> 
> device {
> 	pinctrl-names = "active", "idle";
> 	pinctrl-0 = <>;
> 	pinctrl-1 = <>;
> };
> 
> which has an explicitly empty value. Admittedly, these would both
> compile down to the exact same thing in the DTB, but I think the
> interpretation of the above is pretty readable.
> 
> > I'm wondering if we can let each individual driver to handle this special case?
> > Like checking device id then make decision whether call pinctrl_* APIs.
> > Then we can just do not define those properties for devices who
> > do not need pin configurations.
> 
> The individual client drivers certainly could work that way.
> 
> However, the disadvantage is that the client driver then needs explicit
> code to deal with this case, and this needs to be triggered by using a
Since this is purely specific to IP block(e.g. the driver knows this ip used
in which platform does not need pin configuration), so i guess it's natural
that the driver can also handle it instead of device tree, just like
a lot of existing drivers in kernel do similar things for tricks
on different SoCs.

> different compatible flag (or perhaps some other explicit property).
> You'd have to write this code over and over for each individual driver.
> 
I still do not understand why need a more special compatible flag?
My understanding is that in device driver side, they can do:
if (is_soc_a || is_soc_b) {
	pinctrl_get(dev, "default");
	....
} else if (is_soc_c) {
	/* do nothing */
}
I can't see why we still need a special compatible flag to tell driver.
Just do not define pinctrl-* properties for that devices in device tree.
Did i understand wrong?

> That also means that if you were the first user of an IP block in a
> system which didn't need pin muxing for it, you'd have to modify the
> kernel to support pinctrl being optional before you could use that device.
> 
Why need modify the kernel?
Assuming above example.
I'm a bit confused.

> If the pinctrl subsystem itself hides this from the client driver, then
> you'd never need to add any code to any driver to support this case, and
> all you'd need to do is write a few lines of device tree to use the
> driver; no code changes.
> 
Yes, that is the benefit.
My only concern is that if this may make people confuse when see
such code in device tree since we,i guess, still do not have such examples
in device tree. And i'm afraid this is a bit not HW oriented.
device {
	pinctrl-names = "active", "idle";
	pinctrl-0 = <>;
	pinctrl-1 = <>;
};
So i'm asking if we do it in driver.
Maybe device tree people can give some comments.

Regards
Dong Aisheng

Re: [PATCH] dt: pinctrl: Document device tree binding

[Grant Likely]

On Fri,  9 Mar 2012 11:14:33 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>

Not reviewed this version deeply, but this looks good to me.  Feel free
to merge it if both you and Linus are happy with it.

g.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Grant Likely]

On Fri,  9 Mar 2012 11:14:33 -0700, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

Not reviewed this version deeply, but this looks good to me.  Feel free
to merge it if both you and Linus are happy with it.

g.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Linus Walleij]

On Fri, Mar 9, 2012 at 7:14 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> @@ -0,0 +1,118 @@
> +== Introduction ==
> +
> +Hardware modules that control pin multiplexing or configuration parameters
> +such as pull-up/down, tri-state, drive-strength etc are designated as pin
> +controllers. Each pin controller must be represented as a node in device tree,
> +just like any other hardware module.

Maybe put in a reference to Documentation/pinctrl.txt for in-depth
discussion? Also some stuff may be moved over there as generic
information. A lot of the text here does not seem to be about the
device tree ...

However maybe the use case is outside the Linux kernel too
and in that case I'm happy with it.

> +For a client device to operate correctly, certain pin controllers must
> +set up certain specific pin configurations. Some client devices need a
> +single static pin configuration, e.g. set up during initialization. Others
> +need to reconfigure pins at run-time, for example to tri-state pins when the
> +device is inactive. Hence, each client device can define a set of named
> +states. The number and names of those states is defined by the client device's
> +own binding.

Just so I understand: is "pin configuration" here strictly what we
handle in pinconf.c or does it include multiplexing (pinmux.c)?

I guess it's not multiplexing, just making sure.

Maybe state explicitly that multiplexing is not part of pin config,
else someone will invariably get confused.

> +Note that pin controllers themselves may also be client devices of themselves.

Insert something about this being known as config hogging.

The rest I barely understand so I leave it for the others to discuss...

Yours,
Linus Walleij

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/12/2012 09:20 PM, Dong Aisheng wrote:
> On Tue, Mar 13, 2012 at 01:16:19AM +0800, Stephen Warren wrote:
>> On 03/12/2012 08:34 AM, Dong Aisheng wrote:
>>> On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
>>>> The core pin controller bindings define:
>>>> * The fact that pin controllers expose pin configurations as nodes in
>>>>   device tree.
>>>> * That the bindings for those pin configuration nodes is defined by the
>>>>   individual pin controller drivers.
>>>> * A standardized set of properties for client devices to define numbered
>>>>   or named pin configuration states, each referring to some number of the
>>>>   afore-mentioned pin configuration nodes.
>>>> * That the bindings for the client devices determines the set of numbered
>>>>   or named states that must exist.
...
>>>> +Required properties:
>>>> +pinctrl-0:	List of phandles, each pointing at a pin configuration
>>>> +		node. These referenced pin configuration nodes must be child
>>>> +		nodes of the pin controller that they configure. Multiple
>>>> +		entries may exist in this list so that multiple pin
>>>> +		controllers may be configured, or so that a state may be built
>>>> +		from multiple nodes for a single pin controller, each
>>>> +		contributing part of the overall configuration. See the next
>>>> +		section of this document for details of the format of these
>>>> +		pin configuration nodes.
>>>> +
>>>> +		In some cases, it may be useful to define a state, but for it
>>>> +		to be empty. This may be required when a common IP block is
>>>> +		used in an SoC either without a pin controller, or where the
>>>> +		pin controller does not affect the HW module in question. If
>>>> +		the binding for that IP block requires certain pin states to
>>>> +		exist, they must still be defined, but may be left empty.
>>>> +
>>>
>>> It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
>>> before to address the issues that the shared IP block may need or not need
>>> pinctrl configuration on different platforms(correct me if wrong).
>>
>> Yes, it's to generate the dummy states.
>>
>>> Then, there may be cases like below which may look a bit confusing
>>> to people.
>>> device {
>>> 	pinctrl-names = "active", "idle";
>>> 	pinctrl-0;
>>> 	pinctrl-1;
>>> };
>>
>> I'd personally expect the syntax to look like:
>>
>> device {
>> 	pinctrl-names = "active", "idle";
>> 	pinctrl-0 = <>;
>> 	pinctrl-1 = <>;
>> };
>>
>> which has an explicitly empty value. Admittedly, these would both
>> compile down to the exact same thing in the DTB, but I think the
>> interpretation of the above is pretty readable.
>>
>>> I'm wondering if we can let each individual driver to handle this special case?
>>> Like checking device id then make decision whether call pinctrl_* APIs.
>>> Then we can just do not define those properties for devices who
>>> do not need pin configurations.
>>
>> The individual client drivers certainly could work that way.
>>
>> However, the disadvantage is that the client driver then needs explicit
>> code to deal with this case, and this needs to be triggered by using a
>
> Since this is purely specific to IP block(e.g. the driver knows this ip used
> in which platform does not need pin configuration), so i guess it's natural
> that the driver can also handle it instead of device tree, just like
> a lot of existing drivers in kernel do similar things for tricks
> on different SoCs.

Well, the entire point is that the driver for the IP block shouldn't
know anything about which SoC it's included in, or whether pinmux is
needed, or what pinmux is needed, beyond what's expressed in platform
data or device tree. The whole point of the pinctrl is to completely
remove this knowledge from the driver, and centralize it in the mapping
table.

>> different compatible flag (or perhaps some other explicit property).
>> You'd have to write this code over and over for each individual driver.
>
> I still do not understand why need a more special compatible flag?
> My understanding is that in device driver side, they can do:
> if (is_soc_a || is_soc_b) {
> 	pinctrl_get(dev, "default");
> 	....
> } else if (is_soc_c) {
> 	/* do nothing */
> }

Drivers aren't supposed to contain "is_soc_foo" or "is_machine_foo"
calls. Indeed, in the case of "is_soc_foo", I don't think such an API
even exists. Instead, platform data or device tree should represent the
information that drivers need.

> I can't see why we still need a special compatible flag to tell driver.
> Just do not define pinctrl-* properties for that devices in device tree.
> Did i understand wrong?

The compatible property would be the only way to implement anything like
is_soc_foo. However, it's a bad idea to overload the compatible property
in this way.

>> That also means that if you were the first user of an IP block in a
>> system which didn't need pin muxing for it, you'd have to modify the
>> kernel to support pinctrl being optional before you could use that device.
>
> Why need modify the kernel?
> Assuming above example.
> I'm a bit confused.

If the driver contains code like:

if (is_soc_a) {
    ...
} else if (is_soc_b) {
   ...
}
...

Then in order to support a new SoC, even if the driver doesn't need to
do anything different, you'd need to go and edit the code to add an "if
(is_soc_c)" condition into that list.

>> If the pinctrl subsystem itself hides this from the client driver, then
>> you'd never need to add any code to any driver to support this case, and
>> all you'd need to do is write a few lines of device tree to use the
>> driver; no code changes.
>>
> Yes, that is the benefit.
>
> My only concern is that if this may make people confuse when see
> such code in device tree since we,i guess, still do not have such examples
> in device tree. And i'm afraid this is a bit not HW oriented.
> device {
> 	pinctrl-names = "active", "idle";
> 	pinctrl-0 = <>;
> 	pinctrl-1 = <>;
> };
> So i'm asking if we do it in driver.
> Maybe device tree people can give some comments.

I personally don't think it's that confusing. A zero-length list is
after all still a list. That said, let me see if I can add such an
example to the binding document; the documentation does talk about this
case, but we can certainly add another example to highlight it.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/12/2012 09:20 PM, Dong Aisheng wrote:
> On Tue, Mar 13, 2012 at 01:16:19AM +0800, Stephen Warren wrote:
>> On 03/12/2012 08:34 AM, Dong Aisheng wrote:
>>> On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
>>>> The core pin controller bindings define:
>>>> * The fact that pin controllers expose pin configurations as nodes in
>>>>   device tree.
>>>> * That the bindings for those pin configuration nodes is defined by the
>>>>   individual pin controller drivers.
>>>> * A standardized set of properties for client devices to define numbered
>>>>   or named pin configuration states, each referring to some number of the
>>>>   afore-mentioned pin configuration nodes.
>>>> * That the bindings for the client devices determines the set of numbered
>>>>   or named states that must exist.
...
>>>> +Required properties:
>>>> +pinctrl-0:	List of phandles, each pointing at a pin configuration
>>>> +		node. These referenced pin configuration nodes must be child
>>>> +		nodes of the pin controller that they configure. Multiple
>>>> +		entries may exist in this list so that multiple pin
>>>> +		controllers may be configured, or so that a state may be built
>>>> +		from multiple nodes for a single pin controller, each
>>>> +		contributing part of the overall configuration. See the next
>>>> +		section of this document for details of the format of these
>>>> +		pin configuration nodes.
>>>> +
>>>> +		In some cases, it may be useful to define a state, but for it
>>>> +		to be empty. This may be required when a common IP block is
>>>> +		used in an SoC either without a pin controller, or where the
>>>> +		pin controller does not affect the HW module in question. If
>>>> +		the binding for that IP block requires certain pin states to
>>>> +		exist, they must still be defined, but may be left empty.
>>>> +
>>>
>>> It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
>>> before to address the issues that the shared IP block may need or not need
>>> pinctrl configuration on different platforms(correct me if wrong).
>>
>> Yes, it's to generate the dummy states.
>>
>>> Then, there may be cases like below which may look a bit confusing
>>> to people.
>>> device {
>>> 	pinctrl-names = "active", "idle";
>>> 	pinctrl-0;
>>> 	pinctrl-1;
>>> };
>>
>> I'd personally expect the syntax to look like:
>>
>> device {
>> 	pinctrl-names = "active", "idle";
>> 	pinctrl-0 = <>;
>> 	pinctrl-1 = <>;
>> };
>>
>> which has an explicitly empty value. Admittedly, these would both
>> compile down to the exact same thing in the DTB, but I think the
>> interpretation of the above is pretty readable.
>>
>>> I'm wondering if we can let each individual driver to handle this special case?
>>> Like checking device id then make decision whether call pinctrl_* APIs.
>>> Then we can just do not define those properties for devices who
>>> do not need pin configurations.
>>
>> The individual client drivers certainly could work that way.
>>
>> However, the disadvantage is that the client driver then needs explicit
>> code to deal with this case, and this needs to be triggered by using a
>
> Since this is purely specific to IP block(e.g. the driver knows this ip used
> in which platform does not need pin configuration), so i guess it's natural
> that the driver can also handle it instead of device tree, just like
> a lot of existing drivers in kernel do similar things for tricks
> on different SoCs.

Well, the entire point is that the driver for the IP block shouldn't
know anything about which SoC it's included in, or whether pinmux is
needed, or what pinmux is needed, beyond what's expressed in platform
data or device tree. The whole point of the pinctrl is to completely
remove this knowledge from the driver, and centralize it in the mapping
table.

>> different compatible flag (or perhaps some other explicit property).
>> You'd have to write this code over and over for each individual driver.
>
> I still do not understand why need a more special compatible flag?
> My understanding is that in device driver side, they can do:
> if (is_soc_a || is_soc_b) {
> 	pinctrl_get(dev, "default");
> 	....
> } else if (is_soc_c) {
> 	/* do nothing */
> }

Drivers aren't supposed to contain "is_soc_foo" or "is_machine_foo"
calls. Indeed, in the case of "is_soc_foo", I don't think such an API
even exists. Instead, platform data or device tree should represent the
information that drivers need.

> I can't see why we still need a special compatible flag to tell driver.
> Just do not define pinctrl-* properties for that devices in device tree.
> Did i understand wrong?

The compatible property would be the only way to implement anything like
is_soc_foo. However, it's a bad idea to overload the compatible property
in this way.

>> That also means that if you were the first user of an IP block in a
>> system which didn't need pin muxing for it, you'd have to modify the
>> kernel to support pinctrl being optional before you could use that device.
>
> Why need modify the kernel?
> Assuming above example.
> I'm a bit confused.

If the driver contains code like:

if (is_soc_a) {
    ...
} else if (is_soc_b) {
   ...
}
...

Then in order to support a new SoC, even if the driver doesn't need to
do anything different, you'd need to go and edit the code to add an "if
(is_soc_c)" condition into that list.

>> If the pinctrl subsystem itself hides this from the client driver, then
>> you'd never need to add any code to any driver to support this case, and
>> all you'd need to do is write a few lines of device tree to use the
>> driver; no code changes.
>>
> Yes, that is the benefit.
>
> My only concern is that if this may make people confuse when see
> such code in device tree since we,i guess, still do not have such examples
> in device tree. And i'm afraid this is a bit not HW oriented.
> device {
> 	pinctrl-names = "active", "idle";
> 	pinctrl-0 = <>;
> 	pinctrl-1 = <>;
> };
> So i'm asking if we do it in driver.
> Maybe device tree people can give some comments.

I personally don't think it's that confusing. A zero-length list is
after all still a list. That said, let me see if I can add such an
example to the binding document; the documentation does talk about this
case, but we can certainly add another example to highlight it.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/13/2012 03:14 AM, Linus Walleij wrote:
> On Fri, Mar 9, 2012 at 7:14 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>> @@ -0,0 +1,118 @@
>> +== Introduction ==
>> +
>> +Hardware modules that control pin multiplexing or configuration parameters
>> +such as pull-up/down, tri-state, drive-strength etc are designated as pin
>> +controllers. Each pin controller must be represented as a node in device tree,
>> +just like any other hardware module.
> 
> Maybe put in a reference to Documentation/pinctrl.txt for in-depth
> discussion? Also some stuff may be moved over there as generic
> information. A lot of the text here does not seem to be about the
> device tree ...
> 
> However maybe the use case is outside the Linux kernel too
> and in that case I'm happy with it.

Yes, the idea is that the bindings are OS-independent as much as
possible. That's why it's a little redundant w.r.t. the existing Linux
pinctrl documentation.

>> +For a client device to operate correctly, certain pin controllers must
>> +set up certain specific pin configurations. Some client devices need a
>> +single static pin configuration, e.g. set up during initialization. Others
>> +need to reconfigure pins at run-time, for example to tri-state pins when the
>> +device is inactive. Hence, each client device can define a set of named
>> +states. The number and names of those states is defined by the client device's
>> +own binding.
> 
> Just so I understand: is "pin configuration" here strictly what we
> handle in pinconf.c or does it include multiplexing (pinmux.c)?
> 
> I guess it's not multiplexing, just making sure.
>
> Maybe state explicitly that multiplexing is not part of pin config,
> else someone will invariably get confused.

No, it's intended to cover any aspect at all of pin control hardware,
including muxing. I'm not sure why you would expect pin muxing /not/ to
be represented by these bindings?

>> +Note that pin controllers themselves may also be client devices of themselves.
> 
> Insert something about this being known as config hogging.

I think that's Linux-specific terminology, hence not appropriate for a
generic document. (And as an aside, I don't really like the name
"hogging", or even treating it as some kind of special-case).

> The rest I barely understand so I leave it for the others to discuss...

Hmm. That's unfortunate. It'd be very useful if you could fully digest
this aspect of pinctrl.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Tony Lindgren]

* Stephen Warren <swarren@wwwdotorg.org> [120309 10:17]:
> The core pin controller bindings define:
> * The fact that pin controllers expose pin configurations as nodes in
>   device tree.
> * That the bindings for those pin configuration nodes is defined by the
>   individual pin controller drivers.
> * A standardized set of properties for client devices to define numbered
>   or named pin configuration states, each referring to some number of the
>   afore-mentioned pin configuration nodes.
> * That the bindings for the client devices determines the set of numbered
>   or named states that must exist.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>

Great, looks good to me:

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH] dt: pinctrl: Document device tree binding

[Dong Aisheng]

On Wed, Mar 14, 2012 at 03:27:26AM +0800, Stephen Warren wrote:
> On 03/12/2012 09:20 PM, Dong Aisheng wrote:
> > On Tue, Mar 13, 2012 at 01:16:19AM +0800, Stephen Warren wrote:
> >> On 03/12/2012 08:34 AM, Dong Aisheng wrote:
> >>> On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
> >>>> The core pin controller bindings define:
> >>>> * The fact that pin controllers expose pin configurations as nodes in
> >>>>   device tree.
> >>>> * That the bindings for those pin configuration nodes is defined by the
> >>>>   individual pin controller drivers.
> >>>> * A standardized set of properties for client devices to define numbered
> >>>>   or named pin configuration states, each referring to some number of the
> >>>>   afore-mentioned pin configuration nodes.
> >>>> * That the bindings for the client devices determines the set of numbered
> >>>>   or named states that must exist.
> ...
> >>>> +Required properties:
> >>>> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> >>>> +		node. These referenced pin configuration nodes must be child
> >>>> +		nodes of the pin controller that they configure. Multiple
> >>>> +		entries may exist in this list so that multiple pin
> >>>> +		controllers may be configured, or so that a state may be built
> >>>> +		from multiple nodes for a single pin controller, each
> >>>> +		contributing part of the overall configuration. See the next
> >>>> +		section of this document for details of the format of these
> >>>> +		pin configuration nodes.
> >>>> +
> >>>> +		In some cases, it may be useful to define a state, but for it
> >>>> +		to be empty. This may be required when a common IP block is
> >>>> +		used in an SoC either without a pin controller, or where the
> >>>> +		pin controller does not affect the HW module in question. If
> >>>> +		the binding for that IP block requires certain pin states to
> >>>> +		exist, they must still be defined, but may be left empty.
> >>>> +
> >>>
> >>> It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
> >>> before to address the issues that the shared IP block may need or not need
> >>> pinctrl configuration on different platforms(correct me if wrong).
> >>
> >> Yes, it's to generate the dummy states.
> >>
> >>> Then, there may be cases like below which may look a bit confusing
> >>> to people.
> >>> device {
> >>> 	pinctrl-names = "active", "idle";
> >>> 	pinctrl-0;
> >>> 	pinctrl-1;
> >>> };
> >>
> >> I'd personally expect the syntax to look like:
> >>
> >> device {
> >> 	pinctrl-names = "active", "idle";
> >> 	pinctrl-0 = <>;
> >> 	pinctrl-1 = <>;
> >> };
> >>
> >> which has an explicitly empty value. Admittedly, these would both
> >> compile down to the exact same thing in the DTB, but I think the
> >> interpretation of the above is pretty readable.
> >>
> >>> I'm wondering if we can let each individual driver to handle this special case?
> >>> Like checking device id then make decision whether call pinctrl_* APIs.
> >>> Then we can just do not define those properties for devices who
> >>> do not need pin configurations.
> >>
> >> The individual client drivers certainly could work that way.
> >>
> >> However, the disadvantage is that the client driver then needs explicit
> >> code to deal with this case, and this needs to be triggered by using a
> >
> > Since this is purely specific to IP block(e.g. the driver knows this ip used
> > in which platform does not need pin configuration), so i guess it's natural
> > that the driver can also handle it instead of device tree, just like
> > a lot of existing drivers in kernel do similar things for tricks
> > on different SoCs.
> 
> Well, the entire point is that the driver for the IP block shouldn't
> know anything about which SoC it's included in, or whether pinmux is
> needed, or what pinmux is needed, beyond what's expressed in platform
> data or device tree. The whole point of the pinctrl is to completely
> remove this knowledge from the driver, and centralize it in the mapping
> table.
> 
Good point to me.
The driver does not need to know which SoC it's included in,
but it has to support that SoC first.

> >> different compatible flag (or perhaps some other explicit property).
> >> You'd have to write this code over and over for each individual driver.
> >
> > I still do not understand why need a more special compatible flag?
> > My understanding is that in device driver side, they can do:
> > if (is_soc_a || is_soc_b) {
> > 	pinctrl_get(dev, "default");
> > 	....
> > } else if (is_soc_c) {
> > 	/* do nothing */
> > }
> 
> Drivers aren't supposed to contain "is_soc_foo" or "is_machine_foo"
> calls. Indeed, in the case of "is_soc_foo", I don't think such an API
> even exists. Instead, platform data or device tree should represent the
> information that drivers need.
> 
Hmm, whatever platform data or device tree or device id, we can use a way
to tell driver which SoC it is running on.
The driver private is_soc_a macro or function can be implemented based
that information.

> > I can't see why we still need a special compatible flag to tell driver.
> > Just do not define pinctrl-* properties for that devices in device tree.
> > Did i understand wrong?
> 
> The compatible property would be the only way to implement anything like
> is_soc_foo. However, it's a bad idea to overload the compatible property
> in this way.
> 
I guess i might not describe my idea clearly.
My idea is that the compatible string does that work.
For example:
static const struct of_device_id mxs_mmc_dt_ids[] = {
        { .compatible = "fsl,imx23-mmc", .data = NULL, },
        { .compatible = "fsl,imx28-mmc", .data = NULL, },
        { /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
Replace NULL to some special data like SOC_MX23 or SOC_MX28 can let driver
driver know which SoC it's running on.
Then driver can use private macro like is_soc_foo.

> >> That also means that if you were the first user of an IP block in a
> >> system which didn't need pin muxing for it, you'd have to modify the
> >> kernel to support pinctrl being optional before you could use that device.
> >
> > Why need modify the kernel?
> > Assuming above example.
> > I'm a bit confused.
> 
> If the driver contains code like:
> 
> if (is_soc_a) {
>     ...
> } else if (is_soc_b) {
>    ...
> }
> ...
> 
> Then in order to support a new SoC, even if the driver doesn't need to
> do anything different, you'd need to go and edit the code to add an "if
> (is_soc_c)" condition into that list.
> 
No.
No changes needed if the driver does not need to do anything different.
Compatible string does that work
For example, in a new soc which is fully compatible with current driver.
It can just add device like:
mmc@80010000 {
	compatible = "fsl, xxx-mmc", "fsl,imx28-mmc";
	reg = <..>;
	...
}

> >> If the pinctrl subsystem itself hides this from the client driver, then
> >> you'd never need to add any code to any driver to support this case, and
> >> all you'd need to do is write a few lines of device tree to use the
> >> driver; no code changes.
> >>
> > Yes, that is the benefit.
> >
> > My only concern is that if this may make people confuse when see
> > such code in device tree since we,i guess, still do not have such examples
> > in device tree. And i'm afraid this is a bit not HW oriented.
> > device {
> > 	pinctrl-names = "active", "idle";
> > 	pinctrl-0 = <>;
> > 	pinctrl-1 = <>;
> > };
> > So i'm asking if we do it in driver.
> > Maybe device tree people can give some comments.
> 
> I personally don't think it's that confusing. A zero-length list is
> after all still a list. That said, let me see if I can add such an
> example to the binding document; the documentation does talk about this
> case, but we can certainly add another example to highlight it.
> 
If dt does allow this, i'm also ok with it.

Regards
Dong Aisheng

Re: [PATCH] dt: pinctrl: Document device tree binding

[Dong Aisheng]

On Wed, Mar 14, 2012 at 03:27:26AM +0800, Stephen Warren wrote:
> On 03/12/2012 09:20 PM, Dong Aisheng wrote:
> > On Tue, Mar 13, 2012 at 01:16:19AM +0800, Stephen Warren wrote:
> >> On 03/12/2012 08:34 AM, Dong Aisheng wrote:
> >>> On Sat, Mar 10, 2012 at 02:14:33AM +0800, Stephen Warren wrote:
> >>>> The core pin controller bindings define:
> >>>> * The fact that pin controllers expose pin configurations as nodes in
> >>>>   device tree.
> >>>> * That the bindings for those pin configuration nodes is defined by the
> >>>>   individual pin controller drivers.
> >>>> * A standardized set of properties for client devices to define numbered
> >>>>   or named pin configuration states, each referring to some number of the
> >>>>   afore-mentioned pin configuration nodes.
> >>>> * That the bindings for the client devices determines the set of numbered
> >>>>   or named states that must exist.
> ...
> >>>> +Required properties:
> >>>> +pinctrl-0:	List of phandles, each pointing at a pin configuration
> >>>> +		node. These referenced pin configuration nodes must be child
> >>>> +		nodes of the pin controller that they configure. Multiple
> >>>> +		entries may exist in this list so that multiple pin
> >>>> +		controllers may be configured, or so that a state may be built
> >>>> +		from multiple nodes for a single pin controller, each
> >>>> +		contributing part of the overall configuration. See the next
> >>>> +		section of this document for details of the format of these
> >>>> +		pin configuration nodes.
> >>>> +
> >>>> +		In some cases, it may be useful to define a state, but for it
> >>>> +		to be empty. This may be required when a common IP block is
> >>>> +		used in an SoC either without a pin controller, or where the
> >>>> +		pin controller does not affect the HW module in question. If
> >>>> +		the binding for that IP block requires certain pin states to
> >>>> +		exist, they must still be defined, but may be left empty.
> >>>> +
> >>>
> >>> It looks this functions similar as the PIN_MAP_DUMMY_STATE you introduced
> >>> before to address the issues that the shared IP block may need or not need
> >>> pinctrl configuration on different platforms(correct me if wrong).
> >>
> >> Yes, it's to generate the dummy states.
> >>
> >>> Then, there may be cases like below which may look a bit confusing
> >>> to people.
> >>> device {
> >>> 	pinctrl-names = "active", "idle";
> >>> 	pinctrl-0;
> >>> 	pinctrl-1;
> >>> };
> >>
> >> I'd personally expect the syntax to look like:
> >>
> >> device {
> >> 	pinctrl-names = "active", "idle";
> >> 	pinctrl-0 = <>;
> >> 	pinctrl-1 = <>;
> >> };
> >>
> >> which has an explicitly empty value. Admittedly, these would both
> >> compile down to the exact same thing in the DTB, but I think the
> >> interpretation of the above is pretty readable.
> >>
> >>> I'm wondering if we can let each individual driver to handle this special case?
> >>> Like checking device id then make decision whether call pinctrl_* APIs.
> >>> Then we can just do not define those properties for devices who
> >>> do not need pin configurations.
> >>
> >> The individual client drivers certainly could work that way.
> >>
> >> However, the disadvantage is that the client driver then needs explicit
> >> code to deal with this case, and this needs to be triggered by using a
> >
> > Since this is purely specific to IP block(e.g. the driver knows this ip used
> > in which platform does not need pin configuration), so i guess it's natural
> > that the driver can also handle it instead of device tree, just like
> > a lot of existing drivers in kernel do similar things for tricks
> > on different SoCs.
> 
> Well, the entire point is that the driver for the IP block shouldn't
> know anything about which SoC it's included in, or whether pinmux is
> needed, or what pinmux is needed, beyond what's expressed in platform
> data or device tree. The whole point of the pinctrl is to completely
> remove this knowledge from the driver, and centralize it in the mapping
> table.
> 
Good point to me.
The driver does not need to know which SoC it's included in,
but it has to support that SoC first.

> >> different compatible flag (or perhaps some other explicit property).
> >> You'd have to write this code over and over for each individual driver.
> >
> > I still do not understand why need a more special compatible flag?
> > My understanding is that in device driver side, they can do:
> > if (is_soc_a || is_soc_b) {
> > 	pinctrl_get(dev, "default");
> > 	....
> > } else if (is_soc_c) {
> > 	/* do nothing */
> > }
> 
> Drivers aren't supposed to contain "is_soc_foo" or "is_machine_foo"
> calls. Indeed, in the case of "is_soc_foo", I don't think such an API
> even exists. Instead, platform data or device tree should represent the
> information that drivers need.
> 
Hmm, whatever platform data or device tree or device id, we can use a way
to tell driver which SoC it is running on.
The driver private is_soc_a macro or function can be implemented based
that information.

> > I can't see why we still need a special compatible flag to tell driver.
> > Just do not define pinctrl-* properties for that devices in device tree.
> > Did i understand wrong?
> 
> The compatible property would be the only way to implement anything like
> is_soc_foo. However, it's a bad idea to overload the compatible property
> in this way.
> 
I guess i might not describe my idea clearly.
My idea is that the compatible string does that work.
For example:
static const struct of_device_id mxs_mmc_dt_ids[] = {
        { .compatible = "fsl,imx23-mmc", .data = NULL, },
        { .compatible = "fsl,imx28-mmc", .data = NULL, },
        { /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
Replace NULL to some special data like SOC_MX23 or SOC_MX28 can let driver
driver know which SoC it's running on.
Then driver can use private macro like is_soc_foo.

> >> That also means that if you were the first user of an IP block in a
> >> system which didn't need pin muxing for it, you'd have to modify the
> >> kernel to support pinctrl being optional before you could use that device.
> >
> > Why need modify the kernel?
> > Assuming above example.
> > I'm a bit confused.
> 
> If the driver contains code like:
> 
> if (is_soc_a) {
>     ...
> } else if (is_soc_b) {
>    ...
> }
> ...
> 
> Then in order to support a new SoC, even if the driver doesn't need to
> do anything different, you'd need to go and edit the code to add an "if
> (is_soc_c)" condition into that list.
> 
No.
No changes needed if the driver does not need to do anything different.
Compatible string does that work
For example, in a new soc which is fully compatible with current driver.
It can just add device like:
mmc@80010000 {
	compatible = "fsl, xxx-mmc", "fsl,imx28-mmc";
	reg = <..>;
	...
}

> >> If the pinctrl subsystem itself hides this from the client driver, then
> >> you'd never need to add any code to any driver to support this case, and
> >> all you'd need to do is write a few lines of device tree to use the
> >> driver; no code changes.
> >>
> > Yes, that is the benefit.
> >
> > My only concern is that if this may make people confuse when see
> > such code in device tree since we,i guess, still do not have such examples
> > in device tree. And i'm afraid this is a bit not HW oriented.
> > device {
> > 	pinctrl-names = "active", "idle";
> > 	pinctrl-0 = <>;
> > 	pinctrl-1 = <>;
> > };
> > So i'm asking if we do it in driver.
> > Maybe device tree people can give some comments.
> 
> I personally don't think it's that confusing. A zero-length list is
> after all still a list. That said, let me see if I can add such an
> example to the binding document; the documentation does talk about this
> case, but we can certainly add another example to highlight it.
> 
If dt does allow this, i'm also ok with it.

Regards
Dong Aisheng

Re: [PATCH] dt: pinctrl: Document device tree binding

[Linus Walleij]

On Wed, Mar 14, 2012 at 10:26 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Stephen Warren <swarren@wwwdotorg.org> [120309 10:17]:
>> The core pin controller bindings define:
>> * The fact that pin controllers expose pin configurations as nodes in
>>   device tree.
>> * That the bindings for those pin configuration nodes is defined by the
>>   individual pin controller drivers.
>> * A standardized set of properties for client devices to define numbered
>>   or named pin configuration states, each referring to some number of the
>>   afore-mentioned pin configuration nodes.
>> * That the bindings for the client devices determines the set of numbered
>>   or named states that must exist.
>>
>> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
>
> Great, looks good to me:
>
> Acked-by: Tony Lindgren <tony@atomide.com>

Hm, both you guys agree, then I will be happy that we merge this.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Since it's just toching DT dirs I guess it should go through Grant,
but I'm happy to take it in pinctrl too.

Yours,
Linus Walleij

Re: [PATCH] dt: pinctrl: Document device tree binding

[Linus Walleij]

On Wed, Mar 14, 2012 at 10:26 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> * Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> [120309 10:17]:
>> The core pin controller bindings define:
>> * The fact that pin controllers expose pin configurations as nodes in
>>   device tree.
>> * That the bindings for those pin configuration nodes is defined by the
>>   individual pin controller drivers.
>> * A standardized set of properties for client devices to define numbered
>>   or named pin configuration states, each referring to some number of the
>>   afore-mentioned pin configuration nodes.
>> * That the bindings for the client devices determines the set of numbered
>>   or named states that must exist.
>>
>> Signed-off-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>
> Great, looks good to me:
>
> Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

Hm, both you guys agree, then I will be happy that we merge this.
Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Since it's just toching DT dirs I guess it should go through Grant,
but I'm happy to take it in pinctrl too.

Yours,
Linus Walleij

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/15/2012 10:51 AM, Linus Walleij wrote:
...
> Hm, both you guys agree, then I will be happy that we merge this.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Great. I'll repost a new version very soon based on the feedback (just a
couple of grammar issues and an added example).

> Since it's just toching DT dirs I guess it should go through Grant,
> but I'm happy to take it in pinctrl too.

I've seen many bindings go through the tree for the subsystem they
correspond to, as part of the series that implements them. I'm not
bothered either way though - I'll let you work this out with Grant once
I post the final version.

(I'm hoping to post the actual implementation today, maybe tomorrow)

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/14/2012 09:32 PM, Dong Aisheng wrote:
> On Wed, Mar 14, 2012 at 03:27:26AM +0800, Stephen Warren wrote:
>> On 03/12/2012 09:20 PM, Dong Aisheng wrote:
...
>>> I can't see why we still need a special compatible flag to tell driver.
>>> Just do not define pinctrl-* properties for that devices in device tree.
>>> Did i understand wrong?
>>
>> The compatible property would be the only way to implement anything like
>> is_soc_foo. However, it's a bad idea to overload the compatible property
>> in this way.
>
> I guess i might not describe my idea clearly.
> My idea is that the compatible string does that work.
> For example:
> static const struct of_device_id mxs_mmc_dt_ids[] = {
>         { .compatible = "fsl,imx23-mmc", .data = NULL, },
>         { .compatible = "fsl,imx28-mmc", .data = NULL, },
>         { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
>
> Replace NULL to some special data like SOC_MX23 or SOC_MX28 can let driver
> driver know which SoC it's running on.
> Then driver can use private macro like is_soc_foo.

OK, you could implement is_soc_foo based on the compatible flag.
However, it's best not to do that.

Instead of asking is_soc_foo(), you should really ask has_bug_xxx() or
has_feature_xxx(), and implement those based on either the compatible
flag, or perhaps other properties.

That way, when 10 of the 20 SoCs that contain the IP block all have the
same feature or need the same bugfix, the code that implements the
bugfix just calls a single has_bug_xxx() rather than having to sprinkle
many many is_soc_foo() calls through the code, which gets unmaintainable.

>>>> That also means that if you were the first user of an IP block in a
>>>> system which didn't need pin muxing for it, you'd have to modify the
>>>> kernel to support pinctrl being optional before you could use that device.
>>>
>>> Why need modify the kernel?
>>> Assuming above example.
>>> I'm a bit confused.
>>
>> If the driver contains code like:
>>
>> if (is_soc_a) {
>>     ...
>> } else if (is_soc_b) {
>>    ...
>> }
>> ...
>>
>> Then in order to support a new SoC, even if the driver doesn't need to
>> do anything different, you'd need to go and edit the code to add an "if
>> (is_soc_c)" condition into that list.
>
> No.
> No changes needed if the driver does not need to do anything different.
> Compatible string does that work

To make the driver support a new compatible property value, you have to
edit the driver to add it to the of_device_id table, and recompile it.

Re: [PATCH] dt: pinctrl: Document device tree binding

[Stephen Warren]

On 03/14/2012 09:32 PM, Dong Aisheng wrote:
> On Wed, Mar 14, 2012 at 03:27:26AM +0800, Stephen Warren wrote:
>> On 03/12/2012 09:20 PM, Dong Aisheng wrote:
...
>>> I can't see why we still need a special compatible flag to tell driver.
>>> Just do not define pinctrl-* properties for that devices in device tree.
>>> Did i understand wrong?
>>
>> The compatible property would be the only way to implement anything like
>> is_soc_foo. However, it's a bad idea to overload the compatible property
>> in this way.
>
> I guess i might not describe my idea clearly.
> My idea is that the compatible string does that work.
> For example:
> static const struct of_device_id mxs_mmc_dt_ids[] = {
>         { .compatible = "fsl,imx23-mmc", .data = NULL, },
>         { .compatible = "fsl,imx28-mmc", .data = NULL, },
>         { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
>
> Replace NULL to some special data like SOC_MX23 or SOC_MX28 can let driver
> driver know which SoC it's running on.
> Then driver can use private macro like is_soc_foo.

OK, you could implement is_soc_foo based on the compatible flag.
However, it's best not to do that.

Instead of asking is_soc_foo(), you should really ask has_bug_xxx() or
has_feature_xxx(), and implement those based on either the compatible
flag, or perhaps other properties.

That way, when 10 of the 20 SoCs that contain the IP block all have the
same feature or need the same bugfix, the code that implements the
bugfix just calls a single has_bug_xxx() rather than having to sprinkle
many many is_soc_foo() calls through the code, which gets unmaintainable.

>>>> That also means that if you were the first user of an IP block in a
>>>> system which didn't need pin muxing for it, you'd have to modify the
>>>> kernel to support pinctrl being optional before you could use that device.
>>>
>>> Why need modify the kernel?
>>> Assuming above example.
>>> I'm a bit confused.
>>
>> If the driver contains code like:
>>
>> if (is_soc_a) {
>>     ...
>> } else if (is_soc_b) {
>>    ...
>> }
>> ...
>>
>> Then in order to support a new SoC, even if the driver doesn't need to
>> do anything different, you'd need to go and edit the code to add an "if
>> (is_soc_c)" condition into that list.
>
> No.
> No changes needed if the driver does not need to do anything different.
> Compatible string does that work

To make the driver support a new compatible property value, you have to
edit the driver to add it to the of_device_id table, and recompile it.