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. 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. So I figure using module_param can do the job. Anyway, Is there better way to do this in kernel? > > > thanks, > > greg k-h >
