* [RFC PATCH 0/3] Changes in the internal raw NAND API
@ 2018-07-17 9:54 Miquel Raynal
2018-07-17 9:54 ` [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure Miquel Raynal
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Miquel Raynal @ 2018-07-17 9:54 UTC (permalink / raw)
To: Boris Brezillon, Richard Weinberger, David Woodhouse,
Brian Norris, Marek Vasut
Cc: linux-mtd, Miquel Raynal
Hello,
After having sent two versions of a series removing the
nand_scan_ident/nand_scan_tail limitation that prevents us to allocate
memory at critical moments, we are still discussing the internal API.
To avoid sending again more than 20 patches, this is an RFC of the
internal changes prior to the above modifications in each driver:
1/ Rename struct nand_hw_control -> struct nand_controller which is
much more meaningful.
2/ Rename the function initializing the above structure
nand_hw_control_init() -> nand_controller_init().
3/ Create a nand_controller_ops structure which will be embedded in the
nand_controller structure. These operations are:
int (*attach_chip)(struct nand_chip *) and
void (*detach_chip)(struct nand_chip *).
If we agree on this, I could merge them first and then send the bunch of
patches making use of these API changes.
Thanks,
Miquèl
Miquel Raynal (3):
mtd: rawnand: better name for the controller structure
mtd: rawnand: update the controller structure initialization function
name
mtd: rawnand: add hooks that may be called during nand_scan()
drivers/mtd/nand/raw/atmel/nand-controller.c | 10 +++++-----
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
drivers/mtd/nand/raw/docg4.c | 2 +-
drivers/mtd/nand/raw/fsl_elbc_nand.c | 4 ++--
drivers/mtd/nand/raw/fsl_ifc_nand.c | 4 ++--
drivers/mtd/nand/raw/jz4780_nand.c | 7 ++++---
drivers/mtd/nand/raw/marvell_nand.c | 6 +++---
drivers/mtd/nand/raw/mtk_nand.c | 2 +-
drivers/mtd/nand/raw/nand_base.c | 23 ++++++++++++++++++++---
drivers/mtd/nand/raw/ndfc.c | 4 ++--
drivers/mtd/nand/raw/omap2.c | 2 +-
drivers/mtd/nand/raw/oxnas_nand.c | 4 ++--
drivers/mtd/nand/raw/qcom_nandc.c | 4 ++--
drivers/mtd/nand/raw/s3c2410.c | 4 ++--
drivers/mtd/nand/raw/sunxi_nand.c | 6 +++---
drivers/mtd/nand/raw/tango_nand.c | 4 ++--
drivers/mtd/nand/raw/tegra_nand.c | 6 +++---
drivers/mtd/nand/raw/txx9ndfmc.c | 4 ++--
include/linux/mtd/rawnand.h | 27 ++++++++++++++++++++++-----
19 files changed, 81 insertions(+), 46 deletions(-)
--
2.14.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure 2018-07-17 9:54 [RFC PATCH 0/3] Changes in the internal raw NAND API Miquel Raynal @ 2018-07-17 9:54 ` Miquel Raynal 2018-07-17 11:49 ` Boris Brezillon 2018-07-17 9:54 ` [RFC PATCH 2/3] mtd: rawnand: update the controller structure initialization function name Miquel Raynal 2018-07-17 9:54 ` [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() Miquel Raynal 2 siblings, 1 reply; 12+ messages in thread From: Miquel Raynal @ 2018-07-17 9:54 UTC (permalink / raw) To: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut Cc: linux-mtd, Miquel Raynal In the raw NAND core, a NAND chip is described by a nand_chip structure, while a NAND controller is described with a nand_hw_control structure which is not very meaningful. 'control' is misleading and 'hw' has no meaning here has it refers to hardware ECC operations only, which is too restrictive. Rename this structure nand_controller. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/atmel/nand-controller.c | 8 ++++---- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- drivers/mtd/nand/raw/fsl_elbc_nand.c | 2 +- drivers/mtd/nand/raw/fsl_ifc_nand.c | 2 +- drivers/mtd/nand/raw/jz4780_nand.c | 5 +++-- drivers/mtd/nand/raw/marvell_nand.c | 4 ++-- drivers/mtd/nand/raw/mtk_nand.c | 2 +- drivers/mtd/nand/raw/ndfc.c | 2 +- drivers/mtd/nand/raw/omap2.c | 2 +- drivers/mtd/nand/raw/oxnas_nand.c | 2 +- drivers/mtd/nand/raw/qcom_nandc.c | 2 +- drivers/mtd/nand/raw/s3c2410.c | 2 +- drivers/mtd/nand/raw/sunxi_nand.c | 4 ++-- drivers/mtd/nand/raw/tango_nand.c | 2 +- drivers/mtd/nand/raw/tegra_nand.c | 4 ++-- drivers/mtd/nand/raw/txx9ndfmc.c | 2 +- include/linux/mtd/rawnand.h | 11 ++++++----- 17 files changed, 30 insertions(+), 28 deletions(-) diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c index e686fe73159e..8fcb66df10ea 100644 --- a/drivers/mtd/nand/raw/atmel/nand-controller.c +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c @@ -216,7 +216,7 @@ struct atmel_nand_controller_caps { }; struct atmel_nand_controller { - struct nand_hw_control base; + struct nand_controller base; const struct atmel_nand_controller_caps *caps; struct device *dev; struct regmap *smc; @@ -227,7 +227,7 @@ struct atmel_nand_controller { }; static inline struct atmel_nand_controller * -to_nand_controller(struct nand_hw_control *ctl) +to_nand_controller(struct nand_controller *ctl) { return container_of(ctl, struct atmel_nand_controller, base); } @@ -239,7 +239,7 @@ struct atmel_smc_nand_controller { }; static inline struct atmel_smc_nand_controller * -to_smc_nand_controller(struct nand_hw_control *ctl) +to_smc_nand_controller(struct nand_controller *ctl) { return container_of(to_nand_controller(ctl), struct atmel_smc_nand_controller, base); @@ -263,7 +263,7 @@ struct atmel_hsmc_nand_controller { }; static inline struct atmel_hsmc_nand_controller * -to_hsmc_nand_controller(struct nand_hw_control *ctl) +to_hsmc_nand_controller(struct nand_controller *ctl) { return container_of(to_nand_controller(ctl), struct atmel_hsmc_nand_controller, base); diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 1306aaa7a8bf..2c8e3d3977c0 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -114,7 +114,7 @@ enum { struct brcmnand_controller { struct device *dev; - struct nand_hw_control controller; + struct nand_controller controller; void __iomem *nand_base; void __iomem *nand_fc; /* flash cache */ void __iomem *flash_dma_base; diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c index 51f0b340bc0d..756431ec055b 100644 --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c @@ -61,7 +61,7 @@ struct fsl_elbc_mtd { /* Freescale eLBC FCM controller information */ struct fsl_elbc_fcm_ctrl { - struct nand_hw_control controller; + struct nand_controller controller; struct fsl_elbc_mtd *chips[MAX_BANKS]; u8 __iomem *addr; /* Address of assigned FCM buffer */ diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c index 382b67e97174..92b8b63a8b75 100644 --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c @@ -51,7 +51,7 @@ struct fsl_ifc_mtd { /* overview of the fsl ifc controller */ struct fsl_ifc_nand_ctrl { - struct nand_hw_control controller; + struct nand_controller controller; struct fsl_ifc_mtd *chips[FSL_IFC_BANK_COUNT]; void __iomem *addr; /* Address of assigned IFC buffer */ diff --git a/drivers/mtd/nand/raw/jz4780_nand.c b/drivers/mtd/nand/raw/jz4780_nand.c index e69f6ae4c539..b9afd41132ac 100644 --- a/drivers/mtd/nand/raw/jz4780_nand.c +++ b/drivers/mtd/nand/raw/jz4780_nand.c @@ -44,7 +44,7 @@ struct jz4780_nand_cs { struct jz4780_nand_controller { struct device *dev; struct jz4780_bch *bch; - struct nand_hw_control controller; + struct nand_controller controller; unsigned int num_banks; struct list_head chips; int selected; @@ -65,7 +65,8 @@ static inline struct jz4780_nand_chip *to_jz4780_nand_chip(struct mtd_info *mtd) return container_of(mtd_to_nand(mtd), struct jz4780_nand_chip, chip); } -static inline struct jz4780_nand_controller *to_jz4780_nand_controller(struct nand_hw_control *ctrl) +static inline struct jz4780_nand_controller +*to_jz4780_nand_controller(struct nand_controller *ctrl) { return container_of(ctrl, struct jz4780_nand_controller, controller); } diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c index 80a074cccb82..fa7d284ac8af 100644 --- a/drivers/mtd/nand/raw/marvell_nand.c +++ b/drivers/mtd/nand/raw/marvell_nand.c @@ -318,7 +318,7 @@ struct marvell_nfc_caps { * @dma_buf: 32-bit aligned buffer for DMA transfers (NFCv1 only) */ struct marvell_nfc { - struct nand_hw_control controller; + struct nand_controller controller; struct device *dev; void __iomem *regs; struct clk *core_clk; @@ -335,7 +335,7 @@ struct marvell_nfc { u8 *dma_buf; }; -static inline struct marvell_nfc *to_marvell_nfc(struct nand_hw_control *ctrl) +static inline struct marvell_nfc *to_marvell_nfc(struct nand_controller *ctrl) { return container_of(ctrl, struct marvell_nfc, controller); } diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c index 75c845adb050..254ee76d354d 100644 --- a/drivers/mtd/nand/raw/mtk_nand.c +++ b/drivers/mtd/nand/raw/mtk_nand.c @@ -145,7 +145,7 @@ struct mtk_nfc_clk { }; struct mtk_nfc { - struct nand_hw_control controller; + struct nand_controller controller; struct mtk_ecc_config ecc_cfg; struct mtk_nfc_clk clk; struct mtk_ecc *ecc; diff --git a/drivers/mtd/nand/raw/ndfc.c b/drivers/mtd/nand/raw/ndfc.c index d8a806894937..fdc55fda5db6 100644 --- a/drivers/mtd/nand/raw/ndfc.c +++ b/drivers/mtd/nand/raw/ndfc.c @@ -39,7 +39,7 @@ struct ndfc_controller { void __iomem *ndfcbase; struct nand_chip chip; int chip_select; - struct nand_hw_control ndfc_control; + struct nand_controller ndfc_control; }; static struct ndfc_controller ndfc_ctrl[NDFC_MAX_CS]; diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c index e50c64adc3c8..e943b2e5a5e2 100644 --- a/drivers/mtd/nand/raw/omap2.c +++ b/drivers/mtd/nand/raw/omap2.c @@ -145,7 +145,7 @@ static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc, static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10}; /* Shared among all NAND instances to synchronize access to the ECC Engine */ -static struct nand_hw_control omap_gpmc_controller = { +static struct nand_controller omap_gpmc_controller = { .lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock), .wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq), }; diff --git a/drivers/mtd/nand/raw/oxnas_nand.c b/drivers/mtd/nand/raw/oxnas_nand.c index d649d5944826..3621c29e7221 100644 --- a/drivers/mtd/nand/raw/oxnas_nand.c +++ b/drivers/mtd/nand/raw/oxnas_nand.c @@ -32,7 +32,7 @@ #define OXNAS_NAND_MAX_CHIPS 1 struct oxnas_nand_ctrl { - struct nand_hw_control base; + struct nand_controller base; void __iomem *io_base; struct clk *clk; struct nand_chip *chips[OXNAS_NAND_MAX_CHIPS]; diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c index 994f980c6d86..98800a26b2c7 100644 --- a/drivers/mtd/nand/raw/qcom_nandc.c +++ b/drivers/mtd/nand/raw/qcom_nandc.c @@ -365,7 +365,7 @@ struct nandc_regs { * from all connected NAND devices pagesize */ struct qcom_nand_controller { - struct nand_hw_control controller; + struct nand_controller controller; struct list_head host_list; struct device *dev; diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c index 19661c5d3220..aee043dd4fa9 100644 --- a/drivers/mtd/nand/raw/s3c2410.c +++ b/drivers/mtd/nand/raw/s3c2410.c @@ -162,7 +162,7 @@ enum s3c_nand_clk_state { */ struct s3c2410_nand_info { /* mtd info */ - struct nand_hw_control controller; + struct nand_controller controller; struct s3c2410_nand_mtd *mtds; struct s3c2410_platform_nand *platform; diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c index d831a141a196..de19c81a1789 100644 --- a/drivers/mtd/nand/raw/sunxi_nand.c +++ b/drivers/mtd/nand/raw/sunxi_nand.c @@ -234,7 +234,7 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand) * controller events */ struct sunxi_nfc { - struct nand_hw_control controller; + struct nand_controller controller; struct device *dev; void __iomem *regs; struct clk *ahb_clk; @@ -247,7 +247,7 @@ struct sunxi_nfc { struct dma_chan *dmac; }; -static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_hw_control *ctrl) +static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_controller *ctrl) { return container_of(ctrl, struct sunxi_nfc, controller); } diff --git a/drivers/mtd/nand/raw/tango_nand.c b/drivers/mtd/nand/raw/tango_nand.c index f2052fae21c7..94c8e9ac20b6 100644 --- a/drivers/mtd/nand/raw/tango_nand.c +++ b/drivers/mtd/nand/raw/tango_nand.c @@ -83,7 +83,7 @@ #define MAX_CS 4 struct tango_nfc { - struct nand_hw_control hw; + struct nand_controller hw; void __iomem *reg_base; void __iomem *mem_base; void __iomem *pbus_base; diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c index 22d6a7f9ff80..1d5a646ca1e5 100644 --- a/drivers/mtd/nand/raw/tegra_nand.c +++ b/drivers/mtd/nand/raw/tegra_nand.c @@ -164,7 +164,7 @@ HWSTATUS_RBSY_VALUE(NAND_STATUS_READY)) struct tegra_nand_controller { - struct nand_hw_control controller; + struct nand_controller controller; struct device *dev; void __iomem *regs; int irq; @@ -187,7 +187,7 @@ struct tegra_nand_chip { }; static inline struct tegra_nand_controller * - to_tegra_ctrl(struct nand_hw_control *hw_ctrl) + to_tegra_ctrl(struct nand_controller *hw_ctrl) { return container_of(hw_ctrl, struct tegra_nand_controller, controller); } diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c index b567d212fe7d..89ec8d4c4e83 100644 --- a/drivers/mtd/nand/raw/txx9ndfmc.c +++ b/drivers/mtd/nand/raw/txx9ndfmc.c @@ -73,7 +73,7 @@ struct txx9ndfmc_drvdata { void __iomem *base; unsigned char hold; /* in gbusclock */ unsigned char spw; /* in gbusclock */ - struct nand_hw_control hw_control; + struct nand_controller hw_control; }; static struct platform_device *mtd_to_platdev(struct mtd_info *mtd) diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index e383c7f32574..cfe39fc18417 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -510,20 +510,21 @@ struct nand_id { }; /** - * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independent devices + * struct nand_controller - Control structure for hardware controller + * shared among independent devices * @lock: protection lock * @active: the mtd device which holds the controller currently * @wq: wait queue to sleep on if a NAND operation is in * progress used instead of the per chip wait queue * when a hw controller is available. */ -struct nand_hw_control { +struct nand_controller { spinlock_t lock; struct nand_chip *active; wait_queue_head_t wq; }; -static inline void nand_hw_control_init(struct nand_hw_control *nfc) +static inline void nand_hw_control_init(struct nand_controller *nfc) { nfc->active = NULL; spin_lock_init(&nfc->lock); @@ -1334,11 +1335,11 @@ struct nand_chip { flstate_t state; uint8_t *oob_poi; - struct nand_hw_control *controller; + struct nand_controller *controller; struct nand_ecc_ctrl ecc; unsigned long buf_align; - struct nand_hw_control hwcontrol; + struct nand_controller hwcontrol; uint8_t *bbt; struct nand_bbt_descr *bbt_td; -- 2.14.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure 2018-07-17 9:54 ` [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure Miquel Raynal @ 2018-07-17 11:49 ` Boris Brezillon 2018-07-17 11:58 ` Miquel Raynal 0 siblings, 1 reply; 12+ messages in thread From: Boris Brezillon @ 2018-07-17 11:49 UTC (permalink / raw) To: Miquel Raynal Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd On Tue, 17 Jul 2018 11:54:52 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > In the raw NAND core, a NAND chip is described by a nand_chip structure, > while a NAND controller is described with a nand_hw_control structure > which is not very meaningful. 'control' is misleading and 'hw' has no > meaning here has it refers to hardware ECC operations only, I don't think hw refers to hardware ECC, but I agree that nand_controller is a better name for this structure. How about dropping the last sentence? > which is too > restrictive. > > Rename this structure nand_controller. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Acked-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/nand/raw/atmel/nand-controller.c | 8 ++++---- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- > drivers/mtd/nand/raw/fsl_elbc_nand.c | 2 +- > drivers/mtd/nand/raw/fsl_ifc_nand.c | 2 +- > drivers/mtd/nand/raw/jz4780_nand.c | 5 +++-- > drivers/mtd/nand/raw/marvell_nand.c | 4 ++-- > drivers/mtd/nand/raw/mtk_nand.c | 2 +- > drivers/mtd/nand/raw/ndfc.c | 2 +- > drivers/mtd/nand/raw/omap2.c | 2 +- > drivers/mtd/nand/raw/oxnas_nand.c | 2 +- > drivers/mtd/nand/raw/qcom_nandc.c | 2 +- > drivers/mtd/nand/raw/s3c2410.c | 2 +- > drivers/mtd/nand/raw/sunxi_nand.c | 4 ++-- > drivers/mtd/nand/raw/tango_nand.c | 2 +- > drivers/mtd/nand/raw/tegra_nand.c | 4 ++-- > drivers/mtd/nand/raw/txx9ndfmc.c | 2 +- > include/linux/mtd/rawnand.h | 11 ++++++----- > 17 files changed, 30 insertions(+), 28 deletions(-) > > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c > index e686fe73159e..8fcb66df10ea 100644 > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c > @@ -216,7 +216,7 @@ struct atmel_nand_controller_caps { > }; > > struct atmel_nand_controller { > - struct nand_hw_control base; > + struct nand_controller base; > const struct atmel_nand_controller_caps *caps; > struct device *dev; > struct regmap *smc; > @@ -227,7 +227,7 @@ struct atmel_nand_controller { > }; > > static inline struct atmel_nand_controller * > -to_nand_controller(struct nand_hw_control *ctl) > +to_nand_controller(struct nand_controller *ctl) > { > return container_of(ctl, struct atmel_nand_controller, base); > } > @@ -239,7 +239,7 @@ struct atmel_smc_nand_controller { > }; > > static inline struct atmel_smc_nand_controller * > -to_smc_nand_controller(struct nand_hw_control *ctl) > +to_smc_nand_controller(struct nand_controller *ctl) > { > return container_of(to_nand_controller(ctl), > struct atmel_smc_nand_controller, base); > @@ -263,7 +263,7 @@ struct atmel_hsmc_nand_controller { > }; > > static inline struct atmel_hsmc_nand_controller * > -to_hsmc_nand_controller(struct nand_hw_control *ctl) > +to_hsmc_nand_controller(struct nand_controller *ctl) > { > return container_of(to_nand_controller(ctl), > struct atmel_hsmc_nand_controller, base); > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index 1306aaa7a8bf..2c8e3d3977c0 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -114,7 +114,7 @@ enum { > > struct brcmnand_controller { > struct device *dev; > - struct nand_hw_control controller; > + struct nand_controller controller; > void __iomem *nand_base; > void __iomem *nand_fc; /* flash cache */ > void __iomem *flash_dma_base; > diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c > index 51f0b340bc0d..756431ec055b 100644 > --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c > +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c > @@ -61,7 +61,7 @@ struct fsl_elbc_mtd { > /* Freescale eLBC FCM controller information */ > > struct fsl_elbc_fcm_ctrl { > - struct nand_hw_control controller; > + struct nand_controller controller; > struct fsl_elbc_mtd *chips[MAX_BANKS]; > > u8 __iomem *addr; /* Address of assigned FCM buffer */ > diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c > index 382b67e97174..92b8b63a8b75 100644 > --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c > +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c > @@ -51,7 +51,7 @@ struct fsl_ifc_mtd { > > /* overview of the fsl ifc controller */ > struct fsl_ifc_nand_ctrl { > - struct nand_hw_control controller; > + struct nand_controller controller; > struct fsl_ifc_mtd *chips[FSL_IFC_BANK_COUNT]; > > void __iomem *addr; /* Address of assigned IFC buffer */ > diff --git a/drivers/mtd/nand/raw/jz4780_nand.c b/drivers/mtd/nand/raw/jz4780_nand.c > index e69f6ae4c539..b9afd41132ac 100644 > --- a/drivers/mtd/nand/raw/jz4780_nand.c > +++ b/drivers/mtd/nand/raw/jz4780_nand.c > @@ -44,7 +44,7 @@ struct jz4780_nand_cs { > struct jz4780_nand_controller { > struct device *dev; > struct jz4780_bch *bch; > - struct nand_hw_control controller; > + struct nand_controller controller; > unsigned int num_banks; > struct list_head chips; > int selected; > @@ -65,7 +65,8 @@ static inline struct jz4780_nand_chip *to_jz4780_nand_chip(struct mtd_info *mtd) > return container_of(mtd_to_nand(mtd), struct jz4780_nand_chip, chip); > } > > -static inline struct jz4780_nand_controller *to_jz4780_nand_controller(struct nand_hw_control *ctrl) > +static inline struct jz4780_nand_controller > +*to_jz4780_nand_controller(struct nand_controller *ctrl) > { > return container_of(ctrl, struct jz4780_nand_controller, controller); > } > diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c > index 80a074cccb82..fa7d284ac8af 100644 > --- a/drivers/mtd/nand/raw/marvell_nand.c > +++ b/drivers/mtd/nand/raw/marvell_nand.c > @@ -318,7 +318,7 @@ struct marvell_nfc_caps { > * @dma_buf: 32-bit aligned buffer for DMA transfers (NFCv1 only) > */ > struct marvell_nfc { > - struct nand_hw_control controller; > + struct nand_controller controller; > struct device *dev; > void __iomem *regs; > struct clk *core_clk; > @@ -335,7 +335,7 @@ struct marvell_nfc { > u8 *dma_buf; > }; > > -static inline struct marvell_nfc *to_marvell_nfc(struct nand_hw_control *ctrl) > +static inline struct marvell_nfc *to_marvell_nfc(struct nand_controller *ctrl) > { > return container_of(ctrl, struct marvell_nfc, controller); > } > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c > index 75c845adb050..254ee76d354d 100644 > --- a/drivers/mtd/nand/raw/mtk_nand.c > +++ b/drivers/mtd/nand/raw/mtk_nand.c > @@ -145,7 +145,7 @@ struct mtk_nfc_clk { > }; > > struct mtk_nfc { > - struct nand_hw_control controller; > + struct nand_controller controller; > struct mtk_ecc_config ecc_cfg; > struct mtk_nfc_clk clk; > struct mtk_ecc *ecc; > diff --git a/drivers/mtd/nand/raw/ndfc.c b/drivers/mtd/nand/raw/ndfc.c > index d8a806894937..fdc55fda5db6 100644 > --- a/drivers/mtd/nand/raw/ndfc.c > +++ b/drivers/mtd/nand/raw/ndfc.c > @@ -39,7 +39,7 @@ struct ndfc_controller { > void __iomem *ndfcbase; > struct nand_chip chip; > int chip_select; > - struct nand_hw_control ndfc_control; > + struct nand_controller ndfc_control; > }; > > static struct ndfc_controller ndfc_ctrl[NDFC_MAX_CS]; > diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c > index e50c64adc3c8..e943b2e5a5e2 100644 > --- a/drivers/mtd/nand/raw/omap2.c > +++ b/drivers/mtd/nand/raw/omap2.c > @@ -145,7 +145,7 @@ static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc, > static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10}; > > /* Shared among all NAND instances to synchronize access to the ECC Engine */ > -static struct nand_hw_control omap_gpmc_controller = { > +static struct nand_controller omap_gpmc_controller = { > .lock = __SPIN_LOCK_UNLOCKED(omap_gpmc_controller.lock), > .wq = __WAIT_QUEUE_HEAD_INITIALIZER(omap_gpmc_controller.wq), > }; > diff --git a/drivers/mtd/nand/raw/oxnas_nand.c b/drivers/mtd/nand/raw/oxnas_nand.c > index d649d5944826..3621c29e7221 100644 > --- a/drivers/mtd/nand/raw/oxnas_nand.c > +++ b/drivers/mtd/nand/raw/oxnas_nand.c > @@ -32,7 +32,7 @@ > #define OXNAS_NAND_MAX_CHIPS 1 > > struct oxnas_nand_ctrl { > - struct nand_hw_control base; > + struct nand_controller base; > void __iomem *io_base; > struct clk *clk; > struct nand_chip *chips[OXNAS_NAND_MAX_CHIPS]; > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c > index 994f980c6d86..98800a26b2c7 100644 > --- a/drivers/mtd/nand/raw/qcom_nandc.c > +++ b/drivers/mtd/nand/raw/qcom_nandc.c > @@ -365,7 +365,7 @@ struct nandc_regs { > * from all connected NAND devices pagesize > */ > struct qcom_nand_controller { > - struct nand_hw_control controller; > + struct nand_controller controller; > struct list_head host_list; > > struct device *dev; > diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c > index 19661c5d3220..aee043dd4fa9 100644 > --- a/drivers/mtd/nand/raw/s3c2410.c > +++ b/drivers/mtd/nand/raw/s3c2410.c > @@ -162,7 +162,7 @@ enum s3c_nand_clk_state { > */ > struct s3c2410_nand_info { > /* mtd info */ > - struct nand_hw_control controller; > + struct nand_controller controller; > struct s3c2410_nand_mtd *mtds; > struct s3c2410_platform_nand *platform; > > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c > index d831a141a196..de19c81a1789 100644 > --- a/drivers/mtd/nand/raw/sunxi_nand.c > +++ b/drivers/mtd/nand/raw/sunxi_nand.c > @@ -234,7 +234,7 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand) > * controller events > */ > struct sunxi_nfc { > - struct nand_hw_control controller; > + struct nand_controller controller; > struct device *dev; > void __iomem *regs; > struct clk *ahb_clk; > @@ -247,7 +247,7 @@ struct sunxi_nfc { > struct dma_chan *dmac; > }; > > -static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_hw_control *ctrl) > +static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_controller *ctrl) > { > return container_of(ctrl, struct sunxi_nfc, controller); > } > diff --git a/drivers/mtd/nand/raw/tango_nand.c b/drivers/mtd/nand/raw/tango_nand.c > index f2052fae21c7..94c8e9ac20b6 100644 > --- a/drivers/mtd/nand/raw/tango_nand.c > +++ b/drivers/mtd/nand/raw/tango_nand.c > @@ -83,7 +83,7 @@ > #define MAX_CS 4 > > struct tango_nfc { > - struct nand_hw_control hw; > + struct nand_controller hw; > void __iomem *reg_base; > void __iomem *mem_base; > void __iomem *pbus_base; > diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c > index 22d6a7f9ff80..1d5a646ca1e5 100644 > --- a/drivers/mtd/nand/raw/tegra_nand.c > +++ b/drivers/mtd/nand/raw/tegra_nand.c > @@ -164,7 +164,7 @@ > HWSTATUS_RBSY_VALUE(NAND_STATUS_READY)) > > struct tegra_nand_controller { > - struct nand_hw_control controller; > + struct nand_controller controller; > struct device *dev; > void __iomem *regs; > int irq; > @@ -187,7 +187,7 @@ struct tegra_nand_chip { > }; > > static inline struct tegra_nand_controller * > - to_tegra_ctrl(struct nand_hw_control *hw_ctrl) > + to_tegra_ctrl(struct nand_controller *hw_ctrl) > { > return container_of(hw_ctrl, struct tegra_nand_controller, controller); > } > diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c > index b567d212fe7d..89ec8d4c4e83 100644 > --- a/drivers/mtd/nand/raw/txx9ndfmc.c > +++ b/drivers/mtd/nand/raw/txx9ndfmc.c > @@ -73,7 +73,7 @@ struct txx9ndfmc_drvdata { > void __iomem *base; > unsigned char hold; /* in gbusclock */ > unsigned char spw; /* in gbusclock */ > - struct nand_hw_control hw_control; > + struct nand_controller hw_control; > }; > > static struct platform_device *mtd_to_platdev(struct mtd_info *mtd) > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index e383c7f32574..cfe39fc18417 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -510,20 +510,21 @@ struct nand_id { > }; > > /** > - * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independent devices > + * struct nand_controller - Control structure for hardware controller > + * shared among independent devices > * @lock: protection lock > * @active: the mtd device which holds the controller currently > * @wq: wait queue to sleep on if a NAND operation is in > * progress used instead of the per chip wait queue > * when a hw controller is available. > */ > -struct nand_hw_control { > +struct nand_controller { > spinlock_t lock; > struct nand_chip *active; > wait_queue_head_t wq; > }; > > -static inline void nand_hw_control_init(struct nand_hw_control *nfc) > +static inline void nand_hw_control_init(struct nand_controller *nfc) > { > nfc->active = NULL; > spin_lock_init(&nfc->lock); > @@ -1334,11 +1335,11 @@ struct nand_chip { > flstate_t state; > > uint8_t *oob_poi; > - struct nand_hw_control *controller; > + struct nand_controller *controller; > > struct nand_ecc_ctrl ecc; > unsigned long buf_align; > - struct nand_hw_control hwcontrol; > + struct nand_controller hwcontrol; > > uint8_t *bbt; > struct nand_bbt_descr *bbt_td; ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure 2018-07-17 11:49 ` Boris Brezillon @ 2018-07-17 11:58 ` Miquel Raynal 2018-07-17 12:05 ` Boris Brezillon 0 siblings, 1 reply; 12+ messages in thread From: Miquel Raynal @ 2018-07-17 11:58 UTC (permalink / raw) To: Boris Brezillon Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd Hi Boris, Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 17 Jul 2018 13:49:50 +0200: > On Tue, 17 Jul 2018 11:54:52 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > In the raw NAND core, a NAND chip is described by a nand_chip structure, > > while a NAND controller is described with a nand_hw_control structure > > which is not very meaningful. 'control' is misleading and 'hw' has no > > meaning here has it refers to hardware ECC operations only, > > I don't think hw refers to hardware ECC, but I agree that > nand_controller is a better name for this structure. > > How about dropping the last sentence? Do you mean, removing "'control' is misleading [...] which is too restrictive" or "Rename this structure nand_controller" ? > > > which is too > > restrictive. > > > > Rename this structure nand_controller. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > Acked-by: Boris Brezillon <boris.brezillon@bootlin.com> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure 2018-07-17 11:58 ` Miquel Raynal @ 2018-07-17 12:05 ` Boris Brezillon 0 siblings, 0 replies; 12+ messages in thread From: Boris Brezillon @ 2018-07-17 12:05 UTC (permalink / raw) To: Miquel Raynal Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd On Tue, 17 Jul 2018 13:58:37 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Hi Boris, > > Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 17 Jul 2018 > 13:49:50 +0200: > > > On Tue, 17 Jul 2018 11:54:52 +0200 > > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > In the raw NAND core, a NAND chip is described by a nand_chip structure, > > > while a NAND controller is described with a nand_hw_control structure > > > which is not very meaningful. 'control' is misleading and 'hw' has no > > > meaning here has it refers to hardware ECC operations only, > > > > I don't think hw refers to hardware ECC, but I agree that > > nand_controller is a better name for this structure. > > > > How about dropping the last sentence? > > Do you mean, removing > "'control' is misleading [...] which is too restrictive" > or > "Rename this structure nand_controller" > ? The former. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 2/3] mtd: rawnand: update the controller structure initialization function name 2018-07-17 9:54 [RFC PATCH 0/3] Changes in the internal raw NAND API Miquel Raynal 2018-07-17 9:54 ` [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure Miquel Raynal @ 2018-07-17 9:54 ` Miquel Raynal 2018-07-17 11:51 ` Boris Brezillon 2018-07-17 9:54 ` [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() Miquel Raynal 2 siblings, 1 reply; 12+ messages in thread From: Miquel Raynal @ 2018-07-17 9:54 UTC (permalink / raw) To: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut Cc: linux-mtd, Miquel Raynal After the rename of the structure nand_hw_control -> nand_controller, let's also rename the core function initializing it from nand_hw_control_init() to nand_controller_init(). Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/atmel/nand-controller.c | 2 +- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- drivers/mtd/nand/raw/docg4.c | 2 +- drivers/mtd/nand/raw/fsl_elbc_nand.c | 2 +- drivers/mtd/nand/raw/fsl_ifc_nand.c | 2 +- drivers/mtd/nand/raw/jz4780_nand.c | 2 +- drivers/mtd/nand/raw/marvell_nand.c | 2 +- drivers/mtd/nand/raw/nand_base.c | 2 +- drivers/mtd/nand/raw/ndfc.c | 2 +- drivers/mtd/nand/raw/oxnas_nand.c | 2 +- drivers/mtd/nand/raw/qcom_nandc.c | 2 +- drivers/mtd/nand/raw/s3c2410.c | 2 +- drivers/mtd/nand/raw/sunxi_nand.c | 2 +- drivers/mtd/nand/raw/tango_nand.c | 2 +- drivers/mtd/nand/raw/tegra_nand.c | 2 +- drivers/mtd/nand/raw/txx9ndfmc.c | 2 +- include/linux/mtd/rawnand.h | 2 +- 17 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c index 8fcb66df10ea..1a9b871d01c9 100644 --- a/drivers/mtd/nand/raw/atmel/nand-controller.c +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c @@ -1966,7 +1966,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc, struct device_node *np = dev->of_node; int ret; - nand_hw_control_init(&nc->base); + nand_controller_init(&nc->base); INIT_LIST_HEAD(&nc->chips); nc->dev = dev; nc->caps = caps; diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 2c8e3d3977c0..2e5efa0f9ea2 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2433,7 +2433,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) init_completion(&ctrl->done); init_completion(&ctrl->dma_done); - nand_hw_control_init(&ctrl->controller); + nand_controller_init(&ctrl->controller); INIT_LIST_HEAD(&ctrl->host_list); /* NAND register range */ diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c index bb96cb33cd6b..02778292e1a6 100644 --- a/drivers/mtd/nand/raw/docg4.c +++ b/drivers/mtd/nand/raw/docg4.c @@ -1258,7 +1258,7 @@ static void __init init_mtd_structs(struct mtd_info *mtd) nand->options = NAND_BUSWIDTH_16 | NAND_NO_SUBPAGE_WRITE; nand->IO_ADDR_R = nand->IO_ADDR_W = doc->virtadr + DOC_IOSPACE_DATA; nand->controller = &nand->hwcontrol; - nand_hw_control_init(nand->controller); + nand_controller_init(nand->controller); /* methods */ nand->cmdfunc = docg4_command; diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c index 756431ec055b..0aa54a949653 100644 --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c @@ -879,7 +879,7 @@ static int fsl_elbc_nand_probe(struct platform_device *pdev) } elbc_fcm_ctrl->counter++; - nand_hw_control_init(&elbc_fcm_ctrl->controller); + nand_controller_init(&elbc_fcm_ctrl->controller); fsl_lbc_ctrl_dev->nand = elbc_fcm_ctrl; } else { elbc_fcm_ctrl = fsl_lbc_ctrl_dev->nand; diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c index 92b8b63a8b75..73a5245bddd1 100644 --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c @@ -1004,7 +1004,7 @@ static int fsl_ifc_nand_probe(struct platform_device *dev) ifc_nand_ctrl->addr = NULL; fsl_ifc_ctrl_dev->nand = ifc_nand_ctrl; - nand_hw_control_init(&ifc_nand_ctrl->controller); + nand_controller_init(&ifc_nand_ctrl->controller); } else { ifc_nand_ctrl = fsl_ifc_ctrl_dev->nand; } diff --git a/drivers/mtd/nand/raw/jz4780_nand.c b/drivers/mtd/nand/raw/jz4780_nand.c index b9afd41132ac..49841dad347c 100644 --- a/drivers/mtd/nand/raw/jz4780_nand.c +++ b/drivers/mtd/nand/raw/jz4780_nand.c @@ -369,7 +369,7 @@ static int jz4780_nand_probe(struct platform_device *pdev) nfc->dev = dev; nfc->num_banks = num_banks; - nand_hw_control_init(&nfc->controller); + nand_controller_init(&nfc->controller); INIT_LIST_HEAD(&nfc->chips); ret = jz4780_nand_init_chips(nfc, pdev); diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c index fa7d284ac8af..bd5f9a4b7b16 100644 --- a/drivers/mtd/nand/raw/marvell_nand.c +++ b/drivers/mtd/nand/raw/marvell_nand.c @@ -2745,7 +2745,7 @@ static int marvell_nfc_probe(struct platform_device *pdev) return -ENOMEM; nfc->dev = dev; - nand_hw_control_init(&nfc->controller); + nand_controller_init(&nfc->controller); INIT_LIST_HEAD(&nfc->chips); r = platform_get_resource(pdev, IORESOURCE_MEM, 0); diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 4fa5e20d9690..e21f03ee3251 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -4968,7 +4968,7 @@ static void nand_set_defaults(struct nand_chip *chip) if (!chip->controller) { chip->controller = &chip->hwcontrol; - nand_hw_control_init(chip->controller); + nand_controller_init(chip->controller); } if (!chip->buf_align) diff --git a/drivers/mtd/nand/raw/ndfc.c b/drivers/mtd/nand/raw/ndfc.c index fdc55fda5db6..540fa1a0ea24 100644 --- a/drivers/mtd/nand/raw/ndfc.c +++ b/drivers/mtd/nand/raw/ndfc.c @@ -218,7 +218,7 @@ static int ndfc_probe(struct platform_device *ofdev) ndfc = &ndfc_ctrl[cs]; ndfc->chip_select = cs; - nand_hw_control_init(&ndfc->ndfc_control); + nand_controller_init(&ndfc->ndfc_control); ndfc->ofdev = ofdev; dev_set_drvdata(&ofdev->dev, ndfc); diff --git a/drivers/mtd/nand/raw/oxnas_nand.c b/drivers/mtd/nand/raw/oxnas_nand.c index 3621c29e7221..01b00bb69c1e 100644 --- a/drivers/mtd/nand/raw/oxnas_nand.c +++ b/drivers/mtd/nand/raw/oxnas_nand.c @@ -96,7 +96,7 @@ static int oxnas_nand_probe(struct platform_device *pdev) if (!oxnas) return -ENOMEM; - nand_hw_control_init(&oxnas->base); + nand_controller_init(&oxnas->base); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); oxnas->io_base = devm_ioremap_resource(&pdev->dev, res); diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c index 98800a26b2c7..cbad0980d890 100644 --- a/drivers/mtd/nand/raw/qcom_nandc.c +++ b/drivers/mtd/nand/raw/qcom_nandc.c @@ -2728,7 +2728,7 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) INIT_LIST_HEAD(&nandc->desc_list); INIT_LIST_HEAD(&nandc->host_list); - nand_hw_control_init(&nandc->controller); + nand_controller_init(&nandc->controller); return 0; } diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c index aee043dd4fa9..f7a5fc7daa57 100644 --- a/drivers/mtd/nand/raw/s3c2410.c +++ b/drivers/mtd/nand/raw/s3c2410.c @@ -1094,7 +1094,7 @@ static int s3c24xx_nand_probe(struct platform_device *pdev) platform_set_drvdata(pdev, info); - nand_hw_control_init(&info->controller); + nand_controller_init(&info->controller); /* get the clock source and enable it */ diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c index de19c81a1789..41bd66a9166a 100644 --- a/drivers/mtd/nand/raw/sunxi_nand.c +++ b/drivers/mtd/nand/raw/sunxi_nand.c @@ -2012,7 +2012,7 @@ static int sunxi_nfc_probe(struct platform_device *pdev) return -ENOMEM; nfc->dev = dev; - nand_hw_control_init(&nfc->controller); + nand_controller_init(&nfc->controller); INIT_LIST_HEAD(&nfc->chips); r = platform_get_resource(pdev, IORESOURCE_MEM, 0); diff --git a/drivers/mtd/nand/raw/tango_nand.c b/drivers/mtd/nand/raw/tango_nand.c index 94c8e9ac20b6..dd7a26efdf4f 100644 --- a/drivers/mtd/nand/raw/tango_nand.c +++ b/drivers/mtd/nand/raw/tango_nand.c @@ -654,7 +654,7 @@ static int tango_nand_probe(struct platform_device *pdev) return PTR_ERR(nfc->chan); platform_set_drvdata(pdev, nfc); - nand_hw_control_init(&nfc->hw); + nand_controller_init(&nfc->hw); nfc->freq_kHz = clk_get_rate(clk) / 1000; for_each_child_of_node(pdev->dev.of_node, np) { diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c index 1d5a646ca1e5..06402c0cbd32 100644 --- a/drivers/mtd/nand/raw/tegra_nand.c +++ b/drivers/mtd/nand/raw/tegra_nand.c @@ -1136,7 +1136,7 @@ static int tegra_nand_probe(struct platform_device *pdev) return -ENOMEM; ctrl->dev = &pdev->dev; - nand_hw_control_init(&ctrl->controller); + nand_controller_init(&ctrl->controller); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); ctrl->regs = devm_ioremap_resource(&pdev->dev, res); diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c index 89ec8d4c4e83..dd232d8740a2 100644 --- a/drivers/mtd/nand/raw/txx9ndfmc.c +++ b/drivers/mtd/nand/raw/txx9ndfmc.c @@ -303,7 +303,7 @@ static int __init txx9ndfmc_probe(struct platform_device *dev) dev_info(&dev->dev, "CLK:%ldMHz HOLD:%d SPW:%d\n", (gbusclk + 500000) / 1000000, hold, spw); - nand_hw_control_init(&drvdata->hw_control); + nand_controller_init(&drvdata->hw_control); platform_set_drvdata(dev, drvdata); txx9ndfmc_initialize(dev); diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index cfe39fc18417..f5fb9caf6dca 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -524,7 +524,7 @@ struct nand_controller { wait_queue_head_t wq; }; -static inline void nand_hw_control_init(struct nand_controller *nfc) +static inline void nand_controller_init(struct nand_controller *nfc) { nfc->active = NULL; spin_lock_init(&nfc->lock); -- 2.14.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 2/3] mtd: rawnand: update the controller structure initialization function name 2018-07-17 9:54 ` [RFC PATCH 2/3] mtd: rawnand: update the controller structure initialization function name Miquel Raynal @ 2018-07-17 11:51 ` Boris Brezillon 2018-07-17 12:01 ` Miquel Raynal 0 siblings, 1 reply; 12+ messages in thread From: Boris Brezillon @ 2018-07-17 11:51 UTC (permalink / raw) To: Miquel Raynal Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd On Tue, 17 Jul 2018 11:54:53 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > After the rename of the structure nand_hw_control -> nand_controller, > let's also rename the core function initializing it from > nand_hw_control_init() to nand_controller_init(). > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> I think this should go with patch 1, no need to split that in 2 patches IMO. Other than that, I'm okay with the change, so my ack on the first patch stands. > --- > drivers/mtd/nand/raw/atmel/nand-controller.c | 2 +- > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +- > drivers/mtd/nand/raw/docg4.c | 2 +- > drivers/mtd/nand/raw/fsl_elbc_nand.c | 2 +- > drivers/mtd/nand/raw/fsl_ifc_nand.c | 2 +- > drivers/mtd/nand/raw/jz4780_nand.c | 2 +- > drivers/mtd/nand/raw/marvell_nand.c | 2 +- > drivers/mtd/nand/raw/nand_base.c | 2 +- > drivers/mtd/nand/raw/ndfc.c | 2 +- > drivers/mtd/nand/raw/oxnas_nand.c | 2 +- > drivers/mtd/nand/raw/qcom_nandc.c | 2 +- > drivers/mtd/nand/raw/s3c2410.c | 2 +- > drivers/mtd/nand/raw/sunxi_nand.c | 2 +- > drivers/mtd/nand/raw/tango_nand.c | 2 +- > drivers/mtd/nand/raw/tegra_nand.c | 2 +- > drivers/mtd/nand/raw/txx9ndfmc.c | 2 +- > include/linux/mtd/rawnand.h | 2 +- > 17 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c > index 8fcb66df10ea..1a9b871d01c9 100644 > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c > @@ -1966,7 +1966,7 @@ static int atmel_nand_controller_init(struct atmel_nand_controller *nc, > struct device_node *np = dev->of_node; > int ret; > > - nand_hw_control_init(&nc->base); > + nand_controller_init(&nc->base); > INIT_LIST_HEAD(&nc->chips); > nc->dev = dev; > nc->caps = caps; > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > index 2c8e3d3977c0..2e5efa0f9ea2 100644 > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c > @@ -2433,7 +2433,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) > > init_completion(&ctrl->done); > init_completion(&ctrl->dma_done); > - nand_hw_control_init(&ctrl->controller); > + nand_controller_init(&ctrl->controller); > INIT_LIST_HEAD(&ctrl->host_list); > > /* NAND register range */ > diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c > index bb96cb33cd6b..02778292e1a6 100644 > --- a/drivers/mtd/nand/raw/docg4.c > +++ b/drivers/mtd/nand/raw/docg4.c > @@ -1258,7 +1258,7 @@ static void __init init_mtd_structs(struct mtd_info *mtd) > nand->options = NAND_BUSWIDTH_16 | NAND_NO_SUBPAGE_WRITE; > nand->IO_ADDR_R = nand->IO_ADDR_W = doc->virtadr + DOC_IOSPACE_DATA; > nand->controller = &nand->hwcontrol; > - nand_hw_control_init(nand->controller); > + nand_controller_init(nand->controller); > > /* methods */ > nand->cmdfunc = docg4_command; > diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c > index 756431ec055b..0aa54a949653 100644 > --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c > +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c > @@ -879,7 +879,7 @@ static int fsl_elbc_nand_probe(struct platform_device *pdev) > } > elbc_fcm_ctrl->counter++; > > - nand_hw_control_init(&elbc_fcm_ctrl->controller); > + nand_controller_init(&elbc_fcm_ctrl->controller); > fsl_lbc_ctrl_dev->nand = elbc_fcm_ctrl; > } else { > elbc_fcm_ctrl = fsl_lbc_ctrl_dev->nand; > diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c > index 92b8b63a8b75..73a5245bddd1 100644 > --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c > +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c > @@ -1004,7 +1004,7 @@ static int fsl_ifc_nand_probe(struct platform_device *dev) > ifc_nand_ctrl->addr = NULL; > fsl_ifc_ctrl_dev->nand = ifc_nand_ctrl; > > - nand_hw_control_init(&ifc_nand_ctrl->controller); > + nand_controller_init(&ifc_nand_ctrl->controller); > } else { > ifc_nand_ctrl = fsl_ifc_ctrl_dev->nand; > } > diff --git a/drivers/mtd/nand/raw/jz4780_nand.c b/drivers/mtd/nand/raw/jz4780_nand.c > index b9afd41132ac..49841dad347c 100644 > --- a/drivers/mtd/nand/raw/jz4780_nand.c > +++ b/drivers/mtd/nand/raw/jz4780_nand.c > @@ -369,7 +369,7 @@ static int jz4780_nand_probe(struct platform_device *pdev) > nfc->dev = dev; > nfc->num_banks = num_banks; > > - nand_hw_control_init(&nfc->controller); > + nand_controller_init(&nfc->controller); > INIT_LIST_HEAD(&nfc->chips); > > ret = jz4780_nand_init_chips(nfc, pdev); > diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c > index fa7d284ac8af..bd5f9a4b7b16 100644 > --- a/drivers/mtd/nand/raw/marvell_nand.c > +++ b/drivers/mtd/nand/raw/marvell_nand.c > @@ -2745,7 +2745,7 @@ static int marvell_nfc_probe(struct platform_device *pdev) > return -ENOMEM; > > nfc->dev = dev; > - nand_hw_control_init(&nfc->controller); > + nand_controller_init(&nfc->controller); > INIT_LIST_HEAD(&nfc->chips); > > r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 4fa5e20d9690..e21f03ee3251 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -4968,7 +4968,7 @@ static void nand_set_defaults(struct nand_chip *chip) > > if (!chip->controller) { > chip->controller = &chip->hwcontrol; > - nand_hw_control_init(chip->controller); > + nand_controller_init(chip->controller); > } > > if (!chip->buf_align) > diff --git a/drivers/mtd/nand/raw/ndfc.c b/drivers/mtd/nand/raw/ndfc.c > index fdc55fda5db6..540fa1a0ea24 100644 > --- a/drivers/mtd/nand/raw/ndfc.c > +++ b/drivers/mtd/nand/raw/ndfc.c > @@ -218,7 +218,7 @@ static int ndfc_probe(struct platform_device *ofdev) > ndfc = &ndfc_ctrl[cs]; > ndfc->chip_select = cs; > > - nand_hw_control_init(&ndfc->ndfc_control); > + nand_controller_init(&ndfc->ndfc_control); > ndfc->ofdev = ofdev; > dev_set_drvdata(&ofdev->dev, ndfc); > > diff --git a/drivers/mtd/nand/raw/oxnas_nand.c b/drivers/mtd/nand/raw/oxnas_nand.c > index 3621c29e7221..01b00bb69c1e 100644 > --- a/drivers/mtd/nand/raw/oxnas_nand.c > +++ b/drivers/mtd/nand/raw/oxnas_nand.c > @@ -96,7 +96,7 @@ static int oxnas_nand_probe(struct platform_device *pdev) > if (!oxnas) > return -ENOMEM; > > - nand_hw_control_init(&oxnas->base); > + nand_controller_init(&oxnas->base); > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > oxnas->io_base = devm_ioremap_resource(&pdev->dev, res); > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c > index 98800a26b2c7..cbad0980d890 100644 > --- a/drivers/mtd/nand/raw/qcom_nandc.c > +++ b/drivers/mtd/nand/raw/qcom_nandc.c > @@ -2728,7 +2728,7 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc) > INIT_LIST_HEAD(&nandc->desc_list); > INIT_LIST_HEAD(&nandc->host_list); > > - nand_hw_control_init(&nandc->controller); > + nand_controller_init(&nandc->controller); > > return 0; > } > diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c > index aee043dd4fa9..f7a5fc7daa57 100644 > --- a/drivers/mtd/nand/raw/s3c2410.c > +++ b/drivers/mtd/nand/raw/s3c2410.c > @@ -1094,7 +1094,7 @@ static int s3c24xx_nand_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, info); > > - nand_hw_control_init(&info->controller); > + nand_controller_init(&info->controller); > > /* get the clock source and enable it */ > > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c > index de19c81a1789..41bd66a9166a 100644 > --- a/drivers/mtd/nand/raw/sunxi_nand.c > +++ b/drivers/mtd/nand/raw/sunxi_nand.c > @@ -2012,7 +2012,7 @@ static int sunxi_nfc_probe(struct platform_device *pdev) > return -ENOMEM; > > nfc->dev = dev; > - nand_hw_control_init(&nfc->controller); > + nand_controller_init(&nfc->controller); > INIT_LIST_HEAD(&nfc->chips); > > r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > diff --git a/drivers/mtd/nand/raw/tango_nand.c b/drivers/mtd/nand/raw/tango_nand.c > index 94c8e9ac20b6..dd7a26efdf4f 100644 > --- a/drivers/mtd/nand/raw/tango_nand.c > +++ b/drivers/mtd/nand/raw/tango_nand.c > @@ -654,7 +654,7 @@ static int tango_nand_probe(struct platform_device *pdev) > return PTR_ERR(nfc->chan); > > platform_set_drvdata(pdev, nfc); > - nand_hw_control_init(&nfc->hw); > + nand_controller_init(&nfc->hw); > nfc->freq_kHz = clk_get_rate(clk) / 1000; > > for_each_child_of_node(pdev->dev.of_node, np) { > diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c > index 1d5a646ca1e5..06402c0cbd32 100644 > --- a/drivers/mtd/nand/raw/tegra_nand.c > +++ b/drivers/mtd/nand/raw/tegra_nand.c > @@ -1136,7 +1136,7 @@ static int tegra_nand_probe(struct platform_device *pdev) > return -ENOMEM; > > ctrl->dev = &pdev->dev; > - nand_hw_control_init(&ctrl->controller); > + nand_controller_init(&ctrl->controller); > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > ctrl->regs = devm_ioremap_resource(&pdev->dev, res); > diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c > index 89ec8d4c4e83..dd232d8740a2 100644 > --- a/drivers/mtd/nand/raw/txx9ndfmc.c > +++ b/drivers/mtd/nand/raw/txx9ndfmc.c > @@ -303,7 +303,7 @@ static int __init txx9ndfmc_probe(struct platform_device *dev) > dev_info(&dev->dev, "CLK:%ldMHz HOLD:%d SPW:%d\n", > (gbusclk + 500000) / 1000000, hold, spw); > > - nand_hw_control_init(&drvdata->hw_control); > + nand_controller_init(&drvdata->hw_control); > > platform_set_drvdata(dev, drvdata); > txx9ndfmc_initialize(dev); > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index cfe39fc18417..f5fb9caf6dca 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -524,7 +524,7 @@ struct nand_controller { > wait_queue_head_t wq; > }; > > -static inline void nand_hw_control_init(struct nand_controller *nfc) > +static inline void nand_controller_init(struct nand_controller *nfc) > { > nfc->active = NULL; > spin_lock_init(&nfc->lock); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 2/3] mtd: rawnand: update the controller structure initialization function name 2018-07-17 11:51 ` Boris Brezillon @ 2018-07-17 12:01 ` Miquel Raynal 0 siblings, 0 replies; 12+ messages in thread From: Miquel Raynal @ 2018-07-17 12:01 UTC (permalink / raw) To: Boris Brezillon Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd Hi Boris, Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 17 Jul 2018 13:51:06 +0200: > On Tue, 17 Jul 2018 11:54:53 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > After the rename of the structure nand_hw_control -> nand_controller, > > let's also rename the core function initializing it from > > nand_hw_control_init() to nand_controller_init(). > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > I think this should go with patch 1, no need to split that in 2 > patches IMO. I wasn't sure of the split. I'll fold them. > > Other than that, I'm okay with the change, so my ack on the first patch > stands. Thanks, Miquèl ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() 2018-07-17 9:54 [RFC PATCH 0/3] Changes in the internal raw NAND API Miquel Raynal 2018-07-17 9:54 ` [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure Miquel Raynal 2018-07-17 9:54 ` [RFC PATCH 2/3] mtd: rawnand: update the controller structure initialization function name Miquel Raynal @ 2018-07-17 9:54 ` Miquel Raynal 2018-07-17 12:03 ` Boris Brezillon 2 siblings, 1 reply; 12+ messages in thread From: Miquel Raynal @ 2018-07-17 9:54 UTC (permalink / raw) To: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut Cc: linux-mtd, Miquel Raynal In order to remove the limitation that forbids dynamic allocation in nand_scan_ident(), we must create a path that will be the same for all controller drivers. The idea is to use nand_scan() instead of the widely implemented nand_scan_ident()/nand_scan_tail() couple. In order to achieve this, controller drivers will need to adjust some parameters between these two functions depending on the NAND chip wired on them. For that, a hook called ->attach_chip() is created in the nand_controller structure. This structure may be referenced by two ways: 1/ if the driver does not implement its own controller, the chip->controller hook is not populated before nand_scan() so it cannot be dereferenced: use chip->hwcontrol instead (which is statically allocated and will be referenced later by chip->controller anyway). 2/ through chip->controller if the driver implements its own controller. Another hook, ->detach_chip() is also introduced in order to clean the controller driver's potential allocations in case of failure of nand_scan_tail(). There is no need for the controller driver to call the ->detach_chip() hook directly upon error after a successful nand_scan(). In this situation, calling nand_release() as before is enough. Both ->attac/detach_chip() hooks are located in a nand_controller_ops structure. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/nand_base.c | 21 +++++++++++++++++++-- include/linux/mtd/rawnand.h | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index e21f03ee3251..5e8278281ba8 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -6710,11 +6710,23 @@ EXPORT_SYMBOL(nand_scan_tail); int nand_scan_with_ids(struct mtd_info *mtd, int maxchips, struct nand_flash_dev *ids) { + struct nand_chip *chip = mtd_to_nand(mtd); int ret; ret = nand_scan_ident(mtd, maxchips, ids); - if (!ret) - ret = nand_scan_tail(mtd); + if (ret) + return ret; + + if (chip->controller->ops && chip->controller->ops->attach_chip) { + ret = chip->controller->ops->attach_chip(chip); + if (ret) + return ret; + } + + ret = nand_scan_tail(mtd); + if (ret && chip->controller->ops && chip->controller->ops->detach_chip) + chip->controller->ops->detach_chip(chip); + return ret; } EXPORT_SYMBOL(nand_scan_with_ids); @@ -6742,7 +6754,12 @@ void nand_cleanup(struct nand_chip *chip) /* Free manufacturer priv data. */ nand_manufacturer_cleanup(chip); + + /* Free controller specific allocations after chip identification */ + if (chip->controller->ops && chip->controller->ops->detach_chip) + chip->controller->ops->detach_chip(chip); } + EXPORT_SYMBOL_GPL(nand_cleanup); /** diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index f5fb9caf6dca..d1f2267ae724 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -509,6 +509,19 @@ struct nand_id { int len; }; +/** + * struct nand_controller_ops - Controller operations + * + * @attach_chip: Callback that may be called between nand_detect() and + * nand_scan_tail() during nand_scan() (optional). + * @detach_chip: Callback that may be called if nand_scan_tail() fails + * (optional). + */ +struct nand_controller_ops { + int (*attach_chip)(struct nand_chip *chip); + void (*detach_chip)(struct nand_chip *chip); +}; + /** * struct nand_controller - Control structure for hardware controller * shared among independent devices @@ -517,11 +530,13 @@ struct nand_id { * @wq: wait queue to sleep on if a NAND operation is in * progress used instead of the per chip wait queue * when a hw controller is available. + * @ops: NAND controller operations. */ struct nand_controller { spinlock_t lock; struct nand_chip *active; wait_queue_head_t wq; + const struct nand_controller_ops *ops; }; static inline void nand_controller_init(struct nand_controller *nfc) @@ -529,6 +544,7 @@ static inline void nand_controller_init(struct nand_controller *nfc) nfc->active = NULL; spin_lock_init(&nfc->lock); init_waitqueue_head(&nfc->wq); + nfc->ops = NULL; } /** -- 2.14.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() 2018-07-17 9:54 ` [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() Miquel Raynal @ 2018-07-17 12:03 ` Boris Brezillon 2018-07-17 12:09 ` Miquel Raynal 0 siblings, 1 reply; 12+ messages in thread From: Boris Brezillon @ 2018-07-17 12:03 UTC (permalink / raw) To: Miquel Raynal Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd On Tue, 17 Jul 2018 11:54:54 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > In order to remove the limitation that forbids dynamic allocation in > nand_scan_ident(), we must create a path that will be the same for all > controller drivers. The idea is to use nand_scan() instead of the widely > implemented nand_scan_ident()/nand_scan_tail() couple. In order to ^ used > achieve this, controller drivers will need to adjust some parameters > between these two functions depending on the NAND chip wired on them. > > For that, a hook called ->attach_chip() is created in the > nand_controller structure. This structure may be referenced by two ways: This is no longer true. Now ->attach_chip() is part of nand_controller_ops, and nand_controller has a pointer to a nand_controller_ops implementation. > 1/ if the driver does not implement its own controller, the > chip->controller hook is not populated before nand_scan() so it > cannot be dereferenced: use chip->hwcontrol instead (which is > statically allocated and will be referenced later by chip->controller > anyway). Not related to this patch, but I'd rename chip->hwcontrol into chip->dummycontroller or something that clearly shows that this field should only be used when the controller driver is dumb and can only control a single chip. > 2/ through chip->controller if the driver implements its own controller. > > Another hook, ->detach_chip() is also introduced in order to clean the > controller driver's potential allocations in case of failure of > nand_scan_tail(). There is no need for the controller driver to call the > ->detach_chip() hook directly upon error after a successful nand_scan(). > In this situation, calling nand_release() as before is enough. > > Both ->attac/detach_chip() hooks are located in a nand_controller_ops > structure. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> The implementation looks good (one minor comment below, but you can ignore it if you disagree), so once you've fixed the commit message you can add Acked-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/nand/raw/nand_base.c | 21 +++++++++++++++++++-- > include/linux/mtd/rawnand.h | 16 ++++++++++++++++ > 2 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index e21f03ee3251..5e8278281ba8 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -6710,11 +6710,23 @@ EXPORT_SYMBOL(nand_scan_tail); > int nand_scan_with_ids(struct mtd_info *mtd, int maxchips, > struct nand_flash_dev *ids) > { > + struct nand_chip *chip = mtd_to_nand(mtd); > int ret; > > ret = nand_scan_ident(mtd, maxchips, ids); > - if (!ret) > - ret = nand_scan_tail(mtd); > + if (ret) > + return ret; > + > + if (chip->controller->ops && chip->controller->ops->attach_chip) { > + ret = chip->controller->ops->attach_chip(chip); > + if (ret) > + return ret; > + } > + > + ret = nand_scan_tail(mtd); > + if (ret && chip->controller->ops && chip->controller->ops->detach_chip) > + chip->controller->ops->detach_chip(chip); I'd recommend creating wrappers for detach/attach operations, just in case you need to automate more things in there: static int nand_attach(struct nand_chip *chip) { if (chip->controller->ops && chip->controller->ops->attach_chip) return chip->controller->ops->attach_chip(chip); return 0; } static void nand_detach(struct nand_chip *chip) { if (chip->controller->ops && chip->controller->ops->detach_chip) chip->controller->ops->detach_chip(chip); } > + > return ret; > } > EXPORT_SYMBOL(nand_scan_with_ids); > @@ -6742,7 +6754,12 @@ void nand_cleanup(struct nand_chip *chip) > > /* Free manufacturer priv data. */ > nand_manufacturer_cleanup(chip); > + > + /* Free controller specific allocations after chip identification */ > + if (chip->controller->ops && chip->controller->ops->detach_chip) > + chip->controller->ops->detach_chip(chip); > } > + > EXPORT_SYMBOL_GPL(nand_cleanup); > > /** > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index f5fb9caf6dca..d1f2267ae724 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -509,6 +509,19 @@ struct nand_id { > int len; > }; > > +/** > + * struct nand_controller_ops - Controller operations > + * > + * @attach_chip: Callback that may be called between nand_detect() and ^nand_scan_ident()? > + * nand_scan_tail() during nand_scan() (optional). > + * @detach_chip: Callback that may be called if nand_scan_tail() fails > + * (optional). > + */ > +struct nand_controller_ops { > + int (*attach_chip)(struct nand_chip *chip); > + void (*detach_chip)(struct nand_chip *chip); > +}; > + > /** > * struct nand_controller - Control structure for hardware controller > * shared among independent devices > @@ -517,11 +530,13 @@ struct nand_id { > * @wq: wait queue to sleep on if a NAND operation is in > * progress used instead of the per chip wait queue > * when a hw controller is available. > + * @ops: NAND controller operations. > */ > struct nand_controller { > spinlock_t lock; > struct nand_chip *active; > wait_queue_head_t wq; > + const struct nand_controller_ops *ops; > }; > > static inline void nand_controller_init(struct nand_controller *nfc) > @@ -529,6 +544,7 @@ static inline void nand_controller_init(struct nand_controller *nfc) > nfc->active = NULL; > spin_lock_init(&nfc->lock); > init_waitqueue_head(&nfc->wq); > + nfc->ops = NULL; > } > > /** ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() 2018-07-17 12:03 ` Boris Brezillon @ 2018-07-17 12:09 ` Miquel Raynal 2018-07-17 12:13 ` Boris Brezillon 0 siblings, 1 reply; 12+ messages in thread From: Miquel Raynal @ 2018-07-17 12:09 UTC (permalink / raw) To: Boris Brezillon Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd Hi Boris, Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 17 Jul 2018 14:03:32 +0200: > On Tue, 17 Jul 2018 11:54:54 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > In order to remove the limitation that forbids dynamic allocation in > > nand_scan_ident(), we must create a path that will be the same for all > > controller drivers. The idea is to use nand_scan() instead of the widely > > implemented nand_scan_ident()/nand_scan_tail() couple. In order to > > ^ used > > > achieve this, controller drivers will need to adjust some parameters > > between these two functions depending on the NAND chip wired on them. > > > > For that, a hook called ->attach_chip() is created in the > > nand_controller structure. This structure may be referenced by two ways: > > This is no longer true. Now ->attach_chip() is part of > nand_controller_ops, and nand_controller has a pointer to a > nand_controller_ops implementation. That's right. > > > 1/ if the driver does not implement its own controller, the > > chip->controller hook is not populated before nand_scan() so it > > cannot be dereferenced: use chip->hwcontrol instead (which is > > statically allocated and will be referenced later by chip->controller > > anyway). > > Not related to this patch, but I'd rename chip->hwcontrol into > chip->dummycontroller or something that clearly shows that this field > should only be used when the controller driver is dumb and can only > control a single chip. I like this idea but was not sure of a good name for it. If you are okay I'll do the change in the same patch renaming nand_hw_control -> nand_controller and nand_hw_control_init -> nand_controller_init. I think it's related enough. > > > 2/ through chip->controller if the driver implements its own controller. > > > > Another hook, ->detach_chip() is also introduced in order to clean the > > controller driver's potential allocations in case of failure of > > nand_scan_tail(). There is no need for the controller driver to call the > > ->detach_chip() hook directly upon error after a successful nand_scan(). > > In this situation, calling nand_release() as before is enough. > > > > Both ->attac/detach_chip() hooks are located in a nand_controller_ops > > structure. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > The implementation looks good (one minor comment below, but you can > ignore it if you disagree), so once you've fixed the commit message you > can add > > Acked-by: Boris Brezillon <boris.brezillon@bootlin.com> > > > --- > > drivers/mtd/nand/raw/nand_base.c | 21 +++++++++++++++++++-- > > include/linux/mtd/rawnand.h | 16 ++++++++++++++++ > > 2 files changed, 35 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > > index e21f03ee3251..5e8278281ba8 100644 > > --- a/drivers/mtd/nand/raw/nand_base.c > > +++ b/drivers/mtd/nand/raw/nand_base.c > > @@ -6710,11 +6710,23 @@ EXPORT_SYMBOL(nand_scan_tail); > > int nand_scan_with_ids(struct mtd_info *mtd, int maxchips, > > struct nand_flash_dev *ids) > > { > > + struct nand_chip *chip = mtd_to_nand(mtd); > > int ret; > > > > ret = nand_scan_ident(mtd, maxchips, ids); > > - if (!ret) > > - ret = nand_scan_tail(mtd); > > + if (ret) > > + return ret; > > + > > + if (chip->controller->ops && chip->controller->ops->attach_chip) { > > + ret = chip->controller->ops->attach_chip(chip); > > + if (ret) > > + return ret; > > + } > > + > > + ret = nand_scan_tail(mtd); > > + if (ret && chip->controller->ops && chip->controller->ops->detach_chip) > > + chip->controller->ops->detach_chip(chip); > > I'd recommend creating wrappers for detach/attach operations, just in case > you need to automate more things in there: > > static int nand_attach(struct nand_chip *chip) > { > if (chip->controller->ops && chip->controller->ops->attach_chip) > return chip->controller->ops->attach_chip(chip); > > return 0; > } > > static void nand_detach(struct nand_chip *chip) > { > if (chip->controller->ops && chip->controller->ops->detach_chip) > chip->controller->ops->detach_chip(chip); > } Even better. I'll add it. Thanks, Miquèl ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() 2018-07-17 12:09 ` Miquel Raynal @ 2018-07-17 12:13 ` Boris Brezillon 0 siblings, 0 replies; 12+ messages in thread From: Boris Brezillon @ 2018-07-17 12:13 UTC (permalink / raw) To: Miquel Raynal Cc: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, linux-mtd On Tue, 17 Jul 2018 14:09:39 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > 1/ if the driver does not implement its own controller, the > > > chip->controller hook is not populated before nand_scan() so it > > > cannot be dereferenced: use chip->hwcontrol instead (which is > > > statically allocated and will be referenced later by chip->controller > > > anyway). > > > > Not related to this patch, but I'd rename chip->hwcontrol into > > chip->dummycontroller or something that clearly shows that this field > > should only be used when the controller driver is dumb and can only > > control a single chip. > > I like this idea but was not sure of a good name for it. If you are > okay I'll do the change in the same patch renaming nand_hw_control -> > nand_controller and nand_hw_control_init -> nand_controller_init. I > think it's related enough. Yep, sounds good. Just remember to mention it in your commit message. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-07-17 12:13 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-17 9:54 [RFC PATCH 0/3] Changes in the internal raw NAND API Miquel Raynal 2018-07-17 9:54 ` [RFC PATCH 1/3] mtd: rawnand: better name for the controller structure Miquel Raynal 2018-07-17 11:49 ` Boris Brezillon 2018-07-17 11:58 ` Miquel Raynal 2018-07-17 12:05 ` Boris Brezillon 2018-07-17 9:54 ` [RFC PATCH 2/3] mtd: rawnand: update the controller structure initialization function name Miquel Raynal 2018-07-17 11:51 ` Boris Brezillon 2018-07-17 12:01 ` Miquel Raynal 2018-07-17 9:54 ` [RFC PATCH 3/3] mtd: rawnand: add hooks that may be called during nand_scan() Miquel Raynal 2018-07-17 12:03 ` Boris Brezillon 2018-07-17 12:09 ` Miquel Raynal 2018-07-17 12:13 ` Boris Brezillon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox