| Message ID | 20090430173554.4cb2f585@hyperion.delvare (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Thu, 30 Apr 2009, Jean Delvare wrote: > The i2c core used to maintain a list of client for each adapter. This > is a duplication of what the driver core already does, so this list > will be removed as part of a future cleanup. Anyone using this list > must stop doing so. > > For pvrusb2, I propose the following change, which should lead to an > equally informative output. The only difference is that i2c clients > which are not a v4l2 subdev won't show up, but I guess this case is > not supposed to happen anyway. It will happen for anything i2c used by v4l which itself is not really a part of v4l. That would include, uh, lirc. I will review and test this first chance I get which should be tomorrow. -Mike > > Signed-off-by: Jean Delvare <khali@linux-fr.org> > Cc: Mike Isely <isely@pobox.com> > --- > Mike, can you please review and test this patch? Thanks. > > linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c | 56 +++++------------------ > 1 file changed, 13 insertions(+), 43 deletions(-) > > --- v4l-dvb.orig/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c 2009-04-30 16:52:32.000000000 +0200 > +++ v4l-dvb/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c 2009-04-30 17:20:37.000000000 +0200 > @@ -4920,65 +4920,35 @@ static unsigned int pvr2_hdw_report_clie > unsigned int tcnt = 0; > unsigned int ccnt; > struct i2c_client *client; > - struct list_head *item; > - void *cd; > const char *p; > unsigned int id; > > - ccnt = scnprintf(buf, acnt, "Associated v4l2-subdev drivers:"); > + ccnt = scnprintf(buf, acnt, "Associated v4l2-subdev drivers and I2C clients:\n"); > tcnt += ccnt; > v4l2_device_for_each_subdev(sd, &hdw->v4l2_dev) { > id = sd->grp_id; > p = NULL; > if (id < ARRAY_SIZE(module_names)) p = module_names[id]; > if (p) { > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, " %s", p); > + ccnt = scnprintf(buf + tcnt, acnt - tcnt, " %s:", p); > tcnt += ccnt; > } else { > ccnt = scnprintf(buf + tcnt, acnt - tcnt, > - " (unknown id=%u)", id); > + " (unknown id=%u):", id); > tcnt += ccnt; > } > - } > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "\n"); > - tcnt += ccnt; > - > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "I2C clients:\n"); > - tcnt += ccnt; > - > - mutex_lock(&hdw->i2c_adap.clist_lock); > - list_for_each(item, &hdw->i2c_adap.clients) { > - client = list_entry(item, struct i2c_client, list); > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, > - " %s: i2c=%02x", client->name, client->addr); > - tcnt += ccnt; > - cd = i2c_get_clientdata(client); > - v4l2_device_for_each_subdev(sd, &hdw->v4l2_dev) { > - if (cd == sd) { > - id = sd->grp_id; > - p = NULL; > - if (id < ARRAY_SIZE(module_names)) { > - p = module_names[id]; > - } > - if (p) { > - ccnt = scnprintf(buf + tcnt, > - acnt - tcnt, > - " subdev=%s", p); > - tcnt += ccnt; > - } else { > - ccnt = scnprintf(buf + tcnt, > - acnt - tcnt, > - " subdev= id %u)", > - id); > - tcnt += ccnt; > - } > - break; > - } > + client = v4l2_get_subdevdata(sd); > + if (client) { > + ccnt = scnprintf(buf + tcnt, acnt - tcnt, > + " %s @ %02x\n", client->name, > + client->addr); > + tcnt += ccnt; > + } else { > + ccnt = scnprintf(buf + tcnt, acnt - tcnt, > + " no i2c client\n"); > + tcnt += ccnt; > } > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "\n"); > - tcnt += ccnt; > } > - mutex_unlock(&hdw->i2c_adap.clist_lock); > return tcnt; > } > > > >
On Thu, 30 Apr 2009, Mike Isely wrote: > On Thu, 30 Apr 2009, Jean Delvare wrote: > > > The i2c core used to maintain a list of client for each adapter. This > > is a duplication of what the driver core already does, so this list > > will be removed as part of a future cleanup. Anyone using this list > > must stop doing so. > > > > For pvrusb2, I propose the following change, which should lead to an > > equally informative output. The only difference is that i2c clients > > which are not a v4l2 subdev won't show up, but I guess this case is > > not supposed to happen anyway. > > It will happen for anything i2c used by v4l which itself is not really a > part of v4l. That would include, uh, lirc. > > I will review and test this first chance I get which should be tomorrow. > I've merged and tested this patch. It behaves as expected. I'm putting together a bunch of pvrusb2 changesets right now anyway. I've pulled this one into the collection, with appropriate attributions of course. -Mike > > > > > > Signed-off-by: Jean Delvare <khali@linux-fr.org> > > Cc: Mike Isely <isely@pobox.com> > > --- > > Mike, can you please review and test this patch? Thanks. > > > > linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c | 56 +++++------------------ > > 1 file changed, 13 insertions(+), 43 deletions(-) > > > > --- v4l-dvb.orig/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c 2009-04-30 16:52:32.000000000 +0200 > > +++ v4l-dvb/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c 2009-04-30 17:20:37.000000000 +0200 > > @@ -4920,65 +4920,35 @@ static unsigned int pvr2_hdw_report_clie > > unsigned int tcnt = 0; > > unsigned int ccnt; > > struct i2c_client *client; > > - struct list_head *item; > > - void *cd; > > const char *p; > > unsigned int id; > > > > - ccnt = scnprintf(buf, acnt, "Associated v4l2-subdev drivers:"); > > + ccnt = scnprintf(buf, acnt, "Associated v4l2-subdev drivers and I2C clients:\n"); > > tcnt += ccnt; > > v4l2_device_for_each_subdev(sd, &hdw->v4l2_dev) { > > id = sd->grp_id; > > p = NULL; > > if (id < ARRAY_SIZE(module_names)) p = module_names[id]; > > if (p) { > > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, " %s", p); > > + ccnt = scnprintf(buf + tcnt, acnt - tcnt, " %s:", p); > > tcnt += ccnt; > > } else { > > ccnt = scnprintf(buf + tcnt, acnt - tcnt, > > - " (unknown id=%u)", id); > > + " (unknown id=%u):", id); > > tcnt += ccnt; > > } > > - } > > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "\n"); > > - tcnt += ccnt; > > - > > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "I2C clients:\n"); > > - tcnt += ccnt; > > - > > - mutex_lock(&hdw->i2c_adap.clist_lock); > > - list_for_each(item, &hdw->i2c_adap.clients) { > > - client = list_entry(item, struct i2c_client, list); > > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, > > - " %s: i2c=%02x", client->name, client->addr); > > - tcnt += ccnt; > > - cd = i2c_get_clientdata(client); > > - v4l2_device_for_each_subdev(sd, &hdw->v4l2_dev) { > > - if (cd == sd) { > > - id = sd->grp_id; > > - p = NULL; > > - if (id < ARRAY_SIZE(module_names)) { > > - p = module_names[id]; > > - } > > - if (p) { > > - ccnt = scnprintf(buf + tcnt, > > - acnt - tcnt, > > - " subdev=%s", p); > > - tcnt += ccnt; > > - } else { > > - ccnt = scnprintf(buf + tcnt, > > - acnt - tcnt, > > - " subdev= id %u)", > > - id); > > - tcnt += ccnt; > > - } > > - break; > > - } > > + client = v4l2_get_subdevdata(sd); > > + if (client) { > > + ccnt = scnprintf(buf + tcnt, acnt - tcnt, > > + " %s @ %02x\n", client->name, > > + client->addr); > > + tcnt += ccnt; > > + } else { > > + ccnt = scnprintf(buf + tcnt, acnt - tcnt, > > + " no i2c client\n"); > > + tcnt += ccnt; > > } > > - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "\n"); > > - tcnt += ccnt; > > } > > - mutex_unlock(&hdw->i2c_adap.clist_lock); > > return tcnt; > > } > > > > > > > > > >
On Fri, 1 May 2009 22:25:28 -0500 (CDT), Mike Isely wrote: > On Thu, 30 Apr 2009, Mike Isely wrote: > > > On Thu, 30 Apr 2009, Jean Delvare wrote: > > > > > The i2c core used to maintain a list of client for each adapter. This > > > is a duplication of what the driver core already does, so this list > > > will be removed as part of a future cleanup. Anyone using this list > > > must stop doing so. > > > > > > For pvrusb2, I propose the following change, which should lead to an > > > equally informative output. The only difference is that i2c clients > > > which are not a v4l2 subdev won't show up, but I guess this case is > > > not supposed to happen anyway. > > > > It will happen for anything i2c used by v4l which itself is not really a > > part of v4l. That would include, uh, lirc. > > > > I will review and test this first chance I get which should be tomorrow. > > > > I've merged and tested this patch. It behaves as expected. > > I'm putting together a bunch of pvrusb2 changesets right now anyway. > I've pulled this one into the collection, with appropriate attributions > of course. Excellent, thank you!
--- v4l-dvb.orig/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c 2009-04-30 16:52:32.000000000 +0200 +++ v4l-dvb/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c 2009-04-30 17:20:37.000000000 +0200 @@ -4920,65 +4920,35 @@ static unsigned int pvr2_hdw_report_clie unsigned int tcnt = 0; unsigned int ccnt; struct i2c_client *client; - struct list_head *item; - void *cd; const char *p; unsigned int id; - ccnt = scnprintf(buf, acnt, "Associated v4l2-subdev drivers:"); + ccnt = scnprintf(buf, acnt, "Associated v4l2-subdev drivers and I2C clients:\n"); tcnt += ccnt; v4l2_device_for_each_subdev(sd, &hdw->v4l2_dev) { id = sd->grp_id; p = NULL; if (id < ARRAY_SIZE(module_names)) p = module_names[id]; if (p) { - ccnt = scnprintf(buf + tcnt, acnt - tcnt, " %s", p); + ccnt = scnprintf(buf + tcnt, acnt - tcnt, " %s:", p); tcnt += ccnt; } else { ccnt = scnprintf(buf + tcnt, acnt - tcnt, - " (unknown id=%u)", id); + " (unknown id=%u):", id); tcnt += ccnt; } - } - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "\n"); - tcnt += ccnt; - - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "I2C clients:\n"); - tcnt += ccnt; - - mutex_lock(&hdw->i2c_adap.clist_lock); - list_for_each(item, &hdw->i2c_adap.clients) { - client = list_entry(item, struct i2c_client, list); - ccnt = scnprintf(buf + tcnt, acnt - tcnt, - " %s: i2c=%02x", client->name, client->addr); - tcnt += ccnt; - cd = i2c_get_clientdata(client); - v4l2_device_for_each_subdev(sd, &hdw->v4l2_dev) { - if (cd == sd) { - id = sd->grp_id; - p = NULL; - if (id < ARRAY_SIZE(module_names)) { - p = module_names[id]; - } - if (p) { - ccnt = scnprintf(buf + tcnt, - acnt - tcnt, - " subdev=%s", p); - tcnt += ccnt; - } else { - ccnt = scnprintf(buf + tcnt, - acnt - tcnt, - " subdev= id %u)", - id); - tcnt += ccnt; - } - break; - } + client = v4l2_get_subdevdata(sd); + if (client) { + ccnt = scnprintf(buf + tcnt, acnt - tcnt, + " %s @ %02x\n", client->name, + client->addr); + tcnt += ccnt; + } else { + ccnt = scnprintf(buf + tcnt, acnt - tcnt, + " no i2c client\n"); + tcnt += ccnt; } - ccnt = scnprintf(buf + tcnt, acnt - tcnt, "\n"); - tcnt += ccnt; } - mutex_unlock(&hdw->i2c_adap.clist_lock); return tcnt; }
The i2c core used to maintain a list of client for each adapter. This is a duplication of what the driver core already does, so this list will be removed as part of a future cleanup. Anyone using this list must stop doing so. For pvrusb2, I propose the following change, which should lead to an equally informative output. The only difference is that i2c clients which are not a v4l2 subdev won't show up, but I guess this case is not supposed to happen anyway. Signed-off-by: Jean Delvare <khali@linux-fr.org> Cc: Mike Isely <isely@pobox.com> --- Mike, can you please review and test this patch? Thanks. linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c | 56 +++++------------------ 1 file changed, 13 insertions(+), 43 deletions(-)