On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: > > On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > > > >> Mike Isely wrote: > >>> Mauro: > >>> > >>> You are reading too much into that comment. > >>> > >>> I never said it was valid to do what had been done, only that for the > >>> longest time this is what the driver did and it never caused a problem > >>> that I was made aware of. What I said there was correct, that this is > >>> what the driver had been doing in the past, that it's definitely causing > >>> a problem now and thus that is why this patch exists. > >> As I said, this is not right: > >> "Apparently later kernels don't like this behavior" > > > > Mauro: > > > > That statement was in reference to the fact that previously the problem > > had gone undetected, but now later kernels can notice and complain about > > this, thus "later kernels don't like this behavior". > > > > We can debate that perhaps the statement can be worded better, but that > > doesn't make it *wrong*. > > > > Calling 2.6.12 kernel as "later kernels" doesn't seem right to me (that was > about the kernel were em28xx driver were introduced). The point when the em28xx driver appeared has nothing to do with this. The point when the kernel started complaining about the use of a stack based USB I/O buffers is the relevant point, which was not back in 2.6.12. I learned of this behavior (that is, receiving warnings about the usage) as being new in the 2.6.34 timeframe, the point when a user pointed out the complaint message in his kernel log; at that time I had not yet tested against that kernel version. > > > > >> It is not "later kernels". DMA over stack were never supported. Your driver > >> had a bug that you didn't noticed for long time, probably because nobody > >> reported you this issue, since it appears only on some non-Intel archs and > >> on i386 with more than 3.12 Gb of RAM, and when the stack happens to be after > >> the first 3.12 Gb (with is a somewhat rare condition). > > > > I understand your point perfectly that this was never right or valid. > > In fact, I also understood that point long before you decided to explain > > it to me here - after all my realization of this problem in the driver > > is why I wrote the patch in the first place. Absolutely no argument > > there about the importance of the change. > > > > None of that however justifies putting words into an author's mouth, > > which is effectively what you did by replacing that commit comment. > > > > First of all, it is clearly stated at the patch that the description > were changed and by whom: > [mchehab@xxxxxxxxxx: fix patch description] > > Second, it is an usual upstream practice to fix descriptions where needed. > The patch description is a precious resource for people that are seeking > for similar bugs, and for those that are trying to understand some code. > So, it is important to not send broken/incomplete comments to kernel, > or comments that may have a dubious interpretation. So, subsystem maintainers > frequently need to fix comments. > > Anyway, to end this discussion, I can simply revert the patch from the > staging tree, waiting for a new patch from you with a fixed comment. > > Also, if you prefer, next time, I won't apply any patch from you if I > found a bad comment without your ack, even if it means that you'll > probably loose a merge window. Leave it as is. What's done is done. Your replaced comment is of course still correct. I would just appreciate some better sensitivity in the future about replacing authors' comments, especially since in this case your interpretation of my original comment was wrong. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
