| Message ID | 20240607213553.1743087-1-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: core: Remove an incorrect comment | expand |
On Fri, Jun 07, 2024 at 02:35:53PM -0700, Bart Van Assche wrote: > The comment that scsi_static_device_list would go away was added more than > 18 years ago. Today, that list is still there and 84 additional entries have > been added. This shows that the comment is incorrect. Hence remove that > comment. I agree that the comment as-is is bogus. But it would be good to state that quirks should go into the LLDs if they aren't for devices on a physical bus like SAS, Fibre Channel or parallel SCSI. Most quirks theses days are for unusually buggy consumer devices like UFS or usb-storage/uas and are better placed there instead of in the core scsi code.
On 6/7/24 22:24, Christoph Hellwig wrote: > On Fri, Jun 07, 2024 at 02:35:53PM -0700, Bart Van Assche wrote: >> The comment that scsi_static_device_list would go away was added more than >> 18 years ago. Today, that list is still there and 84 additional entries have >> been added. This shows that the comment is incorrect. Hence remove that >> comment. > > I agree that the comment as-is is bogus. But it would be good to state > that quirks should go into the LLDs if they aren't for devices on a > physical bus like SAS, Fibre Channel or parallel SCSI. Most quirks > theses days are for unusually buggy consumer devices like UFS or > usb-storage/uas and are better placed there instead of in the core > scsi code. How about changing the comment above scsi_static_device_list[] into this? /* * scsi_static_device_list: list of devices that require settings that differ * from the default, includes black-listed (broken) devices. The entries here * are added to the tail of scsi_dev_info_list via scsi_dev_info_list_init. * * If possible, set the BLIST_* flags from inside the SCSI LLD rather than * adding an entry to this list. */ Thanks, Bart.
On Mon, Jun 10, 2024 at 11:08:14AM -0700, Bart Van Assche wrote: > How about changing the comment above scsi_static_device_list[] into this? > > /* > * scsi_static_device_list: list of devices that require settings that differ > * from the default, includes black-listed (broken) devices. The entries here > * are added to the tail of scsi_dev_info_list via scsi_dev_info_list_init. > * > * If possible, set the BLIST_* flags from inside the SCSI LLD rather than > * adding an entry to this list. > */ This sounds good to me.
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c index 85111e14c53b..5c23ab2b98ab 100644 --- a/drivers/scsi/scsi_devinfo.c +++ b/drivers/scsi/scsi_devinfo.c @@ -39,13 +39,9 @@ static LIST_HEAD(scsi_dev_info_list); static char scsi_dev_flags[256]; /* - * scsi_static_device_list: deprecated list of devices that require - * settings that differ from the default, includes black-listed (broken) - * devices. The entries here are added to the tail of scsi_dev_info_list - * via scsi_dev_info_list_init. - * - * Do not add to this list, use the command line or proc interface to add - * to the scsi_dev_info_list. This table will eventually go away. + * scsi_static_device_list: list of devices that require settings that differ + * from the default, includes black-listed (broken) devices. The entries here + * are added to the tail of scsi_dev_info_list via scsi_dev_info_list_init. */ static struct { char *vendor;
The comment that scsi_static_device_list would go away was added more than 18 years ago. Today, that list is still there and 84 additional entries have been added. This shows that the comment is incorrect. Hence remove that comment. Cc: Avri Altman <Avri.Altman@wdc.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/scsi/scsi_devinfo.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)