linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: linux-mtd@lists.infradead.org
Cc: Brian Norris <computersforpeace@gmail.com>
Subject: Error handling in spi-nor and dealing with short reads/writes
Date: Wed, 7 Oct 2015 21:41:37 +0200	[thread overview]
Message-ID: <56157571.4090007@gmail.com> (raw)

I stumbled across some missing error handling in spi-nor and when having a look at the
list archive I found some proposed patches and related discussions.
However it seems like there was no result, at least I didn't find accepted patches
dealing with this issue.

To summarize few issues:

1. I didn't find any clear definition how the read / write callbacks in spi_nor and mtd_info
   are supposed to handle the retlen argument (returning the actual length of the current
   transfer or adding it to the current value of *retlen).
   Maybe due to this missing specification there are different implementations of the read
   callback in spi_nor:
   m25p80_read: sets *retlen to the actual length of the current transfer
   nxp_spifi_read: adds it to *retlen
   fsl_qspi_read: adds it to *retlen
   Luckily this has no impact so far as mtd_read initializes *retlen to 0 before calling
   the _read callback (once).

2. The write callback in spi_nor is defined with a void return type and doesn't allow to
   handle the case of a failing transfer.

3. It's unclear at which layer incomplete reads/writes are acceptable (and therefore which
   layer can / should loop to assemble chunks).

My 2ct:

1. They should return the actual length of the current transfer in *retlen.
   In case of a partial transfer they should return an error (e.g. -EIO or -EMSGSIZE).
   This allows the caller to identify a partial transfer and the amount of
   bytes successfully transferred. Then the caller can decide whether he wants
   to retry for the failed part of the transfer and later assemble the chunks.

2. Make it return int (like _write in mtd_info).

3. I prefer to handle read chunks in spi_nor_read (submitted a related patch already).
   Not sure whether it would make sense to add chunk handling also for writes.
   I'm not aware of any controller which is not able to transfer at least a single page
   at once.

This is meant as a RfC. Also I may have missed important parts of the discussion.
Appreciate any hint or comment.

             reply	other threads:[~2015-10-07 19:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-07 19:41 Heiner Kallweit [this message]
2015-11-18  0:45 ` Error handling in spi-nor and dealing with short reads/writes Brian Norris
2015-11-18 20:54   ` Heiner Kallweit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56157571.4090007@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=computersforpeace@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).