Em Mon, 19 Aug 2024 13:14:19 -0400
Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> escreveu:
> On Mon, Aug 19, 2024 at 06:24:56PM +0200, Mauro Carvalho Chehab wrote:
> > Basically, the actual SMS device type is given by this enum:
> >
> > enum sms_device_type_st {
> > SMS_UNKNOWN_TYPE = -1,
> >
> > SMS_STELLAR = 0,
> > SMS_NOVA_A0,
> > SMS_NOVA_B0,
> > SMS_VEGA,
> > SMS_VENICE,
> > SMS_MING,
> > SMS_PELE,
> > SMS_RIO,
> > SMS_DENVER_1530,
> > SMS_DENVER_2160,
> >
> > SMS_NUM_OF_DEVICE_TYPES /* This is just a count */
> > };
> >
> > But I dunno if there are a 1:1 mapping between type and chipset
> > number. The above type names probably match some vendor internal
> > names, but we never had any tables associating them to a device number,
> > as the vendor never provided us such information.
> >
> > Btw I vaguely remember I heard about a newer Siano chipsets (sm3xxx),
> > but never saw such devices.
> >
> > -
> >
> > Now, I'm not sure about what endpoints this specific driver exports, as
> > I'm lacking vendor's documentation. What I said is that almost all DVB
> > devices have isoc endpoints, but I dunno if this is the case of Siano.
>
> Currently the driver exports only bulk endpoints, even though it doesn't
> check the endpoint type. You can tell because the only routine in it
> that calls usb_submit_urb() is smsusb_submit_urb(), and that routine
> calls usb_fill_bulk_urb() before doing the submission.
>
> Given this, I suggest merging the earlier patch submission from Nikita
> Zhandarovich as-is. If the driver ever evolves to include support for
> isochronous endpoints, the probe function can be modified then.
I'll see if I can try his patch and see if device keeps working. The
logic indeed use endpoints in bulk mode, but I'm not sure if, for all the
BIOS files at drivers/media/common/siano/smscoreapi.[ch], the endpoints
are properly reported as bulk.
What happens if an endpoint is reported as ISOC, but the URB submit
is called without URB_ISO_ASAP? On a quick check, the code at usb_submit_urb()
seems to not complain about that.
I would be a lot more comfortable if the patch were using just
if (usb_endpoint_dir_in(desc))
...
if (usb_endpoint_dir_out(desc))
...
or something like this (to accept both isoc and bulk):
if (!usb_endpoint_xfer_int(epd)) {
if (usb_endpoint_dir_in(desc))
...
if (usb_endpoint_dir_out(desc))
...
}
instead of calling usb_endpoint_xfer_bulk(desc) to check if type
is bulk.
I'll try to do some tests, but not sure when, as I'm traveling abroad
this week.
Thanks,
Mauro
