[PATCH 2/2] mtd: rawnand: brcmnand: move to polling in pio mode on oops write
This change makes sure that Broadcom NAND driver moves to interrupt polling on the first brcmnand_write() call. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 1c95b21aa63b..e6966db5f0d8 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2341,6 +2341,10 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip, for (i = 0; i < ctrl->max_oob; i += 4) oob_reg_write(ctrl, i, 0x); + if (mtd->oops_panic_write) + /* switch to interrupt polling and PIO mode */ + disable_ctrl_irqs(ctrl); + if (use_dma(ctrl) && (has_edu(ctrl) || !oob) && flash_dma_buf_ok(buf)) { if (ctrl->dma_trans(host, addr, (u32 *)buf, oob, mtd->writesize, CMD_PROGRAM_PAGE)) -- 2.17.1
[PATCH 1/2] mtd: rawnand: brcmnand: read/write oob during EDU transfer
Added support to read/write oob during EDU transfers. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 59 +--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 659eaa6f0980..1c95b21aa63b 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -242,6 +242,9 @@ struct brcmnand_controller { u32 edu_ext_addr; u32 edu_cmd; u32 edu_config; + int sas; /* spare area size, per flash cache */ + int sector_size_1k; + u8 *oob; /* flash_dma reg */ const u16 *flash_dma_offsets; @@ -249,7 +252,7 @@ struct brcmnand_controller { dma_addr_t dma_pa; int (*dma_trans)(struct brcmnand_host *host, u64 addr, u32 *buf, -u32 len, u8 dma_cmd); +u8 *oob, u32 len, u8 dma_cmd); /* in-memory cache of the FLASH_CACHE, used only for some commands */ u8 flash_cache[FC_BYTES]; @@ -1479,6 +1482,23 @@ static irqreturn_t brcmnand_edu_irq(int irq, void *data) edu_writel(ctrl, EDU_EXT_ADDR, ctrl->edu_ext_addr); edu_readl(ctrl, EDU_EXT_ADDR); + if (ctrl->oob) { + if (ctrl->edu_cmd == EDU_CMD_READ) { + ctrl->oob += read_oob_from_regs(ctrl, + ctrl->edu_count + 1, + ctrl->oob, ctrl->sas, + ctrl->sector_size_1k); + } else { + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, + ctrl->edu_ext_addr); + brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + ctrl->oob += write_oob_to_regs(ctrl, + ctrl->edu_count, + ctrl->oob, ctrl->sas, + ctrl->sector_size_1k); + } + } + mb(); /* flush previous writes */ edu_writel(ctrl, EDU_CMD, ctrl->edu_cmd); edu_readl(ctrl, EDU_CMD); @@ -1850,9 +1870,10 @@ static void brcmnand_write_buf(struct nand_chip *chip, const uint8_t *buf, * Kick EDU engine */ static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf, - u32 len, u8 cmd) + u8 *oob, u32 len, u8 cmd) { struct brcmnand_controller *ctrl = host->ctrl; + struct brcmnand_cfg *cfg = &host->hwcfg; unsigned long timeo = msecs_to_jiffies(200); int ret = 0; int dir = (cmd == CMD_PAGE_READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE); @@ -1860,6 +1881,9 @@ static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf, unsigned int trans = len >> FC_SHIFT; dma_addr_t pa; + dev_dbg(ctrl->dev, "EDU %s %p:%p\n", ((edu_cmd == EDU_CMD_READ) ? + "read" : "write"), buf, oob); + pa = dma_map_single(ctrl->dev, buf, len, dir); if (dma_mapping_error(ctrl->dev, pa)) { dev_err(ctrl->dev, "unable to map buffer for EDU DMA\n"); @@ -1871,6 +1895,8 @@ static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf, ctrl->edu_ext_addr = addr; ctrl->edu_cmd = edu_cmd; ctrl->edu_count = trans; + ctrl->sas = cfg->spare_area_size; + ctrl->oob = oob; edu_writel(ctrl, EDU_DRAM_ADDR, (u32)ctrl->edu_dram_addr); edu_readl(ctrl, EDU_DRAM_ADDR); @@ -1879,6 +1905,16 @@ static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf, edu_writel(ctrl, EDU_LENGTH, FC_BYTES); edu_readl(ctrl, EDU_LENGTH); + if (ctrl->oob && (ctrl->edu_cmd == EDU_CMD_WRITE)) { + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, + ctrl->edu_ext_addr); + brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + ctrl->oob += write_oob_to_regs(ctrl, + 1, + ctrl->oob, ctrl->sas, + ctrl->sector_size_1k); + } + /* Start edu engine */ mb(); /* flush previo
Re: [PATCH v2 13/23] mtd: nand: raw: brcmnand: brcmnand: Demote non-conformant kernel-doc headers
On Fri, Nov 6, 2020 at 4:37 PM Lee Jones wrote: > > Fixes the following W=1 kernel build warning(s): > > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1854: warning: Function parameter > or member 'host' not described in 'brcmnand_edu_trans' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1854: warning: Function parameter > or member 'addr' not described in 'brcmnand_edu_trans' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1854: warning: Function parameter > or member 'buf' not described in 'brcmnand_edu_trans' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1854: warning: Function parameter > or member 'len' not described in 'brcmnand_edu_trans' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1854: warning: Function parameter > or member 'cmd' not described in 'brcmnand_edu_trans' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'host' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'desc' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'addr' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'buf' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'len' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'dma_cmd' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'begin' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'end' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1951: warning: Function parameter > or member 'next_desc' not described in 'brcmnand_fill_dma_desc' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1977: warning: Function parameter > or member 'host' not described in 'brcmnand_dma_run' > drivers/mtd/nand/raw/brcmnand/brcmnand.c:1977: warning: Function parameter > or member 'desc' not described in 'brcmnand_dma_run' > > Cc: Brian Norris > Cc: Kamal Dasu > Cc: Miquel Raynal > Cc: Richard Weinberger > Cc: Vignesh Raghavendra > Cc: linux-...@lists.infradead.org > Cc: bcm-kernel-feedback-l...@broadcom.com > Signed-off-by: Lee Jones > Acked-by: Florian Fainelli Signed-off-by: Kamal Dasu > --- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index 2da39ab892869..659eaa6f0980c 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -1846,7 +1846,7 @@ static void brcmnand_write_buf(struct nand_chip *chip, > const uint8_t *buf, > } > } > > -/** > +/* > * Kick EDU engine > */ > static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf, > @@ -1937,7 +1937,7 @@ static int brcmnand_edu_trans(struct brcmnand_host > *host, u64 addr, u32 *buf, > return ret; > } > > -/** > +/* > * Construct a FLASH_DMA descriptor as part of a linked list. You must know > the > * following ahead of time: > * - Is this descriptor the beginning or end of a linked list? > @@ -1970,7 +1970,7 @@ static int brcmnand_fill_dma_desc(struct brcmnand_host > *host, > return 0; > } > > -/** > +/* > * Kick the FLASH_DMA engine, with a given DMA descriptor > */ > static void brcmnand_dma_run(struct brcmnand_host *host, dma_addr_t desc) > -- > 2.25.1 >
Re: [PATCH 0/5] qspi binding and DTS fixes
Looks good to me. Signed-off-by: Kamal Dasu On Thu, Aug 27, 2020 at 3:30 PM Scott Branden wrote: > > Patch series looks good. > > Acked-by: Scott Branden > > On 2020-08-27 11:18 a.m., Florian Fainelli wrote: > > Hi all, > > > > This patch series fixes incorrectly defined compatible strings for the > > Broadcom QSPI controller which resulted in the strings not being > > ordered from most to least compatible. > > > > We will need to apply some changes to the spi-bcm-qspi.c driver in > > the future to assume no revision register exist, and these patches > > are a preliminary step towards that goal. > > > > Florian Fainelli (5): > > dt-bindings: spi: Fix spi-bcm-qspi compatible ordering > > ARM: dts: bcm: HR2: Fixed QSPI compatible string > > ARM: dts: NSP: Fixed QSPI compatible string > > ARM: dts: BCM5301X: Fixed QSPI compatible string > > arm64: dts: ns2: Fixed QSPI compatible string > > > > .../bindings/spi/brcm,spi-bcm-qspi.txt | 16 > > arch/arm/boot/dts/bcm-hr2.dtsi | 2 +- > > arch/arm/boot/dts/bcm-nsp.dtsi | 2 +- > > arch/arm/boot/dts/bcm5301x.dtsi | 2 +- > > arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi | 2 +- > > 5 files changed, 12 insertions(+), 12 deletions(-) > > >
Re: [PATCH 0/5] Improvements to spi-bcm-qspi
Mark, This block is used on multiple Broadcom SoCs and would like to get comments from all who deal with iProc and have touched this file as well. Please copy : Florian Fainelli Rayagonda Kokatanur and bcm-kernel-feedback-l...@broadcom.com (maintainer:BROADCOM SPI DRIVER) Kamal On Mon, Jun 15, 2020 at 12:06 AM Mark Tomlinson wrote: > > This series of patches came from a single large Broadcom patch that > implements drivers for a number of their integrated switch chips. Mostly > this is just splitting the qspi driver into smaller parts and doesn't > include much original from me. > > Mark Tomlinson (5): > spi: bcm-qspi: Add support for setting BSPI clock > spi: bcm-qspi: Improve debug reading SPI data > spi: bcm-qspi: Do not split transfers into small chunks > spi: bcm-qspi: Make multiple data blocks interrupt-driven > spi: bcm-qspi: Improve interrupt handling > > drivers/spi/spi-bcm-qspi.c | 189 ++--- > drivers/spi/spi-bcm-qspi.h | 5 +- > 2 files changed, 115 insertions(+), 79 deletions(-) > > -- > 2.27.0 >
[PATCH v2] mtd: set master partition panic write flag
Check and set master panic write flag so that low level drivers can use it to take required action to ensure oops data gets written to assigned mtdoops device partition. Fixes: 9f897bfdd89f ("mtd: Add flag to indicate panic_write") Signed-off-by: Kamal Dasu --- drivers/mtd/mtdcore.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index b47691e1b81c..6efca96dc110 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1235,8 +1235,8 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, return -EROFS; if (!len) return 0; - if (!mtd->oops_panic_write) - mtd->oops_panic_write = true; + if (!master->oops_panic_write) + master->oops_panic_write = true; return master->_panic_write(master, mtd_get_master_ofs(mtd, to), len, retlen, buf); -- 2.17.1
Re: [PATCH 2/2] mtd: rawnand: brcmnand: Ecc error handling on EDU transfers
On Mon, Jun 15, 2020 at 3:19 AM Miquel Raynal wrote: > > Hi Kamal, > > Kamal Dasu wrote on Fri, 12 Jun 2020 12:34:22 > -0400: > > > On Fri, Jun 12, 2020 at 3:07 AM Miquel Raynal > > wrote: > > > > > > Hi Kamal, > > > > > > Kamal Dasu wrote on Thu, 11 Jun 2020 12:04:29 > > > -0400: > > > > > > > On Thu, Jun 11, 2020 at 3:27 AM Miquel Raynal > > > > wrote: > > > > > > > > > > Hi Kamal, > > > > > > > > > > Kamal Dasu wrote on Thu, 11 Jun 2020 01:44:54 > > > > > -0400: > > > > > > > > > > > Implemented ECC correctable and uncorrectable error handling for EDU > > > > > > > > > > Implement? > > > > > > > > > > > reads. If ECC correctable bitflips are encountered on EDU transfer, > > > > > > > > > > extra space ^ > > > > > > > > > > > read page again using pio, This is needed due to a controller > > > > > > lmitation > > > > > > > > > > s/pio/PIO/ > > > > > > > > > > > where read and corrected data is not transferred to the DMA buffer > > > > > > on ECC > > > > > > errors. This holds true for ECC correctable errors beyond set > > > > > > threshold. > > > > > > > > > > error. > > > > > > > > > > Not sure what the last sentence means? > > > > > > > > > > > > > NAND controller allows for setting a correctable ECC threshold number > > > > of bits beyond which it will actually report the error to the driver. > > > > e.g. for BCH-4 the threshold is 3, so 3-bit and 4-bit errors will > > > > generate correctable ECC interrupt however 1-bit and 2-bit errors will > > > > be corrected silently. > > > > From the above example EDU hardware will not transfer corrected data > > > > to the DMA buffer for 3-bit and 4-bit errors that get reported. So > > > > once we detect > > > > the error duing EDU we read the page again using pio. > > > > > > Ok I see what you mean, can't you fake the threshold instead? The NAND > > > controller in Linux is not supposed to handle this threshold, the NAND > > > core is in charge. So what the controller driver should do is just: > > > increase the number of bitflips + return the maximum number or bitflip > > > or increase the failure counter. Is this already the case? > > > > > /* threshold = ceil(BCH-level * 0.75) */ > > brcmnand_wr_corr_thresh(host, DIV_ROUND_UP(chip->ecc.strength * 3, 4)); > > This how the threshold is set, all it means is that for high BCH > > levels don't interrupt on low number (less than threshold) of > > bit_flips. Yes the controller driver only increments correctable ECC > > count. But due the EDU design an EDU operation is disrupted when the > > controller interrupts on correctable ECC errors during subpage ECC > > calculations. Hence the driver needs to read the page again with PIO > > to transfer corrected data. > > IIUC, you are doing the job twice: you should just return a number of > bitflips or an error to the NAND core. So that's why I'm telling that > you should get rid of this threshold. It would avoid the need for the > PIO transfer too. I think you are reading some statements in isolation that probably are causing some confusion. EDU design has a flaw in case of reported ECC error interrupt in that corrected data is not transferred to the DMA buffer. The PIO is needed to read corrected data into the NAND data buffer and only for the reported errors. So there is no need to change the threshold calculation logic, if we get rid of the threshold then we will have to do the PIO read on any correctable bit error if it occurs during EDU reads. > > You also say that the controller "only increments correctable ECC > count", what do you mean exactly? Maybe that statement was a bit misleading. To be clear when an ECC error is reported the controller gives the bit_flips count as well as updates the ECC error address Register and ecc error status registers. This logic works as expected in the hardware. >The controller does not report errors > when the number of bitflips happens to be above the BCH threshold? This > would be the only case where what is currently done would be actually > needed though. It's the other way. The controller only reports bit errors beyond >=threshold value, will not report otherwise and silently correct the data. There is no problem in cases where erros are corrected silently. Now ECC (un)correctable on EDU reads are detected by simply reading back the ECC Error address register. And in case of reported uncorrectable ECC errors are treated as usual. And for reported correctable ECC errors we need to read the page again using PIO so that the corrected data is properly transferred. All this applies to EDU transfer only. > > Thanks, > Miquèl Kamal
Re: [PATCH] mtd: rawnand: brcmnand: force raw OOB writes
Alvaro, On Sat, Jun 13, 2020 at 5:01 AM Álvaro Fernández Rojas wrote: > > Hi Kamal, > > > El 12 jun 2020, a las 20:47, Kamal Dasu escribió: > > > > On Fri, Jun 5, 2020 at 1:07 PM Álvaro Fernández Rojas > > wrote: > >> > >> MTD_OPS_AUTO_OOB is writting OOB with ECC enabled, which changes all ECC > >> bytes > >> from an erased page to 0x00 when JFFS2 cleanmarkers are added with > >> mtd-utils. > >> | BBI | JFFS2 | ECC | JFFS2 | Spare | > >> 0800 ff ff 19 85 20 03 00 00 00 00 00 00 08 ff ff ff > >> > >> However, if OOB is written with ECC disabled, the JFFS2 cleanmarkers are > >> correctly written without changing the ECC bytes: > >> | BBI | JFFS2 | ECC | JFFS2 | Spare | > >> 0800 ff ff 19 85 20 03 ff ff ff 00 00 00 08 ff ff ff > > > > Both brcmand_write_oob_raw() and brcmnand_write_oob() use > > brcmnand_write() that uses PROGRAM_PAGE cmd, means also programs data > > areas and ECC when enabled is always calculated on DATA+OOB. since > > Are you sure about that? I think that HW ECC is only calculated for DATA, not > for OOB. > The fact that we’re not writing any DATA seems to be the problem that is > changing all ECC bytes to 0x00. > Only true for 1-bit hamming code on early controller versions. For BCH algorithms both data+oob are covered. And the controller driver intentionally does not implement a spare area program cmd, even for OOB writes. Also BRCMNAND_ACC_CONTROL_PARTIAL_PAGE=0 (when present) does not allow for spare areas only writes. > > in both cases we only want to modify OOB, data is always assumed to be > > 0xffs (means erased state). So as far as this api always is used on > > the erased page it should be good. Are the sub-pages/oob areas read by > > jffs2 also read without ecc enabled?. Just want to be sure that it > > does not break any other utilities like nandwrite. > > No, sub-pages/oob areas read by JFFS2 with ECC enabled. > I don’t think this breaks anything at all, since I tested nandwrite with OOB > enabled and everything was written perfectly. > Going by the commit message you are assuming 1-bit hamming code, that is the only exception on early version controllers where only data was covered for ecc calculations when enabled. Which version of the controller are you using ?. Did you test with BCH-4 or any BCH ?. > BTW, I tried another approach that forced MTD_OPS_AUTO_OOB to be written as > raw OOB, but it was rejected: > http://patchwork.ozlabs.org/project/linux-mtd/patch/20200504094253.2741109-1-nolt...@gmail.com/ > https://lkml.org/lkml/2020/5/4/215 > Are there any other use cases other than jffs2 where only oob needs to be modified ?. But surely intentionally disabling ecc instead of forcing it is the way, but I am not sure it will still work for other BCH algorithms though where both data and oob are covered. > > > >> > >> Signed-off-by: Álvaro Fernández Rojas > >> --- > >> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 9 + > >> 1 file changed, 1 insertion(+), 8 deletions(-) > >> > >> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > >> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > >> index 8f9ffb46a09f..566281770841 100644 > >> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > >> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > >> @@ -2279,13 +2279,6 @@ static int brcmnand_write_page_raw(struct nand_chip > >> *chip, const uint8_t *buf, > >>return nand_prog_page_end_op(chip); > >> } > >> > >> -static int brcmnand_write_oob(struct nand_chip *chip, int page) > >> -{ > >> - return brcmnand_write(nand_to_mtd(chip), chip, > >> - (u64)page << chip->page_shift, NULL, > >> - chip->oob_poi); > >> -} > >> - > >> static int brcmnand_write_oob_raw(struct nand_chip *chip, int page) > >> { > >>struct mtd_info *mtd = nand_to_mtd(chip); > >> @@ -2642,7 +2635,7 @@ static int brcmnand_init_cs(struct brcmnand_host > >> *host, struct device_node *dn) > >>chip->ecc.write_oob_raw = brcmnand_write_oob_raw; > >>chip->ecc.read_oob_raw = brcmnand_read_oob_raw; > >>chip->ecc.read_oob = brcmnand_read_oob; > >> - chip->ecc.write_oob = brcmnand_write_oob; > >> + chip->ecc.write_oob = brcmnand_write_oob_raw; > >> > >>chip->controller = &ctrl->controller; > >> > >> -- > >> 2.26.2 > >> > > Best regards, > Álvaro.
Re: [Patch] mtd: set master partition panic write flag
Can you please accept this if there are no objections. Kamal > On May 4, 2020, at 7:54 PM, Kamal Dasu wrote: > > Check and set master panic write flag so that low level drivers > can use it to take required action to ensure oops data gets written > to assigned mtdoops device partition. > > Signed-off-by: Kamal Dasu > --- > drivers/mtd/mtdcore.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > index 2916674208b3..7e37ed2f38ea 100644 > --- a/drivers/mtd/mtdcore.c > +++ b/drivers/mtd/mtdcore.c > @@ -1235,8 +1235,8 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, > size_t len, size_t *retlen, >return -EROFS; >if (!len) >return 0; > -if (!mtd->oops_panic_write) > -mtd->oops_panic_write = true; > +if (!master->oops_panic_write) > +master->oops_panic_write = true; > >return master->_panic_write(master, mtd_get_master_ofs(mtd, to), len, >retlen, buf); > -- > 2.17.1 >
[PATCH v2 2/2] mtd: rawnand: brcmnand: ECC error handling on EDU transfers
Implement ECC correctable and uncorrectable error handling for EDU reads. If ECC correctable bitflips are encountered on EDU transfer, read page again using PIO. This is needed due to a NAND controller limitation where corrected data is not transferred to the DMA buffer on ECC error. This applies to ECC correctable errors that are reported by the controller hardware based on set number of bitflips threshold in the controller threshold register, bitflips below the threshold are corrected silently and are not reported by the controller hardware. Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers") Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 26 1 file changed, 26 insertions(+) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 0c1d6e543586..131d5d6e2626 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -1855,6 +1855,22 @@ static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf, edu_writel(ctrl, EDU_STOP, 0); /* force stop */ edu_readl(ctrl, EDU_STOP); + if (!ret && edu_cmd == EDU_CMD_READ) { + u64 err_addr = 0; + + /* +* check for ECC errors here, subpage ECC errors are +* retained in ECC error address register +*/ + err_addr = brcmnand_get_uncorrecc_addr(ctrl); + if (!err_addr) { + err_addr = brcmnand_get_correcc_addr(ctrl); + if (err_addr) + ret = -EUCLEAN; + } else + ret = -EBADMSG; + } + return ret; } @@ -2058,6 +2074,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, u64 err_addr = 0; int err; bool retry = true; + bool edu_err = false; dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf); @@ -2075,6 +2092,10 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, else return -EIO; } + + if (has_edu(ctrl) && err_addr) + edu_err = true; + } else { if (oob) memset(oob, 0x99, mtd->oobsize); @@ -2122,6 +2143,11 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, if (mtd_is_bitflip(err)) { unsigned int corrected = brcmnand_count_corrected(ctrl); + /* in case of EDU correctable error we read again using PIO */ + if (edu_err) + err = brcmnand_read_by_pio(mtd, chip, addr, trans, buf, + oob, &err_addr); + dev_dbg(ctrl->dev, "corrected error at 0x%llx\n", (unsigned long long)err_addr); mtd->ecc_stats.corrected += corrected; -- 2.17.1
[PATCH v2 0/2] mtd: rawnand: brcmnand: ECC error handling on EDU transfers
V2 changes: "mtd: rawnand: brcmnand: Don't default to edu transfer" - no change "mtd: rawnand: brcmnand: ECC error handling on EDU transfers" - Fix typos in commit message and clarify bitflips threhold use - Fix typos in code comments - change bool edu_read to bool edu_err in brcmnand_read() Kamal Dasu (2): mtd: rawnand: brcmnand: Don't default to edu transfer mtd: rawnand: brcmnand: ECC error handling on EDU transfers drivers/mtd/nand/raw/brcmnand/brcmnand.c | 31 ++-- 1 file changed, 29 insertions(+), 2 deletions(-) -- 2.17.1
[PATCH v2 1/2] mtd: rawnand: brcmnand: Don't default to edu transfer
When flash-dma is absent do not default to using flash-edu. Make sure flash-edu is enabled before setting EDU transfer function. Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers") Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 8f9ffb46a09f..0c1d6e543586 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2953,8 +2953,9 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) if (ret < 0) goto err; - /* set edu transfer function to call */ - ctrl->dma_trans = brcmnand_edu_trans; + if (has_edu(ctrl)) + /* set edu transfer function to call */ + ctrl->dma_trans = brcmnand_edu_trans; } /* Disable automatic device ID config, direct addressing */ -- 2.17.1
Re: [PATCH] mtd: rawnand: brcmnand: force raw OOB writes
On Fri, Jun 5, 2020 at 1:07 PM Álvaro Fernández Rojas wrote: > > MTD_OPS_AUTO_OOB is writting OOB with ECC enabled, which changes all ECC bytes > from an erased page to 0x00 when JFFS2 cleanmarkers are added with mtd-utils. > | BBI | JFFS2 | ECC | JFFS2 | Spare | > 0800 ff ff 19 85 20 03 00 00 00 00 00 00 08 ff ff ff > > However, if OOB is written with ECC disabled, the JFFS2 cleanmarkers are > correctly written without changing the ECC bytes: > | BBI | JFFS2 | ECC | JFFS2 | Spare | > 0800 ff ff 19 85 20 03 ff ff ff 00 00 00 08 ff ff ff Both brcmand_write_oob_raw() and brcmnand_write_oob() use brcmnand_write() that uses PROGRAM_PAGE cmd, means also programs data areas and ECC when enabled is always calculated on DATA+OOB. since in both cases we only want to modify OOB, data is always assumed to be 0xffs (means erased state). So as far as this api always is used on the erased page it should be good. Are the sub-pages/oob areas read by jffs2 also read without ecc enabled?. Just want to be sure that it does not break any other utilities like nandwrite. > > Signed-off-by: Álvaro Fernández Rojas > --- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 9 + > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index 8f9ffb46a09f..566281770841 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -2279,13 +2279,6 @@ static int brcmnand_write_page_raw(struct nand_chip > *chip, const uint8_t *buf, > return nand_prog_page_end_op(chip); > } > > -static int brcmnand_write_oob(struct nand_chip *chip, int page) > -{ > - return brcmnand_write(nand_to_mtd(chip), chip, > - (u64)page << chip->page_shift, NULL, > - chip->oob_poi); > -} > - > static int brcmnand_write_oob_raw(struct nand_chip *chip, int page) > { > struct mtd_info *mtd = nand_to_mtd(chip); > @@ -2642,7 +2635,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host, > struct device_node *dn) > chip->ecc.write_oob_raw = brcmnand_write_oob_raw; > chip->ecc.read_oob_raw = brcmnand_read_oob_raw; > chip->ecc.read_oob = brcmnand_read_oob; > - chip->ecc.write_oob = brcmnand_write_oob; > + chip->ecc.write_oob = brcmnand_write_oob_raw; > > chip->controller = &ctrl->controller; > > -- > 2.26.2 >
Re: [PATCH 2/2] mtd: rawnand: brcmnand: Ecc error handling on EDU transfers
On Fri, Jun 12, 2020 at 3:07 AM Miquel Raynal wrote: > > Hi Kamal, > > Kamal Dasu wrote on Thu, 11 Jun 2020 12:04:29 > -0400: > > > On Thu, Jun 11, 2020 at 3:27 AM Miquel Raynal > > wrote: > > > > > > Hi Kamal, > > > > > > Kamal Dasu wrote on Thu, 11 Jun 2020 01:44:54 > > > -0400: > > > > > > > Implemented ECC correctable and uncorrectable error handling for EDU > > > > > > Implement? > > > > > > > reads. If ECC correctable bitflips are encountered on EDU transfer, > > > > > > extra space ^ > > > > > > > read page again using pio, This is needed due to a controller lmitation > > > > > > s/pio/PIO/ > > > > > > > where read and corrected data is not transferred to the DMA buffer on > > > > ECC > > > > errors. This holds true for ECC correctable errors beyond set threshold. > > > > > > error. > > > > > > Not sure what the last sentence means? > > > > > > > NAND controller allows for setting a correctable ECC threshold number > > of bits beyond which it will actually report the error to the driver. > > e.g. for BCH-4 the threshold is 3, so 3-bit and 4-bit errors will > > generate correctable ECC interrupt however 1-bit and 2-bit errors will > > be corrected silently. > > From the above example EDU hardware will not transfer corrected data > > to the DMA buffer for 3-bit and 4-bit errors that get reported. So > > once we detect > > the error duing EDU we read the page again using pio. > > Ok I see what you mean, can't you fake the threshold instead? The NAND > controller in Linux is not supposed to handle this threshold, the NAND > core is in charge. So what the controller driver should do is just: > increase the number of bitflips + return the maximum number or bitflip > or increase the failure counter. Is this already the case? > /* threshold = ceil(BCH-level * 0.75) */ brcmnand_wr_corr_thresh(host, DIV_ROUND_UP(chip->ecc.strength * 3, 4)); This how the threshold is set, all it means is that for high BCH levels don't interrupt on low number (less than threshold) of bit_flips. Yes the controller driver only increments correctable ECC count. But due the EDU design an EDU operation is disrupted when the controller interrupts on correctable ECC errors during subpage ECC calculations. Hence the driver needs to read the page again with PIO to transfer corrected data. > > > > > > > > > > Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu > > > > for dma transfers") > > > > Signed-off-by: Kamal Dasu > > > > --- > > > > > > Minor nits below :) > > > > > > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 26 > > > > 1 file changed, 26 insertions(+) > > > > > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > index 0c1d6e543586..d7daa83c8a58 100644 > > > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > @@ -1855,6 +1855,22 @@ static int brcmnand_edu_trans(struct > > > > brcmnand_host *host, u64 addr, u32 *buf, > > > > edu_writel(ctrl, EDU_STOP, 0); /* force stop */ > > > > edu_readl(ctrl, EDU_STOP); > > > > > > > > + if (ret == 0 && edu_cmd == EDU_CMD_READ) { > > > > > > !ret > > > > > > > + u64 err_addr = 0; > > > > + > > > > + /* > > > > + * check for ecc errors here, subpage ecc erros are > > > > + * retained in ecc error addr register > > > > > > s/ecc/ECC/ > > > s/erros/errors/ > > > s/addr/address/ > > > > > > > + */ > > > > + err_addr = brcmnand_get_uncorrecc_addr(ctrl); > > > > + if (!err_addr) { > > > > + err_addr = brcmnand_get_correcc_addr(ctrl); > > > > + if (err_addr) > > > > + ret = -EUCLEAN; > > > > + } else > > > > + ret = -EBADMSG; > > > > > > I don't like very much to see these values being used within NAND > > > controller drivers but I see it's already the cause, so I gues
Re: [PATCH 2/2] mtd: rawnand: brcmnand: Ecc error handling on EDU transfers
On Thu, Jun 11, 2020 at 3:27 AM Miquel Raynal wrote: > > Hi Kamal, > > Kamal Dasu wrote on Thu, 11 Jun 2020 01:44:54 > -0400: > > > Implemented ECC correctable and uncorrectable error handling for EDU > > Implement? > > > reads. If ECC correctable bitflips are encountered on EDU transfer, > > extra space ^ > > > read page again using pio, This is needed due to a controller lmitation > > s/pio/PIO/ > > > where read and corrected data is not transferred to the DMA buffer on ECC > > errors. This holds true for ECC correctable errors beyond set threshold. > > error. > > Not sure what the last sentence means? > NAND controller allows for setting a correctable ECC threshold number of bits beyond which it will actually report the error to the driver. e.g. for BCH-4 the threshold is 3, so 3-bit and 4-bit errors will generate correctable ECC interrupt however 1-bit and 2-bit errors will be corrected silently. >From the above example EDU hardware will not transfer corrected data to the DMA buffer for 3-bit and 4-bit errors that get reported. So once we detect the error duing EDU we read the page again using pio. > > > > Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu for > > dma transfers") > > Signed-off-by: Kamal Dasu > > --- > > Minor nits below :) > > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 26 > > 1 file changed, 26 insertions(+) > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > index 0c1d6e543586..d7daa83c8a58 100644 > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > @@ -1855,6 +1855,22 @@ static int brcmnand_edu_trans(struct brcmnand_host > > *host, u64 addr, u32 *buf, > > edu_writel(ctrl, EDU_STOP, 0); /* force stop */ > > edu_readl(ctrl, EDU_STOP); > > > > + if (ret == 0 && edu_cmd == EDU_CMD_READ) { > > !ret > > > + u64 err_addr = 0; > > + > > + /* > > + * check for ecc errors here, subpage ecc erros are > > + * retained in ecc error addr register > > s/ecc/ECC/ > s/erros/errors/ > s/addr/address/ > > > + */ > > + err_addr = brcmnand_get_uncorrecc_addr(ctrl); > > + if (!err_addr) { > > + err_addr = brcmnand_get_correcc_addr(ctrl); > > + if (err_addr) > > + ret = -EUCLEAN; > > + } else > > + ret = -EBADMSG; > > I don't like very much to see these values being used within NAND > controller drivers but I see it's already the cause, so I guess I can > live with that. > > > + } > > + > > return ret; > > } > > > > @@ -2058,6 +2074,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct > > nand_chip *chip, > > u64 err_addr = 0; > > int err; > > bool retry = true; > > + bool edu_read = false; > > > > dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, > > buf); > > > > @@ -2075,6 +2092,10 @@ static int brcmnand_read(struct mtd_info *mtd, > > struct nand_chip *chip, > > else > > return -EIO; > > } > > + > > + if (has_edu(ctrl)) > > + edu_read = true; > > You don't need this extra value, you already have the cmd parameter > which tells you if it is a read or a write. You might even want to > create a if block so set dir and edu_cmd and eventually a local > edu_read if you think it still makes sense. > I needed the value since dma and edu read has multiple conditions like oob is not included, buffer is aligned, virtual address is good. This indicates to the if (mtd_is_bitflip(err)) block that the error was from an edu transaction that happened.This way all ecc error handling for dma, edu, pio is in one place. Also there is more controller version specific logic for read error handling in there and this allows us to maintain the hierarchy how we handle both correctable and uncorrectable error. > > + > > } else { > > if (oob) > > memset(oob, 0x99, mtd->oobsize); > > @@ -2122,6 +2143,11 @@ static int brcmnand_read(struct mtd_info *mtd, > > struct nand_chip *chip, > > if (mtd_is_bitflip(err)) { > > unsi
Re: [PATCH 1/2] mtd: rawnand: brcmnand: Don't default to edu transfer
On Thu, Jun 11, 2020 at 3:16 AM Miquel Raynal wrote: > > Hi Kamal, > > Kamal Dasu wrote on Thu, 11 Jun 2020 01:44:53 > -0400: > > > When flash-dma is absent do not default to using flash-edu. > > Make sure flash-edu is enabled before setting EDU transfer > > function. > > > > Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu for > > dma transfers") > > Signed-off-by: Kamal Dasu > > --- > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > index 8f9ffb46a09f..0c1d6e543586 100644 > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > @@ -2953,8 +2953,9 @@ int brcmnand_probe(struct platform_device *pdev, > > struct brcmnand_soc *soc) > > if (ret < 0) > > goto err; > > > > - /* set edu transfer function to call */ > > - ctrl->dma_trans = brcmnand_edu_trans; > > + if (has_edu(ctrl)) > > + /* set edu transfer function to call */ > > + ctrl->dma_trans = brcmnand_edu_trans; > > Does this fallback to returning an error in case !has_edu() ? Othewise, > how is it handled? > The driver will default to pio if both flash-dma and falsh-edu are not present. > Thanks, > Miquèl Kamal
[PATCH 2/2] mtd: rawnand: brcmnand: Ecc error handling on EDU transfers
Implemented ECC correctable and uncorrectable error handling for EDU reads. If ECC correctable bitflips are encountered on EDU transfer, read page again using pio, This is needed due to a controller lmitation where read and corrected data is not transferred to the DMA buffer on ECC errors. This holds true for ECC correctable errors beyond set threshold. Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers") Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 26 1 file changed, 26 insertions(+) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 0c1d6e543586..d7daa83c8a58 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -1855,6 +1855,22 @@ static int brcmnand_edu_trans(struct brcmnand_host *host, u64 addr, u32 *buf, edu_writel(ctrl, EDU_STOP, 0); /* force stop */ edu_readl(ctrl, EDU_STOP); + if (ret == 0 && edu_cmd == EDU_CMD_READ) { + u64 err_addr = 0; + + /* +* check for ecc errors here, subpage ecc erros are +* retained in ecc error addr register +*/ + err_addr = brcmnand_get_uncorrecc_addr(ctrl); + if (!err_addr) { + err_addr = brcmnand_get_correcc_addr(ctrl); + if (err_addr) + ret = -EUCLEAN; + } else + ret = -EBADMSG; + } + return ret; } @@ -2058,6 +2074,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, u64 err_addr = 0; int err; bool retry = true; + bool edu_read = false; dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf); @@ -2075,6 +2092,10 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, else return -EIO; } + + if (has_edu(ctrl)) + edu_read = true; + } else { if (oob) memset(oob, 0x99, mtd->oobsize); @@ -2122,6 +2143,11 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip, if (mtd_is_bitflip(err)) { unsigned int corrected = brcmnand_count_corrected(ctrl); + /* in case of edu correctable error we read again using pio */ + if (edu_read) + err = brcmnand_read_by_pio(mtd, chip, addr, trans, buf, + oob, &err_addr); + dev_dbg(ctrl->dev, "corrected error at 0x%llx\n", (unsigned long long)err_addr); mtd->ecc_stats.corrected += corrected; -- 2.17.1
[PATCH 1/2] mtd: rawnand: brcmnand: Don't default to edu transfer
When flash-dma is absent do not default to using flash-edu. Make sure flash-edu is enabled before setting EDU transfer function. Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers") Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 8f9ffb46a09f..0c1d6e543586 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2953,8 +2953,9 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) if (ret < 0) goto err; - /* set edu transfer function to call */ - ctrl->dma_trans = brcmnand_edu_trans; + if (has_edu(ctrl)) + /* set edu transfer function to call */ + ctrl->dma_trans = brcmnand_edu_trans; } /* Disable automatic device ID config, direct addressing */ -- 2.17.1
[Patch] mtd: set master partition panic write flag
Check and set master panic write flag so that low level drivers can use it to take required action to ensure oops data gets written to assigned mtdoops device partition. Signed-off-by: Kamal Dasu --- drivers/mtd/mtdcore.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 2916674208b3..7e37ed2f38ea 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1235,8 +1235,8 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, return -EROFS; if (!len) return 0; - if (!mtd->oops_panic_write) - mtd->oops_panic_write = true; + if (!master->oops_panic_write) + master->oops_panic_write = true; return master->_panic_write(master, mtd_get_master_ofs(mtd, to), len, retlen, buf); -- 2.17.1
Re: [PATCH] mtd: set mtd partition panic write flag
On Sat, May 2, 2020 at 2:08 PM Miquel Raynal wrote: > > Hi Kamal, > > Miquel Raynal wrote on Thu, 9 Jan 2020 > 18:28:07 +0100: > > > Hi Kamal, > > > > Kamal Dasu wrote on Thu, 9 Jan 2020 10:25:59 > > -0500: > > > > > Miquel, > > > > > > Yes the issue is still open. I was trying to understand the suggestion > > > and did not get a reply on the question I had > > > > > > Richard wrote : > > > "So the right fix would be setting the parent's oops_panic_write in > > > mtd_panic_write(). > > > Then we don't have to touch mtdpart.c" > > > > > > How do I get access to the parts parent in the core ?. Maybe I am > > > missing something. > > > > I think the solution is to set the oops_panic_write of the root parent, > > instead of updating the flag of the mtd device itself (which is maybe a > > partition). > > > > Would this help? > > > > https://www.spinics.net/lists/linux-mtd/msg10454.html > > I'm pinging you here as well, as I think you raise a real issue, and we > agreed on a solution, which can now be easily setup with the above > change which has been applied and support for functions like: > > static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd) > static inline u64 mtd_get_master_ofs(struct mtd_info *mtd, u64 ofs) > static inline bool mtd_is_partition(const struct mtd_info *mtd) > static inline bool mtd_has_partitions(const struct mtd_info *mtd) > So I should only set master->oops_panic_write with the new code ?. > Thanks, > Miquèl Kamal
[Patch] mtd:rawnand: brcmnand: Fix PM resume crash
This change fixes crash observed on PM resume. This bug was introduced in the change made for flash-edu support. Fixes: a5d53ad26a8b ("mtd: rawnand: brcmnand: Add support for flash-edu for dma transfers") Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index e4e3ceeac38f..8f9ffb46a09f 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2728,9 +2728,8 @@ static int brcmnand_resume(struct device *dev) flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0); } - if (has_edu(ctrl)) + if (has_edu(ctrl)) { ctrl->edu_config = edu_readl(ctrl, EDU_CONFIG); - else { edu_writel(ctrl, EDU_CONFIG, ctrl->edu_config); edu_readl(ctrl, EDU_CONFIG); brcmnand_edu_init(ctrl); -- 2.17.1
[PATCH] mtd: set mtd partition panic write flag
Check mtd panic write flag and set the mtd partition panic write flag so that low level drivers can use it to take required action to ensure oops data gets written to assigned mtd partition. Fixes: 9f897bfdd8 ("mtd: Add flag to indicate panic_write") Signed-off-by: Kamal Dasu --- drivers/mtd/mtdpart.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index 7328c066c5ba..b4f6479abeda 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -159,6 +159,10 @@ static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, const u_char *buf) { struct mtd_part *part = mtd_to_part(mtd); + + if (mtd->oops_panic_write) + part->parent->oops_panic_write = true; + return part->parent->_panic_write(part->parent, to + part->offset, len, retlen, buf); } -- 2.17.1
Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected
I was testing with UBI/UBIFS where vmalloced data buffers can be passed to the nand_base and then o the controller driver. Probably applies to older kernel 4.1. Can the Patch1/2 be accepted or you want me to send it separately by removing the nand_base changes ?. Kamal Kamal On Mon, Sep 30, 2019 at 3:39 PM Richard Weinberger wrote: > > - Ursprüngliche Mail - > > Von: "Kamal Dasu" > > This has been observed on MIPS4K and MIPS5K architectures. There is a > > check on the controller driver to use pio in such cases. > > I fear your kernel misses commit: > 074a1e1167af ("MIPS: Bounds check virt_addr_valid") > > Thanks, > //richard
Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected
On Mon, Sep 30, 2019 at 12:25 PM Boris Brezillon wrote: > > On Mon, 30 Sep 2019 12:01:28 -0400 > Kamal Dasu wrote: > > > Does anyone have any comments on this patch ?. > > > > Kamal > > > > On Fri, Sep 6, 2019 at 3:49 PM Kamal Dasu wrote: > > > > > > For controller drivers that use DMA and set NAND_USE_BOUNCE_BUFFER > > > option use data buffers that are not vmalloced, aligned and have > > > valid virtual address to be able to do DMA transfers. This change > > > adds additional check and makes use of data buffer allocated > > > in nand_base driver when it is passed a vmalloced data buffer for > > > DMA transfers. > > > > > > Signed-off-by: Kamal Dasu > > > --- > > > drivers/mtd/nand/raw/nand_base.c | 14 -- > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c > > > b/drivers/mtd/nand/raw/nand_base.c > > > index 91f046d4d452..46f6965a896a 100644 > > > --- a/drivers/mtd/nand/raw/nand_base.c > > > +++ b/drivers/mtd/nand/raw/nand_base.c > > > @@ -45,6 +45,12 @@ > > > > > > #include "internals.h" > > > > > > +static int nand_need_bounce_buf(const void *buf, struct nand_chip *chip) > > > +{ > > > + return !virt_addr_valid(buf) || is_vmalloc_addr(buf) || > > I thought virt_addr_valid() was implying !is_vmalloc_addr(), are you > sure you need that test, and can you tell me on which arch(es) this is > needed. > This has been observed on MIPS4K and MIPS5K architectures. There is a check on the controller driver to use pio in such cases. > > > + !IS_ALIGNED((unsigned long)buf, chip->buf_align); > > > +} > > > + > > > /* Define default oob placement schemes for large and small page devices > > > */ > > > static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section, > > > struct mtd_oob_region *oobregion) > > > @@ -3183,9 +3189,7 @@ static int nand_do_read_ops(struct nand_chip *chip, > > > loff_t from, > > > if (!aligned) > > > use_bufpoi = 1; > > > else if (chip->options & NAND_USE_BOUNCE_BUFFER) > > > - use_bufpoi = !virt_addr_valid(buf) || > > > -!IS_ALIGNED((unsigned long)buf, > > > -chip->buf_align); > > > + use_bufpoi = nand_need_bounce_buf(buf, chip); > > > else > > > use_bufpoi = 0; > > > > > > @@ -4009,9 +4013,7 @@ static int nand_do_write_ops(struct nand_chip > > > *chip, loff_t to, > > > if (part_pagewr) > > > use_bufpoi = 1; > > > else if (chip->options & NAND_USE_BOUNCE_BUFFER) > > > - use_bufpoi = !virt_addr_valid(buf) || > > > -!IS_ALIGNED((unsigned long)buf, > > > -chip->buf_align); > > > + use_bufpoi = nand_need_bounce_buf(buf, chip); > > > else > > > use_bufpoi = 0; > > > > > > -- > > > 2.17.1 > > > >
Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected
Does anyone have any comments on this patch ?. Kamal On Fri, Sep 6, 2019 at 3:49 PM Kamal Dasu wrote: > > For controller drivers that use DMA and set NAND_USE_BOUNCE_BUFFER > option use data buffers that are not vmalloced, aligned and have > valid virtual address to be able to do DMA transfers. This change > adds additional check and makes use of data buffer allocated > in nand_base driver when it is passed a vmalloced data buffer for > DMA transfers. > > Signed-off-by: Kamal Dasu > --- > drivers/mtd/nand/raw/nand_base.c | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c > b/drivers/mtd/nand/raw/nand_base.c > index 91f046d4d452..46f6965a896a 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -45,6 +45,12 @@ > > #include "internals.h" > > +static int nand_need_bounce_buf(const void *buf, struct nand_chip *chip) > +{ > + return !virt_addr_valid(buf) || is_vmalloc_addr(buf) || > + !IS_ALIGNED((unsigned long)buf, chip->buf_align); > +} > + > /* Define default oob placement schemes for large and small page devices */ > static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section, > struct mtd_oob_region *oobregion) > @@ -3183,9 +3189,7 @@ static int nand_do_read_ops(struct nand_chip *chip, > loff_t from, > if (!aligned) > use_bufpoi = 1; > else if (chip->options & NAND_USE_BOUNCE_BUFFER) > - use_bufpoi = !virt_addr_valid(buf) || > -!IS_ALIGNED((unsigned long)buf, > -chip->buf_align); > + use_bufpoi = nand_need_bounce_buf(buf, chip); > else > use_bufpoi = 0; > > @@ -4009,9 +4013,7 @@ static int nand_do_write_ops(struct nand_chip *chip, > loff_t to, > if (part_pagewr) > use_bufpoi = 1; > else if (chip->options & NAND_USE_BOUNCE_BUFFER) > - use_bufpoi = !virt_addr_valid(buf) || > -!IS_ALIGNED((unsigned long)buf, > -chip->buf_align); > + use_bufpoi = nand_need_bounce_buf(buf, chip); > else > use_bufpoi = 0; > > -- > 2.17.1 >
[PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected
For controller drivers that use DMA and set NAND_USE_BOUNCE_BUFFER option use data buffers that are not vmalloced, aligned and have valid virtual address to be able to do DMA transfers. This change adds additional check and makes use of data buffer allocated in nand_base driver when it is passed a vmalloced data buffer for DMA transfers. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/nand_base.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 91f046d4d452..46f6965a896a 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -45,6 +45,12 @@ #include "internals.h" +static int nand_need_bounce_buf(const void *buf, struct nand_chip *chip) +{ + return !virt_addr_valid(buf) || is_vmalloc_addr(buf) || + !IS_ALIGNED((unsigned long)buf, chip->buf_align); +} + /* Define default oob placement schemes for large and small page devices */ static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section, struct mtd_oob_region *oobregion) @@ -3183,9 +3189,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from, if (!aligned) use_bufpoi = 1; else if (chip->options & NAND_USE_BOUNCE_BUFFER) - use_bufpoi = !virt_addr_valid(buf) || -!IS_ALIGNED((unsigned long)buf, -chip->buf_align); + use_bufpoi = nand_need_bounce_buf(buf, chip); else use_bufpoi = 0; @@ -4009,9 +4013,7 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to, if (part_pagewr) use_bufpoi = 1; else if (chip->options & NAND_USE_BOUNCE_BUFFER) - use_bufpoi = !virt_addr_valid(buf) || -!IS_ALIGNED((unsigned long)buf, -chip->buf_align); + use_bufpoi = nand_need_bounce_buf(buf, chip); else use_bufpoi = 0; -- 2.17.1
[PATCH 1/2] mtd: nand: brcmnand: Add support for flash-dma v0
This change adds support for flash dma v0.0. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 33310b8a6eb8..1eade9dc3b0d 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -117,6 +117,18 @@ enum flash_dma_reg { FLASH_DMA_CURRENT_DESC_EXT, }; +/* flash_dma registers v0*/ +static const u16 flash_dma_regs_v0[] = { + [FLASH_DMA_REVISION]= 0x00, + [FLASH_DMA_FIRST_DESC] = 0x04, + [FLASH_DMA_CTRL]= 0x08, + [FLASH_DMA_MODE]= 0x0c, + [FLASH_DMA_STATUS] = 0x10, + [FLASH_DMA_INTERRUPT_DESC] = 0x14, + [FLASH_DMA_ERROR_STATUS]= 0x18, + [FLASH_DMA_CURRENT_DESC]= 0x1c, +}; + /* flash_dma registers v1*/ static const u16 flash_dma_regs_v1[] = { [FLASH_DMA_REVISION]= 0x00, @@ -597,6 +609,8 @@ static void brcmnand_flash_dma_revision_init(struct brcmnand_controller *ctrl) /* flash_dma register offsets */ if (ctrl->nand_version >= 0x0703) ctrl->flash_dma_offsets = flash_dma_regs_v4; + else if (ctrl->nand_version == 0x0602) + ctrl->flash_dma_offsets = flash_dma_regs_v0; else ctrl->flash_dma_offsets = flash_dma_regs_v1; } @@ -1673,8 +1687,11 @@ static void brcmnand_dma_run(struct brcmnand_host *host, dma_addr_t desc) flash_dma_writel(ctrl, FLASH_DMA_FIRST_DESC, lower_32_bits(desc)); (void)flash_dma_readl(ctrl, FLASH_DMA_FIRST_DESC); - flash_dma_writel(ctrl, FLASH_DMA_FIRST_DESC_EXT, upper_32_bits(desc)); - (void)flash_dma_readl(ctrl, FLASH_DMA_FIRST_DESC_EXT); + if (ctrl->nand_version > 0x0602) { + flash_dma_writel(ctrl, FLASH_DMA_FIRST_DESC_EXT, +upper_32_bits(desc)); + (void)flash_dma_readl(ctrl, FLASH_DMA_FIRST_DESC_EXT); + } /* Start FLASH_DMA engine */ ctrl->dma_pending = true; -- 2.17.1
Re: [PATCH v1 1/1] spi: bcm-qspi: Make BSPI default mode
The spi-nor controller defaults to BSPI mode. So its being put to its default mode. Kamal On Tue, Aug 6, 2019 at 8:16 AM Mark Brown wrote: > > On Tue, Aug 06, 2019 at 03:44:34PM +0530, Rayagonda Kokatanur wrote: > > Switch back to BSPI mode after MSPI operations (write and erase) > > are completed. This change will keep qpsi in BSPI mode by default. > > Why?
[PATCH] mtd: rawnand: brcmnand: Fix ecc chunk calculation for erased page bitfips
From: Claire Lin In brcmstb_nand_verify_erased_page(), fix ecc chunk pointer calculation while correcting erased page bitflip. Fixes: 02b88eea9f9c ("mtd: brcmnand: Add check for erased page bitflips") Signed-off-by: Claire Lin Reviewed-by: Ray Jui Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 33310b8..15ef30b 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -1792,6 +1792,7 @@ static int brcmstb_nand_verify_erased_page(struct mtd_info *mtd, int bitflips = 0; int page = addr >> chip->page_shift; int ret; + void *ecc_chunk; if (!buf) buf = nand_get_data_buf(chip); @@ -1804,7 +1805,9 @@ static int brcmstb_nand_verify_erased_page(struct mtd_info *mtd, return ret; for (i = 0; i < chip->ecc.steps; i++, oob += sas) { - ret = nand_check_erased_ecc_chunk(buf, chip->ecc.size, + ecc_chunk = buf + chip->ecc.size * i; + ret = nand_check_erased_ecc_chunk(ecc_chunk, + chip->ecc.size, oob, sas, NULL, 0, chip->ecc.strength); if (ret < 0) -- 1.9.0.138.g2de3478
[PATCH v4 2/2] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
If mtd_oops is in progress, switch to polling during NAND command completion instead of relying on DMA/interrupts so that the mtd_oops buffer can be completely written in the assigned NAND partition. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 48 ++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 8735277..27b22d6 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -151,6 +151,7 @@ struct brcmnand_controller { u32 nand_cs_nand_xor; u32 corr_stat_threshold; u32 flash_dma_mode; + boolpio_poll_mode; }; struct brcmnand_cfg { @@ -815,6 +816,20 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl) return ctrl->flash_dma_base; } +static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl) +{ + if (ctrl->pio_poll_mode) + return; + + if (has_flash_dma(ctrl)) { + ctrl->flash_dma_base = 0; + disable_irq(ctrl->dma_irq); + } + + disable_irq(ctrl->irq); + ctrl->pio_poll_mode = true; +} + static inline bool flash_dma_buf_ok(const void *buf) { return buf && !is_vmalloc_addr(buf) && @@ -1229,15 +1244,42 @@ static void brcmnand_cmd_ctrl(struct nand_chip *chip, int dat, /* intentionally left blank */ } +static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) +{ + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + struct mtd_info *mtd = nand_to_mtd(chip); + bool err = false; + int sts; + + if (mtd->panic_write_triggered) { + /* switch to interrupt polling and PIO mode */ + disable_ctrl_irqs(ctrl); + sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, + NAND_CTRL_RDY, 0); + err = (sts < 0) ? true : false; + } else { + unsigned long timeo = msecs_to_jiffies( + NAND_POLL_STATUS_TIMEOUT_MS); + /* wait for completion interrupt */ + sts = wait_for_completion_timeout(&ctrl->done, timeo); + err = (sts <= 0) ? true : false; + } + + return err; +} + static int brcmnand_waitfunc(struct nand_chip *chip) { struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_controller *ctrl = host->ctrl; - unsigned long timeo = msecs_to_jiffies(100); + bool err = false; dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending); - if (ctrl->cmd_pending && - wait_for_completion_timeout(&ctrl->done, timeo) <= 0) { + if (ctrl->cmd_pending) + err = brcmstb_nand_wait_for_completion(chip); + + if (err) { u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) >> brcmnand_cmd_shift(ctrl); -- 1.9.0.138.g2de3478
[PATCH v4 1/2] mtd: Add flag to indicate panic_write
Added a flag to indicate a panic_write so that low level drivers can use it to take required action where applicable, to ensure oops data gets written to assigned mtd device. Signed-off-by: Kamal Dasu --- drivers/mtd/mtdcore.c | 3 +++ include/linux/mtd/mtd.h | 6 ++ 2 files changed, 9 insertions(+) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 453242d..2e04627 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1124,6 +1124,9 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, return -EROFS; if (!len) return 0; + + mtd->panic_write_triggered = true; + return mtd->_panic_write(mtd, to, len, retlen, buf); } EXPORT_SYMBOL_GPL(mtd_panic_write); diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 936a3fd..02dce49 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -316,6 +316,12 @@ struct mtd_info { int (*_get_device) (struct mtd_info *mtd); void (*_put_device) (struct mtd_info *mtd); + /* +* flag indicates a panic write, low level drivers can take appropriate +* action if required to ensure writes go through +*/ + bool panic_write_triggered; + struct notifier_block reboot_notifier; /* default mode before reboot */ /* ECC status information */ -- 1.9.0.138.g2de3478
Re: [PATCH v3 2/2] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
Richard, You have any other review comments/concerns with this patch, if not can you please sign off on it. Thanks Kamal On Fri, May 17, 2019 at 7:56 AM Kamal Dasu wrote: > > On Fri, May 17, 2019 at 4:12 AM Richard Weinberger > wrote: > > > > On Thu, May 16, 2019 at 6:42 PM Kamal Dasu wrote: > > > > > > If mtd_oops is in progress, switch to polling during NAND command > > > completion instead of relying on DMA/interrupts so that the mtd_oops > > > buffer can be completely written in the assigned NAND partition. > > > > With the new flag the semantics change, as soon a panic write happened, > > the flag will stay and *all* future operates will take the polling/pio path. > > > > Yes that is true. > > > IMHO this is fine since the kernel cannot recover from an oops. > > But just to make sure we all get this. :-) > > An alternative would be to block all further non-panic writes. > > Capturing the panic writes into an mtd device reliably is what the low > level driver is trying to do.If there are non panic writes they will > use pio and interrupt polling as well in this case. > > > -- > > Thanks, > > //richard > > Thanks > Kamal
[PATCH v2 1/3] mtd: nand: raw: brcmnand: Refactored code to introduce helper functions
Refactored NAND ECC and CMD address configuration code to use helper functions. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++ 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..c534ea8 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl, __raw_writel(val, ctrl->nand_fc + word * 4); } +static void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl) +{ + + /* Clear error addresses */ + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); +} + +static u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl) +{ + u64 err_addr; + + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR); + err_addr |= ((u64)(brcmnand_read_reg(ctrl, +BRCMNAND_UNCORR_EXT_ADDR) +& 0x) << 32); + + return err_addr; +} + +static u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl) +{ + u64 err_addr; + + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR); + err_addr |= ((u64)(brcmnand_read_reg(ctrl, +BRCMNAND_CORR_EXT_ADDR) +& 0x) << 32); + + return err_addr; +} + +static void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + + brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, + (host->cs << 16) | ((addr >> 32) & 0x)); + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, + lower_32_bits(addr)); + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); +} + static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs, enum brcmnand_cs_reg reg) { @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd) { struct brcmnand_controller *ctrl = host->ctrl; int ret; + u64 cmd_addr; + + cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + + dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr); - dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd, - brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS)); BUG_ON(ctrl->cmd_pending != 0); ctrl->cmd_pending = cmd; @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command, if (!native_cmd) return; - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, - (host->cs << 16) | ((addr >> 32) & 0x)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); - + brcmnand_set_cmd_addr(mtd, addr); brcmnand_send_cmd(host, native_cmd); brcmnand_waitfunc(chip); @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, struct brcmnand_controller *ctrl = host->ctrl; int i, j, ret = 0; - /* Clear error addresses */ - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); - - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, - (host->cs << 16) | ((addr >> 32) & 0x)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); + brcmnand_clear_ecc_addr(ctrl); for (i = 0; i < trans; i++, addr += FC_BYTES) { - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, - lower_32_bits(addr)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + brcmnand_set_cmd_addr(mtd, addr); /* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */ brcmnand_send_cmd(host, CMD_PAGE_READ); brcmnand_waitfunc(chip); @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_i
[PATCH v2 2/3] mtd: nand: raw: brcmnand: Add support for v7.3 controller
This change adds support for brcm NAND v7.3 controller. This controller uses a newer version of flash_dma engine and change mostly implements these differences. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 102 --- 1 file changed, 80 insertions(+), 22 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index c534ea8..bc7d80b 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -92,6 +92,12 @@ struct brcm_nand_dma_desc { #define FLASH_DMA_ECC_ERROR(1 << 8) #define FLASH_DMA_CORR_ERROR (1 << 9) +/* Bitfields for DMA_MODE */ +#define FLASH_DMA_MODE_STOP_ON_ERROR BIT(1) /* stop in Uncorr ECC error */ +#define FLASH_DMA_MODE_MODEBIT(0) /* link list */ +#define FLASH_DMA_MODE_MASK(FLASH_DMA_MODE_STOP_ON_ERROR | \ + FLASH_DMA_MODE_MODE) + /* 512B flash cache in the NAND controller HW */ #define FC_SHIFT 9U #define FC_BYTES 512U @@ -104,6 +110,51 @@ struct brcm_nand_dma_desc { #define NAND_CTRL_RDY (INTFC_CTLR_READY | INTFC_FLASH_READY) #define NAND_POLL_STATUS_TIMEOUT_MS100 +/* flash_dma registers */ +enum flash_dma_reg { + FLASH_DMA_REVISION = 0, + FLASH_DMA_FIRST_DESC, + FLASH_DMA_FIRST_DESC_EXT, + FLASH_DMA_CTRL, + FLASH_DMA_MODE, + FLASH_DMA_STATUS, + FLASH_DMA_INTERRUPT_DESC, + FLASH_DMA_INTERRUPT_DESC_EXT, + FLASH_DMA_ERROR_STATUS, + FLASH_DMA_CURRENT_DESC, + FLASH_DMA_CURRENT_DESC_EXT, +}; + +/* flash_dma registers v1*/ +static const u16 flash_dma_regs_v1[] = { + [FLASH_DMA_REVISION]= 0x00, + [FLASH_DMA_FIRST_DESC] = 0x04, + [FLASH_DMA_FIRST_DESC_EXT] = 0x08, + [FLASH_DMA_CTRL]= 0x0c, + [FLASH_DMA_MODE]= 0x10, + [FLASH_DMA_STATUS] = 0x14, + [FLASH_DMA_INTERRUPT_DESC] = 0x18, + [FLASH_DMA_INTERRUPT_DESC_EXT] = 0x1c, + [FLASH_DMA_ERROR_STATUS]= 0x20, + [FLASH_DMA_CURRENT_DESC]= 0x24, + [FLASH_DMA_CURRENT_DESC_EXT]= 0x28, +}; + +/* flash_dma registers v4 */ +static const u16 flash_dma_regs_v4[] = { + [FLASH_DMA_REVISION]= 0x00, + [FLASH_DMA_FIRST_DESC] = 0x08, + [FLASH_DMA_FIRST_DESC_EXT] = 0x0c, + [FLASH_DMA_CTRL]= 0x10, + [FLASH_DMA_MODE]= 0x14, + [FLASH_DMA_STATUS] = 0x18, + [FLASH_DMA_INTERRUPT_DESC] = 0x20, + [FLASH_DMA_INTERRUPT_DESC_EXT] = 0x24, + [FLASH_DMA_ERROR_STATUS]= 0x28, + [FLASH_DMA_CURRENT_DESC]= 0x30, + [FLASH_DMA_CURRENT_DESC_EXT]= 0x34, +}; + /* Controller feature flags */ enum { BRCMNAND_HAS_1K_SECTORS = BIT(0), @@ -136,6 +187,8 @@ struct brcmnand_controller { /* List of NAND hosts (one for each chip-select) */ struct list_head host_list; + /* flash_dma reg */ + const u16 *flash_dma_offsets; struct brcm_nand_dma_desc *dma_desc; dma_addr_t dma_pa; @@ -470,7 +523,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl) /* Register offsets */ if (ctrl->nand_version >= 0x0702) ctrl->reg_offsets = brcmnand_regs_v72; - else if (ctrl->nand_version >= 0x0701) + else if (ctrl->nand_version == 0x0701) ctrl->reg_offsets = brcmnand_regs_v71; else if (ctrl->nand_version >= 0x0600) ctrl->reg_offsets = brcmnand_regs_v60; @@ -515,7 +568,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl) } /* Maximum spare area sector size (per 512B) */ - if (ctrl->nand_version >= 0x0702) + if (ctrl->nand_version == 0x0702) ctrl->max_oob = 128; else if (ctrl->nand_version >= 0x0600) ctrl->max_oob = 64; @@ -546,6 +599,15 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl) return 0; } +static void brcmnand_flash_dma_revision_init(struct brcmnand_controller *ctrl) +{ + /* flash_dma register offsets */ + if (ctrl->nand_version >= 0x0703) + ctrl->flash_dma_offsets = flash_dma_regs_v4; + else + ctrl->flash_dma_offsets = flash_dma_regs_v1; +} + static inline u32 brcmnand_read_reg(struct brcmnand_controller *ctrl, enum brcmnand_reg reg) { @@ -668,7 +730,7 @@ static void brcmnand_wr_corr_thresh(struct brcmnand_host *host, u8 val) enum brcmnand_reg reg = BRCMNAND_CORR_THRESHOLD; int cs = host->cs; - if (ctrl->nand_version >= 0x0702) + if (ctrl-&g
[PATCH v2 3/3] dt: bindings: mtd: brcmand: Add brcmnand,brcmnand-v7.3 support
Added brcm,brcmnand-v7.3 as possible compatible string to support brcmnand controller v7.3. Signed-off-by: Kamal Dasu --- Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt index 0b7c373..ad4cd30 100644 --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt @@ -28,6 +28,7 @@ Required properties: brcm,brcmnand-v7.0 brcm,brcmnand-v7.1 brcm,brcmnand-v7.2 + brcm,brcmnand-v7.3 brcm,brcmnand - reg : the register start and length for NAND register region. (optional) Flash DMA register range (if present) -- 1.9.0.138.g2de3478
Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
On Mon, Jun 3, 2019 at 10:18 AM Boris Brezillon wrote: > > On Mon, 3 Jun 2019 10:11:20 -0400 > Kamal Dasu wrote: > > > Boris, > > > > On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon > > wrote: > > > > > > On Thu, 30 May 2019 17:20:35 -0400 > > > Kamal Dasu wrote: > > > > > > > Refactored NAND ECC and CMD address configuration code to use inline > > > > functions. > > > > > > I'd expect the compiler to be smart enough to decide when inlining is > > > appropriate. Did you check that adding the inline specifier actually > > > makes a difference? > > > > This was done to make the code more readable. It does not make any > > difference to performance. > > I meant dropping the inline specifier, not going back to manual > inlining. As a general rule, you don't need to add the 'inline' > specifier unless your function is defined in a header. In all other > cases the compiler is able to inline things on its own when it sees the > number of instructions is small enough or when the function is only > called once. Oh ok got it, will get rid if the inline specifier and send a v2 version of the change. Thanks Kamal
Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
Boris, On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon wrote: > > On Thu, 30 May 2019 17:20:35 -0400 > Kamal Dasu wrote: > > > Refactored NAND ECC and CMD address configuration code to use inline > > functions. > > I'd expect the compiler to be smart enough to decide when inlining is > appropriate. Did you check that adding the inline specifier actually > makes a difference? This was done to make the code more readable. It does not make any difference to performance. > > > > > Signed-off-by: Kamal Dasu > > --- > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 > > +++ > > 1 file changed, 62 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > index ce0b8ff..77b7850 100644 > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct > > brcmnand_controller *ctrl, > > __raw_writel(val, ctrl->nand_fc + word * 4); > > } > > > > +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller > > *ctrl) > > +{ > > + > > + /* Clear error addresses */ > > + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); > > + brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); > > + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); > > + brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); > > +} > > + > > +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller > > *ctrl) > > +{ > > + u64 err_addr; > > + > > + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR); > > + err_addr |= ((u64)(brcmnand_read_reg(ctrl, > > + BRCMNAND_UNCORR_EXT_ADDR) > > + & 0x) << 32); > > + > > + return err_addr; > > +} > > + > > +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller > > *ctrl) > > +{ > > + u64 err_addr; > > + > > + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR); > > + err_addr |= ((u64)(brcmnand_read_reg(ctrl, > > + BRCMNAND_CORR_EXT_ADDR) > > + & 0x) << 32); > > + > > + return err_addr; > > +} > > + > > +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr) > > +{ > > + struct nand_chip *chip = mtd_to_nand(mtd); > > + struct brcmnand_host *host = nand_get_controller_data(chip); > > + struct brcmnand_controller *ctrl = host->ctrl; > > + > > + brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > > +(host->cs << 16) | ((addr >> 32) & 0x)); > > + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > > + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, > > +lower_32_bits(addr)); > > + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > +} > > + > > static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int > > cs, > >enum brcmnand_cs_reg reg) > > { > > @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host > > *host, int cmd) > > { > > struct brcmnand_controller *ctrl = host->ctrl; > > int ret; > > + u64 cmd_addr; > > + > > + cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > + > > + dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr); > > > > - dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd, > > - brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS)); > > BUG_ON(ctrl->cmd_pending != 0); > > ctrl->cmd_pending = cmd; > > > > @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, > > unsigned command, > > if (!native_cmd) > > return; > > > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, > > - (host->cs << 16) | ((addr >> 32) & 0x)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); > > - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr)); > > - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); > > - > > +
[PATCH 2/3] mtd: nand: raw: brcmnand: Add support for v7.3 controller
This change adds support for brcm NAND v7.3 controller. This controller uses a newer version of flash_dma engine and change mostly implements these differences. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 102 --- 1 file changed, 80 insertions(+), 22 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 77b7850..544088f 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -92,6 +92,12 @@ struct brcm_nand_dma_desc { #define FLASH_DMA_ECC_ERROR(1 << 8) #define FLASH_DMA_CORR_ERROR (1 << 9) +/* Bitfields for DMA_MODE */ +#define FLASH_DMA_MODE_STOP_ON_ERROR BIT(1) /* stop in Uncorr ECC error */ +#define FLASH_DMA_MODE_MODEBIT(0) /* link list */ +#define FLASH_DMA_MODE_MASK(FLASH_DMA_MODE_STOP_ON_ERROR | \ + FLASH_DMA_MODE_MODE) + /* 512B flash cache in the NAND controller HW */ #define FC_SHIFT 9U #define FC_BYTES 512U @@ -104,6 +110,51 @@ struct brcm_nand_dma_desc { #define NAND_CTRL_RDY (INTFC_CTLR_READY | INTFC_FLASH_READY) #define NAND_POLL_STATUS_TIMEOUT_MS100 +/* flash_dma registers */ +enum flash_dma_reg { + FLASH_DMA_REVISION = 0, + FLASH_DMA_FIRST_DESC, + FLASH_DMA_FIRST_DESC_EXT, + FLASH_DMA_CTRL, + FLASH_DMA_MODE, + FLASH_DMA_STATUS, + FLASH_DMA_INTERRUPT_DESC, + FLASH_DMA_INTERRUPT_DESC_EXT, + FLASH_DMA_ERROR_STATUS, + FLASH_DMA_CURRENT_DESC, + FLASH_DMA_CURRENT_DESC_EXT, +}; + +/* flash_dma registers v1*/ +static const u16 flash_dma_regs_v1[] = { + [FLASH_DMA_REVISION]= 0x00, + [FLASH_DMA_FIRST_DESC] = 0x04, + [FLASH_DMA_FIRST_DESC_EXT] = 0x08, + [FLASH_DMA_CTRL]= 0x0c, + [FLASH_DMA_MODE]= 0x10, + [FLASH_DMA_STATUS] = 0x14, + [FLASH_DMA_INTERRUPT_DESC] = 0x18, + [FLASH_DMA_INTERRUPT_DESC_EXT] = 0x1c, + [FLASH_DMA_ERROR_STATUS]= 0x20, + [FLASH_DMA_CURRENT_DESC]= 0x24, + [FLASH_DMA_CURRENT_DESC_EXT]= 0x28, +}; + +/* flash_dma registers v4 */ +static const u16 flash_dma_regs_v4[] = { + [FLASH_DMA_REVISION]= 0x00, + [FLASH_DMA_FIRST_DESC] = 0x08, + [FLASH_DMA_FIRST_DESC_EXT] = 0x0c, + [FLASH_DMA_CTRL]= 0x10, + [FLASH_DMA_MODE]= 0x14, + [FLASH_DMA_STATUS] = 0x18, + [FLASH_DMA_INTERRUPT_DESC] = 0x20, + [FLASH_DMA_INTERRUPT_DESC_EXT] = 0x24, + [FLASH_DMA_ERROR_STATUS]= 0x28, + [FLASH_DMA_CURRENT_DESC]= 0x30, + [FLASH_DMA_CURRENT_DESC_EXT]= 0x34, +}; + /* Controller feature flags */ enum { BRCMNAND_HAS_1K_SECTORS = BIT(0), @@ -136,6 +187,8 @@ struct brcmnand_controller { /* List of NAND hosts (one for each chip-select) */ struct list_head host_list; + /* flash_dma reg */ + const u16 *flash_dma_offsets; struct brcm_nand_dma_desc *dma_desc; dma_addr_t dma_pa; @@ -470,7 +523,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl) /* Register offsets */ if (ctrl->nand_version >= 0x0702) ctrl->reg_offsets = brcmnand_regs_v72; - else if (ctrl->nand_version >= 0x0701) + else if (ctrl->nand_version == 0x0701) ctrl->reg_offsets = brcmnand_regs_v71; else if (ctrl->nand_version >= 0x0600) ctrl->reg_offsets = brcmnand_regs_v60; @@ -515,7 +568,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl) } /* Maximum spare area sector size (per 512B) */ - if (ctrl->nand_version >= 0x0702) + if (ctrl->nand_version == 0x0702) ctrl->max_oob = 128; else if (ctrl->nand_version >= 0x0600) ctrl->max_oob = 64; @@ -546,6 +599,15 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl) return 0; } +static void brcmnand_flash_dma_revision_init(struct brcmnand_controller *ctrl) +{ + /* flash_dma register offsets */ + if (ctrl->nand_version >= 0x0703) + ctrl->flash_dma_offsets = flash_dma_regs_v4; + else + ctrl->flash_dma_offsets = flash_dma_regs_v1; +} + static inline u32 brcmnand_read_reg(struct brcmnand_controller *ctrl, enum brcmnand_reg reg) { @@ -668,7 +730,7 @@ static void brcmnand_wr_corr_thresh(struct brcmnand_host *host, u8 val) enum brcmnand_reg reg = BRCMNAND_CORR_THRESHOLD; int cs = host->cs; - if (ctrl->nand_version >= 0x0702) + if (ctrl-&g
[PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
Refactored NAND ECC and CMD address configuration code to use inline functions. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++ 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..77b7850 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl, __raw_writel(val, ctrl->nand_fc + word * 4); } +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl) +{ + + /* Clear error addresses */ + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); + brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); +} + +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl) +{ + u64 err_addr; + + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR); + err_addr |= ((u64)(brcmnand_read_reg(ctrl, +BRCMNAND_UNCORR_EXT_ADDR) +& 0x) << 32); + + return err_addr; +} + +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl) +{ + u64 err_addr; + + err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR); + err_addr |= ((u64)(brcmnand_read_reg(ctrl, +BRCMNAND_CORR_EXT_ADDR) +& 0x) << 32); + + return err_addr; +} + +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + + brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, + (host->cs << 16) | ((addr >> 32) & 0x)); + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); + brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, + lower_32_bits(addr)); + (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); +} + static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs, enum brcmnand_cs_reg reg) { @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd) { struct brcmnand_controller *ctrl = host->ctrl; int ret; + u64 cmd_addr; + + cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + + dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr); - dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd, - brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS)); BUG_ON(ctrl->cmd_pending != 0); ctrl->cmd_pending = cmd; @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command, if (!native_cmd) return; - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, - (host->cs << 16) | ((addr >> 32) & 0x)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); - + brcmnand_set_cmd_addr(mtd, addr); brcmnand_send_cmd(host, native_cmd); brcmnand_waitfunc(chip); @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip, struct brcmnand_controller *ctrl = host->ctrl; int i, j, ret = 0; - /* Clear error addresses */ - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0); - brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0); - - brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS, - (host->cs << 16) | ((addr >> 32) & 0x)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS); + brcmnand_clear_ecc_addr(ctrl); for (i = 0; i < trans; i++, addr += FC_BYTES) { - brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, - lower_32_bits(addr)); - (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); + brcmnand_set_cmd_addr(mtd, addr); /* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */ brcmnand_send_cmd(host, CMD_PAGE_READ); brcmnand_waitfunc(chip); @@ -1630,21 +1666,15 @@
[PATCH 3/3] dt: bindings: mtd: brcmand: Add brcmnand,brcmnand-v7.3 support
Added brcm,brcmnand-v7.3 as possible compatible string to support brcmnand controller v7.3. Signed-off-by: Kamal Dasu --- Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt index 0b7c373..ad4cd30 100644 --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt @@ -28,6 +28,7 @@ Required properties: brcm,brcmnand-v7.0 brcm,brcmnand-v7.1 brcm,brcmnand-v7.2 + brcm,brcmnand-v7.3 brcm,brcmnand - reg : the register start and length for NAND register region. (optional) Flash DMA register range (if present) -- 1.9.0.138.g2de3478
[PATCH] dt: bindings: mtd: replace references to nand.txt with nand-controller.yaml
nand-controller.yaml replaced nand.txt however the references to it were not updated. This change updates these references wherever it appears in bindings documentation. Fixes: 212e49693592 ("dt-bindings: mtd: Add YAML schemas for the generic NAND options") Signed-off-by: Kamal Dasu --- .../devicetree/bindings/mtd/amlogic,meson-nand.txt | 2 +- Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt| 6 +++--- Documentation/devicetree/bindings/mtd/denali-nand.txt | 6 +++--- Documentation/devicetree/bindings/mtd/fsmc-nand.txt| 6 +++--- Documentation/devicetree/bindings/mtd/gpmc-nand.txt| 2 +- Documentation/devicetree/bindings/mtd/hisi504-nand.txt | 2 +- Documentation/devicetree/bindings/mtd/marvell-nand.txt | 14 +++--- Documentation/devicetree/bindings/mtd/mxc-nand.txt | 6 +++--- .../devicetree/bindings/mtd/nvidia-tegra20-nand.txt| 6 +++--- Documentation/devicetree/bindings/mtd/oxnas-nand.txt | 2 +- Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 4 ++-- Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt | 6 +++--- Documentation/devicetree/bindings/mtd/stm32-fmc2-nand.txt | 6 +++--- Documentation/devicetree/bindings/mtd/tango-nand.txt | 2 +- Documentation/devicetree/bindings/mtd/vf610-nfc.txt| 8 15 files changed, 39 insertions(+), 39 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt index 3983c11..5794ab11 100644 --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt @@ -24,7 +24,7 @@ Optional children nodes: Children nodes represent the available nand chips. Other properties: -see Documentation/devicetree/bindings/mtd/nand.txt for generic bindings. +see Documentation/devicetree/bindings/mtd/nand-controller.yaml for generic bindings. Example demonstrate on AXG SoC: diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt index bcda1df..0b7c373 100644 --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt @@ -101,12 +101,12 @@ Required properties: number (e.g., 0, 1, 2, etc.) - #address-cells: see partition.txt - #size-cells : see partition.txt -- nand-ecc-strength : see nand.txt -- nand-ecc-step-size: must be 512 or 1024. See nand.txt +- nand-ecc-strength : see nand-controller.yaml +- nand-ecc-step-size: must be 512 or 1024. See nand-controller.yaml Optional properties: - nand-on-flash-bbt : boolean, to enable the on-flash BBT for this - chip-select. See nand.txt + chip-select. See nand-controller.yaml - brcm,nand-oob-sector-size : integer, to denote the spare area sector size expected for the ECC layout in use. This size, in addition to the strength and step-size, diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt index b14b675..b32aed1 100644 --- a/Documentation/devicetree/bindings/mtd/denali-nand.txt +++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt @@ -22,16 +22,16 @@ Sub-nodes: select is connected. Optional properties: -- nand-ecc-step-size: see nand.txt for details. +- nand-ecc-step-size: see nand-controller.yaml for details. If present, the value must be 512for "altr,socfpga-denali-nand" 1024 for "socionext,uniphier-denali-nand-v5a" 1024 for "socionext,uniphier-denali-nand-v5b" -- nand-ecc-strength: see nand.txt for details. Valid values are: +- nand-ecc-strength: see nand-controller.yaml for details. Valid values are: 8, 15 for "altr,socfpga-denali-nand" 8, 16, 24 for "socionext,uniphier-denali-nand-v5a" 8, 16 for "socionext,uniphier-denali-nand-v5b" -- nand-ecc-maximize: see nand.txt for details +- nand-ecc-maximize: see nand-controller.yaml for details The chip nodes may optionally contain sub-nodes describing partitions of the address space. See partition.txt for more detail. diff --git a/Documentation/devicetree/bindings/mtd/fsmc-nand.txt b/Documentation/devicetree/bindings/mtd/fsmc-nand.txt index 32636eb..6762d3c 100644 --- a/Documentation/devicetree/bindings/mtd/fsmc-nand.txt +++ b/Documentation/devicetree/bindings/mtd/fsmc-nand.txt @@ -30,9 +30,9 @@ Optional properties: command is asserted. Zero means one cycle, 255 means 256 cycles. - bank: default NAND bank to use (0-3 are vali
[PATCH v3 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size
This change supports nand-ecc-step-size and nand-ecc-strength fields in brcmnand DT node to be optional. see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt If both nand-ecc-strength and nand-ecc-step-size are not specified in device tree node for NAND, raw NAND layer does detect ECC information by reading ONFI extended parameter page for parts using ONFI >= 2.1. In case of non-ONFI NAND parts there could be a nand_id table entry with ECC information. If there is valid device tree entry for nand-ecc-strength and nand-ecc-step-size fields it still shall override the detected values. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..1bdd490 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2144,6 +2144,17 @@ static int brcmnand_setup_dev(struct brcmnand_host *host) return -EINVAL; } + if (chip->ecc.mode != NAND_ECC_NONE && + (!chip->ecc.size || !chip->ecc.strength)) { + if (chip->base.eccreq.step_size && chip->base.eccreq.strength) { + /* use detected ECC parameters */ + chip->ecc.size = chip->base.eccreq.step_size; + chip->ecc.strength = chip->base.eccreq.strength; + dev_info(ctrl->dev, "Using ECC step-size %d, strength %d\n", + chip->ecc.size, chip->ecc.strength); + } + } + switch (chip->ecc.size) { case 512: if (chip->ecc.algo == NAND_ECC_HAMMING) -- 1.9.0.138.g2de3478
[PATCH v3 1/2] dt-bindings: mtd: brcmnand: Make nand-ecc-strength and nand-ecc-step-size optional
nand-ecc-strength and nand-ecc-step-size can be made optional as brcmnand driver can support using raw NAND layer detected values. Signed-off-by: Kamal Dasu --- Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt index bcda1df..0d952af 100644 --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt @@ -101,12 +101,12 @@ Required properties: number (e.g., 0, 1, 2, etc.) - #address-cells: see partition.txt - #size-cells : see partition.txt -- nand-ecc-strength : see nand.txt -- nand-ecc-step-size: must be 512 or 1024. See nand.txt Optional properties: +- nand-ecc-strength : see nand-controller.yaml +- nand-ecc-step-size: must be 512 or 1024. See nand-controller.yaml - nand-on-flash-bbt : boolean, to enable the on-flash BBT for this - chip-select. See nand.txt + chip-select. See nand-controller.yaml - brcm,nand-oob-sector-size : integer, to denote the spare area sector size expected for the ECC layout in use. This size, in addition to the strength and step-size, -- 1.9.0.138.g2de3478
[PATCH v2 1/2] dt-bindings: mtd: brcmnand: Make nand-ecc-strength and nand-ecc-step-size optional
nand-ecc-strength and nand-ecc-step-size can be made optional as brcmnand driver can support using raw NAND layer detected values. Signed-off-by: Kamal Dasu --- Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt index bcda1df..29feaba 100644 --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt @@ -101,10 +101,10 @@ Required properties: number (e.g., 0, 1, 2, etc.) - #address-cells: see partition.txt - #size-cells : see partition.txt -- nand-ecc-strength : see nand.txt -- nand-ecc-step-size: must be 512 or 1024. See nand.txt Optional properties: +- nand-ecc-strength : see nand.txt +- nand-ecc-step-size: must be 512 or 1024. See nand.txt - nand-on-flash-bbt : boolean, to enable the on-flash BBT for this chip-select. See nand.txt - brcm,nand-oob-sector-size : integer, to denote the spare area sector size -- 1.9.0.138.g2de3478
[PATCH v2 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size
This change supports nand-ecc-step-size and nand-ecc-strength fields in brcmnand DT node to be optional. see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt If both nand-ecc-strength and nand-ecc-step-size are not specified in device tree node for NAND, raw NAND layer does detect ECC information by reading ONFI extended parameter page for parts using ONFI >= 2.1. In case of non-ONFI NAND parts there could be a nand_id table entry with ECC information. If there is valid device tree entry for nand-ecc-strength and nand-ecc-step-size fields it still shall override the detected values. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..a4d2057 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2144,6 +2144,17 @@ static int brcmnand_setup_dev(struct brcmnand_host *host) return -EINVAL; } + if (chip->ecc.mode != NAND_ECC_NONE && + (!chip->ecc.size || !chip->ecc.strength)) { + if (chip->base.eccreq.step_size && chip->base.eccreq.strength) { + /* use detected ECC parameters */ + chip->ecc.size = chip->base.eccreq.step_size; + chip->ecc.strength = chip->base.eccreq.strength; + pr_info("Using ECC step-size %d, strength %d\n", + chip->ecc.size, chip->ecc.strength); + } + } + switch (chip->ecc.size) { case 512: if (chip->ecc.algo == NAND_ECC_HAMMING) -- 1.9.0.138.g2de3478
Re: [PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size
On Mon, May 20, 2019 at 1:34 PM Miquel Raynal wrote: > > Hi Kamal, > > Kamal Dasu wrote on Mon, 20 May 2019 13:31:52 > -0400: > > > Will make the changes and send a V2 patch. > > > > On Mon, May 20, 2019 at 8:44 AM Miquel Raynal > > wrote: > > > > > > Hi Kamal, > > > > > > Kamal Dasu wrote on Fri, 17 May 2019 14:29:55 > > > -0400: > > > > > > > This change supports nand-ecc-step-size and nand-ecc-strenght fields in > > > > > >strength > > > > > > > brcmnand dt node to be optional. > > > > > >DT^ extra space > > > > > > > see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt > > > > > > > > If both nand-ecc-strength and nand-ecc-step-size are not specified in > > > > device tree node for NAND, nand_base driver does detect onfi ext ecc > > > > > > s/nand_base driver/the raw NAND layer/ > > > s/onfi/ONFI/ > > > s/ecc/ECC/ > > > > > > What is "ext"? Please use plain English here. > > > > > > > info from ONFI extended parameter page for parts using ONFI >= 2.1. In > > > > > > s/info/information/ > > > > > > > case of non-onfi NAND there could be a nand_id table entry with the ecc > > > > > > s/ecc/ECC/ > > > > > > > info. If there is a valid device tree entry for nand-ecc-strength and > > > > nand-ecc-step-size fields it still shall override the detected values. > > > > > > > > Signed-off-by: Kamal Dasu > > > > --- > > > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > index ce0b8ff..e967b30 100644 > > > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > > > @@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct > > > > brcmnand_host *host) > > > > return -EINVAL; > > > > } > > > > > > > > + if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) && > > > > > > Is the case where only size OR strength is valid handled? > > > > Both strength and need to be valid, else the driver will behave like > > before and will fail the probe. > > Yes, but you do not handle the case when either strength OR size is not > valid but the other one is. Is it one purpose? > If I understand you want me to use the following check: if (ecc->mode != NAND_ECC_NONE && (!ecc->size || !ecc->strength)) { if (chip->base.eccreq.step_size && chip->base.eccreq.strength) { /* use the base values */ } > > > > > > > > > + (chip->base.eccreq.strength > 0 && > > > > + chip->base.eccreq.step_size > 0)) { > > > > + /* use detected ecc parameters */ > > > > > >Use ECC > > > > > > > + chip->ecc.size = chip->base.eccreq.step_size; > > > > + chip->ecc.strength = chip->base.eccreq.strength; > > > > + pr_info("Using detected nand-ecc-step-size %d, > > > > nand-ecc-strength %d\n", > > > > + chip->ecc.size, chip->ecc.strength); > > > > + } > > > > + > > > > switch (chip->ecc.size) { > > > > case 512: > > > > if (chip->ecc.algo == NAND_ECC_HAMMING) > > > > > > > > > Thanks, > > > Miquèl > > > > Kamal > > > > > Thanks, > Miquèl Kamal
Re: [PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size
Will make the changes and send a V2 patch. On Mon, May 20, 2019 at 8:44 AM Miquel Raynal wrote: > > Hi Kamal, > > Kamal Dasu wrote on Fri, 17 May 2019 14:29:55 > -0400: > > > This change supports nand-ecc-step-size and nand-ecc-strenght fields in > >strength > > > brcmnand dt node to be optional. > >DT^ extra space > > > see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt > > > > If both nand-ecc-strength and nand-ecc-step-size are not specified in > > device tree node for NAND, nand_base driver does detect onfi ext ecc > > s/nand_base driver/the raw NAND layer/ > s/onfi/ONFI/ > s/ecc/ECC/ > > What is "ext"? Please use plain English here. > > > info from ONFI extended parameter page for parts using ONFI >= 2.1. In > > s/info/information/ > > > case of non-onfi NAND there could be a nand_id table entry with the ecc > > s/ecc/ECC/ > > > info. If there is a valid device tree entry for nand-ecc-strength and > > nand-ecc-step-size fields it still shall override the detected values. > > > > Signed-off-by: Kamal Dasu > > --- > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > index ce0b8ff..e967b30 100644 > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > @@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct brcmnand_host > > *host) > > return -EINVAL; > > } > > > > + if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) && > > Is the case where only size OR strength is valid handled? Both strength and need to be valid, else the driver will behave like before and will fail the probe. > > > + (chip->base.eccreq.strength > 0 && > > + chip->base.eccreq.step_size > 0)) { > > + /* use detected ecc parameters */ > >Use ECC > > > + chip->ecc.size = chip->base.eccreq.step_size; > > + chip->ecc.strength = chip->base.eccreq.strength; > > + pr_info("Using detected nand-ecc-step-size %d, > > nand-ecc-strength %d\n", > > + chip->ecc.size, chip->ecc.strength); > > + } > > + > > switch (chip->ecc.size) { > > case 512: > > if (chip->ecc.algo == NAND_ECC_HAMMING) > > > Thanks, > Miquèl Kamal
[PATCH 2/2] mtd: nand: raw: brcmnand: fallback to detected ecc-strength, ecc-step-size
This change supports nand-ecc-step-size and nand-ecc-strenght fields in brcmnand dt node to be optional. see: Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt If both nand-ecc-strength and nand-ecc-step-size are not specified in device tree node for NAND, nand_base driver does detect onfi ext ecc info from ONFI extended parameter page for parts using ONFI >= 2.1. In case of non-onfi NAND there could be a nand_id table entry with the ecc info. If there is a valid device tree entry for nand-ecc-strength and nand-ecc-step-size fields it still shall override the detected values. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..e967b30 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2144,6 +2144,16 @@ static int brcmnand_setup_dev(struct brcmnand_host *host) return -EINVAL; } + if (!(chip->ecc.size > 0 && chip->ecc.strength > 0) && + (chip->base.eccreq.strength > 0 && +chip->base.eccreq.step_size > 0)) { + /* use detected ecc parameters */ + chip->ecc.size = chip->base.eccreq.step_size; + chip->ecc.strength = chip->base.eccreq.strength; + pr_info("Using detected nand-ecc-step-size %d, nand-ecc-strength %d\n", + chip->ecc.size, chip->ecc.strength); + } + switch (chip->ecc.size) { case 512: if (chip->ecc.algo == NAND_ECC_HAMMING) -- 1.9.0.138.g2de3478
[PATCH 1/2] dt-bindings: mtd: brcmnand: Make nand-ecc-strength and nand-ecc-step-size optional
nand-ecc-strength and nand-ecc-step-size can be made optional as brcmanand driver can support using the nand_base driver detected values. Signed-off-by: Kamal Dasu --- Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt index bcda1df..29feaba 100644 --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt @@ -101,10 +101,10 @@ Required properties: number (e.g., 0, 1, 2, etc.) - #address-cells: see partition.txt - #size-cells : see partition.txt -- nand-ecc-strength : see nand.txt -- nand-ecc-step-size: must be 512 or 1024. See nand.txt Optional properties: +- nand-ecc-strength : see nand.txt +- nand-ecc-step-size: must be 512 or 1024. See nand.txt - nand-on-flash-bbt : boolean, to enable the on-flash BBT for this chip-select. See nand.txt - brcm,nand-oob-sector-size : integer, to denote the spare area sector size -- 1.9.0.138.g2de3478
Re: [PATCH v3 2/2] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
On Fri, May 17, 2019 at 4:12 AM Richard Weinberger wrote: > > On Thu, May 16, 2019 at 6:42 PM Kamal Dasu wrote: > > > > If mtd_oops is in progress, switch to polling during NAND command > > completion instead of relying on DMA/interrupts so that the mtd_oops > > buffer can be completely written in the assigned NAND partition. > > With the new flag the semantics change, as soon a panic write happened, > the flag will stay and *all* future operates will take the polling/pio path. > Yes that is true. > IMHO this is fine since the kernel cannot recover from an oops. > But just to make sure we all get this. :-) > An alternative would be to block all further non-panic writes. Capturing the panic writes into an mtd device reliably is what the low level driver is trying to do.If there are non panic writes they will use pio and interrupt polling as well in this case. > -- > Thanks, > //richard Thanks Kamal
[PATCH v3 2/2] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
If mtd_oops is in progress, switch to polling during NAND command completion instead of relying on DMA/interrupts so that the mtd_oops buffer can be completely written in the assigned NAND partition. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 48 ++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..dca8eb8 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -159,6 +159,7 @@ struct brcmnand_controller { u32 nand_cs_nand_xor; u32 corr_stat_threshold; u32 flash_dma_mode; + boolpio_poll_mode; }; struct brcmnand_cfg { @@ -823,6 +824,20 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl) return ctrl->flash_dma_base; } +static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl) +{ + if (ctrl->pio_poll_mode) + return; + + if (has_flash_dma(ctrl)) { + ctrl->flash_dma_base = 0; + disable_irq(ctrl->dma_irq); + } + + disable_irq(ctrl->irq); + ctrl->pio_poll_mode = true; +} + static inline bool flash_dma_buf_ok(const void *buf) { return buf && !is_vmalloc_addr(buf) && @@ -1237,15 +1252,42 @@ static void brcmnand_cmd_ctrl(struct nand_chip *chip, int dat, /* intentionally left blank */ } +static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) +{ + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + struct mtd_info *mtd = nand_to_mtd(chip); + bool err = false; + int sts; + + if (mtd->oops_panic_write) { + /* switch to interrupt polling and PIO mode */ + disable_ctrl_irqs(ctrl); + sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, + NAND_CTRL_RDY, 0); + err = (sts < 0) ? true : false; + } else { + unsigned long timeo = msecs_to_jiffies( + NAND_POLL_STATUS_TIMEOUT_MS); + /* wait for completion interrupt */ + sts = wait_for_completion_timeout(&ctrl->done, timeo); + err = (sts <= 0) ? true : false; + } + + return err; +} + static int brcmnand_waitfunc(struct nand_chip *chip) { struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_controller *ctrl = host->ctrl; - unsigned long timeo = msecs_to_jiffies(100); + bool err = false; dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending); - if (ctrl->cmd_pending && - wait_for_completion_timeout(&ctrl->done, timeo) <= 0) { + if (ctrl->cmd_pending) + err = brcmstb_nand_wait_for_completion(chip); + + if (err) { u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) >> brcmnand_cmd_shift(ctrl); -- 1.9.0.138.g2de3478
[PATCH v3 1/2] mtd: Add flag to indicate panic_write
Added a flag to indicate a panic_write so that low level drivers can use it to take required action where applicable, to ensure oops data gets written to assigned mtd device. Signed-off-by: Kamal Dasu --- drivers/mtd/mtdcore.c | 3 +++ include/linux/mtd/mtd.h | 6 ++ 2 files changed, 9 insertions(+) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 76b4264..a83decd 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1138,6 +1138,9 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, return -EROFS; if (!len) return 0; + if (!mtd->oops_panic_write) + mtd->oops_panic_write = true; + return mtd->_panic_write(mtd, to, len, retlen, buf); } EXPORT_SYMBOL_GPL(mtd_panic_write); diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 677768b..791c34d 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -330,6 +330,12 @@ struct mtd_info { int (*_get_device) (struct mtd_info *mtd); void (*_put_device) (struct mtd_info *mtd); + /* +* flag indicates a panic write, low level drivers can take appropriate +* action if required to ensure writes go through +*/ + bool oops_panic_write; + struct notifier_block reboot_notifier; /* default mode before reboot */ /* ECC status information */ -- 1.9.0.138.g2de3478
Re: [PATCH v2 2/2] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
Please ignore v2 patch 1/2 and 2/2. The commit ordering is not right. v3 patch on the way Kamal On Thu, May 16, 2019 at 12:01 PM Kamal Dasu wrote: > > If mtd_oops is in progress switch to polling for nand command completion > interrupts and use PIO mode wihtout DMA so that the mtd_oops buffer can > be completely written in the assinged nand partition. > > Signed-off-by: Kamal Dasu > --- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index a30a7f0..dca8eb8 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -835,7 +835,6 @@ static inline void disable_ctrl_irqs(struct > brcmnand_controller *ctrl) > } > > disable_irq(ctrl->irq); > - > ctrl->pio_poll_mode = true; > } > > -- > 1.9.0.138.g2de3478 >
[PATCH v2 2/2] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
If mtd_oops is in progress switch to polling for nand command completion interrupts and use PIO mode wihtout DMA so that the mtd_oops buffer can be completely written in the assinged nand partition. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index a30a7f0..dca8eb8 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -835,7 +835,6 @@ static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl) } disable_irq(ctrl->irq); - ctrl->pio_poll_mode = true; } -- 1.9.0.138.g2de3478
[PATCH v2 1/2] mtd: Add flag to indicate panic_write
Added a flag to indicate a panic_write so that low level drivers can use it to take required action where applicable, to ensure oops data gets written to assigned mtd device. Signed-off-by: Kamal Dasu --- drivers/mtd/mtdcore.c| 3 ++ drivers/mtd/nand/raw/brcmnand/brcmnand.c | 49 ++-- include/linux/mtd/mtd.h | 6 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 76b4264..a83decd 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1138,6 +1138,9 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, return -EROFS; if (!len) return 0; + if (!mtd->oops_panic_write) + mtd->oops_panic_write = true; + return mtd->_panic_write(mtd, to, len, retlen, buf); } EXPORT_SYMBOL_GPL(mtd_panic_write); diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index ce0b8ff..a30a7f0 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -159,6 +159,7 @@ struct brcmnand_controller { u32 nand_cs_nand_xor; u32 corr_stat_threshold; u32 flash_dma_mode; + boolpio_poll_mode; }; struct brcmnand_cfg { @@ -823,6 +824,21 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl) return ctrl->flash_dma_base; } +static inline void disable_ctrl_irqs(struct brcmnand_controller *ctrl) +{ + if (ctrl->pio_poll_mode) + return; + + if (has_flash_dma(ctrl)) { + ctrl->flash_dma_base = 0; + disable_irq(ctrl->dma_irq); + } + + disable_irq(ctrl->irq); + + ctrl->pio_poll_mode = true; +} + static inline bool flash_dma_buf_ok(const void *buf) { return buf && !is_vmalloc_addr(buf) && @@ -1237,15 +1253,42 @@ static void brcmnand_cmd_ctrl(struct nand_chip *chip, int dat, /* intentionally left blank */ } +static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) +{ + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + struct mtd_info *mtd = nand_to_mtd(chip); + bool err = false; + int sts; + + if (mtd->oops_panic_write) { + /* switch to interrupt polling and PIO mode */ + disable_ctrl_irqs(ctrl); + sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, + NAND_CTRL_RDY, 0); + err = (sts < 0) ? true : false; + } else { + unsigned long timeo = msecs_to_jiffies( + NAND_POLL_STATUS_TIMEOUT_MS); + /* wait for completion interrupt */ + sts = wait_for_completion_timeout(&ctrl->done, timeo); + err = (sts <= 0) ? true : false; + } + + return err; +} + static int brcmnand_waitfunc(struct nand_chip *chip) { struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_controller *ctrl = host->ctrl; - unsigned long timeo = msecs_to_jiffies(100); + bool err = false; dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending); - if (ctrl->cmd_pending && - wait_for_completion_timeout(&ctrl->done, timeo) <= 0) { + if (ctrl->cmd_pending) + err = brcmstb_nand_wait_for_completion(chip); + + if (err) { u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) >> brcmnand_cmd_shift(ctrl); diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 677768b..791c34d 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -330,6 +330,12 @@ struct mtd_info { int (*_get_device) (struct mtd_info *mtd); void (*_put_device) (struct mtd_info *mtd); + /* +* flag indicates a panic write, low level drivers can take appropriate +* action if required to ensure writes go through +*/ + bool oops_panic_write; + struct notifier_block reboot_notifier; /* default mode before reboot */ /* ECC status information */ -- 1.9.0.138.g2de3478
Re: [PATCH] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
On Mon, May 6, 2019 at 12:01 PM Richard Weinberger wrote: > > On Wed, May 1, 2019 at 7:52 PM Kamal Dasu wrote: > > > > If mtd_oops is in progress switch to polling for nand command completion > > interrupts and use PIO mode wihtout DMA so that the mtd_oops buffer can > > be completely written in the assinged nand partition. This is needed in > > cases where the panic does not happen on cpu0 and there is only one online > > CPU and the panic is not on cpu0. > > This optimization is highly specific to your hardware and AFAIK cannot > be applied > in general to brcmnand. > > So the problem you see is that depending on the oops you can no longer use dma > or interrupts in the driver? > > How about adding a new flag to panic_nand_write() which tells the nand > driver that > this is a panic write? > That way you can fall back to pio and polling mode without checking cpu > numbers > and oops_in_progress. > Thanks for your review Richard. Will add flag to let low level controller drivers know that that its a panic_write and make brcmnand code more generic and simply fallback to pio and polling in such a case. Will send a V2 patch with these recommended changes. Thanks Kamal > -- > Thanks, > //richard
[PATCH v2] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
If mtd_oops is in progress, switch to polling during NAND command completion instead of relying on DMA/interrupts so that the mtd_oops buffer can be completely written in the assigned NAND partition. This is needed in cases where and there is only one online CPU and the panic is not on cpu0 as all IRQs are wired to cpu0. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 55 ++-- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 482c6f0..128a806 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -823,6 +823,12 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl) return ctrl->flash_dma_base; } +static inline void disable_flash_dma_xfer(struct brcmnand_controller *ctrl) +{ + if (has_flash_dma(ctrl)) + ctrl->flash_dma_base = 0; +} + static inline bool flash_dma_buf_ok(const void *buf) { return buf && !is_vmalloc_addr(buf) && @@ -1237,15 +1243,58 @@ static void brcmnand_cmd_ctrl(struct nand_chip *chip, int dat, /* intentionally left blank */ } +static bool is_mtd_oops_in_progress(void) +{ + int i = 0; + +#ifdef CONFIG_MTD_OOPS + if (oops_in_progress && smp_processor_id()) { + int cpu = 0; + + for_each_online_cpu(cpu) + ++i; + } +#endif + return (i == 1); +} + +static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) +{ + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + bool err = false; + int sts; + + if (is_mtd_oops_in_progress()) { + /* Switch to interrupt polling and PIO mode */ + disable_flash_dma_xfer(ctrl); + sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY | + NAND_STATUS_READY, + NAND_CTRL_RDY | + NAND_STATUS_READY, 0); + err = (sts < 0) ? true : false; + } else { + unsigned long timeo = msecs_to_jiffies( + NAND_POLL_STATUS_TIMEOUT_MS); + /* wait for completion interrupt */ + sts = wait_for_completion_timeout(&ctrl->done, timeo); + err = (sts <= 0) ? true : false; + } + + return err; +} + static int brcmnand_waitfunc(struct nand_chip *chip) { struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_controller *ctrl = host->ctrl; - unsigned long timeo = msecs_to_jiffies(100); + bool err = false; dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending); - if (ctrl->cmd_pending && - wait_for_completion_timeout(&ctrl->done, timeo) <= 0) { + if (ctrl->cmd_pending) + err = brcmstb_nand_wait_for_completion(chip); + + if (err) { u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) >> brcmnand_cmd_shift(ctrl); -- 1.9.0.138.g2de3478
Re: [PATCH] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
On Thu, May 2, 2019 at 4:25 AM Miquel Raynal wrote: > > Hi Kamal, > > Kamal Dasu wrote on Wed, 1 May 2019 13:46:15 > -0400: > > > If mtd_oops is in progress switch to polling for nand command completion > > s/nand/NAND/ Will change this. > > > interrupts and use PIO mode wihtout DMA so that the mtd_oops buffer can > > be completely written in the assinged nand partition. > > What about: > > "If mtd_oops is in progress, switch to polling during NAND command > completion instead of relying on DMA/interrupts so that the mtd_oops > buffer can be completely written in the assigned NAND partition." > Will make this change as well > > This is needed in > > cases where the panic does not happen on cpu0 and there is only one online > > CPU and the panic is not on cpu0. > > "This is needed in case the panic does not happen on CPU0 and there is > only one online CPU." > > I am not sure to understand the problem or how this can happen (and > be a problem). Have you met such issue already or is this purely > speculative? We have seen this issue and tested it on multi core SoCs. > > > > > Signed-off-by: Kamal Dasu > > --- > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 55 > > ++-- > > 1 file changed, 52 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > index 482c6f0..cfbe51a 100644 > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > @@ -823,6 +823,12 @@ static inline bool has_flash_dma(struct > > brcmnand_controller *ctrl) > > return ctrl->flash_dma_base; > > } > > > > +static inline void disable_flash_dma_xfer(struct brcmnand_controller *ctrl) > > +{ > > + if (has_flash_dma(ctrl)) > > + ctrl->flash_dma_base = 0; > > +} > > + > > static inline bool flash_dma_buf_ok(const void *buf) > > { > > return buf && !is_vmalloc_addr(buf) && > > @@ -1237,15 +1243,58 @@ static void brcmnand_cmd_ctrl(struct nand_chip > > *chip, int dat, > > /* intentionally left blank */ > > } > > > > +static bool is_mtd_oops_in_progress(void) > > +{ > > + int i = 0; > > + > > +#ifdef CONFIG_MTD_OOPS > > + if (oops_in_progress && smp_processor_id()) { > > + int cpu = 0; > > + > > + for_each_online_cpu(cpu) > > + ++i; > > + } > > +#endif > > + return i == 1 ? true : false; > > I suppose return (i == 1); is enough > Ok will make the change. > > +} > > + > > +static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) > > +{ > > + struct brcmnand_host *host = nand_get_controller_data(chip); > > + struct brcmnand_controller *ctrl = host->ctrl; > > + bool err = false; > > + int sts; > > + > > + if (is_mtd_oops_in_progress()) { > > + /* Switch to interrupt polling and PIO mode */ > > + disable_flash_dma_xfer(ctrl); > > + sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY | > > +NAND_STATUS_READY, > > +NAND_CTRL_RDY | > > +NAND_STATUS_READY, 0); > > + err = (sts < 0) ? true : false; > > + } else { > > + unsigned long timeo = msecs_to_jiffies( > > + NAND_POLL_STATUS_TIMEOUT_MS); > > + /* wait for completion interrupt */ > > + sts = wait_for_completion_timeout(&ctrl->done, timeo); > > + err = (sts <= 0) ? true : false; > > + } > > + > > + return err; > > +} > > + > > static int brcmnand_waitfunc(struct nand_chip *chip) > > { > > struct brcmnand_host *host = nand_get_controller_data(chip); > > struct brcmnand_controller *ctrl = host->ctrl; > > - unsigned long timeo = msecs_to_jiffies(100); > > + bool err = false; > > > > dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending); > > - if (ctrl->cmd_pending && > > - wait_for_completion_timeout(&ctrl->done, timeo) <= 0) > > { > > What about the wait_for_completion_timeout() call in brcmnand_write()? > brcmnand_write() too calls brcmnand_waitfunc(), will poll if it needs to for completion. > > + if (ctrl->cmd_pending) > > + err = brcmstb_nand_wait_for_completion(chip); > > + > > + if (err) { > > u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) > > >> brcmnand_cmd_shift(ctrl); > > > > > Thanks, > Miquèl Thanks Kamal
[PATCH v2] mtd: rawnand: brcmnand: fix bch ecc layout for large page nand
The oobregion->offset for large page nand parts was wrong, change fixes this error in calculation. Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops") Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 482c6f0..3eefea7 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -939,7 +939,7 @@ static int brcmnand_bch_ooblayout_ecc(struct mtd_info *mtd, int section, if (section >= sectors) return -ERANGE; - oobregion->offset = (section * (sas + 1)) - chip->ecc.bytes; + oobregion->offset = ((section + 1) * sas) - chip->ecc.bytes; oobregion->length = chip->ecc.bytes; return 0; -- 1.9.0.138.g2de3478
Re: [PATCH] mtd: rawnand: brcmnand: fix bch ecc layout for large page nand
On Wed, May 1, 2019 at 2:35 PM Florian Fainelli wrote: > > On 4/26/19 12:22 PM, Kamal Dasu wrote: > > The oobregion->offset for large page nand parts was wrong, change > > fixes this error in calculation. > > Should this have a Fixes tag so this can be backported to stable trees > seemingly automatically? Thanks! > The brcmnand files got moved, however should have the fixes tag, will send V2 patch. > > > > Signed-off-by: Kamal Dasu > > --- > > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > index 482c6f0..3eefea7 100644 > > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > > @@ -939,7 +939,7 @@ static int brcmnand_bch_ooblayout_ecc(struct mtd_info > > *mtd, int section, > > if (section >= sectors) > > return -ERANGE; > > > > - oobregion->offset = (section * (sas + 1)) - chip->ecc.bytes; > > + oobregion->offset = ((section + 1) * sas) - chip->ecc.bytes; > > oobregion->length = chip->ecc.bytes; > > > > return 0; > > > > > -- > Florian
[PATCH] mtd: nand: raw: brcmnand: When oops in progress use pio and interrupt polling
If mtd_oops is in progress switch to polling for nand command completion interrupts and use PIO mode wihtout DMA so that the mtd_oops buffer can be completely written in the assinged nand partition. This is needed in cases where the panic does not happen on cpu0 and there is only one online CPU and the panic is not on cpu0. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 55 ++-- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 482c6f0..cfbe51a 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -823,6 +823,12 @@ static inline bool has_flash_dma(struct brcmnand_controller *ctrl) return ctrl->flash_dma_base; } +static inline void disable_flash_dma_xfer(struct brcmnand_controller *ctrl) +{ + if (has_flash_dma(ctrl)) + ctrl->flash_dma_base = 0; +} + static inline bool flash_dma_buf_ok(const void *buf) { return buf && !is_vmalloc_addr(buf) && @@ -1237,15 +1243,58 @@ static void brcmnand_cmd_ctrl(struct nand_chip *chip, int dat, /* intentionally left blank */ } +static bool is_mtd_oops_in_progress(void) +{ + int i = 0; + +#ifdef CONFIG_MTD_OOPS + if (oops_in_progress && smp_processor_id()) { + int cpu = 0; + + for_each_online_cpu(cpu) + ++i; + } +#endif + return i == 1 ? true : false; +} + +static bool brcmstb_nand_wait_for_completion(struct nand_chip *chip) +{ + struct brcmnand_host *host = nand_get_controller_data(chip); + struct brcmnand_controller *ctrl = host->ctrl; + bool err = false; + int sts; + + if (is_mtd_oops_in_progress()) { + /* Switch to interrupt polling and PIO mode */ + disable_flash_dma_xfer(ctrl); + sts = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY | + NAND_STATUS_READY, + NAND_CTRL_RDY | + NAND_STATUS_READY, 0); + err = (sts < 0) ? true : false; + } else { + unsigned long timeo = msecs_to_jiffies( + NAND_POLL_STATUS_TIMEOUT_MS); + /* wait for completion interrupt */ + sts = wait_for_completion_timeout(&ctrl->done, timeo); + err = (sts <= 0) ? true : false; + } + + return err; +} + static int brcmnand_waitfunc(struct nand_chip *chip) { struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_controller *ctrl = host->ctrl; - unsigned long timeo = msecs_to_jiffies(100); + bool err = false; dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending); - if (ctrl->cmd_pending && - wait_for_completion_timeout(&ctrl->done, timeo) <= 0) { + if (ctrl->cmd_pending) + err = brcmstb_nand_wait_for_completion(chip); + + if (err) { u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START) >> brcmnand_cmd_shift(ctrl); -- 1.9.0.138.g2de3478
[PATCH] mtd: rawnand: brcmnand: fix bch ecc layout for large page nand
The oobregion->offset for large page nand parts was wrong, change fixes this error in calculation. Signed-off-by: Kamal Dasu --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 482c6f0..3eefea7 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -939,7 +939,7 @@ static int brcmnand_bch_ooblayout_ecc(struct mtd_info *mtd, int section, if (section >= sectors) return -ERANGE; - oobregion->offset = (section * (sas + 1)) - chip->ecc.bytes; + oobregion->offset = ((section + 1) * sas) - chip->ecc.bytes; oobregion->length = chip->ecc.bytes; return 0; -- 1.9.0.138.g2de3478
Re: [PATCH] spi: bcm-qspi: when tx/rx buffer is NULL set to 0
On Thu, Feb 7, 2019 at 3:47 PM wrote: > > From: Justin Chen > > Currently we set the tx/rx buffer to 0xff when NULL. This causes > problems with some spi slaves where 0xff is a valid command. Looking > at other drivers, the tx/rx buffer is usually set to 0x00 when NULL. > Following this convention solves the issue. > > Fixes: fa236a7ef240 ("spi: bcm-qspi: Add Broadcom MSPI driver") > Signed-off-by: Justin Chen Signed-off-by: Kamal Dasu > --- > drivers/spi/spi-bcm-qspi.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c > index 584bcb0..79456aa 100644 > --- a/drivers/spi/spi-bcm-qspi.c > +++ b/drivers/spi/spi-bcm-qspi.c > @@ -681,7 +681,7 @@ static void read_from_hw(struct bcm_qspi *qspi, int slots) > if (buf) > buf[tp.byte] = read_rxram_slot_u8(qspi, slot); > dev_dbg(&qspi->pdev->dev, "RD %02x\n", > - buf ? buf[tp.byte] : 0xff); > + buf ? buf[tp.byte] : 0x00); > } else { > u16 *buf = tp.trans->rx_buf; > > @@ -689,7 +689,7 @@ static void read_from_hw(struct bcm_qspi *qspi, int slots) > buf[tp.byte / 2] = read_rxram_slot_u16(qspi, > slot); > dev_dbg(&qspi->pdev->dev, "RD %04x\n", > - buf ? buf[tp.byte] : 0x); > + buf ? buf[tp.byte] : 0x); > } > > update_qspi_trans_byte_count(qspi, &tp, > @@ -744,13 +744,13 @@ static int write_to_hw(struct bcm_qspi *qspi, struct > spi_device *spi) > while (!tstatus && slot < MSPI_NUM_CDRAM) { > if (tp.trans->bits_per_word <= 8) { > const u8 *buf = tp.trans->tx_buf; > - u8 val = buf ? buf[tp.byte] : 0xff; > + u8 val = buf ? buf[tp.byte] : 0x00; > > write_txram_slot_u8(qspi, slot, val); > dev_dbg(&qspi->pdev->dev, "WR %02x\n", val); > } else { > const u16 *buf = tp.trans->tx_buf; > - u16 val = buf ? buf[tp.byte / 2] : 0x; > + u16 val = buf ? buf[tp.byte / 2] : 0x; > > write_txram_slot_u16(qspi, slot, val); > dev_dbg(&qspi->pdev->dev, "WR %04x\n", val); > -- > 2.7.4 >
Re: [RFC/RFT 04/10] i2c: brcmstb: use core helper to mark adapter suspended
On Mon, Dec 10, 2018 at 4:03 PM Wolfram Sang wrote: > > Rejecting transfers should be handled by the core. > > Signed-off-by: Wolfram Sang Reviewed-by: Kamal Dasu > --- > drivers/i2c/busses/i2c-brcmstb.c | 13 ++--- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-brcmstb.c > b/drivers/i2c/busses/i2c-brcmstb.c > index 826d32049996..05c822e5a3f6 100644 > --- a/drivers/i2c/busses/i2c-brcmstb.c > +++ b/drivers/i2c/busses/i2c-brcmstb.c > @@ -170,7 +170,6 @@ struct brcmstb_i2c_dev { > struct bsc_regs *bsc_regmap; > struct i2c_adapter adapter; > struct completion done; > - bool is_suspended; > u32 clk_freq_hz; > int data_regsz; > }; > @@ -467,9 +466,6 @@ static int brcmstb_i2c_xfer(struct i2c_adapter *adapter, > int xfersz = brcmstb_i2c_get_xfersz(dev); > u32 cond, cond_per_msg; > > - if (dev->is_suspended) > - return -EBUSY; > - > /* Loop through all messages */ > for (i = 0; i < num; i++) { > pmsg = &msgs[i]; > @@ -689,10 +685,7 @@ static int brcmstb_i2c_suspend(struct device *dev) > { > struct brcmstb_i2c_dev *i2c_dev = dev_get_drvdata(dev); > > - i2c_lock_bus(&i2c_dev->adapter, I2C_LOCK_ROOT_ADAPTER); > - i2c_dev->is_suspended = true; > - i2c_unlock_bus(&i2c_dev->adapter, I2C_LOCK_ROOT_ADAPTER); > - > + i2c_mark_adapter_suspended(&i2c_dev->adapter, true); > return 0; > } > > @@ -700,10 +693,8 @@ static int brcmstb_i2c_resume(struct device *dev) > { > struct brcmstb_i2c_dev *i2c_dev = dev_get_drvdata(dev); > > - i2c_lock_bus(&i2c_dev->adapter, I2C_LOCK_ROOT_ADAPTER); > brcmstb_i2c_set_bsc_reg_defaults(i2c_dev); > - i2c_dev->is_suspended = false; > - i2c_unlock_bus(&i2c_dev->adapter, I2C_LOCK_ROOT_ADAPTER); > + i2c_mark_adapter_suspended(&i2c_dev->adapter, false); > > return 0; > } > -- > 2.11.0 >
[PATCH v2 1/2] spi: bcm-qspi: Avoid setting MSPI_CDRAM_PCS for spi-nor master
Added fix for probing of spi-nor device non-zero chip selects. Set MSPI_CDRAM_PCS (peripheral chip select) with spi master for MSPI controller and not for MSPI/BSPI spi-nor master controller. Ensure setting of cs bit in chip select register on chip select change. Fixes: fa236a7ef24048 ("spi: bcm-qspi: Add Broadcom MSPI driver") Signed-off-by: Kamal Dasu --- drivers/spi/spi-bcm-qspi.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 1596d35..2946989 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -519,16 +519,19 @@ static void bcm_qspi_disable_bspi(struct bcm_qspi *qspi) static void bcm_qspi_chip_select(struct bcm_qspi *qspi, int cs) { - u32 data = 0; + u32 rd = 0; + u32 wr = 0; - if (qspi->curr_cs == cs) - return; if (qspi->base[CHIP_SELECT]) { - data = bcm_qspi_read(qspi, CHIP_SELECT, 0); - data = (data & ~0xff) | (1 << cs); - bcm_qspi_write(qspi, CHIP_SELECT, 0, data); + rd = bcm_qspi_read(qspi, CHIP_SELECT, 0); + wr = (rd & ~0xff) | (1 << cs); + if (rd == wr) + return; + bcm_qspi_write(qspi, CHIP_SELECT, 0, wr); usleep_range(10, 20); } + + dev_dbg(&qspi->pdev->dev, "using cs:%d\n", cs); qspi->curr_cs = cs; } @@ -755,8 +758,13 @@ static int write_to_hw(struct bcm_qspi *qspi, struct spi_device *spi) dev_dbg(&qspi->pdev->dev, "WR %04x\n", val); } mspi_cdram = MSPI_CDRAM_CONT_BIT; - mspi_cdram |= (~(1 << spi->chip_select) & - MSPI_CDRAM_PCS); + + if (has_bspi(qspi)) + mspi_cdram &= ~1; + else + mspi_cdram |= (~(1 << spi->chip_select) & + MSPI_CDRAM_PCS); + mspi_cdram |= ((tp.trans->bits_per_word <= 8) ? 0 : MSPI_CDRAM_BITSE_BIT); -- 2.7.4
[PATCH v2 0/2] spi: bcm-qspi: Fix spi-nor chip select logic for BSPI
When using the spi-nor master controller always verify the chip select bit in the cs register. Also do not use CDRAM PCS bit in BSPI mode. Additionally make sure to enable/disable BSPI_MAST_N_BOOT_CTRL while using BSPI mode. V2 changes: - Added "Fixes:" tag to 1/2 and 2/2 patches Kamal Dasu (2): spi: bcm-qspi: Avoid setting MSPI_CDRAM_PCS for spi-nor master spi: bcm-qspi: Always read and set BSPI_MAST_N_BOOT_CTRL drivers/spi/spi-bcm-qspi.c | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) -- 2.7.4
[PATCH v2 2/2] spi: bcm-qspi: Always read and set BSPI_MAST_N_BOOT_CTRL
Always confirm the BSPI_MAST_N_BOOT_CTRL bit when enabling or disabling BSPI transfers. Fixes: 4e3b2d236fe00 ("spi: bcm-qspi: Add BSPI spi-nor flash controller driver") Signed-off-by: Kamal Dasu --- drivers/spi/spi-bcm-qspi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 2946989..6573152 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -490,7 +490,7 @@ static int bcm_qspi_bspi_set_mode(struct bcm_qspi *qspi, static void bcm_qspi_enable_bspi(struct bcm_qspi *qspi) { - if (!has_bspi(qspi) || (qspi->bspi_enabled)) + if (!has_bspi(qspi)) return; qspi->bspi_enabled = 1; @@ -505,7 +505,7 @@ static void bcm_qspi_enable_bspi(struct bcm_qspi *qspi) static void bcm_qspi_disable_bspi(struct bcm_qspi *qspi) { - if (!has_bspi(qspi) || (!qspi->bspi_enabled)) + if (!has_bspi(qspi)) return; qspi->bspi_enabled = 0; -- 2.7.4
[PATCH v1 1/2] spi: bcm-qspi: Avoid setting MSPI_CDRAM_PCS for spi-nor master
Added fix for probing of spi-nor device non-zero chip selects. Set MSPI_CDRAM_PCS (peripheral chip select) with spi master for MSPI controller and not for MSPI/BSPI spi-nor master controller. Ensure setting of cs bit in chip select register on chip select change. Signed-off-by: Kamal Dasu --- drivers/spi/spi-bcm-qspi.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 1596d35..2946989 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -519,16 +519,19 @@ static void bcm_qspi_disable_bspi(struct bcm_qspi *qspi) static void bcm_qspi_chip_select(struct bcm_qspi *qspi, int cs) { - u32 data = 0; + u32 rd = 0; + u32 wr = 0; - if (qspi->curr_cs == cs) - return; if (qspi->base[CHIP_SELECT]) { - data = bcm_qspi_read(qspi, CHIP_SELECT, 0); - data = (data & ~0xff) | (1 << cs); - bcm_qspi_write(qspi, CHIP_SELECT, 0, data); + rd = bcm_qspi_read(qspi, CHIP_SELECT, 0); + wr = (rd & ~0xff) | (1 << cs); + if (rd == wr) + return; + bcm_qspi_write(qspi, CHIP_SELECT, 0, wr); usleep_range(10, 20); } + + dev_dbg(&qspi->pdev->dev, "using cs:%d\n", cs); qspi->curr_cs = cs; } @@ -755,8 +758,13 @@ static int write_to_hw(struct bcm_qspi *qspi, struct spi_device *spi) dev_dbg(&qspi->pdev->dev, "WR %04x\n", val); } mspi_cdram = MSPI_CDRAM_CONT_BIT; - mspi_cdram |= (~(1 << spi->chip_select) & - MSPI_CDRAM_PCS); + + if (has_bspi(qspi)) + mspi_cdram &= ~1; + else + mspi_cdram |= (~(1 << spi->chip_select) & + MSPI_CDRAM_PCS); + mspi_cdram |= ((tp.trans->bits_per_word <= 8) ? 0 : MSPI_CDRAM_BITSE_BIT); -- 2.7.4
[PATCH v1 0/2] spi: bcm-qspi: Fix spi-nor chip select logic for BSPI
spi-nor master controller driver needs to verify the chip select bit in the cs register for any cs change during trasfers. Also CDRAM PCS field not used in BSPI mode, set defualt to ~1 indicating active low for zeroth bit. Also always enable/disable BSPI_MAST_N_BOOT_CTRL when using BSPI mode. *** BLURB HERE *** Kamal Dasu (2): spi: bcm-qspi: Avoid setting MSPI_CDRAM_PCS for spi-nor master spi: bcm-qspi: Always change BSPI_MAST_N_BOOT_CTRL state drivers/spi/spi-bcm-qspi.c | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) -- 2.7.4
[PATCH v1 2/2] spi: bcm-qspi: Always change BSPI_MAST_N_BOOT_CTRL state
Always confirm the BSPI_MAST_N_BOOT_CTRL bit when enabling or disabling BSPI transfers. Signed-off-by: Kamal Dasu --- drivers/spi/spi-bcm-qspi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 2946989..6573152 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -490,7 +490,7 @@ static int bcm_qspi_bspi_set_mode(struct bcm_qspi *qspi, static void bcm_qspi_enable_bspi(struct bcm_qspi *qspi) { - if (!has_bspi(qspi) || (qspi->bspi_enabled)) + if (!has_bspi(qspi)) return; qspi->bspi_enabled = 1; @@ -505,7 +505,7 @@ static void bcm_qspi_enable_bspi(struct bcm_qspi *qspi) static void bcm_qspi_disable_bspi(struct bcm_qspi *qspi) { - if (!has_bspi(qspi) || (!qspi->bspi_enabled)) + if (!has_bspi(qspi)) return; qspi->bspi_enabled = 0; -- 2.7.4
[PATCH v8 2/2] mtd: spi-nor: Add spi-nor mtd resume handler
Implemented and populated spi-nor mtd PM handlers for resume ops. spi-nor resume op re-initializes spi-nor flash to its probed state by calling the newly implemented spi_nor_init() function. Signed-off-by: Kamal Dasu --- drivers/mtd/spi-nor/spi-nor.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index ab98ecff..69164bc 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1871,6 +1871,19 @@ static int spi_nor_init(struct spi_nor *nor) return 0; } +/* mtd resume handler */ +static void spi_nor_resume(struct mtd_info *mtd) +{ + struct spi_nor *nor = mtd_to_spi_nor(mtd); + struct device *dev = nor->dev; + int ret; + + /* re-initialize the nor chip */ + ret = spi_nor_init(nor); + if (ret) + dev_err(dev, "resume() failed\n"); +} + int spi_nor_scan(struct spi_nor *nor, const char *name, const struct spi_nor_hwcaps *hwcaps) { @@ -1947,6 +1960,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, mtd->size = params.size; mtd->_erase = spi_nor_erase; mtd->_read = spi_nor_read; + mtd->_resume = spi_nor_resume; /* NOR protection support for STmicro/Micron chips and similar */ if (JEDEC_MFR(info) == SNOR_MFR_MICRON || -- 1.9.1
[PATCH v8 0/2] Add spi-nor flash device pm support
Changes since v7 spi-nor.h - Fix comment style spi-nor.c - Fix condition check to set_4byte() when nor->addr_width == 4 in spi_nor_init() - Bring back quad_enable logic to spi_nor_setup() and remove it from spi_nor_scan() - Remove duplicate comment section in spi_nor_scan() - Leave dev_info() string as before in spi_nor_scan() The V8 changes below implements power management support using the mtd handlers for resume in the spi-nor driver. spi-nor mtd pm resume() calls newly implemented spi_nor_init() function that sets up the spi-nor flash to its pre-suspend/power-on probed state. This is needed S2/S3 PM on platfroms that turn off power to the spi-nor flash on pm suspend. Kamal Dasu (2): mtd: spi-nor: add spi_nor_init() function mtd: spi-nor: Add spi-nor mtd resume handler drivers/mtd/spi-nor/spi-nor.c | 68 +++ include/linux/mtd/spi-nor.h | 10 +++ 2 files changed, 60 insertions(+), 18 deletions(-) -- 1.9.1
[PATCH v8 1/2] mtd: spi-nor: add spi_nor_init() function
This patch extracts some chunks from spi_nor_init_params and spi_nor_scan() and moves them into a new spi_nor_init() function. Indeed, spi_nor_init() regroups all the required SPI flash commands to be sent to the SPI flash memory before performing any runtime operations (Fast Read, Page Program, Sector Erase, ...). Hence spi_nor_init(): 1) removes the flash protection if applicable for certain vendors. 2) sets the Quad Enable bit, if needed, before using Quad SPI protocols. 3) makes the memory enter its (stateful) 4-byte address mode, if needed, for SPI flash memory > 128Mbits not supporting the 4-byte address instruction set. spi_nor_scan() now ends by calling spi_nor_init() once the probe phase has completed. Further patches could also use spi_nor_init() to implement the mtd->_resume() handler for the spi-nor framework. Signed-off-by: Kamal Dasu --- drivers/mtd/spi-nor/spi-nor.c | 54 --- include/linux/mtd/spi-nor.h | 10 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 1413828..ab98ecff 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1832,14 +1832,42 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, /* Enable Quad I/O if needed. */ enable_quad_io = (spi_nor_get_protocol_width(nor->read_proto) == 4 || spi_nor_get_protocol_width(nor->write_proto) == 4); - if (enable_quad_io && params->quad_enable) { - err = params->quad_enable(nor); + if (enable_quad_io && params->quad_enable) + nor->quad_enable = params->quad_enable; + + return 0; +} + +static int spi_nor_init(struct spi_nor *nor) +{ + int err; + + /* +* Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up +* with the software protection bits set +*/ + if (JEDEC_MFR(nor->info) == SNOR_MFR_ATMEL || + JEDEC_MFR(nor->info) == SNOR_MFR_INTEL || + JEDEC_MFR(nor->info) == SNOR_MFR_SST || + nor->info->flags & SPI_NOR_HAS_LOCK) { + write_enable(nor); + write_sr(nor, 0); + spi_nor_wait_till_ready(nor); + } + + if (nor->quad_enable) { + err = nor->quad_enable(nor); if (err) { dev_err(nor->dev, "quad mode not supported\n"); return err; } } + if ((nor->addr_width == 4) && + (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) && + !(nor->info->flags & SPI_NOR_4B_OPCODES)) + set_4byte(nor, nor->info, 1); + return 0; } @@ -1910,20 +1938,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, if (ret) return ret; - /* -* Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up -* with the software protection bits set -*/ - - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL || - JEDEC_MFR(info) == SNOR_MFR_INTEL || - JEDEC_MFR(info) == SNOR_MFR_SST || - info->flags & SPI_NOR_HAS_LOCK) { - write_enable(nor); - write_sr(nor, 0); - spi_nor_wait_till_ready(nor); - } - if (!mtd->name) mtd->name = dev_name(dev); mtd->priv = nor; @@ -2002,8 +2016,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, if (JEDEC_MFR(info) == SNOR_MFR_SPANSION || info->flags & SPI_NOR_4B_OPCODES) spi_nor_set_4byte_opcodes(nor, info); - else - set_4byte(nor, info, 1); } else { nor->addr_width = 3; } @@ -2020,6 +2032,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, return ret; } + /* Send all the required SPI flash commands to initialize device */ + nor->info = info; + ret = spi_nor_init(nor); + if (ret) + return ret; + dev_info(dev, "%s (%lld Kbytes)\n", info->name, (long long)mtd->size >> 10); diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 55faa2f..247f8fb 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -221,10 +221,17 @@ enum spi_nor_option_flags { }; /** + * struct flash_info - Forward declaration of a structure used internally by + *spi_nor_scan() + */ +struct flash_info; + +/** * struct spi_nor - Structure for defining a the SPI NOR layer * @mtd: point to a mtd_info structure * @lock: the lock for the read/write/eras
[PATCH v7 1/2] mtd: spi-nor: add spi_nor_init() function
This patch extracts some chunks from spi_nor_init_params and spi_nor_scan() and moves them into a new spi_nor_init() function. Indeed, spi_nor_init() regroups all the required SPI flash commands to be sent to the SPI flash memory before performing any runtime operations (Fast Read, Page Program, Sector Erase, ...). Hence spi_nor_init(): 1) removes the flash protection if applicable for certain vendors. 2) sets the Quad Enable bit, if needed, before using Quad SPI protocols. 3) makes the memory enter its (stateful) 4-byte address mode, if needed, for SPI flash memory > 128Mbits not supporting the 4-byte address instruction set. spi_nor_scan() now ends by calling spi_nor_init() once the probe phase has completed. Further patches could also use spi_nor_init() to implement the mtd->_resume() handler for the spi-nor framework. Signed-off-by: Kamal Dasu --- drivers/mtd/spi-nor/spi-nor.c | 62 ++- include/linux/mtd/spi-nor.h | 9 +++ 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 1413828..10033ed 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1784,7 +1784,6 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, const struct spi_nor_hwcaps *hwcaps) { u32 ignored_mask, shared_mask; - bool enable_quad_io; int err; /* @@ -1829,20 +1828,42 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, return err; } - /* Enable Quad I/O if needed. */ - enable_quad_io = (spi_nor_get_protocol_width(nor->read_proto) == 4 || - spi_nor_get_protocol_width(nor->write_proto) == 4); - if (enable_quad_io && params->quad_enable) { - err = params->quad_enable(nor); + return 0; +} + +static int spi_nor_init(struct spi_nor *nor) +{ + int err; + + /* +* Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up +* with the software protection bits set +*/ + if (JEDEC_MFR(nor->info) == SNOR_MFR_ATMEL || + JEDEC_MFR(nor->info) == SNOR_MFR_INTEL || + JEDEC_MFR(nor->info) == SNOR_MFR_SST || + nor->info->flags & SPI_NOR_HAS_LOCK) { + write_enable(nor); + write_sr(nor, 0); + spi_nor_wait_till_ready(nor); + } + + if (nor->quad_enable) { + err = nor->quad_enable(nor); if (err) { dev_err(nor->dev, "quad mode not supported\n"); return err; } } + if ((JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) && + !(nor->info->flags & SPI_NOR_4B_OPCODES)) + set_4byte(nor, nor->info, 1); + return 0; } + int spi_nor_scan(struct spi_nor *nor, const char *name, const struct spi_nor_hwcaps *hwcaps) { @@ -1853,6 +1874,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, struct device_node *np = spi_nor_get_flash_node(nor); int ret; int i; + bool enable_quad_io; ret = spi_nor_check(nor); if (ret) @@ -1915,15 +1937,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, * with the software protection bits set */ - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL || - JEDEC_MFR(info) == SNOR_MFR_INTEL || - JEDEC_MFR(info) == SNOR_MFR_SST || - info->flags & SPI_NOR_HAS_LOCK) { - write_enable(nor); - write_sr(nor, 0); - spi_nor_wait_till_ready(nor); - } - if (!mtd->name) mtd->name = dev_name(dev); mtd->priv = nor; @@ -2002,8 +2015,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, if (JEDEC_MFR(info) == SNOR_MFR_SPANSION || info->flags & SPI_NOR_4B_OPCODES) spi_nor_set_4byte_opcodes(nor, info); - else - set_4byte(nor, info, 1); } else { nor->addr_width = 3; } @@ -2020,8 +2031,21 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, return ret; } - dev_info(dev, "%s (%lld Kbytes)\n", info->name, - (long long)mtd->size >> 10); + /* Send all the required SPI flash commands to initialize device */ + nor->info = info; + /* Enable Quad I/O if needed. */ + enable_quad_io = (spi_nor_get_protocol_width(nor->read_proto) == 4 || + spi_nor_get_protocol_width(nor->write_proto) == 4); + if (enable_quad_io && params.quad_enable) +
[PATCH v7 2/2] mtd: spi-nor: Add spi-nor mtd resume handler
Implemented and populated spi-nor mtd PM handlers for resume ops. spi-nor resume op re-initializes spi-nor flash to its probed state by calling the newly implemented spi_nor_init() function. Signed-off-by: Kamal Dasu --- drivers/mtd/spi-nor/spi-nor.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 10033ed..64c131d 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1863,6 +1863,18 @@ static int spi_nor_init(struct spi_nor *nor) return 0; } +/* mtd resume handler */ +static void spi_nor_resume(struct mtd_info *mtd) +{ + struct spi_nor *nor = mtd_to_spi_nor(mtd); + struct device *dev = nor->dev; + int ret; + + /* re-initialize the nor chip */ + ret = spi_nor_init(nor); + if (ret) + dev_err(dev, "resume() failed\n"); +} int spi_nor_scan(struct spi_nor *nor, const char *name, const struct spi_nor_hwcaps *hwcaps) @@ -1946,6 +1958,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, mtd->size = params.size; mtd->_erase = spi_nor_erase; mtd->_read = spi_nor_read; + mtd->_resume = spi_nor_resume; /* NOR protection support for STmicro/Micron chips and similar */ if (JEDEC_MFR(info) == SNOR_MFR_MICRON || -- 1.9.1
[PATCH v7 0/2] Add spi-nor flash device pm support
Changes since v6 spi-nor.h - Reverted all v6 changes - Added info pointer to spi-nor structure - Added quad_enable function pointer spi-nor.c - Reverted all v6 changes - Refactored spi_nor_init() function - Added mtd resume handlers The V7 changes below implements power management support using the mtd handlers for resume in the spi-nor driver. spi-nor mtd pm resume() calls newly implemented spi_nor_init() function that sets up the spi-nor flash to its pre-suspend/power-on probed state. This is needed S2/S3 PM on platfroms that turn off power to the spi-nor flash on pm suspend. Kamal Dasu (2): mtd: spi-nor: add spi_nor_init() function mtd: spi-nor: Add spi-nor mtd resume handler drivers/mtd/spi-nor/spi-nor.c | 75 --- include/linux/mtd/spi-nor.h | 9 ++ 2 files changed, 65 insertions(+), 19 deletions(-) -- 1.9.1
[PATCH] spi: bcm-qspi: Remove hardcoded settings and spi-nor.h dependency
The newly added broadcom qspi driver in drivers/spi produces a build warning when CONFIG_MTD is disabled: include/linux/mtd/cfi.h:76:2: #warning No CONFIG_MTD_CFI_Ix selected. No NOR chip support can work. [-Werror=cpp] There has been discussion on this in the link provided below. This fix in SPI controller drivers implementing the ->spi_flash_read handler, now uses the settings provided inside the 'struct spi_flash_read_message' parameter instead of hardcoding them. Made changes to bcm_qspi_bspi_set_flex_mode() to set the BSPI controller using the passed msg structure and remove the need to include file by removing all use of SPINOR_OP_READ* macros. Fixes: 4e3b2d236fe0 ("spi: bcm-qspi: Add BSPI spi-nor flash controller driver") Link: https://patchwork.kernel.org/patch/9624585/ Signed-off-by: Kamal Dasu --- drivers/spi/spi-bcm-qspi.c | 89 +- 1 file changed, 33 insertions(+), 56 deletions(-) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index b19722b..6ef6c44 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include @@ -349,76 +348,60 @@ static void bcm_qspi_bspi_set_xfer_params(struct bcm_qspi *qspi, u8 cmd_byte, bcm_qspi_write(qspi, BSPI, BSPI_FLEX_MODE_ENABLE, flex_mode); } -static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi *qspi, int width, - int addrlen, int hp) +static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi *qspi, + struct spi_flash_read_message *msg, + int hp) { int bpc = 0, bpp = 0; - u8 command = SPINOR_OP_READ_FAST; - int flex_mode = 1, rv = 0; - bool spans_4byte = false; + u8 command = msg->read_opcode; + int width = msg->data_nbits ? msg->data_nbits : SPI_NBITS_SINGLE; + int addrlen = msg->addr_width; + int addr_nbits = msg->addr_nbits ? msg->addr_nbits : SPI_NBITS_SINGLE; + int flex_mode = 1; dev_dbg(&qspi->pdev->dev, "set flex mode w %x addrlen %x hp %d\n", width, addrlen, hp); - if (addrlen == BSPI_ADDRLEN_4BYTES) { + if (addrlen == BSPI_ADDRLEN_4BYTES) bpp = BSPI_BPP_ADDR_SELECT_MASK; - spans_4byte = true; - } - bpp |= 8; + bpp |= msg->dummy_bytes * (8/addr_nbits); switch (width) { case SPI_NBITS_SINGLE: if (addrlen == BSPI_ADDRLEN_3BYTES) /* default mode, does not need flex_cmd */ flex_mode = 0; - else - command = SPINOR_OP_READ_FAST_4B; break; case SPI_NBITS_DUAL: bpc = 0x0001; if (hp) { bpc |= 0x00010100; /* address and mode are 2-bit */ bpp = BSPI_BPP_MODE_SELECT_MASK; - command = OPCODE_DIOR; - if (spans_4byte) - command = OPCODE_DIOR_4B; - } else { - command = SPINOR_OP_READ_1_1_2; - if (spans_4byte) - command = SPINOR_OP_READ_1_1_2_4B; } break; case SPI_NBITS_QUAD: bpc = 0x0002; if (hp) { bpc |= 0x00020200; /* address and mode are 4-bit */ - bpp = 4; /* dummy cycles */ - bpp |= BSPI_BPP_ADDR_SELECT_MASK; - command = OPCODE_QIOR; - if (spans_4byte) - command = OPCODE_QIOR_4B; - } else { - command = SPINOR_OP_READ_1_1_4; - if (spans_4byte) - command = SPINOR_OP_READ_1_1_4_4B; + bpp |= BSPI_BPP_MODE_SELECT_MASK; } break; default: - rv = -EINVAL; - break; + return -EINVAL; } - if (rv == 0) - bcm_qspi_bspi_set_xfer_params(qspi, command, bpp, bpc, - flex_mode); + bcm_qspi_bspi_set_xfer_params(qspi, command, bpp, bpc, flex_mode); - return rv; + return 0; } -static int bcm_qspi_bspi_set_override(struct bcm_qspi *qspi, int width, - int addrlen, int hp) +static int bcm_qspi_bspi_set_override(struct bcm_qspi *qspi, + struct spi_flash_read_message *msg, + int hp) { + int width = msg->data_nbits ? msg->data_nbits : SPI_NBITS_SINGLE; + int addrlen = msg->addr_wid
Re: [PATCH] spi: document broadcom qspi driver as broken
On Wed, Jul 26, 2017 at 3:03 AM, Arnd Bergmann wrote: > On Tue, Jul 25, 2017 at 11:39 PM, Kamal Dasu wrote: >> Arnd, Cyrille, >> >> I am working on fixing spi-bcm-qspi.c as per Cyrill's suggestion as >> mentioned here : https://patchwork.kernel.org/patch/9624585/. >> And remove the use of SPINOR_OP_READ* and there by remove need to >> include spi-nor.h. > > Ok, thanks a lot! > > Do you think your fix will be applicable to stable backports, or should we > add a simple dependency first for the stable kernels and remove that > dependency again with the proper fix? > The fix would be applicable to stable backport, we don't need to add a dependency. >Arnd Kamal
Re: [PATCH] spi: document broadcom qspi driver as broken
Arnd, Cyrille, I am working on fixing spi-bcm-qspi.c as per Cyrill's suggestion as mentioned here : https://patchwork.kernel.org/patch/9624585/. And remove the use of SPINOR_OP_READ* and there by remove need to include spi-nor.h. Thanks Kamal On Fri, Jul 21, 2017 at 6:00 PM, Arnd Bergmann wrote: > The newly broadcom qspi driver in drivers/spi produces a build > warning when CONFIG_MTD is disabled: > > include/linux/mtd/cfi.h:76:2: #warning No CONFIG_MTD_CFI_Ix selected. No NOR > chip support can work. [-Werror=cpp] > > I had suggested a workaround earlier, but Cyrille Pitchen explained > that actually the broadcom qspi is broken here and needs to be > fixed, see the lenghthy reply in patchwork. > > As nothing has happened on that driver, this tries to at least > avoid the build failure, by marking the driver as broken unless > CONFIG_MTD is enabled. Also, I add a WARN_ON_ONCE runtime > that triggers when the spi-nor framework and the driver disagree > about the command opcode, which was one of several issues that > Cyrille pointed out. > > My patch does not attempt to fix any of the actual bugs though, > it just tries to avoid the build error while highlighting the > problems. Ideally, someone would step up to create a tested > fix. If that doesn't happen, please merge my version instead > as a workaround. > > Fixes: fa236a7ef240 ("spi: bcm-qspi: Add Broadcom MSPI driver") > Link: https://patchwork.kernel.org/patch/9334097/ > Link: https://patchwork.kernel.org/patch/9624585/ > Signed-off-by: Arnd Bergmann > Cc: Cyrille Pitchen > --- > --- > drivers/spi/Kconfig| 1 + > drivers/spi/spi-bcm-qspi.c | 23 --- > 2 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > index 9b31351fe429..c7a80f9d6dd0 100644 > --- a/drivers/spi/Kconfig > +++ b/drivers/spi/Kconfig > @@ -164,6 +164,7 @@ config SPI_BCM_QSPI > tristate "Broadcom BSPI and MSPI controller support" > depends on ARCH_BRCMSTB || ARCH_BCM || ARCH_BCM_IPROC || \ > BMIPS_GENERIC || COMPILE_TEST > + depends on BROKEN || MTD > default ARCH_BCM_IPROC > help > Enables support for the Broadcom SPI flash and MSPI controller. > diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c > index b19722ba908c..a388a3873552 100644 > --- a/drivers/spi/spi-bcm-qspi.c > +++ b/drivers/spi/spi-bcm-qspi.c > @@ -349,12 +349,13 @@ static void bcm_qspi_bspi_set_xfer_params(struct > bcm_qspi *qspi, u8 cmd_byte, > bcm_qspi_write(qspi, BSPI, BSPI_FLEX_MODE_ENABLE, flex_mode); > } > > -static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi *qspi, int width, > - int addrlen, int hp) > +static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi *qspi, > + struct spi_flash_read_message *msg, > + int width, int addrlen, int hp) > { > int bpc = 0, bpp = 0; > u8 command = SPINOR_OP_READ_FAST; > - int flex_mode = 1, rv = 0; > + int flex_mode = 1; > bool spans_4byte = false; > > dev_dbg(&qspi->pdev->dev, "set flex mode w %x addrlen %x hp %d\n", > @@ -405,15 +406,14 @@ static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi > *qspi, int width, > } > break; > default: > - rv = -EINVAL; > - break; > + return -EINVAL; > } > > - if (rv == 0) > - bcm_qspi_bspi_set_xfer_params(qspi, command, bpp, bpc, > - flex_mode); > + WARN_ON_ONCE(command != msg->read_opcode); > > - return rv; > + bcm_qspi_bspi_set_xfer_params(qspi, command, bpp, bpc, > + flex_mode); > + return 0; > } > > static int bcm_qspi_bspi_set_override(struct bcm_qspi *qspi, int width, > @@ -461,6 +461,7 @@ static int bcm_qspi_bspi_set_override(struct bcm_qspi > *qspi, int width, > } > > static int bcm_qspi_bspi_set_mode(struct bcm_qspi *qspi, > + struct spi_flash_read_message *msg, > int width, int addrlen, int hp) > { > int error = 0; > @@ -491,7 +492,7 @@ static int bcm_qspi_bspi_set_mode(struct bcm_qspi *qspi, > } > > if (qspi->xfer_mode.flex_mode) > - error = bcm_qspi_bspi_set_flex_mode(qspi, width, addrlen, hp); > + error = bcm_qspi_bspi_set_flex_mode(qspi, msg, width, > addrlen, hp); > > if (error) { > dev_warn(&qspi->pdev->dev, > @@ -1012,7 +1013,7 @@ static int bcm_qspi_flash_read(struct spi_device *spi, > > io_width = msg->data_nbits ? msg->data_nbits : SPI_NBITS_SINGLE; > addrlen = msg->addr_width; > - ret = bcm_qspi_bspi_set_mode(qspi, io_width, addrlen, -1); > + ret = bcm_qspi_bspi_set_mode(qspi, msg,
Re: [PATCH v2 1/2] mtd: brcmnand: iProc big endian and ONFI support
On Wed, Aug 10, 2016 at 10:24 AM, Boris Brezillon wrote: > On Wed, 20 Jul 2016 14:53:50 -0700 > Ray Jui wrote: > >> This patch adds big endian and ONFI support for various iProc based >> SoCs that use the core brcmstb NAND controller > > Brian, Kamal, can you review this patch? > >> >> This patch was originally implemented by Prafulla Kota >> and fully tested on iProc based NS2 SVK >> >> Signed-off-by: Prafulla Kota >> Signed-off-by: Ray Jui Acked-by: Kamal Dasu >> --- >> drivers/mtd/nand/brcmnand/brcmnand.c | 12 ++-- >> drivers/mtd/nand/brcmnand/brcmnand.h | 13 - >> drivers/mtd/nand/brcmnand/iproc_nand.c | 18 ++ >> 3 files changed, 28 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c >> b/drivers/mtd/nand/brcmnand/brcmnand.c >> index b76ad7c..12a1585 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c >> @@ -1279,7 +1279,7 @@ static void brcmnand_cmdfunc(struct mtd_info *mtd, >> unsigned command, >> u32 *flash_cache = (u32 *)ctrl->flash_cache; >> int i; >> >> - brcmnand_soc_data_bus_prepare(ctrl->soc); >> + brcmnand_soc_data_bus_prepare(ctrl->soc, true); >> >> /* >>* Must cache the FLASH_CACHE now, since changes in >> @@ -1292,7 +1292,7 @@ static void brcmnand_cmdfunc(struct mtd_info *mtd, >> unsigned command, >>*/ >> flash_cache[i] = be32_to_cpu(brcmnand_read_fc(ctrl, >> i)); >> >> - brcmnand_soc_data_bus_unprepare(ctrl->soc); >> + brcmnand_soc_data_bus_unprepare(ctrl->soc, true); >> >> /* Cleanup from HW quirk: restore SECTOR_SIZE_1K */ >> if (host->hwcfg.sector_size_1k) >> @@ -1508,12 +1508,12 @@ static int brcmnand_read_by_pio(struct mtd_info >> *mtd, struct nand_chip *chip, >> brcmnand_waitfunc(mtd, chip); >> >> if (likely(buf)) { >> - brcmnand_soc_data_bus_prepare(ctrl->soc); >> + brcmnand_soc_data_bus_prepare(ctrl->soc, false); >> >> for (j = 0; j < FC_WORDS; j++, buf++) >> *buf = brcmnand_read_fc(ctrl, j); >> >> - brcmnand_soc_data_bus_unprepare(ctrl->soc); >> + brcmnand_soc_data_bus_unprepare(ctrl->soc, false); >> } >> >> if (oob) >> @@ -1678,12 +1678,12 @@ static int brcmnand_write(struct mtd_info *mtd, >> struct nand_chip *chip, >> (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); >> >> if (buf) { >> - brcmnand_soc_data_bus_prepare(ctrl->soc); >> + brcmnand_soc_data_bus_prepare(ctrl->soc, false); >> >> for (j = 0; j < FC_WORDS; j++, buf++) >> brcmnand_write_fc(ctrl, j, *buf); >> >> - brcmnand_soc_data_bus_unprepare(ctrl->soc); >> + brcmnand_soc_data_bus_unprepare(ctrl->soc, false); >> } else if (oob) { >> for (j = 0; j < FC_WORDS; j++) >> brcmnand_write_fc(ctrl, j, 0x); >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h >> b/drivers/mtd/nand/brcmnand/brcmnand.h >> index ef5eabb..5c44cd4 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.h >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.h >> @@ -23,19 +23,22 @@ struct dev_pm_ops; >> struct brcmnand_soc { >> bool (*ctlrdy_ack)(struct brcmnand_soc *soc); >> void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en); >> - void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare); >> + void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare, >> + bool is_param); >> }; >> >> -static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc) >> +static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc, >> + bool is_param) >> { >> if (soc && soc->prepare_data_bus) >> - soc->prepare_data_bus(soc, true); >> + soc->prepare_data_bus(soc, true, is_param); >> } >> >> -static inline void brcmnand_soc_data_b
Re: [PATCH v2 1/2] mtd: brcmnand: iProc big endian and ONFI support
Boris This change looks good to me. On Wed, Aug 10, 2016 at 10:24 AM, Boris Brezillon wrote: > On Wed, 20 Jul 2016 14:53:50 -0700 > Ray Jui wrote: > >> This patch adds big endian and ONFI support for various iProc based >> SoCs that use the core brcmstb NAND controller > > Brian, Kamal, can you review this patch? > >> >> This patch was originally implemented by Prafulla Kota >> and fully tested on iProc based NS2 SVK >> >> Signed-off-by: Prafulla Kota >> Signed-off-by: Ray Jui Reviewed-by: Kamal Dasu >> --- >> drivers/mtd/nand/brcmnand/brcmnand.c | 12 ++-- >> drivers/mtd/nand/brcmnand/brcmnand.h | 13 - >> drivers/mtd/nand/brcmnand/iproc_nand.c | 18 ++ >> 3 files changed, 28 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c >> b/drivers/mtd/nand/brcmnand/brcmnand.c >> index b76ad7c..12a1585 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c >> @@ -1279,7 +1279,7 @@ static void brcmnand_cmdfunc(struct mtd_info *mtd, >> unsigned command, >> u32 *flash_cache = (u32 *)ctrl->flash_cache; >> int i; >> >> - brcmnand_soc_data_bus_prepare(ctrl->soc); >> + brcmnand_soc_data_bus_prepare(ctrl->soc, true); >> >> /* >>* Must cache the FLASH_CACHE now, since changes in >> @@ -1292,7 +1292,7 @@ static void brcmnand_cmdfunc(struct mtd_info *mtd, >> unsigned command, >>*/ >> flash_cache[i] = be32_to_cpu(brcmnand_read_fc(ctrl, >> i)); >> >> - brcmnand_soc_data_bus_unprepare(ctrl->soc); >> + brcmnand_soc_data_bus_unprepare(ctrl->soc, true); >> >> /* Cleanup from HW quirk: restore SECTOR_SIZE_1K */ >> if (host->hwcfg.sector_size_1k) >> @@ -1508,12 +1508,12 @@ static int brcmnand_read_by_pio(struct mtd_info >> *mtd, struct nand_chip *chip, >> brcmnand_waitfunc(mtd, chip); >> >> if (likely(buf)) { >> - brcmnand_soc_data_bus_prepare(ctrl->soc); >> + brcmnand_soc_data_bus_prepare(ctrl->soc, false); >> >> for (j = 0; j < FC_WORDS; j++, buf++) >> *buf = brcmnand_read_fc(ctrl, j); >> >> - brcmnand_soc_data_bus_unprepare(ctrl->soc); >> + brcmnand_soc_data_bus_unprepare(ctrl->soc, false); >> } >> >> if (oob) >> @@ -1678,12 +1678,12 @@ static int brcmnand_write(struct mtd_info *mtd, >> struct nand_chip *chip, >> (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS); >> >> if (buf) { >> - brcmnand_soc_data_bus_prepare(ctrl->soc); >> + brcmnand_soc_data_bus_prepare(ctrl->soc, false); >> >> for (j = 0; j < FC_WORDS; j++, buf++) >> brcmnand_write_fc(ctrl, j, *buf); >> >> - brcmnand_soc_data_bus_unprepare(ctrl->soc); >> + brcmnand_soc_data_bus_unprepare(ctrl->soc, false); >> } else if (oob) { >> for (j = 0; j < FC_WORDS; j++) >> brcmnand_write_fc(ctrl, j, 0x); >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h >> b/drivers/mtd/nand/brcmnand/brcmnand.h >> index ef5eabb..5c44cd4 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.h >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.h >> @@ -23,19 +23,22 @@ struct dev_pm_ops; >> struct brcmnand_soc { >> bool (*ctlrdy_ack)(struct brcmnand_soc *soc); >> void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en); >> - void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare); >> + void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare, >> + bool is_param); >> }; >> >> -static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc) >> +static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc, >> + bool is_param) >> { >> if (soc && soc->prepare_data_bus) >> - soc->prepare_data_bus(soc, true); >> + soc->prepare_data_bus(soc, true, is_param); >> } >> >
Re: [PATCH v0 3/4] i2c: brcmstb: Use complete() instead of complete_all()
LGTM On Wed, Aug 3, 2016 at 8:03 AM, Daniel Wagner wrote: > From: Daniel Wagner > > There is only one waiter for the completion, therefore there > is no need to use complete_all(). Let's make that clear by > using complete() instead of complete_all(). > > The usage pattern of the completion is: > > brcmstb_send_i2c_cmd() > reinit_completion() > ... > /* initiate transfer by setting iic_enable */ > ... > brcmstb_i2c_wait_for_completion() > > Signed-off-by: Daniel Wagner Reviewed-by: Kamal Dasu > --- > drivers/i2c/busses/i2c-brcmstb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-brcmstb.c > b/drivers/i2c/busses/i2c-brcmstb.c > index 6a8cfc1..ab4ff96 100644 > --- a/drivers/i2c/busses/i2c-brcmstb.c > +++ b/drivers/i2c/busses/i2c-brcmstb.c > @@ -228,7 +228,7 @@ static irqreturn_t brcmstb_i2c_isr(int irq, void *devid) > return IRQ_NONE; > > brcmstb_i2c_enable_disable_irq(dev, INT_DISABLE); > - complete_all(&dev->done); > + complete(&dev->done); > > dev_dbg(dev->device, "isr handled"); > return IRQ_HANDLED; > -- > 2.7.4
Re: [PATCH] mtd: nand: brcmnand: Change BUG_ON in brcmnand_send_cmd
On Sat, Jul 9, 2016 at 9:30 PM, Brian Norris wrote: > On Fri, Jul 08, 2016 at 10:36:39AM -0700, Florian Fainelli wrote: >> Change the BUG_ON() condition in brcmnand_send_cmd() which checks for >> the interrupt status "controller ready" bit to a WARN_ON. >> >> There is no good reason to kill the system when this condition occur >> because we could have systems which listed the NAND controller as >> available (e.g: from Device Tree), but the NAND chip could be >> malfunctioning and not responding. >> >> Signed-off-by: Florian Fainelli > > Acked-by: Brian Norris > Reviewed-by: Kamal Dasu >> --- >> Note that I even hesitated to remove that completely, but there is >> some value in knowing about this condition since it helps figuring >> out what could be wrong. >> >> drivers/mtd/nand/brcmnand/brcmnand.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c >> b/drivers/mtd/nand/brcmnand/brcmnand.c >> index b6062a2f3dfd..72bdc283778d 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c >> @@ -1165,7 +1165,7 @@ static void brcmnand_send_cmd(struct brcmnand_host >> *host, int cmd) >> ctrl->cmd_pending = cmd; >> >> intfc = brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS); >> - BUG_ON(!(intfc & INTFC_CTLR_READY)); >> + WARN_ON(!(intfc & INTFC_CTLR_READY)); >> >> mb(); /* flush previous writes */ >> brcmnand_write_reg(ctrl, BRCMNAND_CMD_START, >> -- >> 2.7.4 >> -- Kamal Dasu Principal Engineer | Broadcom Ltd. 200 Brickstone Sq Andover | 978-719-1405
Re: [PATCH v2] mtd: brcmnand: Add v7.2 controller support
This patch looks good to me. Acked-by: Kamal Dasu Thanks Kamal On Thu, Jun 2, 2016 at 1:13 PM, Boris Brezillon wrote: > On Tue, 31 May 2016 10:23:02 -0700 > Florian Fainelli wrote: > >> The 7.2 controller differs in a few area compared to its predecssor (7.1): >> >> - NAND scrambler, which we are not using just yet >> - higher ECC levels (up to 120 bits) per 1KB data blocks, also not supported >> yet >> - up to 128B OOB >> >> This patchs adds the ncessary code to support such a controller >> generation and updates the Device Tree binding. >> > > This patch looks good to me. Brian, Kamal, do you want to add your ack? > >> Signed-off-by: Florian Fainelli >> --- >> Changes in v2: >> >> - fixed a bunch of conditionals on the versions which should >> have been exclusive (spotted by Thomas) >> >> .../devicetree/bindings/mtd/brcm,brcmnand.txt | 1 + >> drivers/mtd/nand/brcmnand/brcmnand.c | 91 >> ++ >> 2 files changed, 78 insertions(+), 14 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> index deb24cbdda82..c8d71e75cb09 100644 >> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> @@ -27,6 +27,7 @@ Required properties: >> brcm,brcmnand-v6.2 >> brcm,brcmnand-v7.0 >> brcm,brcmnand-v7.1 >> + brcm,brcmnand-v7.2 >> brcm,brcmnand >> - reg : the register start and length for NAND register region. >> (optional) Flash DMA register range (if present) >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c >> b/drivers/mtd/nand/brcmnand/brcmnand.c >> index b76ad7c0144f..0a54a0f5a4b1 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c >> @@ -340,6 +340,36 @@ static const u16 brcmnand_regs_v71[] = { >> [BRCMNAND_FC_BASE] = 0x400, >> }; >> >> +/* BRCMNAND v7.2 */ >> +static const u16 brcmnand_regs_v72[] = { >> + [BRCMNAND_CMD_START]= 0x04, >> + [BRCMNAND_CMD_EXT_ADDRESS] = 0x08, >> + [BRCMNAND_CMD_ADDRESS] = 0x0c, >> + [BRCMNAND_INTFC_STATUS] = 0x14, >> + [BRCMNAND_CS_SELECT]= 0x18, >> + [BRCMNAND_CS_XOR] = 0x1c, >> + [BRCMNAND_LL_OP]= 0x20, >> + [BRCMNAND_CS0_BASE] = 0x50, >> + [BRCMNAND_CS1_BASE] = 0, >> + [BRCMNAND_CORR_THRESHOLD] = 0xdc, >> + [BRCMNAND_CORR_THRESHOLD_EXT] = 0xe0, >> + [BRCMNAND_UNCORR_COUNT] = 0xfc, >> + [BRCMNAND_CORR_COUNT] = 0x100, >> + [BRCMNAND_CORR_EXT_ADDR]= 0x10c, >> + [BRCMNAND_CORR_ADDR]= 0x110, >> + [BRCMNAND_UNCORR_EXT_ADDR] = 0x114, >> + [BRCMNAND_UNCORR_ADDR] = 0x118, >> + [BRCMNAND_SEMAPHORE]= 0x150, >> + [BRCMNAND_ID] = 0x194, >> + [BRCMNAND_ID_EXT] = 0x198, >> + [BRCMNAND_LL_RDATA] = 0x19c, >> + [BRCMNAND_OOB_READ_BASE]= 0x200, >> + [BRCMNAND_OOB_READ_10_BASE] = 0, >> + [BRCMNAND_OOB_WRITE_BASE] = 0x400, >> + [BRCMNAND_OOB_WRITE_10_BASE]= 0, >> + [BRCMNAND_FC_BASE] = 0x600, >> +}; >> + >> enum brcmnand_cs_reg { >> BRCMNAND_CS_CFG_EXT = 0, >> BRCMNAND_CS_CFG, >> @@ -435,7 +465,9 @@ static int brcmnand_revision_init(struct >> brcmnand_controller *ctrl) >> } >> >> /* Register offsets */ >> - if (ctrl->nand_version >= 0x0701) >> + if (ctrl->nand_version >= 0x0702) >> + ctrl->reg_offsets = brcmnand_regs_v72; >> + else if (ctrl->nand_version >= 0x0701) >> ctrl->reg_offsets = brcmnand_regs_v71; >> else if (ctrl->nand_version >= 0x0600) >> ctrl->reg_offsets = brcmnand_regs_v60; >> @@ -480,7 +512,9 @@ static int brcmnand_revision_init(struct >> brcmnand_controller *ctrl) >> } >> >> /* Maximum spare area sector size (per 512B) */ >> - if (ctrl->nand_version >= 0x0600) >> + if (ctrl->nand_version >= 0x0702) >> + ctrl->max