Hi Damien, On Fri, 26 Jul 2024, Damien Le Moal wrote:
Introduce the function ata_dev_print_quirks() to print the quirk flags that will be applied to a scanned device. This new function is called from ata_dev_quirks() when a match on a device model or device model and revision is found for a device in the __ata_dev_quirks array. To implement this function, the ATA_QUIRK_ flags are redefined using the new enum ata_quirk which defines the bit shift for each quirk flag. The array of strings ata_quirk_names is used to define the name of each flag, which are printed by ata_dev_print_quirks(). Example output for a device listed in the __ata_dev_quirks array and which has the ATA_QUIRK_DISABLE flag applied: [10193.461270] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) [10193.469190] ata1.00: Model 'ASMT109x- Config', rev '2143 5', applying quirks: disable [10193.469195] ata1.00: unsupported device, disabling [10193.481564] ata1.00: disable device enum ata_quirk also defines the __ATA_QUIRK_MAX value as one plus the last quirk flag defined. This value is used in ata_dev_quirks() to add a build time check that all quirk flags fit within the unsigned int (32-bits) quirks field of struct ata_device. Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx> Reviewed-by: Igor Pylypiv <ipylypiv@xxxxxxxxxx>
Thanks for your patch, which is now commit 58157d607aecb4e0 ("ata: libata: Print quirks applied to devices") in libata/for-next.
--- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4273,15 +4336,18 @@ static unsigned int ata_dev_quirks(const struct ata_device *dev) unsigned char model_rev[ATA_ID_FW_REV_LEN + 1]; const struct ata_dev_quirks_entry *ad = __ata_dev_quirks; + /* dev->quirks is an unsigned int. */ + BUILD_BUG_ON(__ATA_QUIRK_MAX > 32); + ata_id_c_string(dev->id, model_num, ATA_ID_PROD, sizeof(model_num)); ata_id_c_string(dev->id, model_rev, ATA_ID_FW_REV, sizeof(model_rev)); while (ad->model_num) { - if (glob_match(ad->model_num, model_num)) { - if (ad->model_rev == NULL) - return ad->quirks; - if (glob_match(ad->model_rev, model_rev)) - return ad->quirks; + if (glob_match(ad->model_num, model_num) && + (!ad->model_rev || glob_match(ad->model_rev, model_rev))) { + ata_dev_print_quirks(dev, model_num, model_rev, + ad->quirks); + return ad->quirks; } ad++; }
During boot-up on Salvator-XS (using rcar-sata), the quirk info is printed not once, but four times. Is that intentional? ata1: link resume succeeded after 1 retries +rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device input: keys as /devices/platform/keys/input/input0 ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq ata1.00: ATA-7: Maxtor 6L160M0, BANC1G10, max UDMA/133 ata1.00: 320173056 sectors, multi 0: LBA48 NCQ (not used) +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq ata1.00: configured for UDMA/133 scsi 0:0:0:0: Direct-Access ATA Maxtor 6L160M0 1G10 PQ: 0 ANSI: 5 sd 0:0:0:0: [sda] 320173056 512-byte logical blocks: (164 GB/153 GiB) sd 0:0:0:0: [sda] Write Protect is off sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sd 0:0:0:0: [sda] Preferred minimum I/O size 512 bytes sda: sda1 sd 0:0:0:0: [sda] Attached SCSI disk During resume from s2idle or s2ram, the same info is printed again, fourfold: ata1: link resume succeeded after 1 retries ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq +ata1.00: Model 'Maxtor 6L160M0', rev 'BANC1G10', applying quirks: noncq ata1.00: configured for UDMA/133 ata1.00: Entering active power mode Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds