Re: [PATCH] mtd: rawnand: Do not check for bad block if bbt is unavailable

Manivannan Sadhasivam
2021-02-03T10:13:12Z

Hi Miquel,

On 3 February 2021 3:35:22 PM IST, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>Hi Manivannan,
>
>Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote on Wed,
>03 Feb 2021 15:28:20 +0530:
>
>> Hi Miquel,
>>
>> On 2 February 2021 1:44:59 PM IST, Miquel Raynal
><miquel.raynal@bootlin.com> wrote:
>> >Hi Manivannan,
>> >
>> >Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote on
>Tue,
>> >2 Feb 2021 09:46:14 +0530:
>> >
>> >> Hi,
>> >>
>> >> On Mon, Feb 01, 2021 at 03:18:24PM +0100, Miquel Raynal wrote:
>> >> > Hi Manivannan,
>> >> >
>> >> > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote
>on
>> >Sat,
>> >> > 30 Jan 2021 09:24:12 +0530:
>> >> >
>> >> > > The bbt pointer will be unavailable when NAND_SKIP_BBTSCAN
>option
>> >is
>> >> > > set for a NAND chip. The intention is to skip scanning for the
>
>> >bad
>> >> > > blocks during boot time.
>> >> >
>> >> > I don't have the same understanding: this flag skips the bad
>block
>> >> > table scan, not the bad block scan. We do want to scan all the
>> >devices
>> >> > in order to construct a RAM based table.
>> >> >
>> >> > > However, the MTD core will call
>> >> > > _block_isreserved() and _block_isbad() callbacks
>unconditionally
>> >for
>> >> > > the rawnand devices due to the callbacks always present while
>
>> >collecting
>> >> > > the ecc stats.
>> >> > >
>> >> > > The _block_isreserved() callback for rawnand will bail out if
>bbt
>> >> > > pointer is not available. But _block_isbad() will continue
>> >without
>> >> > > checking for it. So this contradicts with the
>NAND_SKIP_BBTSCAN
>> >option
>> >> > > since the bad block check will happen anyways (ie., not much
>> >difference
>> >> > > between scanning for bad blocks and checking each block for
>bad
>> >ones).
>> >> > >
>> >> > > Hence, do not check for the bad block if bbt pointer is
>> >unavailable.
>> >> >
>> >> > Not checking for bad blocks at all feels insane. I don't really
>get
>> >the
>> >> > scope and goal of such change?
>> >> >
>> >>
>> >> The issue I encountered is, on the Telit FN980 device one of the
>> >> partition seems to be protected. So trying to read the bad blocks
>in
>> >> that partition makes the device to reboot during boot.
>> >
>> >o_O
>> >
>> >Reading a protected block makes the device to reboot?
>> >
>> >What is the exact device? Can you share the datasheet? Is this
>behavior
>> >expected? Because it seems really broken to me, a read should not
>> >trigger *anything* that bad.
>> >
>>
>> I got more information from the vendor, Telit. The access to the 3rd
>partition is protected by Trustzone and any access in non privileged
>mode (where Linux kernel runs) causes kernel panic and the device
>reboots.
>
>Ok, so this is not a chip feature but more a host constraint.
>
>In this case it would be a good idea to add a host DT property which
>describes the zone to avoid accessing it. Something like:
>
> secure-area/secure-section = <start length>;
>
>From the core perspective, we should parse this property early enough
>and return -EIO when trying to access this area.
>
>Does this solution sound reasonable to you?
>

This sounds good to me. I'll give it a go and share the patch soon.

Thanks,
Mani

>Thanks,
>Miquèl

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.