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

[Manivannan Sadhasivam]

Hi Miquel, 

On 3 February 2021 3:35:22 PM IST, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
>Hi Manivannan,
>
>Manivannan Sadhasivam <manivannan.sadhasivam at 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 at bootlin.com> wrote:
>> >Hi Manivannan,
>> >
>> >Manivannan Sadhasivam <manivannan.sadhasivam at 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 at 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.