[PATCH] Added set feature command in FSL IFC nand controller driver for ONFI nand
Ronak Desai
ronak.desai at rockwellcollins.com
Mon Apr 23 12:55:48 PDT 2018
On Sun, Apr 22, 2018 at 12:07 PM, Miquel Raynal
<miquel.raynal at bootlin.com> wrote:
> Hi Ronak,
>
> On Tue, 17 Apr 2018 14:10:59 -0500, Ronak Desai
> <ronak.desai at rockwellcollins.com> wrote:
>
>> This patch adds set feature command (EFh) support in Freescale IFC nand
>> controller driver.
>>
>> The SET FEATURES (EFh) command is used to modify the target's default
>> power-on behavior. This command uses one-byte feature address to
>> determine which sub-feature parameters will be modified.
>>
>
> Could you rebase on top of 4.17-rc1 at least, this file has moved and
> the core changed about set/get_features() handling.
>
> Also while you are at it, the prefix should be
> 'mtd: rawnand: fsl_ifc: '.
>
Agree, I will do that.
>> Signed-off-by: Ronak Desai <ronak.desai at rockwellcollins.com>
>> ---
>> drivers/mtd/nand/fsl_ifc_nand.c | 51 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
>> index af85c4b..20b97ef 100644
>> --- a/drivers/mtd/nand/fsl_ifc_nand.c
>> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
>> @@ -613,6 +613,23 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned int command,
>> fsl_ifc_run_command(mtd);
>> return;
>>
>> + case NAND_CMD_SET_FEATURES: {
>> + ifc_out32((IFC_FIR_OP_CW0 << IFC_NAND_FIR0_OP0_SHIFT) |
>> + (IFC_FIR_OP_UA << IFC_NAND_FIR0_OP1_SHIFT) |
>> + (IFC_FIR_OP_WBCD << IFC_NAND_FIR0_OP2_SHIFT),
>> + &ifc->ifc_nand.nand_fir0);
>> +
>> + ifc_out32((NAND_CMD_SET_FEATURES << IFC_NAND_FCR0_CMD0_SHIFT),
>> + &ifc->ifc_nand.nand_fcr0);
>> +
>> + ifc_out32(column, &ifc->ifc_nand.row3);
>> +
>> + /* Write only 4 bytes from flash buffer */
>> + ifc_out32(4, &ifc->ifc_nand.nand_fbcr);
>> + fsl_ifc_run_command(mtd);
>> + return;
>> + }
>> +
>> case NAND_CMD_RNDOUT: {
>> __le16 Tccs = 0;
>> chip->onfi_version ? (Tccs = chip->onfi_params.t_ccs)
>> @@ -905,6 +922,39 @@ static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv)
>> ifc_out32(csor_ext, &ifc_global->csor_cs[cs].csor_ext);
>> }
>>
>> +/**
>> + * fsl_ifc_onfi_set_features- 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 fsl_ifc_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
>> + int addr, uint8_t *subfeature_param)
>> +{
>> + int status;
>> + int i;
>> +
>> +#ifdef CONFIG_SYS_NAND_ONFI_DETECTION
>> + if (!chip->onfi_version ||
>> + !(le16_to_cpu(chip->onfi_params.opt_cmd)
>> + & ONFI_OPT_CMD_SET_GET_FEATURES))
>> + return -EINVAL;
>> +#endif
>
> No need to do that, the core will take care of it, see [1].
Agree, I will remove this.
>
>> +
>> + /* Want data from start of the buffer */
>> + set_addr(mtd, 0, 0, 0);
>
> This is the only thing that differs from the core's implementation. I
> see most of the time it is called from ->cmdfunc(), could you move it
> there? If yes you could get rid of this entire hook and rely on the
> core's function.
>
NAND_CMD_SET_FEATURES command sends the sub-feature param from flash
buffer on the provided address and I added this here because I wanted
to make sure
that the data is set from start of the buffer. I looked at the core's
implementation
and I see that the NAND_CMD_SET_FEATURES command is called first and then
the data is filled into the buffer which will not work in case of my
implementation.
So, I will have to keep this here and the function, please suggest. I
will wait for your
feedback before submitting V2. Moreover, I would suggest to check
sequence in core's
implementation as the command should run after setting the data and not before.
>> +
>> + for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
>> + chip->write_byte(mtd, subfeature_param[i]);
>> +
>> + chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, 0);
>> +
>> + status = chip->waitfunc(mtd, chip);
>> + if (status & NAND_STATUS_FAIL)
>> + return -EIO;
>> + return 0;
>> +}
>> static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
>> {
>> struct fsl_ifc_ctrl *ctrl = priv->ctrl;
>> @@ -932,6 +982,7 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
>> chip->select_chip = fsl_ifc_select_chip;
>> chip->cmdfunc = fsl_ifc_cmdfunc;
>> chip->waitfunc = fsl_ifc_wait;
>> + chip->onfi_set_features = fsl_ifc_onfi_set_features;
>
> Should be chip->set_features now. And fsl_ifc_onfi_set_features() could
> become fsl_ifc_set_features().
>
> If the driver does not support get_features, you should add a:
>
> chip->get_features = nand_get_set_features_notsupp;
>
Agree, I will update this.
>>
>> chip->bbt_td = &bbt_main_descr;
>> chip->bbt_md = &bbt_mirror_descr;
>> --
>> 1.7.9.5
>>
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
> [1]
> https://elixir.bootlin.com/linux/v4.17-rc1/source/drivers/mtd/nand/raw/nand_base.c#L1204
>
> Thanks,
> Miquèl
>
> --
> Miquel Raynal, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
--
Ronak A Desai
Sr. Software Engineer
Airborne Information Solutions / RC Linux Platform Software
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
Phone : 319-263-2667 / VPN : 263-2667
Ronak.Desai at rockwellcollins.com
https://www.rockwellcollins.com/
More information about the linux-mtd
mailing list