Hi all,
This patch series adds support for the BRCMNAND controller revision 3.4
embedded in MIPS-based SoCs such as 5357, typically found in the Netgear
WNR3500L v2 and other kinds of Wi-Fi routers. The upstream platform that
uses this controller is under arch/mips/bcm47xx/ and does not use Device
Tree (and probably never will by now). BCMA (Broadcom AMBA) is a special
kind of discoverable memory mapped interface which requires the use of
special accessors to read from/write to the hardware block.
The integration of brcmnand into that SoC is a bit quirky in that every
register offering byte level data about the flash (OOB, device ID, etc.)
requires byte swapping. The command shift should also have been 24, but
is in fact 0, took me a while to understand why no reads were actually
working because of that.
This has been tested with Linux 5.10.82 and Linus' master with OpenWrt
and confirmed that the squashfs + jffs2 overlay that OpenWrt creates is
entirely functional and that written data is made persistent.
There are a number of open questions:
- the flash that is used is a s34ml01g100tf100 part which is said to be
ONFI 1.0 compliant however the ONFI signature is not ONFI, but the
device ID of the flash as if read with address 0, is that a known
issue?
- because no ONFI detection is taking place we fallback to the
identification table matching, but there is no default ECC
size/strength filed by the rawnand framework, probably made largely
worse by the absence of a suitable Device Tree information?
Florian Fainelli (9):
mtd: rawnand: brcmnand: Allow SoC to provide I/O operations
mtd: rawnand: brcmnand: Assign soc as early as possible
mtd: rawnand: brcmnand: Avoid pdev in brcmnand_init_cs()
mtd: rawnand: brcmnand: Move OF operations out of brcmnand_init_cs()
mtd: rawnand: brcmnand: Allow working without interrupts
mtd: rawnand: brcmnand: Add platform data structure for BCMA
mtd: rawnand: brcmnand: Allow platform data instantation
mtd: rawnand: brcmnand: BCMA controller uses command shift of 0
mtd: rawnand: brcmnand: Add BCMA shim
MAINTAINERS | 1 +
drivers/bcma/driver_chipcommon_nflash.c | 17 ++-
drivers/mtd/nand/raw/Kconfig | 11 ++
drivers/mtd/nand/raw/brcmnand/Makefile | 2 +
drivers/mtd/nand/raw/brcmnand/bcma_nand.c | 131 ++++++++++++++++++
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 142 +++++++++++++-------
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 ++++
include/linux/bcma/bcma_driver_chipcommon.h | 5 +
include/linux/platform_data/brcmnand.h | 12 ++
9 files changed, 291 insertions(+), 53 deletions(-)
create mode 100644 drivers/mtd/nand/raw/brcmnand/bcma_nand.c
create mode 100644 include/linux/platform_data/brcmnand.h
--
2.25.1
Allow a brcmnand_soc instance to provide a custom set of I/O operations
which we will require when using this driver on a BCMA bus which is not
directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
to use the SoC operations if provided.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
In order to key off the brcmnand_probe() code in subsequent changes
depending upon ctlr->soc, assign that variable as early as possible,
instead of much later when we have checked that it is non-NULL.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
In preparation for encapsulation more of what the loop calling
brcmnand_init_cs() does, avoid using platform_device when it is the
device behind platform_device that we are using for printing errors.
No functional change.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
In order to initialize a given chip select object for use by the
brcmnand driver, move all of the Device Tree specific routines outside
of brcmnand_init_cs() in order to make it usable in a platform data
configuration which will be necessary for supporting BCMA chips.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
Update the BCMA's chipcommon nand flash driver to detect which
chip-select is used and pass that information via platform data to the
brcmnand driver. Make sure that the brcmnand platform data structure is
always at the beginning of the platform data of the "nflash" device
created by BCMA to allow brcmnand to safely de-reference it.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
MAINTAINERS | 1 +
drivers/bcma/driver_chipcommon_nflash.c | 17 ++++++++++++++++-
include/linux/bcma/bcma_driver_chipcommon.h | 5 +++++
include/linux/platform_data/brcmnand.h | 12 ++++++++++++
4 files changed, 34 insertions(+), 1 deletion(-)
create mode 100644 include/linux/platform_data/brcmnand.h
@@ -33,8 +40,16 @@ int bcma_nflash_init(struct bcma_drv_cc *cc)cc->nflash.present=true;if(cc->core->id.rev==38&&-(cc->status&BCMA_CC_CHIPST_5357_NAND_BOOT))+(cc->status&BCMA_CC_CHIPST_5357_NAND_BOOT)){cc->nflash.boot=true;+/* Determine the chip select that is being used */+reg=bcma_cc_read32(cc,BCMA_CC_NAND_CS_NAND_SELECT)&0xff;+cc->nflash.brcmnand_info.chip_select=ffs(reg)-1;+cc->nflash.brcmnand_info.part_probe_types=probes;+cc->nflash.brcmnand_info.ecc_stepsize=512;+cc->nflash.brcmnand_info.ecc_strength=1;+bcma_nflash_dev.name=bcma_nflash_alt_name;+}/* Prepare platform device, but don't register it yet. It's too early,*malloc(requiredbydevice_private_init)isnotavailableyet.*/
@@ -599,6 +600,10 @@ struct bcma_sflash {#ifdef CONFIG_BCMA_NFLASHstructbcma_nflash{+/* Must be the fist member for the brcmnand driver to+*de-referencethatstructure.+*/+structbrcmnand_platform_databrcmnand_info;boolpresent;boolboot;/* This is the flash the SoC boots from */};
Make use of the recently refactored code in brcmnand_init_cs() and
derive the chip-select from the platform data that is supplied. Update
the various code paths to avoid relying on possibly non-existent
resources, too.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 45 ++++++++++++++++++------
1 file changed, 35 insertions(+), 10 deletions(-)
@@ -2813,7 +2815,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host)if(ret)returnret;-ret=mtd_device_register(mtd,NULL,0);+ret=mtd_device_parse_register(mtd,part_probe_types,NULL,NULL,0);if(ret)nand_cleanup(chip);
@@ -2982,17 +2984,15 @@ static int brcmnand_edu_setup(struct platform_device *pdev)intbrcmnand_probe(structplatform_device*pdev,structbrcmnand_soc*soc){+structbrcmnand_platform_data*pd=dev_get_platdata(&pdev->dev);structdevice*dev=&pdev->dev;structdevice_node*dn=dev->of_node,*child;structbrcmnand_controller*ctrl;+structbrcmnand_host*host;structresource*res;intret;-/* We only support device-tree instantiation */-if(!dn)-return-ENODEV;--if(!of_match_node(brcmnand_of_match,dn))+if(dn&&!of_match_node(brcmnand_of_match,dn))return-ENODEV;ctrl=devm_kzalloc(dev,sizeof(*ctrl),GFP_KERNEL);
@@ -3013,7 +3013,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)/* NAND register range */res=platform_get_resource(pdev,IORESOURCE_MEM,0);ctrl->nand_base=devm_ioremap_resource(dev,res);-if(IS_ERR(ctrl->nand_base))+if(IS_ERR(ctrl->nand_base)&&!brcmnand_soc_has_ops(soc))returnPTR_ERR(ctrl->nand_base);/* Enable clock before using NAND registers */
@@ -3177,7 +3176,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)nand_set_flash_node(&host->chip,dn);-ret=brcmnand_init_cs(host);+ret=brcmnand_init_cs(host,NULL);if(ret){devm_kfree(dev,host);continue;/* Try all chip-selects */
@@ -3187,6 +3186,32 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)}}+if(!list_empty(&ctrl->host_list))+return0;++if(!pd){+ret=-ENODEV;+gotoerr;+}++/* If we got there we must have been probing via platform data */+host=devm_kzalloc(dev,sizeof(*host),GFP_KERNEL);+if(!host){+ret=-ENOMEM;+gotoerr;+}+host->pdev=pdev;+host->ctrl=ctrl;+host->cs=pd->chip_select;+host->chip.ecc.size=pd->ecc_stepsize;+host->chip.ecc.strength=pd->ecc_strength;++ret=brcmnand_init_cs(host,pd->part_probe_types);+if(ret)+gotoerr;++list_add_tail(&host->node,&ctrl->host_list);+/* No chip-selects could initialize properly */if(list_empty(&ctrl->host_list)){ret=-ENODEV;
The BCMA devices include the brcmnand controller but they do not wire up
any interrupt line, allow the main interrupt to be optional and update
the completion path to also check for the lack of an interrupt line.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 52 +++++++++++-------------
1 file changed, 24 insertions(+), 28 deletions(-)
Add a BCMA shim to allow us to register the brcmnand driver using the
BCMA bus which provides indirect memory mapped access to SoC registers.
There are a number of registers that need to be byte swapped because
they are natively big endian, coming directly from the NAND chip, and
there is no bus interface unlike the iProc or STB platforms that
performs the byte swapping for us.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/Kconfig | 11 ++
drivers/mtd/nand/raw/brcmnand/Makefile | 2 +
drivers/mtd/nand/raw/brcmnand/bcma_nand.c | 131 ++++++++++++++++++++++
3 files changed, 144 insertions(+)
create mode 100644 drivers/mtd/nand/raw/brcmnand/bcma_nand.c
For some odd and unexplained reason the BCMA NAND controller, albeit
revision 3.4 uses a command shift of 0 instead of 24 as it should be,
quirk that.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -908,6 +908,12 @@ static void brcmnand_wr_corr_thresh(struct brcmnand_host *host, u8 val)staticinlineintbrcmnand_cmd_shift(structbrcmnand_controller*ctrl){+/* Kludge for the BCMA-based NAND controller which does not actually+*shiftthecommand+*/+if(ctrl->nand_version==0x0304&&brcmnand_soc_has_ops(ctrl->soc))+return0;+if(ctrl->nand_version<0x0602)return24;return0;
From: Andy Shevchenko <hidden> Date: 2021-12-25 17:46:20
On Sat, Dec 25, 2021 at 1:41 AM Florian Fainelli [off-list ref] wrote:
The BCMA devices include the brcmnand controller but they do not wire up
any interrupt line, allow the main interrupt to be optional and update
the completion path to also check for the lack of an interrupt line.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
On Sat, Dec 25, 2021 at 1:41 AM Florian Fainelli [off-list ref] wrote:
quoted
The BCMA devices include the brcmnand controller but they do not wire up
any interrupt line, allow the main interrupt to be optional and update
the completion path to also check for the lack of an interrupt line.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
ret = ...
if (ret > 0) {
And drop 0 from the equation, OF never uses 0 as valid vIRQ.
OK but the point of this patch series is to allow the use of the brcmnand driver in a configuration without OF. I don't really see the point in continuing to use unsigned int instead of just letting request_irq() play through and tell us if the interrupt descriptor was valid later on.
--
Florian
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:17 -0800:
quoted hunk
Allow a brcmnand_soc instance to provide a custom set of I/O operations
which we will require when using this driver on a BCMA bus which is not
directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
to use the SoC operations if provided.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
It might be worth looking into more optimized ways to do these checks,
in particular the read/write_reg ones because you're checking against
some static data which cannot be optimized out by the compiler but
won't change in the lifetime of the kernel.
Thanks,
Miquèl
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:20 -0800:
In order to initialize a given chip select object for use by the
brcmnand driver, move all of the Device Tree specific routines outside
of brcmnand_init_cs() in order to make it usable in a platform data
configuration which will be necessary for supporting BCMA chips.
TBH I'm note a big fan of the idea. I'm not sure going back to
supporting platform data this way really is a good idea... There are so
much things that are well described with DT that we now rely upon that
I am not entirely convinced by these changes :-/ The move is generally
in the other direction: getting rid of the legacy platform data.
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:25 -0800:
quoted hunk
Add a BCMA shim to allow us to register the brcmnand driver using the
BCMA bus which provides indirect memory mapped access to SoC registers.
There are a number of registers that need to be byte swapped because
they are natively big endian, coming directly from the NAND chip, and
there is no bus interface unlike the iProc or STB platforms that
performs the byte swapping for us.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/Kconfig | 11 ++
drivers/mtd/nand/raw/brcmnand/Makefile | 2 +
drivers/mtd/nand/raw/brcmnand/bcma_nand.c | 131 ++++++++++++++++++++++
3 files changed, 144 insertions(+)
create mode 100644 drivers/mtd/nand/raw/brcmnand/bcma_nand.c
+MODULE_DESCRIPTION("NAND driver for BCMA chips");
NAND controller driver, I want to enforce this *controller* word
because NAND is way too vague and refers to nothing specific.
Plus, it's not really a controller driver that we have here, more like
a glue interface.
Thanks,
Miquèl
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:17 -0800:
quoted
Allow a brcmnand_soc instance to provide a custom set of I/O operations
which we will require when using this driver on a BCMA bus which is not
directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
to use the SoC operations if provided.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
It might be worth looking into more optimized ways to do these checks,
in particular the read/write_reg ones because you're checking against
some static data which cannot be optimized out by the compiler but
won't change in the lifetime of the kernel.
I suppose I could add an addition if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA) at the front of brcmnand_soc_has_ops(), would that address your concern or you have something else in mind?
--
Florian
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:20 -0800:
quoted
In order to initialize a given chip select object for use by the
brcmnand driver, move all of the Device Tree specific routines outside
of brcmnand_init_cs() in order to make it usable in a platform data
configuration which will be necessary for supporting BCMA chips.
TBH I'm note a big fan of the idea. I'm not sure going back to
supporting platform data this way really is a good idea... There are so
much things that are well described with DT that we now rely upon that
I am not entirely convinced by these changes :-/ The move is generally
in the other direction: getting rid of the legacy platform data.
In the cover letter there is an explanation as to why we need to introduce platform data/device support here: the platforms on which this NAND controller shim is used do not have Device Tree support, and won't have it in the future either. They are old platforms (first SoC supported by bcm47xx is maybe 15 years old now) but they are still in active and wide use by the OpenWrt, dd-wrt communities.
--
Florian
+MODULE_DESCRIPTION("NAND driver for BCMA chips");
NAND controller driver, I want to enforce this *controller* word
because NAND is way too vague and refers to nothing specific.
Plus, it's not really a controller driver that we have here, more like
a glue interface.
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:25 -0800:
quoted
Add a BCMA shim to allow us to register the brcmnand driver using the
BCMA bus which provides indirect memory mapped access to SoC registers.
There are a number of registers that need to be byte swapped because
they are natively big endian, coming directly from the NAND chip, and
there is no bus interface unlike the iProc or STB platforms that
performs the byte swapping for us.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/Kconfig | 11 ++
drivers/mtd/nand/raw/brcmnand/Makefile | 2 +
drivers/mtd/nand/raw/brcmnand/bcma_nand.c | 131 ++++++++++++++++++++++
3 files changed, 144 insertions(+)
create mode 100644 drivers/mtd/nand/raw/brcmnand/bcma_nand.c
Hi Florian,
f.fainelli@gmail.com wrote on Mon, 3 Jan 2022 09:27:28 -0800:
On 1/3/2022 8:56 AM, Miquel Raynal wrote:
quoted
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:20 -0800:
quoted
In order to initialize a given chip select object for use by the
brcmnand driver, move all of the Device Tree specific routines outside
of brcmnand_init_cs() in order to make it usable in a platform data
configuration which will be necessary for supporting BCMA chips.
TBH I'm note a big fan of the idea. I'm not sure going back to
supporting platform data this way really is a good idea... There are so
much things that are well described with DT that we now rely upon that
I am not entirely convinced by these changes :-/ The move is generally
in the other direction: getting rid of the legacy platform data.
In the cover letter there is an explanation as to why we need to introduce platform data/device support here: the platforms on which this NAND controller shim is used do not have Device Tree support, and won't have it in the future either. They are old platforms (first SoC supported by bcm47xx is maybe 15 years old now) but they are still in active and wide use by the OpenWrt, dd-wrt communities.
Yeah, I read the cover letter, I understand these platforms won't ever
be updated so you're stuck. I'll close my eyes.
Thanks,
Miquèl
Hi Florian,
f.fainelli@gmail.com wrote on Mon, 3 Jan 2022 09:24:26 -0800:
On 1/3/2022 8:49 AM, Miquel Raynal wrote:
quoted
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:17 -0800:
quoted
Allow a brcmnand_soc instance to provide a custom set of I/O operations
which we will require when using this driver on a BCMA bus which is not
directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
to use the SoC operations if provided.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
It might be worth looking into more optimized ways to do these checks,
in particular the read/write_reg ones because you're checking against
some static data which cannot be optimized out by the compiler but
won't change in the lifetime of the kernel.
I suppose I could add an addition if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA) at the front of brcmnand_soc_has_ops(), would that address your concern or you have something else in mind?
I don't like much the #ifdef solution, instead you might think of
static keys, or even better using a regmap. Regmap implementation is
free, you can use either one way or the other and for almost no
overhead compared to the bunch of functions you have here.
Thanks,
Miquèl
Hi Miquel,
miquel.raynal@bootlin.com wrote on Tue, 4 Jan 2022 09:32:21 +0100:
Hi Florian,
f.fainelli@gmail.com wrote on Mon, 3 Jan 2022 09:24:26 -0800:
quoted
On 1/3/2022 8:49 AM, Miquel Raynal wrote:
quoted
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:17 -0800:
quoted
Allow a brcmnand_soc instance to provide a custom set of I/O operations
which we will require when using this driver on a BCMA bus which is not
directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
to use the SoC operations if provided.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
It might be worth looking into more optimized ways to do these checks,
in particular the read/write_reg ones because you're checking against
some static data which cannot be optimized out by the compiler but
won't change in the lifetime of the kernel.
I suppose I could add an addition if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA) at the front of brcmnand_soc_has_ops(), would that address your concern or you have something else in mind?
I don't like much the #ifdef solution, instead you might think of
static keys, or even better using a regmap. Regmap implementation is
free, you can use either one way or the other and for almost no
overhead compared to the bunch of functions you have here.
Maybe regmaps will actually be slower than these regular if's. Perhaps
static keys are the best option?
Cheers,
Miquèl
Hi Miquel,
miquel.raynal@bootlin.com wrote on Tue, 4 Jan 2022 09:32:21 +0100:
quoted
Hi Florian,
f.fainelli@gmail.com wrote on Mon, 3 Jan 2022 09:24:26 -0800:
quoted
On 1/3/2022 8:49 AM, Miquel Raynal wrote:
quoted
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:17 -0800:
quoted
Allow a brcmnand_soc instance to provide a custom set of I/O operations
which we will require when using this driver on a BCMA bus which is not
directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
to use the SoC operations if provided.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
It might be worth looking into more optimized ways to do these checks,
in particular the read/write_reg ones because you're checking against
some static data which cannot be optimized out by the compiler but
won't change in the lifetime of the kernel.
I suppose I could add an addition if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA) at the front of brcmnand_soc_has_ops(), would that address your concern or you have something else in mind?
I don't like much the #ifdef solution, instead you might think of
static keys, or even better using a regmap. Regmap implementation is
free, you can use either one way or the other and for almost no
overhead compared to the bunch of functions you have here.
Maybe regmaps will actually be slower than these regular if's. Perhaps
static keys are the best option?
OK static keys would probably work. I am not sure that the additional
branches for each register access would actually be causing a noticeable
performance impact. Pretty much any chip where this controller is used
has a DMA interface that you program and kick, the PIO is already
assumed to be slow, and each register access is about 200ns on STB chips
at least.
--
Florian
Hi Florian,
f.fainelli@gmail.com wrote on Tue, 4 Jan 2022 10:34:35 -0800:
On 1/4/22 12:57 AM, Miquel Raynal wrote:
quoted
Hi Miquel,
miquel.raynal@bootlin.com wrote on Tue, 4 Jan 2022 09:32:21 +0100:
quoted
Hi Florian,
f.fainelli@gmail.com wrote on Mon, 3 Jan 2022 09:24:26 -0800:
quoted
On 1/3/2022 8:49 AM, Miquel Raynal wrote:
quoted
Hi Florian,
f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:17 -0800:
quoted
Allow a brcmnand_soc instance to provide a custom set of I/O operations
which we will require when using this driver on a BCMA bus which is not
directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
to use the SoC operations if provided.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
It might be worth looking into more optimized ways to do these checks,
in particular the read/write_reg ones because you're checking against
some static data which cannot be optimized out by the compiler but
won't change in the lifetime of the kernel.
I suppose I could add an addition if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA) at the front of brcmnand_soc_has_ops(), would that address your concern or you have something else in mind?
I don't like much the #ifdef solution, instead you might think of
static keys, or even better using a regmap. Regmap implementation is
free, you can use either one way or the other and for almost no
overhead compared to the bunch of functions you have here.
Maybe regmaps will actually be slower than these regular if's. Perhaps
static keys are the best option?
OK static keys would probably work. I am not sure that the additional
branches for each register access would actually be causing a noticeable
performance impact. Pretty much any chip where this controller is used
has a DMA interface that you program and kick, the PIO is already
assumed to be slow, and each register access is about 200ns on STB chips
at least.
I see. I'll let you decide what you prefer, I won't block if you stick
to the regular if/else implementation.
Thanks,
Miquèl