[U-Boot] Macronix NOR_SPI and Quad I/O

[U-Boot] Macronix NOR_SPI and Quad I/O

[Champ, Andy]

Hi all,


in the table in drivers/mtd/spi/spi_flash_ids.c there is a flag WR_QPP set against Macronix devices (including the ones Dumitru is just adding).


This is used when programming the devices on a 4-bit bus to select the command to use for programming - either CMD_QUAD_PAGE_PROGRAM (0x32) or CMD_PAGE_PROGRAM (0x2).


The Macronix devices that I have a spec for do not mention command 0x32. Each of the devices that I have a spec for ( MX25L25635F MX25U51245G MX25V8035F and MX25V1635F ) use command 0x38 instead.


This would suggest to me that devices that support quad I/O, and have Macronix NOR-SPI, won't be able to program them.


Am I missing something, or is this a problem?


I'm not able to check because we don't use Quad I/O.


Regards

Andy Champ

Amazon Lab126

Cambridge, UK.

[U-Boot] Macronix NOR_SPI and Quad I/O

[Jagan Teki]

On Fri, Nov 25, 2016 at 10:07 PM, Champ, Andy <andycham@amazon.co.uk> wrote:
> Hi all,
>
>
> in the table in drivers/mtd/spi/spi_flash_ids.c there is a flag WR_QPP set against Macronix devices (including the ones Dumitru is just adding).
>
>
> This is used when programming the devices on a 4-bit bus to select the command to use for programming - either CMD_QUAD_PAGE_PROGRAM (0x32) or CMD_PAGE_PROGRAM (0x2).
>
>
> The Macronix devices that I have a spec for do not mention command 0x32. Each of the devices that I have a spec for ( MX25L25635F MX25U51245G MX25V8035F and MX25V1635F ) use command 0x38 instead.

We need to fix this, till now no Macronix has been tested with QUAD I
think, please send the suitable fix will review.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] Macronix NOR_SPI and Quad I/O

[Chin Liang See]

On Sab, 2016-11-26 at 08:43 +0530, Jagan Teki wrote:
> On Fri, Nov 25, 2016 at 10:07 PM, Champ, Andy <andycham@amazon.co.uk>
> wrote:
> > 
> > Hi all,
> > 
> > 
> > in the table in drivers/mtd/spi/spi_flash_ids.c there is a flag
> > WR_QPP set against Macronix devices (including the ones Dumitru is
> > just adding).
> > 
> > 
> > This is used when programming the devices on a 4-bit bus to select
> > the command to use for programming - either CMD_QUAD_PAGE_PROGRAM
> > (0x32) or CMD_PAGE_PROGRAM (0x2).
> > 
> > 
> > The Macronix devices that I have a spec for do not mention command
> > 0x32. Each of the devices that I have a spec for ( MX25L25635F
> > MX25U51245G MX25V8035F and MX25V1635F ) use command 0x38 instead.
> We need to fix this, till now no Macronix has been tested with QUAD I
> think, please send the suitable fix will review.
> 

Too bad that I don't have any Macronix part with me too.

Thanks
Chin Liang

> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.
> 
> ________________________________
> 
> Confidentiality Notice.
> This message may contain information that is confidential or
> otherwise protected from disclosure. If you are not the intended
> recipient, you are hereby notified that any use, disclosure,
> dissemination, distribution, or copying of this message, or any
> attachments, is strictly prohibited. If you have received this
> message in error, please advise the sender by reply e-mail, and
> delete the message and any attachments. Thank you.

[U-Boot] Macronix NOR_SPI and Quad I/O

[Champ, Andy]

I don't have anything that does Quad IO, so I can't test it either.

I think the best thing to do is to disable it for the Macronix parts, and put a comment in the table explaining why. With any luck one day somebody somewhere will have a device with a Macronix chip and quad I/O, and they can do it properly.

If this sounds good I'll put a patch together, run it past our open source lawyers (I'm not allowed to just push!) and send it through.

Alternatively I could put together a patch that sets a different flag bit for Macronix Quad I/O and push that. The only thing is of course that I have no way to test it, and as we all know untested code never works.

Which way should I go?

Regards
Andy Champ
________________________________________
From: Jagan Teki <jagan@openedev.com>
Sent: 26 November 2016 03:13
To: Champ, Andy
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com; dumitru.bacrau at intel.com
Subject: Re: [U-Boot] Macronix NOR_SPI and Quad I/O

On Fri, Nov 25, 2016 at 10:07 PM, Champ, Andy <andycham@amazon.co.uk> wrote:
> Hi all,
>
>
> in the table in drivers/mtd/spi/spi_flash_ids.c there is a flag WR_QPP set against Macronix devices (including the ones Dumitru is just adding).
>
>
> This is used when programming the devices on a 4-bit bus to select the command to use for programming - either CMD_QUAD_PAGE_PROGRAM (0x32) or CMD_PAGE_PROGRAM (0x2).
>
>
> The Macronix devices that I have a spec for do not mention command 0x32. Each of the devices that I have a spec for ( MX25L25635F MX25U51245G MX25V8035F and MX25V1635F ) use command 0x38 instead.

We need to fix this, till now no Macronix has been tested with QUAD I
think, please send the suitable fix will review.

thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] Macronix NOR_SPI and Quad I/O

[Bacrau, Dumitru]

Hi guys,

I do have Macronix parts and Quad capability. Unfortunately I am not using the latest U-Boot code (but looks similar) and also I do not have much time today and tomorrow.

Would it be OK if you waited until Wednesday so I can investigate and test this? If it is urgent go ahead and do what you need to, then we can add Quad later.

Thanks,
Radu

-----Original Message-----
From: Champ, Andy [mailto:andycham at amazon.co.uk] 
Sent: Monday, November 28, 2016 4:00 AM
To: Jagan Teki <jagan@openedev.com>
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com; Bacrau, Dumitru <dumitru.bacrau@intel.com>
Subject: Re: [U-Boot] Macronix NOR_SPI and Quad I/O

I don't have anything that does Quad IO, so I can't test it either.

I think the best thing to do is to disable it for the Macronix parts, and put a comment in the table explaining why. With any luck one day somebody somewhere will have a device with a Macronix chip and quad I/O, and they can do it properly.

If this sounds good I'll put a patch together, run it past our open source lawyers (I'm not allowed to just push!) and send it through.

Alternatively I could put together a patch that sets a different flag bit for Macronix Quad I/O and push that. The only thing is of course that I have no way to test it, and as we all know untested code never works.

Which way should I go?

Regards
Andy Champ
________________________________________
From: Jagan Teki <jagan@openedev.com>
Sent: 26 November 2016 03:13
To: Champ, Andy
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com; dumitru.bacrau at intel.com
Subject: Re: [U-Boot] Macronix NOR_SPI and Quad I/O

On Fri, Nov 25, 2016 at 10:07 PM, Champ, Andy <andycham@amazon.co.uk> wrote:
> Hi all,
>
>
> in the table in drivers/mtd/spi/spi_flash_ids.c there is a flag WR_QPP set against Macronix devices (including the ones Dumitru is just adding).
>
>
> This is used when programming the devices on a 4-bit bus to select the command to use for programming - either CMD_QUAD_PAGE_PROGRAM (0x32) or CMD_PAGE_PROGRAM (0x2).
>
>
> The Macronix devices that I have a spec for do not mention command 0x32. Each of the devices that I have a spec for ( MX25L25635F MX25U51245G MX25V8035F and MX25V1635F ) use command 0x38 instead.

We need to fix this, till now no Macronix has been tested with QUAD I think, please send the suitable fix will review.

thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India.

[U-Boot] Macronix NOR_SPI and Quad I/O

[Champ, Andy]

Hi Radu,
As far as I am concerned there is no urgency at all.
I just happened to notice as I was looking through the code that the command does not agree with the Macronix device spec, and I thought I should tell someone! Our device does not support Quad I/O.

Thanks
Andy Champ

-----Original Message-----
From: Bacrau, Dumitru [mailto:dumitru.bacrau at intel.com] 
Sent: 28 November 2016 15:50
To: Champ, Andy <andycham@amazon.co.uk>; Jagan Teki <jagan@openedev.com>
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com
Subject: RE: [U-Boot] Macronix NOR_SPI and Quad I/O

Hi guys,

I do have Macronix parts and Quad capability. Unfortunately I am not using the latest U-Boot code (but looks similar) and also I do not have much time today and tomorrow.

Would it be OK if you waited until Wednesday so I can investigate and test this? If it is urgent go ahead and do what you need to, then we can add Quad later.

Thanks,
Radu

-----Original Message-----
From: Champ, Andy [mailto:andycham at amazon.co.uk] 
Sent: Monday, November 28, 2016 4:00 AM
To: Jagan Teki <jagan@openedev.com>
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com; Bacrau, Dumitru <dumitru.bacrau@intel.com>
Subject: Re: [U-Boot] Macronix NOR_SPI and Quad I/O

I don't have anything that does Quad IO, so I can't test it either.

I think the best thing to do is to disable it for the Macronix parts, and put a comment in the table explaining why. With any luck one day somebody somewhere will have a device with a Macronix chip and quad I/O, and they can do it properly.

If this sounds good I'll put a patch together, run it past our open source lawyers (I'm not allowed to just push!) and send it through.

Alternatively I could put together a patch that sets a different flag bit for Macronix Quad I/O and push that. The only thing is of course that I have no way to test it, and as we all know untested code never works.

Which way should I go?

Regards
Andy Champ
________________________________________
From: Jagan Teki <jagan@openedev.com>
Sent: 26 November 2016 03:13
To: Champ, Andy
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com; dumitru.bacrau at intel.com
Subject: Re: [U-Boot] Macronix NOR_SPI and Quad I/O

On Fri, Nov 25, 2016 at 10:07 PM, Champ, Andy <andycham@amazon.co.uk> wrote:
> Hi all,
>
>
> in the table in drivers/mtd/spi/spi_flash_ids.c there is a flag WR_QPP set against Macronix devices (including the ones Dumitru is just adding).
>
>
> This is used when programming the devices on a 4-bit bus to select the command to use for programming - either CMD_QUAD_PAGE_PROGRAM (0x32) or CMD_PAGE_PROGRAM (0x2).
>
>
> The Macronix devices that I have a spec for do not mention command 0x32. Each of the devices that I have a spec for ( MX25L25635F MX25U51245G MX25V8035F and MX25V1635F ) use command 0x38 instead.

We need to fix this, till now no Macronix has been tested with QUAD I think, please send the suitable fix will review.

thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India.

[U-Boot] Macronix NOR_SPI and Quad I/O

[Bacrau, Dumitru]

Hi Andy,

I have double-checked the datasheets and indeed the Quad Page Program opcode for Macronix is 0x38 and not 0x32 like the U-Boot code has defined in CMD_QUAD_PAGE_PROGRAM.

My version of the code is similar to the current u-boot mainline, and it also has the WR_QPP flag. But its usage depends on another flag that never gets set, so basically I have never used Quad mode for writing in my testing.

In the latest U-Boot code from u-boot-spi the situation is similar, except the above parameter is retrieved from the U-Boot device tree parameter  "spi-tx-bus-width". But none of the device trees set that parameter to '4' so again the Quad mode is never used for writing anyway.

Note that Quad mode is very useful for reading, where it effectively quadruples the speed. But in case of writing, it only speeds up data transmission to the flash device, which then starts writing, and the host keeps polling it for completion. Because of this, there is not a lot to gain from using quad mode for writing.

Ideally we would need a way to convey that the Macronix devices use a different instruction opcode for quad writing. This could be in spi_flash_info structure or somewhere in the device tree.

Regards,
Radu

-----Original Message-----
From: Champ, Andy [mailto:andycham at amazon.co.uk] 
Sent: Monday, November 28, 2016 9:59 AM
To: Bacrau, Dumitru <dumitru.bacrau@intel.com>; Jagan Teki <jagan@openedev.com>
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com
Subject: RE: [U-Boot] Macronix NOR_SPI and Quad I/O

Hi Radu,
As far as I am concerned there is no urgency at all.
I just happened to notice as I was looking through the code that the command does not agree with the Macronix device spec, and I thought I should tell someone! Our device does not support Quad I/O.

Thanks
Andy Champ

-----Original Message-----
From: Bacrau, Dumitru [mailto:dumitru.bacrau at intel.com] 
Sent: 28 November 2016 15:50
To: Champ, Andy <andycham@amazon.co.uk>; Jagan Teki <jagan@openedev.com>
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com
Subject: RE: [U-Boot] Macronix NOR_SPI and Quad I/O

Hi guys,

I do have Macronix parts and Quad capability. Unfortunately I am not using the latest U-Boot code (but looks similar) and also I do not have much time today and tomorrow.

Would it be OK if you waited until Wednesday so I can investigate and test this? If it is urgent go ahead and do what you need to, then we can add Quad later.

Thanks,
Radu

-----Original Message-----
From: Champ, Andy [mailto:andycham at amazon.co.uk] 
Sent: Monday, November 28, 2016 4:00 AM
To: Jagan Teki <jagan@openedev.com>
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com; Bacrau, Dumitru <dumitru.bacrau@intel.com>
Subject: Re: [U-Boot] Macronix NOR_SPI and Quad I/O

I don't have anything that does Quad IO, so I can't test it either.

I think the best thing to do is to disable it for the Macronix parts, and put a comment in the table explaining why. With any luck one day somebody somewhere will have a device with a Macronix chip and quad I/O, and they can do it properly.

If this sounds good I'll put a patch together, run it past our open source lawyers (I'm not allowed to just push!) and send it through.

Alternatively I could put together a patch that sets a different flag bit for Macronix Quad I/O and push that. The only thing is of course that I have no way to test it, and as we all know untested code never works.

Which way should I go?

Regards
Andy Champ
________________________________________
From: Jagan Teki <jagan@openedev.com>
Sent: 26 November 2016 03:13
To: Champ, Andy
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com; dumitru.bacrau at intel.com
Subject: Re: [U-Boot] Macronix NOR_SPI and Quad I/O

On Fri, Nov 25, 2016 at 10:07 PM, Champ, Andy <andycham@amazon.co.uk> wrote:
> Hi all,
>
>
> in the table in drivers/mtd/spi/spi_flash_ids.c there is a flag WR_QPP set against Macronix devices (including the ones Dumitru is just adding).
>
>
> This is used when programming the devices on a 4-bit bus to select the command to use for programming - either CMD_QUAD_PAGE_PROGRAM (0x32) or CMD_PAGE_PROGRAM (0x2).
>
>
> The Macronix devices that I have a spec for do not mention command 0x32. Each of the devices that I have a spec for ( MX25L25635F MX25U51245G MX25V8035F and MX25V1635F ) use command 0x38 instead.

We need to fix this, till now no Macronix has been tested with QUAD I think, please send the suitable fix will review.

thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India.

[U-Boot] Macronix NOR_SPI and Quad I/O

[Champ, Andy]

Hi Radu,

It seems to me there are two approaches:
- Set a different flag for this different command
- Check the manufacturer ID before selecting the command.
Either way the data we'd need is in the params variable, nicely to hand.

The code I've been working on is for write protecting Macronix and WInbond NOR-SPI devices; I'll publish that at some point. The write protect code has callbacks, which probably makes sense when the handling is more complex - the commands, and the areas you can protect, are both different on a per-manufacturer basis.

A flag is nice and simple, which is good given that nobody needs this right now. I do wonder if quad writes are worth doing at all - in which case the fix is /really/ simple. Remove the code, and the flag!

As I said, I don't need this - it's just that I don't like to see broken code lying around.

Thanks
Andy Champ

-----Original Message-----
From: Bacrau, Dumitru [mailto:dumitru.bacrau at intel.com] 
Sent: 28 November 2016 17:33
To: Champ, Andy <andycham@amazon.co.uk>; Jagan Teki <jagan@openedev.com>
Cc: u-boot at lists.denx.de; jteki at openedev.com; radu.bacrau at gmail.com; clsee at altera.com
Subject: RE: [U-Boot] Macronix NOR_SPI and Quad I/O

Hi Andy,

I have double-checked the datasheets and indeed the Quad Page Program opcode for Macronix is 0x38 and not 0x32 like the U-Boot code has defined in CMD_QUAD_PAGE_PROGRAM.

My version of the code is similar to the current u-boot mainline, and it also has the WR_QPP flag. But its usage depends on another flag that never gets set, so basically I have never used Quad mode for writing in my testing.

In the latest U-Boot code from u-boot-spi the situation is similar, except the above parameter is retrieved from the U-Boot device tree parameter  "spi-tx-bus-width". But none of the device trees set that parameter to '4' so again the Quad mode is never used for writing anyway.

Note that Quad mode is very useful for reading, where it effectively quadruples the speed. But in case of writing, it only speeds up data transmission to the flash device, which then starts writing, and the host keeps polling it for completion. Because of this, there is not a lot to gain from using quad mode for writing.

Ideally we would need a way to convey that the Macronix devices use a different instruction opcode for quad writing. This could be in spi_flash_info structure or somewhere in the device tree.

Regards,
Radu