Re: [PATCH] mmc_spi: use EILSEQ for possible transmission errors

[Matt Fleming]

On Tue, May 19, 2009 at 09:49:07PM -0700, David Brownell wrote:
> On Tuesday 19 May 2009, Matt Fleming wrote:
> > Hmm, always returning -EILSEQ is devious. What happens if we sent an
> > illegal command? The value of "value" is passed up to the callers via
> > cmd->error and so may eventually get printed in the pr_debug() call in
> > mmc_request_done(), line 86.
> 
> True, but a pr_debug from mmc_spi could help that.  A patch doing
> that would need to be less aggressive about ripping out the current
> fault-parsing logic, but it could continue reporting -EILSEQ to
> cope with the possible response mangling.
> 

Yes, a less aggressive patch would be fine.

> > Whereas before the error would display EINVAL for an illegal command
> > now it'll display EILSEQ, which makes no sense. Seeing EILSEQ in my
> > log when really the error is EINVAL is gonna really confuse me.
> > 
> > IMHO always assuming that command errors are caused by transmission
> > problems is not the right solution.
> 
> Do you have a better solution to propose though?  If Wolfgang
> is actually observing transmission errors there, I'm not sure
> a better one is to be had.
> 

You're right. It's not cool for me to criticise and not give any
suggestions. I've been trying to come up with some middle-ground
solution since yesterday.

I think if the patch was less aggressive at returning EILSEQ, it'd be
fine. For instance, returning EILSEQ if the response you get doesn't
make any sense given the command you sent or if the response is just
garbage.

It might even make sense to retry commands (say twice, and only if
it's safe to do so) and if the response codes differ in bizarre ways
then the status bits have probably been garbled on the bus and EILSEQ
is the correct error code to use. I'm less sure about this bit though,
it depends on exactly what we're trying to fix.

Wolfgang, is there a particular scenario where you're seeing the
status bits trashed by noise on the line? Or is this just a cautionary
patch?

On Tue, May 19, 2009 at 01:47:45PM +0200, Wolfgang Mües wrote:
> 
> > IMHO always assuming that command errors are caused by transmission
> > problems is not the right solution.
> 
> It's not your or my fault, it's the fault of the spi SD/MMC card protocol 
> designer. You may send him to me, already prepared: naked and in chains ;-)

This is the funniest reply I've ever read.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/