| Message ID | 20220301095640.313852-1-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mtd: spi-nor: Fix Quad Enable method for MX25L12835F | expand |
Hi Tudor, Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus <tudor.ambarus@microchip.com>: > > Hi, Heiko, > > Would you please apply this patch set on top of: > https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ When applying that series the flash will be properly detected. [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) Thanks --- Heiko
Am 2022-03-01 11:29, schrieb Heiko Thiery: > Hi Tudor, > > Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus > <tudor.ambarus@microchip.com>: >> >> Hi, Heiko, >> >> Would you please apply this patch set on top of: >> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ > > When applying that series the flash will be properly detected. > > [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) > > Thanks But doesn't that mean that the previous series introduces possible regressions for other flashes, too? -michael
On 3/1/22 12:38, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2022-03-01 11:29, schrieb Heiko Thiery: >> Hi Tudor, >> >> Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus >> <tudor.ambarus@microchip.com>: >>> >>> Hi, Heiko, >>> >>> Would you please apply this patch set on top of: >>> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ >> >> When applying that series the flash will be properly detected. >> >> [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) >> >> Thanks > > But doesn't that mean that the previous series introduces > possible regressions for other flashes, too? > no, because the other flashes are using the deprecated way of initializing parameters which calls default_init() hooks. Cheers, ta
On 3/1/22 12:29, Heiko Thiery wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Tudor, > > Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus > <tudor.ambarus@microchip.com>: >> >> Hi, Heiko, >> >> Would you please apply this patch set on top of: >> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ > > When applying that series the flash will be properly detected. > > [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) > Cool. I'll resubmit a new series where I'll add your tested by tag. I'll also reword the commit messages. Also, can you run such a script to verify erase-write-read? #!/bin/sh dd if=/dev/urandom of=./qspi_test bs=1M count=6 mtd_debug erase /dev/mtd5 0 6291456 mtd_debug write /dev/mtd5 0 6291456 qspi_test mtd_debug read /dev/mtd5 0 6291456 qspi_read sha1sum qspi_test qspi_read Thanks! ta
On 3/1/22 12:54, Tudor Ambarus wrote: > On 3/1/22 12:29, Heiko Thiery wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> Hi Tudor, >> >> Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus >> <tudor.ambarus@microchip.com>: >>> >>> Hi, Heiko, >>> >>> Would you please apply this patch set on top of: >>> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ >> >> When applying that series the flash will be properly detected. >> >> [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) >> > > Cool. I'll resubmit a new series where I'll add your tested by tag. I'll also > reword the commit messages. > > Also, can you run such a script to verify erase-write-read? > #!/bin/sh > > dd if=/dev/urandom of=./qspi_test bs=1M count=6 > mtd_debug erase /dev/mtd5 0 6291456 > mtd_debug write /dev/mtd5 0 6291456 qspi_test > mtd_debug read /dev/mtd5 0 6291456 qspi_read > sha1sum qspi_test qspi_read > Also, would be good to dump all the sysfs entries, I have just the SFDP one. I promise this is the last request :). Here's an example: # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id c22016 # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer macronix # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/partname mx25l3233f # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp > mx25l3233f-sfdp # xxd -p mx25l3233f-sfdp 53464450000101ff00000109300000ffc2000104600000ffffffffffffff ffffffffffffffffffffffffffffffffffffe520f1ffffffff0144eb086b 083b04bbeeffffffffff00ffffff00ff0c200f5210d800ffffffffffffff ffffffffffff003650269cf97764fecfffffffffffff # sha1sum mx25l3233f-sfdp 1b6e0f75b4a6d08d570082992455affa72b2dc81 mx25l3233f-sfdp
Am 2022-03-01 11:52, schrieb Tudor.Ambarus@microchip.com: > On 3/1/22 12:38, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2022-03-01 11:29, schrieb Heiko Thiery: >>> Hi Tudor, >>> >>> Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus >>> <tudor.ambarus@microchip.com>: >>>> >>>> Hi, Heiko, >>>> >>>> Would you please apply this patch set on top of: >>>> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ >>> >>> When applying that series the flash will be properly detected. >>> >>> [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) >>> >>> Thanks >> >> But doesn't that mean that the previous series introduces >> possible regressions for other flashes, too? >> > no, because the other flashes are using the deprecated way of > initializing > parameters which calls default_init() hooks. ahh right! But we should clear the quad_enable in the case the SFDP doesn't specify it. Right now, we are falling back to a function which doesn't make sense, and might even be harmful. -michael
On 3/1/22 14:36, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2022-03-01 11:52, schrieb Tudor.Ambarus@microchip.com: >> On 3/1/22 12:38, Michael Walle wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know >>> the content is safe >>> >>> Am 2022-03-01 11:29, schrieb Heiko Thiery: >>>> Hi Tudor, >>>> >>>> Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus >>>> <tudor.ambarus@microchip.com>: >>>>> >>>>> Hi, Heiko, >>>>> >>>>> Would you please apply this patch set on top of: >>>>> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ >>>> >>>> When applying that series the flash will be properly detected. >>>> >>>> [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) >>>> >>>> Thanks >>> >>> But doesn't that mean that the previous series introduces >>> possible regressions for other flashes, too? >>> >> no, because the other flashes are using the deprecated way of >> initializing >> parameters which calls default_init() hooks. > > ahh right! > > But we should clear the quad_enable in the case the SFDP doesn't > specify it. Right now, we are falling back to a function > which doesn't make sense, and might even be harmful. > do you mean the one set in the default_init() hook for macronix? we should get rid of the default_init(), yes. We should use SFDP where possible, where not possible we should use the late_init() hook to set the Quad Enable method. I'm cleaning all these right now.
Am 2022-03-01 13:46, schrieb Tudor.Ambarus@microchip.com: > On 3/1/22 14:36, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2022-03-01 11:52, schrieb Tudor.Ambarus@microchip.com: >>> On 3/1/22 12:38, Michael Walle wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>> know >>>> the content is safe >>>> >>>> Am 2022-03-01 11:29, schrieb Heiko Thiery: >>>>> Hi Tudor, >>>>> >>>>> Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus >>>>> <tudor.ambarus@microchip.com>: >>>>>> >>>>>> Hi, Heiko, >>>>>> >>>>>> Would you please apply this patch set on top of: >>>>>> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ >>>>> >>>>> When applying that series the flash will be properly detected. >>>>> >>>>> [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) >>>>> >>>>> Thanks >>>> >>>> But doesn't that mean that the previous series introduces >>>> possible regressions for other flashes, too? >>>> >>> no, because the other flashes are using the deprecated way of >>> initializing >>> parameters which calls default_init() hooks. >> >> ahh right! >> >> But we should clear the quad_enable in the case the SFDP doesn't >> specify it. Right now, we are falling back to a function >> which doesn't make sense, and might even be harmful. >> > > do you mean the one set in the default_init() hook for macronix? we > should > get rid of the default_init(), yes. We should use SFDP where possible, > where > not possible we should use the late_init() hook to set the Quad Enable > method. The one we set in spi_nor_init_default_params(), is that planned to be removed, too? I was thinking about something along the following patch: https://lore.kernel.org/linux-mtd/20220301124935.2893622-1-michael@walle.cc/ -michael
Hi, Am Di., 1. März 2022 um 11:54 Uhr schrieb <Tudor.Ambarus@microchip.com>: > > On 3/1/22 12:29, Heiko Thiery wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Hi Tudor, > > > > Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus > > <tudor.ambarus@microchip.com>: > >> > >> Hi, Heiko, > >> > >> Would you please apply this patch set on top of: > >> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ > > > > When applying that series the flash will be properly detected. > > > > [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) > > > > Cool. I'll resubmit a new series where I'll add your tested by tag. I'll also > reword the commit messages. > > Also, can you run such a script to verify erase-write-read? > #!/bin/sh > > dd if=/dev/urandom of=./qspi_test bs=1M count=6 > mtd_debug erase /dev/mtd5 0 6291456 > mtd_debug write /dev/mtd5 0 6291456 qspi_test > mtd_debug read /dev/mtd5 0 6291456 qspi_read > sha1sum qspi_test qspi_read Here is the output: # sh verify_erase_write_read.sh dd if=/dev/urandom of=./qspi_test bs=1M count=6 6+0 records in 6+0 records out time mtd_debug erase /dev/mtd0 0 6291456 Erased 6291456 bytes from address 0x00000000 in flash real 0m 43.24s user 0m 0.00s sys 0m 24.37s time mtd_debug write /dev/mtd0 0 6291456 qspi_test Copied 6291456 bytes from qspi_test to address 0x00000000 in flash real 0m 14.42s user 0m 0.00s sys 0m 7.82s time mtd_debug read /dev/mtd0 0 6291456 qspi_read Copied 6291456 bytes from address 0x00000000 in flash to qspi_read real 0m 0.53s user 0m 0.00s sys 0m 0.39s sha1sum qspi_test qspi_read c7fbc59b26f913ed96f0e7134657c0f9076c40bb qspi_test c7fbc59b26f913ed96f0e7134657c0f9076c40bb qspi_read > > Thanks! > ta
Hi Tudor, Am Di., 1. März 2022 um 11:56 Uhr schrieb <Tudor.Ambarus@microchip.com>: > > On 3/1/22 12:54, Tudor Ambarus wrote: > > On 3/1/22 12:29, Heiko Thiery wrote: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> > >> Hi Tudor, > >> > >> Am Di., 1. März 2022 um 10:56 Uhr schrieb Tudor Ambarus > >> <tudor.ambarus@microchip.com>: > >>> > >>> Hi, Heiko, > >>> > >>> Would you please apply this patch set on top of: > >>> https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ > >> > >> When applying that series the flash will be properly detected. > >> > >> [ 1.322655] spi-nor spi0.0: mx25l12835f (16384 Kbytes) > >> > > > > Cool. I'll resubmit a new series where I'll add your tested by tag. I'll also > > reword the commit messages. > > > > Also, can you run such a script to verify erase-write-read? > > #!/bin/sh > > > > dd if=/dev/urandom of=./qspi_test bs=1M count=6 > > mtd_debug erase /dev/mtd5 0 6291456 > > mtd_debug write /dev/mtd5 0 6291456 qspi_test > > mtd_debug read /dev/mtd5 0 6291456 qspi_read > > sha1sum qspi_test qspi_read > > > > Also, would be good to dump all the sysfs entries, I have just the SFDP one. > I promise this is the last request :). Here's an example: Then I am curious if this is the last request ;-) > # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id > c22016 > # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer > macronix > # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/partname > mx25l3233f > # cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp > mx25l3233f-sfdp > > > # xxd -p mx25l3233f-sfdp > 53464450000101ff00000109300000ffc2000104600000ffffffffffffff > ffffffffffffffffffffffffffffffffffffe520f1ffffffff0144eb086b > 083b04bbeeffffffffff00ffffff00ff0c200f5210d800ffffffffffffff > ffffffffffff003650269cf97764fecfffffffffffff > > # sha1sum mx25l3233f-sfdp > 1b6e0f75b4a6d08d570082992455affa72b2dc81 mx25l3233f-sfdp > and here you're: # cat /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/jedec_id c22018 # cat /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/manufacturer macronix # cat /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/partname mx25l12835f # xxd -p /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp 53464450000101ff00000109300000ffc2000104600000ffffffffffffff ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b 083b04bbfeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff ffffffffffff003600279df9c06485cbffffffffffff # sha1sum /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp c5e5abe6c5650a9d9c448690b1eeebdf5bfe57d4 /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
Hi, Heiko, Would you please apply this patch set on top of: https://lore.kernel.org/linux-mtd/20220228134505.203270-1-tudor.ambarus@microchip.com/ Thanks! Tudor Ambarus (2): mtd: spi-nor: macronix: Move set_4byte_addr_mode() to late_init() mtd: spi-nor: macronix: Use post_bfpt hook to fix quad_enable method for MX25L12835F drivers/mtd/spi-nor/macronix.c | 8 ++++++++ 1 file changed, 8 insertions(+)