Re: [PATCH] mtd: spi-nor: fix options for mx66l51235f

[Alexander Amelkin <a.amelkin@yadro.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4829 bytes --]


28.08.2018 19:03, Marek Vasut wrote:
>
>>>> Probably it would be best not to merge this patch into Linux
>>>> kernel. I'll discuss it with colleagues from OpenPOWER community
>>>> and we'll probably fix it on SBE side.
>>> Can you fix the firmware ? If so, that'd be amazing.
>> It turns out that it is impossible to fix that on firmware side
>> mainly because the firmware does not send any opcodes over SPI at
>> all. It relies on the SPI flash controller to do the address mapping
>> so the firmware accesses the chip as a usual memory address range.
>> The flash controller is located in our case inside Aspeed AST2400 BMC
>> SoC and does not support 4B opcodes. It expects the chip to respond
>> to generic READ and WRITE opcodes with (configurable) either 3 or 4
>> address byte cycles. So the only way to make it work is put the chip
>> in EN4B mode when BMC boots.
> And who does that ? Or is the EN4B set by default and U-Boot/Linux
> _clears_ it , thus making the board unbootable ?
>
> Doesn't it simply mean your board is missing SPI NOR hardware reset line?
No, you're probably just missing it. In servers the BMC is always on when PSUs are connected to mains.
The flash chip in question is the one used to boot the host (OpenPOWER P8), not the BMC (ASpeed AST2400).
U-boot on AST2400 has nothing to do with the chip. When BMC boots, it loads a driver for the host's chip (because it is connected to the BMC SoC for the BMC to be able to upgrade the host's firmware), and the driver initializes the chip to the right mode. The BMC reads host firmware version info, etc. from the chip. It may also update the firmware or parts of it. Then it powers up the host and the host reads the chip via the memory window mapped on LPC bus by the BMC.
The BMC hardware (AST2400 in our case) translates memory transactions into SPI Flash commands.
Unfortunately for us, AST2400 (unlike newer models) only supports generic 3-byte commands (READ, READ FAST, PP, etc.), but has a 'hack' that allows for using 4-byte addresses with those commands. Thus if an over-16MB flash chip is connected (and the firmware image itself is over 16MB), and thus AST2400 is configured in 4-byte address cycle mode, it is crucial that the flash chip is in EN4B mode as well. That's what was provided by the spi-nor driver before Roman Yeryomin's commit and by this patch that reverts that commit. When OpenBMC project upgraded from kernel 4.13 to 4.17, it sucked in the commit made in version 4.15 and thus broke the mechanism I described above.

The flash chip in question does have a hardware reset line, but it does not matter in this case at all. Resetting the chip will bring it into EX4B (3-byte) mode, and that's not what is needed. Basically, that's exactly what is happening now (without this patch).

By the way, the newer AST2500 BMC chip has support for 4-byte commands and most probably does not have this problem.

>
>> As this is quite a specific case, I
>> guess you won't be willing to remove the SPI_NOR_4B_OPCODES option
>> globally. We'll have to keep the patch local to openbmc project
> I am more worried about this being actually unfixable in reliable way.
Well, configuring the chip for EN4B mode before powering on the host is quite reliable.
There is no-one to interfere with this setup in-between and even after the host is booted, as there is no driver in Linux for AST2400-based SPI controller on LPC bus (as seen from the host). There is only driver for that peripheral as seen from BMC.

>
>>> But still, there can be a software which does rogue transmission on
>>> the SPI bus and flips the flash into 3B mode before reset, so the
>>> system will not boot anyway. Do you have a way to deal with that
>>> somehow ?
>> Sure. As I said, there is no way to turn on the host without BMC
>> knowing/controlling it. The BMC ensures the chip is in the right
>> mode. That's actually the reason why this patch appeared in the first
>> place. Also, there is no rogue software on BMC.
> OK, but if the BMC puts the chip in a defined state, how come the system
> will refuse to (re)boot if Linux interferes with the chip's volatile
> register settings ?
Because it was BMC's Linux that got broken, not host's.
The commit fetched into BMC's linux while upgrading 4.13 to 4.17 resulted in putting the chip in a wrong state.

I start thinking about maybe adding an option to "jedec,spi-nor" DTS binding like "enable-4byte-mode", so that we could get rid of our local patch and just specify that option in the devicetree. The option would force EN4B on the chip if it supports that mode. What do you think? How do I do it if you approve? Send patches separately in here and in the devicetree mailing list or cross-post both patches to both lists?

Best regards,
Alexander.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]