On Fri, Jun 21, 2013 at 11:19 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 21 Jun 2013, Konstantin Filatov wrote:
>
>> > --- a/drivers/usb/host/uhci-q.c
>> > +++ b/drivers/usb/host/uhci-q.c
>> > @@ -977,6 +977,9 @@ static int uhci_submit_common(struct uhci_hcd *uhci, struct urb *urb,
>> > for (;;) { /* Allow zero length packets */
>> > int pktsze = maxsze;
>> >
>> > + if (this_sg_len < pktsze) /* The short packet */
>> > + pktsze = this_sg_len;
>> > This will build a short packet transfer descriptor which is not the last
>> > one, and it isn't correct.
>> Why do you think so? Could you please refer to the specification of UHCI.
>
> It _isn't_ correct. But the fault lies in the driver that submitted
> the SG transfer in the first place, not in uhci-hcd. This is the right
> thing to do.
>
> Also, its the same as what ehci-hcd does. Oddly enough, it looks like
> nobody ever added SG support to ohci-hcd.
>
> By the way, the patch could be improved if it removed the line
>
> pktsze = len;
>
> This won't be needed, because the code is careful to guarantee that
> this_sg_len <= len always.
>
>> Your patch skips a chunk of data to transfer. This is a corruption of
>> data. It's unacceptable. We can return an error code if we treat the
>> request as malformed, but we can't transfer data selectively.
>
> I agree. Fix up the patch description as Sergei suggested, and you can
> add
>
> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
But the patch violates USB spec, doesn't it?
Also it sets the later packet size of TD as the small one, is it correct?
Thanks,
--
Ming Lei
--
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
