Bjorn Helgaas <helgaas@xxxxxxxxxx> 于2023年3月9日周四 06:57写道： > > Please adjust the subject line to match previous history, e.g., > > PCI: vmd: Add ... MSI ... > OK. I will fix it. Sorry for ignoring the subject format. > > In the legacy, the vmd msi-mode can only be adjusted by configing > > vmd_ids table.This patch adds another way to adjust msi mode by > > adjusting module param, which allow users easier to adjust the vmd > > according to the I/O scenario without rebuilding driver.There are two > > params could be recognized: on, off. The default param is "NULL", > > the goal is not to affect the existing settings of the device. > > Please add a space after the period that ends each sentence. > Capitalize "MSI" to match usage in spec. > Sorry for the format issue. I neglected them. My bad~ > > Signed-off-by: Xinghui Li <korantli@xxxxxxxxxxx> > > Reviewed-by: Nirmal Patel <nirmal.patel@xxxxxxxxxxxxxxx> > > I didn't see a response from Nirmal on the mailing list with the > Reviewed-by. I think it's better if Nirmal responds to the patch > directly on the mailing list with the Reviewed-by, and whoever applies > the patch can incorporate it. Otherwise we have no visibility into > any interaction between you and Nirmal. > I pinged Nirmal to reply to this patch, It seems he forgot to cc the mail list in the previous version's discussion. > > +/* > > + * The VMD msi_remap module parameter provides the alternative way > > + * to adjust msi mode when loading vmd.ko other than vmd_ids table. > > + * There are two params could be recognized: > > + * > > + * 1-off > > + * 2-on > > It looks like your code matches either "on" or "off", not "1" or "2". > I will change the comment. It does mislead the reader. I mean the No.1 param is "on" and the No.2 param is "off" > > + * The default param is "NULL", the goal is not to affect the existing > > + * settings of the device. > > + */ > > +char *msi_remap = "NULL"; > > Looks like this should be static? And using "NULL" (as opposed to > something like the empty string "") suggests some intrinsic meaning of > "NULL", but I think there is no intrinsic meaning and the only point > is that "NULL" doesn't match either "on" or "off". > The "static" one is better, I will add it. Initial parameters with "NULL" just aim to mismatch "on" or "off". Do you prefer to init it without the default string?
