It's interesting that there's semi active discussion about this driver.
I ran into the driver being completely and utterly broken today when I
bought a Fujifilm DPC-R1 card reader (that was unsold for ages at the
local camera store to read out my Minolta α-9's exposure info) that uses
this chipset. It will either generate an oops error or media error
depending on when the card is inserted into the reader, but it at least
detects it.
I've submitted a bug (#217862) about this issue. The behavior is a bit
interesting on older kernels (as in 5.15) as it simply generates the
"transfer buffer is on stack" error but does not detect any media in the
slot whatsoever or attempt to.
Jake (aka Pawlicker)
On 8/29/2023 7:28 PM, Alan Stern wrote:
On Tue, Aug 29, 2023 at 11:42:07PM +0200, Oliver Neukum wrote:
On 29.08.23 20:49, Alan Stern wrote:
On Tue, Aug 29, 2023 at 08:14:05PM +0200, Oliver Neukum wrote:
Hi Alan,
as you did something on this driver, doesn't
this condition:
(status[0] & 0x80) ||
((status[0] & 0x1F) == 0x10) || ((status[1] & 0x01) == 0)
look odd to you? Especially the parentheses?
(The actual text in my copy of the file is:
Yes, I rearranged it by the parantheses.
if ((status[0] & 0x80) || ((status[0] & 0x1F) == 0x10)
|| ((status[1] & 0x01) == 0)) {
This probably doesn't affect your point...)
Certainly the layout is a little peculiar, and the extra parentheses
don't help any. But they don't really hurt, either, and the meaning is
clear. It doesn't look obviously wrong.
Ok, then this is just me. THe parantheses would make perfect sense
if the actual intent were:
(status[0] & 0x80) ||
((status[0] & 0x1F) == 0x10) && ((status[1] & 0x01) == 0)
In fact, if I were writing that expression, I would do:
(status[0] & 0x80) ||
((status[0] & 0x1F) == 0x10 && (status[1] & 0x01) == 0)
simply because I don't like relying on the relative precedence of || and
&&. That's just my own neurosis. (On the other hand I have no qualms
about the relative precedence of && and ==, because that combination of
operators gets used all the time. Maybe Daniel Drake preferred not to
rely on it?)
For all we know, this is what the code _should_ be. Without any
documentation on the meaning of the status bits, there's no way to tell.
Those two lines go back to the original version of the driver, added in
2005 by commit e80b0fade09e ("[PATCH] USB Storage: add alauda support"),
written by Daniel Drake and edited by Matt Dharm. So it's been around
for quite a while and there may not be many devices left that need this
driver.
Yes, I know. Hence my question.
Did you want to change it?
Nope. I just looked through the log and saw your patch for the
failure of the transfer and the subsequent test looked
messed up.
At this point, I don't think it matters much.
Alan Stern
