Martin, > > So I do have all the reasons to conclude this value has indeed been > > arbitrarily chosen, don't I? > > The SAT spec defines the contents of the first part of the page. The > trailing 512 bytes are defined in the ATA spec. I think that would best be reflected in code somehow as lone `64' doesn't say anything really. > > If you insist that the value of 64 stay, then please come up with a > > suitable macro name to define along with SCSI_VPD_PG_LEN that reflects > > the meaning of 64 in this context and I'll be happy to update 3/3 > > accordingly, but please explain why the value of 64 is any better than > > 255 here and the inconsistency with the buffer size justified. > > Can please you try the following patch? I have tried it and it's neutral, that is with 1/3 applied the HBA still works and with 1/3 removed it still breaks (2/3 and 3/3 obviously don't build anymore). Unsurprisingly, as it's the call to `scsi_get_vpd_page' rather than `scsi_get_vpd_buf' that causes an issue here. I think the latter function isn't called in my setup, and it's not clear to me anymore after so long why I didn't poke at it. It looks to me like the `retry_pg' code there can be gone now with your update in place as it duplicates buffer sizing, and with that included it'll be a nice clean-up. NB you'll need to adjust drivers/scsi/mpt3sas/mpt3sas_scsih.c accordingly if we are to move forward with this change, as it's another user of the SCSI_VPD_PG_LEN macro. Given what has been said in the discussion so far would you consider 2/3 and 3/3 unnecessary? In the course of verifying your change I have looked through our code again and found that inline magic numbers are there in several though not all places where `scsi_get_vpd_page' gets called, so I think it would make sense to get rid of them all at once with a single self-contained change. Thank you for your input and the extra fix. Maciej
