Em Mon, 19 Aug 2024 10:32:05 -0400 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> escreveu: > On Mon, Aug 19, 2024 at 10:15:11AM +0200, Mauro Carvalho Chehab wrote: > > This patch is duplicated of this one: > > > > https://patchwork.linuxtv.org/project/linux-media/patch/20240409143634.33230-1-n.zhandarovich@xxxxxxxxxx/ > > > > The part I didn't like with such approach is that it checks only for > > bulk endpoints. Most media devices have also isoc. Now, I'm not sure > > about Siano devices. There are 3 different major chipsets supported > > by this driver (sm1000, sm11xx, sm2xxx). Among them, sm1000 has one > > USB ID for cold boot, and, once firmware is loaded, it gains another > > USB ID for a a warm boot. > > Are you sure about all this? The one source file in > drivers/media/usb/siano refers only to bulk endpoints, and the files in > drivers/media/common/siano don't refer to endpoints or URBs at all. > Nothing in either directory refers to isochronous endpoints. Is there > some other place I should be looking? > Also, while there are many constants in those files whose names start > with SMS1, there aren't any whose names start with SMS2 or SM2 (or their > lowercase equivalents). And the Kconfig help text mentions only Siano > SMS1xxx. > > > > Your patch and the previously submitted one are not only checking > > for the direction, but it is also discarding isoc endpoints. > > Applying a change like that without testing with real hardware of > > those three types just to make fuzz testing happy, sounded a little > > bit risky to my taste. > > > > I would be more willing to pick it if the check would either be > > tested on real hardware or if the logic would be changed to > > accept either bulk or isoc endpoints, just like the current code. > > If the driver did apply to devices that used isochronous transfers, the > ideal approach would be to check the endpoint type against the device > type. However, as it stands this doesn't seem to be necessary. The initial driver had support only for SM1000 and SM11xx. There is a small note there about the sm1000 devices there (I guess this is the chipset number of Stellar, but my memories might be tricking me), but without a real association with the chipset number: /* This device is only present before firmware load */ { USB_DEVICE(0x187f, 0x0010), .driver_info = SMS1XXX_BOARD_SIANO_STELLAR_ROM }, /* This device pops up after firmware load */ { USB_DEVICE(0x187f, 0x0100), .driver_info = SMS1XXX_BOARD_SIANO_STELLAR }, Years later, support for sm2xxx was added. Those two boards, for instance (see drivers/media/common/siano/sms-cards.c) are variants of sm2xxx (one of them is sm2270, if I'm not mistaken) that supports Brazilian TV standard: [SMS1XXX_BOARD_SIANO_PELE] = { .name = "Siano Pele Digital Receiver", .type = SMS_PELE, .default_mode = DEVICE_MODE_ISDBT_BDA, }, [SMS1XXX_BOARD_SIANO_RIO] = { .name = "Siano Rio Digital Receiver", .type = SMS_RIO, .default_mode = DEVICE_MODE_ISDBT_BDA, }, There are some boards there with a different version of sm22xx that supports only DVB (can't remember anymore what boards). 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. Thanks, Mauro
