Re: [PATCH] mtd: rawnand: cadence: Add support for NV-DDR interface mode

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

Hi Niravkumar,

On 24/10/2025 at 15:13:06 +08, niravkumarlaxmidas.rabara@altera.com wrote:

> From: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
>
> Add support for NV-DDR mode in the Cadence NAND controller driver.
>
> Signed-off-by: Niravkumar L Rabara <niravkumarlaxmidas.rabara@altera.com>
> ---

Thanks for the patch, very happy to see people implementing this
interface!

[...]

> +	if (dll_phy_gate_open_delay > NVDDR_GATE_CFG_MIN)
> +		ie_start = NVDDR_GATE_CFG_MIN;

Can you double check here? I would expect < instead of > given that you
compare with something you named "minimum". Maybe it is legitimate, just
warning.

> +	else
> +		ie_start = dll_phy_gate_open_delay;
> +
> +	dll_phy_rd_delay = ((nvddr->tDQSCK_max + board_delay) +
> +				(clk_period / 2)) / clk_period;
> +	if (dll_phy_rd_delay <= NVDDR_PHY_RD_DELAY)
> +		rd_del_sel = dll_phy_rd_delay + 2;
> +	else
> +		rd_del_sel = NVDDR_PHY_RD_DELAY_MAX;
> +

[...]

> +static int
> +cadence_nand_setup_interface(struct nand_chip *chip, int chipnr,
> +			     const struct nand_interface_config *conf)
> +{
> +	int ret = 0;
> +
> +	if (nand_interface_is_sdr(conf)) {
> +		const struct nand_sdr_timings *sdr = nand_get_sdr_timings(conf);
> +
> +		if (IS_ERR(sdr))
> +			return PTR_ERR(sdr);
> +
> +		ret = cadence_nand_setup_sdr_interface(chip, sdr);
> +	} else if (chipnr >= 0) {

This isn't very clear. Please make it a separate condition if you think
you must handle this case. Otherwise you're mixing it with the SDR
vs. NVDDR choice, and that's misleading.

> +		const struct nand_nvddr_timings *nvddr = nand_get_nvddr_timings(conf);
> +
> +		if (IS_ERR(nvddr))
> +			return PTR_ERR(nvddr);
> +
> +		ret = cadence_nand_setup_nvddr_interface(chip, nvddr);
> +	}
> +	return ret;
> +}
> +
>  static int cadence_nand_attach_chip(struct nand_chip *chip)
>  {
>  	struct cdns_nand_ctrl *cdns_ctrl = to_cdns_nand_ctrl(chip->controller);

Otherwise looks good to me!
Miquèl

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