Re: [PATCH v4 03/19] mtd: rawnand: Create a nand_chip operations structure

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Mon, 25 May 2020 19:42:23 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> And move nand_chip hooks there.
> 
> While moving entries from one structure to the other, adapt the
> documentation style.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>

Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>

> ---
>  drivers/mtd/nand/raw/nand_base.c     | 20 ++++++++---------
>  drivers/mtd/nand/raw/nand_hynix.c    |  2 +-
>  drivers/mtd/nand/raw/nand_macronix.c | 10 ++++-----
>  drivers/mtd/nand/raw/nand_micron.c   |  2 +-
>  include/linux/mtd/rawnand.h          | 32 ++++++++++++++++------------
>  5 files changed, 35 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 6a6a0a36b3fd..b86f641f6d74 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -3285,10 +3285,10 @@ static int nand_setup_read_retry(struct nand_chip *chip, int retry_mode)
>  	if (retry_mode >= chip->read_retries)
>  		return -EINVAL;
>  
> -	if (!chip->setup_read_retry)
> +	if (!chip->ops.setup_read_retry)
>  		return -EOPNOTSUPP;
>  
> -	return chip->setup_read_retry(chip, retry_mode);
> +	return chip->ops.setup_read_retry(chip, retry_mode);
>  }
>  
>  static void nand_wait_readrdy(struct nand_chip *chip)
> @@ -4532,8 +4532,8 @@ static int nand_suspend(struct mtd_info *mtd)
>  	int ret = 0;
>  
>  	mutex_lock(&chip->lock);
> -	if (chip->suspend)
> -		ret = chip->suspend(chip);
> +	if (chip->ops.suspend)
> +		ret = chip->ops.suspend(chip);
>  	if (!ret)
>  		chip->suspended = 1;
>  	mutex_unlock(&chip->lock);
> @@ -4551,8 +4551,8 @@ static void nand_resume(struct mtd_info *mtd)
>  
>  	mutex_lock(&chip->lock);
>  	if (chip->suspended) {
> -		if (chip->resume)
> -			chip->resume(chip);
> +		if (chip->ops.resume)
> +			chip->ops.resume(chip);
>  		chip->suspended = 0;
>  	} else {
>  		pr_err("%s called for a chip which is not in suspended state\n",
> @@ -4581,10 +4581,10 @@ static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
> -	if (!chip->lock_area)
> +	if (!chip->ops.lock_area)
>  		return -ENOTSUPP;
>  
> -	return chip->lock_area(chip, ofs, len);
> +	return chip->ops.lock_area(chip, ofs, len);
>  }
>  
>  /**
> @@ -4597,10 +4597,10 @@ static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
> -	if (!chip->unlock_area)
> +	if (!chip->ops.unlock_area)
>  		return -ENOTSUPP;
>  
> -	return chip->unlock_area(chip, ofs, len);
> +	return chip->ops.unlock_area(chip, ofs, len);
>  }
>  
>  /* Set default functions */
> diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
> index 7caedaa5b9e5..7d1be53f27f3 100644
> --- a/drivers/mtd/nand/raw/nand_hynix.c
> +++ b/drivers/mtd/nand/raw/nand_hynix.c
> @@ -337,7 +337,7 @@ static int hynix_mlc_1xnm_rr_init(struct nand_chip *chip,
>  	rr->nregs = nregs;
>  	rr->regs = hynix_1xnm_mlc_read_retry_regs;
>  	hynix->read_retry = rr;
> -	chip->setup_read_retry = hynix_nand_setup_read_retry;
> +	chip->ops.setup_read_retry = hynix_nand_setup_read_retry;
>  	chip->read_retries = nmodes;
>  
>  out:
> diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
> index 09c254c97b5c..1472f925f386 100644
> --- a/drivers/mtd/nand/raw/nand_macronix.c
> +++ b/drivers/mtd/nand/raw/nand_macronix.c
> @@ -130,7 +130,7 @@ static void macronix_nand_onfi_init(struct nand_chip *chip)
>  		return;
>  
>  	chip->read_retries = MACRONIX_NUM_READ_RETRY_MODES;
> -	chip->setup_read_retry = macronix_nand_setup_read_retry;
> +	chip->ops.setup_read_retry = macronix_nand_setup_read_retry;
>  
>  	if (p->supports_set_get_features) {
>  		bitmap_set(p->set_feature_list,
> @@ -242,8 +242,8 @@ static void macronix_nand_block_protection_support(struct nand_chip *chip)
>  	bitmap_set(chip->parameters.set_feature_list,
>  		   ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
>  
> -	chip->lock_area = mxic_nand_lock;
> -	chip->unlock_area = mxic_nand_unlock;
> +	chip->ops.lock_area = mxic_nand_lock;
> +	chip->ops.unlock_area = mxic_nand_unlock;
>  }
>  
>  static int nand_power_down_op(struct nand_chip *chip)
> @@ -312,8 +312,8 @@ static void macronix_nand_deep_power_down_support(struct nand_chip *chip)
>  	if (i < 0)
>  		return;
>  
> -	chip->suspend = mxic_nand_suspend;
> -	chip->resume = mxic_nand_resume;
> +	chip->ops.suspend = mxic_nand_suspend;
> +	chip->ops.resume = mxic_nand_resume;
>  }
>  
>  static int macronix_nand_init(struct nand_chip *chip)
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index 3589b4fce0d4..4385092a9325 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -84,7 +84,7 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
>  		struct nand_onfi_vendor_micron *micron = (void *)p->onfi->vendor;
>  
>  		chip->read_retries = micron->read_retry_options;
> -		chip->setup_read_retry = micron_nand_setup_read_retry;
> +		chip->ops.setup_read_retry = micron_nand_setup_read_retry;
>  	}
>  
>  	if (p->supports_set_get_features) {
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 7d62e0e719ac..b33cd68852c4 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1027,16 +1027,31 @@ struct nand_legacy {
>  	struct nand_controller dummy_controller;
>  };
>  
> +/**
> + * struct nand_chip_ops - NAND chip operations
> + * @suspend: Suspend operation
> + * @resume: Resume operation
> + * @lock_area: Lock operation
> + * @unlock_area: Unlock operation
> + * @setup_read_retry: Set the read-retry mode (mostly needed for MLC NANDs)
> + */
> +struct nand_chip_ops {
> +	int (*suspend)(struct nand_chip *chip);
> +	void (*resume)(struct nand_chip *chip);
> +	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> +	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> +	int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
> +};
> +
>  /**
>   * struct nand_chip - NAND Private Flash Chip Data
>   * @base:		Inherit from the generic NAND device
> + * @ops:		NAND chip operations
>   * @legacy:		All legacy fields/hooks. If you develop a new driver,
>   *			don't even try to use any of these fields/hooks, and if
>   *			you're modifying an existing driver that is using those
>   *			fields/hooks, you should consider reworking the driver
>   *			avoid using them.
> - * @setup_read_retry:	[FLASHSPECIFIC] flash (vendor) specific function for
> - *			setting the read-retry mode. Mostly needed for MLC NAND.
>   * @ecc:		[BOARDSPECIFIC] ECC control structure
>   * @buf_align:		minimum buffer alignment required by a platform
>   * @oob_poi:		"poison value buffer," used for laying out OOB data
> @@ -1081,8 +1096,6 @@ struct nand_legacy {
>   * @lock:		lock protecting the suspended field. Also used to
>   *			serialize accesses to the NAND device.
>   * @suspended:		set to 1 when the device is suspended, 0 when it's not.
> - * @suspend:		[REPLACEABLE] specific NAND device suspend operation
> - * @resume:		[REPLACEABLE] specific NAND device resume operation
>   * @bbt:		[INTERN] bad block table pointer
>   * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
>   *			lookup.
> @@ -1096,17 +1109,13 @@ struct nand_legacy {
>   * @manufacturer:	[INTERN] Contains manufacturer information
>   * @manufacturer.desc:	[INTERN] Contains manufacturer's description
>   * @manufacturer.priv:	[INTERN] Contains manufacturer private information
> - * @lock_area:		[REPLACEABLE] specific NAND chip lock operation
> - * @unlock_area:	[REPLACEABLE] specific NAND chip unlock operation
>   */
>  
>  struct nand_chip {
>  	struct nand_device base;
> -
> +	struct nand_chip_ops ops;
>  	struct nand_legacy legacy;
>  
> -	int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
> -
>  	unsigned int options;
>  	unsigned int bbt_options;
>  
> @@ -1138,8 +1147,6 @@ struct nand_chip {
>  
>  	struct mutex lock;
>  	unsigned int suspended : 1;
> -	int (*suspend)(struct nand_chip *chip);
> -	void (*resume)(struct nand_chip *chip);
>  
>  	u8 *oob_poi;
>  	struct nand_controller *controller;
> @@ -1159,9 +1166,6 @@ struct nand_chip {
>  		const struct nand_manufacturer *desc;
>  		void *priv;
>  	} manufacturer;
> -
> -	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> -	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>  };
>  
>  extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;


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