于 2012/7/24 22:59, Alan Stern 写道:
On Tue, 24 Jul 2012, Lan Tianyu wrote:

于 2012/7/24 22:21, Alan Stern 写道:
On Tue, 24 Jul 2012, Lan Tianyu wrote:
@@ -209,10 +209,13 @@ set_avoid_reset_quirk(struct device *dev, struct 
device_attribute *attr,
        if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)

Please do me a favor: Rename the "config" variable to something else,
such as "morphs" or "val".  In USB, the word "config" already has a
separate meaning.
Ok. I will do it. A seperate patch?

Up to you; I don't care.


                return -EINVAL;
        usb_lock_device(udev);
-       if (config)
+       if (config) {
                udev->quirks |= USB_QUIRK_RESET_MORPHS;
-       else
+               udev->persist_enabled = 0;
+       } else {
                udev->quirks &= ~USB_QUIRK_RESET_MORPHS;
+               udev->persist_enabled = 1;

This line doesn't seem right.  What if the user _wants_ persist_enabled
to be 0?
This seems attribute avoid_reset_quirk is conflicted with persist.
  From my opinion, avoid_reset_quirk may have high priority.
So we can add a USB_QUIRK_RESET_MORPHS check in the attribute persist
store callback, if USB_QUIRK_RESET_MORPHS flag had been set, persist
couldn't be changed and remain 0 since this patch set persist to 0 when
USB_QUIRK_RESET_MORPHS flag is set. Do this make sense?

Yes, good idea.

When you do this, you should also add another little paragraph to
Documentation/usb/persist.txt.  At the end of the "What is the
solution?" section, explain why the persist attribute cannot be set if
the avoid_reset_quirk attribute is on.

Ok. I get it.

Hmmm, what about hubs?  They don't have a persist attribute because
persist is _always_ supposed to be enabled for a hub.  Maybe this means
they shouldn't have an avoid_reset_quirk attribute either.
Oh. It;s tough since the avoid_reset_quirk attribute is added when device
is created. How about remove it in the hub probe()?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to