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