Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Ping?

On Wed, Sep 21, 2016 at 10:26 AM, Ricardo Ribalda Delgado
 wrote:
> Hi
>
>>>
>>> Unsinged long is more than enough for s3an. I think v7 is ok.
>>
>> Then, why did you change the spi_nor_s3an_addr_convert() prototype?
>>
>
> Because it is used in spi_nor_read and spi_nor_write, which use loff_t.
>
> I think it is more clean to have only one comment explaining the max
> size of the s3an, instead of having them scattered around. But if
> think that it is better to have unsinged int as prototype I have no
> problem in changing it.
>
> Regards!
>
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Ping?

On Wed, Sep 21, 2016 at 10:26 AM, Ricardo Ribalda Delgado
 wrote:
> Hi
>
>>>
>>> Unsinged long is more than enough for s3an. I think v7 is ok.
>>
>> Then, why did you change the spi_nor_s3an_addr_convert() prototype?
>>
>
> Because it is used in spi_nor_read and spi_nor_write, which use loff_t.
>
> I think it is more clean to have only one comment explaining the max
> size of the s3an, instead of having them scattered around. But if
> think that it is better to have unsinged int as prototype I have no
> problem in changing it.
>
> Regards!
>
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi

>>
>> Unsinged long is more than enough for s3an. I think v7 is ok.
>
> Then, why did you change the spi_nor_s3an_addr_convert() prototype?
>

Because it is used in spi_nor_read and spi_nor_write, which use loff_t.

I think it is more clean to have only one comment explaining the max
size of the s3an, instead of having them scattered around. But if
think that it is better to have unsinged int as prototype I have no
problem in changing it.

Regards!


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi

>>
>> Unsinged long is more than enough for s3an. I think v7 is ok.
>
> Then, why did you change the spi_nor_s3an_addr_convert() prototype?
>

Because it is used in spi_nor_read and spi_nor_write, which use loff_t.

I think it is more clean to have only one comment explaining the max
size of the s3an, instead of having them scattered around. But if
think that it is better to have unsinged int as prototype I have no
problem in changing it.

Regards!


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 10:19:10 +0200
Ricardo Ribalda Delgado  wrote:

> Hi Boris
> 
> On Wed, Sep 21, 2016 at 10:14 AM, Boris Brezillon
>  wrote:
> > Wait. If you really want to manipulate an loff_t variable, you can do
> >
> > offset = do_div(addr, nor->page_size);
> >
> >
> >  
> that leads to:
> 
>   CC  drivers/mtd/spi-nor/spi-nor.o
> drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
> drivers/mtd/spi-nor/spi-nor.c:359:11: warning: comparison of distinct
> pointer types lacks a cast [enabled by default]
> 
> 
> and if I do the casting
> 
> offset = do_div((uint64_t)addr, nor->page_size);
> 
> 
> drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as unary
> '&' operand
> scr
> 
> Unsinged long is more than enough for s3an. I think v7 is ok.

Then, why did you change the spi_nor_s3an_addr_convert() prototype? 

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 10:19:10 +0200
Ricardo Ribalda Delgado  wrote:

> Hi Boris
> 
> On Wed, Sep 21, 2016 at 10:14 AM, Boris Brezillon
>  wrote:
> > Wait. If you really want to manipulate an loff_t variable, you can do
> >
> > offset = do_div(addr, nor->page_size);
> >
> >
> >  
> that leads to:
> 
>   CC  drivers/mtd/spi-nor/spi-nor.o
> drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
> drivers/mtd/spi-nor/spi-nor.c:359:11: warning: comparison of distinct
> pointer types lacks a cast [enabled by default]
> 
> 
> and if I do the casting
> 
> offset = do_div((uint64_t)addr, nor->page_size);
> 
> 
> drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as unary
> '&' operand
> scr
> 
> Unsinged long is more than enough for s3an. I think v7 is ok.

Then, why did you change the spi_nor_s3an_addr_convert() prototype? 

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 10:14:20 +0200
Boris Brezillon  wrote:

> On Wed, 21 Sep 2016 09:57:23 +0200
> Ricardo Ribalda Delgado  wrote:
> 
> > Hi Boris
> > 
> > On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
> >  wrote:  
> > > Hi Ricardo,
> > >
> > > Please try to pass the version in your subject prefix (pass
> > > --subject-prefix="PATCH vX" to git format-patch).
> > >
> > 
> > Sorry about that.
> > 
> >   
> > > On Tue, 20 Sep 2016 17:45:51 +0200
> > > Ricardo Ribalda Delgado  wrote:
> >   
> > >>  /*
> > >> + * This code converts an address to the Default Address Mode, that has 
> > >> non
> > >> + * power of two page sizes. We must support this mode because it is the 
> > >> default
> > >> + * mode supported by Xilinx tools, it can access the whole flash area 
> > >> and
> > >> + * changing over to the Power-of-two mode is irreversible and corrupts 
> > >> the
> > >> + * original data.
> > >> + */
> > >> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned 
> > >> int addr)
> > >> +{
> > >> + unsigned int offset;
> > >> +
> > >> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);
> > >
> > > Can you send a new version with
> > >
> > > offset = addr % nor->page_size;
> > >
> > > to see if kbuild test robot still complains.
> > >
> > 
> > This code:
> > 
> > static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> > {
> > unsigned int offset;
> > 
> > offset = addr % nor->page_size;
> > 
> > return ((addr - offset) << 1) | offset;
> > }
> > 
> > 
> > When built like:
> > 
> >wget 
> > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> > -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # save the attached .config to linux build tree
> > make.cross ARCH=blackfin
> > 
> > Produces this error:
> > 
> > 
> > drivers/built-in.o: In function `spi_nor_write':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to 
> > `__moddi3'
> > drivers/built-in.o: In function `spi_nor_read':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to 
> > `__moddi3'
> > drivers/built-in.o: In function `spi_nor_erase':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to 
> > `__moddi3'
> > Makefile:956: recipe for target 'vmlinux' failed
> > 
> > 
> > But I think I found the right combination:
> > 
> > 
> > static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> > {
> > unsigned int offset = addr;
> > 
> > offset %= nor->page_size;
> > 
> > return ((addr - offset) << 1) | offset;
> > }
> > 
> > 
> > This one works fine on x86 and blackfin
> > 
> > Sending v7  
> 
> Wait. If you really want to manipulate an loff_t variable, you can do
> 
>   offset = do_div(addr, nor->page_size);
> 
> 

Actually, you should have an intermediate u64 var, or just turn addr
into an u64 (loff_t is a long long not an unsigned long long).

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 10:14:20 +0200
Boris Brezillon  wrote:

> On Wed, 21 Sep 2016 09:57:23 +0200
> Ricardo Ribalda Delgado  wrote:
> 
> > Hi Boris
> > 
> > On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
> >  wrote:  
> > > Hi Ricardo,
> > >
> > > Please try to pass the version in your subject prefix (pass
> > > --subject-prefix="PATCH vX" to git format-patch).
> > >
> > 
> > Sorry about that.
> > 
> >   
> > > On Tue, 20 Sep 2016 17:45:51 +0200
> > > Ricardo Ribalda Delgado  wrote:
> >   
> > >>  /*
> > >> + * This code converts an address to the Default Address Mode, that has 
> > >> non
> > >> + * power of two page sizes. We must support this mode because it is the 
> > >> default
> > >> + * mode supported by Xilinx tools, it can access the whole flash area 
> > >> and
> > >> + * changing over to the Power-of-two mode is irreversible and corrupts 
> > >> the
> > >> + * original data.
> > >> + */
> > >> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned 
> > >> int addr)
> > >> +{
> > >> + unsigned int offset;
> > >> +
> > >> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);
> > >
> > > Can you send a new version with
> > >
> > > offset = addr % nor->page_size;
> > >
> > > to see if kbuild test robot still complains.
> > >
> > 
> > This code:
> > 
> > static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> > {
> > unsigned int offset;
> > 
> > offset = addr % nor->page_size;
> > 
> > return ((addr - offset) << 1) | offset;
> > }
> > 
> > 
> > When built like:
> > 
> >wget 
> > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> > -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # save the attached .config to linux build tree
> > make.cross ARCH=blackfin
> > 
> > Produces this error:
> > 
> > 
> > drivers/built-in.o: In function `spi_nor_write':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to 
> > `__moddi3'
> > drivers/built-in.o: In function `spi_nor_read':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to 
> > `__moddi3'
> > drivers/built-in.o: In function `spi_nor_erase':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to 
> > `__moddi3'
> > Makefile:956: recipe for target 'vmlinux' failed
> > 
> > 
> > But I think I found the right combination:
> > 
> > 
> > static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> > {
> > unsigned int offset = addr;
> > 
> > offset %= nor->page_size;
> > 
> > return ((addr - offset) << 1) | offset;
> > }
> > 
> > 
> > This one works fine on x86 and blackfin
> > 
> > Sending v7  
> 
> Wait. If you really want to manipulate an loff_t variable, you can do
> 
>   offset = do_div(addr, nor->page_size);
> 
> 

Actually, you should have an intermediate u64 var, or just turn addr
into an u64 (loff_t is a long long not an unsigned long long).

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi Boris

On Wed, Sep 21, 2016 at 10:14 AM, Boris Brezillon
 wrote:
> Wait. If you really want to manipulate an loff_t variable, you can do
>
> offset = do_div(addr, nor->page_size);
>
>
>
that leads to:

  CC  drivers/mtd/spi-nor/spi-nor.o
drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
drivers/mtd/spi-nor/spi-nor.c:359:11: warning: comparison of distinct
pointer types lacks a cast [enabled by default]


and if I do the casting

offset = do_div((uint64_t)addr, nor->page_size);


drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as unary
'&' operand
scr

Unsinged long is more than enough for s3an. I think v7 is ok.


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi Boris

On Wed, Sep 21, 2016 at 10:14 AM, Boris Brezillon
 wrote:
> Wait. If you really want to manipulate an loff_t variable, you can do
>
> offset = do_div(addr, nor->page_size);
>
>
>
that leads to:

  CC  drivers/mtd/spi-nor/spi-nor.o
drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
drivers/mtd/spi-nor/spi-nor.c:359:11: warning: comparison of distinct
pointer types lacks a cast [enabled by default]


and if I do the casting

offset = do_div((uint64_t)addr, nor->page_size);


drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as unary
'&' operand
scr

Unsinged long is more than enough for s3an. I think v7 is ok.


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 09:57:23 +0200
Ricardo Ribalda Delgado  wrote:

> Hi Boris
> 
> On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
>  wrote:
> > Hi Ricardo,
> >
> > Please try to pass the version in your subject prefix (pass
> > --subject-prefix="PATCH vX" to git format-patch).
> >  
> 
> Sorry about that.
> 
> 
> > On Tue, 20 Sep 2016 17:45:51 +0200
> > Ricardo Ribalda Delgado  wrote:  
> 
> >>  /*
> >> + * This code converts an address to the Default Address Mode, that has non
> >> + * power of two page sizes. We must support this mode because it is the 
> >> default
> >> + * mode supported by Xilinx tools, it can access the whole flash area and
> >> + * changing over to the Power-of-two mode is irreversible and corrupts the
> >> + * original data.
> >> + */
> >> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int 
> >> addr)
> >> +{
> >> + unsigned int offset;
> >> +
> >> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);  
> >
> > Can you send a new version with
> >
> > offset = addr % nor->page_size;
> >
> > to see if kbuild test robot still complains.
> >  
> 
> This code:
> 
> static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> {
> unsigned int offset;
> 
> offset = addr % nor->page_size;
> 
> return ((addr - offset) << 1) | offset;
> }
> 
> 
> When built like:
> 
>wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=blackfin
> 
> Produces this error:
> 
> 
> drivers/built-in.o: In function `spi_nor_write':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to 
> `__moddi3'
> drivers/built-in.o: In function `spi_nor_read':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to 
> `__moddi3'
> drivers/built-in.o: In function `spi_nor_erase':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to 
> `__moddi3'
> Makefile:956: recipe for target 'vmlinux' failed
> 
> 
> But I think I found the right combination:
> 
> 
> static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> {
> unsigned int offset = addr;
> 
> offset %= nor->page_size;
> 
> return ((addr - offset) << 1) | offset;
> }
> 
> 
> This one works fine on x86 and blackfin
> 
> Sending v7

Wait. If you really want to manipulate an loff_t variable, you can do

offset = do_div(addr, nor->page_size);

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 09:57:23 +0200
Ricardo Ribalda Delgado  wrote:

> Hi Boris
> 
> On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
>  wrote:
> > Hi Ricardo,
> >
> > Please try to pass the version in your subject prefix (pass
> > --subject-prefix="PATCH vX" to git format-patch).
> >  
> 
> Sorry about that.
> 
> 
> > On Tue, 20 Sep 2016 17:45:51 +0200
> > Ricardo Ribalda Delgado  wrote:  
> 
> >>  /*
> >> + * This code converts an address to the Default Address Mode, that has non
> >> + * power of two page sizes. We must support this mode because it is the 
> >> default
> >> + * mode supported by Xilinx tools, it can access the whole flash area and
> >> + * changing over to the Power-of-two mode is irreversible and corrupts the
> >> + * original data.
> >> + */
> >> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int 
> >> addr)
> >> +{
> >> + unsigned int offset;
> >> +
> >> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);  
> >
> > Can you send a new version with
> >
> > offset = addr % nor->page_size;
> >
> > to see if kbuild test robot still complains.
> >  
> 
> This code:
> 
> static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> {
> unsigned int offset;
> 
> offset = addr % nor->page_size;
> 
> return ((addr - offset) << 1) | offset;
> }
> 
> 
> When built like:
> 
>wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=blackfin
> 
> Produces this error:
> 
> 
> drivers/built-in.o: In function `spi_nor_write':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to 
> `__moddi3'
> drivers/built-in.o: In function `spi_nor_read':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to 
> `__moddi3'
> drivers/built-in.o: In function `spi_nor_erase':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to 
> `__moddi3'
> Makefile:956: recipe for target 'vmlinux' failed
> 
> 
> But I think I found the right combination:
> 
> 
> static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> {
> unsigned int offset = addr;
> 
> offset %= nor->page_size;
> 
> return ((addr - offset) << 1) | offset;
> }
> 
> 
> This one works fine on x86 and blackfin
> 
> Sending v7

Wait. If you really want to manipulate an loff_t variable, you can do

offset = do_div(addr, nor->page_size);

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi Boris

On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
 wrote:
> Hi Ricardo,
>
> Please try to pass the version in your subject prefix (pass
> --subject-prefix="PATCH vX" to git format-patch).
>

Sorry about that.


> On Tue, 20 Sep 2016 17:45:51 +0200
> Ricardo Ribalda Delgado  wrote:

>>  /*
>> + * This code converts an address to the Default Address Mode, that has non
>> + * power of two page sizes. We must support this mode because it is the 
>> default
>> + * mode supported by Xilinx tools, it can access the whole flash area and
>> + * changing over to the Power-of-two mode is irreversible and corrupts the
>> + * original data.
>> + */
>> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int 
>> addr)
>> +{
>> + unsigned int offset;
>> +
>> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);
>
> Can you send a new version with
>
> offset = addr % nor->page_size;
>
> to see if kbuild test robot still complains.
>

This code:

static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
{
unsigned int offset;

offset = addr % nor->page_size;

return ((addr - offset) << 1) | offset;
}


When built like:

   wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin

Produces this error:


drivers/built-in.o: In function `spi_nor_write':
drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to `__moddi3'
drivers/built-in.o: In function `spi_nor_read':
drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to `__moddi3'
drivers/built-in.o: In function `spi_nor_erase':
drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to `__moddi3'
Makefile:956: recipe for target 'vmlinux' failed


But I think I found the right combination:


static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
{
unsigned int offset = addr;

offset %= nor->page_size;

return ((addr - offset) << 1) | offset;
}


This one works fine on x86 and blackfin

Sending v7


Thanks!


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi Boris

On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
 wrote:
> Hi Ricardo,
>
> Please try to pass the version in your subject prefix (pass
> --subject-prefix="PATCH vX" to git format-patch).
>

Sorry about that.


> On Tue, 20 Sep 2016 17:45:51 +0200
> Ricardo Ribalda Delgado  wrote:

>>  /*
>> + * This code converts an address to the Default Address Mode, that has non
>> + * power of two page sizes. We must support this mode because it is the 
>> default
>> + * mode supported by Xilinx tools, it can access the whole flash area and
>> + * changing over to the Power-of-two mode is irreversible and corrupts the
>> + * original data.
>> + */
>> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int 
>> addr)
>> +{
>> + unsigned int offset;
>> +
>> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);
>
> Can you send a new version with
>
> offset = addr % nor->page_size;
>
> to see if kbuild test robot still complains.
>

This code:

static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
{
unsigned int offset;

offset = addr % nor->page_size;

return ((addr - offset) << 1) | offset;
}


When built like:

   wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin

Produces this error:


drivers/built-in.o: In function `spi_nor_write':
drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to `__moddi3'
drivers/built-in.o: In function `spi_nor_read':
drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to `__moddi3'
drivers/built-in.o: In function `spi_nor_erase':
drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to `__moddi3'
Makefile:956: recipe for target 'vmlinux' failed


But I think I found the right combination:


static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
{
unsigned int offset = addr;

offset %= nor->page_size;

return ((addr - offset) << 1) | offset;
}


This one works fine on x86 and blackfin

Sending v7


Thanks!


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

Hi Ricardo,

Please try to pass the version in your subject prefix (pass
--subject-prefix="PATCH vX" to git format-patch).

On Tue, 20 Sep 2016 17:45:51 +0200
Ricardo Ribalda Delgado  wrote:

> Xilinx Spartan-3AN FPGAs contain an In-System Flash where they keep
> their configuration data and (optionally) some user data.
> 
> The protocol of this flash follows most of the spi-nor standard. With
> the following differences:
> 
> - Page size might not be a power of two.
> - The address calculation (default addressing mode).
> - The spi nor commands used.
> 
> Protocol is described on Xilinx User Guide UG333
> 
> Reviewed-by: Cyrille Pitchen 
> Signed-off-by: Ricardo Ribalda Delgado 
> ---
> v6:
> Suggested by: Boris Brezillon 
> -Replace triple operator with if/else
> 
> v5:
> -Rebase on top of l2-mtd/master
> Suggested by: Cyrille Pitchen :
> -Fix to+1 bug
> -Move all address conversions to spi-nor
> -Replace pr_dev with dev_err
> 
> v4:
> -Rebase on top of l2-mtd/master
> 
> v3:
> -Rebase on top of mtd-next
> -Rename ADDR_NATIVE to ADDR_DEFAULT to follow UG333 naming
> -Fix bug on probe
> 
> v2: Suggested by Brian Norris 
> 
> -Remove inline qualifier
> -Improve documentation of Default Addressing Mode
> -Convert function callbacks into SNOR_F_
> -Fix missmatch braces
> -Improve documentation of SPI_S3AN flag
> 
>  drivers/mtd/spi-nor/spi-nor.c | 124 
> --
>  include/linux/mtd/spi-nor.h   |  12 
>  2 files changed, 131 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d0fc165d7d66..d29842177808 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c

[...]

>  /*
> + * This code converts an address to the Default Address Mode, that has non
> + * power of two page sizes. We must support this mode because it is the 
> default
> + * mode supported by Xilinx tools, it can access the whole flash area and
> + * changing over to the Power-of-two mode is irreversible and corrupts the
> + * original data.
> + */
> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int 
> addr)
> +{
> + unsigned int offset;
> +
> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);

Can you send a new version with

offset = addr % nor->page_size;

to see if kbuild test robot still complains.

> +
> + return ((addr - offset) << 1) | offset;
> +}
> +

Thanks,

Boris

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

Hi Ricardo,

Please try to pass the version in your subject prefix (pass
--subject-prefix="PATCH vX" to git format-patch).

On Tue, 20 Sep 2016 17:45:51 +0200
Ricardo Ribalda Delgado  wrote:

> Xilinx Spartan-3AN FPGAs contain an In-System Flash where they keep
> their configuration data and (optionally) some user data.
> 
> The protocol of this flash follows most of the spi-nor standard. With
> the following differences:
> 
> - Page size might not be a power of two.
> - The address calculation (default addressing mode).
> - The spi nor commands used.
> 
> Protocol is described on Xilinx User Guide UG333
> 
> Reviewed-by: Cyrille Pitchen 
> Signed-off-by: Ricardo Ribalda Delgado 
> ---
> v6:
> Suggested by: Boris Brezillon 
> -Replace triple operator with if/else
> 
> v5:
> -Rebase on top of l2-mtd/master
> Suggested by: Cyrille Pitchen :
> -Fix to+1 bug
> -Move all address conversions to spi-nor
> -Replace pr_dev with dev_err
> 
> v4:
> -Rebase on top of l2-mtd/master
> 
> v3:
> -Rebase on top of mtd-next
> -Rename ADDR_NATIVE to ADDR_DEFAULT to follow UG333 naming
> -Fix bug on probe
> 
> v2: Suggested by Brian Norris 
> 
> -Remove inline qualifier
> -Improve documentation of Default Addressing Mode
> -Convert function callbacks into SNOR_F_
> -Fix missmatch braces
> -Improve documentation of SPI_S3AN flag
> 
>  drivers/mtd/spi-nor/spi-nor.c | 124 
> --
>  include/linux/mtd/spi-nor.h   |  12 
>  2 files changed, 131 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d0fc165d7d66..d29842177808 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c

[...]

>  /*
> + * This code converts an address to the Default Address Mode, that has non
> + * power of two page sizes. We must support this mode because it is the 
> default
> + * mode supported by Xilinx tools, it can access the whole flash area and
> + * changing over to the Power-of-two mode is irreversible and corrupts the
> + * original data.
> + */
> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int 
> addr)
> +{
> + unsigned int offset;
> +
> + offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);

Can you send a new version with

offset = addr % nor->page_size;

to see if kbuild test robot still complains.

> +
> + return ((addr - offset) << 1) | offset;
> +}
> +

Thanks,

Boris

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hello

It has been almost two months :)... Any feedback about the patch
before I port it to the next version and resend?

Regards!

On Fri, Jan 22, 2016 at 3:52 PM, Ricardo Ribalda Delgado
 wrote:
> ping?
>
>
> On Fri, Jan 15, 2016 at 5:25 PM, Ricardo Ribalda Delgado
>  wrote:
>> Xilinx Spartan-3AN contain an embedded spi device where they keep their
>> configuration data and optionally some user data.
>>
>> The protocol of this flash follows most of the spi-nor standard. With
>> the following differences:
>>
>> - Page size might not be a power of two.
>> - The address calculation.
>> - The spi nor commands used.
>>
>> Signed-off-by: Ricardo Ribalda Delgado 
>> ---
>>  drivers/mtd/devices/m25p80.c  |  3 ++
>>  drivers/mtd/spi-nor/spi-nor.c | 97 
>> +--
>>  include/linux/mtd/spi-nor.h   | 13 ++
>>  3 files changed, 110 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index c9c3b7fa3051..869576735978 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -49,6 +49,9 @@ static int m25p80_read_reg(struct spi_nor *nor, u8 code, 
>> u8 *val, int len)
>>
>>  static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>>  {
>> +   if (nor->addr_convert)
>> +   addr = nor->addr_convert(nor, addr);
>> +
>> /* opcode is in cmd[0] */
>> cmd[1] = addr >> (nor->addr_width * 8 -  8);
>> cmd[2] = addr >> (nor->addr_width * 8 - 16);
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index ed0c19c558b5..90215f053916 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -69,6 +69,7 @@ struct flash_info {
>>  #defineSPI_NOR_DUAL_READ   0x20/* Flash supports Dual Read 
>> */
>>  #defineSPI_NOR_QUAD_READ   0x40/* Flash supports Quad Read 
>> */
>>  #defineUSE_FSR 0x80/* use flag status register 
>> */
>> +#defineSPI_S3AN0x100   /* Xililnx S3AN Embedded SPI 
>> nor*/
>>  };
>>
>>  #define JEDEC_MFR(info)((info)->id[0])
>> @@ -211,6 +212,21 @@ static inline int set_4byte(struct spi_nor *nor, const 
>> struct flash_info *info,
>> return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
>> }
>>  }
>> +
>> +static inline int s3an_sr_ready(struct spi_nor *nor)
>> +{
>> +   int ret;
>> +   u8 val;
>> +
>> +   ret = nor->read_reg(nor, SPINOR_OP_XRDSR, , 1);
>> +   if (ret < 0) {
>> +   pr_err("error %d reading SR\n", (int) ret);
>> +   return ret;
>> +   }
>> +
>> +   return !!(val & XSR_RDY);
>> +}
>> +
>>  static inline int spi_nor_sr_ready(struct spi_nor *nor)
>>  {
>> int sr = read_sr(nor);
>> @@ -232,7 +248,7 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
>>  static int spi_nor_ready(struct spi_nor *nor)
>>  {
>> int sr, fsr;
>> -   sr = spi_nor_sr_ready(nor);
>> +   sr = nor->sr_ready(nor);
>> if (sr < 0)
>> return sr;
>> fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
>> @@ -328,6 +344,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 
>> addr)
>>  * Default implementation, if driver doesn't have a specialized HW
>>  * control
>>  */
>> +   if (nor->addr_convert)
>> +   addr = nor->addr_convert(nor, addr);
>> +
>> for (i = nor->addr_width - 1; i >= 0; i--) {
>> buf[i] = addr & 0xff;
>> addr >>= 8;
>> @@ -362,7 +381,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct 
>> erase_info *instr)
>> return ret;
>>
>> /* whole-chip erase? */
>> -   if (len == mtd->size) {
>> +   if (len == mtd->size && !(nor->flags & SNOR_NO_OP_CHIP_ERASE)) {
>> unsigned long timeout;
>>
>> write_enable(nor);
>> @@ -680,6 +699,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, 
>> loff_t ofs, uint64_t len)
>> .addr_width = (_addr_width),\
>> .flags = (_flags),
>>
>> +#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)   \
>> +   .id = { \
>> +   ((_jedec_id) >> 16) & 0xff, \
>> +   ((_jedec_id) >> 8) & 0xff,  \
>> +   (_jedec_id) & 0xff  \
>> +   },  \
>> +   .id_len = 3,\
>> +   .sector_size = (8*_page_size),  \
>> +   .n_sectors = (_n_sectors),  \

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hello

It has been almost two months :)... Any feedback about the patch
before I port it to the next version and resend?

Regards!

On Fri, Jan 22, 2016 at 3:52 PM, Ricardo Ribalda Delgado
 wrote:
> ping?
>
>
> On Fri, Jan 15, 2016 at 5:25 PM, Ricardo Ribalda Delgado
>  wrote:
>> Xilinx Spartan-3AN contain an embedded spi device where they keep their
>> configuration data and optionally some user data.
>>
>> The protocol of this flash follows most of the spi-nor standard. With
>> the following differences:
>>
>> - Page size might not be a power of two.
>> - The address calculation.
>> - The spi nor commands used.
>>
>> Signed-off-by: Ricardo Ribalda Delgado 
>> ---
>>  drivers/mtd/devices/m25p80.c  |  3 ++
>>  drivers/mtd/spi-nor/spi-nor.c | 97 
>> +--
>>  include/linux/mtd/spi-nor.h   | 13 ++
>>  3 files changed, 110 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index c9c3b7fa3051..869576735978 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -49,6 +49,9 @@ static int m25p80_read_reg(struct spi_nor *nor, u8 code, 
>> u8 *val, int len)
>>
>>  static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>>  {
>> +   if (nor->addr_convert)
>> +   addr = nor->addr_convert(nor, addr);
>> +
>> /* opcode is in cmd[0] */
>> cmd[1] = addr >> (nor->addr_width * 8 -  8);
>> cmd[2] = addr >> (nor->addr_width * 8 - 16);
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index ed0c19c558b5..90215f053916 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -69,6 +69,7 @@ struct flash_info {
>>  #defineSPI_NOR_DUAL_READ   0x20/* Flash supports Dual Read 
>> */
>>  #defineSPI_NOR_QUAD_READ   0x40/* Flash supports Quad Read 
>> */
>>  #defineUSE_FSR 0x80/* use flag status register 
>> */
>> +#defineSPI_S3AN0x100   /* Xililnx S3AN Embedded SPI 
>> nor*/
>>  };
>>
>>  #define JEDEC_MFR(info)((info)->id[0])
>> @@ -211,6 +212,21 @@ static inline int set_4byte(struct spi_nor *nor, const 
>> struct flash_info *info,
>> return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
>> }
>>  }
>> +
>> +static inline int s3an_sr_ready(struct spi_nor *nor)
>> +{
>> +   int ret;
>> +   u8 val;
>> +
>> +   ret = nor->read_reg(nor, SPINOR_OP_XRDSR, , 1);
>> +   if (ret < 0) {
>> +   pr_err("error %d reading SR\n", (int) ret);
>> +   return ret;
>> +   }
>> +
>> +   return !!(val & XSR_RDY);
>> +}
>> +
>>  static inline int spi_nor_sr_ready(struct spi_nor *nor)
>>  {
>> int sr = read_sr(nor);
>> @@ -232,7 +248,7 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
>>  static int spi_nor_ready(struct spi_nor *nor)
>>  {
>> int sr, fsr;
>> -   sr = spi_nor_sr_ready(nor);
>> +   sr = nor->sr_ready(nor);
>> if (sr < 0)
>> return sr;
>> fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
>> @@ -328,6 +344,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 
>> addr)
>>  * Default implementation, if driver doesn't have a specialized HW
>>  * control
>>  */
>> +   if (nor->addr_convert)
>> +   addr = nor->addr_convert(nor, addr);
>> +
>> for (i = nor->addr_width - 1; i >= 0; i--) {
>> buf[i] = addr & 0xff;
>> addr >>= 8;
>> @@ -362,7 +381,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct 
>> erase_info *instr)
>> return ret;
>>
>> /* whole-chip erase? */
>> -   if (len == mtd->size) {
>> +   if (len == mtd->size && !(nor->flags & SNOR_NO_OP_CHIP_ERASE)) {
>> unsigned long timeout;
>>
>> write_enable(nor);
>> @@ -680,6 +699,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, 
>> loff_t ofs, uint64_t len)
>> .addr_width = (_addr_width),\
>> .flags = (_flags),
>>
>> +#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)   \
>> +   .id = { \
>> +   ((_jedec_id) >> 16) & 0xff, \
>> +   ((_jedec_id) >> 8) & 0xff,  \
>> +   (_jedec_id) & 0xff  \
>> +   },  \
>> +   .id_len = 3,\
>> +   .sector_size = (8*_page_size),  \
>> +   .n_sectors = (_n_sectors),  \
>> +   .page_size = _page_size,\
>> + 

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

ping?


On Fri, Jan 15, 2016 at 5:25 PM, Ricardo Ribalda Delgado
 wrote:
> Xilinx Spartan-3AN contain an embedded spi device where they keep their
> configuration data and optionally some user data.
>
> The protocol of this flash follows most of the spi-nor standard. With
> the following differences:
>
> - Page size might not be a power of two.
> - The address calculation.
> - The spi nor commands used.
>
> Signed-off-by: Ricardo Ribalda Delgado 
> ---
>  drivers/mtd/devices/m25p80.c  |  3 ++
>  drivers/mtd/spi-nor/spi-nor.c | 97 
> +--
>  include/linux/mtd/spi-nor.h   | 13 ++
>  3 files changed, 110 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c9c3b7fa3051..869576735978 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -49,6 +49,9 @@ static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 
> *val, int len)
>
>  static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>  {
> +   if (nor->addr_convert)
> +   addr = nor->addr_convert(nor, addr);
> +
> /* opcode is in cmd[0] */
> cmd[1] = addr >> (nor->addr_width * 8 -  8);
> cmd[2] = addr >> (nor->addr_width * 8 - 16);
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index ed0c19c558b5..90215f053916 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -69,6 +69,7 @@ struct flash_info {
>  #defineSPI_NOR_DUAL_READ   0x20/* Flash supports Dual Read */
>  #defineSPI_NOR_QUAD_READ   0x40/* Flash supports Quad Read */
>  #defineUSE_FSR 0x80/* use flag status register */
> +#defineSPI_S3AN0x100   /* Xililnx S3AN Embedded SPI 
> nor*/
>  };
>
>  #define JEDEC_MFR(info)((info)->id[0])
> @@ -211,6 +212,21 @@ static inline int set_4byte(struct spi_nor *nor, const 
> struct flash_info *info,
> return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
> }
>  }
> +
> +static inline int s3an_sr_ready(struct spi_nor *nor)
> +{
> +   int ret;
> +   u8 val;
> +
> +   ret = nor->read_reg(nor, SPINOR_OP_XRDSR, , 1);
> +   if (ret < 0) {
> +   pr_err("error %d reading SR\n", (int) ret);
> +   return ret;
> +   }
> +
> +   return !!(val & XSR_RDY);
> +}
> +
>  static inline int spi_nor_sr_ready(struct spi_nor *nor)
>  {
> int sr = read_sr(nor);
> @@ -232,7 +248,7 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
>  static int spi_nor_ready(struct spi_nor *nor)
>  {
> int sr, fsr;
> -   sr = spi_nor_sr_ready(nor);
> +   sr = nor->sr_ready(nor);
> if (sr < 0)
> return sr;
> fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
> @@ -328,6 +344,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 
> addr)
>  * Default implementation, if driver doesn't have a specialized HW
>  * control
>  */
> +   if (nor->addr_convert)
> +   addr = nor->addr_convert(nor, addr);
> +
> for (i = nor->addr_width - 1; i >= 0; i--) {
> buf[i] = addr & 0xff;
> addr >>= 8;
> @@ -362,7 +381,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct 
> erase_info *instr)
> return ret;
>
> /* whole-chip erase? */
> -   if (len == mtd->size) {
> +   if (len == mtd->size && !(nor->flags & SNOR_NO_OP_CHIP_ERASE)) {
> unsigned long timeout;
>
> write_enable(nor);
> @@ -680,6 +699,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, 
> loff_t ofs, uint64_t len)
> .addr_width = (_addr_width),\
> .flags = (_flags),
>
> +#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)   \
> +   .id = { \
> +   ((_jedec_id) >> 16) & 0xff, \
> +   ((_jedec_id) >> 8) & 0xff,  \
> +   (_jedec_id) & 0xff  \
> +   },  \
> +   .id_len = 3,\
> +   .sector_size = (8*_page_size),  \
> +   .n_sectors = (_n_sectors),  \
> +   .page_size = _page_size,\
> +   .addr_width = 3,\
> +   .flags = SPI_NOR_NO_FR | SPI_S3AN,
> +
>  /* NOTE: double check command sets and memory organization when you add
>   * more nor chips.  This current list focusses on newer chips, which
>   * have been converging on command sets which 

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

ping?


On Fri, Jan 15, 2016 at 5:25 PM, Ricardo Ribalda Delgado
 wrote:
> Xilinx Spartan-3AN contain an embedded spi device where they keep their
> configuration data and optionally some user data.
>
> The protocol of this flash follows most of the spi-nor standard. With
> the following differences:
>
> - Page size might not be a power of two.
> - The address calculation.
> - The spi nor commands used.
>
> Signed-off-by: Ricardo Ribalda Delgado 
> ---
>  drivers/mtd/devices/m25p80.c  |  3 ++
>  drivers/mtd/spi-nor/spi-nor.c | 97 
> +--
>  include/linux/mtd/spi-nor.h   | 13 ++
>  3 files changed, 110 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c9c3b7fa3051..869576735978 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -49,6 +49,9 @@ static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 
> *val, int len)
>
>  static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>  {
> +   if (nor->addr_convert)
> +   addr = nor->addr_convert(nor, addr);
> +
> /* opcode is in cmd[0] */
> cmd[1] = addr >> (nor->addr_width * 8 -  8);
> cmd[2] = addr >> (nor->addr_width * 8 - 16);
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index ed0c19c558b5..90215f053916 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -69,6 +69,7 @@ struct flash_info {
>  #defineSPI_NOR_DUAL_READ   0x20/* Flash supports Dual Read */
>  #defineSPI_NOR_QUAD_READ   0x40/* Flash supports Quad Read */
>  #defineUSE_FSR 0x80/* use flag status register */
> +#defineSPI_S3AN0x100   /* Xililnx S3AN Embedded SPI 
> nor*/
>  };
>
>  #define JEDEC_MFR(info)((info)->id[0])
> @@ -211,6 +212,21 @@ static inline int set_4byte(struct spi_nor *nor, const 
> struct flash_info *info,
> return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
> }
>  }
> +
> +static inline int s3an_sr_ready(struct spi_nor *nor)
> +{
> +   int ret;
> +   u8 val;
> +
> +   ret = nor->read_reg(nor, SPINOR_OP_XRDSR, , 1);
> +   if (ret < 0) {
> +   pr_err("error %d reading SR\n", (int) ret);
> +   return ret;
> +   }
> +
> +   return !!(val & XSR_RDY);
> +}
> +
>  static inline int spi_nor_sr_ready(struct spi_nor *nor)
>  {
> int sr = read_sr(nor);
> @@ -232,7 +248,7 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
>  static int spi_nor_ready(struct spi_nor *nor)
>  {
> int sr, fsr;
> -   sr = spi_nor_sr_ready(nor);
> +   sr = nor->sr_ready(nor);
> if (sr < 0)
> return sr;
> fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
> @@ -328,6 +344,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 
> addr)
>  * Default implementation, if driver doesn't have a specialized HW
>  * control
>  */
> +   if (nor->addr_convert)
> +   addr = nor->addr_convert(nor, addr);
> +
> for (i = nor->addr_width - 1; i >= 0; i--) {
> buf[i] = addr & 0xff;
> addr >>= 8;
> @@ -362,7 +381,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct 
> erase_info *instr)
> return ret;
>
> /* whole-chip erase? */
> -   if (len == mtd->size) {
> +   if (len == mtd->size && !(nor->flags & SNOR_NO_OP_CHIP_ERASE)) {
> unsigned long timeout;
>
> write_enable(nor);
> @@ -680,6 +699,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, 
> loff_t ofs, uint64_t len)
> .addr_width = (_addr_width),\
> .flags = (_flags),
>
> +#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)   \
> +   .id = { \
> +   ((_jedec_id) >> 16) & 0xff, \
> +   ((_jedec_id) >> 8) & 0xff,  \
> +   (_jedec_id) & 0xff  \
> +   },  \
> +   .id_len = 3,\
> +   .sector_size = (8*_page_size),  \
> +   .n_sectors = (_n_sectors),  \
> +   .page_size = _page_size,\
> +   .addr_width = 3,\
> +   .flags = SPI_NOR_NO_FR | SPI_S3AN,
> +
>  /* NOTE: double check command sets and memory organization when you add
>   * more nor chips.  This current list focusses on newer chips, which
>