Hi Sergei,


> > +static void rpc_spi_mem_set_prep_op_cfg(struct spi_device *spi,
> > +               const struct spi_mem_op *op,
> > +               u64 *offs, size_t *len)
> > +{
> > +   struct rpc_spi *rpc = spi_controller_get_devdata(spi->controller);
> > +
> > +   rpc->cmd = RPC_SMCMR_CMD(op->cmd.opcode);
> > +   rpc->smenr = RPC_SMENR_CDE |
> > +           RPC_SMENR_CDB(ilog2(op->cmd.buswidth));
> > +   rpc->totalxferlen = 1;
> > +   rpc->xfer_dir = SPI_MEM_NO_DATA;
> > +   rpc->xferlen = 0;
> > +   rpc->addr = 0;
> > +
> > +   if (op->addr.nbytes) {
> > +      rpc->smenr |= RPC_SMENR_ADB(ilog2(op->addr.buswidth));
> > +      if (op->addr.nbytes == 4)
> > +         rpc->smenr |= RPC_SMENR_ADE(0xf);
> > +      else
> > +         rpc->smenr |= RPC_SMENR_ADE(0x7);
> > +
> > +      if (offs && len)
> > +         rpc->addr = *offs;
> > +      else
> > +         rpc->addr = op->addr.val;
> > +      rpc->totalxferlen += op->addr.nbytes;
> > +   }
> > +
> > +   if (op->dummy.nbytes) {
> > +      rpc->smenr |= RPC_SMENR_DME;
> > +      rpc->dummy = RPC_SMDMCR_DMCYC(op->dummy.nbytes);
>
>    So you haven't fixed this bug? I repeat, the driver doesn't work right
> w/o this fixed!

Do you mean

rpc->dummy = RPC_SMDMCR_DMCYC(op->dummy.nbytes) * 8;   ?

What is your flash part number?

The problem you found is in 1 bit or 4 bits width ?

>
> [...]
> > +static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
> > +                   u64 offs, size_t len, void *buf)
> > +{
> > +   struct rpc_spi *rpc =
> > +         spi_controller_get_devdata(desc->mem->spi->controller);
> > +   int ret;
> > +
> > +   if (offs + desc->info.offset + len > U32_MAX)
> > +      return -EINVAL;
> > +
> > +   if (len > 0x4000000)
> > +      len = 0x4000000;
>
>    Ugh...

by mtd->size ?

>
> > +
> > +   ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
> > +   if (ret)
> > +      return ret;
> > +
> > +   rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
> > +                &desc->info.op_tmpl, &offs, &len);
> > +
> > +   regmap_update_bits(rpc->regmap, RPC_CMNCR, RPC_CMNCR_MD, 0);
> > +   regmap_write(rpc->regmap, RPC_DRCR, RPC_DRCR_RBURST(32) |
> > +           RPC_DRCR_RBE);
> > +
> > +   regmap_write(rpc->regmap, RPC_DRCMR, rpc->cmd);
> > +   regmap_write(rpc->regmap, RPC_DREAR, RPC_DREAR_EAC(1));
>
>    So you're not even trying to support flashes larger than the read dirmap?
> Now I don't think it's acceptable (and I have rewritten this code internally).

what about the size comes form mtd->size ?

>
> [...]
> > +static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
> > +               u64 offs, size_t len, const void *buf)
> > +{
> > +   struct rpc_spi *rpc =
> > +         spi_controller_get_devdata(desc->mem->spi->controller);
> > +   int ret;
> > +
> > +   if (offs + desc->info.offset + len > U32_MAX)
> > +      return -EINVAL;
> > +
> > +   if (len > RPC_WBUF_SIZE)
> > +      len = RPC_WBUF_SIZE;
>
>    Ugh! Again, I no longer think this is acceptable... Maybe we just need
> to drop the support of the write buffer...
>

why ?
Using write buffer got the much better write performance.

> [...]
> > +static void rpc_spi_transfer_setup(struct rpc_spi *rpc,
> > +               struct spi_message *msg)
> > +{
> [...]
> > +   if (xfercnt > 2 && xfer[1].len && xfer[1].tx_buf) {
> > +      rpc->smenr |=
> > +         RPC_SMENR_ADB(ilog2((unsigned int)xfer[1].tx_nbits));
> > +
> > +      for (i = 0; i < xfer[1].len; i++)
> > +         rpc->addr |= ((u8 *)xfer[1].tx_buf)[i] <<
> > +                 (8 * (xfer[1].len - i - 1));
> > +
> > +      if (xfer[1].len == 4)
> > +         rpc->smenr |= RPC_SMENR_ADE(0xf);
> > +      else
> > +         rpc->smenr |= RPC_SMENR_ADE(0x7);
> > +   }
> > +
> > +   if (xfercnt > 3 && xfer[2].len && xfer[2].tx_buf) {
> > +      rpc->smenr |= RPC_SMENR_DME;
> > +      rpc->dummy = RPC_SMDMCR_DMCYC(xfer[2].len);
>
>    Needs to be multiplied by 8 (or other value) as well...

Same above you mentioned:

rpc->dummy = RPC_SMDMCR_DMCYC(xfer[2].len) * 8;    


best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================