proc_fops: convert drivers/isdn/ to seq_file
in [Kernel]
|
Prev: irq: Add node_affinity CPU masks for smarter irqbalance hints
Next: two new linux-next trees (SPI and devicetree)
From: Andrew Morton on 24 Nov 2009 19:30 On Mon, 23 Nov 2009 04:56:25 +0300 Alexey Dobriyan <adobriyan(a)gmail.com> wrote: > Signed-off-by: Alexey Dobriyan <adobriyan(a)gmail.com> > --- > > Documentation/isdn/INTERFACE.CAPI | 4 > drivers/isdn/capi/capi.c | 99 +++++----------- > drivers/isdn/capi/capidrv.c | 54 ++------ > drivers/isdn/capi/kcapi.c | 8 - > drivers/isdn/hardware/avm/avmcard.h | 6 > drivers/isdn/hardware/avm/b1.c | 54 +++++--- > drivers/isdn/hardware/avm/b1dma.c | 71 +++++------ > drivers/isdn/hardware/avm/b1isa.c | 2 > drivers/isdn/hardware/avm/b1pci.c | 4 > drivers/isdn/hardware/avm/b1pcmcia.c | 2 > drivers/isdn/hardware/avm/c4.c | 53 +++++--- > drivers/isdn/hardware/avm/t1isa.c | 2 > drivers/isdn/hardware/avm/t1pci.c | 2 > drivers/isdn/hardware/eicon/capimain.c | 40 +++--- > drivers/isdn/hardware/eicon/diva_didd.c | 44 +++---- > drivers/isdn/hardware/eicon/divasi.c | 48 ++++--- > drivers/isdn/hardware/eicon/divasproc.c | 198 ++++++++++++++------------------ > drivers/isdn/hysdn/hycapi.c | 54 ++++---- > include/linux/isdn/capilli.h | 3 > net/bluetooth/cmtp/capi.c | 37 +++-- > 20 files changed, 368 insertions(+), 417 deletions(-) linux-next collision: drivers/isdn/gigaset/capi.c: In function 'gigaset_isdn_register': drivers/isdn/gigaset/capi.c:2259: error: 'struct capi_ctr' has no member named 'ctr_read_proc' -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alexey Dobriyan on 25 Nov 2009 00:00 On Tue, Nov 24, 2009 at 04:25:41PM -0800, Andrew Morton wrote: > On Mon, 23 Nov 2009 04:56:25 +0300 > Alexey Dobriyan <adobriyan(a)gmail.com> wrote: > linux-next collision: > > drivers/isdn/gigaset/capi.c: In function 'gigaset_isdn_register': > drivers/isdn/gigaset/capi.c:2259: error: 'struct capi_ctr' has no member named 'ctr_read_proc' Please, fold into my patch: --- a/drivers/isdn/gigaset/capi.c +++ b/drivers/isdn/gigaset/capi.c @@ -2106,35 +2106,22 @@ static char *gigaset_procinfo(struct capi_ctr *ctr) return ctr->name; /* ToDo: more? */ } -/** - * gigaset_ctr_read_proc() - build controller proc file entry - * @page: buffer of PAGE_SIZE bytes for receiving the entry. - * @start: unused. - * @off: unused. - * @count: unused. - * @eof: unused. - * @ctr: controller descriptor structure. - * - * Return value: length of generated entry - */ -static int gigaset_ctr_read_proc(char *page, char **start, off_t off, - int count, int *eof, struct capi_ctr *ctr) +static int gigaset_proc_show(struct seq_file *m, void *v) { + struct capi_ctr *ctr = m->private; struct cardstate *cs = ctr->driverdata; char *s; int i; - int len = 0; - len += sprintf(page+len, "%-16s %s\n", "name", ctr->name); - len += sprintf(page+len, "%-16s %s %s\n", "dev", + + seq_printf(m, "%-16s %s\n", "name", ctr->name); + seq_printf(m, "%-16s %s %s\n", "dev", dev_driver_string(cs->dev), dev_name(cs->dev)); - len += sprintf(page+len, "%-16s %d\n", "id", cs->myid); + seq_printf(m, "%-16s %d\n", "id", cs->myid); if (cs->gotfwver) - len += sprintf(page+len, "%-16s %d.%d.%d.%d\n", "firmware", + seq_printf(m, "%-16s %d.%d.%d.%d\n", "firmware", cs->fwver[0], cs->fwver[1], cs->fwver[2], cs->fwver[3]); - len += sprintf(page+len, "%-16s %d\n", "channels", - cs->channels); - len += sprintf(page+len, "%-16s %s\n", "onechannel", - cs->onechannel ? "yes" : "no"); + seq_printf(m, "%-16s %d\n", "channels", cs->channels); + seq_printf(m, "%-16s %s\n", "onechannel", cs->onechannel ? "yes" : "no"); switch (cs->mode) { case M_UNKNOWN: @@ -2152,7 +2139,7 @@ static int gigaset_ctr_read_proc(char *page, char **start, off_t off, default: s = "??"; } - len += sprintf(page+len, "%-16s %s\n", "mode", s); + seq_printf(m, "%-16s %s\n", "mode", s); switch (cs->mstate) { case MS_UNINITIALIZED: @@ -2176,25 +2163,25 @@ static int gigaset_ctr_read_proc(char *page, char **start, off_t off, default: s = "??"; } - len += sprintf(page+len, "%-16s %s\n", "mstate", s); + seq_printf(m, "%-16s %s\n", "mstate", s); - len += sprintf(page+len, "%-16s %s\n", "running", + seq_printf(m, "%-16s %s\n", "running", cs->running ? "yes" : "no"); - len += sprintf(page+len, "%-16s %s\n", "connected", + seq_printf(m, "%-16s %s\n", "connected", cs->connected ? "yes" : "no"); - len += sprintf(page+len, "%-16s %s\n", "isdn_up", + seq_printf(m, "%-16s %s\n", "isdn_up", cs->isdn_up ? "yes" : "no"); - len += sprintf(page+len, "%-16s %s\n", "cidmode", + seq_printf(m, "%-16s %s\n", "cidmode", cs->cidmode ? "yes" : "no"); for (i = 0; i < cs->channels; i++) { - len += sprintf(page+len, "[%d]%-13s %d\n", i, "corrupted", + seq_printf(m, "[%d]%-13s %d\n", i, "corrupted", cs->bcs[i].corrupted); - len += sprintf(page+len, "[%d]%-13s %d\n", i, "trans_down", + seq_printf(m, "[%d]%-13s %d\n", i, "trans_down", cs->bcs[i].trans_down); - len += sprintf(page+len, "[%d]%-13s %d\n", i, "trans_up", + seq_printf(m, "[%d]%-13s %d\n", i, "trans_up", cs->bcs[i].trans_up); - len += sprintf(page+len, "[%d]%-13s %d\n", i, "chstate", + seq_printf(m, "[%d]%-13s %d\n", i, "chstate", cs->bcs[i].chstate); switch (cs->bcs[i].proto2) { case L2_BITSYNC: @@ -2209,11 +2196,24 @@ static int gigaset_ctr_read_proc(char *page, char **start, off_t off, default: s = "??"; } - len += sprintf(page+len, "[%d]%-13s %s\n", i, "proto2", s); + seq_printf(m, "[%d]%-13s %s\n", i, "proto2", s); } - return len; + return 0; } +static int gigaset_proc_open(struct inode *inode, struct file *file) +{ + return single_open(file, gigaset_proc_show, PDE(inode)->data); +} + +static const struct file_operations gigaset_proc_fops = { + .owner = THIS_MODULE, + .open = gigaset_proc_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + static struct capi_driver capi_driver_gigaset = { .name = "gigaset", @@ -2256,7 +2256,7 @@ int gigaset_isdn_register(struct cardstate *cs, const char *isdnid) iif->ctr.release_appl = gigaset_release_appl; iif->ctr.send_message = gigaset_send_message; iif->ctr.procinfo = gigaset_procinfo; - iif->ctr.ctr_read_proc = gigaset_ctr_read_proc; + iif->ctr.proc_fops = &gigaset_proc_fops; INIT_LIST_HEAD(&iif->appls); skb_queue_head_init(&iif->sendqueue); atomic_set(&iif->sendqlen, 0); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tilman Schmidt on 28 Nov 2009 07:30 Am 23.11.2009 02:56 schrieb Alexey Dobriyan: > --- a/Documentation/isdn/INTERFACE.CAPI > +++ b/Documentation/isdn/INTERFACE.CAPI > @@ -149,8 +149,8 @@ char *(*procinfo)(struct capi_ctr *ctrlr) > pointer to a callback function returning the entry for the device in > the CAPI controller info table, /proc/capi/controller > > -read_proc_t *ctr_read_proc > - pointer to the read_proc callback function for the device's proc file > +const struct file_operations *proc_fops > + pointers to callback functions for the device's proc file > system entry, /proc/capi/controllers/<n>; will be called with a > pointer to the device's capi_ctr structure as the last (data) argument > This doesn't look correct. AFACIS, most of the callback functions in proc_fops don't have a last argument named data or looking as if it might lend itself to passing a pointer to the device's capi_ctr structure. In fact, later in your patch you replace uses of that last argument by accesses to the m->private member of the struct seq_file *m argument, like here: > --- a/drivers/isdn/hardware/avm/b1.c > +++ b/drivers/isdn/hardware/avm/b1.c [...] > @@ -634,18 +636,17 @@ irqreturn_t b1_interrupt(int interrupt, void *devptr) > } > > /* ------------------------------------------------------------- */ > -int b1ctl_read_proc(char *page, char **start, off_t off, > - int count, int *eof, struct capi_ctr *ctrl) > +static int b1ctl_proc_show(struct seq_file *m, void *v) > { > + struct capi_ctr *ctrl = m->private; > avmctrl_info *cinfo = (avmctrl_info *)(ctrl->driverdata); > avmcard *card = cinfo->card; > u8 flag; [...] So I guess the paragraph Documentation/isdn/INTERFACE.CAPI needs to be adapted to describe correctly where the callbacks will find their controller data structure after that change. OTOH, the new proc_show functions sport a second argument void *v that doesn't appear to get used anywhere. It would be nice if that could be explained a bit somewhere. Thanks, Tilman -- Tilman Schmidt E-Mail: tilman(a)imap.cc Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
From: Tilman Schmidt on 29 Nov 2009 19:50 Am 25.11.2009 05:59 schrieb Alexey Dobriyan: > --- a/drivers/isdn/gigaset/capi.c > +++ b/drivers/isdn/gigaset/capi.c > @@ -2106,35 +2106,22 @@ static char *gigaset_procinfo(struct capi_ctr *ctr) > return ctr->name; /* ToDo: more? */ > } > > -/** > - * gigaset_ctr_read_proc() - build controller proc file entry > - * @page: buffer of PAGE_SIZE bytes for receiving the entry. > - * @start: unused. > - * @off: unused. > - * @count: unused. > - * @eof: unused. > - * @ctr: controller descriptor structure. > - * > - * Return value: length of generated entry > - */ > -static int gigaset_ctr_read_proc(char *page, char **start, off_t off, > - int count, int *eof, struct capi_ctr *ctr) > +static int gigaset_proc_show(struct seq_file *m, void *v) I would prefer that, instead of throwing the kerneldoc comment away, you adapted it to the new function. Specifically, I would like to know what the second argument "void *v" is for. > { > + struct capi_ctr *ctr = m->private; See below. > +static int gigaset_proc_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, gigaset_proc_show, PDE(inode)->data); I'd like to understand how that works. According to Documentation/filesystems/seq_file.txt, the value of the last argument will be passed to the proc_show function in the private field of the seq_file structure. Your gigaset_proc_show() function assumes that this will be the capi_ctr pointer for the device. So what is PDE(inode)->data and where does it get set to the capi_ctr pointer for the device? Thanks, Tilman -- Tilman Schmidt E-Mail: tilman(a)imap.cc Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
From: Alexey Dobriyan on 30 Nov 2009 15:20
On Mon, Nov 30, 2009 at 01:42:32AM +0100, Tilman Schmidt wrote: > Am 25.11.2009 05:59 schrieb Alexey Dobriyan: > > --- a/drivers/isdn/gigaset/capi.c > > +++ b/drivers/isdn/gigaset/capi.c > > @@ -2106,35 +2106,22 @@ static char *gigaset_procinfo(struct capi_ctr *ctr) > > return ctr->name; /* ToDo: more? */ > > } > > > > -/** > > - * gigaset_ctr_read_proc() - build controller proc file entry > > - * @page: buffer of PAGE_SIZE bytes for receiving the entry. > > - * @start: unused. > > - * @off: unused. > > - * @count: unused. > > - * @eof: unused. > > - * @ctr: controller descriptor structure. > > - * > > - * Return value: length of generated entry > > - */ > > -static int gigaset_ctr_read_proc(char *page, char **start, off_t off, > > - int count, int *eof, struct capi_ctr *ctr) > > +static int gigaset_proc_show(struct seq_file *m, void *v) > > I would prefer that, instead of throwing the kerneldoc comment away, > you adapted it to the new function. Specifically, I would like to > know what the second argument "void *v" is for. Sorry, no. I personally find the corellation between kernel-docness and useleness to be highly positive. Comment is removed because after removal of parameters "description", nothing is left. > > { > > + struct capi_ctr *ctr = m->private; > > See below. > > > +static int gigaset_proc_open(struct inode *inode, struct file *file) > > +{ > > + return single_open(file, gigaset_proc_show, PDE(inode)->data); > > I'd like to understand how that works. > > According to Documentation/filesystems/seq_file.txt, the value of > the last argument will be passed to the proc_show function in the > private field of the seq_file structure. Your gigaset_proc_show() > function assumes that this will be the capi_ctr pointer for the > device. > > So what is PDE(inode)->data and where does it get set to the > capi_ctr pointer for the device? When you create proc entry, you set ->data for proc entry, it's accessible as PDE(inode)->data. Now you need to pass it doen to actual hook. It got stashed eventually into struct seq_file::private (see single_open()) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |