[U-Boot] [PATCH v2 0/7] dm: x86: PCI/SPI fixes for minnowboard MAX

[Simon Glass]

Hi Andrew,

On 2 July 2015 at 13:59, Andrew Bradford <andrew at bradfordembedded.com> wrote:
> Hi Simon,
>
> On 06/29 09:07, Andrew Bradford wrote:
>> On 06/25 11:55, Simon Glass wrote:
>> > The SPI flash starts off protected on baytrail. The code which is supposed to
>> > fix this is broken. This series fixes that, enables the SPI environment and
>> > adds documentation.
>> >
>> > Also when driver model is enabled for PCI some bugs appear. This series fixes
>> > those and enables driver model for PCI on minnowboard MAX.
>> >
>> > Changes in v2:
>> > - Continue to use writew for ICH7
>> > - Use ich_read/write() for BIOS protection update
>> > - Fix typos in README.x86
>> > - Rename the ops and ids arrays for consistency
>> > - Drop the coreboot PCI driver which is no-longer needed
>> > - Only limit the PCI system memory region on x86 machines
>> >
>> > Simon Glass (7):
>> >   dm: spi: Correct status register access width
>> >   dm: spi: Correct BIOS protection logic for ICH9
>> >   dm: spi: Enable environment for minnowmax
>> >   x86: Add ROM image description for minnowmax
>> >   x86: pci: Tidy up the generic x86 PCI driver
>> >   dm: x86: minnowmax: Move PCI to use driver model
>> >   dm: x86: baytrail: Correct PCI region 3 when driver model is used
>> >
>> >  arch/x86/cpu/baytrail/Makefile    |  1 -
>> >  arch/x86/cpu/baytrail/pci.c       | 46 ---------------------------------------
>> >  arch/x86/cpu/coreboot/pci.c       | 21 ------------------
>> >  arch/x86/cpu/cpu.c                |  1 +
>> >  arch/x86/dts/minnowmax.dts        | 10 +++++++++
>> >  common/board_f.c                  |  4 ++++
>> >  configs/minnowmax_defconfig       |  1 +
>> >  doc/README.x86                    | 17 +++++++++++++++
>> >  drivers/pci/pci-uclass.c          |  8 +++++--
>> >  drivers/pci/pci_x86.c             | 13 ++++++-----
>> >  drivers/spi/ich.c                 | 15 ++++++++-----
>> >  include/asm-generic/global_data.h |  1 +
>> >  include/configs/minnowmax.h       |  6 ++---
>> >  13 files changed, 60 insertions(+), 84 deletions(-)
>> >  delete mode 100644 arch/x86/cpu/baytrail/pci.c
>>
>> Tested-by: Andrew Bradford <andrew.bradford at kodakalaris.com>
>>
>> I don't actually have a Minnowmax board or any E3800 board that has the
>> type of SPI flash that's on Minnowmax, but when I try to do an `env
>> save` it just seems to hang unless I interrupt it with ^C (output
>> below). Even waiting 15 minutes for the env write to complete doesn't
>> help (I don't currently have a logic analyzer hooked up, sorry).
>>
>> I'm running the minnowmax_defconfig but adjusted only to handle that I
>> have a SODIMM installed so telling FSP to set memory_down to 0 (hence 1
>> GB of memory).  My board is close enough to Minnow Max to be useful
>> (patches to support it hopefully coming real soon now).
>>
>> But the PCI bits seem to be working for me! :)
>
> <snip>
>
> I've enabled debug output and with this patchset from you I believe I'm
> able to erase and read the SPI flash fine, I just cannot write to it.
> That I seem to be able to do erases but not writes confuses me.
>
> This is tested on an E3845 based board that's fairly similar to
> Minnowmax but uses a different SPI flash.  I know my SPI flash is OK,
> I'm able to program it with a Dediprog SF100 programmer without issue.
>
> A debug output enabled log is below.  It seems that my writes are
> resulting in the FCERR bit being set in the Software Sequencing Flash
> Status Register, indicating that either I'm violating a protection
> policy or I'm writing to a programmed cycle register when a programming
> access is already in progress.  I do not believe I'm writing to any
> registers during a programming operation, so it seems I'm violating some
> kind of protection but all of the protections I see in the data sheet
> disallow writes *AND* erases, so I'm a bit confused.
>
> drivers/spi/ich.c and the sequence of events to do writes
> all seems correct to me as I understand the Bay Trail datasheet.
>
> Any pointers on what I could look at to try to better understand why
> FCERR is getting set during writes?

I saw the same thing but put it down to the protection still being in
place in certain areas - it has hit me before and I went through a
similar process as you did here. But actually it's a bug - I switched
the parameters of the function around. I'll send an update series
soon. Thanks for testing this and finding the problem.

>
> Thanks,
> Andrew

[snip]