On Tue, Dec 03, 2024 at 08:30:40AM +0800, xiehongyu1@xxxxxxxxxx wrote: > Hi Greg, > > On 2024/12/2 14:38, Greg KH wrote: > > On Mon, Dec 02, 2024 at 11:18:55AM +0800, xiehongyu1@xxxxxxxxxx wrote: > >> From: Hongyu Xie <xiehongyu1@xxxxxxxxxx> > >> > >> In the old way, vendor name and product name need to be put in > >> xhci_compliance_mode_recovery_timer_quirk_check, it's not convenient. > >> > >> So add two module param for convenience. > > > > Please no. Module parameters are from the 1990's, they do not scale or > > work well anymore, please never add them. > > > >> > >> usage: put xhci-hcd.compliance_vendor=[vendor name] > >> xhci-hcd.compliance_product=[product name] in cmdline. > >> > >> In Ubuntu you can use "dmidecode -t system" to get vendor name and > >> product name. > >> > >> Signed-off-by: Hongyu Xie <xiehongyu1@xxxxxxxxxx> > >> --- > >> drivers/usb/host/xhci.c | 18 ++++++++++++++++-- > >> 1 file changed, 16 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > >> index 5ebde8cae4fc..2007c27bfaf4 100644 > >> --- a/drivers/usb/host/xhci.c > >> +++ b/drivers/usb/host/xhci.c > >> @@ -39,6 +39,14 @@ static unsigned long long quirks; > >> module_param(quirks, ullong, S_IRUGO); > >> MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default"); > >> > >> +static char *compliance_product; > >> +module_param(compliance_product, charp, 0644); > >> +MODULE_PARM_DESC(compliance_product, "Product name for compliance comparison"); > >> + > >> +static char *compliance_vendor; > >> +module_param(compliance_vendor, charp, 0644); > >> +MODULE_PARM_DESC(compliance_vendor, "Vendor name for compliance comparison"); > > > > Also, you have provided no documentation as to why these are needed at > > all, so that's not going to work :( > Engenieer from RENESA suggested to put vendor name and product name in > xhci_compliance_mode_recovery_timer_quirk_check for some buggy > motherboard that using uPD720201. Why not fix the hardware instead? And what is this going to "check"? > For oem(or odm), there might be multiple names for the same > motherboard(or same design). And put all the names in > xhci_compliance_mode_recovery_timer_quirk_check might not be a good > idea. It should be ok to do that if all of this is broken hardware, right? > So I figure using module_param can do the job. Anyway, Is there > better way to do this in kernel? sysfs attribute? That way you set it for the specific device you care about, not for ALL devices in the system which is what this patch does. Remember, many systems have multiple xhci controllers (I have a laptop somewhere around here with 4 xhci host controllers.) thanks, greg k-h
