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

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 4 Feb 2021 10:04:08 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 4 Feb
> 2021 09:59:45 +0100:
> 
> > On Thu, 4 Feb 2021 14:22:21 +0530
> > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:
> >   
> > > On Thu, Feb 04, 2021 at 09:13:36AM +0100, Miquel Raynal wrote:    
> > > > Hi Manivannan,
> > > > 
> > > > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote on Wed,
> > > > 03 Feb 2021 17:11:31 +0530:
> > > >       
> > > > > On 3 February 2021 4:54:22 PM IST, Boris Brezillon <boris.brezillon@collabora.com> wrote:      
> > > > > >On Wed, 03 Feb 2021 16:22:42 +0530
> > > > > >Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:
> > > > > >        
> > > > > >> On 3 February 2021 3:49:14 PM IST, Boris Brezillon        
> > > > > ><boris.brezillon@collabora.com> wrote:        
> > > > > >> >On Wed, 03 Feb 2021 15:42:02 +0530
> > > > > >> >Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:
> > > > > >> >          
> > > > > >> >> >> 
> > > > > >> >> >> 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.           
> > > > > >> >
> > > > > >> >Out of curiosity, is it a per-CS-line thing or is this section
> > > > > >> >protected on all CS?
> > > > > >> >          
> > > > > >> 
> > > > > >> Sorry, I didn't get your question.         
> > > > > >
> > > > > >The qcom controller can handle several chips, each connected through a
> > > > > >different CS (chip-select) line, right? I'm wondering if the firmware
> > > > > >running in secure mode has the ability to block access for a specific
> > > > > >CS line or if all CS lines have the same constraint. That will impact
> > > > > >the way you describe it in your DT (in one case the secure-region
> > > > > >property should be under the controller node, in the other case it
> > > > > >should be under the NAND chip node).        
> > > > > 
> > > > > Right. I believe the implementation is common to all NAND chips so the property should be in the controller node.       
> > > > 
> > > > Looks weird: do you mean that each of the chips will have a secure area?      
> > > 
> > > I way I said is, the "secure-region" property will be present in the controller
> > > node and not in the NAND chip node since this is not related to the device
> > > functionality.
> > > 
> > > But for referencing the NAND device, the property can have the phandle as below:
> > > 
> > > secure-region = <&nand0 0xffff>;    
> > 
> > My question was really what happens from a functional PoV. If you have
> > per-chip protection at the FW level, this property should be under the
> > NAND node. OTH, if the FW doesn't look at the selected chip before
> > blocking the access, it should be at the controller level. So, you
> > really have to understand what the secure FW does.  
> 
> I'm not so sure actually, that's why I like the phandle to nand0 -> in
> any case it's not a property of the NAND chip itself, it's kind of a
> host constraint, so I don't get why the property should be at the
> NAND node level?

I would argue that we already have plenty of NAND properties that
encode things controlled by the host (ECC, partitions, HW randomizer,
boot device, and all kind of controller specific stuff) :P. Having
the props under the NAND node makes it clear what those things are
applied to, and it's also easier to parse for the driver (you already
have to parse each node to get the reg property anyway).

> 
> Also, we should probably support several secure regions (which could be
> a way to express the fact that the FW does not look at the CS)?

Sure, the secure-region should probably be renamed secure-regions, even
if it's defined at the NAND chip level.