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. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
