Re: [PATCH v2 17/35] mtd: spi-nor: Introduce spi_nor_nonsfdp_flags_init()
From: Michael Walle <hidden>
Date: 2021-10-22 13:06:04
Am 2021-10-22 14:42, schrieb Tudor.Ambarus@microchip.com:
On 10/22/21 3:10 PM, Pratyush Yadav wrote:quoted
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On 22/10/21 01:21PM, Michael Walle wrote:quoted
Am 2021-08-17 12:24, schrieb Pratyush Yadav:quoted
On 27/07/21 07:52AM, Tudor Ambarus wrote:quoted
Used to initialize the NOR flags for settings that are not defined in the JESD216 SFDP standard, thus can not be retrieved when parsing SFDP. No functional change.I am worried if the order in which these flags are set can cause some subtle bugs. I can see one instance of it with SNOR_F_HAS_LOCK. spi_nor_late_init_params() checks for SNOR_F_HAS_LOCK and if there are no locking ops specified, it sets the default locking ops. This works fine before this patch because the flag is set before the function is called. But now, the flag will be set _after_ the function is called, and so you will never be able to set the default flags.Maybe we should just forbid to look at the SNOR_F_ flags in these functions. Instead the information could also be deduced by looking at the SPI_NOR_ flags.not true.
I guess you mean that some init flash init functions might set it, too. Right? IMHO this goes into the spaghetti code direction again. spi_nor_late_init_params() must not look at the SNOR_F_ flags. There are too much inter-function dependencies and it is really hard to follow the call chain. The locking ops should also go into the (static) flash init which only depends on the SPI_NOR_ flags. If SNOR_F_HAS_LOCK will be added at in an init function, then this code should also take care of setting the correct locking ops. Maybe both can be set together with a helper. -michael _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel