Hello Alan,
On Tuesday, July 16, 2013 5:17 PM Alan Stern wrote:
> On Tue, 16 Jul 2013, Andrzej Pietrasiewicz wrote:
>
> > When configfs is in place, the things related to intialization of
> > struct fsg_common will be split over a number of places.
> > This patch adds several functions which together cover the former
> > intialization routine fsg_common_init. As a consequence, issuing of
> > some debug messages needs to be adjusted.
>
> > --- a/drivers/usb/gadget/storage_common.c
> > +++ b/drivers/usb/gadget/storage_common.c
> > @@ -179,7 +179,7 @@ EXPORT_SYMBOL(fsg_ss_function); void
> > fsg_lun_close(struct fsg_lun *curlun) {
> > if (curlun->filp) {
> > - LDBG(curlun, "close backing file\n");
> > + pr_debug("close backing file\n");
>
> Here and in lots of other places, you have replaced a log message that
> includes the LUN's name and driver with a message that includes
> neither. If somebody reads the system log after this change, how are
> they going to know _which_ LUN had its backing file closed? That
> information should be included in the message.
>
I should have been more explicit in the commit log.
My main concern with the LDBG and its friends is that they need
to have a device to operate on.
So far this hasn't been an issue, because the luns have been
represented in sysfs. By the way, using sysfs for this kind
of things could be considered "abuse". In a configfs-based
gadget the luns are represented in configfs instead,
so there is no device to use for dev_dbg and similar;
the devices are used in the legacy g_mass_storage.ko, though.
As far as I can tell the only reason each lun contains a struct device
is for it to appear in sysfs, and the only reason for it to appear
in sysfs is to have a file-like attribute for the user to read from/
write to. Now that configfs is taking over this responsibility,
there is no need for struct device. This is what I think,
I guess that Michal's explanation would be authoritative.
@Michal: Could you throw some light on the subject?
Meanwhile I kept the struct device in struct fsg_lun
for the sake of Compatibility and it should be kept until
the g_xxxx.ko modules are dropped altogether
(which might be a long time).
> Why not just change the definitions of LDBG, LINFO, and so on instead
> of changing all the places where they get called?
>
If lun's device is to be omitted, there is no point in passing luns
to the said macros, which means changing each macro invocation anyway.
However, if a lun were useful in the macro your suggestion is
absolutely right.
Thanks,
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
