On 9/29/20 7:29 PM, Pratyush Yadav wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On 29/09/20 03:42PM, Tudor.Ambarus@microchip.com wrote:

quoted

Hi, Pratyush,

I'm replying to v10 so that we continue the discussion, but this applies to v13 as well.

On 7/21/20 2:29 PM, Pratyush Yadav wrote:

quoted

quoted

quoted

@@ -2368,12 +2517,16 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
        struct spi_nor_flash_parameter *params = nor->params;
        unsigned int cap;

-       /* DTR modes are not supported yet, mask them all. */
-       *hwcaps &= ~SNOR_HWCAPS_DTR;
-
        /* X-X-X modes are not supported yet, mask them all. */
        *hwcaps &= ~SNOR_HWCAPS_X_X_X;

+       /*
+        * If the reset line is broken, we do not want to enter a stateful
+        * mode.
+        */
+       if (nor->flags & SNOR_F_BROKEN_RESET)
+               *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);

A dedicated reset line is not enough for flashes that keep their state
in non-volatile bits. Since we can't protect from unexpected crashes in
the non volatile state case, we should enter these modes only with an
explicit request, i.e. an optional DT property: "update-nonvolatile-state",
or something similar.

I wrote this patch with the assumption that we won't be supporting> non-volatile configuration as of now. In the previous discussions we

I think we have to take care of the stateful flashes now, otherwise we'll risk
to end up with users that let their flashes in a mode from which they can't recover.
I made some small RFC patches in reply to your v13, let me know what you think.

I haven't gone through them yet. Will check tomorrow.

quoted

quoted

came to the conclusion that it is not easy to detect the flash if it
boots in any mode other than 1S-1S-1S [0]. So if we update non-volatile
state, the flash would be useless after a reboot because we won't be
able to detect it in 8D mode. It doesn't matter if the reset line is
connected or not because it will reset the flash to the non-volatile
state, and we can't detect it from the non-volatile state.

correct, so a reset line for stateful modes doesn't help and the comment from the
code should be updated. s/stateful/stateless

We are talking about two different kinds of "state" here. The state you

Right, I used 'stateful' for flashes that enter in a X-X-X mode by setting a
non-volatile bit and 'stateless' for those that enter in a X-X-X mode
via volatile bits.

are talking about is the persistent state of the flash configured via
non-volatile registers. Yes, a reset line doesn't help in that case at
all.

quoted

The other state is the non-persistent state we set on the flash. Using

1S-1S-8D mode is stateless in the sense that we didn't change any state
on the flash to be able to use this mode, and only had to use the
correct opcode. If we execute a 1S-1S-1S command next it will also work
because the flash is still interpreting opcodes in 1S mode. Using
8D-8D-8D or 4S-4S-4S mode is stateful because we did have to configure
some state on the flash (which can very well be volatile). Once 8D-8D-8D
or 4S-4S-4S mode is entered, we cannot execute 1S-1S-1S commands until
we reset the flash because now the flash is interpreting commands in 4S
or 8D mode. This means we introduced some state on the flash.

Having a reset line will not help against the former but will help
against the latter. If the flash is in a stateful mode like 8D-8D-8D
without a reset line, an unexpected reset could leave bootloader unable
to boot because it issues the commands in 1S-1S-1S mode that the flash
cannot interpret. So even if the state we set is volatile, we still want
to avoid doing it if there is no reset line.

So I think the code and comment should stay as they are.

Ok. Cheers,
ta
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel