[AMD Official Use Only - General]
Hi Dan,
> -----Original Message-----
> From: Dan Carpenter <dan.carpen...@linaro.org>
> Sent: Wednesday, March 13, 2024 2:28 PM
> To: Bhumkar, Tejas Arvind <tejas.arvind.bhum...@amd.com>
> Cc: u-boot@lists.denx.de; ja...@amarulasolutions.com; n-fran...@ti.com;
> Simek, Michal <michal.si...@amd.com>; Abbarapu, Venkatesh
> <venkatesh.abbar...@amd.com>; git (AMD-Xilinx) <g...@amd.com>; T
> Karthik Reddy <t.karthik.re...@xilinx.com>; Ashok Reddy Soma
> <ashok.reddy.s...@amd.com>
> Subject: Re: [PATCH 01/19] spi: cadence_qspi: Add support for DDR PHY mode
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> > diff --git a/drivers/mtd/spi/spi-nor-core.c
> > b/drivers/mtd/spi/spi-nor-core.c index faf02c7778..5895b5de09 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -1511,8 +1511,10 @@ static const struct flash_info
> *spi_nor_read_id(struct spi_nor *nor)
> > info = spi_nor_ids;
> > for (; info->name; info++) {
> > if (info->id_len) {
> > - if (!memcmp(info->id, id, info->id_len))
> > + if ((!memcmp(info->id, id, info->id_len)) &&
> > + memcpy(nor->spi->device_id, id,
> > + SPI_NOR_MAX_ID_LEN)) {
>
> Please, don't put a memcpy() into a condition. It looks like a memcmp() to
> the
> eye.
>
> > return info;
> > + }
>
> if (!memcmp(info->id, id, info->id_len)) {
> memcpy(nor->spi->device_id, id, SPI_NOR_MAX_ID_LEN);
> return info;
> }
>
> > }
> > }
> >
>
> [ snip ]
[Tejas ] : Thanks for the review, I will update this in v2
>
> > static int cadence_spi_mem_exec_op(struct spi_slave *spi,
> > const struct spi_mem_op *op) { @@
> > -353,6 +649,9 @@ static int cadence_spi_mem_exec_op(struct spi_slave
> *spi,
> > break;
> > }
> >
> > + if (op->cmd.dtr)
> > + err = cadence_spi_setup_ddrmode(spi, op);
> > +
> > return err;
>
>
> I think there should be another if (err) return err after the end of the
> switch
> statement.
[Tejas ] : I will update this too in v2.
>
> > }
> >
>
> regards,
> dan carpenter
