Re: [PATCH 2/6] mtd: nand: Use wrappers to call onfi GET/SET_FEATURES

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Boris,

On Tue, 6 Feb 2018 15:55:08 +0100, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> On Sat,  3 Feb 2018 10:55:40 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> >  /**
> > - * nand_onfi_set_features- [REPLACEABLE] set features for ONFI nand
> > + * nand_set_features_default - [REPLACEABLE] set features for ONFI NAND
> >   * @mtd: MTD device structure
> >   * @chip: nand chip info structure
> >   * @addr: feature address.
> >   * @subfeature_param: the subfeature parameters, a four bytes array.
> >   */
> > -static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> > -			int addr, uint8_t *subfeature_param)
> > +static int nand_set_features_default(struct mtd_info *mtd,
> > +				     struct nand_chip *chip, int addr,
> > +				     uint8_t *subfeature_param)  
> 
> This name change is not mentioned in the commit message, and it's
> probably something you should do in a separate patch. And how about
> moving the default specifier just after nand_ =>
> nand_default_set_features().

This change has been moved in a separate patch, that will be integrated
in the series that prepares to this one (about a better handling of
timings in the core).

> 
> >  {
> > -	if (!chip->onfi_version ||
> > -	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> > -	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> > -		return -EINVAL;
> > -
> >  	return nand_set_features_op(chip, addr, subfeature_param);
> >  }
> >  
> >  /**
> > - * nand_onfi_get_features- [REPLACEABLE] get features for ONFI nand
> > + * nand_get_features_default - [REPLACEABLE] get features for ONFI NAND
> >   * @mtd: MTD device structure
> >   * @chip: nand chip info structure
> >   * @addr: feature address.
> >   * @subfeature_param: the subfeature parameters, a four bytes array.
> >   */
> > -static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
> > -			int addr, uint8_t *subfeature_param)
> > +static int nand_get_features_default(struct mtd_info *mtd,
> > +				     struct nand_chip *chip, int addr,
> > +				     uint8_t *subfeature_param)  
> 
> Ditto.
> 
> >  {
> > -	if (!chip->onfi_version ||
> > -	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> > -	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> > -		return -EINVAL;
> > -
> >  	return nand_get_features_op(chip, addr, subfeature_param);
> >  }
> >  
> > @@ -5015,9 +5056,9 @@ static void nand_set_defaults(struct nand_chip *chip)
> >  
> >  	/* set for ONFI nand */
> >  	if (!chip->onfi_set_features)
> > -		chip->onfi_set_features = nand_onfi_set_features;
> > +		chip->onfi_set_features = nand_set_features_default;
> >  	if (!chip->onfi_get_features)
> > -		chip->onfi_get_features = nand_onfi_get_features;
> > +		chip->onfi_get_features = nand_get_features_default;  
> 
> We should probably also rename the hooks at some point, because GET/SET
> FEATURES operations are not limited to ONFi compliant chips.

Done. Will also be in the other series. Next version is coming soon.

Thanks,
Miquèl

-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com