Re: [PATCH v5 2/3] mtd: spi-nor: Move m25p80 code in spi-nor.c

[Boris Brezillon]

On Thu, 8 Aug 2019 17:44:35 +0300
Tomer Maimon <tmaimon77@xxxxxxxxx> wrote:

> Hi Boris,
> 
> On Thu, 8 Aug 2019 at 17:04, Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> wrote:
> 
> > Hi Tomer,
> >
> > On Thu, 8 Aug 2019 13:05:14 +0300
> > Tomer Maimon <tmaimon77@xxxxxxxxx> wrote:
> >  
> > > @@ -688,6 +1003,16 @@ static int spi_nor_erase_sector(struct spi_nor  
> > *nor,  
> > > > u32 addr)
> > > >         if (nor->erase)
> > > >                 return nor->erase(nor, addr);
> > > >
> > > > +       if (nor->spimem) {
> > > > +               struct spi_mem_op op =
> > > > +                       SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode,  
> > 1),  
> > > > +                                  SPI_MEM_OP_ADDR(nor->addr_width,  
> > addr,  
> > > > 1),
> > > > +                                  SPI_MEM_OP_NO_DUMMY,
> > > > +                                  SPI_MEM_OP_NO_DATA);
> > > > +
> > > > +               return spi_mem_exec_op(nor->spimem, &op);
> > > > +       }
> > > > +
> > > >  
> > >
> > > static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
> > > {
> > >
> > >       int i;
> > >
> > >       if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
> > >               addr = spi_nor_s3an_addr_convert(nor, addr);
> > >
> > >       if (nor->erase)
> > >               return nor->erase(nor, addr);
> > >
> > >       /*
> > >        * Default implementation, if driver doesn't have a specialized HW
> > >        * control
> > >        */
> > >       for (i = nor->addr_width - 1; i >= 0; i--) {
> > >               nor->bouncebuf[i] = addr & 0xff;
> > >               addr >>= 8;
> > >       }
> > >
> > >       if (nor->spimem) {
> > >               struct spi_mem_op op =
> > >                       SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 1),
> > >                       SPI_MEM_OP_NO_ADDR,
> > >                       SPI_MEM_OP_NO_DUMMY,
> > >                       SPI_MEM_OP_DATA_OUT(nor->addr_width,  
> > nor->bouncebuf, 1));
> >
> > That's wrong. If you need that, that's probably a spi-mem controller
> > driver issue. Address cycles should be passed through the
> > spi_mem_op->addr field, not packed with the data cycles.
> >  
> 
> Maybe I missing something but this is not the way it
> working in the current spi-nor and m25p80 driver...
> in function  spi_nor_erase_sector it calling
> nor->write_reg(nor, nor->erase_opcode, nor->bouncebuf, nor->addr_width);
> 
> so it will call m25p80_write_reg and in this function the op sets as follow:
> 
>         struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1),
>                                           SPI_MEM_OP_NO_ADDR,
>                                           SPI_MEM_OP_NO_DUMMY,
>                                           SPI_MEM_OP_DATA_OUT(len, NULL, 1));
> 
> Do we like to modify it?

Yes. The spi-mem core is taking care of this conversion [1].

[1]https://elixir.bootlin.com/linux/v5.3-rc3/source/drivers/spi/spi-mem.c#L329