From: Grant Likely <hidden> Date: 2007-05-14 19:11:33
Move the i2c-mpc driver over to using the new i2c infrastructure.
Specifically, it now uses i2c_add_numberd_adapter so that the bus number
can be determined ahead of time and used to register i2c clients before
the bus is instantiated.
Tested on an MPC5200 based board
Signed-off-by: Grant Likely <redacted>
---
This patch will need to be tested to make sure it does not break any 8xxx
board ports.
Work still to be done (in another patch): support for pulling i2c client
registrations out of the device tree.
drivers/i2c/busses/i2c-mpc.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
From: Grant Likely <hidden> Date: 2007-05-14 19:32:43
On 5/14/07, Grant Likely [off-list ref] wrote:
Work still to be done (in another patch): support for pulling i2c client
registrations out of the device tree.
I had also fogotten about Scott Woods patches from last Nov. which
take care of this.
g.
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
From: Jean Delvare <hidden> Date: 2007-05-15 13:25:35
On Mon, 14 May 2007 13:11:23 -0600, Grant Likely wrote:
Move the i2c-mpc driver over to using the new i2c infrastructure.
Specifically, it now uses i2c_add_numberd_adapter so that the bus number
i2c_add_numbered_adapter (missing e)
can be determined ahead of time and used to register i2c clients before
the bus is instantiated.
Tested on an MPC5200 based board
Signed-off-by: Grant Likely <redacted>
Acked-by: Jean Delvare <redacted>
quoted hunk
---
This patch will need to be tested to make sure it does not break any 8xxx
board ports.
Work still to be done (in another patch): support for pulling i2c client
registrations out of the device tree.
drivers/i2c/busses/i2c-mpc.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
@@ -327,9 +327,10 @@ static int fsl_i2c_probe(struct platform_device *pdev)platform_set_drvdata(pdev,i2c);i2c->adap=mpc_ops;+i2c->adap.nr=pdev->id;
By the way: mpc_ops is a static i2c_adapter, so given that
the reason for using pdev->id that way was that there might
be more than one such platform device ... shouldn't allocation
of the adapter be moved into allocation of the "i2c->" object?
Or at least, add a check to ensure that the static mpc_ops
structure isn't in use before progressing this probe().
- Dave
@@ -327,9 +327,10 @@ static int fsl_i2c_probe(struct platform_device *pdev)platform_set_drvdata(pdev,i2c);i2c->adap=mpc_ops;+i2c->adap.nr=pdev->id;
By the way: mpc_ops is a static i2c_adapter, so given that
the reason for using pdev->id that way was that there might
be more than one such platform device ... shouldn't allocation
of the adapter be moved into allocation of the "i2c->" object?
Take another look; this is a funny quirk of the driver. The
assignment is 'i2c->adap = mpc_ops'; not 'i2c->adap = &mpc_ops'. And
in struct mpc_i2c, the field is declared as 'struct i2c_adapter adap',
not 'struct i2c_adapter *adap'. The driver instance gets a copy of
the mpc_ops structure to initialize it, not a pointer to the staticly
defined structure. I got bitten by the same thing when I was looking
at the code.
Not to mention that mpc_ops is poorly named.
Cheers,
g.
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
From: David Brownell <hidden> Date: 2007-05-15 16:05:25
On Tuesday 15 May 2007, Grant Likely wrote:
Take another look; this is a funny quirk of the driver. The
assignment is 'i2c->adap = mpc_ops'; not 'i2c->adap = &mpc_ops'. And
in struct mpc_i2c, the field is declared as 'struct i2c_adapter adap',
not 'struct i2c_adapter *adap'. The driver instance gets a copy of
the mpc_ops structure to initialize it, not a pointer to the staticly
defined structure. I got bitten by the same thing when I was looking
at the code.
I see -- you're right. That "template" idiom is a good one to get
rid of, FWIW ... not only is it confusing, but it also wastes space.
- Dave
From: Jean Delvare <hidden> Date: 2007-05-16 18:25:17
On Tue, 15 May 2007 09:05:20 -0700, David Brownell wrote:
On Tuesday 15 May 2007, Grant Likely wrote:
quoted
Take another look; this is a funny quirk of the driver. The
assignment is 'i2c->adap = mpc_ops'; not 'i2c->adap = &mpc_ops'. And
in struct mpc_i2c, the field is declared as 'struct i2c_adapter adap',
not 'struct i2c_adapter *adap'. The driver instance gets a copy of
the mpc_ops structure to initialize it, not a pointer to the staticly
defined structure. I got bitten by the same thing when I was looking
at the code.
I see -- you're right. That "template" idiom is a good one to get
rid of, FWIW ... not only is it confusing, but it also wastes space.
Well, maybe you can submit a patch fixing this one?
--
Jean Delvare
From: Grant Likely <hidden> Date: 2007-05-16 18:38:12
On 5/16/07, Jean Delvare [off-list ref] wrote:
On Tue, 15 May 2007 09:05:20 -0700, David Brownell wrote:
quoted
On Tuesday 15 May 2007, Grant Likely wrote:
quoted
Take another look; this is a funny quirk of the driver. The
assignment is 'i2c->adap = mpc_ops'; not 'i2c->adap = &mpc_ops'. And
in struct mpc_i2c, the field is declared as 'struct i2c_adapter adap',
not 'struct i2c_adapter *adap'. The driver instance gets a copy of
the mpc_ops structure to initialize it, not a pointer to the staticly
defined structure. I got bitten by the same thing when I was looking
at the code.
I see -- you're right. That "template" idiom is a good one to get
rid of, FWIW ... not only is it confusing, but it also wastes space.
Well, maybe you can submit a patch fixing this one?
Heh, oops.
I had replied to David saying that I would do this bit; but I forgot
to cc the list.
I'll take care of this.
Cheers,
g.
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
By the way: mpc_ops is a static i2c_adapter, so given that
the reason for using pdev->id that way was that there might
be more than one such platform device ... shouldn't allocation
of the adapter be moved into allocation of the "i2c->" object?
Or at least, add a check to ensure that the static mpc_ops
structure isn't in use before progressing this probe().
The mpc_ops struct is only used as a template; it gets copied into
i2c->adap for each adapter instance.
-Scott