Re: [PATCH v13 19/20] mtd: rawnand: Use the ECC framework user input parsing bits

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>,
	linux-mtd@lists.infradead.org,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Fabio Estevam <festevam@gmail.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>
Subject: Re: [PATCH v13 19/20] mtd: rawnand: Use the ECC framework user input parsing bits
Date: Mon, 28 Sep 2020 17:18:35 +0200	[thread overview]
Message-ID: <20200928171835.30c14cf2@xps13> (raw)
In-Reply-To: <20200921093603.GA19513@pengutronix.de>

Hi Sascha,

Sascha Hauer <s.hauer@pengutronix.de> wrote on Mon, 21 Sep 2020
11:36:03 +0200:

> On Thu, Aug 27, 2020 at 10:52:07AM +0200, Miquel Raynal wrote:
> > Many helpers are generic to all NAND chips, they should not be
> > raw-NAND specific, so use the generic ones.
> > 
> > To avoid moving all the raw NAND core "history" into the generic NAND
> > layer, we keep a part of this parsing in the raw NAND core to ensure
> > backward compatibility.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---  
> 
> ...
> 
> > -static int of_get_nand_ecc_step_size(struct device_node *np)
> > +static void of_get_nand_ecc_legacy_user_config(struct nand_chip *chip)
> >  {
> > -	int ret;
> > -	u32 val;
> > +	struct device_node *dn = nand_get_flash_node(chip);
> > +	struct nand_ecc_props *user_conf = &chip->base.ecc.user_conf;
> >  
> > -	ret = of_property_read_u32(np, "nand-ecc-step-size", &val);
> > -	return ret ? ret : val;
> > -}
> > +	if (user_conf->engine_type != NAND_ECC_ENGINE_TYPE_INVALID)
> > +		user_conf->engine_type = of_get_rawnand_ecc_engine_type_legacy(dn);  
> 
> I think the intention is to set user_conf->engine_type when it's not set already,
> so this should be == instead of !=
> 
> >  
> > -static int of_get_nand_ecc_strength(struct device_node *np)
> > -{
> > -	int ret;
> > -	u32 val;
> > +	if (user_conf->algo != NAND_ECC_ALGO_UNKNOWN)
> > +		user_conf->algo = of_get_rawnand_ecc_algo_legacy(dn);  
> 
> ditto
> 
> >  
> > -	ret = of_property_read_u32(np, "nand-ecc-strength", &val);
> > -	return ret ? ret : val;
> > +	if (user_conf->placement != NAND_ECC_PLACEMENT_UNKNOWN)
> > +		user_conf->placement = of_get_rawnand_ecc_placement_legacy(dn);  
> 
> ditto
> 
> Sascha
> 

Sorry for the delay I was away.

You are completely right, I messed up with the logic here... This is
now fixed, thanks!

Miquèl

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