| Message ID | 565610B9.5040407@simon.arlott.org.uk |
|---|---|
| State | Superseded |
| Headers | show |
+ Broadcom list + Kamal Hi Simon, On Wed, Nov 25, 2015 at 07:49:13PM +0000, Simon Arlott wrote: > The BCM63268 has a NAND interrupt register with combined status and enable > registers. It also has a clock for the NAND controller that needs to be > enabled. > > Set up the device by enabling the clock, disabling and acking all > interrupts, then handle the CTRL_READY interrupt. > > Add a brcmnand_get_socdata() function so that bcm63268_nand can obtain its > data and disable the clock when the device is removed. > > Signed-off-by: Simon Arlott <simon@fire.lp0.eu> > --- > Added EXPORT_SYMBOL_GPL(brcmnand_get_socdata) > > (As the brcmnand module must be loaded first its compatible string will > apply to any existing devices before the soc-specific module can be > loaded.) What's this comment supposed to mean? The brcmnand module will not directly probe any devices. It doesn't register any driver structs by itself. (BTW given that, it probably doesn't need its MODULE_DEVICE_TABLE.) > drivers/mtd/nand/brcmnand/Makefile | 1 + > drivers/mtd/nand/brcmnand/bcm63268_nand.c | 179 ++++++++++++++++++++++++++++++ > drivers/mtd/nand/brcmnand/brcmnand.c | 8 ++ > drivers/mtd/nand/brcmnand/brcmnand.h | 1 + > 4 files changed, 189 insertions(+) > create mode 100644 drivers/mtd/nand/brcmnand/bcm63268_nand.c > > diff --git a/drivers/mtd/nand/brcmnand/Makefile b/drivers/mtd/nand/brcmnand/Makefile > index 3b1fbfd..b83a9ae 100644 > --- a/drivers/mtd/nand/brcmnand/Makefile > +++ b/drivers/mtd/nand/brcmnand/Makefile > @@ -2,5 +2,6 @@ > # more specific iproc_nand.o, for instance > obj-$(CONFIG_MTD_NAND_BRCMNAND) += iproc_nand.o > obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63138_nand.o > +obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63268_nand.o > obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmstb_nand.o > obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand.o > diff --git a/drivers/mtd/nand/brcmnand/bcm63268_nand.c b/drivers/mtd/nand/brcmnand/bcm63268_nand.c > new file mode 100644 > index 0000000..70ad907 > --- /dev/null > +++ b/drivers/mtd/nand/brcmnand/bcm63268_nand.c > @@ -0,0 +1,179 @@ > +/* > + * Copyright 2015 Simon Arlott > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * Derived from bcm63138_nand.c: > + * Copyright © 2015 Broadcom Corporation > + * > + * Derived from bcm963xx_4.12L.06B_consumer/shared/opensource/include/bcm963xx/63268_map_part.h: > + * Copyright 2000-2010 Broadcom Corporation > + * > + * Derived from bcm963xx_4.12L.06B_consumer/shared/opensource/flash/nandflash.c: > + * Copyright 2000-2010 Broadcom Corporation > + */ > + > +#include <linux/device.h> > +#include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/ioport.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +#include "brcmnand.h" > + > +struct bcm63268_nand_soc { > + struct brcmnand_soc soc; > + void __iomem *base; > + struct clk *clk; > +}; > + > +#define BCM63268_NAND_INT 0x00 > +#define BCM63268_NAND_STATUS_SHIFT 0 > +#define BCM63268_NAND_STATUS_MASK (0xfff << BCM63268_NAND_STATUS_SHIFT) > +#define BCM63268_NAND_ENABLE_SHIFT 16 > +#define BCM63268_NAND_ENABLE_MASK (0xffff << BCM63268_NAND_ENABLE_SHIFT) > +#define BCM63268_NAND_BASE_ADDR0 0x04 > +#define BCM63268_NAND_BASE_ADDR1 0x0c > + > +enum { > + BCM63268_NP_READ = BIT(0), > + BCM63268_BLOCK_ERASE = BIT(1), > + BCM63268_COPY_BACK = BIT(2), > + BCM63268_PAGE_PGM = BIT(3), > + BCM63268_CTRL_READY = BIT(4), > + BCM63268_DEV_RBPIN = BIT(5), > + BCM63268_ECC_ERR_UNC = BIT(6), > + BCM63268_ECC_ERR_CORR = BIT(7), > +}; > + > +static bool bcm63268_nand_intc_ack(struct brcmnand_soc *soc) > +{ > + struct bcm63268_nand_soc *priv = > + container_of(soc, struct bcm63268_nand_soc, soc); > + void __iomem *mmio = priv->base + BCM63268_NAND_INT; > + u32 val = brcmnand_readl(mmio); > + > + if (val & (BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT)) { > + /* Ack interrupt */ > + val &= ~BCM63268_NAND_STATUS_MASK; > + val |= BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT; > + brcmnand_writel(val, mmio); > + return true; > + } > + > + return false; > +} > + > +static void bcm63268_nand_intc_set(struct brcmnand_soc *soc, bool en) > +{ > + struct bcm63268_nand_soc *priv = > + container_of(soc, struct bcm63268_nand_soc, soc); > + void __iomem *mmio = priv->base + BCM63268_NAND_INT; > + u32 val = brcmnand_readl(mmio); > + > + /* Don't ack any interrupts */ > + val &= ~BCM63268_NAND_STATUS_MASK; > + > + if (en) > + val |= BCM63268_CTRL_READY << BCM63268_NAND_ENABLE_SHIFT; > + else > + val &= ~(BCM63268_CTRL_READY << BCM63268_NAND_ENABLE_SHIFT); > + > + brcmnand_writel(val, mmio); > +} > + > +static int bcm63268_nand_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct bcm63268_nand_soc *priv; > + struct brcmnand_soc *soc; > + struct resource *res; > + int ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + soc = &priv->soc; > + > + res = platform_get_resource_byname(pdev, > + IORESOURCE_MEM, "nand-intr-base"); > + if (!res) > + return -EINVAL; > + > + priv->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(priv->base)) > + return PTR_ERR(priv->base); > + > + priv->clk = devm_clk_get(&pdev->dev, "nand"); > + if (IS_ERR(priv->clk)) > + return PTR_ERR(priv->clk); Perhaps we should put this clock handling in brcmnand.c? Just have it treat the clock as optional (i.e., ignore errors except for EPROBE_DEFER?), so we don't fail if no clock was provided? This could help other platforms too, if they gain clock support. If we do this, you'll want to document the clock in the common binding, not the bcm63268-specific part. Also, could it help to disable/enable the clock during suspend/resume? If you move it to brcmnand.c, this would also be pretty simple. > + > + ret = clk_prepare_enable(priv->clk); > + if (ret) > + return ret; > + > + soc->ctlrdy_ack = bcm63268_nand_intc_ack; > + soc->ctlrdy_set_enabled = bcm63268_nand_intc_set; > + > + /* Disable and ack all interrupts */ > + brcmnand_writel(0, priv->base + BCM63268_NAND_INT); > + brcmnand_writel(BCM63268_NAND_STATUS_MASK, > + priv->base + BCM63268_NAND_INT); > + > + ret = brcmnand_probe(pdev, soc); > + if (ret) > + clk_disable_unprepare(priv->clk); > + > + return ret; > +} > + > +static int bcm63268_nand_remove(struct platform_device *pdev) > +{ > + struct brcmnand_soc *soc = brcmnand_get_socdata(pdev); > + struct bcm63268_nand_soc *priv = NULL; > + int ret; > + > + if (soc) > + priv = container_of(soc, struct bcm63268_nand_soc, soc); > + > + ret = brcmnand_remove(pdev); > + if (ret) > + return ret; > + > + if (priv) > + clk_disable_unprepare(priv->clk); > + > + return 0; > +} > + > +static const struct of_device_id bcm63268_nand_of_match[] = { > + { .compatible = "brcm,nand-bcm63268" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, bcm63268_nand_of_match); > + > +static struct platform_driver bcm63268_nand_driver = { > + .probe = bcm63268_nand_probe, > + .remove = bcm63268_nand_remove, > + .driver = { > + .name = "bcm63268_nand", > + .pm = &brcmnand_pm_ops, > + .of_match_table = bcm63268_nand_of_match, > + } > +}; > +module_platform_driver(bcm63268_nand_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Simon Arlott"); > +MODULE_DESCRIPTION("NAND driver for BCM63268"); > diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c > index 2c8f67f..5f26b8a 100644 > --- a/drivers/mtd/nand/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/brcmnand/brcmnand.c > @@ -2262,6 +2262,14 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) > } > EXPORT_SYMBOL_GPL(brcmnand_probe); > > +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev) > +{ > + struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev); > + > + return ctrl ? ctrl->soc : NULL; > +} > +EXPORT_SYMBOL_GPL(brcmnand_get_socdata); If you move the clk handling to the core brcmnand.c, then you won't need this still. > + > int brcmnand_remove(struct platform_device *pdev) > { > struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev); > diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h b/drivers/mtd/nand/brcmnand/brcmnand.h > index ef5eabb..6680419 100644 > --- a/drivers/mtd/nand/brcmnand/brcmnand.h > +++ b/drivers/mtd/nand/brcmnand/brcmnand.h > @@ -65,6 +65,7 @@ static inline void brcmnand_writel(u32 val, void __iomem *addr) > > int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc); > int brcmnand_remove(struct platform_device *pdev); > +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev); > > extern const struct dev_pm_ops brcmnand_pm_ops; > Brian
On 02/12/15 19:18, Brian Norris wrote: > + Broadcom list + Kamal > > Hi Simon, > > On Wed, Nov 25, 2015 at 07:49:13PM +0000, Simon Arlott wrote: >> The BCM63268 has a NAND interrupt register with combined status and enable >> registers. It also has a clock for the NAND controller that needs to be >> enabled. >> >> Set up the device by enabling the clock, disabling and acking all >> interrupts, then handle the CTRL_READY interrupt. >> >> Add a brcmnand_get_socdata() function so that bcm63268_nand can obtain its >> data and disable the clock when the device is removed. >> >> Signed-off-by: Simon Arlott <simon@fire.lp0.eu> >> --- >> Added EXPORT_SYMBOL_GPL(brcmnand_get_socdata) >> >> (As the brcmnand module must be loaded first its compatible string will >> apply to any existing devices before the soc-specific module can be >> loaded.) > > What's this comment supposed to mean? The brcmnand module will not > directly probe any devices. It doesn't register any driver structs by > itself. I didn't notice that it didn't actually probe devices. In that case the documentation should not require "brcm,brcmnand" for soc-specific variants because the hardware's not really compatible with it. > (BTW given that, it probably doesn't need its MODULE_DEVICE_TABLE.) > >> drivers/mtd/nand/brcmnand/Makefile | 1 + >> drivers/mtd/nand/brcmnand/bcm63268_nand.c | 179 ++++++++++++++++++++++++++++++ >> drivers/mtd/nand/brcmnand/brcmnand.c | 8 ++ >> drivers/mtd/nand/brcmnand/brcmnand.h | 1 + >> 4 files changed, 189 insertions(+) >> create mode 100644 drivers/mtd/nand/brcmnand/bcm63268_nand.c >> >> diff --git a/drivers/mtd/nand/brcmnand/Makefile b/drivers/mtd/nand/brcmnand/Makefile >> index 3b1fbfd..b83a9ae 100644 >> --- a/drivers/mtd/nand/brcmnand/Makefile >> +++ b/drivers/mtd/nand/brcmnand/Makefile >> @@ -2,5 +2,6 @@ >> # more specific iproc_nand.o, for instance >> obj-$(CONFIG_MTD_NAND_BRCMNAND) += iproc_nand.o >> obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63138_nand.o >> +obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63268_nand.o >> obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmstb_nand.o >> obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand.o >> diff --git a/drivers/mtd/nand/brcmnand/bcm63268_nand.c b/drivers/mtd/nand/brcmnand/bcm63268_nand.c >> new file mode 100644 >> index 0000000..70ad907 >> --- /dev/null >> +++ b/drivers/mtd/nand/brcmnand/bcm63268_nand.c >> @@ -0,0 +1,179 @@ >> +/* >> + * Copyright 2015 Simon Arlott >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * Derived from bcm63138_nand.c: >> + * Copyright © 2015 Broadcom Corporation >> + * >> + * Derived from bcm963xx_4.12L.06B_consumer/shared/opensource/include/bcm963xx/63268_map_part.h: >> + * Copyright 2000-2010 Broadcom Corporation >> + * >> + * Derived from bcm963xx_4.12L.06B_consumer/shared/opensource/flash/nandflash.c: >> + * Copyright 2000-2010 Broadcom Corporation >> + */ >> + >> +#include <linux/device.h> >> +#include <linux/clk.h> >> +#include <linux/io.h> >> +#include <linux/ioport.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_address.h> >> +#include <linux/platform_device.h> >> +#include <linux/slab.h> >> + >> +#include "brcmnand.h" >> + >> +struct bcm63268_nand_soc { >> + struct brcmnand_soc soc; >> + void __iomem *base; >> + struct clk *clk; >> +}; >> + >> +#define BCM63268_NAND_INT 0x00 >> +#define BCM63268_NAND_STATUS_SHIFT 0 >> +#define BCM63268_NAND_STATUS_MASK (0xfff << BCM63268_NAND_STATUS_SHIFT) >> +#define BCM63268_NAND_ENABLE_SHIFT 16 >> +#define BCM63268_NAND_ENABLE_MASK (0xffff << BCM63268_NAND_ENABLE_SHIFT) >> +#define BCM63268_NAND_BASE_ADDR0 0x04 >> +#define BCM63268_NAND_BASE_ADDR1 0x0c >> + >> +enum { >> + BCM63268_NP_READ = BIT(0), >> + BCM63268_BLOCK_ERASE = BIT(1), >> + BCM63268_COPY_BACK = BIT(2), >> + BCM63268_PAGE_PGM = BIT(3), >> + BCM63268_CTRL_READY = BIT(4), >> + BCM63268_DEV_RBPIN = BIT(5), >> + BCM63268_ECC_ERR_UNC = BIT(6), >> + BCM63268_ECC_ERR_CORR = BIT(7), >> +}; >> + >> +static bool bcm63268_nand_intc_ack(struct brcmnand_soc *soc) >> +{ >> + struct bcm63268_nand_soc *priv = >> + container_of(soc, struct bcm63268_nand_soc, soc); >> + void __iomem *mmio = priv->base + BCM63268_NAND_INT; >> + u32 val = brcmnand_readl(mmio); >> + >> + if (val & (BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT)) { >> + /* Ack interrupt */ >> + val &= ~BCM63268_NAND_STATUS_MASK; >> + val |= BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT; >> + brcmnand_writel(val, mmio); >> + return true; >> + } >> + >> + return false; >> +} >> + >> +static void bcm63268_nand_intc_set(struct brcmnand_soc *soc, bool en) >> +{ >> + struct bcm63268_nand_soc *priv = >> + container_of(soc, struct bcm63268_nand_soc, soc); >> + void __iomem *mmio = priv->base + BCM63268_NAND_INT; >> + u32 val = brcmnand_readl(mmio); >> + >> + /* Don't ack any interrupts */ >> + val &= ~BCM63268_NAND_STATUS_MASK; >> + >> + if (en) >> + val |= BCM63268_CTRL_READY << BCM63268_NAND_ENABLE_SHIFT; >> + else >> + val &= ~(BCM63268_CTRL_READY << BCM63268_NAND_ENABLE_SHIFT); >> + >> + brcmnand_writel(val, mmio); >> +} >> + >> +static int bcm63268_nand_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct bcm63268_nand_soc *priv; >> + struct brcmnand_soc *soc; >> + struct resource *res; >> + int ret; >> + >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + soc = &priv->soc; >> + >> + res = platform_get_resource_byname(pdev, >> + IORESOURCE_MEM, "nand-intr-base"); >> + if (!res) >> + return -EINVAL; >> + >> + priv->base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(priv->base)) >> + return PTR_ERR(priv->base); >> + >> + priv->clk = devm_clk_get(&pdev->dev, "nand"); >> + if (IS_ERR(priv->clk)) >> + return PTR_ERR(priv->clk); > > Perhaps we should put this clock handling in brcmnand.c? Just have it > treat the clock as optional (i.e., ignore errors except for > EPROBE_DEFER?), so we don't fail if no clock was provided? This could > help other platforms too, if they gain clock support. Unless most soc variants have clocks I'd prefer to leave it in this module. > If we do this, you'll want to document the clock in the common binding, > not the bcm63268-specific part. > > Also, could it help to disable/enable the clock during suspend/resume? > If you move it to brcmnand.c, this would also be pretty simple. Alternatively, it could proxy the brcmnand_pm_ops functions. I don't have any way to test suspend/resume. >> + >> + ret = clk_prepare_enable(priv->clk); >> + if (ret) >> + return ret; >> + >> + soc->ctlrdy_ack = bcm63268_nand_intc_ack; >> + soc->ctlrdy_set_enabled = bcm63268_nand_intc_set; >> + >> + /* Disable and ack all interrupts */ >> + brcmnand_writel(0, priv->base + BCM63268_NAND_INT); >> + brcmnand_writel(BCM63268_NAND_STATUS_MASK, >> + priv->base + BCM63268_NAND_INT); >> + >> + ret = brcmnand_probe(pdev, soc); >> + if (ret) >> + clk_disable_unprepare(priv->clk); >> + >> + return ret; >> +} >> + >> +static int bcm63268_nand_remove(struct platform_device *pdev) >> +{ >> + struct brcmnand_soc *soc = brcmnand_get_socdata(pdev); >> + struct bcm63268_nand_soc *priv = NULL; >> + int ret; >> + >> + if (soc) >> + priv = container_of(soc, struct bcm63268_nand_soc, soc); >> + >> + ret = brcmnand_remove(pdev); >> + if (ret) >> + return ret; >> + >> + if (priv) >> + clk_disable_unprepare(priv->clk); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id bcm63268_nand_of_match[] = { >> + { .compatible = "brcm,nand-bcm63268" }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, bcm63268_nand_of_match); >> + >> +static struct platform_driver bcm63268_nand_driver = { >> + .probe = bcm63268_nand_probe, >> + .remove = bcm63268_nand_remove, >> + .driver = { >> + .name = "bcm63268_nand", >> + .pm = &brcmnand_pm_ops, >> + .of_match_table = bcm63268_nand_of_match, >> + } >> +}; >> +module_platform_driver(bcm63268_nand_driver); >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_AUTHOR("Simon Arlott"); >> +MODULE_DESCRIPTION("NAND driver for BCM63268"); >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c >> index 2c8f67f..5f26b8a 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c >> @@ -2262,6 +2262,14 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) >> } >> EXPORT_SYMBOL_GPL(brcmnand_probe); >> >> +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev) >> +{ >> + struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev); >> + >> + return ctrl ? ctrl->soc : NULL; >> +} >> +EXPORT_SYMBOL_GPL(brcmnand_get_socdata); > > If you move the clk handling to the core brcmnand.c, then you won't need > this still. Would you prefer a clock name in the soc data structure that is used to call devm_clk_get()? >> + >> int brcmnand_remove(struct platform_device *pdev) >> { >> struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev); >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h b/drivers/mtd/nand/brcmnand/brcmnand.h >> index ef5eabb..6680419 100644 >> --- a/drivers/mtd/nand/brcmnand/brcmnand.h >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.h >> @@ -65,6 +65,7 @@ static inline void brcmnand_writel(u32 val, void __iomem *addr) >> >> int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc); >> int brcmnand_remove(struct platform_device *pdev); >> +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev); >> >> extern const struct dev_pm_ops brcmnand_pm_ops; >> > > Brian >
Hi, On Wed, Dec 02, 2015 at 07:54:49PM +0000, Simon Arlott wrote: > On 02/12/15 19:18, Brian Norris wrote: > > On Wed, Nov 25, 2015 at 07:49:13PM +0000, Simon Arlott wrote: > >> +static int bcm63268_nand_probe(struct platform_device *pdev) > >> +{ > >> + struct device *dev = &pdev->dev; > >> + struct bcm63268_nand_soc *priv; > >> + struct brcmnand_soc *soc; > >> + struct resource *res; > >> + int ret; > >> + > >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > >> + if (!priv) > >> + return -ENOMEM; > >> + soc = &priv->soc; > >> + > >> + res = platform_get_resource_byname(pdev, > >> + IORESOURCE_MEM, "nand-intr-base"); > >> + if (!res) > >> + return -EINVAL; > >> + > >> + priv->base = devm_ioremap_resource(dev, res); > >> + if (IS_ERR(priv->base)) > >> + return PTR_ERR(priv->base); > >> + > >> + priv->clk = devm_clk_get(&pdev->dev, "nand"); > >> + if (IS_ERR(priv->clk)) > >> + return PTR_ERR(priv->clk); > > > > Perhaps we should put this clock handling in brcmnand.c? Just have it > > treat the clock as optional (i.e., ignore errors except for > > EPROBE_DEFER?), so we don't fail if no clock was provided? This could > > help other platforms too, if they gain clock support. > > Unless most soc variants have clocks I'd prefer to leave it in this > module. I'm quite confident your SoC is not the only one with clocks. > > If we do this, you'll want to document the clock in the common binding, > > not the bcm63268-specific part. > > > > Also, could it help to disable/enable the clock during suspend/resume? > > If you move it to brcmnand.c, this would also be pretty simple. > > Alternatively, it could proxy the brcmnand_pm_ops functions. I don't > have any way to test suspend/resume. OK, no need to add it now then. It can be added if/when it's needed. > >> + > >> + ret = clk_prepare_enable(priv->clk); > >> + if (ret) > >> + return ret; > >> + > >> + soc->ctlrdy_ack = bcm63268_nand_intc_ack; > >> + soc->ctlrdy_set_enabled = bcm63268_nand_intc_set; > >> + > >> + /* Disable and ack all interrupts */ > >> + brcmnand_writel(0, priv->base + BCM63268_NAND_INT); > >> + brcmnand_writel(BCM63268_NAND_STATUS_MASK, > >> + priv->base + BCM63268_NAND_INT); > >> + > >> + ret = brcmnand_probe(pdev, soc); > >> + if (ret) > >> + clk_disable_unprepare(priv->clk); > >> + > >> + return ret; > >> +} > >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c > >> index 2c8f67f..5f26b8a 100644 > >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c > >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c > >> @@ -2262,6 +2262,14 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) > >> } > >> EXPORT_SYMBOL_GPL(brcmnand_probe); > >> > >> +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev) > >> +{ > >> + struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev); > >> + > >> + return ctrl ? ctrl->soc : NULL; > >> +} > >> +EXPORT_SYMBOL_GPL(brcmnand_get_socdata); > > > > If you move the clk handling to the core brcmnand.c, then you won't need > > this still. > > Would you prefer a clock name in the soc data structure that is used to > call devm_clk_get()? Not really. If we specify a clock name now, we can suggest other SoC's to use the same name (where possible). So we wouldn't need any new code or documentation, and we definitely don't need each snowflake sub-driver to pass a new name. Brian
diff --git a/drivers/mtd/nand/brcmnand/Makefile b/drivers/mtd/nand/brcmnand/Makefile index 3b1fbfd..b83a9ae 100644 --- a/drivers/mtd/nand/brcmnand/Makefile +++ b/drivers/mtd/nand/brcmnand/Makefile @@ -2,5 +2,6 @@ # more specific iproc_nand.o, for instance obj-$(CONFIG_MTD_NAND_BRCMNAND) += iproc_nand.o obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63138_nand.o +obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63268_nand.o obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmstb_nand.o obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand.o diff --git a/drivers/mtd/nand/brcmnand/bcm63268_nand.c b/drivers/mtd/nand/brcmnand/bcm63268_nand.c new file mode 100644 index 0000000..70ad907 --- /dev/null +++ b/drivers/mtd/nand/brcmnand/bcm63268_nand.c @@ -0,0 +1,179 @@ +/* + * Copyright 2015 Simon Arlott + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Derived from bcm63138_nand.c: + * Copyright © 2015 Broadcom Corporation + * + * Derived from bcm963xx_4.12L.06B_consumer/shared/opensource/include/bcm963xx/63268_map_part.h: + * Copyright 2000-2010 Broadcom Corporation + * + * Derived from bcm963xx_4.12L.06B_consumer/shared/opensource/flash/nandflash.c: + * Copyright 2000-2010 Broadcom Corporation + */ + +#include <linux/device.h> +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/ioport.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#include "brcmnand.h" + +struct bcm63268_nand_soc { + struct brcmnand_soc soc; + void __iomem *base; + struct clk *clk; +}; + +#define BCM63268_NAND_INT 0x00 +#define BCM63268_NAND_STATUS_SHIFT 0 +#define BCM63268_NAND_STATUS_MASK (0xfff << BCM63268_NAND_STATUS_SHIFT) +#define BCM63268_NAND_ENABLE_SHIFT 16 +#define BCM63268_NAND_ENABLE_MASK (0xffff << BCM63268_NAND_ENABLE_SHIFT) +#define BCM63268_NAND_BASE_ADDR0 0x04 +#define BCM63268_NAND_BASE_ADDR1 0x0c + +enum { + BCM63268_NP_READ = BIT(0), + BCM63268_BLOCK_ERASE = BIT(1), + BCM63268_COPY_BACK = BIT(2), + BCM63268_PAGE_PGM = BIT(3), + BCM63268_CTRL_READY = BIT(4), + BCM63268_DEV_RBPIN = BIT(5), + BCM63268_ECC_ERR_UNC = BIT(6), + BCM63268_ECC_ERR_CORR = BIT(7), +}; + +static bool bcm63268_nand_intc_ack(struct brcmnand_soc *soc) +{ + struct bcm63268_nand_soc *priv = + container_of(soc, struct bcm63268_nand_soc, soc); + void __iomem *mmio = priv->base + BCM63268_NAND_INT; + u32 val = brcmnand_readl(mmio); + + if (val & (BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT)) { + /* Ack interrupt */ + val &= ~BCM63268_NAND_STATUS_MASK; + val |= BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT; + brcmnand_writel(val, mmio); + return true; + } + + return false; +} + +static void bcm63268_nand_intc_set(struct brcmnand_soc *soc, bool en) +{ + struct bcm63268_nand_soc *priv = + container_of(soc, struct bcm63268_nand_soc, soc); + void __iomem *mmio = priv->base + BCM63268_NAND_INT; + u32 val = brcmnand_readl(mmio); + + /* Don't ack any interrupts */ + val &= ~BCM63268_NAND_STATUS_MASK; + + if (en) + val |= BCM63268_CTRL_READY << BCM63268_NAND_ENABLE_SHIFT; + else + val &= ~(BCM63268_CTRL_READY << BCM63268_NAND_ENABLE_SHIFT); + + brcmnand_writel(val, mmio); +} + +static int bcm63268_nand_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct bcm63268_nand_soc *priv; + struct brcmnand_soc *soc; + struct resource *res; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + soc = &priv->soc; + + res = platform_get_resource_byname(pdev, + IORESOURCE_MEM, "nand-intr-base"); + if (!res) + return -EINVAL; + + priv->base = devm_ioremap_resource(dev, res); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + priv->clk = devm_clk_get(&pdev->dev, "nand"); + if (IS_ERR(priv->clk)) + return PTR_ERR(priv->clk); + + ret = clk_prepare_enable(priv->clk); + if (ret) + return ret; + + soc->ctlrdy_ack = bcm63268_nand_intc_ack; + soc->ctlrdy_set_enabled = bcm63268_nand_intc_set; + + /* Disable and ack all interrupts */ + brcmnand_writel(0, priv->base + BCM63268_NAND_INT); + brcmnand_writel(BCM63268_NAND_STATUS_MASK, + priv->base + BCM63268_NAND_INT); + + ret = brcmnand_probe(pdev, soc); + if (ret) + clk_disable_unprepare(priv->clk); + + return ret; +} + +static int bcm63268_nand_remove(struct platform_device *pdev) +{ + struct brcmnand_soc *soc = brcmnand_get_socdata(pdev); + struct bcm63268_nand_soc *priv = NULL; + int ret; + + if (soc) + priv = container_of(soc, struct bcm63268_nand_soc, soc); + + ret = brcmnand_remove(pdev); + if (ret) + return ret; + + if (priv) + clk_disable_unprepare(priv->clk); + + return 0; +} + +static const struct of_device_id bcm63268_nand_of_match[] = { + { .compatible = "brcm,nand-bcm63268" }, + {}, +}; +MODULE_DEVICE_TABLE(of, bcm63268_nand_of_match); + +static struct platform_driver bcm63268_nand_driver = { + .probe = bcm63268_nand_probe, + .remove = bcm63268_nand_remove, + .driver = { + .name = "bcm63268_nand", + .pm = &brcmnand_pm_ops, + .of_match_table = bcm63268_nand_of_match, + } +}; +module_platform_driver(bcm63268_nand_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Simon Arlott"); +MODULE_DESCRIPTION("NAND driver for BCM63268"); diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c index 2c8f67f..5f26b8a 100644 --- a/drivers/mtd/nand/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/brcmnand/brcmnand.c @@ -2262,6 +2262,14 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc) } EXPORT_SYMBOL_GPL(brcmnand_probe); +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev) +{ + struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev); + + return ctrl ? ctrl->soc : NULL; +} +EXPORT_SYMBOL_GPL(brcmnand_get_socdata); + int brcmnand_remove(struct platform_device *pdev) { struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev); diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h b/drivers/mtd/nand/brcmnand/brcmnand.h index ef5eabb..6680419 100644 --- a/drivers/mtd/nand/brcmnand/brcmnand.h +++ b/drivers/mtd/nand/brcmnand/brcmnand.h @@ -65,6 +65,7 @@ static inline void brcmnand_writel(u32 val, void __iomem *addr) int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc); int brcmnand_remove(struct platform_device *pdev); +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev); extern const struct dev_pm_ops brcmnand_pm_ops;
The BCM63268 has a NAND interrupt register with combined status and enable registers. It also has a clock for the NAND controller that needs to be enabled. Set up the device by enabling the clock, disabling and acking all interrupts, then handle the CTRL_READY interrupt. Add a brcmnand_get_socdata() function so that bcm63268_nand can obtain its data and disable the clock when the device is removed. Signed-off-by: Simon Arlott <simon@fire.lp0.eu> --- Added EXPORT_SYMBOL_GPL(brcmnand_get_socdata) (As the brcmnand module must be loaded first its compatible string will apply to any existing devices before the soc-specific module can be loaded.) drivers/mtd/nand/brcmnand/Makefile | 1 + drivers/mtd/nand/brcmnand/bcm63268_nand.c | 179 ++++++++++++++++++++++++++++++ drivers/mtd/nand/brcmnand/brcmnand.c | 8 ++ drivers/mtd/nand/brcmnand/brcmnand.h | 1 + 4 files changed, 189 insertions(+) create mode 100644 drivers/mtd/nand/brcmnand/bcm63268_nand.c