lkml.org 
[lkml]   [2010]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] USB: g_serial: Allow to override the default VID/PID
    On Mon, Nov 22, 2010 at 11:20:55AM +0100, Robert Lukassen wrote:
    > Hi,
    >
    > I just revisited the patch I submitted and I can see that it needs
    > fixing. The intention of the patch was to provide the idVendor &
    > idProduct kernel module parameter overrides to the individual
    > functions at 'bind()' time. Prior to the patch, the overrides were
    > done after the bind, making it hard for individual functions to
    > determine the correct value for idVendor (some functions may need
    > to use that value, e.g. USB audio when it registers itself
    > as an ALSA device; it would want to use the same Vendor id).
    >
    > The patch makes sure that that happens, however, right after the bind()
    > the patched values are lost due to the copying of the gadget specific
    > device descriptor. Here's the snippet from 2.6.34:
    >
    > 999 usb_gadget_set_selfpowered(gadget);
    > 1000
    >
    > 1001 /* interface and string IDs start at zero via kzalloc.
    > 1002 * we force endpoints to start unassigned; few controller
    > 1003 * drivers will zero ep->driver_data.
    > 1004 */
    > 1005 usb_ep_autoconfig_reset(cdev->gadget);
    > 1006
    > 1007 /* composite gadget needs to assign strings for whole device (like
    > 1008 * serial number), register function drivers, potentially update
    > 1009 * power state and consumption, etc
    > 1010 */
    > 1011 status = composite->bind(cdev);
    > 1012 if (status < 0)
    > 1013 goto fail;
    > 1014
    > 1015 cdev->desc = *composite->dev;
    > 1016 cdev->desc.bMaxPacketSize0 = gadget->ep0->maxpacket;
    > 1017
    > 1018 /* standardized runtime overrides for device ID data */
    > 1019 if (idVendor)
    > 1020 cdev->desc.idVendor = cpu_to_le16(idVendor);
    > 1021 if (idProduct)
    > 1022 cdev->desc.idProduct = cpu_to_le16(idProduct);
    > 1023 if (bcdDevice)
    > 1024 cdev->desc.bcdDevice = cpu_to_le16(bcdDevice);
    > 1025
    >
    > In line 1015, the composite device descriptor is copied to the cdev->desc
    > field. Then the values get patched.
    >
    > My patch just moved lines 1018-1024, so that the functions could access
    > the patched values. The ill side effect is that these patched values
    > are lost when the *composite->dev (== device descriptor) is copied
    > in line 1015.
    >
    > It would seem that the patch should also have moved lines 1015-1016 in front
    > of the composite->bind(cdev) call.
    >
    > It would require all composite gadgets (such as serial.c) and all functions
    > (such as f_acm.c) to do any modifications to the device descriptor as passed
    > to them in the 'bind' call. For functions that is natural, as they should be
    > unaware of the gadget that uses them.
    >
    > The composite gadgets such as 'serial' seem to deal with it differently.
    > The serial.c gadget refers to the device descriptor as 'device_desc', and it
    > is a static struct. It references it in its 'init' function (which is ok, it is
    > prior to the registration point) and in the gs_bind() call. At that point it should
    > probably not be using 'device_desc', but cdev->desc.
    >
    > I think we can go one of two ways. I'd be happy to go over the composite gadgets and
    > change their use of their device descriptors, such that they use cdev->desc at
    > bind() time. Combining that with the moving the copying of the *composite->dev
    > to before calling 'composite->bind()' would make things consistent.
    >
    > The other way is reverting my original patch. For those functions that need access
    > to the overridden idVendor/idProduct code, we'll need to work out another
    > (generic) solution.

    So, do you want me to revert your original patch and wait for you to
    come up with a "better" solution?

    That seems like the correct thing to do at the moment.

    thanks,

    greg k-h


    \
     
     \ /
      Last update: 2010-11-30 18:51    [W:0.026 / U:92.836 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site