Re: [PATCH 05/17] mtd: rawnand: cafe: Demistify register fields

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Mon, 27 Apr 2020 21:42:11 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> >  struct cafe_priv {
> >  	struct nand_chip nand;
> > @@ -104,7 +195,8 @@ static const char *part_probes[] = { "cmdlinepart", "RedBoot", NULL };
> >  static int cafe_device_ready(struct nand_chip *chip)
> >  {
> >  	struct cafe_priv *cafe = nand_get_controller_data(chip);
> > -	int result = !!(cafe_readl(cafe, NAND_STATUS) & 0x40000000);
> > +	int result = !!(cafe_readl(cafe, NAND_STATUS) &
> > +			CAFE_NAND_STATUS_FLASH_BUSY);
> >  	uint32_t irqs = cafe_readl(cafe, NAND_IRQ);
> >  
> >  	cafe_writel(cafe, irqs, NAND_IRQ);  
> 
> [...]
> 
> > @@ -318,14 +430,14 @@ static void cafe_select_chip(struct nand_chip *chip, int chipnr)
> >  {
> >  	struct cafe_priv *cafe = nand_get_controller_data(chip);
> >  
> > +	if (chipnr < 0 || chipnr > 1)
> > +		return;
> > +  
> 
> I think this change should not be part of this patch?

Or dropped entirely, since we're getting rid of this function anyway.

> 
> >  	cafe_dev_dbg(&cafe->pdev->dev, "select_chip %d\n", chipnr);
> >  
> >  	/* Mask the appropriate bit into the stored value of ctl1
> >  	   which will be used by cafe_nand_cmdfunc() */
> > -	if (chipnr)
> > -		cafe->ctl1 |= CTRL1_CHIPSELECT;
> > -	else
> > -		cafe->ctl1 &= ~CTRL1_CHIPSELECT;
> > +	cafe->ctl1 |= CAFE_FIELD_PREP(NAND_CTRL1, CE, chipnr);  
> 
> I don't master these macros yet, but are you sure CTRL1_CHIPSELECT will
> actually get cleared if (!chipnr) ?

You're correct, I only ever set the field. I'll fix that.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/