Re: [PATCH 1/3] ath6kl: Only use match sets when firmware supports it
Hi Kalle, Have you had a chance to check out these patches yet? -- Kyle Roeschley Software Engineer National Instruments
[PATCH] USB: serial: cp210x: Add ID for NI USB serial console
Added the USB VID and PID for the USB serial console on some National
Instruments devices.
Signed-off-by: Kyle Roeschley
---
drivers/usb/serial/cp210x.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 06d502b3e913..cc5b2d0d340d 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -213,6 +213,7 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x3195, 0xF190) }, /* Link Instruments MSO-19 */
{ USB_DEVICE(0x3195, 0xF280) }, /* Link Instruments MSO-28 */
{ USB_DEVICE(0x3195, 0xF281) }, /* Link Instruments MSO-28 */
+ { USB_DEVICE(0x3923, 0x7A0B) }, /* National Instruments USB Serial
Console */
{ USB_DEVICE(0x413C, 0x9500) }, /* DW700 GPS USB interface */
{ } /* Terminating Entry */
};
--
2.16.2
Re: [PATCH] spi: Add a sysfs interface to instantiate devices
On Fri, Dec 22, 2017 at 03:56:03PM +, Mark Brown wrote:
> On Thu, Dec 21, 2017 at 09:05:43PM +, Trent Piepho wrote:
> > On Thu, 2017-12-21 at 14:03 -0600, Kyle Roeschley wrote:
> > > Add a sysfs interface to instantiate and delete SPI devices using the
> > > spidev driver. This can be used when developing a driver on a
> > > self-soldered board which doesn't yet have proper SPI device declaration
> > > at the platform level, and presumably for various debugging situations.
>
> > > Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
> > > devices").
>
> > The i2c interface allows one to specify the type of device to create.
> > Why must this interface be linked to spidev and only capable of
> > creating spidev devices?
>
> Right, that doesn't seem good. I also can't see anything in the actual
> code which suggests that this is tied to spidev except the log messages.
>
Quoting Geert's email [1] on the subject:
> To me, the above sounds a bit contradictive: either you have
> 1. a simple (trivial) description, which can be handled by spidev and
> userspace, and thus by just writing " spidev" to a new_device
> sysfs node, or
> 2. a complex description, for which you need a specialized in-kernel driver,
> so you're gonna need a real DT node (and overlays?) to describe it.
>
> I don't think writing a complex description to a new_device sysfs node makes
> sense.
And regarding not being linked to spidev, see modalias in new_device_store:
> > > + struct spi_board_info bi = {
> > > + .modalias = "spidev",
> > > + .max_speed_hz = ctlr->max_speed_hz,
> > > + };
[1] https://marc.info/?l=linux-spi&m=151199390921251&w=2
Happy holidays,
--
Kyle Roeschley
Software Engineer
National Instruments
[PATCH] spi: Add a sysfs interface to instantiate devices
Add a sysfs interface to instantiate and delete SPI devices using the
spidev driver. This can be used when developing a driver on a
self-soldered board which doesn't yet have proper SPI device declaration
at the platform level, and presumably for various debugging situations.
Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
devices").
Signed-off-by: Kyle Roeschley
---
Documentation/spi/spi-summary | 14
drivers/spi/spi.c | 78 +++
include/linux/spi/spi.h | 3 ++
3 files changed, 95 insertions(+)
diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary
index 1721c1b570c3..51d9747c4426 100644
--- a/Documentation/spi/spi-summary
+++ b/Documentation/spi/spi-summary
@@ -339,6 +339,20 @@ up the spi bus master, and will likely need
spi_new_device() to provide the
board info based on the board that was hotplugged. Of course, you'd later
call at least spi_unregister_device() when that board is removed.
+Alternatively, a sysfs interface was added to let the user create devices which
+using the spidev driver. This interface is made of 2 attribute files which are
+created in every SPI master directory: new_device and delete_device. Both files
+are write only and you must write the decimal SPI chip select number to them in
+order to properly instantiate or delete a SPI device. As no two devices can be
+attached to the same master with the same chip select line, the chip select
+number is sufficient to uniquely identify the device to be deleted.
+
+Example:
+# echo 1 > /sys/class/spi_master/spi0/new_device
+
+In general, this interface should only be used when in-kernel device
+declaration can't be done.
+
When Linux includes support for MMC/SD/SDIO/DataFlash cards through SPI, those
configurations will also be dynamic. Fortunately, such devices all support
basic device identification probes, so they should hotplug normally.
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b33a727a0158..648ccdf359f9 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -242,8 +242,85 @@ static const struct attribute_group
spi_controller_statistics_group = {
.attrs = spi_controller_statistics_attrs,
};
+static ssize_t
+new_device_store(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct spi_controller *ctlr = container_of(dev, struct spi_controller,
+ dev);
+ struct spi_device *spi;
+ struct spi_board_info bi = {
+ .modalias = "spidev",
+ .max_speed_hz = ctlr->max_speed_hz,
+ };
+
+ if (kstrtou16(buf, 0, &bi.chip_select) < 0)
+ return -EINVAL;
+
+ spi = spi_new_device(ctlr, &bi);
+ if (!spi) {
+ dev_err(dev, "can't create new device\n");
+ return -ENXIO;
+ }
+
+ mutex_lock(&ctlr->bus_lock_mutex);
+ list_add_tail(&spi->userspace_device, &ctlr->userspace_devices);
+ mutex_unlock(&ctlr->bus_lock_mutex);
+
+ dev_info(dev, "created spidev device %s\n", dev_name(&spi->dev));
+
+ return count;
+}
+static DEVICE_ATTR_WO(new_device);
+
+static ssize_t
+delete_device_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct spi_controller *ctlr = container_of(dev, struct spi_controller,
+ dev);
+ struct spi_device *spi, *next;
+ int ret = -ENXIO;
+ u16 cs;
+
+ if (kstrtou16(buf, 0, &cs) < 0)
+ return -EINVAL;
+
+ mutex_lock(&ctlr->bus_lock_mutex);
+ list_for_each_entry_safe(spi, next, &ctlr->userspace_devices,
+userspace_device) {
+ if (spi->chip_select != cs)
+ continue;
+
+ dev_info(dev, "deleting spidev device %s\n",
+dev_name(&spi->dev));
+ list_del(&spi->userspace_device);
+ spi_unregister_device(spi);
+ ret = count;
+ break;
+ }
+ mutex_unlock(&ctlr->bus_lock_mutex);
+
+ if (ret == -ENXIO)
+ dev_err(dev, "can't find spidev device %u in list\n", cs);
+
+ return ret;
+}
+static DEVICE_ATTR_WO(delete_device);
+
+static struct attribute *spi_controller_userspace_attrs[] = {
+ &dev_attr_new_device.attr,
+ &dev_attr_delete_device.attr,
+ NULL,
+};
+
+static const struct attribute_group spi_controller_userspace_group = {
+ .attrs = spi_controller_userspace_attrs,
+};
+
static const struct attribute_group *spi_master_groups[] = {
&spi_controller_statistics_group,
+
Re: [PATCH] mmc: core: Wait for Vdd to settle on card power off
On Fri, Sep 22, 2017 at 11:38:40AM +0200, Ulf Hansson wrote: > On 21 September 2017 at 19:47, Kyle Roeschley wrote: > > The SD spec version 6.0 section 6.4.1.5 requires that Vdd must be > > lowered to less than 0.5V for a minimum of 1 ms when powering off a > > card. Increase our wait to 10 ms so that voltage has time to drain down > > to 0.5V and cards can power off correctly. > > > > Signed-off-by: Kyle Roeschley > > --- > > drivers/mmc/core/core.c | 10 -- > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > > index 66c9cf49ad2f..38630246de26 100644 > > --- a/drivers/mmc/core/core.c > > +++ b/drivers/mmc/core/core.c > > @@ -1679,18 +1679,16 @@ void mmc_power_off(struct mmc_host *host) > > mmc_set_initial_state(host); > > > > /* > > -* Some configurations, such as the 802.11 SDIO card in the OLPC > > -* XO-1.5, require a short delay after poweroff before the card > > -* can be successfully turned on again. > > +* The SD spec requires at least 1 ms with Vdd at less than 0.5 V > > +* before a card can be re-powered, but we need to wait longer so > > that > > +* the voltage has time to drain. > > */ > > - mmc_delay(1); > > + mmc_delay(10); > > No, this isn't the proper place of adding more "magic" delays. > > Instead, make sure the related ->set_ios() callback in the mmc host > driver deals with this instead. In case it uses an external regulator, > via the regulator API, then this is something that should be > controlled with the definition of the regulator. > Thanks for pointing me in the right direction, I'll reimplement the fix there. -- Kyle Roeschley Software Engineer National Instruments
Re: [PATCH] pinctrl: intel: Mask interrupts on driver probe
On Fri, Sep 22, 2017 at 08:47:12AM +0300, Mika Westerberg wrote: > On Thu, Sep 21, 2017 at 02:20:03PM -0500, Kyle Roeschley wrote: > > Powering off the system on Apollo Lake does not clear the interrupt > > enable registers for the GPIOs. To avoid an interrupt storm on driver > > probe, clear all interrupt enables before enabling our interrupt line. > > It is up to the BIOS to set the proper mask and program the pads > accordingly. Which platform and BIOS this is? > > I would rather not do this because it might cause other problems. I haven't seen any issues in testing. This is on a platform based on the Oxbow Hill CRB, but it's entirely possible that there's a bug in the BIOS power sequencing behavior. I'll dig around and see if something's been missed. -- Kyle Roeschley Software Engineer National Instruments
[PATCH] pinctrl: intel: Mask interrupts on driver probe
Powering off the system on Apollo Lake does not clear the interrupt
enable registers for the GPIOs. To avoid an interrupt storm on driver
probe, clear all interrupt enables before enabling our interrupt line.
Signed-off-by: Kyle Roeschley
---
drivers/pinctrl/intel/pinctrl-intel.c | 43 +++
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
b/drivers/pinctrl/intel/pinctrl-intel.c
index 71df0f70b61f..f08006417aed 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1043,6 +1043,26 @@ static struct irq_chip intel_gpio_irqchip = {
.flags = IRQCHIP_MASK_ON_SUSPEND,
};
+static void intel_gpio_irq_init(struct intel_pinctrl *pctrl)
+{
+ size_t i;
+
+ for (i = 0; i < pctrl->ncommunities; i++) {
+ const struct intel_community *community;
+ void __iomem *base;
+ unsigned gpp;
+
+ community = &pctrl->communities[i];
+ base = community->regs;
+
+ for (gpp = 0; gpp < community->ngpps; gpp++) {
+ /* Mask and clear all interrupts */
+ writel(0, base + community->ie_offset + gpp * 4);
+ writel(0x, base + GPI_IS + gpp * 4);
+ }
+ }
+}
+
static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
{
int ret;
@@ -1068,6 +1088,9 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl,
int irq)
return ret;
}
+ /* Mask all interrupts */
+ intel_gpio_irq_init(pctrl);
+
/*
* We need to request the interrupt here (instead of providing chip
* to the irq directly) because on some platforms several GPIO
@@ -1341,26 +1364,6 @@ int intel_pinctrl_suspend(struct device *dev)
}
EXPORT_SYMBOL_GPL(intel_pinctrl_suspend);
-static void intel_gpio_irq_init(struct intel_pinctrl *pctrl)
-{
- size_t i;
-
- for (i = 0; i < pctrl->ncommunities; i++) {
- const struct intel_community *community;
- void __iomem *base;
- unsigned gpp;
-
- community = &pctrl->communities[i];
- base = community->regs;
-
- for (gpp = 0; gpp < community->ngpps; gpp++) {
- /* Mask and clear all interrupts */
- writel(0, base + community->ie_offset + gpp * 4);
- writel(0x, base + GPI_IS + gpp * 4);
- }
- }
-}
-
int intel_pinctrl_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
--
2.14.1
[PATCH] mmc: core: Wait for Vdd to settle on card power off
The SD spec version 6.0 section 6.4.1.5 requires that Vdd must be
lowered to less than 0.5V for a minimum of 1 ms when powering off a
card. Increase our wait to 10 ms so that voltage has time to drain down
to 0.5V and cards can power off correctly.
Signed-off-by: Kyle Roeschley
---
drivers/mmc/core/core.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 66c9cf49ad2f..38630246de26 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1679,18 +1679,16 @@ void mmc_power_off(struct mmc_host *host)
mmc_set_initial_state(host);
/*
-* Some configurations, such as the 802.11 SDIO card in the OLPC
-* XO-1.5, require a short delay after poweroff before the card
-* can be successfully turned on again.
+* The SD spec requires at least 1 ms with Vdd at less than 0.5 V
+* before a card can be re-powered, but we need to wait longer so that
+* the voltage has time to drain.
*/
- mmc_delay(1);
+ mmc_delay(10);
}
void mmc_power_cycle(struct mmc_host *host, u32 ocr)
{
mmc_power_off(host);
- /* Wait at least 1 ms according to SD spec */
- mmc_delay(1);
mmc_power_up(host, ocr);
}
--
2.14.1
[PATCH] ARM: zynq: Reserve correct amount of non-DMA RAM
On Zynq, we haven't been reserving the correct amount of DMA-incapable
RAM to keep DMA away from it (per the Zynq TRM Section 4.1, it should be
the first 512k). In older kernels, this was masked by the
memblock_reserve call in arm_memblock_init(). Now, reserve the correct
amount excplicitly rather than relying on swapper_pg_dir, which is an
address and not a size anyway.
Fixes: 46f5b96 ("ARM: zynq: Reserve not DMAable space in front of the
kernel")
Signed-off-by: Kyle Roeschley
---
Found when migrating from 4.1 to 4.6.
arch/arm/mach-zynq/common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 6cefdb8..75885bc 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -59,7 +59,7 @@ void __iomem *zynq_scu_base;
static void __init zynq_memory_init(void)
{
if (!__pa(PAGE_OFFSET))
- memblock_reserve(__pa(PAGE_OFFSET), __pa(swapper_pg_dir));
+ memblock_reserve(__pa(PAGE_OFFSET), 0x8);
}
static struct platform_device zynq_cpuidle_device = {
--
2.9.3
Re: [RFC] net: phy: smsc: Disable auto-negotiation on startup
On Tue, Oct 11, 2016 at 09:32:30AM -0500, Jeremy Linton wrote:
> On 10/10/2016 12:41 PM, Kyle Roeschley wrote:
> > Because the SMSC PHY completes auto-negotiation before the driver is
> > ready to handle interrupts, the PHY state machine never realizes that we
> > have a link. Clear the ANENABLE bit on initialization, which lets
> > genphy_config_aneg do its thing when that code is hit later.
> >
> > While this patch does fix the problem we see (no link on boot without
> > re-plugging the cable), it seems like the generic PHY code should be
> > able to handle auto-negotiation completing before interrupts are
> > enabled. Submitted as an RFC in the hopes that someone has an idea as to
> > how that could be done.
>
> Hi,
>
> Which smsc chip/driver? Maybe assuring the device interrupts are enabled
> before the phy is started is a solution?
>
> The whole problem sounds similar to what was recently happening in the
> smsc911x driver, but AFAIK that driver is basically only polling at this
> point so connecting the phy before the interrupts are enabled shouldn't be a
> problem.
>
We're using the SMSC LAN8720A with the Cadence MACB ethernet controller.
Interrupts are enabled before the phy is started, but it looks like the patch
Florian pointed me to (https://www.spinics.net/lists/netdev/msg397857.html)
fixes my interrupt problem.
> >
> > This fix is copied from commit 99f81afc139c ("phy: micrel: Disable auto
> > negotiation on startup").
> >
> > Signed-off-by: Kyle Roeschley
> > ---
> > drivers/net/phy/smsc.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> > index b62c4aa..8de8011 100644
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -62,6 +62,16 @@ static int smsc_phy_config_init(struct phy_device
> > *phydev)
> > return rc;
> > }
> >
> > + if (phy_interrupt_is_valid(phydev)) {
> > + rc = phy_read(phydev, MII_BMCR);
> > + if (rc < 0)
> > + return rc;
> > +
> > + rc = phy_write(phydev, MII_BMCR, rc & ~BMCR_ANENABLE);
> > + if (rc < 0)
> > + return rc;
> > + }
> > +
> > return smsc_phy_ack_interrupt(phydev);
> > }
> >
> >
>
--
Kyle Roeschley
Software Engineer
National Instruments
Re: [RFC] net: phy: smsc: Disable auto-negotiation on startup
On Wed, Oct 12, 2016 at 02:13:06AM -0700, Florian Fainelli wrote:
> On 10/10/2016 10:41 AM, Kyle Roeschley wrote:
> > Because the SMSC PHY completes auto-negotiation before the driver is
> > ready to handle interrupts, the PHY state machine never realizes that we
> > have a link. Clear the ANENABLE bit on initialization, which lets
> > genphy_config_aneg do its thing when that code is hit later.
> >
> > While this patch does fix the problem we see (no link on boot without
> > re-plugging the cable), it seems like the generic PHY code should be
> > able to handle auto-negotiation completing before interrupts are
> > enabled. Submitted as an RFC in the hopes that someone has an idea as to
> > how that could be done.
> >
> > This fix is copied from commit 99f81afc139c ("phy: micrel: Disable auto
> > negotiation on startup").
>
> Do you mind trying:
>
> https://www.spinics.net/lists/netdev/msg397857.html
>
> and see if you do get link interrupts without your patch applied? Thanks!
Yep, that fixes it. I figured there was some state machine issue I was missing.
Thanks very much!
> --
> Florian
--
Kyle Roeschley
Software Engineer
National Instruments
[RFC] net: phy: smsc: Disable auto-negotiation on startup
Because the SMSC PHY completes auto-negotiation before the driver is
ready to handle interrupts, the PHY state machine never realizes that we
have a link. Clear the ANENABLE bit on initialization, which lets
genphy_config_aneg do its thing when that code is hit later.
While this patch does fix the problem we see (no link on boot without
re-plugging the cable), it seems like the generic PHY code should be
able to handle auto-negotiation completing before interrupts are
enabled. Submitted as an RFC in the hopes that someone has an idea as to
how that could be done.
This fix is copied from commit 99f81afc139c ("phy: micrel: Disable auto
negotiation on startup").
Signed-off-by: Kyle Roeschley
---
drivers/net/phy/smsc.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index b62c4aa..8de8011 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -62,6 +62,16 @@ static int smsc_phy_config_init(struct phy_device *phydev)
return rc;
}
+ if (phy_interrupt_is_valid(phydev)) {
+ rc = phy_read(phydev, MII_BMCR);
+ if (rc < 0)
+ return rc;
+
+ rc = phy_write(phydev, MII_BMCR, rc & ~BMCR_ANENABLE);
+ if (rc < 0)
+ return rc;
+ }
+
return smsc_phy_ack_interrupt(phydev);
}
--
2.9.3
[PATCH v8 1/2] mtd: nand_bbt: Move BBT block selection logic out of write_bbt()
From: Boris Brezillon
This clarifies the write_bbt() by removing the write label and
clarifying the error/exit path.
Signed-off-by: Boris Brezillon
Tested-by: Kyle Roeschley
---
v8: Move the chip indexing change back into patch 2
v7: Move all code cleanup into first patch
Correct documentation of mark_bbt_block_bad
Make pr_warn messages consistent
Add Tested-bys
v6: Split functionality of write_bbt out into new functions
v5: De-duplicate bad block handling
v4: Don't ignore write protection while marking bad BBT blocks
Correctly call block_markbad
Minor cleanups
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
drivers/mtd/nand/nand_bbt.c | 110 +---
1 file changed, 74 insertions(+), 36 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..9fbd664 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -605,6 +605,69 @@ static void search_read_bbts(struct mtd_info *mtd, uint8_t
*buf,
}
/**
+ * get_bbt_block - Get the first valid eraseblock suitable to store a BBT
+ * @this: the NAND device
+ * @td: the BBT description
+ * @md: the mirror BBT descriptor
+ * @chip: the CHIP selector
+ *
+ * This functions returns a positive block number pointing a valid eraseblock
+ * suitable to store a BBT (i.e. in the range reserved for BBT), or -ENOSPC if
+ * all blocks are already used of marked bad. If td->pages[chip] was already
+ * pointing to a valid block we re-use it, otherwise we search for the next
+ * valid one.
+ */
+static int get_bbt_block(struct nand_chip *this, struct nand_bbt_descr *td,
+struct nand_bbt_descr *md, int chip)
+{
+ int startblock, dir, page, numblocks, i;
+
+ /*
+* There was already a version of the table, reuse the page. This
+* applies for absolute placement too, as we have the page number in
+* td->pages.
+*/
+ if (td->pages[chip] != -1)
+ return td->pages[chip] >>
+ (this->bbt_erase_shift - this->page_shift);
+
+ numblocks = (int)(this->chipsize >> this->bbt_erase_shift);
+ if (!(td->options & NAND_BBT_PERCHIP))
+ numblocks *= this->numchips;
+
+ /*
+* Automatic placement of the bad block table. Search direction
+* top -> down?
+*/
+ if (td->options & NAND_BBT_LASTBLOCK) {
+ startblock = numblocks * (chip + 1) - 1;
+ dir = -1;
+ } else {
+ startblock = chip * numblocks;
+ dir = 1;
+ }
+
+ for (i = 0; i < td->maxblocks; i++) {
+ int block = startblock + dir * i;
+
+ /* Check, if the block is bad */
+ switch (bbt_get_entry(this, block)) {
+ case BBT_BLOCK_WORN:
+ case BBT_BLOCK_FACTORY_BAD:
+ continue;
+ }
+
+ page = block << (this->bbt_erase_shift - this->page_shift);
+
+ /* Check, if the block is used by the mirror table */
+ if (!md || md->pages[chip] != page)
+ return block;
+ }
+
+ return -ENOSPC;
+}
+
+/**
* write_bbt - [GENERIC] (Re)write the bad block table
* @mtd: MTD device structure
* @buf: temporary buffer
@@ -621,7 +684,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
struct nand_chip *this = mtd_to_nand(mtd);
struct erase_info einfo;
int i, res, chip = 0;
- int bits, startblock, dir, page, offs, numblocks, sft, sftmsk;
+ int bits, page, offs, numblocks, sft, sftmsk;
int nrchips, pageoffs, ooboffs;
uint8_t msk[4];
uint8_t rcode = td->reserved_block_code;
@@ -653,45 +716,20 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
/* Loop through the chips */
for (; chip < nrchips; chip++) {
- /*
-* There was already a version of the table, reuse the page
-* This applies for absolute placement too, as we have the
-* page nr. in td->pages.
-*/
- if (td->pages[chip] != -1) {
- page = td->pages[chip];
- goto write;
+ int block;
+
+ block = get_bbt_block(this, td, md, chip);
+ if (block < 0) {
+ pr_err("No space left to write bad block table\n");
+ res = block;
+ goto outerr;
}
/*
-* Automatic placement of the bad block table. Search d
[PATCH v8 2/2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Signed-off-by: Kyle Roeschley
Signed-off-by: Boris Brezillon
Suggested-by: Jeff Westfahl
Tested-by: Kyle Roeschley
---
drivers/mtd/nand/nand_bbt.c | 51 +++--
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 9fbd664..7695efe 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -668,6 +668,37 @@ static int get_bbt_block(struct nand_chip *this, struct
nand_bbt_descr *td,
}
/**
+ * mark_bbt_block_bad - Mark one of the block reserved for BBT bad
+ * @this: the NAND device
+ * @td: the BBT description
+ * @chip: the CHIP selector
+ * @block: the BBT block to mark
+ *
+ * Blocks reserved for BBT can become bad. This functions is an helper to mark
+ * such blocks as bad. It takes care of updating the in-memory BBT, marking the
+ * block as bad using a bad block marker and invalidating the associated
+ * td->pages[] entry.
+ */
+static void mark_bbt_block_bad(struct nand_chip *this,
+ struct nand_bbt_descr *td,
+ int chip, int block)
+{
+ struct mtd_info *mtd = nand_to_mtd(this);
+ loff_t to;
+ int res;
+
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ to = (loff_t)block << this->bbt_erase_shift;
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block %d bad\n",
+ res, block);
+
+ td->pages[chip] = -1;
+}
+
+/**
* write_bbt - [GENERIC] (Re)write the bad block table
* @mtd: MTD device structure
* @buf: temporary buffer
@@ -715,7 +746,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
}
/* Loop through the chips */
- for (; chip < nrchips; chip++) {
+ while (chip < nrchips) {
int block;
block = get_bbt_block(this, td, md, chip);
@@ -825,20 +856,28 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
- goto outerr;
+ if (res < 0) {
+ pr_warn("nand_bbt: error while erasing BBT block %d\n",
+ res);
+ mark_bbt_block_bad(this, td, chip, block);
+ continue;
+ }
res = scan_write_bbt(mtd, to, len, buf,
td->options & NAND_BBT_NO_OOB ? NULL :
&buf[len]);
- if (res < 0)
- goto outerr;
+ if (res < 0) {
+ pr_warn("nand_bbt: error while writing BBT block %d\n",
+ res);
+ mark_bbt_block_bad(this, td, chip, block);
+ continue;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
/* Mark it as used */
- td->pages[chip] = page;
+ td->pages[chip++] = page;
}
return 0;
--
2.8.1
Re: [PATCH v7 1/2] mtd: nand_bbt: Move BBT block selection logic out of write_bbt()
On Sat, Aug 13, 2016 at 12:37:03AM +0200, Boris Brezillon wrote:
> On Fri, 12 Aug 2016 16:58:22 -0500
> Kyle Roeschley wrote:
>
[...]
> > + while (chip < nrchips) {
>
> I'm probably missing something, but why are you turning the for loop
> into a while loop in this patch? The commit message does not mention
> that, and I don't see why you need it before you actually start
> reworking the code to recover from BBT write failures (which is done in
> patch 2).
>
You had changed it in patch 2 (http://code.bulix.org/e16nvo-104988) and I just
shuffled it to the first patch since it seemed to make sense as additional code
cleanup. I'll go ahead and drop it though if you don't want it in.
--
Kyle Roeschley
Software Engineer
National Instruments
Re: [PATCH v6 1/2] mtd: nand_bbt: Move BBT block selection logic out of write_bbt()
Hi Boris, On Fri, Aug 12, 2016 at 09:15:03PM +0200, Boris Brezillon wrote: > Hi Kyle, > > On Fri, 12 Aug 2016 12:54:49 -0500 > Kyle Roeschley wrote: > > > From: Boris Brezillon > > > > This clarifies the write_bbt() by removing the write label and clarifying > > the error/exit path. > > > > Signed-off-by: Boris Brezillon > > Just want to make sure you actually tested those patches, because I > didn't :). > > Can you add your Tested-by on this one and confirm you've tested patch > 2 as well. Whoops, I goofed and only tested with both patches applied. Thanks for the catch. I'll go ahead and test the first alone and submit a v7. Regards, -- Kyle Roeschley Software Engineer National Instruments
[PATCH v7 2/2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Signed-off-by: Kyle Roeschley
Signed-off-by: Boris Brezillon
Suggested-by: Jeff Westfahl
Tested-by: Kyle Roeschley
---
drivers/mtd/nand/nand_bbt.c | 47 +
1 file changed, 43 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 918db24..7695efe 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -668,6 +668,37 @@ static int get_bbt_block(struct nand_chip *this, struct
nand_bbt_descr *td,
}
/**
+ * mark_bbt_block_bad - Mark one of the block reserved for BBT bad
+ * @this: the NAND device
+ * @td: the BBT description
+ * @chip: the CHIP selector
+ * @block: the BBT block to mark
+ *
+ * Blocks reserved for BBT can become bad. This functions is an helper to mark
+ * such blocks as bad. It takes care of updating the in-memory BBT, marking the
+ * block as bad using a bad block marker and invalidating the associated
+ * td->pages[] entry.
+ */
+static void mark_bbt_block_bad(struct nand_chip *this,
+ struct nand_bbt_descr *td,
+ int chip, int block)
+{
+ struct mtd_info *mtd = nand_to_mtd(this);
+ loff_t to;
+ int res;
+
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ to = (loff_t)block << this->bbt_erase_shift;
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block %d bad\n",
+ res, block);
+
+ td->pages[chip] = -1;
+}
+
+/**
* write_bbt - [GENERIC] (Re)write the bad block table
* @mtd: MTD device structure
* @buf: temporary buffer
@@ -825,14 +856,22 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
- goto outerr;
+ if (res < 0) {
+ pr_warn("nand_bbt: error while erasing BBT block %d\n",
+ res);
+ mark_bbt_block_bad(this, td, chip, block);
+ continue;
+ }
res = scan_write_bbt(mtd, to, len, buf,
td->options & NAND_BBT_NO_OOB ? NULL :
&buf[len]);
- if (res < 0)
- goto outerr;
+ if (res < 0) {
+ pr_warn("nand_bbt: error while writing BBT block %d\n",
+ res);
+ mark_bbt_block_bad(this, td, chip, block);
+ continue;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
--
2.8.1
[PATCH v7 1/2] mtd: nand_bbt: Move BBT block selection logic out of write_bbt()
From: Boris Brezillon
This clarifies the write_bbt() by removing the write label and
clarifying the error/exit path.
Signed-off-by: Boris Brezillon
Tested-by: Kyle Roeschley
---
v7: Move all code cleanup into first patch
Correct documentation of mark_bbt_block_bad
Make pr_warn messages consistent
Add Tested-bys
v6: Split functionality of write_bbt out into new functions
v5: De-duplicate bad block handling
v4: Don't ignore write protection while marking bad BBT blocks
Correctly call block_markbad
Minor cleanups
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
drivers/mtd/nand/nand_bbt.c | 114 +---
1 file changed, 76 insertions(+), 38 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..918db24 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -605,6 +605,69 @@ static void search_read_bbts(struct mtd_info *mtd, uint8_t
*buf,
}
/**
+ * get_bbt_block - Get the first valid eraseblock suitable to store a BBT
+ * @this: the NAND device
+ * @td: the BBT description
+ * @md: the mirror BBT descriptor
+ * @chip: the CHIP selector
+ *
+ * This functions returns a positive block number pointing a valid eraseblock
+ * suitable to store a BBT (i.e. in the range reserved for BBT), or -ENOSPC if
+ * all blocks are already used of marked bad. If td->pages[chip] was already
+ * pointing to a valid block we re-use it, otherwise we search for the next
+ * valid one.
+ */
+static int get_bbt_block(struct nand_chip *this, struct nand_bbt_descr *td,
+struct nand_bbt_descr *md, int chip)
+{
+ int startblock, dir, page, numblocks, i;
+
+ /*
+* There was already a version of the table, reuse the page. This
+* applies for absolute placement too, as we have the page number in
+* td->pages.
+*/
+ if (td->pages[chip] != -1)
+ return td->pages[chip] >>
+ (this->bbt_erase_shift - this->page_shift);
+
+ numblocks = (int)(this->chipsize >> this->bbt_erase_shift);
+ if (!(td->options & NAND_BBT_PERCHIP))
+ numblocks *= this->numchips;
+
+ /*
+* Automatic placement of the bad block table. Search direction
+* top -> down?
+*/
+ if (td->options & NAND_BBT_LASTBLOCK) {
+ startblock = numblocks * (chip + 1) - 1;
+ dir = -1;
+ } else {
+ startblock = chip * numblocks;
+ dir = 1;
+ }
+
+ for (i = 0; i < td->maxblocks; i++) {
+ int block = startblock + dir * i;
+
+ /* Check, if the block is bad */
+ switch (bbt_get_entry(this, block)) {
+ case BBT_BLOCK_WORN:
+ case BBT_BLOCK_FACTORY_BAD:
+ continue;
+ }
+
+ page = block << (this->bbt_erase_shift - this->page_shift);
+
+ /* Check, if the block is used by the mirror table */
+ if (!md || md->pages[chip] != page)
+ return block;
+ }
+
+ return -ENOSPC;
+}
+
+/**
* write_bbt - [GENERIC] (Re)write the bad block table
* @mtd: MTD device structure
* @buf: temporary buffer
@@ -621,7 +684,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
struct nand_chip *this = mtd_to_nand(mtd);
struct erase_info einfo;
int i, res, chip = 0;
- int bits, startblock, dir, page, offs, numblocks, sft, sftmsk;
+ int bits, page, offs, numblocks, sft, sftmsk;
int nrchips, pageoffs, ooboffs;
uint8_t msk[4];
uint8_t rcode = td->reserved_block_code;
@@ -652,46 +715,21 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
}
/* Loop through the chips */
- for (; chip < nrchips; chip++) {
- /*
-* There was already a version of the table, reuse the page
-* This applies for absolute placement too, as we have the
-* page nr. in td->pages.
-*/
- if (td->pages[chip] != -1) {
- page = td->pages[chip];
- goto write;
+ while (chip < nrchips) {
+ int block;
+
+ block = get_bbt_block(this, td, md, chip);
+ if (block < 0) {
+ pr_err("No space left to write bad block table\n");
+ res = block;
+ goto outerr;
}
/*
-* Automatic placement of the bad block table. Search direction
-*
[PATCH v6 2/2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Signed-off-by: Kyle Roeschley
Signed-off-by: Boris Brezillon
Suggested-by: Jeff Westfahl
---
drivers/mtd/nand/nand_bbt.c | 57 -
1 file changed, 51 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 19f97e9..fdf9d90f 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -668,6 +668,37 @@ static int get_bbt_block(struct nand_chip *this, struct
nand_bbt_descr *td,
}
/**
+ * mark_bbt_block_bad - Mark one of the block reserved for BBT bad
+ * @this: the NAND device
+ * @td: the BBT description
+ * @md: the mirror BBT descriptor
+ * @chip: the CHIP selector
+ *
+ * Blocks reserved for BBT can become bad. This functions is an helper to mark
+ * such blocks as bad. It takes care of updating the in-memory BBT, marking the
+ * block as bad using a bad block marker and invalidating the associated
+ * td->pages[] entry.
+ */
+static void mark_bbt_block_bad(struct nand_chip *this,
+ struct nand_bbt_descr *td,
+ int chip, int block)
+{
+ struct mtd_info *mtd = nand_to_mtd(this);
+ loff_t to;
+ int res;
+
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ to = (loff_t)block << this->bbt_erase_shift;
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block %d bad\n",
+ res, block);
+
+ td->pages[chip] = -1;
+}
+
+/**
* write_bbt - [GENERIC] (Re)write the bad block table
* @mtd: MTD device structure
* @buf: temporary buffer
@@ -715,7 +746,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
}
/* Loop through the chips */
- for (; chip < nrchips; chip++) {
+ while (chip < nrchips) {
int block;
block = get_bbt_block(this, td, md, chip);
@@ -725,6 +756,12 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
goto outerr;
}
+ /*
+* get_bbt_block() returns a block number, shift the value to
+* get a page number.
+*/
+ page = block << (this->bbt_erase_shift - this->page_shift);
+
/* Set up shift count and masks for the flash table */
bits = td->options & NAND_BBT_NRBITS_MSK;
msk[2] = ~rcode;
@@ -819,20 +856,28 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
- goto outerr;
+ if (res < 0) {
+ pr_warn("nand_bbt: error while erasing BBT block %d\n",
+ res);
+ mark_bbt_block_bad(this, td, chip, block);
+ continue;
+ }
res = scan_write_bbt(mtd, to, len, buf,
td->options & NAND_BBT_NO_OOB ? NULL :
&buf[len]);
- if (res < 0)
- goto outerr;
+ if (res < 0) {
+ pr_warn("nand_bbt: error while writing bad block table
%d\n",
+ res);
+ mark_bbt_block_bad(this, td, chip, block);
+ continue;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
/* Mark it as used */
- td->pages[chip] = page;
+ td->pages[chip++] = page;
}
return 0;
--
2.8.1
[PATCH v6 1/2] mtd: nand_bbt: Move BBT block selection logic out of write_bbt()
From: Boris Brezillon
This clarifies the write_bbt() by removing the write label and clarifying
the error/exit path.
Signed-off-by: Boris Brezillon
---
v6: Split functionality of write_bbt out into new functions
v5: De-duplicate bad block handling
v4: Don't ignore write protection while marking bad BBT blocks
Correctly call block_markbad
Minor cleanups
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
drivers/mtd/nand/nand_bbt.c | 108
1 file changed, 70 insertions(+), 38 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..19f97e9 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -605,6 +605,69 @@ static void search_read_bbts(struct mtd_info *mtd, uint8_t
*buf,
}
/**
+ * get_bbt_block - Get the first valid eraseblock suitable to store a BBT
+ * @this: the NAND device
+ * @td: the BBT description
+ * @md: the mirror BBT descriptor
+ * @chip: the CHIP selector
+ *
+ * This functions returns a positive block number pointing a valid eraseblock
+ * suitable to store a BBT (i.e. in the range reserved for BBT), or -ENOSPC if
+ * all blocks are already used of marked bad. If td->pages[chip] was already
+ * pointing to a valid block we re-use it, otherwise we search for the next
+ * valid one.
+ */
+static int get_bbt_block(struct nand_chip *this, struct nand_bbt_descr *td,
+struct nand_bbt_descr *md, int chip)
+{
+ int startblock, dir, page, numblocks, i;
+
+ /*
+* There was already a version of the table, reuse the page. This
+* applies for absolute placement too, as we have the page number in
+* td->pages.
+*/
+ if (td->pages[chip] != -1)
+ return td->pages[chip] >>
+ (this->bbt_erase_shift - this->page_shift);
+
+ numblocks = (int)(this->chipsize >> this->bbt_erase_shift);
+ if (!(td->options & NAND_BBT_PERCHIP))
+ numblocks *= this->numchips;
+
+ /*
+* Automatic placement of the bad block table. Search direction
+* top -> down?
+*/
+ if (td->options & NAND_BBT_LASTBLOCK) {
+ startblock = numblocks * (chip + 1) - 1;
+ dir = -1;
+ } else {
+ startblock = chip * numblocks;
+ dir = 1;
+ }
+
+ for (i = 0; i < td->maxblocks; i++) {
+ int block = startblock + dir * i;
+
+ /* Check, if the block is bad */
+ switch (bbt_get_entry(this, block)) {
+ case BBT_BLOCK_WORN:
+ case BBT_BLOCK_FACTORY_BAD:
+ continue;
+ }
+
+ page = block << (this->bbt_erase_shift - this->page_shift);
+
+ /* Check, if the block is used by the mirror table */
+ if (!md || md->pages[chip] != page)
+ return block;
+ }
+
+ return -ENOSPC;
+}
+
+/**
* write_bbt - [GENERIC] (Re)write the bad block table
* @mtd: MTD device structure
* @buf: temporary buffer
@@ -621,7 +684,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
struct nand_chip *this = mtd_to_nand(mtd);
struct erase_info einfo;
int i, res, chip = 0;
- int bits, startblock, dir, page, offs, numblocks, sft, sftmsk;
+ int bits, page, offs, numblocks, sft, sftmsk;
int nrchips, pageoffs, ooboffs;
uint8_t msk[4];
uint8_t rcode = td->reserved_block_code;
@@ -653,45 +716,14 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
/* Loop through the chips */
for (; chip < nrchips; chip++) {
- /*
-* There was already a version of the table, reuse the page
-* This applies for absolute placement too, as we have the
-* page nr. in td->pages.
-*/
- if (td->pages[chip] != -1) {
- page = td->pages[chip];
- goto write;
- }
-
- /*
-* Automatic placement of the bad block table. Search direction
-* top -> down?
-*/
- if (td->options & NAND_BBT_LASTBLOCK) {
- startblock = numblocks * (chip + 1) - 1;
- dir = -1;
- } else {
- startblock = chip * numblocks;
- dir = 1;
- }
+ int block;
- for (i = 0; i < td->maxblocks; i++) {
- int block = startblock + dir * i;
- /* Check, if the block is bad */
- switch (bbt_get_entry(this, block)) {
- case BB
[RESEND v5] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Based on original code implemented by Jeff Westfahl
.
Signed-off-by: Kyle Roeschley
Suggested-by: Jeff Westfahl
---
v5: De-duplicate bad block handling
v4: Don't ignore write protection while marking bad BBT blocks
Correctly call block_markbad
Minor cleanups
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
drivers/mtd/nand/nand_bbt.c | 40 ++--
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..c9255f8 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -620,7 +620,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
{
struct nand_chip *this = mtd_to_nand(mtd);
struct erase_info einfo;
- int i, res, chip = 0;
+ int i, res, chip = 0, found_bad_block = 0;
int bits, startblock, dir, page, offs, numblocks, sft, sftmsk;
int nrchips, pageoffs, ooboffs;
uint8_t msk[4];
@@ -663,6 +663,27 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
goto write;
}
+next:
+ if (found_bad_block) {
+ /*
+* We found a bad block on the last loop iteration.
+* Mark it as such and see if there's another block
+* available in the BBT area.
+*/
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to erase block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ found_bad_block = 0;
+ }
+
/*
* Automatic placement of the bad block table. Search direction
* top -> down?
@@ -787,14 +808,21 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
+ if (res == -EIO) {
+ found_bad_block = 1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
- res = scan_write_bbt(mtd, to, len, buf,
- td->options & NAND_BBT_NO_OOB ? NULL :
- &buf[len]);
- if (res < 0)
+ res = scan_write_bbt(mtd, to, len, buf, td->options &
+NAND_BBT_NO_OOB ? NULL : &buf[len]);
+ if (res == -EIO) {
+ found_bad_block = 1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
--
2.8.1
[PATCH v5] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Based on original code implemented by Jeff Westfahl
.
Signed-off-by: Kyle Roeschley
Suggested-by: Jeff Westfahl
---
v5: De-duplicate bad block handling
v4: Don't ignore write protection while marking bad BBT blocks
Correctly call block_markbad
Minor cleanups
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
drivers/mtd/nand/nand_bbt.c | 41 +++--
1 file changed, 39 insertions(+), 2 deletions(-)
drivers/mtd/nand/nand_bbt.c | 40 ++--
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..c9255f8 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -620,7 +620,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
{
struct nand_chip *this = mtd_to_nand(mtd);
struct erase_info einfo;
- int i, res, chip = 0;
+ int i, res, chip = 0, found_bad_block = 0;
int bits, startblock, dir, page, offs, numblocks, sft, sftmsk;
int nrchips, pageoffs, ooboffs;
uint8_t msk[4];
@@ -663,6 +663,27 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
goto write;
}
+next:
+ if (found_bad_block) {
+ /*
+* We found a bad block on the last loop iteration.
+* Mark it as such and see if there's another block
+* available in the BBT area.
+*/
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to erase block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ found_bad_block = 0;
+ }
+
/*
* Automatic placement of the bad block table. Search direction
* top -> down?
@@ -787,14 +808,21 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
+ if (res == -EIO) {
+ found_bad_block = 1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
- res = scan_write_bbt(mtd, to, len, buf,
- td->options & NAND_BBT_NO_OOB ? NULL :
- &buf[len]);
- if (res < 0)
+ res = scan_write_bbt(mtd, to, len, buf, td->options &
+NAND_BBT_NO_OOB ? NULL : &buf[len]);
+ if (res == -EIO) {
+ found_bad_block = 1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
--
2.8.1
[RESEND PATCH v4] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Based on original code implemented by Jeff Westfahl
.
Signed-off-by: Kyle Roeschley
Suggested-by: Jeff Westfahl
---
v4: Don't ignore write protection while marking bad BBT blocks
Correctly call block_markbad
Minor cleanups
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
drivers/mtd/nand/nand_bbt.c | 41 +++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..dfc68e0 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -663,6 +663,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
goto write;
}
+next:
/*
* Automatic placement of the bad block table. Search direction
* top -> down?
@@ -787,14 +788,50 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
+ if (res == -EIO) {
+ /*
+* This block is bad. Mark it as such and see if
+* there's another block available in the BBT area.
+*/
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to erase block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
res = scan_write_bbt(mtd, to, len, buf,
td->options & NAND_BBT_NO_OOB ? NULL :
&buf[len]);
- if (res < 0)
+ if (res == -EIO) {
+ /*
+* This block is bad. Mark it as such and see if
+* there's another block available in the BBT area.
+*/
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to write block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
--
2.8.1
[PATCH v4] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Based on original code implemented by Jeff Westfahl
.
Signed-off-by: Kyle Roeschley
Suggested-by: Jeff Westfahl
---
v4: Don't ignore write protection while marking bad BBT blocks
Correctly call block_markbad
Minor cleanups
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
drivers/mtd/nand/nand_bbt.c | 41 +++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..dfc68e0 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -663,6 +663,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
goto write;
}
+next:
/*
* Automatic placement of the bad block table. Search direction
* top -> down?
@@ -787,14 +788,50 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
+ if (res == -EIO) {
+ /*
+* This block is bad. Mark it as such and see if
+* there's another block available in the BBT area.
+*/
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to erase block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
res = scan_write_bbt(mtd, to, len, buf,
td->options & NAND_BBT_NO_OOB ? NULL :
&buf[len]);
- if (res < 0)
+ if (res == -EIO) {
+ /*
+* This block is bad. Mark it as such and see if
+* there's another block available in the BBT area.
+*/
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to write block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, to);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ goto next;
+ } else if (res) {
goto outerr;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
--
2.8.1
Re: [PATCH v3] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
On Fri, Apr 29, 2016 at 09:16:31PM +0200, Boris Brezillon wrote:
> On Fri, 29 Apr 2016 12:34:18 -0500
> Kyle Roeschley wrote:
>
> > Hi Boris,
> >
> > On Wed, Mar 30, 2016 at 03:16:23PM +0200, Boris Brezillon wrote:
> > > +Peter, who's currently reworking the NAND BBT code.
> > >
> > > On Wed, 30 Mar 2016 15:13:51 +0200
> > > Boris Brezillon wrote:
> > >
> > > > Hi Kyle,
> > > >
> > > > On Fri, 25 Mar 2016 17:31:16 -0500
> > > > Kyle Roeschley wrote:
> > > >
> > > > > If erasing or writing the BBT fails, we should mark the current BBT
> > > > > block as bad and use the BBT descriptor to scan for the next available
> > > > > unused block in the BBT. We should only return a failure if there
> > > > > isn't
> > > > > any space left.
> > > > >
> > > > > Based on original code implemented by Jeff Westfahl
> > > > > .
> > > > >
> > > > > Signed-off-by: Kyle Roeschley
> > > > > Suggested-by: Jeff Westfahl
> > > > > ---
> > > > > This v3 is in response to comments from Brian Norris and Bean Ho on
> > > > > 8/26/15:
> > > > > http://lists.infradead.org/pipermail/linux-mtd/2015-August/061411.html
> > > > >
> > > > > v3: Don't overload mtd->priv
> > > > > Keep nand_erase_nand from erroring on protected BBT blocks
> > > > >
> > > > > v2: Mark OOB area in each block as well as BBT
> > > > > Avoid marking read-only, bad address, or known bad blocks as bad
> > > > > ---
> > > > > drivers/mtd/nand/nand_base.c | 4 ++--
> > > > > drivers/mtd/nand/nand_bbt.c | 37
> > > > > +++--
> > > > > 2 files changed, 37 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/mtd/nand/nand_base.c
> > > > > b/drivers/mtd/nand/nand_base.c
> > > > > index b6facac..9ad8a86 100644
> > > > > --- a/drivers/mtd/nand/nand_base.c
> > > > > +++ b/drivers/mtd/nand/nand_base.c
> > > > > @@ -2916,8 +2916,8 @@ int nand_erase_nand(struct mtd_info *mtd,
> > > > > struct erase_info *instr,
> > > > > /* Select the NAND device */
> > > > > chip->select_chip(mtd, chipnr);
> > > > >
> > > > > - /* Check, if it is write protected */
> > > > > - if (nand_check_wp(mtd)) {
> > > > > + /* Check if it is write protected, unless we're erasing BBT */
> > > > > + if (nand_check_wp(mtd) && !allowbbt) {
> > > >
> > > > Hm, will this really work. Can a write-protected device accept erase
> > > > commands?
> > > >
> >
> > Having looked into this more, no. Since v2, we called block_markbad in
> > write_bbt incorrectly and caused the chip to report that it was write
> > protected. Fixing that makes this unnecessary.
> >
> > > > > pr_debug("%s: device is write protected!\n",
> > > > > __func__);
> > > > > instr->state = MTD_ERASE_FAILED;
> > > > > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> > > > > index 2fbb523..01526e5 100644
> > > > > --- a/drivers/mtd/nand/nand_bbt.c
> > > > > +++ b/drivers/mtd/nand/nand_bbt.c
> > > > > @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd,
> > > > > uint8_t *buf,
> > > > > page = td->pages[chip];
> > > > > goto write;
> > > > > }
> > > > > + next:
> > > >
> > > > Please put this label at the beginning of the line and fix all the other
> > > > issues reported by checkpatch (I know we already have a 'write' label
> > > > which does not follow this rule, but let's try to avoid adding new
> > > > ones).
> > > >
> >
> > Will do.
> >
> > > > >
> > > > > /*
> > > > >* Automatic placement of the bad block table. Search
> > > > > direction
> > > > > @@ -787,14 +788,46 @@ static int write_bbt(struct mtd_i
Re: [PATCH v3] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
Hi Boris,
On Wed, Mar 30, 2016 at 03:16:23PM +0200, Boris Brezillon wrote:
> +Peter, who's currently reworking the NAND BBT code.
>
> On Wed, 30 Mar 2016 15:13:51 +0200
> Boris Brezillon wrote:
>
> > Hi Kyle,
> >
> > On Fri, 25 Mar 2016 17:31:16 -0500
> > Kyle Roeschley wrote:
> >
> > > If erasing or writing the BBT fails, we should mark the current BBT
> > > block as bad and use the BBT descriptor to scan for the next available
> > > unused block in the BBT. We should only return a failure if there isn't
> > > any space left.
> > >
> > > Based on original code implemented by Jeff Westfahl
> > > .
> > >
> > > Signed-off-by: Kyle Roeschley
> > > Suggested-by: Jeff Westfahl
> > > ---
> > > This v3 is in response to comments from Brian Norris and Bean Ho on
> > > 8/26/15:
> > > http://lists.infradead.org/pipermail/linux-mtd/2015-August/061411.html
> > >
> > > v3: Don't overload mtd->priv
> > > Keep nand_erase_nand from erroring on protected BBT blocks
> > >
> > > v2: Mark OOB area in each block as well as BBT
> > > Avoid marking read-only, bad address, or known bad blocks as bad
> > > ---
> > > drivers/mtd/nand/nand_base.c | 4 ++--
> > > drivers/mtd/nand/nand_bbt.c | 37 +++--
> > > 2 files changed, 37 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > > index b6facac..9ad8a86 100644
> > > --- a/drivers/mtd/nand/nand_base.c
> > > +++ b/drivers/mtd/nand/nand_base.c
> > > @@ -2916,8 +2916,8 @@ int nand_erase_nand(struct mtd_info *mtd, struct
> > > erase_info *instr,
> > > /* Select the NAND device */
> > > chip->select_chip(mtd, chipnr);
> > >
> > > - /* Check, if it is write protected */
> > > - if (nand_check_wp(mtd)) {
> > > + /* Check if it is write protected, unless we're erasing BBT */
> > > + if (nand_check_wp(mtd) && !allowbbt) {
> >
> > Hm, will this really work. Can a write-protected device accept erase
> > commands?
> >
Having looked into this more, no. Since v2, we called block_markbad in
write_bbt incorrectly and caused the chip to report that it was write
protected. Fixing that makes this unnecessary.
> > > pr_debug("%s: device is write protected!\n",
> > > __func__);
> > > instr->state = MTD_ERASE_FAILED;
> > > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> > > index 2fbb523..01526e5 100644
> > > --- a/drivers/mtd/nand/nand_bbt.c
> > > +++ b/drivers/mtd/nand/nand_bbt.c
> > > @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t
> > > *buf,
> > > page = td->pages[chip];
> > > goto write;
> > > }
> > > + next:
> >
> > Please put this label at the beginning of the line and fix all the other
> > issues reported by checkpatch (I know we already have a 'write' label
> > which does not follow this rule, but let's try to avoid adding new
> > ones).
> >
Will do.
> > >
> > > /*
> > >* Automatic placement of the bad block table. Search direction
> > > @@ -787,14 +788,46 @@ static int write_bbt(struct mtd_info *mtd, uint8_t
> > > *buf,
> > > einfo.addr = to;
> > > einfo.len = 1 << this->bbt_erase_shift;
> > > res = nand_erase_nand(mtd, &einfo, 1);
> > > - if (res < 0)
> > > + if (res == -EIO) {
> > > + /* This block is bad. Mark it as such and see if
> > > + * there's another block available in the BBT area. */
> > > + int block = page >>
> > > + (this->bbt_erase_shift - this->page_shift);
> > > + pr_info("nand_bbt: failed to erase block %d when
> > > writing BBT\n",
> > > + block);
> > > + bbt_mark_entry(this, block, BBT_BLOCK_WORN);
> > > +
> > > + res = this->block_markbad(mtd, block);
> >
> > Not sure we should mark the block bad until we managed to write a new
> > BBT. ITOH, if we do so and the new BBT write is interrupted, it
> > will trigger a
[PATCH v3] mtd: nand_bbt: scan for next free bbt block if writing bbt fails
If erasing or writing the BBT fails, we should mark the current BBT
block as bad and use the BBT descriptor to scan for the next available
unused block in the BBT. We should only return a failure if there isn't
any space left.
Based on original code implemented by Jeff Westfahl
.
Signed-off-by: Kyle Roeschley
Suggested-by: Jeff Westfahl
---
This v3 is in response to comments from Brian Norris and Bean Ho on 8/26/15:
http://lists.infradead.org/pipermail/linux-mtd/2015-August/061411.html
v3: Don't overload mtd->priv
Keep nand_erase_nand from erroring on protected BBT blocks
v2: Mark OOB area in each block as well as BBT
Avoid marking read-only, bad address, or known bad blocks as bad
---
drivers/mtd/nand/nand_base.c | 4 ++--
drivers/mtd/nand/nand_bbt.c | 37 +++--
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index b6facac..9ad8a86 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2916,8 +2916,8 @@ int nand_erase_nand(struct mtd_info *mtd, struct
erase_info *instr,
/* Select the NAND device */
chip->select_chip(mtd, chipnr);
- /* Check, if it is write protected */
- if (nand_check_wp(mtd)) {
+ /* Check if it is write protected, unless we're erasing BBT */
+ if (nand_check_wp(mtd) && !allowbbt) {
pr_debug("%s: device is write protected!\n",
__func__);
instr->state = MTD_ERASE_FAILED;
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2fbb523..01526e5 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
page = td->pages[chip];
goto write;
}
+ next:
/*
* Automatic placement of the bad block table. Search direction
@@ -787,14 +788,46 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
einfo.addr = to;
einfo.len = 1 << this->bbt_erase_shift;
res = nand_erase_nand(mtd, &einfo, 1);
- if (res < 0)
+ if (res == -EIO) {
+ /* This block is bad. Mark it as such and see if
+* there's another block available in the BBT area. */
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to erase block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, block);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ goto next;
+ } else if (res < 0) {
goto outerr;
+ }
res = scan_write_bbt(mtd, to, len, buf,
td->options & NAND_BBT_NO_OOB ? NULL :
&buf[len]);
- if (res < 0)
+ if (res == -EIO) {
+ /* This block is bad. Mark it as such and see if
+* there's another block available in the BBT area. */
+ int block = page >>
+ (this->bbt_erase_shift - this->page_shift);
+ pr_info("nand_bbt: failed to write block %d when
writing BBT\n",
+ block);
+ bbt_mark_entry(this, block, BBT_BLOCK_WORN);
+
+ res = this->block_markbad(mtd, block);
+ if (res)
+ pr_warn("nand_bbt: error %d while marking block
%d bad\n",
+ res, block);
+ td->pages[chip] = -1;
+ goto next;
+ } else if (res < 0) {
goto outerr;
+ }
pr_info("Bad block table written to 0x%012llx, version
0x%02X\n",
(unsigned long long)to, td->version[chip]);
--
2.8.0.rc3
Re: [PATCH 1/2] misc: add nirtfeatures driver
Hi Lee,
On Wed, Mar 16, 2016 at 07:48:24AM +, Lee Jones wrote:
> Is there any reason why the functionality can't be split up into
> different source files? Create an LED driver, a Switch (whatever that
> is) driver and a Watchdog driver, place them in drivers/{appropriate},
> then register from each of them using the MFD API.
I goofed a bit in the commit messages; this driver exposes LEDs and buttons
("switches") and gives access to information such as the reason for last system
reset, FPGA boot mode, and hardware revision among other things. The watchdog
was split into another driver and shouldn't have been mentioned.
As for splitting this driver up, I'm not sure I see the necessity. I'm also
unsure as to the distinction between a platform driver and an MFD driver. In
the case of this driver, the CPLD is always and only present on a specific
series of embedded controllers so I think it might be more appropriate to put
it in drivers/platform. CC-ing Darren Hart, x86 platform driver maintainer, for
input and because I forgot to copy him earlier in this chain.
Kyle Roeschley
National Instruments
Re: [PATCH 2/2] misc: nirtfeatures: physical interface elements
On Tue, Mar 15, 2016 at 09:53:27AM -0500, Josh Cartwright wrote: > Hey Kyle- > > On Mon, Mar 14, 2016 at 04:55:08PM -0500, Kyle Roeschley wrote: > > From: Gratian Crisan > > From what I understand, this was mostly Aaron's work, so he should get > authorship. I could be wrong, though, but you'll want to check. > Correct, that was just 'git format-patch' grabbing the name of the final committer on our internal repo. I'll clear that up. > > These changes add support for PIEs (physical interface elements), which > > are defined as physical elements fixed to a controller/chassis with > > which a user can interact (e.g. LEDs and switches) and whose meaning > > is user-defined and implementation-specific. > [..] > > --- > > drivers/misc/nirtfeatures.c | 753 > > > > 1 file changed, 694 insertions(+), 59 deletions(-) > > This patchset is awkwardly split up, especially because you are removing > what you are adding in the first patch. I would suggest coming up with > a better patch breakdown that doesn't do this, to make it easier on > reviewers. > > Perhaps, breaking it up into a patchset where each patch implements a > different class of functionality (leds, input). Seems reasonable, I'll try to clean this up some. > > Josh -- Kyle Roeschley Software Engineer National Instruments
Re: [PATCH 1/2] misc: add nirtfeatures driver
On Mon, Mar 14, 2016 at 05:30:22PM -0500, Josh Cartwright wrote: > On Mon, Mar 14, 2016 at 03:05:59PM -0700, Greg KH wrote: > > On Mon, Mar 14, 2016 at 04:54:32PM -0500, Kyle Roeschley wrote: > > > From: Jeff Westfahl > > > > > > This driver introduces support for hardware features of National > > > Instruments real-time controllers. This is an ACPI device that exposes > > > LEDs, switches, and watchdogs. > > > > If it's an acpi driver, why not put it in drivers/acpi? > > For the same reason we don't move all drivers for devices-on-a-PCI-bus > into drivers/pci? > > Drivers typically exist in the sourcetree with other drivers which > implement similar functionality, which works great for devices with > clear functional boundaries (GPIO controller drivers in drivers/gpio, > led drivers in drivers/leds, etc. etc.); but for devices which are a > hodgepodge of functionality, there isn't really a good fit anywhere > except maybe in misc or mfd. > > We could move it to mfd, but drivers in drivers/mfd which don't make use > of MFD_CORE seems equally strange (although, I suppose there is > precedent). Maybe Lee has some thoughts. Maybe drivers/platform? I notice several ACPI drivers there that also have grab bags of functionality. Regards, Kyle Roeschley National Instruments
[PATCH 2/2] misc: nirtfeatures: physical interface elements
From: Gratian Crisan
These changes add support for PIEs (physical interface elements), which
are defined as physical elements fixed to a controller/chassis with
which a user can interact (e.g. LEDs and switches) and whose meaning
is user-defined and implementation-specific.
The support for these elements, in terms of enumerating and interacting
with them (i.e. retrieving the list of elements, getting/setting their
current state, enabling notifications, etc.) is embedded within the
BIOS as a set of ACPI methods. The changes to the CPLD driver act as a
bridge between these methods and existing Linux kernel facilities as
described below to expose the elements and any applicable metadata to
user mode. The metadata or knowledge needed for the interpretation
thereof is not a prerequisite to interacting with the elements--it is
there for upper-level value add software to use to improve the user
experience. In other words, Linux users familiar with the class drivers
by which the elements are surfaced should not have any issues
interfacing with them without knowing the meaning of the attached
metadata.
Output elements, which consist currently of LEDs, are surfaced via the
LED class driver. Each LED and color becomes its own LED class device
with the naming convention 'nilrt:{name}:{color}'. Any additional
attributes/metadata intended for upper-level software are appended to
the name, each separated by colons, as suggested by the LED class driver
documentation in the Linux kernel proper, except where there is already
a standard way to communicate a specific piece of metadata (e.g.,
maximum brightness, which is exposed via the /sys/class/leds/.../
max_brightness attribute node).
Input elements as surfaced via the input class driver. As with output
elements, each input element registers its own separate driver whose
name and associated metadata are transmitted via the name attribute
attached to the input device, retrievable via the EVIOCGNAME ioctl,
using the same convention as described above for output elements. The
input class driver model is that events are pushed (i.e. reported) to
indicate state changes, so to facilitate this, the CPLD driver has an
ACPI notify callback that is invoked when an input element changes state
and its BIOS support generates a general purpose event per the ACPI
GPE model. The notify callback checks the instantaneous state of the
input element and reports a keyboard event on its particular device with
a scan code of 256 (BTN_0), where a key down event means that the input
element is in the '1' state (down, engaged, on, pressed, etc.) and a key
up event means that the input element is in the '0' state (off,
disengaged, released, etc.). User mode software can then monitor for
these specific events to determine when the state of the element has
changed, or can use the EVIOCGKEY ioctl on the appropriate input device
to retrieve the instantaneous state of the element.
Signed-off-by: Aaron Rossetto
Signed-off-by: Josh Cartwright
Signed-off-by: Gratian Crisan
Signed-off-by: Kyle Roeschley
---
drivers/misc/nirtfeatures.c | 753
1 file changed, 694 insertions(+), 59 deletions(-)
diff --git a/drivers/misc/nirtfeatures.c b/drivers/misc/nirtfeatures.c
index 37298f2..2feedb5 100644
--- a/drivers/misc/nirtfeatures.c
+++ b/drivers/misc/nirtfeatures.c
@@ -20,6 +20,8 @@
#include
#include
+#include
+
#define MODULE_NAME "nirtfeatures"
/* Register addresses */
@@ -60,13 +62,48 @@
#define NIRTF_SYSTEM_LEDS_POWER_GREEN 0x02
#define NIRTF_SYSTEM_LEDS_POWER_YELLOW 0x01
-#define NIRTF_RT_LEDS_USER2_GREEN 0x08
-#define NIRTF_RT_LEDS_USER2_YELLOW 0x04
-#define NIRTF_RT_LEDS_USER1_GREEN 0x02
-#define NIRTF_RT_LEDS_USER1_YELLOW 0x01
-
#define to_nirtfeatures(dev) acpi_driver_data(to_acpi_device(dev))
+/*
+ *=
+ * ACPI NI physical interface element support
+ *=
+ */
+#define MAX_NAMELEN64
+#define MAX_NODELEN128
+#define MIN_PIE_CAPS_VERSION 2
+#define MAX_PIE_CAPS_VERSION 2
+
+enum nirtfeatures_pie_class {
+ PIE_CLASS_INPUT = 0,
+ PIE_CLASS_OUTPUT = 1
+};
+
+enum nirtfeatures_pie_type {
+ PIE_TYPE_UNKNOWN = 0,
+ PIE_TYPE_SWITCH = 1,
+ PIE_TYPE_LED = 2
+};
+
+struct nirtfeatures_pie_descriptor {
+ char name[MAX_NAMELEN];
+ enum nirtfeatures_pie_class pie_class;
+ enum nirtfeatures_pie_type pie_type;
+ bool is_user_visible;
+ unsigned int notification_value;
+};
+
+struct nirtfeatures_pie_descriptor_led_color {
+ char name[MAX_NAMELEN];
+ int brightness_range_low;
+ int brightness_range_high;
+};
+
+struct nirtfeatures_pie_location {
+ unsigned int element;
+ unsigned int subelement;
+};
+
/* Structures */
[PATCH 1/2] misc: add nirtfeatures driver
From: Jeff Westfahl
This driver introduces support for hardware features of National
Instruments real-time controllers. This is an ACPI device that exposes
LEDs, switches, and watchdogs.
Signed-off-by: Jeff Westfahl
Signed-off-by: Kyle Roeschley
---
drivers/misc/Kconfig| 9 +
drivers/misc/Makefile | 1 +
drivers/misc/nirtfeatures.c | 575
3 files changed, 585 insertions(+)
create mode 100644 drivers/misc/nirtfeatures.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 1557951..d97f71e 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -526,6 +526,15 @@ config VEXPRESS_SYSCFG
bus. System Configuration interface is one of the possible means
of generating transactions on this bus.
+config NI_RT_FEATURES
+ bool "NI 903x/913x support"
+ depends on X86 && ACPI
+ help
+ This driver exposes LEDs and other features of NI 903x/913x Real-Time
+ controllers.
+
+ If unsure, say N (but it's safe to say "Y").
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 537d7f3..539127d 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/
obj-$(CONFIG_ECHO) += echo/
obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
obj-$(CONFIG_CXL_BASE) += cxl/
+obj-$(CONFIG_NI_RT_FEATURES) += nirtfeatures.o
diff --git a/drivers/misc/nirtfeatures.c b/drivers/misc/nirtfeatures.c
new file mode 100644
index 000..37298f2
--- /dev/null
+++ b/drivers/misc/nirtfeatures.c
@@ -0,0 +1,575 @@
+/*
+ * Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MODULE_NAME "nirtfeatures"
+
+/* Register addresses */
+
+#define NIRTF_YEAR 0x01
+#define NIRTF_MONTH0x02
+#define NIRTF_DAY 0x03
+#define NIRTF_HOUR 0x04
+#define NIRTF_MINUTE 0x05
+#define NIRTF_SCRATCH 0x06
+#define NIRTF_PLATFORM_MISC0x07
+#define NIRTF_PROC_RESET_SOURCE0x11
+#define NIRTF_CONTROLLER_MODE 0x12
+#define NIRTF_SYSTEM_LEDS 0x20
+#define NIRTF_STATUS_LED_SHIFT10x21
+#define NIRTF_STATUS_LED_SHIFT00x22
+#define NIRTF_RT_LEDS 0x23
+
+#define NIRTF_IO_SIZE 0x40
+
+/* Register values */
+
+#define NIRTF_PLATFORM_MISC_ID_MASK0x07
+#define NIRTF_PLATFORM_MISC_ID_MANHATTAN 0
+#define NIRTF_PLATFORM_MISC_ID_HAMMERHEAD 4
+#define NIRTF_PLATFORM_MISC_ID_WINGHEAD5
+
+#define NIRTF_CONTROLLER_MODE_NO_FPGA_SW 0x40
+#define NIRTF_CONTROLLER_MODE_HARD_BOOT_N 0x20
+#define NIRTF_CONTROLLER_MODE_NO_FPGA 0x10
+#define NIRTF_CONTROLLER_MODE_RECOVERY 0x08
+#define NIRTF_CONTROLLER_MODE_CONSOLE_OUT 0x04
+#define NIRTF_CONTROLLER_MODE_IP_RESET 0x02
+#define NIRTF_CONTROLLER_MODE_SAFE 0x01
+
+#define NIRTF_SYSTEM_LEDS_STATUS_RED 0x08
+#define NIRTF_SYSTEM_LEDS_STATUS_YELLOW0x04
+#define NIRTF_SYSTEM_LEDS_POWER_GREEN 0x02
+#define NIRTF_SYSTEM_LEDS_POWER_YELLOW 0x01
+
+#define NIRTF_RT_LEDS_USER2_GREEN 0x08
+#define NIRTF_RT_LEDS_USER2_YELLOW 0x04
+#define NIRTF_RT_LEDS_USER1_GREEN 0x02
+#define NIRTF_RT_LEDS_USER1_YELLOW 0x01
+
+#define to_nirtfeatures(dev) acpi_driver_data(to_acpi_device(dev))
+
+/* Structures */
+
+struct nirtfeatures {
+ struct acpi_device *acpi_device;
+ u16 io_base;
+ spinlock_t lock;
+ u8 revision[5];
+ const char *bpstring;
+ struct nirtfeatures_led *extra_leds;
+ unsigned num_extra_leds;
+};
+
+struct nirtfeatures_led {
+ struct led_classdev cdev;
+ struct nirtfeatures *nirtfeatures;
+ u8 address;
+ u8 mask;
+ u8 pattern_hi_addr;
+ u8 pattern_lo_addr;
+};
+
+/* sysfs files */
+
+static ssize_t revision_show(struct device *dev, struct device_attribute *attr,
+char *buf)
+{
+ struct nirtfeatures *nirtfeatures = to_nirtfeatures(dev);
+
+ return sprintf(buf, "20%02X/%02X/%02X %02X:%02X\n",
+ nirtfeatures->revision[0], nir
