Re: [PATCH v2 1/6] memory: ti-aemif: Create aemif_set_cs_timings()

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Bastien Curutchet <bastien.curutchet@bootlin.com>
Cc: Santosh Shilimkar <ssantosh@kernel.org>,
	 Krzysztof Kozlowski <krzk@kernel.org>,
	 Richard Weinberger <richard@nod.at>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	 linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	 Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	 Herve Codina <herve.codina@bootlin.com>,
	Christopher Cordahi <christophercordahi@nanometrics.ca>
Subject: Re: [PATCH v2 1/6] memory: ti-aemif: Create aemif_set_cs_timings()
Date: Mon, 11 Nov 2024 20:16:34 +0100	[thread overview]
Message-ID: <8734jxshrh.fsf@bootlin.com> (raw)
In-Reply-To: <20241106085507.76425-2-bastien.curutchet@bootlin.com> (Bastien Curutchet's message of "Wed, 6 Nov 2024 09:55:02 +0100")

Hi Bastien,

2 very minor nits

> +/**
> + * struct aemif_cs_timings: structure to hold cs timing configuration

Nitpick: s/cs/CS/ in plain english?

...

> +/**
> + * aemif_set_cs_timings - Set the timing configuration of a given chip select.
> + * @aemif: aemif device to configure
> + * @cs: index of the chip select to configure.

Nit: Spurious dot                               ^

...

> -	/* ... But configuring tighter timings is not an option. */
> -	else if (result > max)
> -		result = -EINVAL;
> -

Maybe this is too early to remove the check? If someone bisects and
falls in-between the patches of your series, NAND support would be
broken I guess?

With this fixed, feel free to add my:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

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