[RFC,v1,0/3] *spi-mem: adding setup and callback function

Message

Tomer Maimon July 29, 2019, 2:25 p.m. UTC

Lately we have working on Flash interface unit (FIU) SPI driver that 
using spi-mem interface, Our FIU HW module support direct Flash Rd//Wr.

In our SOC (32 bit dual core ARM) we have 3 FIU's that using memory mapping as follow:

FIU0 - have 2 chip select and each one have 128MB memory mapping (total 256MB memory mapping)
FIU1 - have 4 chip select and each one have 128MB memory mapping (total 512MB memory mapping)
FIU2 - have 4 chip select and each one have 16MB memory mapping (total 32MB memory mapping)

Totally 800MB memory mapping.

When the FIU driver probe it don't know the size of each Flash that 
connected to the FIU, so the entire memory mapping is allocated for each FIU 
according the FIU device tree memory map parameters.
It means, if we enable all three FIU's the drivers will try to allocate totally 800MB.

In 32bit system it is problematic because the kernel have only 1GB 
of memory allocation so the vmalloc cannot take 800MB.

When implementing the FIU driver in the mtd/spi-nor we allocating memory address only 
for detected Flash with exact size (usually we are not using 128MB Flash), and in that case usually we allocating much less memory.

To solve this issue we needed to overcome two things:

1.	Get argument from the upper layer (spi-mem layer) 
2.	Calling the get argument function after SPI_NOR_SCAN function. (the MTD Flash size filled in  SPI_NOR_SCAN function)

The attach patch set solving the describe issue by:

1.	Add spi-mem callback function and value to the SPI device 
	for passing an argument from the spi-mem layer to the spi layer
2.	Add spi-mem setup function to the spi-memory operation that running 
	after the spi-mem probe finished.
3.	Implement function callback in the m25p80 driver that execute 
	get Flash size.

The patch set tested on NPCM750 EVB with FIU driver (implemented with SPI-MEM interface).

Thanks for your attention.

Tomer

Tomer Maimon (3):
  spi: spi-mem: add spi-mem setup function
  spi: spi-mem: add callback function to spi-mem device
  mtd: m25p80: add get Flash size callback support

 drivers/mtd/devices/m25p80.c | 12 ++++++++++++
 drivers/spi/spi-mem.c        | 27 ++++++++++++++++++++++++++-
 include/linux/spi/spi-mem.h  | 11 +++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

Comments

Boris Brezillon July 29, 2019, 3:28 p.m. UTC | #1

Hi Tomer,

On Mon, 29 Jul 2019 17:25:01 +0300
Tomer Maimon <[e-mail]> wrote:

> Lately we have working on Flash interface unit (FIU) SPI driver that 
> using spi-mem interface, Our FIU HW module support direct Flash Rd//Wr.
> 
> In our SOC (32 bit dual core ARM) we have 3 FIU's that using memory mapping as follow:
> 
> FIU0 - have 2 chip select and each one have 128MB memory mapping (total 256MB memory mapping)
> FIU1 - have 4 chip select and each one have 128MB memory mapping (total 512MB memory mapping)
> FIU2 - have 4 chip select and each one have 16MB memory mapping (total 32MB memory mapping)
> 
> Totally 800MB memory mapping.
> 
> When the FIU driver probe it don't know the size of each Flash that 
> connected to the FIU, so the entire memory mapping is allocated for each FIU 
> according the FIU device tree memory map parameters.

Do you need those mappings to be active to support simple reg accesses?

> It means, if we enable all three FIU's the drivers will try to allocate totally 800MB.
> 
> In 32bit system it is problematic because the kernel have only 1GB 
> of memory allocation so the vmalloc cannot take 800MB.
> 
> When implementing the FIU driver in the mtd/spi-nor we allocating memory address only 
> for detected Flash with exact size (usually we are not using 128MB Flash), and in that case usually we allocating much less memory.
> 
> To solve this issue we needed to overcome two things:
> 
> 1.	Get argument from the upper layer (spi-mem layer) 
> 2.	Calling the get argument function after SPI_NOR_SCAN function. (the MTD Flash size filled in  SPI_NOR_SCAN function)

That's clearly breaking the layering we've tried to restore with the
spi-nor/spi-mem split, and I don't see why this is needed since we now
have a way to create direct mappings dynamically (with the dirmap API).
Have you tried implementing the dirmap hooks in your driver?

Regards,

Boris

Boris Brezillon July 30, 2019, 6:54 a.m. UTC | #2

Trimmed the recipient list a bit and used Frieder's new address.
+Sergey

On Mon, 29 Jul 2019 23:55:05 +0300
Tomer Maimon <[e-mail]> wrote:

> Hi Boris,
> 
> Thanks for the prompt reply,
> 
> 
> 
> On Mon, 29 Jul 2019 at 18:29, Boris Brezillon <[e-mail]>
> wrote:
> 
> > Hi Tomer,
> >
> > On Mon, 29 Jul 2019 17:25:01 +0300
> > Tomer Maimon <[e-mail]> wrote:
> >  
> > > Lately we have working on Flash interface unit (FIU) SPI driver that
> > > using spi-mem interface, Our FIU HW module support direct Flash Rd//Wr.
> > >
> > > In our SOC (32 bit dual core ARM) we have 3 FIU's that using memory  
> > mapping as follow:  
> > >
> > > FIU0 - have 2 chip select and each one have 128MB memory mapping (total  
> > 256MB memory mapping)  
> > > FIU1 - have 4 chip select and each one have 128MB memory mapping (total  
> > 512MB memory mapping)  
> > > FIU2 - have 4 chip select and each one have 16MB memory mapping (total  
> > 32MB memory mapping)  
> > >
> > > Totally 800MB memory mapping.
> > >
> > > When the FIU driver probe it don't know the size of each Flash that
> > > connected to the FIU, so the entire memory mapping is allocated for each  
> > FIU  
> > > according the FIU device tree memory map parameters.  
> >
> > Do you need those mappings to be active to support simple reg accesses?
> >  
> > > It means, if we enable all three FIU's the drivers will try to allocate  
> > totally 800MB.  
> > >
> > > In 32bit system it is problematic because the kernel have only 1GB
> > > of memory allocation so the vmalloc cannot take 800MB.
> > >
> > > When implementing the FIU driver in the mtd/spi-nor we allocating memory  
> > address only  
> > > for detected Flash with exact size (usually we are not using 128MB  
> > Flash), and in that case usually we allocating much less memory.  
> > >
> > > To solve this issue we needed to overcome two things:
> > >
> > > 1.    Get argument from the upper layer (spi-mem layer)
> > > 2.    Calling the get argument function after SPI_NOR_SCAN function.  
> > (the MTD Flash size filled in  SPI_NOR_SCAN function)
> >
> > That's clearly breaking the layering we've tried to restore with the
> > spi-nor/spi-mem split, and I don't see why this is needed since we now
> > have a way to create direct mappings dynamically (with the dirmap API).
> > Have you tried implementing the dirmap hooks in your driver?  
> 
> 
>  Sorry but I wasn't familiar with the direct mapping in the spi-mem, it
> seems it needed to implemented in the m25p80 driver as well, am I correct?

There's this patch [1] floating around. IIRC, Sergey was waiting for
the m25p80 -> spi-nor merge to send a v5. Vignesh, any updates on that
one? If you don't have time to work on that, maybe Sergey could send a
v5.

[1]https://www.spinics.net/lists/linux-mtd/msg07358.html

Sergei Shtylyov July 30, 2019, 4:53 p.m. UTC | #3

On 07/30/2019 09:54 AM, Boris Brezillon wrote:

> Trimmed the recipient list a bit and used Frieder's new address.
> +Sergey

  TY. :-)

> On Mon, 29 Jul 2019 23:55:05 +0300
> Tomer Maimon <[e-mail]> wrote:
> 
>> Hi Boris,
>>
>> Thanks for the prompt reply,
>>
>>
>>
>> On Mon, 29 Jul 2019 at 18:29, Boris Brezillon <[e-mail]>
>> wrote:
>>
>>> Hi Tomer,
>>>
>>> On Mon, 29 Jul 2019 17:25:01 +0300
>>> Tomer Maimon <[e-mail]> wrote:
>>>  
>>>> Lately we have working on Flash interface unit (FIU) SPI driver that
>>>> using spi-mem interface, Our FIU HW module support direct Flash Rd//Wr.
>>>>
>>>> In our SOC (32 bit dual core ARM) we have 3 FIU's that using memory  
>>> mapping as follow:  
>>>>
>>>> FIU0 - have 2 chip select and each one have 128MB memory mapping (total  
>>> 256MB memory mapping)  
>>>> FIU1 - have 4 chip select and each one have 128MB memory mapping (total  
>>> 512MB memory mapping)  
>>>> FIU2 - have 4 chip select and each one have 16MB memory mapping (total  
>>> 32MB memory mapping)  
>>>>
>>>> Totally 800MB memory mapping.
>>>>
>>>> When the FIU driver probe it don't know the size of each Flash that
>>>> connected to the FIU, so the entire memory mapping is allocated for each  
>>> FIU  
>>>> according the FIU device tree memory map parameters.  
>>>
>>> Do you need those mappings to be active to support simple reg accesses?
>>>  
>>>> It means, if we enable all three FIU's the drivers will try to allocate  
>>> totally 800MB.  
>>>>
>>>> In 32bit system it is problematic because the kernel have only 1GB
>>>> of memory allocation so the vmalloc cannot take 800MB.
>>>>
>>>> When implementing the FIU driver in the mtd/spi-nor we allocating memory  
>>> address only  
>>>> for detected Flash with exact size (usually we are not using 128MB  
>>> Flash), and in that case usually we allocating much less memory.  
>>>>
>>>> To solve this issue we needed to overcome two things:
>>>>
>>>> 1.    Get argument from the upper layer (spi-mem layer)
>>>> 2.    Calling the get argument function after SPI_NOR_SCAN function.  
>>> (the MTD Flash size filled in  SPI_NOR_SCAN function)
>>>
>>> That's clearly breaking the layering we've tried to restore with the
>>> spi-nor/spi-mem split, and I don't see why this is needed since we now
>>> have a way to create direct mappings dynamically (with the dirmap API).
>>> Have you tried implementing the dirmap hooks in your driver?  
>>
>>
>>  Sorry but I wasn't familiar with the direct mapping in the spi-mem, it
>> seems it needed to implemented in the m25p80 driver as well, am I correct?
> 
> There's this patch [1] floating around. IIRC, Sergey was waiting for
> the m25p80 -> spi-nor merge to send a v5.

   No, not really waiting for it. I was asked to recast the patch using
the managed device APIs, and I got sucked into my HyperFlash driver and
dropped the ball...

> Vignesh, any updates on that
> one? If you don't have time to work on that, maybe Sergey could send a
> v5.

   I can try recasting it RSN, if it's blocking some other stuff...

> [1]https://www.spinics.net/lists/linux-mtd/msg07358.html

MBR, Sergei

Vignesh Raghavendra July 30, 2019, 5:48 p.m. UTC | #4

On 30-Jul-19 12:24 PM, Boris Brezillon wrote:
> Trimmed the recipient list a bit and used Frieder's new address.
> +Sergey
> 
> On Mon, 29 Jul 2019 23:55:05 +0300
> Tomer Maimon <[e-mail]> wrote:
> 
>> Hi Boris,
>>
>> Thanks for the prompt reply,
>>
>>
>>
>> On Mon, 29 Jul 2019 at 18:29, Boris Brezillon <[e-mail]>
>> wrote:
>>
>>> Hi Tomer,
>>>
>>> On Mon, 29 Jul 2019 17:25:01 +0300
>>> Tomer Maimon <[e-mail]> wrote:
>>>  
>>>> Lately we have working on Flash interface unit (FIU) SPI driver that
>>>> using spi-mem interface, Our FIU HW module support direct Flash Rd//Wr.
>>>>
>>>> In our SOC (32 bit dual core ARM) we have 3 FIU's that using memory  
>>> mapping as follow:  
>>>>
>>>> FIU0 - have 2 chip select and each one have 128MB memory mapping (total  
>>> 256MB memory mapping)  
>>>> FIU1 - have 4 chip select and each one have 128MB memory mapping (total  
>>> 512MB memory mapping)  
>>>> FIU2 - have 4 chip select and each one have 16MB memory mapping (total  
>>> 32MB memory mapping)  
>>>>
>>>> Totally 800MB memory mapping.
>>>>
>>>> When the FIU driver probe it don't know the size of each Flash that
>>>> connected to the FIU, so the entire memory mapping is allocated for each  
>>> FIU  
>>>> according the FIU device tree memory map parameters.  
>>>
>>> Do you need those mappings to be active to support simple reg accesses?
>>>  
>>>> It means, if we enable all three FIU's the drivers will try to allocate  
>>> totally 800MB.  
>>>>
>>>> In 32bit system it is problematic because the kernel have only 1GB
>>>> of memory allocation so the vmalloc cannot take 800MB.
>>>>
>>>> When implementing the FIU driver in the mtd/spi-nor we allocating memory  
>>> address only  
>>>> for detected Flash with exact size (usually we are not using 128MB  
>>> Flash), and in that case usually we allocating much less memory.  
>>>>
>>>> To solve this issue we needed to overcome two things:
>>>>
>>>> 1.    Get argument from the upper layer (spi-mem layer)
>>>> 2.    Calling the get argument function after SPI_NOR_SCAN function.  
>>> (the MTD Flash size filled in  SPI_NOR_SCAN function)
>>>
>>> That's clearly breaking the layering we've tried to restore with the
>>> spi-nor/spi-mem split, and I don't see why this is needed since we now
>>> have a way to create direct mappings dynamically (with the dirmap API).
>>> Have you tried implementing the dirmap hooks in your driver?  
>>
>>
>>  Sorry but I wasn't familiar with the direct mapping in the spi-mem, it
>> seems it needed to implemented in the m25p80 driver as well, am I correct?
> 
> There's this patch [1] floating around. IIRC, Sergey was waiting for
> the m25p80 -> spi-nor merge to send a v5. Vignesh, any updates on that
> one? If you don't have time to work on that, maybe Sergey could send a
> v5.
> 

I did send an updated series of merging m25p80 to spi-nor last week and
have received few comments. Will respin one more version this week
(mostly by tomorrow).

Regards
Vignesh

Boris Brezillon July 30, 2019, 6:04 p.m. UTC | #5

On Tue, 30 Jul 2019 23:18:25 +0530
Vignesh Raghavendra <[e-mail]> wrote:

> On 30-Jul-19 12:24 PM, Boris Brezillon wrote:
> > Trimmed the recipient list a bit and used Frieder's new address.
> > +Sergey
> > 
> > On Mon, 29 Jul 2019 23:55:05 +0300
> > Tomer Maimon <[e-mail]> wrote:
> >   
> >> Hi Boris,
> >>
> >> Thanks for the prompt reply,
> >>
> >>
> >>
> >> On Mon, 29 Jul 2019 at 18:29, Boris Brezillon <[e-mail]>
> >> wrote:
> >>  
> >>> Hi Tomer,
> >>>
> >>> On Mon, 29 Jul 2019 17:25:01 +0300
> >>> Tomer Maimon <[e-mail]> wrote:
> >>>    
> >>>> Lately we have working on Flash interface unit (FIU) SPI driver that
> >>>> using spi-mem interface, Our FIU HW module support direct Flash Rd//Wr.
> >>>>
> >>>> In our SOC (32 bit dual core ARM) we have 3 FIU's that using memory    
> >>> mapping as follow:    
> >>>>
> >>>> FIU0 - have 2 chip select and each one have 128MB memory mapping (total    
> >>> 256MB memory mapping)    
> >>>> FIU1 - have 4 chip select and each one have 128MB memory mapping (total    
> >>> 512MB memory mapping)    
> >>>> FIU2 - have 4 chip select and each one have 16MB memory mapping (total    
> >>> 32MB memory mapping)    
> >>>>
> >>>> Totally 800MB memory mapping.
> >>>>
> >>>> When the FIU driver probe it don't know the size of each Flash that
> >>>> connected to the FIU, so the entire memory mapping is allocated for each    
> >>> FIU    
> >>>> according the FIU device tree memory map parameters.    
> >>>
> >>> Do you need those mappings to be active to support simple reg accesses?
> >>>    
> >>>> It means, if we enable all three FIU's the drivers will try to allocate    
> >>> totally 800MB.    
> >>>>
> >>>> In 32bit system it is problematic because the kernel have only 1GB
> >>>> of memory allocation so the vmalloc cannot take 800MB.
> >>>>
> >>>> When implementing the FIU driver in the mtd/spi-nor we allocating memory    
> >>> address only    
> >>>> for detected Flash with exact size (usually we are not using 128MB    
> >>> Flash), and in that case usually we allocating much less memory.    
> >>>>
> >>>> To solve this issue we needed to overcome two things:
> >>>>
> >>>> 1.    Get argument from the upper layer (spi-mem layer)
> >>>> 2.    Calling the get argument function after SPI_NOR_SCAN function.    
> >>> (the MTD Flash size filled in  SPI_NOR_SCAN function)
> >>>
> >>> That's clearly breaking the layering we've tried to restore with the
> >>> spi-nor/spi-mem split, and I don't see why this is needed since we now
> >>> have a way to create direct mappings dynamically (with the dirmap API).
> >>> Have you tried implementing the dirmap hooks in your driver?    
> >>
> >>
> >>  Sorry but I wasn't familiar with the direct mapping in the spi-mem, it
> >> seems it needed to implemented in the m25p80 driver as well, am I correct?  
> > 
> > There's this patch [1] floating around. IIRC, Sergey was waiting for
> > the m25p80 -> spi-nor merge to send a v5. Vignesh, any updates on that
> > one? If you don't have time to work on that, maybe Sergey could send a
> > v5.
> >   
> 
> I did send an updated series of merging m25p80 to spi-nor last week and
> have received few comments. Will respin one more version this week
> (mostly by tomorrow).

Okay, great!

Vignesh Raghavendra Aug. 1, 2019, 6:42 a.m. UTC | #6

Hi,

On 31/07/19 1:49 PM, Tomer Maimon wrote:
> Hi Vignesh,
> 
> Does your new merge version will support direct spi-mem API?
> 


No, I don't have a driver to test out dirmap APIs. So, that would need
to be added separately on top.

I have posted next version of my series here (expect more revisions):
https://patchwork.ozlabs.org/cover/1140269/

Feel free to test and rebase dirmap API addition on top of it.

Regards
Vignesh


> Thanks,
> 
> Tomer
> 
> On Tue, 30 Jul 2019 at 21:04, Boris Brezillon
> <[e-mail] <mailto:[e-mail]>>
> wrote:
> 
>     On Tue, 30 Jul 2019 23:18:25 +0530
>     Vignesh Raghavendra <[e-mail] <mailto:[e-mail]>> wrote:
> 
>     > On 30-Jul-19 12:24 PM, Boris Brezillon wrote:
>     > > Trimmed the recipient list a bit and used Frieder's new address.
>     > > +Sergey
>     > >
>     > > On Mon, 29 Jul 2019 23:55:05 +0300
>     > > Tomer Maimon <[e-mail] <mailto:[e-mail]>>
>     wrote:
>     > >   
>     > >> Hi Boris,
>     > >>
>     > >> Thanks for the prompt reply,
>     > >>
>     > >>
>     > >>
>     > >> On Mon, 29 Jul 2019 at 18:29, Boris Brezillon
>     <[e-mail] <mailto:[e-mail]>>
>     > >> wrote:
>     > >> 
>     > >>> Hi Tomer,
>     > >>>
>     > >>> On Mon, 29 Jul 2019 17:25:01 +0300
>     > >>> Tomer Maimon <[e-mail]
>     <mailto:[e-mail]>> wrote:
>     > >>>   
>     > >>>> Lately we have working on Flash interface unit (FIU) SPI
>     driver that
>     > >>>> using spi-mem interface, Our FIU HW module support direct
>     Flash Rd//Wr.
>     > >>>>
>     > >>>> In our SOC (32 bit dual core ARM) we have 3 FIU's that using
>     memory   
>     > >>> mapping as follow:   
>     > >>>>
>     > >>>> FIU0 - have 2 chip select and each one have 128MB memory
>     mapping (total   
>     > >>> 256MB memory mapping)   
>     > >>>> FIU1 - have 4 chip select and each one have 128MB memory
>     mapping (total   
>     > >>> 512MB memory mapping)   
>     > >>>> FIU2 - have 4 chip select and each one have 16MB memory
>     mapping (total   
>     > >>> 32MB memory mapping)   
>     > >>>>
>     > >>>> Totally 800MB memory mapping.
>     > >>>>
>     > >>>> When the FIU driver probe it don't know the size of each
>     Flash that
>     > >>>> connected to the FIU, so the entire memory mapping is
>     allocated for each   
>     > >>> FIU   
>     > >>>> according the FIU device tree memory map parameters.   
>     > >>>
>     > >>> Do you need those mappings to be active to support simple reg
>     accesses?
>     > >>>   
>     > >>>> It means, if we enable all three FIU's the drivers will try
>     to allocate   
>     > >>> totally 800MB.   
>     > >>>>
>     > >>>> In 32bit system it is problematic because the kernel have
>     only 1GB
>     > >>>> of memory allocation so the vmalloc cannot take 800MB.
>     > >>>>
>     > >>>> When implementing the FIU driver in the mtd/spi-nor we
>     allocating memory   
>     > >>> address only   
>     > >>>> for detected Flash with exact size (usually we are not using
>     128MB   
>     > >>> Flash), and in that case usually we allocating much less
>     memory.   
>     > >>>>
>     > >>>> To solve this issue we needed to overcome two things:
>     > >>>>
>     > >>>> 1.    Get argument from the upper layer (spi-mem layer)
>     > >>>> 2.    Calling the get argument function after SPI_NOR_SCAN
>     function.   
>     > >>> (the MTD Flash size filled in  SPI_NOR_SCAN function)
>     > >>>
>     > >>> That's clearly breaking the layering we've tried to restore
>     with the
>     > >>> spi-nor/spi-mem split, and I don't see why this is needed
>     since we now
>     > >>> have a way to create direct mappings dynamically (with the
>     dirmap API).
>     > >>> Have you tried implementing the dirmap hooks in your driver?   
>     > >>
>     > >>
>     > >>  Sorry but I wasn't familiar with the direct mapping in the
>     spi-mem, it
>     > >> seems it needed to implemented in the m25p80 driver as well, am
>     I correct? 
>     > >
>     > > There's this patch [1] floating around. IIRC, Sergey was waiting for
>     > > the m25p80 -> spi-nor merge to send a v5. Vignesh, any updates
>     on that
>     > > one? If you don't have time to work on that, maybe Sergey could
>     send a
>     > > v5.
>     > >   
>     >
>     > I did send an updated series of merging m25p80 to spi-nor last
>     week and
>     > have received few comments. Will respin one more version this week
>     > (mostly by tomorrow).
> 
>     Okay, great!
>