Re: [RFC] binding for nvec mfd device

Re: [RFC] binding for nvec mfd device

[Marc Dietrich]

Hi Stephen,

Am Mittwoch, 31. Juli 2013, 15:03:14 schrieb Stephen Warren:
> The generic I2C bindings already define that the other chips on the I2C
> bus appear directly underneath the I2C controller's DT node. Perhaps it
> isn't a big issue to change that, since each I2C controller can define
> the layout of its own node?
> 
> Anyway, we can probably get away without introducing multiple levels by
> adding some more bits or cells into the reg address for I2C child nodes:
> 
> i2c@xxxxxxxx {
> 	compatible = "nvidia,tegra20-i2c";
> 	... resources
> 	#address-cells = <2>;
> 
> 	codec {
> 		// 0 means external slave, 0x1c is slave's address
> 		reg = <0 0x1c>;
> 		...
> 	};
> 
> 	tegraslave {
> 		// 0 means internal slave, 0x80 is controller's address
> 		reg = <1 0x80>;
> 		...
> 	};
> };
> 
> ... where each of those child nodes could be repeated N times. We could
> also or in 0x80000000 to the reg values in the child nodes rather than
> using a separate cell if we wanted.

thinking a little more about this, this is way to complicated. "Master" and 
"Slave" functions are properties of the same i2c controller. Therefore, just 
adding a small property to the i2c controller node saying "enable slave 
support" is sufficient, as all the resources are shared (which is another good 
argument that it is the same device, hence same node). I would just add the 
slave i2c address, which is all the slave driver needs, e.g.
"slave-address = <8c>;" to enable slave mode operation. Both modes can be 
enabled at the same time (for loopback comm). AFAIK, if only the slave part is 
used, the master part shouldn't hurt if programmed (same as other way around).

The difficult part is how to specify the master if only the slave is used. As 
the master is naturally on the same bus as the slave, it should be a child 
node of the controller. Therefore it needs a special i2c address, maybe 
0x80000000 as you mentioned above, but this will be rejected by the i2c core. 
Instantiating the nvec as an i2c client could be another problem.

It looks like that the i2c subsystem needs some modifications to allow to 
specify a master device which is attached to a slave controller. Maybe someone 
from the i2c folks (cc'ed) can comment on this?

Marc

Re: [RFC] binding for nvec mfd device

[Stephen Warren]

On 09/17/2013 01:53 AM, Marc Dietrich wrote:
> Hi Stephen,
> 
> Am Mittwoch, 31. Juli 2013, 15:03:14 schrieb Stephen Warren:
>> The generic I2C bindings already define that the other chips on the I2C
>> bus appear directly underneath the I2C controller's DT node. Perhaps it
>> isn't a big issue to change that, since each I2C controller can define
>> the layout of its own node?
>>
>> Anyway, we can probably get away without introducing multiple levels by
>> adding some more bits or cells into the reg address for I2C child nodes:
>>
>> i2c@xxxxxxxx {
>> 	compatible = "nvidia,tegra20-i2c";
>> 	... resources
>> 	#address-cells = <2>;
>>
>> 	codec {
>> 		// 0 means external slave, 0x1c is slave's address
>> 		reg = <0 0x1c>;
>> 		...
>> 	};
>>
>> 	tegraslave {
>> 		// 0 means internal slave, 0x80 is controller's address
>> 		reg = <1 0x80>;
>> 		...
>> 	};
>> };
>>
>> ... where each of those child nodes could be repeated N times. We could
>> also or in 0x80000000 to the reg values in the child nodes rather than
>> using a separate cell if we wanted.
> 
> thinking a little more about this, this is way to complicated.

Really, this seems extremely simple to me.

> "Master" and 
> "Slave" functions are properties of the same i2c controller. Therefore, just 
> adding a small property to the i2c controller node saying "enable slave 
> support" is sufficient, as all the resources are shared (which is another good 
> argument that it is the same device, hence same node). I would just add the 
> slave i2c address, which is all the slave driver needs, e.g.

Perhaps so for this one controller, but we should strive to create a
binding style that can work with any I2C controller that can be master
or slave. For a general binding, I think we need to support multiple
slave addresses. The style above seems to support that with little
complexity.

> "slave-address = <8c>;" to enable slave mode operation. Both modes can be 
> enabled at the same time (for loopback comm). AFAIK, if only the slave part is 
> used, the master part shouldn't hurt if programmed (same as other way around).
> 
> The difficult part is how to specify the master if only the slave is used. As 
> the master is naturally on the same bus as the slave, it should be a child 
> node of the controller. Therefore it needs a special i2c address, maybe 

Why would the external I2C master be represented in the DT bindings at
all? It's not really a SW-accessible entity; its presence is only
observable not something you can initiate interaction with.

I assume you're not talking about the Tegra I2C controller when you say
"master" above; that is already represented in the DT as the top-level
I2C node.

> 0x80000000 as you mentioned above, but this will be rejected by the i2c core. 
> Instantiating the nvec as an i2c client could be another problem.
...
> It looks like that the i2c subsystem needs some modifications to allow to 
> specify a master device which is attached to a slave controller. Maybe someone 
> from the i2c folks (cc'ed) can comment on this?

Yes, you certainly will need some enhancements to the I2C core to
support slave mode. I suggest that I2C drivers gain an .of_xlate
callback which translates child node DT reg values (addresses) into the
(master-vs-slave, I2C address) pair.

Re: [RFC] binding for nvec mfd device

[Marc Dietrich]

Am Dienstag, 17. September 2013, 15:48:12 schrieb Stephen Warren:
> On 09/17/2013 01:53 AM, Marc Dietrich wrote:
> > Hi Stephen,
> > 
> > Am Mittwoch, 31. Juli 2013, 15:03:14 schrieb Stephen Warren:
> >> The generic I2C bindings already define that the other chips on the I2C
> >> bus appear directly underneath the I2C controller's DT node. Perhaps it
> >> isn't a big issue to change that, since each I2C controller can define
> >> the layout of its own node?
> >> 
> >> Anyway, we can probably get away without introducing multiple levels by
> >> adding some more bits or cells into the reg address for I2C child nodes:
> >> 
> >> i2c@xxxxxxxx {
> >> 
> >> 	compatible = "nvidia,tegra20-i2c";
> >> 	... resources
> >> 	#address-cells = <2>;
> >> 	
> >> 	codec {
> >> 	
> >> 		// 0 means external slave, 0x1c is slave's address
> >> 		reg = <0 0x1c>;
> >> 		...
> >> 	
> >> 	};
> >> 	
> >> 	tegraslave {
> >> 	
> >> 		// 0 means internal slave, 0x80 is controller's address
> >> 		reg = <1 0x80>;
> >> 		...
> >> 	
> >> 	};
> >> 
> >> };
> >> 
> >> ... where each of those child nodes could be repeated N times. We could
> >> also or in 0x80000000 to the reg values in the child nodes rather than
> >> using a separate cell if we wanted.
> > 
> > thinking a little more about this, this is way to complicated.
> 
> Really, this seems extremely simple to me.
> 
> > "Master" and
> > "Slave" functions are properties of the same i2c controller. Therefore,
> > just adding a small property to the i2c controller node saying "enable
> > slave support" is sufficient, as all the resources are shared (which is
> > another good argument that it is the same device, hence same node). I
> > would just add the slave i2c address, which is all the slave driver
> > needs, e.g.
> 
> Perhaps so for this one controller, but we should strive to create a
> binding style that can work with any I2C controller that can be master
> or slave. For a general binding, I think we need to support multiple
> slave addresses. The style above seems to support that with little
> complexity.

I have no idea how other implementations may look like. The downstream kernel 
seems to program the slave address in a secondary step after the controller is 
initialized [1]. So every "client" which binds to the slave driver can use its 
own address, but only one client at the same time is allowed. As said before, 
the may also exist devices (!tegra) with multiple slave addresses at the same 
time.

Your approach above seems fulfill these properties (what about a single slave 
controller without a master?), but it will be tricky for the slave to find the 
controller resources it needs. I'm thinking of an insane (but valid) 
configuration where all i2c ports of the SoC are connected together and one 
port plays the master and all others are slaves. In this case we need to add a 
property to the slave node which points to his controller instance.

> > "slave-address = <8c>;" to enable slave mode operation. Both modes can be
> > enabled at the same time (for loopback comm). AFAIK, if only the slave
> > part is used, the master part shouldn't hurt if programmed (same as other
> > way around).
> > 
> > The difficult part is how to specify the master if only the slave is used.
> > As the master is naturally on the same bus as the slave, it should be a
> > child node of the controller. Therefore it needs a special i2c address,
> > maybe
> Why would the external I2C master be represented in the DT bindings at
> all? It's not really a SW-accessible entity; its presence is only
> observable not something you can initiate interaction with.

No, we can and nvec does it regulary using a "side"-channel (a tegra gpio 
connected to a master irq input). This side-channel is non standard and each 
vendor may do it differently. But you are right, we don't need an entry for 
the master, but an entry for the driver (the other side of the communication) 
which binds to the slave controller.

> I assume you're not talking about the Tegra I2C controller when you say
> "master" above; that is already represented in the DT as the top-level
> I2C node.
> 
> > 0x80000000 as you mentioned above, but this will be rejected by the i2c
> > core. Instantiating the nvec as an i2c client could be another problem.
> 
> ...
> 
> > It looks like that the i2c subsystem needs some modifications to allow to
> > specify a master device which is attached to a slave controller. Maybe
> > someone from the i2c folks (cc'ed) can comment on this?
> 
> Yes, you certainly will need some enhancements to the I2C core to
> support slave mode. I suggest that I2C drivers gain an .of_xlate
> callback which translates child node DT reg values (addresses) into the
> (master-vs-slave, I2C address) pair.

I somehow feared that :-( Now my free time became rather limited recently and 
I don't feel that I'm the best person to solve these problem(s). Andrey said 
that he may be interested finishing at least a proper device tree 
representation (especially to get his great nvec uboot support upstreamed). 
Kernel support is a different beast and I really hope that the code which 
lives in staging area currently is not doomed because integration with the i2c 
subsystem is too complex.

Marc


[1] http://nv-tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=blob;f=drivers/i2c/i2c-slave.c;h=280a860cd2e84e15b2c1aef0e938d750f56863dd;hb=rel-roth-ota-1#l120

Re: [RFC] binding for nvec mfd device

[Stephen Warren]

On 09/23/2013 08:52 AM, Marc Dietrich wrote:
> Am Dienstag, 17. September 2013, 15:48:12 schrieb Stephen Warren:
>> On 09/17/2013 01:53 AM, Marc Dietrich wrote:
>>> Hi Stephen,
>>>
>>> Am Mittwoch, 31. Juli 2013, 15:03:14 schrieb Stephen Warren:
>>>> The generic I2C bindings already define that the other chips on the I2C
>>>> bus appear directly underneath the I2C controller's DT node. Perhaps it
>>>> isn't a big issue to change that, since each I2C controller can define
>>>> the layout of its own node?
>>>>
>>>> Anyway, we can probably get away without introducing multiple levels by
>>>> adding some more bits or cells into the reg address for I2C child nodes:
>>>>
>>>> i2c@xxxxxxxx {
>>>>
>>>> 	compatible = "nvidia,tegra20-i2c";
>>>> 	... resources
>>>> 	#address-cells = <2>;
>>>> 	
>>>> 	codec {
>>>> 	
>>>> 		// 0 means external slave, 0x1c is slave's address
>>>> 		reg = <0 0x1c>;
>>>> 		...
>>>> 	
>>>> 	};
>>>> 	
>>>> 	tegraslave {
>>>> 	
>>>> 		// 0 means internal slave, 0x80 is controller's address
>>>> 		reg = <1 0x80>;
>>>> 		...
>>>> 	
>>>> 	};
>>>>
>>>> };
>>>>
>>>> ... where each of those child nodes could be repeated N times. We could
>>>> also or in 0x80000000 to the reg values in the child nodes rather than
>>>> using a separate cell if we wanted.
>>>
>>> thinking a little more about this, this is way to complicated.
>>
>> Really, this seems extremely simple to me.
>>
>>> "Master" and
>>> "Slave" functions are properties of the same i2c controller. Therefore,
>>> just adding a small property to the i2c controller node saying "enable
>>> slave support" is sufficient, as all the resources are shared (which is
>>> another good argument that it is the same device, hence same node). I
>>> would just add the slave i2c address, which is all the slave driver
>>> needs, e.g.
>>
>> Perhaps so for this one controller, but we should strive to create a
>> binding style that can work with any I2C controller that can be master
>> or slave. For a general binding, I think we need to support multiple
>> slave addresses. The style above seems to support that with little
>> complexity.
> 
> I have no idea how other implementations may look like. The downstream kernel 
> seems to program the slave address in a secondary step after the controller is 
> initialized [1]. So every "client" which binds to the slave driver can use its 
> own address, but only one client at the same time is allowed. As said before, 
> the may also exist devices (!tegra) with multiple slave addresses at the same 
> time.
> 
> Your approach above seems fulfill these properties (what about a single slave 
> controller without a master?),

There would simply be a node for the controller, and a single child node
for the slave setup, and no child nodes for mastered devices.

> but it will be tricky for the slave to find the
> controller resources it needs.

I don't understand this. The resources are only needed by I2C driver,
and all come from the I2C driver node. If the I2C driver ends up being
split into separate master/slave portions, that information can easily
be passed from the main driver probe()/... to those other sub-parts of
the driver.

> I'm thinking of an insane (but valid)
> configuration where all i2c ports of the SoC are connected together and one 
> port plays the master and all others are slaves. In this case we need to add a 
> property to the slave node which points to his controller instance.

Why do the slave nodes care where they're mastered from?

I think you'd just have the following

/* master */
i2c@xxxxx {
    foo@0x40 {
        reg = <MASTER 0x40>;
        compatible = "nvidia,nvec";
    }
};

i2c@yyyy {
    foo@40 {
        reg = <SLAVE 0x40>;
        compatible = "nvidia,nvec-slave";
    }
};

There's no need for the slave child node to know that it is mastered
from the Tegra I2C controller; all it cares about is that there is some
I2C bus that it needs to respond to transactions upon.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] binding for nvec mfd device

[Andrey Danin]

On Mon, 2013-09-23 at 10:36 -0600, Stephen Warren wrote:
> On 09/23/2013 08:52 AM, Marc Dietrich wrote:
> > Am Dienstag, 17. September 2013, 15:48:12 schrieb Stephen Warren:
> >> On 09/17/2013 01:53 AM, Marc Dietrich wrote:
> >>> Hi Stephen,
> >>>
> >>> Am Mittwoch, 31. Juli 2013, 15:03:14 schrieb Stephen Warren:
> >>>> The generic I2C bindings already define that the other chips on the I2C
> >>>> bus appear directly underneath the I2C controller's DT node. Perhaps it
> >>>> isn't a big issue to change that, since each I2C controller can define
> >>>> the layout of its own node?
> >>>>
> >>>> Anyway, we can probably get away without introducing multiple levels by
> >>>> adding some more bits or cells into the reg address for I2C child nodes:
> >>>>
> >>>> i2c@xxxxxxxx {
> >>>>
> >>>> 	compatible = "nvidia,tegra20-i2c";
> >>>> 	... resources
> >>>> 	#address-cells = <2>;
> >>>> 	
> >>>> 	codec {
> >>>> 	
> >>>> 		// 0 means external slave, 0x1c is slave's address
> >>>> 		reg = <0 0x1c>;
> >>>> 		...
> >>>> 	
> >>>> 	};
> >>>> 	
> >>>> 	tegraslave {
> >>>> 	
> >>>> 		// 0 means internal slave, 0x80 is controller's address
> >>>> 		reg = <1 0x80>;
> >>>> 		...
> >>>> 	
> >>>> 	};
> >>>>
> >>>> };
> >>>>
> >>>> ... where each of those child nodes could be repeated N times. We could
> >>>> also or in 0x80000000 to the reg values in the child nodes rather than
> >>>> using a separate cell if we wanted.
> >>>
> >>> thinking a little more about this, this is way to complicated.
> >>
> >> Really, this seems extremely simple to me.
> >>
> >>> "Master" and
> >>> "Slave" functions are properties of the same i2c controller. Therefore,
> >>> just adding a small property to the i2c controller node saying "enable
> >>> slave support" is sufficient, as all the resources are shared (which is
> >>> another good argument that it is the same device, hence same node). I
> >>> would just add the slave i2c address, which is all the slave driver
> >>> needs, e.g.
> >>
> >> Perhaps so for this one controller, but we should strive to create a
> >> binding style that can work with any I2C controller that can be master
> >> or slave. For a general binding, I think we need to support multiple
> >> slave addresses. The style above seems to support that with little
> >> complexity.
> > 
> > I have no idea how other implementations may look like. The downstream kernel 
> > seems to program the slave address in a secondary step after the controller is 
> > initialized [1]. So every "client" which binds to the slave driver can use its 
> > own address, but only one client at the same time is allowed. As said before, 
> > the may also exist devices (!tegra) with multiple slave addresses at the same 
> > time.
> > 
> > Your approach above seems fulfill these properties (what about a single slave 
> > controller without a master?),
> 
> There would simply be a node for the controller, and a single child node
> for the slave setup, and no child nodes for mastered devices.
> 
> > but it will be tricky for the slave to find the
> > controller resources it needs.
> 
> I don't understand this. The resources are only needed by I2C driver,
> and all come from the I2C driver node. If the I2C driver ends up being
> split into separate master/slave portions, that information can easily
> be passed from the main driver probe()/... to those other sub-parts of
> the driver.
> 
> > I'm thinking of an insane (but valid)
> > configuration where all i2c ports of the SoC are connected together and one 
> > port plays the master and all others are slaves. In this case we need to add a 
> > property to the slave node which points to his controller instance.
> 
> Why do the slave nodes care where they're mastered from?
> 
> I think you'd just have the following
> 
> /* master */
> i2c@xxxxx {
>     foo@0x40 {
>         reg = <MASTER 0x40>;
>         compatible = "nvidia,nvec";
>     }
> };
> 
> i2c@yyyy {
>     foo@40 {
>         reg = <SLAVE 0x40>;
>         compatible = "nvidia,nvec-slave";
>     }
> };
> 
> There's no need for the slave child node to know that it is mastered
> from the Tegra I2C controller; all it cares about is that there is some
> I2C bus that it needs to respond to transactions upon.
> 
> 
This binding describes only case, when I2C device are connected to I2C
controller.

Assume that I2C controller #1 (@xxxxx), I2C controller #2 (@yyyy), and
nvec I2C master device are connected to same bus.
How dt must be composed in this case ? Must i2c@xxxxx and i2c@yyyy be in
parent/child relation (in terms of dt) ?


p.s. sorry, prev message was not formatted correctly, so I resend it.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] binding for nvec mfd device

[Marc Dietrich]

Am Dienstag, 24. September 2013, 11:39:21 schrieb Andrey Danin:
> On Mon, 2013-09-23 at 10:36 -0600, Stephen Warren wrote:
> > On 09/23/2013 08:52 AM, Marc Dietrich wrote:
> > > Am Dienstag, 17. September 2013, 15:48:12 schrieb Stephen Warren:
> > >> On 09/17/2013 01:53 AM, Marc Dietrich wrote:
> > >>> Hi Stephen,
> > >>> 
> > >>> Am Mittwoch, 31. Juli 2013, 15:03:14 schrieb Stephen Warren:
> > >>>> The generic I2C bindings already define that the other chips on the
> > >>>> I2C
> > >>>> bus appear directly underneath the I2C controller's DT node. Perhaps
> > >>>> it
> > >>>> isn't a big issue to change that, since each I2C controller can
> > >>>> define
> > >>>> the layout of its own node?
> > >>>> 
> > >>>> Anyway, we can probably get away without introducing multiple levels
> > >>>> by
> > >>>> adding some more bits or cells into the reg address for I2C child
> > >>>> nodes:
> > >>>> 
> > >>>> i2c@xxxxxxxx {
> > >>>> 
> > >>>> 	compatible = "nvidia,tegra20-i2c";
> > >>>> 	... resources
> > >>>> 	#address-cells = <2>;
> > >>>> 	
> > >>>> 	codec {
> > >>>> 	
> > >>>> 		// 0 means external slave, 0x1c is slave's address
> > >>>> 		reg = <0 0x1c>;
> > >>>> 		...
> > >>>> 	
> > >>>> 	};
> > >>>> 	
> > >>>> 	tegraslave {
> > >>>> 	
> > >>>> 		// 0 means internal slave, 0x80 is controller's address
> > >>>> 		reg = <1 0x80>;
> > >>>> 		...
> > >>>> 	
> > >>>> 	};
> > >>>> 
> > >>>> };
> > >>>> 
> > >>>> ... where each of those child nodes could be repeated N times. We
> > >>>> could
> > >>>> also or in 0x80000000 to the reg values in the child nodes rather
> > >>>> than
> > >>>> using a separate cell if we wanted.
> > >>> 
> > >>> thinking a little more about this, this is way to complicated.
> > >> 
> > >> Really, this seems extremely simple to me.
> > >> 
> > >>> "Master" and
> > >>> "Slave" functions are properties of the same i2c controller.
> > >>> Therefore,
> > >>> just adding a small property to the i2c controller node saying "enable
> > >>> slave support" is sufficient, as all the resources are shared (which
> > >>> is
> > >>> another good argument that it is the same device, hence same node). I
> > >>> would just add the slave i2c address, which is all the slave driver
> > >>> needs, e.g.
> > >> 
> > >> Perhaps so for this one controller, but we should strive to create a
> > >> binding style that can work with any I2C controller that can be master
> > >> or slave. For a general binding, I think we need to support multiple
> > >> slave addresses. The style above seems to support that with little
> > >> complexity.
> > > 
> > > I have no idea how other implementations may look like. The downstream
> > > kernel seems to program the slave address in a secondary step after the
> > > controller is initialized [1]. So every "client" which binds to the
> > > slave driver can use its own address, but only one client at the same
> > > time is allowed. As said before, the may also exist devices (!tegra)
> > > with multiple slave addresses at the same time.
> > > 
> > > Your approach above seems fulfill these properties (what about a single
> > > slave controller without a master?),
> > 
> > There would simply be a node for the controller, and a single child node
> > for the slave setup, and no child nodes for mastered devices.
> > 
> > > but it will be tricky for the slave to find the
> > > controller resources it needs.
> > 
> > I don't understand this. The resources are only needed by I2C driver,
> > and all come from the I2C driver node. If the I2C driver ends up being
> > split into separate master/slave portions, that information can easily
> > be passed from the main driver probe()/... to those other sub-parts of
> > the driver.
> > 
> > > I'm thinking of an insane (but valid)
> > > configuration where all i2c ports of the SoC are connected together and
> > > one
> > > port plays the master and all others are slaves. In this case we need to
> > > add a property to the slave node which points to his controller
> > > instance.> 
> > Why do the slave nodes care where they're mastered from?
> > 
> > I think you'd just have the following
> > 
> > /* master */
> > i2c@xxxxx {
> > 
> >     foo@0x40 {
> >     
> >         reg = <MASTER 0x40>;
> >         compatible = "nvidia,nvec";
> >     
> >     }
> > 
> > };
> > 
> > i2c@yyyy {
> > 
> >     foo@40 {
> >     
> >         reg = <SLAVE 0x40>;
> >         compatible = "nvidia,nvec-slave";
> >     
> >     }
> > 
> > };
> > 
> > There's no need for the slave child node to know that it is mastered
> > from the Tegra I2C controller; all it cares about is that there is some
> > I2C bus that it needs to respond to transactions upon.
> 
> This binding describes only case, when I2C device are connected to I2C
> controller.
> 
> Assume that I2C controller #1 (@xxxxx), I2C controller #2 (@yyyy), and
> nvec I2C master device are connected to same bus.
> How dt must be composed in this case ? Must i2c@xxxxx and i2c@yyyy be in
> parent/child relation (in terms of dt) ?

What I learned from the somehow unfortune discussion yesterday on IRC is that 
we cannot describe the exact i2c topology in the device tree, especially if it 
is software configuable (slave/master changing roles).

I also think there is a misunderstanding about how things get layered in 
software (and which info from device tree the drivers need). In your example 
above, there is a child node unter each controller which is responsible for 
slave transfers. The nvec can be a separate node in the device tree outside of 
the i2c structure, but needs one or more pointers to slave drivers which he 
can use to send his command byte streams to. E.g.

i2c@xxxx {
	compatible = "nvidia,tegra-i2c";
	// resources for the i2c controller
	first_slave: slave@40 {
		compatible = "nvidia,tegra-i2c-slave";
		reg = <40>; // i2c client address
		gpios = <4e>; // for side channel
	};
};

i2c@yyyy {
	compatible = "nvidia,tegra-i2c";
	// resources for the i2c controller
	second_slave: slave@41 {
		compatible = "nvidia,tegra-i2c-slave";
		reg = <41>; // i2c client address
		gpios = <4f>; // for side channel
	};
};

external_master {
	compatible = "ext,master";
	slaves = &first_slave,&second_slave;
};

or

nvec {
	compatible = "nvidia,nvec";
	slave = &first_slave;
	keyboard {
		// keyboard resources
	};
};

I'm not sure where the side channel belongs to. In case of nvec, the gpio must
be released after the first byte went over the wire, so putting it into nvec 
node is not possible (or needs a callback from the layer below).

Marc

Re: [RFC] binding for nvec mfd device

[Stephen Warren]

On 09/24/2013 03:33 AM, Marc Dietrich wrote:
...
> What I learned from the somehow unfortune discussion yesterday on IRC is that 
> we cannot describe the exact i2c topology in the device tree, especially if it 
> is software configuable (slave/master changing roles).

Well, there's also no need to represent the exact I2C topology; we only
need to represent the parts that SW cares about. The existence of other
masters on the bus is not something that SW typically cares about. SW
can't directly interact with other masters (through just plain I2C
protocol at least), and can only observe their presence by potential
arbitration loss events.

> I also think there is a misunderstanding about how things get layered in 
> software (and which info from device tree the drivers need). In your example 
> above, there is a child node unter each controller which is responsible for 
> slave transfers. The nvec can be a separate node in the device tree outside of 
> the i2c structure, but needs one or more pointers to slave drivers which he 
> can use to send his command byte streams to. E.g.

No, in my example, the slave node /is/ the NVEC protocol. There is no
separate NVEC node outside the I2C node/tree.

> i2c@xxxx {
> 	compatible = "nvidia,tegra-i2c";
> 	// resources for the i2c controller
> 	first_slave: slave@40 {
> 		compatible = "nvidia,tegra-i2c-slave";

That's not what I wrote. This should be:

		compatible = "nvidia,nvec-slave";

... which instantiates an I2C slave protocol driver, and hooks it up so
that if the I2C slave controller receives an externally-initiated I2C
transaction to I2C address 0x40, that transaction will be passed to the
NVEC driver for processing and response.


> 		reg = <40>; // i2c client address
> 		gpios = <4e>; // for side channel
> 	};
> };

...
> external_master {
> 	compatible = "ext,master";
> 	slaves = &first_slave,&second_slave;
> };
> 
> or
> 
> nvec {
> 	compatible = "nvidia,nvec";
> 	slave = &first_slave;
> 	keyboard {
> 		// keyboard resources
> 	};
> };

So that separate nvec node is not required. All configuration of the
NVEC slave protocol should be part of the slave@40 node above.

IIRC, the way NVEC works is that a GPIO is used to request that the NVEC
IC poll the slave device if the slave device wants to initiate a
transaction with the NVEC IC. If that's correct, the model above should
work fine.

However, if when Tegra wants to make a master transaction on the I2C bus
(which would therefore have to be to a device other than the NVEC IC),
if the NVEC IC and Tegra need to arbitrate for the bus outside the plain
I2C protocol, then you'd want to model this as an I2C bus mux instead:

	i2c1: i2c@xxxx {
		compatible = "nvidia,tegra-i2c";

		foo@40 {
			reg = <SLAVE 0x40>;
			compatible = "nvidia,nvec-slave";
			keyboard {
				...
			};
			// any other parameters NVEC slave
			// protocol needs go here
		}
	};

	i2cmux {
		compatible = "nvidia,i2c-mux-nvec";
		#address-cells = <1>;
		#size-cells = <0>;
		nvec-gpios = <&gpio1 22 0>;
		i2c-parent = <&i2c1>;

		// all other devices on the I2C bus go here:
		ssd1307: oled@3c {
			compatible = "solomon,ssd1307fb-i2c";
				reg = <0x3c>;
				pwms = <&pwm 4 3000>;
				reset-gpios = <&gpio2 7 1>;
				reset-active-low;
			};
		};
	};

In this case, perhaps the drivers for the NVEC bus mux and NVEC slave
protocol might need to communicate with each-other. In which case, a
phandle between the two may be appropriate.

Re: [RFC] binding for nvec mfd device

[Stephen Warren]

On 09/24/2013 01:19 AM, Andrey Danin wrote:
> On Mon, Sep 23, 2013 at 8:36 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org
...
>     I think you'd just have the following
> 
>     /* master */
>     i2c@xxxxx {
>         foo@0x40 {
>             reg = <MASTER 0x40>;
>             compatible = "nvidia,nvec";
>         }
>     };
> 
>     i2c@yyyy {
>         foo@40 {
>             reg = <SLAVE 0x40>;
>             compatible = "nvidia,nvec-slave";
>         }
>     };
> 
>     There's no need for the slave child node to know that it is mastered
>     from the Tegra I2C controller; all it cares about is that there is some
>     I2C bus that it needs to respond to transactions upon.
> 
> This binding describes only case, when I2C device are connected to I2C
> controller.
> 
> Assume that I2C controller #1 (@xxxxx), I2C controller #2 (@yyyy), and
> nvec I2C master device are connected to same bus.
> How dt must be composed in this case ? Must i2c@xxxxx and i2c@yyyy be in
> parent/child relation (in terms of dt) ?

None of the I2C bindings currently allow one to specify that multiple of
the on-SoC controllers are connected to the same bus.

I'm not sure it's particularly useful to represent this anyway. Hardware
hooked up like this is pretty rare to start with (i.e. I know of no
board at all that's connected this way). I assume that if such HW did
exist, you'd simply assign each I2C slave to a particular I2C master,
and hence only put a DT node for it under a single DT master node.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] binding for nvec mfd device

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]


> It looks like that the i2c subsystem needs some modifications to allow to 
> specify a master device which is attached to a slave controller. Maybe someone 
> from the i2c folks (cc'ed) can comment on this?

The linux i2c core does not really support controllers being a slave.
There are probably some more obstacles on the way if you want to do
this, depends on what you want to do.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]