[PATCH v3 5/7] mtd: rawnand: loongson: Add Loongson-2K0500 NAND controller support
Binbin Zhou
zhoubb.aaron at gmail.com
Mon Aug 25 00:21:37 PDT 2025
Hi Miquel:
Thanks for your review.
On Sun, Aug 24, 2025 at 11:26 PM Miquel Raynal
<miquel.raynal at bootlin.com> wrote:
>
> 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...
Keguang has a different opinion on this part [1]. Since his reply
email and my V3 patchset email were sent at about the same time, I
plan to address these comments in the V4 patchset.
He found that the Loongson-1 series NAND controller also needs
dma_set_mask_and_coherent(), so we still need to explicitly set the
value of dma_bits.
Specifically:
The flags variable will be split into:
```
unsigned int dma_bits;
int (*dma_config)(struct device *dev);
```
[1]: https://lore.kernel.org/all/CAJhJPsW3KGL=FH5ZVqvRr-RD+ih09mFNuk7wGXepGVSw7bTaiw@mail.gmail.com/
>
> > 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.
OK. I will do it.
>
> Otherwise the rest lgtm.
>
> Thanks,
> Miquèl
--
Thanks.
Binbin
More information about the linux-mtd
mailing list