2013-09-29 08:49:29

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 0/8] i2c: Remove redundant driver field from the i2c_client struct

Hi,

This series removes the redundant driver field from the i2c_client struct. The
field is redundant since the same pointer can be accessed through
to_i2c_driver(client->dev.driver). The commit log suggests that the field has
been around since forever (since before v2.6.12-rc2) and it looks as if it was
simply forgotten to remove it during the conversion of the i2c framework to the
generic device driver model.

Nevertheless there are a still a few users of the field around. This series
first updates all users to use an alternative method of accessing the same data
and then the last patch removes the driver field from the i2c_client struct.

Note that due to this changes on most architectures neither the code size nor
the type of generated instructions will change. This is due to the fact that we
aren't really interested in the pointer value itself, but rather want to
dereference it to access one of the fields of the struct. offset_of() (and hence
to_i2c_driver) works by subtracting a offset from the pointer, so the compiler
can internally create the sum of these two offsets and use that to access the
field.

E.g. on ARM the expression client->driver->command(...) generates

...
ldr r3, [r0, #28]
ldr r3, [r3, #32]
blx r3
...

and the expression to_i2c_driver(client->dev.driver)->command(...) generates

...
ldr r3, [r0, #160]
ldr r3, [r3, #-4]
blx r3
...

Other architectures will generate similar code.

The most common pattern is to use the i2c_driver to get to the device_driver
struct embedded in it. The same struct can easily be accessed through the device
struct embedded in the i2c_client struct. E.g. client->driver->driver.field can
be replaced by client->dev.driver->field. Here again the generated code is
almost identical and only the offsets differ.

E.g. on ARM the expression 'client->driver->driver.owner' generates

ldr r3, [r0, #28]
ldr r0, [r3, #44]

and 'client->dev.driver->owner' generates

ldr r3, [r0, #160]
ldr r0, [r3, #8]

The kernel overall code size is slightly reduced since the code that manages the
driver field is removed and of course also the runtime memory footprint of the
i2c_client struct is reduced.

- Lars

Lars-Peter Clausen (8):
[media] s5c73m3: Don't use i2c_client->driver
[media] exynos4-is: Don't use i2c_client->driver
[media] core: Don't use i2c_client->driver
drm: encoder_slave: Don't use i2c_client->driver
drm: nouveau: Don't use i2c_client->driver
ALSA: ppc: keywest: Don't use i2c_client->driver
ASoC: imx-wm8962: Don't use i2c_client->driver
i2c: Remove redundant 'driver' field from the i2c_client struct

drivers/gpu/drm/drm_encoder_slave.c | 8 ++++----
drivers/gpu/drm/nouveau/core/subdev/therm/ic.c | 3 ++-
drivers/i2c/i2c-core.c | 21 ++++++++++++---------
drivers/i2c/i2c-smbus.c | 10 ++++++----
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +-
drivers/media/platform/exynos4-is/media-dev.c | 6 +++---
drivers/media/v4l2-core/tuner-core.c | 6 +++---
drivers/media/v4l2-core/v4l2-common.c | 10 +++++-----
include/linux/i2c.h | 2 --
include/media/v4l2-common.h | 2 +-
sound/ppc/keywest.c | 4 ++--
sound/soc/fsl/imx-wm8962.c | 2 +-
12 files changed, 40 insertions(+), 36 deletions(-)

--
1.8.0


2013-09-29 08:49:37

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 2/8] [media] exynos4-is: Don't use i2c_client->driver

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client->driver.driver->field' and
'client->dev.driver->field' are identical, so replace all occurrences of the
former with the later.

Signed-off-by: Lars-Peter Clausen <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Sylwester Nawrocki <[email protected]>
---
drivers/media/platform/exynos4-is/media-dev.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index a835112..7a4ee4c 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -411,8 +411,8 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,

device_lock(&client->dev);

- if (!client->driver ||
- !try_module_get(client->driver->driver.owner)) {
+ if (!client->dev.driver ||
+ !try_module_get(client->dev.driver->owner)) {
ret = -EPROBE_DEFER;
v4l2_info(&fmd->v4l2_dev, "No driver found for %s\n",
node->full_name);
@@ -442,7 +442,7 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,
fmd->num_sensors++;

mod_put:
- module_put(client->driver->driver.owner);
+ module_put(client->dev.driver->owner);
dev_put:
device_unlock(&client->dev);
put_device(&client->dev);
--
1.8.0

2013-09-29 08:49:42

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 4/8] drm: encoder_slave: Don't use i2c_client->driver

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client->driver.driver->field' and
'client->dev.driver->field' are identical, so replace all occurrences of the
former with the later. To get direct access to the i2c_driver struct use
'to_i2c_driver(client->dev.driver)'.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/gpu/drm/drm_encoder_slave.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_encoder_slave.c b/drivers/gpu/drm/drm_encoder_slave.c
index 0cfb60f..d18b88b 100644
--- a/drivers/gpu/drm/drm_encoder_slave.c
+++ b/drivers/gpu/drm/drm_encoder_slave.c
@@ -67,12 +67,12 @@ int drm_i2c_encoder_init(struct drm_device *dev,
goto fail;
}

- if (!client->driver) {
+ if (!client->dev.driver) {
err = -ENODEV;
goto fail_unregister;
}

- module = client->driver->driver.owner;
+ module = client->dev.driver->owner;
if (!try_module_get(module)) {
err = -ENODEV;
goto fail_unregister;
@@ -80,7 +80,7 @@ int drm_i2c_encoder_init(struct drm_device *dev,

encoder->bus_priv = client;

- encoder_drv = to_drm_i2c_encoder_driver(client->driver);
+ encoder_drv = to_drm_i2c_encoder_driver(to_i2c_driver(client->dev.driver));

err = encoder_drv->encoder_init(client, dev, encoder);
if (err)
@@ -111,7 +111,7 @@ void drm_i2c_encoder_destroy(struct drm_encoder *drm_encoder)
{
struct drm_encoder_slave *encoder = to_encoder_slave(drm_encoder);
struct i2c_client *client = drm_i2c_encoder_get_client(drm_encoder);
- struct module *module = client->driver->driver.owner;
+ struct module *module = client->dev.driver->owner;

i2c_unregister_device(client);
encoder->bus_priv = NULL;
--
1.8.0

2013-09-29 08:49:53

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 6/8] ALSA: ppc: keywest: Don't use i2c_client->driver

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. Use 'to_i2c_driver(client->dev.driver)' instead to get direct
access to the i2c_driver struct.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
sound/ppc/keywest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
index 01aecc2..0d1c27e 100644
--- a/sound/ppc/keywest.c
+++ b/sound/ppc/keywest.c
@@ -65,7 +65,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
* already bound. If not it means binding failed, and then there
* is no point in keeping the device instantiated.
*/
- if (!keywest_ctx->client->driver) {
+ if (!keywest_ctx->client->dev.driver) {
i2c_unregister_device(keywest_ctx->client);
keywest_ctx->client = NULL;
return -ENODEV;
@@ -76,7 +76,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
* This is safe because i2c-core holds the core_lock mutex for us.
*/
list_add_tail(&keywest_ctx->client->detected,
- &keywest_ctx->client->driver->clients);
+ &to_i2c_driver(keywest_ctx->client->dev.driver)->clients);
return 0;
}

--
1.8.0

2013-09-29 08:49:50

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 5/8] drm: nouveau: Don't use i2c_client->driver

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. Use 'to_i2c_driver(client->dev.driver)' instead to get direct access to
the i2c_driver struct.

Signed-off-by: Lars-Peter Clausen <[email protected]>
Cc: Martin Peres <[email protected]>
---
drivers/gpu/drm/nouveau/core/subdev/therm/ic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c b/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c
index 8b3adec..eae939d 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c
@@ -41,7 +41,8 @@ probe_monitoring_device(struct nouveau_i2c_port *i2c,
if (!client)
return false;

- if (!client->driver || client->driver->detect(client, info)) {
+ if (!client->dev.driver ||
+ to_i2c_driver(client->dev.driver)->detect(client, info)) {
i2c_unregister_device(client);
return false;
}
--
1.8.0

2013-09-29 08:50:08

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 8/8] i2c: Remove redundant 'driver' field from the i2c_client struct

The 'driver' field of the i2c_client struct is redundant. The same data can be
accessed through to_i2c_driver(client->dev.driver). The generated code for both
approaches in more or less the same.

E.g. on ARM the expression client->driver->command(...) generates

...
ldr r3, [r0, #28]
ldr r3, [r3, #32]
blx r3
...

and the expression to_i2c_driver(client->dev.driver)->command(...) generates

...
ldr r3, [r0, #160]
ldr r3, [r3, #-4]
blx r3
...

Other architectures will generate similar code.

All users of the 'driver' field outside of the I2C core have already been
converted. So this only leaves the core itself. This patch converts the
remaining few users in the I2C core and then removes the 'driver' field from the
i2c_client struct.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/i2c/i2c-core.c | 21 ++++++++++++---------
drivers/i2c/i2c-smbus.c | 10 ++++++----
include/linux/i2c.h | 2 --
3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 29d3f04..430c001 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -248,17 +248,16 @@ static int i2c_device_probe(struct device *dev)
driver = to_i2c_driver(dev->driver);
if (!driver->probe || !driver->id_table)
return -ENODEV;
- client->driver = driver;
+
if (!device_can_wakeup(&client->dev))
device_init_wakeup(&client->dev,
client->flags & I2C_CLIENT_WAKE);
dev_dbg(dev, "probe\n");

status = driver->probe(client, i2c_match_id(driver->id_table, client));
- if (status) {
- client->driver = NULL;
+ if (status)
i2c_set_clientdata(client, NULL);
- }
+
return status;
}

@@ -279,10 +278,9 @@ static int i2c_device_remove(struct device *dev)
dev->driver = NULL;
status = 0;
}
- if (status == 0) {
- client->driver = NULL;
+ if (status == 0)
i2c_set_clientdata(client, NULL);
- }
+
return status;
}

@@ -1606,9 +1604,14 @@ static int i2c_cmd(struct device *dev, void *_arg)
{
struct i2c_client *client = i2c_verify_client(dev);
struct i2c_cmd_arg *arg = _arg;
+ struct i2c_driver *driver;
+
+ if (!client || !client->dev.driver)
+ return 0;

- if (client && client->driver && client->driver->command)
- client->driver->command(client, arg->cmd, arg->arg);
+ driver = to_i2c_driver(client->dev.driver);
+ if (driver->command)
+ driver->command(client, arg->cmd, arg->arg);
return 0;
}

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 44d4c60..c99b229 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -46,6 +46,7 @@ static int smbus_do_alert(struct device *dev, void *addrp)
{
struct i2c_client *client = i2c_verify_client(dev);
struct alert_data *data = addrp;
+ struct i2c_driver *driver;

if (!client || client->addr != data->addr)
return 0;
@@ -54,12 +55,13 @@ static int smbus_do_alert(struct device *dev, void *addrp)

/*
* Drivers should either disable alerts, or provide at least
- * a minimal handler. Lock so client->driver won't change.
+ * a minimal handler. Lock so the driver won't change.
*/
device_lock(dev);
- if (client->driver) {
- if (client->driver->alert)
- client->driver->alert(client, data->flag);
+ if (client->dev.driver) {
+ driver = to_i2c_driver(client->dev.driver);
+ if (driver->alert)
+ driver->alert(client, data->flag);
else
dev_warn(&client->dev, "no driver alert()!\n");
} else
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 2ab11dc..eff50e0 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -205,7 +205,6 @@ struct i2c_driver {
* @name: Indicates the type of the device, usually a chip name that's
* generic enough to hide second-sourcing and compatible revisions.
* @adapter: manages the bus segment hosting this I2C device
- * @driver: device's driver, hence pointer to access routines
* @dev: Driver model device node for the slave.
* @irq: indicates the IRQ generated by this device (if any)
* @detected: member of an i2c_driver.clients list or i2c-core's
@@ -222,7 +221,6 @@ struct i2c_client {
/* _LOWER_ 7 bits */
char name[I2C_NAME_SIZE];
struct i2c_adapter *adapter; /* the adapter we sit on */
- struct i2c_driver *driver; /* and our access routines */
struct device dev; /* the device structure */
int irq; /* irq issued by device */
struct list_head detected;
--
1.8.0

2013-09-29 08:50:43

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 7/8] ASoC: imx-wm8962: Don't use i2c_client->driver

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. Check i2c_client->dev.driver instead to see if a driver is bound to the
device.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
sound/soc/fsl/imx-wm8962.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
index 6c60666..f84ecbf 100644
--- a/sound/soc/fsl/imx-wm8962.c
+++ b/sound/soc/fsl/imx-wm8962.c
@@ -215,7 +215,7 @@ static int imx_wm8962_probe(struct platform_device *pdev)
goto fail;
}
codec_dev = of_find_i2c_device_by_node(codec_np);
- if (!codec_dev || !codec_dev->driver) {
+ if (!codec_dev || !codec_dev->dev.driver) {
dev_err(&pdev->dev, "failed to find codec platform device\n");
ret = -EINVAL;
goto fail;
--
1.8.0

2013-09-29 08:51:20

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 3/8] [media] core: Don't use i2c_client->driver

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client->driver.driver->field' and
'client->dev.driver->field' are identical, so replace all occurrences of the
former with the later.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/media/v4l2-core/tuner-core.c | 6 +++---
drivers/media/v4l2-core/v4l2-common.c | 10 +++++-----
include/media/v4l2-common.h | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
index ddc9379..4133af0 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -43,7 +43,7 @@

#define UNSET (-1U)

-#define PREFIX (t->i2c->driver->driver.name)
+#define PREFIX (t->i2c->dev.driver->name)

/*
* Driver modprobe parameters
@@ -452,7 +452,7 @@ static void set_type(struct i2c_client *c, unsigned int type,
}

tuner_dbg("%s %s I2C addr 0x%02x with type %d used for 0x%02x\n",
- c->adapter->name, c->driver->driver.name, c->addr << 1, type,
+ c->adapter->name, c->dev.driver->name, c->addr << 1, type,
t->mode_mask);
return;

@@ -556,7 +556,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
int mode_mask;

if (pos->i2c->adapter != adap ||
- strcmp(pos->i2c->driver->driver.name, "tuner"))
+ strcmp(pos->i2c->dev.driver->name, "tuner"))
continue;

mode_mask = pos->mode_mask;
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 037d7a5..433d6d7 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -236,14 +236,14 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
v4l2_subdev_init(sd, ops);
sd->flags |= V4L2_SUBDEV_FL_IS_I2C;
/* the owner is the same as the i2c_client's driver owner */
- sd->owner = client->driver->driver.owner;
+ sd->owner = client->dev.driver->owner;
sd->dev = &client->dev;
/* i2c_client and v4l2_subdev point to one another */
v4l2_set_subdevdata(sd, client);
i2c_set_clientdata(client, sd);
/* initialize name */
snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
- client->driver->driver.name, i2c_adapter_id(client->adapter),
+ client->dev.driver->name, i2c_adapter_id(client->adapter),
client->addr);
}
EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
@@ -274,11 +274,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
loaded. This delay-load mechanism doesn't work if other drivers
want to use the i2c device, so explicitly loading the module
is the best alternative. */
- if (client == NULL || client->driver == NULL)
+ if (client == NULL || client->dev.driver == NULL)
goto error;

/* Lock the module so we can safely get the v4l2_subdev pointer */
- if (!try_module_get(client->driver->driver.owner))
+ if (!try_module_get(client->dev.driver->owner))
goto error;
sd = i2c_get_clientdata(client);

@@ -287,7 +287,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
if (v4l2_device_register_subdev(v4l2_dev, sd))
sd = NULL;
/* Decrease the module use count to match the first try_module_get. */
- module_put(client->driver->driver.owner);
+ module_put(client->dev.driver->owner);

error:
/* If we have a client but no subdev, then something went wrong and
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 16550c4..a707529 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -35,7 +35,7 @@
printk(level "%s %d-%04x: " fmt, name, i2c_adapter_id(adapter), addr , ## arg)

#define v4l_client_printk(level, client, fmt, arg...) \
- v4l_printk(level, (client)->driver->driver.name, (client)->adapter, \
+ v4l_printk(level, (client)->dev.driver->name, (client)->adapter, \
(client)->addr, fmt , ## arg)

#define v4l_err(client, fmt, arg...) \
--
1.8.0

2013-09-29 08:51:39

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 1/8] [media] s5c73m3: Don't use i2c_client->driver

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client->driver.driver->field' and
'client->dev.driver->field' are identical, so replace all occurrences of the
former with the later.

Signed-off-by: Lars-Peter Clausen <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Andrzej Hajda <[email protected]>
---
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index b76ec0e..1083890 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1581,7 +1581,7 @@ static int s5c73m3_probe(struct i2c_client *client,
oif_sd = &state->oif_sd;

v4l2_subdev_init(sd, &s5c73m3_subdev_ops);
- sd->owner = client->driver->driver.owner;
+ sd->owner = client->dev.driver->owner;
v4l2_set_subdevdata(sd, state);
strlcpy(sd->name, "S5C73M3", sizeof(sd->name));

--
1.8.0

2013-09-29 12:23:30

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 7/8] ASoC: imx-wm8962: Don't use i2c_client->driver

On Sun, Sep 29, 2013 at 10:51:05AM +0200, Lars-Peter Clausen wrote:
> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. Check i2c_client->dev.driver instead to see if a driver is bound to the
> device.

Acked-by: Mark Brown <[email protected]>


Attachments:
(No filename) (282.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-09-29 17:41:47

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 0/8] i2c: Remove redundant driver field from the i2c_client struct

On Sun, Sep 29, 2013 at 10:50:58AM +0200, Lars-Peter Clausen wrote:

> This series removes the redundant driver field from the i2c_client struct. The
> field is redundant since the same pointer can be accessed through
> to_i2c_driver(client->dev.driver). The commit log suggests that the field has
> been around since forever (since before v2.6.12-rc2) and it looks as if it was
> simply forgotten to remove it during the conversion of the i2c framework to the
> generic device driver model.

Great! Looks proper from a first glimpse. I'd think it makes sense to
take all patches via I2C. So, I am looking for ACKs for other subsystems
touched.

Thanks,

Wolfram


Attachments:
(No filename) (667.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-09-29 17:45:49

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH 1/8] [media] s5c73m3: Don't use i2c_client->driver

On 09/29/2013 10:50 AM, Lars-Peter Clausen wrote:
> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
>
> Signed-off-by: Lars-Peter Clausen<[email protected]>
> Cc: Kyungmin Park<[email protected]>
> Cc: Andrzej Hajda<[email protected]>

Acked-by: Sylwester Nawrocki <[email protected]>

2013-09-29 17:46:16

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH 2/8] [media] exynos4-is: Don't use i2c_client->driver

On 09/29/2013 10:51 AM, Lars-Peter Clausen wrote:
> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
>
> Signed-off-by: Lars-Peter Clausen<[email protected]>
> Cc: Kyungmin Park<[email protected]>
> Cc: Sylwester Nawrocki<[email protected]>

Acked-by: Sylwester Nawrocki <[email protected]>

2013-09-30 09:19:41

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 6/8] ALSA: ppc: keywest: Don't use i2c_client->driver

At Sun, 29 Sep 2013 10:51:04 +0200,
Lars-Peter Clausen wrote:
>
> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. Use 'to_i2c_driver(client->dev.driver)' instead to get direct
> access to the i2c_driver struct.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>

Acked-by: Takashi Iwai <[email protected]>


thanks,

Takashi

> ---
> sound/ppc/keywest.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
> index 01aecc2..0d1c27e 100644
> --- a/sound/ppc/keywest.c
> +++ b/sound/ppc/keywest.c
> @@ -65,7 +65,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
> * already bound. If not it means binding failed, and then there
> * is no point in keeping the device instantiated.
> */
> - if (!keywest_ctx->client->driver) {
> + if (!keywest_ctx->client->dev.driver) {
> i2c_unregister_device(keywest_ctx->client);
> keywest_ctx->client = NULL;
> return -ENODEV;
> @@ -76,7 +76,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
> * This is safe because i2c-core holds the core_lock mutex for us.
> */
> list_add_tail(&keywest_ctx->client->detected,
> - &keywest_ctx->client->driver->clients);
> + &to_i2c_driver(keywest_ctx->client->dev.driver)->clients);
> return 0;
> }
>
> --
> 1.8.0
>

2013-09-30 12:50:43

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 3/8] [media] core: Don't use i2c_client->driver

On 09/29/2013 10:51 AM, Lars-Peter Clausen wrote:
> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>

Acked-by: Hans Verkuil <[email protected]>

Regards,

Hans

> ---
> drivers/media/v4l2-core/tuner-core.c | 6 +++---
> drivers/media/v4l2-core/v4l2-common.c | 10 +++++-----
> include/media/v4l2-common.h | 2 +-
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> index ddc9379..4133af0 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -43,7 +43,7 @@
>
> #define UNSET (-1U)
>
> -#define PREFIX (t->i2c->driver->driver.name)
> +#define PREFIX (t->i2c->dev.driver->name)
>
> /*
> * Driver modprobe parameters
> @@ -452,7 +452,7 @@ static void set_type(struct i2c_client *c, unsigned int type,
> }
>
> tuner_dbg("%s %s I2C addr 0x%02x with type %d used for 0x%02x\n",
> - c->adapter->name, c->driver->driver.name, c->addr << 1, type,
> + c->adapter->name, c->dev.driver->name, c->addr << 1, type,
> t->mode_mask);
> return;
>
> @@ -556,7 +556,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
> int mode_mask;
>
> if (pos->i2c->adapter != adap ||
> - strcmp(pos->i2c->driver->driver.name, "tuner"))
> + strcmp(pos->i2c->dev.driver->name, "tuner"))
> continue;
>
> mode_mask = pos->mode_mask;
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 037d7a5..433d6d7 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -236,14 +236,14 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> v4l2_subdev_init(sd, ops);
> sd->flags |= V4L2_SUBDEV_FL_IS_I2C;
> /* the owner is the same as the i2c_client's driver owner */
> - sd->owner = client->driver->driver.owner;
> + sd->owner = client->dev.driver->owner;
> sd->dev = &client->dev;
> /* i2c_client and v4l2_subdev point to one another */
> v4l2_set_subdevdata(sd, client);
> i2c_set_clientdata(client, sd);
> /* initialize name */
> snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> - client->driver->driver.name, i2c_adapter_id(client->adapter),
> + client->dev.driver->name, i2c_adapter_id(client->adapter),
> client->addr);
> }
> EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
> @@ -274,11 +274,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
> loaded. This delay-load mechanism doesn't work if other drivers
> want to use the i2c device, so explicitly loading the module
> is the best alternative. */
> - if (client == NULL || client->driver == NULL)
> + if (client == NULL || client->dev.driver == NULL)
> goto error;
>
> /* Lock the module so we can safely get the v4l2_subdev pointer */
> - if (!try_module_get(client->driver->driver.owner))
> + if (!try_module_get(client->dev.driver->owner))
> goto error;
> sd = i2c_get_clientdata(client);
>
> @@ -287,7 +287,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
> if (v4l2_device_register_subdev(v4l2_dev, sd))
> sd = NULL;
> /* Decrease the module use count to match the first try_module_get. */
> - module_put(client->driver->driver.owner);
> + module_put(client->dev.driver->owner);
>
> error:
> /* If we have a client but no subdev, then something went wrong and
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 16550c4..a707529 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -35,7 +35,7 @@
> printk(level "%s %d-%04x: " fmt, name, i2c_adapter_id(adapter), addr , ## arg)
>
> #define v4l_client_printk(level, client, fmt, arg...) \
> - v4l_printk(level, (client)->driver->driver.name, (client)->adapter, \
> + v4l_printk(level, (client)->dev.driver->name, (client)->adapter, \
> (client)->addr, fmt , ## arg)
>
> #define v4l_err(client, fmt, arg...) \
>

2013-10-03 20:38:14

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 0/8] i2c: Remove redundant driver field from the i2c_client struct

On Sun, Sep 29, 2013 at 10:50:58AM +0200, Lars-Peter Clausen wrote:
> Hi,
>
> This series removes the redundant driver field from the i2c_client struct. The
> field is redundant since the same pointer can be accessed through
> to_i2c_driver(client->dev.driver). The commit log suggests that the field has
> been around since forever (since before v2.6.12-rc2) and it looks as if it was
> simply forgotten to remove it during the conversion of the i2c framework to the
> generic device driver model.

Applied to for-next with great pleasure, thanks!


Attachments:
(No filename) (551.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-10-17 17:45:56

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 3/8] [media] core: Don't use i2c_client->driver

Em Sun, 29 Sep 2013 10:51:01 +0200
Lars-Peter Clausen <[email protected]> escreveu:

> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>

Acked-by: Mauro Carvalho Chehab <[email protected]>

> ---
> drivers/media/v4l2-core/tuner-core.c | 6 +++---
> drivers/media/v4l2-core/v4l2-common.c | 10 +++++-----
> include/media/v4l2-common.h | 2 +-
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> index ddc9379..4133af0 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -43,7 +43,7 @@
>
> #define UNSET (-1U)
>
> -#define PREFIX (t->i2c->driver->driver.name)
> +#define PREFIX (t->i2c->dev.driver->name)
>
> /*
> * Driver modprobe parameters
> @@ -452,7 +452,7 @@ static void set_type(struct i2c_client *c, unsigned int type,
> }
>
> tuner_dbg("%s %s I2C addr 0x%02x with type %d used for 0x%02x\n",
> - c->adapter->name, c->driver->driver.name, c->addr << 1, type,
> + c->adapter->name, c->dev.driver->name, c->addr << 1, type,
> t->mode_mask);
> return;
>
> @@ -556,7 +556,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
> int mode_mask;
>
> if (pos->i2c->adapter != adap ||
> - strcmp(pos->i2c->driver->driver.name, "tuner"))
> + strcmp(pos->i2c->dev.driver->name, "tuner"))
> continue;
>
> mode_mask = pos->mode_mask;
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 037d7a5..433d6d7 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -236,14 +236,14 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> v4l2_subdev_init(sd, ops);
> sd->flags |= V4L2_SUBDEV_FL_IS_I2C;
> /* the owner is the same as the i2c_client's driver owner */
> - sd->owner = client->driver->driver.owner;
> + sd->owner = client->dev.driver->owner;
> sd->dev = &client->dev;
> /* i2c_client and v4l2_subdev point to one another */
> v4l2_set_subdevdata(sd, client);
> i2c_set_clientdata(client, sd);
> /* initialize name */
> snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> - client->driver->driver.name, i2c_adapter_id(client->adapter),
> + client->dev.driver->name, i2c_adapter_id(client->adapter),
> client->addr);
> }
> EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
> @@ -274,11 +274,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
> loaded. This delay-load mechanism doesn't work if other drivers
> want to use the i2c device, so explicitly loading the module
> is the best alternative. */
> - if (client == NULL || client->driver == NULL)
> + if (client == NULL || client->dev.driver == NULL)
> goto error;
>
> /* Lock the module so we can safely get the v4l2_subdev pointer */
> - if (!try_module_get(client->driver->driver.owner))
> + if (!try_module_get(client->dev.driver->owner))
> goto error;
> sd = i2c_get_clientdata(client);
>
> @@ -287,7 +287,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
> if (v4l2_device_register_subdev(v4l2_dev, sd))
> sd = NULL;
> /* Decrease the module use count to match the first try_module_get. */
> - module_put(client->driver->driver.owner);
> + module_put(client->dev.driver->owner);
>
> error:
> /* If we have a client but no subdev, then something went wrong and
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 16550c4..a707529 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -35,7 +35,7 @@
> printk(level "%s %d-%04x: " fmt, name, i2c_adapter_id(adapter), addr , ## arg)
>
> #define v4l_client_printk(level, client, fmt, arg...) \
> - v4l_printk(level, (client)->driver->driver.name, (client)->adapter, \
> + v4l_printk(level, (client)->dev.driver->name, (client)->adapter, \
> (client)->addr, fmt , ## arg)
>
> #define v4l_err(client, fmt, arg...) \


--

Cheers,
Mauro

2013-10-17 17:46:18

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 2/8] [media] exynos4-is: Don't use i2c_client->driver

Em Sun, 29 Sep 2013 10:51:00 +0200
Lars-Peter Clausen <[email protected]> escreveu:

> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> Cc: Kyungmin Park <[email protected]>
> Cc: Sylwester Nawrocki <[email protected]>

Acked-by: Mauro Carvalho Chehab <[email protected]>

> ---
> drivers/media/platform/exynos4-is/media-dev.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index a835112..7a4ee4c 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -411,8 +411,8 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,
>
> device_lock(&client->dev);
>
> - if (!client->driver ||
> - !try_module_get(client->driver->driver.owner)) {
> + if (!client->dev.driver ||
> + !try_module_get(client->dev.driver->owner)) {
> ret = -EPROBE_DEFER;
> v4l2_info(&fmd->v4l2_dev, "No driver found for %s\n",
> node->full_name);
> @@ -442,7 +442,7 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,
> fmd->num_sensors++;
>
> mod_put:
> - module_put(client->driver->driver.owner);
> + module_put(client->dev.driver->owner);
> dev_put:
> device_unlock(&client->dev);
> put_device(&client->dev);


--

Cheers,
Mauro