diff mbox

[07/12] dt-bindings: i2c: Add support for 'i2c-bus' subnode

Message ID 1466697545-11868-8-git-send-email-jonathanh@nvidia.com
State Superseded
Headers show

Commit Message

Jon Hunter June 23, 2016, 3:59 p.m. UTC
The I2C driver core for boards using device-tree assumes any subnode of
an I2C adapter in the device-tree blob as being a I2C slave device.
Although this makes complete sense, some I2C adapters may have subnodes
which are not I2C slaves but subnodes presenting other features. For
example some Tegra devices have an I2C interface which may share its
pins with other devices and to share these pins subnodes for
representing these pins so they have be shared via the pinctrl framework
are needed.

To allow I2C adapters to have non-I2C specific subnodes in device-tree
that are not parsed by the I2C driver core by adding support for a
'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
subnode is present then all I2C slaves must be placed under this subnode.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Rob Herring (Arm) June 24, 2016, 7:23 p.m. UTC | #1
On Thu, Jun 23, 2016 at 04:59:00PM +0100, Jon Hunter wrote:
> The I2C driver core for boards using device-tree assumes any subnode of
> an I2C adapter in the device-tree blob as being a I2C slave device.
> Although this makes complete sense, some I2C adapters may have subnodes
> which are not I2C slaves but subnodes presenting other features. For
> example some Tegra devices have an I2C interface which may share its
> pins with other devices and to share these pins subnodes for
> representing these pins so they have be shared via the pinctrl framework
> are needed.
> 
> To allow I2C adapters to have non-I2C specific subnodes in device-tree
> that are not parsed by the I2C driver core by adding support for a
> 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
> subnode is present then all I2C slaves must be placed under this subnode.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin June 27, 2016, 12:04 p.m. UTC | #2
On 2016-06-23 17:59, Jon Hunter wrote:
> The I2C driver core for boards using device-tree assumes any subnode of
> an I2C adapter in the device-tree blob as being a I2C slave device.
> Although this makes complete sense, some I2C adapters may have subnodes
> which are not I2C slaves but subnodes presenting other features. For
> example some Tegra devices have an I2C interface which may share its
> pins with other devices and to share these pins subnodes for
> representing these pins so they have be shared via the pinctrl framework
> are needed.
> 
> To allow I2C adapters to have non-I2C specific subnodes in device-tree
> that are not parsed by the I2C driver core by adding support for a
> 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
> subnode is present then all I2C slaves must be placed under this subnode.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index f31b2ad1552b..71bea55d4c1b 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
>  - clock-frequency
>  	frequency of bus clock in Hz.
>  
> +- i2c-bus
> +	For I2C adapters that have child nodes that are a mixture of both I2C
> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
> +	subnode can be used for populating I2C devices. If the 'i2c-bus'
> +	subnode is present, only subnodes of this will be considered as I2C
> +	slaves. The properties, '#address-cells' and '#size-cells' must be
> +	defined under this subnode if present.

Hmmm, those #-properties are listed above, under "Required properties", which
is no longer 100% true. Maybe rephrase to

	slaves. The required properties '#address-cells' and '#size-cells'
	must be	defined under this subnode instead, if this subnode is present.

to make the rules (even) clearer?

Cheers,
Peter

> +
>  - i2c-scl-falling-time-ns
>  	Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C
>  	specification.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter June 28, 2016, 8:21 a.m. UTC | #3
On 27/06/16 13:04, Peter Rosin wrote:
> On 2016-06-23 17:59, Jon Hunter wrote:
>> The I2C driver core for boards using device-tree assumes any subnode of
>> an I2C adapter in the device-tree blob as being a I2C slave device.
>> Although this makes complete sense, some I2C adapters may have subnodes
>> which are not I2C slaves but subnodes presenting other features. For
>> example some Tegra devices have an I2C interface which may share its
>> pins with other devices and to share these pins subnodes for
>> representing these pins so they have be shared via the pinctrl framework
>> are needed.
>>
>> To allow I2C adapters to have non-I2C specific subnodes in device-tree
>> that are not parsed by the I2C driver core by adding support for a
>> 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
>> subnode is present then all I2C slaves must be placed under this subnode.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> ---
>>  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
>> index f31b2ad1552b..71bea55d4c1b 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
>> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
>> @@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
>>  - clock-frequency
>>  	frequency of bus clock in Hz.
>>  
>> +- i2c-bus
>> +	For I2C adapters that have child nodes that are a mixture of both I2C
>> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
>> +	subnode can be used for populating I2C devices. If the 'i2c-bus'
>> +	subnode is present, only subnodes of this will be considered as I2C
>> +	slaves. The properties, '#address-cells' and '#size-cells' must be
>> +	defined under this subnode if present.
> 
> Hmmm, those #-properties are listed above, under "Required properties", which
> is no longer 100% true. Maybe rephrase to
> 
> 	slaves. The required properties '#address-cells' and '#size-cells'
> 	must be	defined under this subnode instead, if this subnode is present.
> 
> to make the rules (even) clearer?

I see what you are saying but I wonder if the following is better ...

 slaves. The required properties '#address-cells' and '#size-cells'
 must be defined under this subnode if present and not the parent node.

Cheers
Jon
Peter Rosin June 28, 2016, 10:20 a.m. UTC | #4
On 2016-06-28 10:21, Jon Hunter wrote:
> 
> On 27/06/16 13:04, Peter Rosin wrote:
>> On 2016-06-23 17:59, Jon Hunter wrote:
>>> The I2C driver core for boards using device-tree assumes any subnode of
>>> an I2C adapter in the device-tree blob as being a I2C slave device.
>>> Although this makes complete sense, some I2C adapters may have subnodes
>>> which are not I2C slaves but subnodes presenting other features. For
>>> example some Tegra devices have an I2C interface which may share its
>>> pins with other devices and to share these pins subnodes for
>>> representing these pins so they have be shared via the pinctrl framework
>>> are needed.
>>>
>>> To allow I2C adapters to have non-I2C specific subnodes in device-tree
>>> that are not parsed by the I2C driver core by adding support for a
>>> 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
>>> subnode is present then all I2C slaves must be placed under this subnode.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
>>> index f31b2ad1552b..71bea55d4c1b 100644
>>> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
>>> @@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
>>>  - clock-frequency
>>>  	frequency of bus clock in Hz.
>>>  
>>> +- i2c-bus
>>> +	For I2C adapters that have child nodes that are a mixture of both I2C
>>> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
>>> +	subnode can be used for populating I2C devices. If the 'i2c-bus'
>>> +	subnode is present, only subnodes of this will be considered as I2C
>>> +	slaves. The properties, '#address-cells' and '#size-cells' must be
>>> +	defined under this subnode if present.
>>
>> Hmmm, those #-properties are listed above, under "Required properties", which
>> is no longer 100% true. Maybe rephrase to
>>
>> 	slaves. The required properties '#address-cells' and '#size-cells'
>> 	must be	defined under this subnode instead, if this subnode is present.
>>
>> to make the rules (even) clearer?
> 
> I see what you are saying but I wonder if the following is better ...
> 
>  slaves. The required properties '#address-cells' and '#size-cells'
>  must be defined under this subnode if present and not the parent node.

Naaw, I don't like that either, I associate the last "and not" with the
"if present" part and not the intended "under this subnode". Then I go
WTF when I fail to parse. Maybe just add an example instead...

Or just forget me ever saying anything. Sorry for making a fuss over this.

Cheers,
Peter

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang June 28, 2016, 9:32 p.m. UTC | #5
> For
> example some Tegra devices have an I2C interface which may share its
> pins with other devices and to share these pins subnodes for
> representing these pins so they have be shared via the pinctrl framework
> are needed.

I think the above sentence is hard to grasp. Can you split it into more
sentences perhaps?

> +- i2c-bus
> +	For I2C adapters that have child nodes that are a mixture of both I2C
> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'

I suggest to drop the phrase in the paranthesis. It is true for your
case, but I don't think it's generic. So, it is not an obvious example
like "Yes, sure, I see that a pin controller has I2C nodes and non-I2C
nodes". At least for me, it was more like "It has what?"
Jon Hunter June 29, 2016, 8:05 a.m. UTC | #6
On 28/06/16 22:32, Wolfram Sang wrote:
> * PGP Signed by an unknown key
> 
>> For
>> example some Tegra devices have an I2C interface which may share its
>> pins with other devices and to share these pins subnodes for
>> representing these pins so they have be shared via the pinctrl framework
>> are needed.
> 
> I think the above sentence is hard to grasp. Can you split it into more
> sentences perhaps?

OK, yes does seem a bit of a mouthful.

>> +- i2c-bus
>> +	For I2C adapters that have child nodes that are a mixture of both I2C
>> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
> 
> I suggest to drop the phrase in the paranthesis. It is true for your
> case, but I don't think it's generic. So, it is not an obvious example
> like "Yes, sure, I see that a pin controller has I2C nodes and non-I2C
> nodes". At least for me, it was more like "It has what?"

OK, will drop that part.

Cheers
Jon
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index f31b2ad1552b..71bea55d4c1b 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -32,6 +32,14 @@  wants to support one of the below features, it should adapt the bindings below.
 - clock-frequency
 	frequency of bus clock in Hz.
 
+- i2c-bus
+	For I2C adapters that have child nodes that are a mixture of both I2C
+	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
+	subnode can be used for populating I2C devices. If the 'i2c-bus'
+	subnode is present, only subnodes of this will be considered as I2C
+	slaves. The properties, '#address-cells' and '#size-cells' must be
+	defined under this subnode if present.
+
 - i2c-scl-falling-time-ns
 	Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C
 	specification.