On 26/11/18 6:12 PM, Boris Brezillon wrote:
On Mon, 26 Nov 2018 13:37:46 +0100
Boris Brezillon <boris.brezil...@bootlin.com> wrote:
On Mon, 26 Nov 2018 16:42:48 +0530
Jagan Teki <ja...@openedev.com> wrote:
On 26/11/18 2:12 PM, Boris Brezillon wrote:
Hi Jagan,
On Thu, 22 Nov 2018 09:40:56 +0100
Boris Brezillon <boris.brezil...@bootlin.com> wrote:
+ /*
+ * Setting mtd->priv to NULL is the best we can do. Thanks to that,
+ * the MTD layer can still call mtd hooks without risking a
+ * use-after-free bug. Still, things should be fixed to prevent the
+ * spi_flash object from being destroyed when del_mtd_device() fails.
+ */
+ sf_mtd_info.priv = NULL;
+ printf("Failed to unregister MTD %s and the spi_flash object is going away:
you're in deep trouble!",
+ sf_mtd_info.name);
Why do we need this print?
Yes we do, just to keep the user informed that something bad happened
and its spi-flash is no longer usable (at least through the MTD layer).
can't we do the same thing in MTD core
itself, so-that it can be generic for all flash objects.
del_mtd_device() can fail, so it's the caller responsibility to decide
what to do when that happens. Some users will propagate the error to
the upper layer and maybe cancel the device removal (AFAICT,
driver->remove() can return an error, not sure what happens in this
case though). For others, like spi-flash, the device will go away, and
all subsequent accesses will fail.
I'm about to send a new version fixing the problem I mentioned in patch
3, but before doing that, I'd like to know if my answer convinced you or
if you'd still prefer this message to go away (or be placed in
mtdcore/mtdpart.c).
I'm thinking of having the message still in MTD by showing which
interface it would belongs, along with the details.
Then we'd need something less
Sorry, I inadvertently hit the send button :-/.
So, I was about to say that we need something less worrisome than the
message I added in the SF layer if we move it to the MTD layer because
some drivers might actually do the right thing when del_mtd_device()
returns an error. I keep thinking that putting an error message in
mtdcore.c is not the right thing to do, but if you insist, I'll add
one (maybe a debug()). In any case I'd like to keep the one we have
here, because in this specific case, there's simply nothing we can do
when MTD dev removal fails.
Look like other flashes were deleting only via mtd_partitions. how would
they know and does it not need for them to print the same information?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
