Received-SPF: pass (google.com: domain of linux-kernel+bounces-182598-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 17 May 2024 15:56:13 -0400
From: Frank Li <Frank.li@nxp.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Han Xu <han.xu@nxp.com>,
	Vinod Koul <vkoul@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>, Marek Vasut <marex@denx.de>,
	linux-mtd@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/5] mtd: rawnand: gpmi: add iMX8QXP support.
Message-ID: <Zke2XTKqgKR4sEoY@lizhi-Precision-Tower-5810>
References: <20240517-gpmi_nand-v1-0-73bb8d2cd441@nxp.com>
 <20240517-gpmi_nand-v1-3-73bb8d2cd441@nxp.com>
 <20240517203904.2879419a@xps-13>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240517203904.2879419a@xps-13>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?iso-8859-1?Q?7gKvXYJWm9QApjTBqtxTWObwakUT3GDh+eaj7T3WjZP31jwK/lVPam61kO?=
 =?iso-8859-1?Q?fYD4HhDBZS6Cz2Fmc4zM0r8iCAN28n1aTP8YiT+XulGAxiz9EQjJI1EiaA?=
 =?iso-8859-1?Q?sLtznnQ4sFzmAQI6VW87Q3hUE4W3O70o3cJ4YssdU75DhexryOduqTrugl?=
 =?iso-8859-1?Q?cM5r3Cfjoen0u1jKCT77M0kTlw9GzAg1n/UR7CMIqxzfbIr6osA3SQ8Yqd?=
 =?iso-8859-1?Q?KVotxAIlRxjdgJf0crNCUZPr931NFIVzGVSg3VTsVpiU4VM9MMhRkJE/oU?=
 =?iso-8859-1?Q?1n5fseAbQrvC3hFAk3vB0VzhsD5jqu+hVbYnuZ6UpTYZLqdVY7HHSJOQut?=
 =?iso-8859-1?Q?e87mdWTCB1J+m7JJcmumGiszbWTaVjLOwSgmDAXNFoIkdMqeWFjSVRu7/J?=
 =?iso-8859-1?Q?NMtJeSeCY58lNUjVraTEumNwq69B4OEDs235NtwILy/ng6JlXojfn+tJbf?=
 =?iso-8859-1?Q?F9WNt9H0DrkApXL5wCcBVOiNFQcpy1v+2h9C69lGkeW3x84Pf3kCfTlaIl?=
 =?iso-8859-1?Q?xCJiXYvcYRuF0J2X/jcKMxfNIptVBd0JLyyU4VJ8Tsi29EK6gGbTRTqQtP?=
 =?iso-8859-1?Q?6eoYzdWC+uHy8Ou1TZHzsjLTfwASW4Tu5Y+7IFaR5NO7Tt2d9P75HksxaE?=
 =?iso-8859-1?Q?j7cfNqsjSQkZxQeodt5llC/MdpE7NmDhAx8eTMxPDVJeS5SzaRF8p3O8st?=
 =?iso-8859-1?Q?PcfH4Q8ESoXnjcMHLZKqd0kDdVACYh1o7IJxajitGxxc0kYsi6+dGWlEJL?=
 =?iso-8859-1?Q?jtI7LHY23KOO87MvWJIicwT+bvc4g/QB5LzThA12q4n4/CmAFOqD7hVPRm?=
 =?iso-8859-1?Q?OieX1kWi/OoQjxRCAxOwiG2CBkblhppW4s/n4DVsyDqmhWJsNj1hWNWadc?=
 =?iso-8859-1?Q?flRTl4wZ9R6C/Xm2/PcMH5yCl9Wk2VjAXabAWSVVFhZ5M6y7zr7PyTspO2?=
 =?iso-8859-1?Q?dVatBLmEtpnMKQ8wIAK4UU5/u10ACnKKasnP3lrxyDTELJ+4NnHP4myQnp?=
 =?iso-8859-1?Q?xQGnDtOEUg1BGkQVgglwcO8qmFvSz41Tbmx7Tx1zX1SpzOJ3N7Z/bBea5g?=
 =?iso-8859-1?Q?7JmKSZJ97tG2BUajrEPT8GhdyDgBe/VWebOAlA7ccx64je4xdoqwmLSkQg?=
 =?iso-8859-1?Q?zUSCkwI7fjv/w1j+5mDXPFFF1pHv5IyqDpWFrhw+FOj6Q7wBwHP3TeIXgW?=
 =?iso-8859-1?Q?jVVjMVDuY08B8nu8c63L4Owf0R1yAnbwxcvTEDnkt79F0mMHa4DXcjTMBy?=
 =?iso-8859-1?Q?eo1yloMTs/u618bmLgk1K2qHJwfgd1+I9TtDCAXTL82I0VhJnGdOnKpR5d?=
 =?iso-8859-1?Q?HRXHuSeDQVku7UX9Rvv5t74NLuzktlu8JCZMKgIrZIOJXcX+OKtB0Biu72?=
 =?iso-8859-1?Q?9BJR9kyYdBAOFbLLImXL88ibXi4+bD74MRam/l2sKPcWZtDxnnoOzWFIH8?=
 =?iso-8859-1?Q?3EoVP8OfR+x/7nCrZcmjD8YFTY7FzX/x/+Zo8wfT8MYBJC8csq5Zp0/0sF?=
 =?iso-8859-1?Q?AG3YPaOIfBzvzKVh0JXUVGcaHcU3Q6C7rlRzV1cumRi0myOUx9OqPjQQGM?=
 =?iso-8859-1?Q?CTUGd9MPhHu7uwaPnl85fj0ovM/t3BVFlbpotgKCGqfZ1kW5stGrvcx+bM?=
 =?iso-8859-1?Q?iXjyVh+skSzwXfPmi4lFKCwLM806oOpub9?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1349dfe4-6ce8-4bf6-307c-08dc76ab6bc1
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB9642.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2024 19:56:20.1538
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JlVD8VdnJnPnMoHIo6qAhw+jOpjyCbyED5g9/ABhH9BNygc/RC4+8n9pA9m4ZzVr1BEPwpIuhfknLpFXS6U69Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB8336

On Fri, May 17, 2024 at 08:39:04PM +0200, Miquel Raynal wrote:
> Hi Frank,
> 
> Frank.Li@nxp.com wrote on Fri, 17 May 2024 14:09:50 -0400:
> 
> > From: Han Xu <han.xu@nxp.com>
> > 
> > Add "fsl,imx8qxp-gpmi-nand" compatible string. iMX8QXP gpmi nand is similar
> > with iMX7D. But it using 4 clock: "gpmi_io", "gpmi_apb", "gpmi_bch" and
> 
>   to?             is         clocks
> 
> > "gpmi_bch_apb".
> > 
> > Signed-off-by: Han Xu <han.xu@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 20 +++++++++++++++++---
> >  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h |  4 ++++
> >  2 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > index e71ad2fcec232..f90c5207bacb6 100644
> > --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > @@ -983,7 +983,8 @@ static int gpmi_setup_interface(struct nand_chip *chip, int chipnr,
> >  		return PTR_ERR(sdr);
> >  
> >  	/* Only MX28/MX6 GPMI controller can reach EDO timings */
> > -	if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !GPMI_IS_MX6(this))
> > +	if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) &&
> > +	    !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)))
> 
> Feels completely redundant, no? If it's not an imx6 nor an imx28, it
> already returns -ENOTSUPP.

if this is mx8

	sdr->tRC_min <= 25000:   true
        !GPMI_IS_MX28(this):     true
        !GPMI_IS_MX6(this):      true

        so whole statement is true

after change
       sdr->tRC_min <= 25000:   true                                      
       !GPMI_IS_MX28(this):     true                                      
       !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)): false
        (GPMI_IS_MX6(this) || GPMI_IS_MX8(this)): true
            GPMI_IS_MX6(this):   false
            GPMI_IS_MX8(this):   true

so whole statement is false. Maybe use 

sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !(GPMI_IS_MX6(this) &&
!(GPMI_IS_MX8(this))

looks better.

Or use a drvflag. 
     if (sdr->tRC_min <= 25000 && (!this->drvflag->support_tRC_min))

Frank

> 
> >  		return -ENOTSUPP;
> 
> 
> Fine otherwise.
> 
> Thanks,
> Miqu?l