Re: [PATCH v3 5/7] mtd: rawnand: loongson: Add Loongson-2K0500 NAND controller support

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
Date: Sun, 24 Aug 2025 17:26:29 +0200	[thread overview]
Message-ID: <87ms7olp0a.fsf@bootlin.com> (raw)
In-Reply-To: <589b4dfab9c1edd95fa05cba80ec58ea9ca099da.1755757841.git.zhoubinbin@loongson.cn> (Binbin Zhou's message of "Thu, 21 Aug 2025 14:39:19 +0800")

Hi Binbin,

> +#define LOONGSON_NAND_64BIT_DMA		BIT(0)
> +
>  #define BITS_PER_WORD			(4 * BITS_PER_BYTE)
>  
>  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);
>  };
>  
> @@ -745,7 +767,7 @@ static int loongson_nand_controller_init(struct loongson_nand_host *host)
>  	struct device *dev = host->dev;
>  	struct dma_chan *chan;
>  	struct dma_slave_config cfg = {};
> -	int ret;
> +	int ret, val;
>  
>  	host->regmap = devm_regmap_init_mmio(dev, host->reg_base, &loongson_nand_regmap_config);
>  	if (IS_ERR(host->regmap))
> @@ -755,6 +777,9 @@ static int loongson_nand_controller_init(struct loongson_nand_host *host)
>  		regmap_update_bits(host->regmap, LOONGSON_NAND_PARAM, host->data->id_cycle_field,
>  				   host->data->max_id_cycle << __ffs(host->data->id_cycle_field));
>  
> +	if (host->data->flags & LOONGSON_NAND_64BIT_DMA)
> +		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +
>  	chan = dma_request_chan(dev, "rxtx");
>  	if (IS_ERR(chan))
>  		return dev_err_probe(dev, PTR_ERR(chan), "failed to request DMA channel\n");
> @@ -770,7 +795,14 @@ static int loongson_nand_controller_init(struct loongson_nand_host *host)
>  
>  	init_completion(&host->dma_complete);
>  
> -	return 0;
> +	val = 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èl

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