[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