From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Binbin Zhou <zhoubinbin@loongson.cn>
Cc: Binbin Zhou <zhoubb.aaron@gmail.com>,  Huacai Chen
 <chenhuacai@loongson.cn>,  Rob Herring <robh+dt@kernel.org>,  Krzysztof
 Kozlowski <krzk+dt@kernel.org>,  Conor Dooley <conor+dt@kernel.org>,
  Keguang Zhang <keguang.zhang@gmail.com>,  Richard Weinberger
 <richard@nod.at>,  Vignesh Raghavendra <vigneshr@ti.com>,  Huacai Chen
 <chenhuacai@kernel.org>,  Xuerui Wang <kernel@xen0n.name>,
  loongarch@lists.linux.dev,  devicetree@vger.kernel.org,
  linux-mtd@lists.infradead.org
Subject: Re: [PATCH v3 5/7] mtd: rawnand: loongson: Add Loongson-2K0500 NAND
 controller support
In-Reply-To: <589b4dfab9c1edd95fa05cba80ec58ea9ca099da.1755757841.git.zhoubinbin@loongson.cn>
	(Binbin Zhou's message of "Thu, 21 Aug 2025 14:39:19 +0800")
References: <cover.1755757841.git.zhoubinbin@loongson.cn>
	<589b4dfab9c1edd95fa05cba80ec58ea9ca099da.1755757841.git.zhoubinbin@loongson.cn>
User-Agent: mu4e 1.12.7; emacs 30.1
Date: Sun, 24 Aug 2025 17:26:29 +0200
Message-ID: <87ms7olp0a.fsf@bootlin.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Hi Binbin,

> +#define LOONGSON_NAND_64BIT_DMA		BIT(0)
> +
>  #define BITS_PER_WORD			(4 * BITS_PER_BYTE)
>=20=20
>  struct loongson_nand_host;
> @@ -83,6 +104,7 @@ struct loongson_nand_data {
>  	unsigned int hold_cycle;
>  	unsigned int wait_cycle;
>  	unsigned int nand_cs;
> +	unsigned int flags;

Can we turn this into a boolean instead and give it a DMA related name?
I'm not sure the list of flags will extend rapidly...

>  	void (*set_addr)(struct loongson_nand_host *host, struct loongson_nand_=
op *op);
>  };
>=20=20
> @@ -745,7 +767,7 @@ static int loongson_nand_controller_init(struct loong=
son_nand_host *host)
>  	struct device *dev =3D host->dev;
>  	struct dma_chan *chan;
>  	struct dma_slave_config cfg =3D {};
> -	int ret;
> +	int ret, val;
>=20=20
>  	host->regmap =3D devm_regmap_init_mmio(dev, host->reg_base, &loongson_n=
and_regmap_config);
>  	if (IS_ERR(host->regmap))
> @@ -755,6 +777,9 @@ static int loongson_nand_controller_init(struct loong=
son_nand_host *host)
>  		regmap_update_bits(host->regmap, LOONGSON_NAND_PARAM, host->data->id_c=
ycle_field,
>  				   host->data->max_id_cycle << __ffs(host->data->id_cycle_field));
>=20=20
> +	if (host->data->flags & LOONGSON_NAND_64BIT_DMA)
> +		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +
>  	chan =3D dma_request_chan(dev, "rxtx");
>  	if (IS_ERR(chan))
>  		return dev_err_probe(dev, PTR_ERR(chan), "failed to request DMA channe=
l\n");
> @@ -770,7 +795,14 @@ static int loongson_nand_controller_init(struct loon=
gson_nand_host *host)
>=20=20
>  	init_completion(&host->dma_complete);
>=20=20
> -	return 0;
> +	val =3D FIELD_PREP(LOONGSON_NAND_MAP_CS1_SEL, LOONGSON_NAND_CS_SEL1)
> +	    | FIELD_PREP(LOONGSON_NAND_MAP_RDY1_SEL, LOONGSON_NAND_CS_RDY1)
> +	    | FIELD_PREP(LOONGSON_NAND_MAP_CS2_SEL, LOONGSON_NAND_CS_SEL2)
> +	    | FIELD_PREP(LOONGSON_NAND_MAP_RDY2_SEL, LOONGSON_NAND_CS_RDY2)
> +	    | FIELD_PREP(LOONGSON_NAND_MAP_CS3_SEL, LOONGSON_NAND_CS_SEL3)
> +	    | FIELD_PREP(LOONGSON_NAND_MAP_RDY3_SEL, LOONGSON_NAND_CS_RDY3);

Just a nit about the style, '|' should be last character on the previous
line.

Otherwise the rest lgtm.

Thanks,
Miqu=C3=A8l