Message ID | 1376023752-3105-2-git-send-email-marc.ceeeee@gmail.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
On Thu, Aug 08, 2013 at 09:49:10PM -0700, Marc C wrote: > From: Marc Carino <marc.ceeeee@gmail.com> > > SATA 3.1 added an "auxiliary" field to the host-to-device FIS. > > Since there is no analog between the new field and the ATA > taskfile, a new element was added to 'struct ata_queued_cmd." Ummm... I'm sorry but I'm gonna do 180 from Sergei on this one. The auxiliary field is part of ata taskfile for all intents and purposes. FIS is the new command structure anyway and struct ata_taskfile proper should be able to describe the command with ata_queuedcmd providing the surrounding context. The argument that ata_taskfile shouldn't contain anything which wasn't in PATA taskfile is bogus as it already contains ATA_TFLAG_*. So, please put the aux field into ata_taskfile. That's where it belongs. Thanks.
Hello. On 08/09/2013 08:49 AM, Marc C wrote: > From: Marc Carino <marc.ceeeee@gmail.com> > SATA 3.1 added an "auxiliary" field to the host-to-device FIS. > Since there is no analog between the new field and the ATA > taskfile, a new element was added to 'struct ata_queued_cmd." > Signed-off-by: Marc Carino <marc.ceeeee@gmail.com> [...] > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index c24354d..9d02c47 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -532,6 +532,34 @@ int atapi_cmd_type(u8 opcode) > } > > /** > + * ata_qc_to_fis - Convert struct ata_queued_cmd to SATA FIS structure > + * @qc: struct ata_queued_cmd to convert > + * @pmp: Port multiplier port > + * @is_cmd: This FIS is for command > + * @fis: Buffer into which data will output > + * > + * Converts a struct ata_queued_cmd to a Serial ATA > + * FIS structure (Register - Host to Device). > + * > + * Beginning with SATA 3.1, the ATA taskfile does not completely describe > + * all of the fields in a host-to-device FIS. More specifically, the > + * 'auxiliary' field has no ATA taskfile analog, and thus requires us > + * to populate the FIS via the ata_queued_cmd structure. > + * > + * LOCKING: > + * Inherited from caller. > + */ > +void ata_qc_to_fis(const struct ata_queued_cmd *qc, u8 pmp, int is_cmd, u8 *fis) > +{ > + ata_tf_to_fis(&qc->tf, pmp, is_cmd, fis); > + > + fis[16] = qc->auxiliary & 0xff; > + fis[17] = (qc->auxiliary >> 8) & 0xff; > + fis[18] = (qc->auxiliary >> 16) & 0xff; > + fis[19] = (qc->auxiliary >> 24) & 0xff; > +} > + > +/** > * ata_tf_to_fis - Convert ATA taskfile to SATA FIS structure > * @tf: Taskfile to convert > * @pmp: Port multiplier port > @@ -6877,6 +6905,7 @@ EXPORT_SYMBOL_GPL(ata_sg_init); > EXPORT_SYMBOL_GPL(ata_qc_complete); > EXPORT_SYMBOL_GPL(ata_qc_complete_multiple); > EXPORT_SYMBOL_GPL(atapi_cmd_type); > +EXPORT_SYMBOL_GPL(ata_qc_to_fis); > EXPORT_SYMBOL_GPL(ata_tf_to_fis); I think we should now unexport this function and make it static since it would be no longer valid to call it from the drivers... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 08/09/2013 08:49 AM, Marc C wrote: > From: Marc Carino <marc.ceeeee@gmail.com> > SATA 3.1 added an "auxiliary" field to the host-to-device FIS. > Since there is no analog between the new field and the ATA > taskfile, a new element was added to 'struct ata_queued_cmd." > Signed-off-by: Marc Carino <marc.ceeeee@gmail.com> > --- > drivers/ata/acard-ahci.c | 2 +- > drivers/ata/libahci.c | 2 +- > drivers/ata/libata-core.c | 29 +++++++++++++++++++++++++++++ > drivers/ata/sata_fsl.c | 2 +- > drivers/ata/sata_mv.c | 2 +- > drivers/ata/sata_qstor.c | 2 +- > drivers/ata/sata_sil24.c | 2 +- > include/linux/libata.h | 4 ++++ > 8 files changed, 39 insertions(+), 6 deletions(-) Looks like you missed drivers/scsi/libsas/sas_ata.c... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/09/2013 06:17 PM, Sergei Shtylyov wrote: >> From: Marc Carino <marc.ceeeee@gmail.com> >> SATA 3.1 added an "auxiliary" field to the host-to-device FIS. >> Since there is no analog between the new field and the ATA >> taskfile, a new element was added to 'struct ata_queued_cmd." >> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com> > [...] >> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c >> index c24354d..9d02c47 100644 >> --- a/drivers/ata/libata-core.c >> +++ b/drivers/ata/libata-core.c >> @@ -532,6 +532,34 @@ int atapi_cmd_type(u8 opcode) >> } >> >> /** >> + * ata_qc_to_fis - Convert struct ata_queued_cmd to SATA FIS structure >> + * @qc: struct ata_queued_cmd to convert >> + * @pmp: Port multiplier port >> + * @is_cmd: This FIS is for command >> + * @fis: Buffer into which data will output >> + * >> + * Converts a struct ata_queued_cmd to a Serial ATA >> + * FIS structure (Register - Host to Device). >> + * >> + * Beginning with SATA 3.1, the ATA taskfile does not completely describe >> + * all of the fields in a host-to-device FIS. More specifically, the >> + * 'auxiliary' field has no ATA taskfile analog, and thus requires us >> + * to populate the FIS via the ata_queued_cmd structure. >> + * >> + * LOCKING: >> + * Inherited from caller. >> + */ >> +void ata_qc_to_fis(const struct ata_queued_cmd *qc, u8 pmp, int is_cmd, u8 >> *fis) >> +{ >> + ata_tf_to_fis(&qc->tf, pmp, is_cmd, fis); >> + >> + fis[16] = qc->auxiliary & 0xff; >> + fis[17] = (qc->auxiliary >> 8) & 0xff; >> + fis[18] = (qc->auxiliary >> 16) & 0xff; >> + fis[19] = (qc->auxiliary >> 24) & 0xff; >> +} >> + >> +/** >> * ata_tf_to_fis - Convert ATA taskfile to SATA FIS structure >> * @tf: Taskfile to convert >> * @pmp: Port multiplier port >> @@ -6877,6 +6905,7 @@ EXPORT_SYMBOL_GPL(ata_sg_init); >> EXPORT_SYMBOL_GPL(ata_qc_complete); >> EXPORT_SYMBOL_GPL(ata_qc_complete_multiple); >> EXPORT_SYMBOL_GPL(atapi_cmd_type); >> +EXPORT_SYMBOL_GPL(ata_qc_to_fis); >> EXPORT_SYMBOL_GPL(ata_tf_to_fis); > I think we should now unexport this function and make it static since it > would be no longer valid to call it from the drivers... I was somewhat rash in this conclusion: drivers use this function not only to issue commands but also to fake D2H FIS and not only that... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/09/2013 06:03 PM, Tejun Heo wrote: >> From: Marc Carino <marc.ceeeee@gmail.com> >> SATA 3.1 added an "auxiliary" field to the host-to-device FIS. >> Since there is no analog between the new field and the ATA >> taskfile, a new element was added to 'struct ata_queued_cmd." > Ummm... I'm sorry but I'm gonna do 180 from Sergei on this one. The That's very unfortunate for me. :-( > auxiliary field is part of ata taskfile for all intents and purposes. > FIS is the new command structure anyway and struct ata_taskfile proper > should be able to describe the command with ata_queuedcmd providing > the surrounding context. The argument that ata_taskfile shouldn't > contain anything which wasn't in PATA taskfile is bogus as it already > contains ATA_TFLAG_*. That's what make 'struct ata_taskfile' bogus. I'm going to remove 'protocol', 'flags', and 'ctl' fields from there (at least to save a space in 'struct ata_queued_cmd' because they're not used in 'result_tf'). > So, please put the aux field into ata_taskfile. That's where it > belongs. Can't agree to that. I was going to make 'struct ata_taskfile' reflect the historical notion and remove from it all not belonging to that notion. Alas, libata has a bad history of mistreating the historical terms... > Thanks. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 09, 2013 at 06:36:19PM +0400, Sergei Shtylyov wrote: > >Ummm... I'm sorry but I'm gonna do 180 from Sergei on this one. The > > That's very unfortunate for me. :-( Hehe, sorry. :) > >So, please put the aux field into ata_taskfile. That's where it > >belongs. > > Can't agree to that. I was going to make 'struct ata_taskfile' > reflect the historical notion and remove from it all not belonging > to that notion. Alas, libata has a bad history of mistreating the > historical terms... But what does sticking to the historical definition give us when the whole concept of TF is no longer maintained in new specs. The distinction between "command proper" and "supporting / plumbing information" is still useful so I think it's natural to assign the former to ata_taskfile and the latter to ata_queuedcmd. After all, there are cases where we want to describe command without all the plumbing stuff libata imposes and given that the aux field is part of FIS proper it might even get used for command results too in the future. I mean, FIS is the new TF. We can rename ata_taskfile to ata_fis and map things the other way but that'd just be extra churn. Thanks.
On 08/09/2013 06:03 PM, Tejun Heo wrote: >> From: Marc Carino <marc.ceeeee@gmail.com> >> SATA 3.1 added an "auxiliary" field to the host-to-device FIS. >> Since there is no analog between the new field and the ATA >> taskfile, a new element was added to 'struct ata_queued_cmd." > Ummm... I'm sorry but I'm gonna do 180 from Sergei on this one. The > auxiliary field is part of ata taskfile for all intents and purposes. Which would be another abuse of the ATA taskfile term (reminds me of SFF abuse wehre everybody hushed me down saying that it's too late to change anything and for all intenets and purposes SFF means "taskfile based" for everybody). As Marc rightly said, this field won't be deliverable by the usual taskfile methods. Moreover, it won't be used in qc->result_tf' and so will waste 4 bytes for no reason. > FIS is the new command structure anyway and struct ata_taskfile proper > should be able to describe the command with ata_queuedcmd providing > the surrounding context. It depends on your definition of surrounding context. In my definition, 'flags' and 'protocol' fields should be a part of surrounding context. 'ctl' too sicen the device control register was never considered a part of the ATA taskfile and it's not used in 'qc->result_tf' (as well as the other two fields), hence just wasting memory for no reason. > The argument that ata_taskfile shouldn't > contain anything which wasn't in PATA taskfile is bogus as it already > contains ATA_TFLAG_*. I regret that I haven't done my patches earlier to get rid of this example of abuse of the ATA taskfile before. > So, please put the aux field into ata_taskfile. That's where it > belongs. Sigh. > Thanks. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 08/09/2013 06:53 PM, Tejun Heo wrote: >>> Ummm... I'm sorry but I'm gonna do 180 from Sergei on this one. The >> That's very unfortunate for me. :-( > Hehe, sorry. :) I've started to work on my taskfile patchset about a year ago (while being in hospital) and worked on it on my copious free time (perhaps, not actively enough until I realized I don't have much time anymore), so it doesn't sound funny for me. If you're going to reject my patches once submitted outright, just tell me now, and with some regret for the wasted time, I'll find a better use for my free time, making a note to myself that the taskfile support in libata is hopeless and the maintainer doesn't care a bit about that. (In case you want an example of better taskfile support, look at IDE). >>> So, please put the aux field into ata_taskfile. That's where it >>> belongs. >> Can't agree to that. I was going to make 'struct ata_taskfile' >> reflect the historical notion and remove from it all not belonging >> to that notion. Alas, libata has a bad history of mistreating the >> historical terms... > But what does sticking to the historical definition give us when the > whole concept of TF is no longer maintained in new specs. It predated even the old specs, so what? > The > distinction between "command proper" and "supporting / plumbing > information" is still useful Yes, it's just the border was drawn incorrectly from the start IMO. Jeff even placed the structure and friends into ata.h for no apparent reason (part of those *enum* friends eventually got shared by IDE). > so I think it's natural to assign the > former to ata_taskfile and the latter to ata_queuedcmd. That's exactly what I want to do. > After all, there are cases where we want to describe command without all the > plumbing stuff libata imposes Exactly what I want to achieve. > and given that the aux field is part of > FIS proper it might even get used for command results too in the > future. If and when this happens, we'll see. So far, the structure of H2D and D2H FIS is different enough that libata doesn't have a stucture defined for FISes, and just treats them as array of u8. > I mean, FIS is the new TF. FISes are used by what, handful of controllers? While the others, even SATA are still taskfile based and can't transfer FISes directly (especially true for the embedded world where not everybody wants to use AHCI). So that's kind of wishful thinking for now. And don't forget about the PATA legacy you'll have to drag around forever. > We can rename ata_taskfile to > ata_fis and map things the other way but that'd just be extra churn. Indeed. And totally stupid move concerning the amount of direct FIS support in different hardware. > Thanks. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, Sergei. On Sat, Aug 10, 2013 at 01:39:08AM +0400, Sergei Shtylyov wrote: > I've started to work on my taskfile patchset about a year ago > (while being in hospital) and worked on it on my copious free time > (perhaps, not actively enough until I realized I don't have much > time anymore), so it doesn't sound funny for me. If you're going to > reject my patches once submitted outright, just tell me now, and > with some regret for the wasted time, I'll find a better use for my > free time, making a note to myself that the taskfile support in > libata is hopeless and the maintainer doesn't care a bit about that. > (In case you want an example of better taskfile support, look at > IDE). Can you explain why following the traditional TF definition matters so much? What practical difference does that make? The new field is part of command code definition, so it belongs with the rest of them regardless of what the structure it's contained in is named. If the only reason for strictly separating TF regs into a separate struct is because the spec says so, I indeed don't give a flying hoot. Also, the only controller interface which would continue to be relevant is ahci and that's it. There will be no new development whatsoever happening with TF based interface, ever. I don't see why you're getting all passive agressive about it. If you have technical arguments, dish them out. Thanks.
Hello. On 08/10/2013 01:51 AM, Tejun Heo wrote: >> I've started to work on my taskfile patchset about a year ago >> (while being in hospital) and worked on it on my copious free time >> (perhaps, not actively enough until I realized I don't have much >> time anymore), so it doesn't sound funny for me. If you're going to >> reject my patches once submitted outright, just tell me now, and >> with some regret for the wasted time, I'll find a better use for my >> free time, making a note to myself that the taskfile support in >> libata is hopeless and the maintainer doesn't care a bit about that. >> (In case you want an example of better taskfile support, look at >> IDE). > Can you explain why following the traditional TF definition matters so > much? Because it's more clear, saves some memory and matches the (rather poor) capabilities of the libata's driver taskfile interface (which you can't throw out however you wanted). > What practical difference does that make? The new field is > part of command code definition, so it belongs with the rest of them > regardless of what the structure it's contained in is named. So why not place it to 'struct ata_queued_cmd' then? If it doesn't really matter where to put it if it serves to describe a command, and additionally will save you some memory? > If the only reason for strictly separating TF regs into a separate struct is > because the spec says so, No. Besides, as I told you, taskfile isn't in any spec, it's pre-ATA term. > I indeed don't give a flying hoot. Excellent reply from a maintainer. :-D > Also, the only controller interface which would continue to be > relevant is ahci and that's it. There will be no new development > whatsoever happening with TF based interface, ever. In x86 world maybe but how much does it help you with the legacy stuff you have to drag around? Tell about AHCI to my embedded customer, Renesas. Remember the most recent sata_rcar driver I have submitted? It's taskfile based (there's also SATA driver we at MontaVista did write but didn't submit). And it's used in their top-notch R-Car SoCs. > I don't see why you're getting all passive agressive about it. Don't intimidate me with psychological terms. :-) > If you have technical arguments, dish them out. I have. It seems you intentionally ignore them, and reply to non-technical passage only. > Thanks. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hey, On Sat, Aug 10, 2013 at 02:17:24AM +0400, Sergei Shtylyov wrote: > So why not place it to 'struct ata_queued_cmd' then? If it > doesn't really matter where to put it if it serves to describe a > command, and additionally will save you some memory? Because it's just more churn? Now TF basically matches what goes into FIS. Adding aux to qc break that for no good reason (no, 4 bytes isn't a good reason). > In x86 world maybe but how much does it help you with the legacy > stuff you have to drag around? > Tell about AHCI to my embedded customer, Renesas. Remember the > most recent sata_rcar driver I have submitted? It's taskfile based > (there's also SATA driver we at MontaVista did write but didn't > submit). And it's used in their top-notch R-Car SoCs. Oh sure, we'll carry the existing ones and there will be new drivers for sure. I'm saying that in terms of command protocol and standard, it is and has long been a dead end. Nothing can happen there anymore. There's no point in putting TF based interface in the center of attention. > >I don't see why you're getting all passive agressive about it. > > Don't intimidate me with psychological terms. :-) Please adjust your tone then because your original reply was very easily escalatable. If you want to escalate things, I'm game but if you don't want that, please make that clear too. > >If you have technical arguments, dish them out. > > I have. It seems you intentionally ignore them, and reply to > non-technical passage only. The only concrete thing I got upto this point is saving 4 bytes, which is a bit difficult to consider seriously. Other parts, I don't get at all. Sure there are TF drivers, but how does adding aux field to TF make it worse for them? Thanks.
Hello. On 08/10/2013 02:26 AM, Tejun Heo wrote: >> So why not place it to 'struct ata_queued_cmd' then? If it >> doesn't really matter where to put it if it serves to describe a >> command, and additionally will save you some memory? > Because it's just more churn? Now TF basically matches what goes into > FIS. Adding aux to qc break that for no good reason (no, 4 bytes > isn't a good reason). Funny to remember, when I was just starting my Linux IDE development back in 2006, I published a patch that added 4-byte field to 'ide_hwif_t' and some people started complaining about memory waste, so that I had to promise I'd send a next patch that would remove another 40bte field. Clearly, they didn't expect an explosion in IDE development that stated happening the next year. :-) That time however is gone, and I probably can't promise as stable stream of patches as it was back then, especially given my one-year development period for the mentioned taskfile patches (which I still can't publish due to completely unexpected issue guaranteeing several debugging sessions). >> In x86 world maybe but how much does it help you with the legacy >> stuff you have to drag around? >> Tell about AHCI to my embedded customer, Renesas. Remember the >> most recent sata_rcar driver I have submitted? It's taskfile based >> (there's also SATA driver we at MontaVista did write but didn't >> submit). And it's used in their top-notch R-Car SoCs. > Oh sure, we'll carry the existing ones and there will be new drivers > for sure. I'm saying that in terms of command protocol and standard, > it is and has long been a dead end. Nothing can happen there anymore. > There's no point in putting TF based interface in the center of > attention. I still would like it improved regardless how old the issues there are, I'm just not sure I can do it. Being engaged in the embedded area, I feel more comfortable with the old stuff than the new, hence my interest. >>> I don't see why you're getting all passive agressive about it. >> Don't intimidate me with psychological terms. :-) > Please adjust your tone then because your original reply was very > easily escalatable. If you want to escalate things, I'm game but if > you don't want that, please make that clear too. No, I don't want to escalate to Linus, if you meant it. I was just feeling somewhat bitter. However, I would like to hear a clear answer to my question about the taskfile patches currnetly in the works (although they seem to make less sence now that the taskfile "purity" is out of question): is there a chance you apply them or will you reject them as a needless churn? >>> If you have technical arguments, dish them out. >> I have. It seems you intentionally ignore them, and reply to >> non-technical passage only. > The only concrete thing I got upto this point is saving 4 bytes, which > is a bit difficult to consider seriously. Other parts, I don't get at > all. Sure there are TF drivers, but how does adding aux field to TF > make it worse for them? Well, it seems I'm just out of arguments at this point. It seems just aesthetically unpleasing and not right to me to have a field in taskfile which can't be delivered via the taskfile interface, if you want. To be honest, I also have a patch that makes the taskfile lose it's 'hob_' fields, and so the '[result_]tf' variables in the 'struct ata_queued_cmd' becoming arrays of 2 elements but I was somewhat afraid of publishing it because it's not exactly a memory saver (and a candidate of being labelled a pointless churn). Adding the new field to struct ata_taskfile just would make this patch completely impractical... > Thanks. Not at all. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, Sergei. On Sun, Aug 11, 2013 at 01:59:05AM +0400, Sergei Shtylyov wrote: > >Oh sure, we'll carry the existing ones and there will be new drivers > >for sure. I'm saying that in terms of command protocol and standard, > >it is and has long been a dead end. Nothing can happen there anymore. > >There's no point in putting TF based interface in the center of > >attention. > > I still would like it improved regardless how old the issues > there are, I'm just not sure I can do it. Being engaged in the > embedded area, I feel more comfortable with the old stuff than the > new, hence my interest. Sure, improvements are always welcome. My point is that when making trade-offs, it'd make more sense to prioritize design concerns for FIS based ones rather than the other way around. Here, the only concrete downside for TF based ones is 4 extra bytes in ata_taskfile, which I don't think matters at all. > No, I don't want to escalate to Linus, if you meant it. I was > just feeling somewhat bitter. However, I would like to hear a clear > answer to my question about the taskfile patches currnetly in the > works (although they seem to make less sence now that the taskfile > "purity" is out of question): is there a chance you apply them or > will you reject them as a needless churn? I can't say for sure before actually looking at the patch but I'm likely to nack it if the only reason behind it is saving some bytes and conformance to the traditional definition of taskfile, especially as I'd much prefer to be able to put all fields of a FIS into ata_taskfile. If the moving out the prot and flags out of ata_tf makes the code generally cleaner, sure, but I'm a bit skeptical it would. Why don't you post the patch as-is? Let's see how the whole thing looks like. > Well, it seems I'm just out of arguments at this point. It seems > just aesthetically unpleasing and not right to me to have a field in > taskfile which can't be delivered via the taskfile interface, if you > want. > To be honest, I also have a patch that makes the taskfile lose > it's 'hob_' fields, and so the '[result_]tf' variables in the > 'struct ata_queued_cmd' becoming arrays of 2 elements but I was > somewhat afraid of publishing it because it's not exactly a memory > saver (and a candidate of being labelled a pointless churn). Adding > the new field to struct ata_taskfile just would make this patch > completely impractical... So, two things. Code cleanup is always a good idea; however, "aesthetically unpleasing" is pretty subjective and I'm not too likely to be fond of changes which are puristic in its nature without any practical cleanup value. Secondly, optimization is good but it has to matter. There's no point in saving some bytes in a struct which aren't allocated in massive numbers. Likewise, there's no point in optimizing some cycles in paths which aren't hot. Straight-forwardness and maintainability should be the prime concerns in most cases. Thanks.
diff --git a/drivers/ata/acard-ahci.c b/drivers/ata/acard-ahci.c index fd665d9..a7bf4c4 100644 --- a/drivers/ata/acard-ahci.c +++ b/drivers/ata/acard-ahci.c @@ -274,7 +274,7 @@ static void acard_ahci_qc_prep(struct ata_queued_cmd *qc) */ cmd_tbl = pp->cmd_tbl + qc->tag * AHCI_CMD_TBL_SZ; - ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, cmd_tbl); + ata_qc_to_fis(qc, qc->dev->link->pmp, 1, cmd_tbl); if (is_atapi) { memset(cmd_tbl + AHCI_CMD_TBL_CDB, 0, 32); memcpy(cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, qc->dev->cdb_len); diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index acfd0f7..2283ea4 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -1498,7 +1498,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc) */ cmd_tbl = pp->cmd_tbl + qc->tag * AHCI_CMD_TBL_SZ; - ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, cmd_tbl); + ata_qc_to_fis(qc, qc->dev->link->pmp, 1, cmd_tbl); if (is_atapi) { memset(cmd_tbl + AHCI_CMD_TBL_CDB, 0, 32); memcpy(cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, qc->dev->cdb_len); diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index c24354d..9d02c47 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -532,6 +532,34 @@ int atapi_cmd_type(u8 opcode) } /** + * ata_qc_to_fis - Convert struct ata_queued_cmd to SATA FIS structure + * @qc: struct ata_queued_cmd to convert + * @pmp: Port multiplier port + * @is_cmd: This FIS is for command + * @fis: Buffer into which data will output + * + * Converts a struct ata_queued_cmd to a Serial ATA + * FIS structure (Register - Host to Device). + * + * Beginning with SATA 3.1, the ATA taskfile does not completely describe + * all of the fields in a host-to-device FIS. More specifically, the + * 'auxiliary' field has no ATA taskfile analog, and thus requires us + * to populate the FIS via the ata_queued_cmd structure. + * + * LOCKING: + * Inherited from caller. + */ +void ata_qc_to_fis(const struct ata_queued_cmd *qc, u8 pmp, int is_cmd, u8 *fis) +{ + ata_tf_to_fis(&qc->tf, pmp, is_cmd, fis); + + fis[16] = qc->auxiliary & 0xff; + fis[17] = (qc->auxiliary >> 8) & 0xff; + fis[18] = (qc->auxiliary >> 16) & 0xff; + fis[19] = (qc->auxiliary >> 24) & 0xff; +} + +/** * ata_tf_to_fis - Convert ATA taskfile to SATA FIS structure * @tf: Taskfile to convert * @pmp: Port multiplier port @@ -6877,6 +6905,7 @@ EXPORT_SYMBOL_GPL(ata_sg_init); EXPORT_SYMBOL_GPL(ata_qc_complete); EXPORT_SYMBOL_GPL(ata_qc_complete_multiple); EXPORT_SYMBOL_GPL(atapi_cmd_type); +EXPORT_SYMBOL_GPL(ata_qc_to_fis); EXPORT_SYMBOL_GPL(ata_tf_to_fis); EXPORT_SYMBOL_GPL(ata_tf_from_fis); EXPORT_SYMBOL_GPL(ata_pack_xfermask); diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c index 19720a0..bbdba01 100644 --- a/drivers/ata/sata_fsl.c +++ b/drivers/ata/sata_fsl.c @@ -525,7 +525,7 @@ static void sata_fsl_qc_prep(struct ata_queued_cmd *qc) cd = (struct command_desc *)pp->cmdentry + tag; cd_paddr = pp->cmdentry_paddr + tag * SATA_FSL_CMD_DESC_SIZE; - ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, (u8 *) &cd->cfis); + ata_qc_to_fis(qc, qc->dev->link->pmp, 1, (u8 *) &cd->cfis); VPRINTK("Dumping cfis : 0x%x, 0x%x, 0x%x\n", cd->cfis[0], cd->cfis[1], cd->cfis[2]); diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 56be318..96fc238 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -2267,7 +2267,7 @@ static unsigned int mv_qc_issue_fis(struct ata_queued_cmd *qc) u32 fis[5]; int err = 0; - ata_tf_to_fis(&qc->tf, link->pmp, 1, (void *)fis); + ata_qc_to_fis(qc, link->pmp, 1, (void *)fis); err = mv_send_fis(ap, fis, ARRAY_SIZE(fis)); if (err) return err; diff --git a/drivers/ata/sata_qstor.c b/drivers/ata/sata_qstor.c index 3b0dd57..e0ce396 100644 --- a/drivers/ata/sata_qstor.c +++ b/drivers/ata/sata_qstor.c @@ -311,7 +311,7 @@ static void qs_qc_prep(struct ata_queued_cmd *qc) buf[28] = dflags; /* frame information structure (FIS) */ - ata_tf_to_fis(&qc->tf, 0, 1, &buf[32]); + ata_qc_to_fis(qc, 0, 1, &buf[32]); } static inline void qs_packet_start(struct ata_queued_cmd *qc) diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c index aa1051b..7e56d48 100644 --- a/drivers/ata/sata_sil24.c +++ b/drivers/ata/sata_sil24.c @@ -877,7 +877,7 @@ static void sil24_qc_prep(struct ata_queued_cmd *qc) } prb->ctrl = cpu_to_le16(ctrl); - ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, prb->fis); + ata_qc_to_fis(qc, qc->dev->link->pmp, 1, prb->fis); if (qc->flags & ATA_QCFLAG_DMAMAP) sil24_fill_sg(qc, sge); diff --git a/include/linux/libata.h b/include/linux/libata.h index 283d66b..a4601cc 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -604,6 +604,7 @@ struct ata_queued_cmd { struct ata_taskfile tf; u8 cdb[ATAPI_CDB_LEN]; + u32 auxiliary; unsigned long flags; /* ATA_QCFLAG_xxx */ unsigned int tag; @@ -1145,6 +1146,8 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, u32 val, unsigned long interval, unsigned long timeout); extern int atapi_cmd_type(u8 opcode); +extern void ata_qc_to_fis(const struct ata_queued_cmd *qc, + u8 pmp, int is_cmd, u8 *fis); extern void ata_tf_to_fis(const struct ata_taskfile *tf, u8 pmp, int is_cmd, u8 *fis); extern void ata_tf_from_fis(const u8 *fis, struct ata_taskfile *tf); @@ -1658,6 +1661,7 @@ static inline void ata_qc_reinit(struct ata_queued_cmd *qc) qc->n_elem = 0; qc->err_mask = 0; qc->sect_size = ATA_SECT_SIZE; + qc->auxiliary = 0; ata_tf_init(qc->dev, &qc->tf);