[U-Boot] [PATCH v2 00/12] spi: sf: ICH SPI driver fix and flash params update
This series fix several bugs in current ICH SPI driver as well as adding byte program support for the SST25* flash.
Flash params are updated to explicitly list supported read commands and change flash sector size to 4KiB as long as flash supports sector erase (20h) command.
Changes for v2: - Rebased to u-boot-spi/mater. - Reviewed and updated the params of all currently supported flash parts per their datasheets. - Corrected AT25DF321 JEDEC ID. - Corrected Atmel bulk erase command to 50h instead of D8h. - Added AT25DF321A, W25X10, W25X20, W25X80 params.
Bin Meng (12): spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address spi/ich.c: Set the rx operation mode for ich 7 spi: sf: Support byte program for sst spi flash sf: Update SST flash params sf: Update Atmel flash params sf: Update EON flash params sf: Update GigaDevice flash params sf: Update Macronix flash params sf: Update Spansion flash params sf: Update Micron flash params sf: Update Winbond flash params sf: Give proper spacing between flash table params
drivers/mtd/spi/sf_internal.h | 11 ++- drivers/mtd/spi/sf_ops.c | 31 +++++++ drivers/mtd/spi/sf_params.c | 186 ++++++++++++++++++++++-------------------- drivers/mtd/spi/sf_probe.c | 12 ++- drivers/spi/ich.c | 26 +++--- include/spi.h | 1 + 6 files changed, 159 insertions(+), 108 deletions(-)
On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote:
This series fix several bugs in current ICH SPI driver as well as adding byte program support for the SST25* flash.
Flash params are updated to explicitly list supported read commands and change flash sector size to 4KiB as long as flash supports sector erase (20h) command.
Changes for v2:
- Rebased to u-boot-spi/mater.
- Reviewed and updated the params of all currently supported flash parts per their datasheets.
- Corrected AT25DF321 JEDEC ID.
- Corrected Atmel bulk erase command to 50h instead of D8h.
- Added AT25DF321A, W25X10, W25X20, W25X80 params.
Bin Meng (12): spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address spi/ich.c: Set the rx operation mode for ich 7 spi: sf: Support byte program for sst spi flash sf: Update SST flash params sf: Update Atmel flash params sf: Update EON flash params sf: Update GigaDevice flash params sf: Update Macronix flash params sf: Update Spansion flash params sf: Update Micron flash params sf: Update Winbond flash params sf: Give proper spacing between flash table params
I think you combined two or more changes(unrelated) in a common patches and Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Please fix those and send me one more.
Mean while I will look at your scenario like you're controller only supports AS, As I said before as AS of AF both are similar way of transferring except the dummy bits passing from the driver, try to see the fix on driver point of of instead of digging common sf stuff.
drivers/mtd/spi/sf_internal.h | 11 ++- drivers/mtd/spi/sf_ops.c | 31 +++++++ drivers/mtd/spi/sf_params.c | 186 ++++++++++++++++++++++-------------------- drivers/mtd/spi/sf_probe.c | 12 ++- drivers/spi/ich.c | 26 +++--- include/spi.h | 1 + 6 files changed, 159 insertions(+), 108 deletions(-)
-- 1.8.2.1
thanks!
Hi Jagan,
On Wed, Nov 5, 2014 at 5:21 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote:
This series fix several bugs in current ICH SPI driver as well as adding byte program support for the SST25* flash.
Flash params are updated to explicitly list supported read commands and change flash sector size to 4KiB as long as flash supports sector erase (20h) command.
Changes for v2:
- Rebased to u-boot-spi/mater.
- Reviewed and updated the params of all currently supported flash parts per their datasheets.
- Corrected AT25DF321 JEDEC ID.
- Corrected Atmel bulk erase command to 50h instead of D8h.
- Added AT25DF321A, W25X10, W25X20, W25X80 params.
Bin Meng (12): spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address spi/ich.c: Set the rx operation mode for ich 7 spi: sf: Support byte program for sst spi flash sf: Update SST flash params sf: Update Atmel flash params sf: Update EON flash params sf: Update GigaDevice flash params sf: Update Macronix flash params sf: Update Spansion flash params sf: Update Micron flash params sf: Update Winbond flash params sf: Give proper spacing between flash table params
I think you combined two or more changes(unrelated) in a common patches and Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Do you mean I should let PATCH 1/2/3 go as a separate patch set? Since these 3 are tested on my x86 board, could it be Simon to pick up these patches instead of through the u-boot-spi? Also I don't understand you comments about "adding bulk erase support in e_cmd_rd is not correct". The e_cmd_rd in sf_params is updated to specify all supported read commands the flash can support. There is no bulk erase here.
Please fix those and send me one more.
Mean while I will look at your scenario like you're controller only supports AS, As I said before as AS of AF both are similar way of transferring except the dummy bits passing from the driver, try to see the fix on driver point of of instead of digging common sf stuff.
Fixing on the driver part might be possible, might be not. Even though it is possible, I don't want to do that as the ICH manual explicitly says fast read command (0Bh) is not supported by the controller. As far as I can test, actually all of the commands which require an additional dummy byte after the address cycle fail to work. The matches what the manual says.
Regards, Bin
Hi Jagan,
On Wed, Nov 5, 2014 at 10:56 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 5:21 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote:
This series fix several bugs in current ICH SPI driver as well as adding byte program support for the SST25* flash.
Flash params are updated to explicitly list supported read commands and change flash sector size to 4KiB as long as flash supports sector erase (20h) command.
Changes for v2:
- Rebased to u-boot-spi/mater.
- Reviewed and updated the params of all currently supported flash parts per their datasheets.
- Corrected AT25DF321 JEDEC ID.
- Corrected Atmel bulk erase command to 50h instead of D8h.
- Added AT25DF321A, W25X10, W25X20, W25X80 params.
Bin Meng (12): spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address spi/ich.c: Set the rx operation mode for ich 7 spi: sf: Support byte program for sst spi flash sf: Update SST flash params sf: Update Atmel flash params sf: Update EON flash params sf: Update GigaDevice flash params sf: Update Macronix flash params sf: Update Spansion flash params sf: Update Micron flash params sf: Update Winbond flash params sf: Give proper spacing between flash table params
I think you combined two or more changes(unrelated) in a common patches and Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Do you mean I should let PATCH 1/2/3 go as a separate patch set? Since these 3 are tested on my x86 board, could it be Simon to pick up these patches instead of through the u-boot-spi? Also I don't understand you comments about "adding bulk erase support in e_cmd_rd is not correct". The e_cmd_rd in sf_params is updated to specify all supported read commands the flash can support. There is no bulk erase here.
Please fix those and send me one more.
Mean while I will look at your scenario like you're controller only supports AS, As I said before as AS of AF both are similar way of transferring except the dummy bits passing from the driver, try to see the fix on driver point of of instead of digging common sf stuff.
Fixing on the driver part might be possible, might be not. Even though it is possible, I don't want to do that as the ICH manual explicitly says fast read command (0Bh) is not supported by the controller. As far as I can test, actually all of the commands which require an additional dummy byte after the address cycle fail to work. The matches what the manual says.
Regards, Bin
A gentle ping.
Regards, Bin
On 12 November 2014 07:57, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 10:56 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 5:21 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote:
This series fix several bugs in current ICH SPI driver as well as adding byte program support for the SST25* flash.
Flash params are updated to explicitly list supported read commands and change flash sector size to 4KiB as long as flash supports sector erase (20h) command.
Changes for v2:
- Rebased to u-boot-spi/mater.
- Reviewed and updated the params of all currently supported flash parts per their datasheets.
- Corrected AT25DF321 JEDEC ID.
- Corrected Atmel bulk erase command to 50h instead of D8h.
- Added AT25DF321A, W25X10, W25X20, W25X80 params.
Bin Meng (12): spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address spi/ich.c: Set the rx operation mode for ich 7 spi: sf: Support byte program for sst spi flash sf: Update SST flash params sf: Update Atmel flash params sf: Update EON flash params sf: Update GigaDevice flash params sf: Update Macronix flash params sf: Update Spansion flash params sf: Update Micron flash params sf: Update Winbond flash params sf: Give proper spacing between flash table params
I think you combined two or more changes(unrelated) in a common patches and Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Do you mean I should let PATCH 1/2/3 go as a separate patch set? Since these 3 are tested on my x86 board, could it be Simon to pick up these patches instead of through the u-boot-spi? Also I don't understand you comments about "adding bulk erase support in e_cmd_rd is not correct". The e_cmd_rd in sf_params is updated to specify all supported read commands the flash can support. There is no bulk erase here.
Please fix those and send me one more.
Mean while I will look at your scenario like you're controller only supports AS, As I said before as AS of AF both are similar way of transferring except the dummy bits passing from the driver, try to see the fix on driver point of of instead of digging common sf stuff.
Fixing on the driver part might be possible, might be not. Even though it is possible, I don't want to do that as the ICH manual explicitly says fast read command (0Bh) is not supported by the controller. As far as I can test, actually all of the commands which require an additional dummy byte after the address cycle fail to work. The matches what the manual says.
Regards, Bin
A gentle ping.
Will back soon, please give some time.
thanks!
Hi Jagan,
On Wed, Nov 12, 2014 at 3:04 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 12 November 2014 07:57, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 10:56 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 5:21 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote:
This series fix several bugs in current ICH SPI driver as well as adding byte program support for the SST25* flash.
Flash params are updated to explicitly list supported read commands and change flash sector size to 4KiB as long as flash supports sector erase (20h) command.
Changes for v2:
- Rebased to u-boot-spi/mater.
- Reviewed and updated the params of all currently supported flash parts per their datasheets.
- Corrected AT25DF321 JEDEC ID.
- Corrected Atmel bulk erase command to 50h instead of D8h.
- Added AT25DF321A, W25X10, W25X20, W25X80 params.
Bin Meng (12): spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address spi/ich.c: Set the rx operation mode for ich 7 spi: sf: Support byte program for sst spi flash sf: Update SST flash params sf: Update Atmel flash params sf: Update EON flash params sf: Update GigaDevice flash params sf: Update Macronix flash params sf: Update Spansion flash params sf: Update Micron flash params sf: Update Winbond flash params sf: Give proper spacing between flash table params
I think you combined two or more changes(unrelated) in a common patches and Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Do you mean I should let PATCH 1/2/3 go as a separate patch set? Since these 3 are tested on my x86 board, could it be Simon to pick up these patches instead of through the u-boot-spi? Also I don't understand you comments about "adding bulk erase support in e_cmd_rd is not correct". The e_cmd_rd in sf_params is updated to specify all supported read commands the flash can support. There is no bulk erase here.
Please fix those and send me one more.
Mean while I will look at your scenario like you're controller only supports AS, As I said before as AS of AF both are similar way of transferring except the dummy bits passing from the driver, try to see the fix on driver point of of instead of digging common sf stuff.
Fixing on the driver part might be possible, might be not. Even though it is possible, I don't want to do that as the ICH manual explicitly says fast read command (0Bh) is not supported by the controller. As far as I can test, actually all of the commands which require an additional dummy byte after the address cycle fail to work. The matches what the manual says.
Regards, Bin
A gentle ping.
Will back soon, please give some time.
Any update on this patch series?
Regards, Bin
Hi Jagan,
On Sun, Nov 23, 2014 at 9:43 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 12, 2014 at 3:04 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 12 November 2014 07:57, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 10:56 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 5:21 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote:
This series fix several bugs in current ICH SPI driver as well as adding byte program support for the SST25* flash.
Flash params are updated to explicitly list supported read commands and change flash sector size to 4KiB as long as flash supports sector erase (20h) command.
Changes for v2:
- Rebased to u-boot-spi/mater.
- Reviewed and updated the params of all currently supported flash parts per their datasheets.
- Corrected AT25DF321 JEDEC ID.
- Corrected Atmel bulk erase command to 50h instead of D8h.
- Added AT25DF321A, W25X10, W25X20, W25X80 params.
Bin Meng (12): spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address spi/ich.c: Set the rx operation mode for ich 7 spi: sf: Support byte program for sst spi flash sf: Update SST flash params sf: Update Atmel flash params sf: Update EON flash params sf: Update GigaDevice flash params sf: Update Macronix flash params sf: Update Spansion flash params sf: Update Micron flash params sf: Update Winbond flash params sf: Give proper spacing between flash table params
I think you combined two or more changes(unrelated) in a common patches and Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Do you mean I should let PATCH 1/2/3 go as a separate patch set? Since these 3 are tested on my x86 board, could it be Simon to pick up these patches instead of through the u-boot-spi? Also I don't understand you comments about "adding bulk erase support in e_cmd_rd is not correct". The e_cmd_rd in sf_params is updated to specify all supported read commands the flash can support. There is no bulk erase here.
Please fix those and send me one more.
Mean while I will look at your scenario like you're controller only supports AS, As I said before as AS of AF both are similar way of transferring except the dummy bits passing from the driver, try to see the fix on driver point of of instead of digging common sf stuff.
Fixing on the driver part might be possible, might be not. Even though it is possible, I don't want to do that as the ICH manual explicitly says fast read command (0Bh) is not supported by the controller. As far as I can test, actually all of the commands which require an additional dummy byte after the address cycle fail to work. The matches what the manual says.
Regards, Bin
A gentle ping.
Will back soon, please give some time.
Any update on this patch series?
Ping? Is there any chance this patch series will be merged into v2015.01? I see some other people are posting patches to add more SPI flash support in the parameter table which might do something different from what my patch series are trying to correct. Also I am going to post my x86 patches, and the SPI flash support on my x86 board requires the first 4 patches in this series.
Regards, Bin
+Tom
Hi Jagan,
On 2 December 2014 at 19:33, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Sun, Nov 23, 2014 at 9:43 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 12, 2014 at 3:04 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 12 November 2014 07:57, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 10:56 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 5:21 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote: > This series fix several bugs in current ICH SPI driver as well as > adding byte program support for the SST25* flash. > > Flash params are updated to explicitly list supported read commands > and change flash sector size to 4KiB as long as flash supports > sector erase (20h) command. > > Changes for v2: > - Rebased to u-boot-spi/mater. > - Reviewed and updated the params of all currently supported flash > parts per their datasheets. > - Corrected AT25DF321 JEDEC ID. > - Corrected Atmel bulk erase command to 50h instead of D8h. > - Added AT25DF321A, W25X10, W25X20, W25X80 params. > > > Bin Meng (12): > spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address > spi/ich.c: Set the rx operation mode for ich 7 > spi: sf: Support byte program for sst spi flash > sf: Update SST flash params > sf: Update Atmel flash params > sf: Update EON flash params > sf: Update GigaDevice flash params > sf: Update Macronix flash params > sf: Update Spansion flash params > sf: Update Micron flash params > sf: Update Winbond flash params > sf: Give proper spacing between flash table params
I think you combined two or more changes(unrelated) in a common patches and Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Do you mean I should let PATCH 1/2/3 go as a separate patch set? Since these 3 are tested on my x86 board, could it be Simon to pick up these patches instead of through the u-boot-spi? Also I don't understand you comments about "adding bulk erase support in e_cmd_rd is not correct". The e_cmd_rd in sf_params is updated to specify all supported read commands the flash can support. There is no bulk erase here.
Please fix those and send me one more.
Mean while I will look at your scenario like you're controller only supports AS, As I said before as AS of AF both are similar way of transferring except the dummy bits passing from the driver, try to see the fix on driver point of of instead of digging common sf stuff.
Fixing on the driver part might be possible, might be not. Even though it is possible, I don't want to do that as the ICH manual explicitly says fast read command (0Bh) is not supported by the controller. As far as I can test, actually all of the commands which require an additional dummy byte after the address cycle fail to work. The matches what the manual says.
Regards, Bin
A gentle ping.
Will back soon, please give some time.
Any update on this patch series?
Ping? Is there any chance this patch series will be merged into v2015.01? I see some other people are posting patches to add more SPI flash support in the parameter table which might do something different from what my patch series are trying to correct. Also I am going to post my x86 patches, and the SPI flash support on my x86 board requires the first 4 patches in this series.
I think we need to move forward with this series. We really should be trying to get outstanding patches applied now so there is sufficient time for testing before the release. From what I can see Bin has addressed your feedback on the v1 series.
Please let me know if you are going to apply this to SPI and pull through to mainline in the next day or so, otherwise I'll retest and take it through the x86 tree to avoid further delay. We have made good progress with x86 in this release but SPI is a critical piece.
Regards, Simon
Hi Simon,
On 4 December 2014 at 18:34, Simon Glass sjg@chromium.org wrote:
+Tom
Hi Jagan,
On 2 December 2014 at 19:33, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Sun, Nov 23, 2014 at 9:43 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 12, 2014 at 3:04 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
On 12 November 2014 07:57, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 10:56 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Jagan,
On Wed, Nov 5, 2014 at 5:21 AM, Jagan Teki jagannadh.teki@gmail.com wrote: > On 1 November 2014 14:23, Bin Meng bmeng.cn@gmail.com wrote: >> This series fix several bugs in current ICH SPI driver as well as >> adding byte program support for the SST25* flash. >> >> Flash params are updated to explicitly list supported read commands >> and change flash sector size to 4KiB as long as flash supports >> sector erase (20h) command. >> >> Changes for v2: >> - Rebased to u-boot-spi/mater. >> - Reviewed and updated the params of all currently supported flash >> parts per their datasheets. >> - Corrected AT25DF321 JEDEC ID. >> - Corrected Atmel bulk erase command to 50h instead of D8h. >> - Added AT25DF321A, W25X10, W25X20, W25X80 params. >> >> >> Bin Meng (12): >> spi/ich.c: Fix a bug of reading from a non-64 bytes aligned address >> spi/ich.c: Set the rx operation mode for ich 7 >> spi: sf: Support byte program for sst spi flash >> sf: Update SST flash params >> sf: Update Atmel flash params >> sf: Update EON flash params >> sf: Update GigaDevice flash params >> sf: Update Macronix flash params >> sf: Update Spansion flash params >> sf: Update Micron flash params >> sf: Update Winbond flash params >> sf: Give proper spacing between flash table params > > I think you combined two or more changes(unrelated) in a common patches and > Added Bulk erase support in e_cmd_rd of sf_params ie quite not correct.
Do you mean I should let PATCH 1/2/3 go as a separate patch set? Since these 3 are tested on my x86 board, could it be Simon to pick up these patches instead of through the u-boot-spi? Also I don't understand you comments about "adding bulk erase support in e_cmd_rd is not correct". The e_cmd_rd in sf_params is updated to specify all supported read commands the flash can support. There is no bulk erase here.
> Please fix those and send me one more. > > Mean while I will look at your scenario like you're controller only supports AS, > As I said before as AS of AF both are similar way of transferring > except the dummy > bits passing from the driver, try to see the fix on driver point of of > instead of digging > common sf stuff.
Fixing on the driver part might be possible, might be not. Even though it is possible, I don't want to do that as the ICH manual explicitly says fast read command (0Bh) is not supported by the controller. As far as I can test, actually all of the commands which require an additional dummy byte after the address cycle fail to work. The matches what the manual says.
Regards, Bin
A gentle ping.
Will back soon, please give some time.
Any update on this patch series?
Ping? Is there any chance this patch series will be merged into v2015.01? I see some other people are posting patches to add more SPI flash support in the parameter table which might do something different from what my patch series are trying to correct. Also I am going to post my x86 patches, and the SPI flash support on my x86 board requires the first 4 patches in this series.
I think we need to move forward with this series. We really should be trying to get outstanding patches applied now so there is sufficient time for testing before the release. From what I can see Bin has addressed your feedback on the v1 series.
Please let me know if you are going to apply this to SPI and pull through to mainline in the next day or so, otherwise I'll retest and take it through the x86 tree to avoid further delay. We have made good progress with x86 in this release but SPI is a critical piece.
Yes, I understand your concerns.
Bin, sent a series that critically effects the sf framework and I'm working on that now and if there are some critical's for you out of whole series.
Please send those at-least, I will review and at max will apply those in next week end.
thanks!
participants (3)
-
Bin Meng -
Jagan Teki -
Simon Glass