Re: [PATCHv3 12/14] libata: NCQ encapsulation for ZAC MANAGEMENT OUT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hannes,

>Add NCQ encapsulation for ZAC MANAGEMENT OUT and evaluate
>NCQ Non-Data log pages to figure out if NCQ encapsulation
>is supported.
...
>diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>index 6afd084..43403aa 100644
>--- a/drivers/ata/libata-scsi.c
>+++ b/drivers/ata/libata-scsi.c
>@@ -3506,11 +3506,19 @@ static unsigned int ata_scsi_zbc_out_xlat(struct ata_queued_cmd *qc)
> 
> 	reset_all = cdb[14] & 0x1;
> 
>-	tf->protocol = ATA_PROT_NODATA;
>-	tf->command = ATA_CMD_ZAC_MGMT_OUT;
>-	tf->feature = sa;
>-	tf->hob_feature = reset_all & 0x1;
>-
>+	if (ata_ncq_enabled(qc->dev) &&
>+	    ata_fpdma_zac_mgmt_out_supported(qc->dev)) {
>+		tf->protocol = ATA_PROT_NCQ;
>+		tf->command = ATA_CMD_NCQ_NON_DATA;
>+		tf->hob_nsect = ATA_SUBCMD_NCQ_NON_DATA_ZAC_MGMT_OUT;
>+		tf->nsect = qc->tag << 3;
>+		tf->auxiliary = sa | (reset_all & 0x1) << 8;
>+	} else {
>+		tf->protocol = ATA_PROT_NODATA;
>+		tf->command = ATA_CMD_ZAC_MGMT_OUT;
>+		tf->feature = sa;
>+		tf->hob_feature = reset_all & 0x1;
>+	}

Calls to ata_is_data(prot) for ATA_PROT_NCQ task files will always
return "true", even if the command is ATA_CMD_NCQ_NON_DATA (because
the DMA prot flag is always set by ata_prot_flags for NCQ protocol).
This result in all the ATA_CMD_ZAC_MGMT_OUT commands to fail in
ata_qc_issue because of the check: 

        if (WARN_ON_ONCE(ata_is_data(prot) &&
                         (!qc->sg || !qc->n_elem || !qc->nbytes)))
                goto sys_err;


I am not sure the best way to fix this... Hacking ata_qc_issue using the protocol
AND command code to define a bool "is_data" prevents the error but ata_is_data is
also used in different drivers so a more solid fix seem necessary.

Adding a new ATA_PROT_NCQ_NODATA for ATA_CMD_NCQ_NON_DATA would be better, but this
does not really correspond to anything in the standards. And with this, the error
moves to the command sg dma setup complaining that the DMA direction is not known.
Any ideas ?


Also, the "& 0x1" in "tf->auxiliary = sa | (reset_all & 0x1) << 8;" is not necessary
since reset_all is initialized already as "reset_all = cdb[14] & 0x1;"

Best regards.

------------------------
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital company
Damien.LeMoal@xxxxxxxx
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa, 
Kanagawa, 252-0888 Japan
www.hgst.com
Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.
��.n��������+%������w��{.n�����{��'^�)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux