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.
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.
Alan Stern
--
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
