* [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver @ 2011-05-04 19:10 Chris Metcalf 2011-05-05 6:41 ` Arnd Bergmann 0 siblings, 1 reply; 27+ messages in thread From: Chris Metcalf @ 2011-05-04 19:10 UTC (permalink / raw) To: linux-kernel, Arnd Bergmann This commit does two things: it adds the infrastructure for a new arch/tile/drivers/ directory, and it populates it with the first instance of a driver for that directory, a SPI Flash ROM (srom) driver. The directory is motivated as follows. While some classes of driver implementations should be grouped together so they are easily modified as a class (network, ATA, RTC, PCI, I2C, etc etc) there are "miscellaneous" drivers that don't benefit from any sharing with other driver implementations. If those drivers are for hardware that can plausibly be used by multiple architectures, it makes sense to put them somewhere like drivers/char. But if they are only usable on a single architecture (in this case drivers written for the Tilera para-virtualization model using our hypervisor) it makes sense to group such drivers with their architecture, to avoid cluttering the "drivers" hierarchy for other architectures that can't use that driver. The actual SROM driver is fairly uncontroversial, and is just a simple driver that allows user space to read and write the SROM at a raw level. (A separate MTD driver exists for "tile", but this is not that driver.) The driver is particularly useful since the Tile chip can boot directly from the SROM, so providing this driver interface allows for updating the boot image prior to a reboot. Signed-off-by: Chris Metcalf <cmetcalf@tilera.com> --- arch/tile/Kconfig | 2 + arch/tile/Makefile | 2 + arch/tile/configs/tile_defconfig | 1 + arch/tile/configs/tilegx_defconfig | 1 + arch/tile/drivers/Kconfig | 8 + arch/tile/drivers/Makefile | 13 + arch/tile/drivers/srom.c | 499 ++++++++++++++++++++++++++++++++++ arch/tile/include/hv/drv_srom_intf.h | 41 +++ 8 files changed, 567 insertions(+), 0 deletions(-) create mode 100644 arch/tile/drivers/Kconfig create mode 100644 arch/tile/drivers/Makefile create mode 100644 arch/tile/drivers/srom.c create mode 100644 arch/tile/include/hv/drv_srom_intf.h diff --git a/arch/tile/Kconfig b/arch/tile/Kconfig index 635e1bf..be47e07 100644 --- a/arch/tile/Kconfig +++ b/arch/tile/Kconfig @@ -315,6 +315,8 @@ config KERNEL_PL kernel will be built to run at. Generally you should use the default value here. +source "arch/tile/drivers/Kconfig" + endmenu # Tilera-specific configuration menu "Bus options" diff --git a/arch/tile/Makefile b/arch/tile/Makefile index 17acce7..d7f778c 100644 --- a/arch/tile/Makefile +++ b/arch/tile/Makefile @@ -50,6 +50,8 @@ head-y := arch/tile/kernel/head_$(BITS).o libs-y += arch/tile/lib/ libs-y += $(LIBGCC_PATH) +drivers-y += arch/tile/drivers/ + # See arch/tile/Kbuild for content of core part of the kernel core-y += arch/tile/ diff --git a/arch/tile/configs/tile_defconfig b/arch/tile/configs/tile_defconfig index cb47612..041c456 100644 --- a/arch/tile/configs/tile_defconfig +++ b/arch/tile/configs/tile_defconfig @@ -224,6 +224,7 @@ CONFIG_DEFAULT_MMAP_MIN_ADDR=4096 CONFIG_VMALLOC_RESERVE=0x1000000 CONFIG_HARDWALL=y CONFIG_KERNEL_PL=1 +CONFIG_TILE_SROM=y # # Bus options diff --git a/arch/tile/configs/tilegx_defconfig b/arch/tile/configs/tilegx_defconfig index 776ec0c..6343962 100644 --- a/arch/tile/configs/tilegx_defconfig +++ b/arch/tile/configs/tilegx_defconfig @@ -251,6 +251,7 @@ CONFIG_DEFAULT_MMAP_MIN_ADDR=4096 CONFIG_VMALLOC_RESERVE=0x1000000 CONFIG_HARDWALL=y CONFIG_KERNEL_PL=1 +CONFIG_TILE_SROM=y # # Bus options diff --git a/arch/tile/drivers/Kconfig b/arch/tile/drivers/Kconfig new file mode 100644 index 0000000..e1f9551 --- /dev/null +++ b/arch/tile/drivers/Kconfig @@ -0,0 +1,8 @@ +config TILE_SROM + bool "Character-device access to the Tilera on-board SROM" + default y + ---help--- + This device provides character-level read-write access + to the SROM, typically via the "0", "1", "2", and "info" + devices in /dev/srom/. + diff --git a/arch/tile/drivers/Makefile b/arch/tile/drivers/Makefile new file mode 100644 index 0000000..311e741 --- /dev/null +++ b/arch/tile/drivers/Makefile @@ -0,0 +1,13 @@ +# +# Makefile for TILE drivers that don't belong elsewhere in the tree. +# Typically these are drivers that just present a simple char +# file_operations interface. +# +# For other Tilera-specific drivers, see: +# +# drivers/net/tile/ network +# drivers/tty/hvc/hvc_tile.c hypervisor console +# drivers/edac/tile_edac.c EDAC driver +# + +obj-$(CONFIG_TILE_SROM) += srom.o diff --git a/arch/tile/drivers/srom.c b/arch/tile/drivers/srom.c new file mode 100644 index 0000000..1a5b070 --- /dev/null +++ b/arch/tile/drivers/srom.c @@ -0,0 +1,499 @@ +/* + * Copyright 2011 Tilera Corporation. All Rights Reserved. + * + * 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, version 2. + * + * 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, GOOD TITLE or + * NON INFRINGEMENT. See the GNU General Public License for + * more details. + * + * SPI Flash ROM driver + * + * This source code is derived from code provided in "Linux Device + * Drivers" by Alessandro Rubini and Jonathan Corbet, published by + * O'Reilly & Associates. + */ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/kernel.h> /* printk() */ +#include <linux/slab.h> /* kmalloc() */ +#include <linux/fs.h> /* everything... */ +#include <linux/errno.h> /* error codes */ +#include <linux/types.h> /* size_t */ +#include <linux/proc_fs.h> +#include <linux/fcntl.h> /* O_ACCMODE */ +#include <linux/aio.h> +#include <linux/pagemap.h> +#include <linux/hugetlb.h> +#include <linux/uaccess.h> +#include <hv/hypervisor.h> +#include <linux/ioctl.h> +#include <linux/cdev.h> +#include <linux/delay.h> +#include <hv/drv_srom_intf.h> + +/* + * Size of our hypervisor I/O requests. We break up large transfers + * so that we don't spend large uninterrupted spans of time in the + * hypervisor. Erasing an SROM sector takes a significant fraction of + * a second, so if we allowed the user to, say, do one I/O to write the + * entire ROM, we'd get soft lockup timeouts, or worse. + */ +#define SROM_CHUNK_SIZE 4096 + +/* + * When hypervisor is busy (e.g. erasing), poll the status periodically. + */ + +/* + * Interval to poll the state in msec + */ +#define SROM_WAIT_TRY_INTERVAL 20 + +/* + * Maximum times to poll the state + */ +#define SROM_MAX_WAIT_TRY_TIMES 1000 + +struct srom_dev { + struct cdev cdev; /* Character device structure */ + int hv_devhdl; /* Handle for hypervisor device */ + u32 size; /* Size of this device */ + char *info_string; /* String returned by the info device + if this is it; NULL otherwise */ + struct mutex srom_lock; /* Allow only one accessor at a time */ +}; + +static int srom_major; /* Dynamic major by default */ +static int srom_devs = 4; /* One per SROM partition plus one info file */ + +/* Minor number of the info file */ +static inline int srom_info_minor(void) +{ + return srom_devs - 1; +} + +module_param(srom_major, int, 0); +module_param(srom_devs, int, 0); +MODULE_AUTHOR("Tilera Corporation"); +MODULE_LICENSE("Dual BSD/GPL"); + +static struct srom_dev *srom_devices; /* allocated in srom_init */ + +/** + * Macro to complete a read/write transaction. Returns last hv + * syscall return value. + */ +#define SROM_RD_OR_WR(func, ...) \ + ({ \ + int _hv_retval = func(__VA_ARGS__); \ + int retries; \ + \ + while (_hv_retval < 0) { \ + if (_hv_retval == HV_EBUSY) { \ + retries = SROM_MAX_WAIT_TRY_TIMES; \ + \ + while (retries > 0) { \ + _hv_retval = func(__VA_ARGS__); \ + if (_hv_retval == HV_EBUSY) { \ + msleep(SROM_WAIT_TRY_INTERVAL); \ + retries--; \ + } else { \ + break; \ + } \ + } \ + \ + if (_hv_retval == HV_EBUSY) { \ + pr_err("srom: "#func" failed (" \ + "timeout)\n"); \ + retval = -EIO; \ + goto op_done; \ + } \ + } else if (_hv_retval == HV_EAGAIN) { \ + _hv_retval = func(__VA_ARGS__); \ + } else { \ + pr_err("srom: "#func" failed, " \ + "error %d\n", _hv_retval); \ + retval = -EIO; \ + goto op_done; \ + } \ + } \ + _hv_retval; \ + }) + +/** + * srom_open() - Device open routine. + * @inode: Inode for this device. + * @filp: File for this specific open of the device. + * + * Returns zero, or an error code. + */ +static int srom_open(struct inode *inode, struct file *filp) +{ + struct srom_dev *dev; + + /* Find the device */ + dev = container_of(inode->i_cdev, struct srom_dev, cdev); + + /* Now open the hypervisor device if we haven't already. */ + if (dev->hv_devhdl == 0) { + char buf[20]; + int instance = iminor(inode); + if (instance != srom_info_minor()) { + sprintf(buf, "srom/0/%d", instance); + dev->hv_devhdl = hv_dev_open((HV_VirtAddr)buf, 0); + if (dev->hv_devhdl > 0) { + dev->size = 0; + if (mutex_lock_interruptible(&dev->srom_lock)) + return -ERESTARTSYS; + hv_dev_pread(dev->hv_devhdl, 0, + (HV_VirtAddr)&dev->size, + sizeof(dev->size), + SROM_TOTAL_SIZE_OFF); + mutex_unlock(&dev->srom_lock); + } + } else { + u32 sector_size = 0; + u32 page_size = 0; + + sprintf(buf, "srom/0/0"); + dev->hv_devhdl = hv_dev_open((HV_VirtAddr)buf, 0); + if (dev->hv_devhdl > 0) { + static const int info_string_size = 80; + + if (mutex_lock_interruptible(&dev->srom_lock)) + return -ERESTARTSYS; + hv_dev_pread(dev->hv_devhdl, 0, + (HV_VirtAddr)§or_size, + sizeof(sector_size), + SROM_SECTOR_SIZE_OFF); + hv_dev_pread(dev->hv_devhdl, 0, + (HV_VirtAddr)&page_size, + sizeof(page_size), + SROM_PAGE_SIZE_OFF); + mutex_unlock(&dev->srom_lock); + + dev->info_string = kmalloc(info_string_size, + GFP_KERNEL); + snprintf(dev->info_string, info_string_size, + "sector_size: %d\npage_size: %d\n", + sector_size, page_size); + } + } + } + + /* If we tried and failed to open it, fail. */ + if (dev->hv_devhdl < 0) { + switch (dev->hv_devhdl) { + case HV_ENODEV: + return -ENODEV; + default: + return (ssize_t)dev->hv_devhdl; + } + } + + filp->private_data = dev; + + return 0; /* success */ +} + + +/** + * srom_release() - Device release routine. + * @inode: Inode for this device. + * @filp: File for this specific open of the device. + * + * Returns zero, or an error code. + */ +static int srom_release(struct inode *inode, struct file *filp) +{ + struct srom_dev *dev = filp->private_data; + int retval = 0; + char dummy; + + /* + * First we need to make sure we've flushed anything written to + * the ROM. + */ + if (mutex_lock_interruptible(&dev->srom_lock)) { + retval = -ERESTARTSYS; + filp->private_data = NULL; + return retval; + } + + if (dev->hv_devhdl > 0) { + SROM_RD_OR_WR(hv_dev_pwrite, dev->hv_devhdl, + 0, (HV_VirtAddr)&dummy, 1, SROM_FLUSH_OFF); + } + +op_done: + mutex_unlock(&dev->srom_lock); + filp->private_data = NULL; + + return retval; +} + + +/** + * srom_read() - Read (control) data from the device. + * @filp: File for this specific open of the device. + * @buf: User's data buffer. + * @count: Number of bytes requested. + * @f_pos: File position. + * + * Returns number of bytes read, or an error code. + */ +static ssize_t srom_read(struct file *filp, char __user *buf, + size_t count, loff_t *f_pos) +{ + int retval = 0; + void *kernbuf; + struct srom_dev *dev = filp->private_data; + + if (dev->hv_devhdl < 0) + return -EINVAL; + + if (dev->info_string) { + int info_len = strlen(dev->info_string); + int bytes_avail = info_len - *f_pos; + int xfer_len = (bytes_avail < count) ? bytes_avail : count; + + if (xfer_len <= 0) + return 0; + + if (copy_to_user(buf, dev->info_string + *f_pos, xfer_len)) + return -EFAULT; + *f_pos += xfer_len; + return xfer_len; + } + + kernbuf = kmalloc(SROM_CHUNK_SIZE, GFP_KERNEL); + if (!kernbuf) + return -ENOMEM; + + if (mutex_lock_interruptible(&dev->srom_lock)) { + retval = -ERESTARTSYS; + kfree(kernbuf); + return retval; + } + + while (count) { + int hv_retval; + int bytes_this_pass = count; + + if (bytes_this_pass > SROM_CHUNK_SIZE) + bytes_this_pass = SROM_CHUNK_SIZE; + + hv_retval = SROM_RD_OR_WR(hv_dev_pread, dev->hv_devhdl, + 0, (HV_VirtAddr) kernbuf, bytes_this_pass, + *f_pos); + + if (hv_retval > 0) { + if (copy_to_user(buf, kernbuf, hv_retval) != 0) { + retval = -EFAULT; + break; + } + } else if (hv_retval == 0) { + break; + } + + retval += hv_retval; + *f_pos += hv_retval; + buf += hv_retval; + count -= hv_retval; + } + +op_done: + mutex_unlock(&dev->srom_lock); + kfree(kernbuf); + + return retval; +} + +/** + * srom_write() - Write (control) data to the device. + * @filp: File for this specific open of the device. + * @buf: User's data buffer. + * @count: Number of bytes requested. + * @f_pos: File position. + * + * Returns number of bytes written, or an error code. + */ +static ssize_t srom_write(struct file *filp, const char __user *buf, + size_t count, loff_t *f_pos) +{ + int retval = 0; + void *kernbuf; + struct srom_dev *dev = filp->private_data; + + if (dev->hv_devhdl < 0 || dev->info_string) + return -EINVAL; + + kernbuf = kmalloc(SROM_CHUNK_SIZE, GFP_KERNEL); + if (!kernbuf) + return -ENOMEM; + + if (mutex_lock_interruptible(&dev->srom_lock)) { + retval = -ERESTARTSYS; + kfree(kernbuf); + return retval; + } + + while (count) { + int hv_retval; + int bytes_this_pass = count; + + if (bytes_this_pass > SROM_CHUNK_SIZE) + bytes_this_pass = SROM_CHUNK_SIZE; + + if (copy_from_user(kernbuf, buf, bytes_this_pass) != 0) { + retval = -EFAULT; + break; + } + + hv_retval = SROM_RD_OR_WR(hv_dev_pwrite, dev->hv_devhdl, + 0, (HV_VirtAddr) kernbuf, bytes_this_pass, + *f_pos); + + if (hv_retval == 0) + break; + + retval += hv_retval; + *f_pos += hv_retval; + buf += hv_retval; + count -= hv_retval; + } + +op_done: + mutex_unlock(&dev->srom_lock); + kfree(kernbuf); + + return retval; +} + +/** + * srom_llseek() - Change the current device offset. + * @filp: File for this specific open of the device. + * @off: New offset value. + * @whence: Base for new offset value. + * + * Returns new offset, or an error code. + */ +static loff_t srom_llseek(struct file *filp, loff_t off, int whence) +{ + struct srom_dev *dev = filp->private_data; + long newpos; + + switch (whence) { + case 0: /* SEEK_SET */ + newpos = off; + break; + + case 1: /* SEEK_CUR */ + newpos = filp->f_pos + off; + break; + + case 2: /* SEEK_END */ + newpos = dev->size + off; + break; + + default: /* can't happen */ + return -EINVAL; + } + + if (newpos < 0 || newpos > dev->size) + return -EINVAL; + + filp->f_pos = newpos; + return newpos; +} + + +/* + * The fops + */ +static const struct file_operations srom_fops = { + .owner = THIS_MODULE, + .llseek = srom_llseek, + .read = srom_read, + .write = srom_write, + .open = srom_open, + .release = srom_release, +}; + +/** + * srom_setup_cdev() - Set up a device instance in the cdev table. + * @dev: Per-device SROM state. + * @index: Device to set up. + */ +static void srom_setup_cdev(struct srom_dev *dev, int index) +{ + int err, devno = MKDEV(srom_major, index); + + cdev_init(&dev->cdev, &srom_fops); + dev->cdev.owner = THIS_MODULE; + dev->cdev.ops = &srom_fops; + err = cdev_add(&dev->cdev, devno, 1); + /* Fail gracefully if need be */ + if (err) + pr_notice("Error %d adding srom%d", err, index); +} + +/** srom_init() - Initialize the driver's module. */ +static int srom_init(void) +{ + int result, i; + dev_t dev = MKDEV(srom_major, 0); + + /* + * Register our major, and accept a dynamic number. + */ + if (srom_major) + result = register_chrdev_region(dev, srom_devs, "srom"); + else { + result = alloc_chrdev_region(&dev, 0, srom_devs, "srom"); + srom_major = MAJOR(dev); + } + if (result < 0) + return result; + + + /* + * Allocate the devices -- we can't have them static, as the number + * can be specified at load time. + */ + srom_devices = kzalloc(srom_devs * sizeof(struct srom_dev), + GFP_KERNEL); + if (!srom_devices) { + unregister_chrdev_region(dev, srom_devs); + return -ENOMEM; + } + for (i = 0; i < srom_devs; i++) { + srom_setup_cdev(srom_devices + i, i); + mutex_init(&srom_devices[i].srom_lock); + } + + return 0; /* succeed */ +} + +/** srom_cleanup() - Clean up the driver's module. */ +static void srom_cleanup(void) +{ + int i; + + for (i = 0; i < srom_devs; i++) { + cdev_del(&srom_devices[i].cdev); + kfree(srom_devices[i].info_string); + } + kfree(srom_devices); + unregister_chrdev_region(MKDEV(srom_major, 0), srom_devs); +} + +module_init(srom_init); +module_exit(srom_cleanup); diff --git a/arch/tile/include/hv/drv_srom_intf.h b/arch/tile/include/hv/drv_srom_intf.h new file mode 100644 index 0000000..6395faa --- /dev/null +++ b/arch/tile/include/hv/drv_srom_intf.h @@ -0,0 +1,41 @@ +/* + * Copyright 2011 Tilera Corporation. All Rights Reserved. + * + * 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, version 2. + * + * 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, GOOD TITLE or + * NON INFRINGEMENT. See the GNU General Public License for + * more details. + */ + +/** + * @file drv_srom_intf.h + * Interface definitions for the SPI Flash ROM driver. + */ + +#ifndef _SYS_HV_INCLUDE_DRV_SROM_INTF_H +#define _SYS_HV_INCLUDE_DRV_SROM_INTF_H + +/** Read this offset to get the total device size. */ +#define SROM_TOTAL_SIZE_OFF 0xF0000000 + +/** Read this offset to get the device sector size. */ +#define SROM_SECTOR_SIZE_OFF 0xF0000004 + +/** Read this offset to get the device page size. */ +#define SROM_PAGE_SIZE_OFF 0xF0000008 + +/** Write this offset to flush any pending writes. */ +#define SROM_FLUSH_OFF 0xF1000000 + +/** Write this offset, plus the byte offset of the start of a sector, to + * erase a sector. Any write data is ignored, but there must be at least + * one byte of write data. Only applies when the driver is in MTD mode. + */ +#define SROM_ERASE_OFF 0xF2000000 + +#endif /* _SYS_HV_INCLUDE_DRV_SROM_INTF_H */ -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-04 19:10 [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver Chris Metcalf @ 2011-05-05 6:41 ` Arnd Bergmann 2011-05-06 19:37 ` Chris Metcalf 2011-05-20 18:05 ` Chris Metcalf 0 siblings, 2 replies; 27+ messages in thread From: Arnd Bergmann @ 2011-05-05 6:41 UTC (permalink / raw) To: Chris Metcalf; +Cc: linux-kernel On Wednesday 04 May 2011, Chris Metcalf wrote: > This commit does two things: it adds the infrastructure for a new > arch/tile/drivers/ directory, and it populates it with the first > instance of a driver for that directory, a SPI Flash ROM (srom) driver. > > The directory is motivated as follows. While some classes of > driver implementations should be grouped together so they are easily > modified as a class (network, ATA, RTC, PCI, I2C, etc etc) there are > "miscellaneous" drivers that don't benefit from any sharing with other > driver implementations. If those drivers are for hardware that can > plausibly be used by multiple architectures, it makes sense to put > them somewhere like drivers/char. But if they are only usable on > a single architecture (in this case drivers written for the Tilera > para-virtualization model using our hypervisor) it makes sense to group > such drivers with their architecture, to avoid cluttering the "drivers" > hierarchy for other architectures that can't use that driver. I generally advise against putting any device drivers into arch/*/, on the ground that it is much easier to find similar drivers when they are in a common place. We probably have a few similar drivers in the tree already, e.g drivers/misc/eeprom/* and drivers/char/ps3flash.c and drivers/platform/x86/intel_scu_ipcutil.c are examples of preexisting drivers. I'd probably put this one in driver/misc/eeprom and make the interface look like the other ones (sysfs bin attribute instead of character device), which is a trivial change. Alternatively, you could create drivers/platform/tile to hold tile specific device drivers, instead of keeping them under arch/tile. > The actual SROM driver is fairly uncontroversial, and is just a simple > driver that allows user space to read and write the SROM at a raw level. > (A separate MTD driver exists for "tile", but this is not that driver.) > The driver is particularly useful since the Tile chip can boot directly > from the SROM, so providing this driver interface allows for updating > the boot image prior to a reboot. I think the sysfs bin attribute works well here because you don't need any ioctl, and the contents are basically a representation of a flat file. The implementation would be almost the same. > +/** > + * srom_llseek() - Change the current device offset. > + * @filp: File for this specific open of the device. > + * @off: New offset value. > + * @whence: Base for new offset value. > + * > + * Returns new offset, or an error code. > + */ > +static loff_t srom_llseek(struct file *filp, loff_t off, int whence) > +{ > + struct srom_dev *dev = filp->private_data; > + long newpos; > + > + switch (whence) { > + case 0: /* SEEK_SET */ > + newpos = off; > + break; > + > + case 1: /* SEEK_CUR */ > + newpos = filp->f_pos + off; > + break; > + > + case 2: /* SEEK_END */ > + newpos = dev->size + off; > + break; > + > + default: /* can't happen */ > + return -EINVAL; > + } > + > + if (newpos < 0 || newpos > dev->size) > + return -EINVAL; > + > + filp->f_pos = newpos; > + return newpos; > +} This looks unnecessary, just use generic_file_llseek. Arnd ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-05 6:41 ` Arnd Bergmann @ 2011-05-06 19:37 ` Chris Metcalf 2011-05-20 18:05 ` Chris Metcalf 1 sibling, 0 replies; 27+ messages in thread From: Chris Metcalf @ 2011-05-06 19:37 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linux-kernel On 5/5/2011 2:41 AM, Arnd Bergmann wrote: > On Wednesday 04 May 2011, Chris Metcalf wrote: >> This commit does two things: it adds the infrastructure for a new >> arch/tile/drivers/ directory, and it populates it with the first >> instance of a driver for that directory, a SPI Flash ROM (srom) driver. >> >> The directory is motivated as follows. While some classes of >> driver implementations should be grouped together so they are easily >> modified as a class (network, ATA, RTC, PCI, I2C, etc etc) there are >> "miscellaneous" drivers that don't benefit from any sharing with other >> driver implementations. If those drivers are for hardware that can >> plausibly be used by multiple architectures, it makes sense to put >> them somewhere like drivers/char. But if they are only usable on >> a single architecture (in this case drivers written for the Tilera >> para-virtualization model using our hypervisor) it makes sense to group >> such drivers with their architecture, to avoid cluttering the "drivers" >> hierarchy for other architectures that can't use that driver. > I generally advise against putting any device drivers into arch/*/, > on the ground that it is much easier to find similar drivers when > they are in a common place. We probably have a few similar drivers > in the tree already, e.g drivers/misc/eeprom/* and drivers/char/ps3flash.c > and drivers/platform/x86/intel_scu_ipcutil.c are examples of preexisting > drivers. > > I'd probably put this one in driver/misc/eeprom and make the interface > look like the other ones (sysfs bin attribute instead of character > device), which is a trivial change. > > Alternatively, you could create drivers/platform/tile to hold tile > specific device drivers, instead of keeping them under arch/tile. For the implementation, as you say, it's worth conforming to the other eeprom drivers regardless of where we put the actual implementation, so I'll look into the sysfs infrastructure (I haven't used it before, but I need to learn about it anyway). As far as the location in the tree, I think it's 50/50 on something like our eeprom driver. On the one hand the actual implementation is very Tilera-specific and very similar to all the other simple Tilera-specific para-virtualized drivers, which are mostly just read/write wrappers around hypervisor syscalls (we currently have about a dozen of these for various minor bits of I/O). On the other hand there is a little bit of eeprom commonality too, but on balance it feels like it makes more sense to keep it with the other "misc" Tilera drivers. I'd say neither answer is obviously wrong :-) As to where the "misc" Tilera drivers should go, I see that so far there is only drivers/platform/x86 (as created by Len Brown in 2008, along with a note that "other architectures will create drivers/platform/<arch>"). Is it fair to say that "drivers/s390", "drivers/parisc", and "drivers/sh" (for example) are all in their current locations just for historical reasons, and should in principle also be under "drivers/platform"? If so, yes, drivers/platform/tile seems like a reasonable location. >> +/** >> + * srom_llseek() - Change the current device offset. >> [...] > This looks unnecessary, just use generic_file_llseek. It looks like I also can just discard the llseek code altogether if we switch over to the sysfs model - even better :-) -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-05 6:41 ` Arnd Bergmann 2011-05-06 19:37 ` Chris Metcalf @ 2011-05-20 18:05 ` Chris Metcalf 2011-05-20 18:46 ` Arnd Bergmann 1 sibling, 1 reply; 27+ messages in thread From: Chris Metcalf @ 2011-05-20 18:05 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-kernel, Eric W. Biederman, Chris Wright, Greg Kroah-Hartman, Benjamin Thery, Phil Carmody On 5/5/2011 2:41 AM, Arnd Bergmann wrote: > On Wednesday 04 May 2011, Chris Metcalf wrote: >> This commit does two things: it adds the infrastructure for a new >> arch/tile/drivers/ directory, and it populates it with the first >> instance of a driver for that directory, a SPI Flash ROM (srom) driver. >> >> [...] > I generally advise against putting any device drivers into arch/*/, > on the ground that it is much easier to find similar drivers when > they are in a common place. We probably have a few similar drivers > in the tree already, e.g drivers/misc/eeprom/* and drivers/char/ps3flash.c > and drivers/platform/x86/intel_scu_ipcutil.c are examples of preexisting > drivers. > > I'd probably put this one in driver/misc/eeprom and make the interface > look like the other ones (sysfs bin attribute instead of character > device), which is a trivial change. > > [...] >> The actual SROM driver is fairly uncontroversial, and is just a simple >> driver that allows user space to read and write the SROM at a raw level. >> (A separate MTD driver exists for "tile", but this is not that driver.) >> The driver is particularly useful since the Tile chip can boot directly >> from the SROM, so providing this driver interface allows for updating >> the boot image prior to a reboot. > I think the sysfs bin attribute works well here because you don't need > any ioctl, and the contents are basically a representation of a flat > file. The implementation would be almost the same. This works well, except for the fact that we take advantage of the fact that the hypervisor driver internally buffers up writes to the current EEPROM sector, and flushes it to hardware only when explicitly told to do so, or when we start writing to another sector. This avoids excessive wear on the EEPROM and also handles detection of whether we need to do a sector erase before the re-write. However, it also means that we need to be able to issue the final explicit flush, or else the last write just sits in the hypervisor buffer indefinitely. To make that happen I think I need to extend the bin_attribute structure, and the bin.c release() function, slightly: --- fs/sysfs/bin.c~ 2011-05-20 14:02:43.000000000 -0400 +++ fs/sysfs/bin.c 2011-05-20 13:31:23.240866000 -0400 @@ -434,18 +434,26 @@ } static int release(struct inode * inode, struct file * file) { struct bin_buffer *bb = file->private_data; + struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; + struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr; + int rc = 0; + + if (attr->release && sysfs_get_active(attr_sd)) { + rc = attr->release(file, attr_sd->s_parent->s_dir.kobj, attr); + sysfs_put_active(attr_sd); + } mutex_lock(&sysfs_bin_lock); hlist_del(&bb->list); mutex_unlock(&sysfs_bin_lock); kfree(bb->buffer); kfree(bb); - return 0; + return rc; } const struct file_operations bin_fops = { .read = read, .write = write, --- include/linux/sysfs.h~ 2011-05-20 14:02:43.000000000 -0400 +++ include/linux/sysfs.h 2011-05-20 13:23:57.915080000 -0400 @@ -93,10 +93,11 @@ char *, loff_t, size_t); ssize_t (*write)(struct file *,struct kobject *, struct bin_attribute *, char *, loff_t, size_t); int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr, struct vm_area_struct *vma); + int (*release)(struct file *, struct kobject *, struct bin_attribute *); }; /** * sysfs_bin_attr_init - initialize a dynamically allocated bin_attribute * @attr: struct bin_attribute to initialize This change shouldn't require any changes to any other bin_attribute users elsewhere in the kernel. If something like this seems plausible, I already have the patch ready, and I can send it to LKML. I've cc'ed Greg K-H and a few other recent submitters to bin.c in case they'd like to weigh in at this point too; thanks. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-20 18:05 ` Chris Metcalf @ 2011-05-20 18:46 ` Arnd Bergmann 2011-05-20 22:40 ` Eric Biederman 0 siblings, 1 reply; 27+ messages in thread From: Arnd Bergmann @ 2011-05-20 18:46 UTC (permalink / raw) To: Chris Metcalf Cc: linux-kernel, Eric W. Biederman, Chris Wright, Greg Kroah-Hartman, Benjamin Thery, Phil Carmody On Friday 20 May 2011 20:05:05 Chris Metcalf wrote: > This works well, except for the fact that we take advantage of the fact > that the hypervisor driver internally buffers up writes to the current > EEPROM sector, and flushes it to hardware only when explicitly told to do > so, or when we start writing to another sector. This avoids excessive wear > on the EEPROM and also handles detection of whether we need to do a sector > erase before the re-write. However, it also means that we need to be able > to issue the final explicit flush, or else the last write just sits in the > hypervisor buffer indefinitely. To make that happen I think I need to > extend the bin_attribute structure, and the bin.c release() function, slightly: Yes, that would work. Another option would be to add a flush() callback to the bin_attribute. Since filp_close calls both fops->flush and fops->release, that would also make it possible to flush the buffer without closing it, if necessary. Arnd ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-20 18:46 ` Arnd Bergmann @ 2011-05-20 22:40 ` Eric Biederman 2011-05-20 23:39 ` Chris Metcalf 0 siblings, 1 reply; 27+ messages in thread From: Eric Biederman @ 2011-05-20 22:40 UTC (permalink / raw) To: Arnd Bergmann Cc: Chris Metcalf, linux-kernel, Chris Wright, Greg Kroah-Hartman, Benjamin Thery, Phil Carmody On Fri, May 20, 2011 at 11:46 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 20 May 2011 20:05:05 Chris Metcalf wrote: >> This works well, except for the fact that we take advantage of the fact >> that the hypervisor driver internally buffers up writes to the current >> EEPROM sector, and flushes it to hardware only when explicitly told to do >> so, or when we start writing to another sector. This avoids excessive wear >> on the EEPROM and also handles detection of whether we need to do a sector >> erase before the re-write. However, it also means that we need to be able >> to issue the final explicit flush, or else the last write just sits in the >> hypervisor buffer indefinitely. To make that happen I think I need to >> extend the bin_attribute structure, and the bin.c release() function, slightly: > > Yes, that would work. Another option would be to add a flush() callback > to the bin_attribute. Since filp_close calls both fops->flush and > fops->release, that would also make it possible to flush the buffer > without closing it, if necessary. Ouch! Please do not make sysfs the direct access method to your device. I may be wrong but I don't think any other driver relies exclusively on sysfs. Certainly with the introduction of a flush you are introducing an completely different usage paradigm from current users and will need an entirely different set of tools. You are vastly exceeding the one value per file rule of sysfs. Please look at the i2c core and if at all possible build a proper interface to your eeprom, that existing programs have a chance of utilizing rather than extending sysfs in ways that means cli tools can not work with it. Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-20 22:40 ` Eric Biederman @ 2011-05-20 23:39 ` Chris Metcalf 2011-05-21 3:21 ` Greg KH 2011-05-21 7:46 ` [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver Eric Biederman 0 siblings, 2 replies; 27+ messages in thread From: Chris Metcalf @ 2011-05-20 23:39 UTC (permalink / raw) To: Eric Biederman Cc: Arnd Bergmann, linux-kernel, Chris Wright, Greg Kroah-Hartman, Benjamin Thery, Phil Carmody On 5/20/2011 6:40 PM, Eric Biederman wrote: > Please do not make sysfs the direct access method to your device. > I may be wrong but I don't think any other driver relies exclusively on sysfs. I'm basing my implementation on drivers/misc/eeprom/. All the drivers there use the same sysfs model. > Certainly with the introduction of a flush you are introducing an completely > different usage paradigm from current users and will need an entirely different > set of tools. I don't think using my proposed implementation will be detectably different for most user tools. The addition of the flush() method just allows my implementation to defer the final sector's write until the device is closed (sectors are in fact still written to hardware as one does seek() or write() from one sector to another; only the "current" sector is cached by the hypervisor). I suppose some third-party tool that kept the eeprom file descriptor open indefinitely and did multiple writes to the same sector might not work as expected with this implementation. But it seems hard to imagine a use case for such a tool. The direct motivation for this case is to "impedance match" to the hypervisor driver for this device, which handles sector management internally, so the Linux device doesn't have to. Having a 'flush' method avoids excessive re-writes of the same sector for certain access patterns. The only alternatives that I see are to rewrite the tile userspace tools, but they are the way they are because the current model gives good consistency guarantees for writing the boot rom in the presence of arbitrary failure modes; or, to add something like a delayed timer event that allows the Linux driver to notify the hypervisor driver that writes are likely complete and it can write out the last sector. Neither of these are particularly attractive. > You are vastly exceeding the one value per file rule of sysfs. True, but bin_attribute has always been an exception to that rule anyway. > Please look at the i2c core and if at all possible build a proper interface to > your eeprom, that existing programs have a chance of utilizing rather than > extending sysfs in ways that means cli tools can not work with it. This driver is a paravirtualized hypervisor driver, so not really an I2C driver at all (in fact it handles both SPI and I2C eeproms almost identically within the Linux driver). And I think the driver's "eeprom" file should be compatible with userspace cli tools, assuming they close their file descriptor when they're done writing. I apologize for not including more of the back story in this email when adding the cc's, by the way. Originally I proposed a straightforward character device for this role. Arnd Bergmann encouraged me to look at kernel precedents like drivers/char/eeprom/, which is why I converted this driver to sysfs. The first post in this thread is here: https://lkml.org/lkml/2011/5/4/415 . Since then I've come around to believing that it's more valuable to group this driver with the other eeprom drivers, rather than with the other paravirtualized tile drivers. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-20 23:39 ` Chris Metcalf @ 2011-05-21 3:21 ` Greg KH 2011-05-21 9:33 ` Arnd Bergmann 2011-05-21 7:46 ` [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver Eric Biederman 1 sibling, 1 reply; 27+ messages in thread From: Greg KH @ 2011-05-21 3:21 UTC (permalink / raw) To: Chris Metcalf Cc: Eric Biederman, Arnd Bergmann, linux-kernel, Chris Wright, Benjamin Thery, Phil Carmody On Fri, May 20, 2011 at 07:39:10PM -0400, Chris Metcalf wrote: > On 5/20/2011 6:40 PM, Eric Biederman wrote: > > Please do not make sysfs the direct access method to your device. > > I may be wrong but I don't think any other driver relies exclusively on sysfs. > > I'm basing my implementation on drivers/misc/eeprom/. All the drivers > there use the same sysfs model. > > > Certainly with the introduction of a flush you are introducing an completely > > different usage paradigm from current users and will need an entirely different > > set of tools. > > I don't think using my proposed implementation will be detectably different > for most user tools. The addition of the flush() method just allows my > implementation to defer the final sector's write until the device is closed > (sectors are in fact still written to hardware as one does seek() or > write() from one sector to another; only the "current" sector is cached by > the hypervisor). I suppose some third-party tool that kept the eeprom file > descriptor open indefinitely and did multiple writes to the same sector > might not work as expected with this implementation. But it seems hard to > imagine a use case for such a tool. > > The direct motivation for this case is to "impedance match" to the > hypervisor driver for this device, which handles sector management > internally, so the Linux device doesn't have to. Having a 'flush' method > avoids excessive re-writes of the same sector for certain access patterns. > The only alternatives that I see are to rewrite the tile userspace tools, > but they are the way they are because the current model gives good > consistency guarantees for writing the boot rom in the presence of > arbitrary failure modes; or, to add something like a delayed timer event > that allows the Linux driver to notify the hypervisor driver that writes > are likely complete and it can write out the last sector. Neither of these > are particularly attractive. > > > You are vastly exceeding the one value per file rule of sysfs. > > True, but bin_attribute has always been an exception to that rule anyway. No it hasn't. A bin attribute is for something that is "one" value, it is a binary file that the kernel doesn't know anything about, nor does it intrepret it. It sounds like you want this binary attribute file to be a bit different than the "normal" one, and because of that you are wanting to modify the sysfs file, which proves that you are doing things differently here. So, I agree with Eric, please do something else as you are not really wanting to use a binary attribute, you want something else, like a character driver. Why can't you do that? > This driver is a paravirtualized hypervisor driver, so not really an I2C > driver at all (in fact it handles both SPI and I2C eeproms almost > identically within the Linux driver). And I think the driver's "eeprom" > file should be compatible with userspace cli tools, assuming they close > their file descriptor when they're done writing. Big assumption. > I apologize for not including more of the back story in this email when > adding the cc's, by the way. Yeah, that made things difficult :) > Originally I proposed a straightforward character device for this > role. Ah, ok, I think you should do that. > Arnd Bergmann encouraged me to look at kernel precedents like > drivers/char/eeprom/, which is why I converted this driver to sysfs. > The first post in this thread is here: > https://lkml.org/lkml/2011/5/4/415 . Since then I've come around to > believing that it's more valuable to group this driver with the other > eeprom drivers, rather than with the other paravirtualized tile > drivers. See above why I don't think that is so. thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 3:21 ` Greg KH @ 2011-05-21 9:33 ` Arnd Bergmann 2011-05-21 13:52 ` Chris Metcalf 0 siblings, 1 reply; 27+ messages in thread From: Arnd Bergmann @ 2011-05-21 9:33 UTC (permalink / raw) To: Greg KH Cc: Chris Metcalf, Eric Biederman, linux-kernel, Chris Wright, Benjamin Thery, Phil Carmody On Saturday 21 May 2011 05:21:02 Greg KH wrote: > On Fri, May 20, 2011 at 07:39:10PM -0400, Chris Metcalf wrote: > > On 5/20/2011 6:40 PM, Eric Biederman wrote: > > > > > You are vastly exceeding the one value per file rule of sysfs. > > > > True, but bin_attribute has always been an exception to that rule anyway. > > No it hasn't. A bin attribute is for something that is "one" value, it > is a binary file that the kernel doesn't know anything about, nor does > it intrepret it. > > It sounds like you want this binary attribute file to be a bit different > than the "normal" one, and because of that you are wanting to modify the > sysfs file, which proves that you are doing things differently here. I initially argued that the file is the same as all the other ones in drivers/misc/eeprom/*.c. None of them care what the data is and they pass it through to the user. It would also be possible to always flush after each write, which makes the interface act more like you would expect it to, at the cost of lower performance in case the user writes it in small blocks. > > Originally I proposed a straightforward character device for this > > role. > > Ah, ok, I think you should do that. > > > Arnd Bergmann encouraged me to look at kernel precedents like > > drivers/char/eeprom/, which is why I converted this driver to sysfs. > > The first post in this thread is here: > > https://lkml.org/lkml/2011/5/4/415 . Since then I've come around to > > believing that it's more valuable to group this driver with the other > > eeprom drivers, rather than with the other paravirtualized tile > > drivers. > > See above why I don't think that is so. What I only now noticed is that the other eeprom drivers only support reading the eeprom, not writing it, so there is a significant difference. Using the bin_attribute doesn't sound completely wrong to me, especially if you put it in your /sys/hypervisor/* direcory together with the regular attributes we talked about. The character device would also be an option (better than /proc/ppc/update_flash that is used on pSeries), but if we do that, I would group it together with the other similar files (ps3flash, nwflash, ...) in a new subdirectory. Arnd ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 9:33 ` Arnd Bergmann @ 2011-05-21 13:52 ` Chris Metcalf 2011-05-21 15:02 ` Arnd Bergmann 2011-05-21 15:50 ` Eric Biederman 0 siblings, 2 replies; 27+ messages in thread From: Chris Metcalf @ 2011-05-21 13:52 UTC (permalink / raw) To: Arnd Bergmann Cc: Greg KH, Eric Biederman, linux-kernel, Chris Wright, Benjamin Thery, Phil Carmody (Resending since Thunderbird "helpfully" made the previous attempt include an HTML portion, for no obvious reason.) On 5/20/2011 11:21 PM, Greg KH wrote: > On Fri, May 20, 2011 at 07:39:10PM -0400, Chris Metcalf wrote: >> This driver is a paravirtualized hypervisor driver, so not really an I2C >> driver at all (in fact it handles both SPI and I2C eeproms almost >> identically within the Linux driver). And I think the driver's "eeprom" >> file should be compatible with userspace cli tools, assuming they close >> their file descriptor when they're done writing. > Big assumption. Interesting! I did in fact assume that most tools that read or write eeproms would tend to be "batch" tools, i.e. they would read or write whatever it was they wanted, then exit; the tool we have that modifies this EEPROM is written like that, for example. It sounds like you're saying there are (or may be) tools that open the file descriptor and do writes to it, then don't exit or close the file descriptor, but expect the last write to have hit the device immediately. It's fair to say that since my suggested API does defer the last sector's worth of writes until the app exits, it potentially violates that assumption, so it's a bad idea. On 5/21/2011 3:46 AM, Eric Biederman wrote: > What is wrong with an mtd driver? More backstory: we actually have an MTD driver for this device (in our local tree as drivers/mtd/devices/tile_srom.c) but we haven't yet tried to push it back to the community. But it doesn't work for the most important purpose of this device, namely to serve as one of the possible boot streams at power-on. For this, we have to control exactly what data is on what sector, which means a character (or sysfs) device. > Looking a bit back into the conversation it appears clear that you are > talking about something that resembles NOR flash with multiple sectors, > etc. > > eeproms have random byte access and are typically 256 bytes. You devices > doesn't sound anything like an eeprom. Yes and no. As Arnd points out in his followup, the hypervisor driver for this SPI ROM makes it look a lot like a typical eeprom. But, perhaps, not quite close enough to count as a "real" eeprom. On 5/21/2011 5:33 AM, Arnd Bergmann wrote: > What I only now noticed is that the other eeprom drivers only support > reading the eeprom, not writing it, so there is a significant difference. However, this point isn't quite correct. Both the at24 (I2C) and at25 (SPI) sysfs drivers support the "write" callback for at least some of their supported hardware. It's true that all the other drivers are read-only. > Using the bin_attribute doesn't sound completely wrong to me, especially > if you put it in your /sys/hypervisor/* direcory together with the > regular attributes we talked about. The character device would also > be an option (better than /proc/ppc/update_flash that is used on pSeries), > but if we do that, I would group it together with the other similar > files (ps3flash, nwflash, ...) in a new subdirectory. Sounds like the consensus is that a character driver is in fact the best option here. > We do have precedent for multiple interfaces that have the same > purpose as this one: > > drivers/misc/eeprom/* > drivers/char/ps3flash.c > drivers/char/nwflash.c > drivers/char/bfin-otp.c > arch/powerpc/kernel/rtas_flash.c > drivers/sbus/char/jsflash.c I'm certainly happy to push the original SPI ROM character device as drivers/platform/tile/srom.c. I'm not sure there's enough value in trying to group that device together with other devices that are sort of similar but with pretty variant ancestry and not a lot of need for commonality -- other than all have a file_operations structure :-) In this case I think grouping it with other paravirtualized tile drivers may be the closer connection. Having gone through the process of creating a sysfs driver, I will also go ahead and remove the SPI support from it (since that's the part that requires "flush") and post the remaining pure-EEPROM I2C paravirtualized driver to LKML. Since it has no need for flushes, it ends up exactly parallel to at24.c. So here's my thought. How does this sound? - Add drivers/platform/tile - Put the original character device there as srom.c - Drop the bin_attribute "flush" changes - Add drivers/misc/eeprom/tile.c for our "pure" I2C EEPROM driver I appreciate everyone's feedback and help on this! -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 13:52 ` Chris Metcalf @ 2011-05-21 15:02 ` Arnd Bergmann 2011-05-21 15:31 ` Chris Metcalf 2011-05-21 15:50 ` Eric Biederman 1 sibling, 1 reply; 27+ messages in thread From: Arnd Bergmann @ 2011-05-21 15:02 UTC (permalink / raw) To: Chris Metcalf Cc: Greg KH, Eric Biederman, linux-kernel, Chris Wright, Benjamin Thery, Phil Carmody On Saturday 21 May 2011 15:52:39 Chris Metcalf wrote: > On 5/21/2011 5:33 AM, Arnd Bergmann wrote: > > What I only now noticed is that the other eeprom drivers only support > > reading the eeprom, not writing it, so there is a significant difference. > > However, this point isn't quite correct. Both the at24 (I2C) and at25 > (SPI) sysfs drivers support the "write" callback for at least some of their > supported hardware. It's true that all the other drivers are read-only. Ok, I stand corrected again. > > Using the bin_attribute doesn't sound completely wrong to me, especially > > if you put it in your /sys/hypervisor/* direcory together with the > > regular attributes we talked about. The character device would also > > be an option (better than /proc/ppc/update_flash that is used on pSeries), > > but if we do that, I would group it together with the other similar > > files (ps3flash, nwflash, ...) in a new subdirectory. > > Sounds like the consensus is that a character driver is in fact the best > option here. At least the one that meets the least resistance ;-) > > We do have precedent for multiple interfaces that have the same > > purpose as this one: > > > > drivers/misc/eeprom/* > > drivers/char/ps3flash.c > > drivers/char/nwflash.c > > drivers/char/bfin-otp.c > > arch/powerpc/kernel/rtas_flash.c > > drivers/sbus/char/jsflash.c > > I'm certainly happy to push the original SPI ROM character device as > drivers/platform/tile/srom.c. I'm not sure there's enough value in trying > to group that device together with other devices that are sort of similar > but with pretty variant ancestry and not a lot of need for commonality -- > other than all have a file_operations structure :-) In this case I think > grouping it with other paravirtualized tile drivers may be the closer > connection. The general tendency is to always group drivers by their purpose these days, instead of by the bus or other interface that they are attached to. I would definitely prefer grouping it with drivers of the same purpose. The main reason is to make sure that a driver writer can easily find existing drivers with the same purpose take them as example code, so that new code uses the same API as existing code. > Having gone through the process of creating a sysfs driver, I will also go > ahead and remove the SPI support from it (since that's the part that > requires "flush") and post the remaining pure-EEPROM I2C paravirtualized > driver to LKML. Since it has no need for flushes, it ends up exactly > parallel to at24.c. > > So here's my thought. How does this sound? > > - Add drivers/platform/tile > - Put the original character device there as srom.c > - Drop the bin_attribute "flush" changes > - Add drivers/misc/eeprom/tile.c for our "pure" I2C EEPROM driver > > I appreciate everyone's feedback and help on this! Can you explain why there are both i2c and spi drivers? If they have the same purpose (e.g. providing a place to store the kernel binary) and same data layout, I would recommend using the same user interface for both. If you want to keep the srom.c driver with a chardev interface for the SPI chip only, I would recommend sticking it into drivers/char/ for now, or possibly a new drivers/char/flash/. I have agreed to take over future maintainership of drivers/char and driver/misc during UDS in Budapest, basically to keep random stuff from getting added there, so I will try to find a better place for flash drivers like this. Arnd ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 15:02 ` Arnd Bergmann @ 2011-05-21 15:31 ` Chris Metcalf 0 siblings, 0 replies; 27+ messages in thread From: Chris Metcalf @ 2011-05-21 15:31 UTC (permalink / raw) To: Arnd Bergmann Cc: Greg KH, Eric Biederman, linux-kernel, Chris Wright, Benjamin Thery, Phil Carmody On 5/21/2011 11:02 AM, Arnd Bergmann wrote: > On Saturday 21 May 2011 15:52:39 Chris Metcalf wrote: >> Sounds like the consensus is that a character driver is in fact the best >> option here. > At least the one that meets the least resistance ;-) Well, perhaps that's what I meant :-) I do take Greg K-H's point that we don't really want to mess with the cleanliness of the bin_attribute API, though. > The general tendency is to always group drivers by their purpose these > days, instead of by the bus or other interface that they are attached > to. I would definitely prefer grouping it with drivers of the same > purpose. > > The main reason is to make sure that a driver writer can easily find > existing drivers with the same purpose take them as example code, so > that new code uses the same API as existing code. > > If you want to keep the srom.c driver with a chardev interface for > the SPI chip only, I would recommend sticking it into drivers/char/ > for now, or possibly a new drivers/char/flash/. I will plan to push it to drivers/char/tile-srom.c for now, then. > I have agreed to take over future maintainership of drivers/char and > driver/misc during UDS in Budapest, basically to keep random > stuff from getting added there, so I will try to find a better place > for flash drivers like this. I'll add my thanks for helping keep the kernel a better place. It's great that there are folks like yourself who are willing to devote time to code and structural cleanliness rather than just shoving in whatever they can get away with :-) > Can you explain why there are both i2c and spi drivers? If they > have the same purpose (e.g. providing a place to store the kernel > binary) and same data layout, I would recommend using the > same user interface for both. The I2C eeprom driver can't be used to store the boot stream, just the SPI driver; the I2C eeprom driver is mostly for customer use. In fact the drivers are similar enough that I rewrote the SPI driver for sysfs first, since it was harder, but then found that grafting in support for the I2C driver was pretty easy, since all I had to do was add some conditionalization (different "chunk" size for writes, no need for write retries during sector erase, no support for page_size and sector_size attributes). The absence of "size" attributes for the I2C driver is the only visible user interface difference, but it's fundamental to the devices. It's tempting to combine the drivers into one, but it feels like we might as well publish the I2C driver with the other eeprom sysfs drivers, and the SPI driver as a character device, as discussed. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 13:52 ` Chris Metcalf 2011-05-21 15:02 ` Arnd Bergmann @ 2011-05-21 15:50 ` Eric Biederman 2011-05-23 20:10 ` Chris Metcalf ` (2 more replies) 1 sibling, 3 replies; 27+ messages in thread From: Eric Biederman @ 2011-05-21 15:50 UTC (permalink / raw) To: Chris Metcalf Cc: Arnd Bergmann, Greg KH, linux-kernel, Chris Wright, Benjamin Thery, Phil Carmody On Sat, May 21, 2011 at 6:52 AM, Chris Metcalf <cmetcalf@tilera.com> wrote: > Interesting! I did in fact assume that most tools that read or write > eeproms would tend to be "batch" tools, i.e. they would read or write > whatever it was they wanted, then exit; the tool we have that modifies this > EEPROM is written like that, for example. It sounds like you're saying > there are (or may be) tools that open the file descriptor and do writes to > it, then don't exit or close the file descriptor, but expect the last write > to have hit the device immediately. It's fair to say that since my > suggested API does defer the last sector's worth of writes until the app > exits, it potentially violates that assumption, so it's a bad idea. Plus the other gigantic difference that I have seen very few eeproms that can hold a sectors worth of data. My reading of this conversation is that you are confusing a configuration eeprom with a nor flash boot device. That is a bit like confusing a yellow sticky not with the Encyclopedia Britannica. They aren't at all the same thing. > On 5/21/2011 3:46 AM, Eric Biederman wrote: >> What is wrong with an mtd driver? > > More backstory: we actually have an MTD driver for this device (in our > local tree as drivers/mtd/devices/tile_srom.c) but we haven't yet tried to > push it back to the community. But it doesn't work for the most important > purpose of this device, namely to serve as one of the possible boot streams > at power-on. For this, we have to control exactly what data is on what > sector, which means a character (or sysfs) device. How can an MTD driver not work to control exactly what data is on what sector???? That is what MTD drivers do. It sounds like you have a bug in either your userspace tools or your implementation of your mtd driver. That is definitely not a reason to dump your device into a char device (mtdchar already exists). > Sounds like the consensus is that a character driver is in fact the best > option here. I think the consensus is that your design is buggy. Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 15:50 ` Eric Biederman @ 2011-05-23 20:10 ` Chris Metcalf 2011-05-28 15:13 ` [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM Chris Metcalf 2011-06-02 15:04 ` [PATCH v3] " Chris Metcalf 2 siblings, 0 replies; 27+ messages in thread From: Chris Metcalf @ 2011-05-23 20:10 UTC (permalink / raw) To: Eric Biederman Cc: Arnd Bergmann, Greg KH, linux-kernel, Chris Wright, Benjamin Thery, Phil Carmody On 5/21/2011 11:50 AM, Eric Biederman wrote: > On Sat, May 21, 2011 at 6:52 AM, Chris Metcalf <cmetcalf@tilera.com> wrote: >> Interesting! I did in fact assume that most tools that read or write >> eeproms would tend to be "batch" tools, i.e. they would read or write >> whatever it was they wanted, then exit; the tool we have that modifies this >> EEPROM is written like that, for example. It sounds like you're saying >> there are (or may be) tools that open the file descriptor and do writes to >> it, then don't exit or close the file descriptor, but expect the last write >> to have hit the device immediately. It's fair to say that since my >> suggested API does defer the last sector's worth of writes until the app >> exits, it potentially violates that assumption, so it's a bad idea. > Plus the other gigantic difference that I have seen very few eeproms > that can hold a sectors worth of data. > > My reading of this conversation is that you are confusing a configuration > eeprom with a nor flash boot device. That is a bit like confusing a yellow > sticky not with the Encyclopedia Britannica. They aren't at all the > same thing. Well, I don't think I'm confusing the two types of hardware, per se. The issue is that the hypervisor provides an API to Linux (or other operating systems) that attempts to make them look reasonably similar. In particular, for the SPI ROM, the hypervisor implicitly manages background reading of the sector data, doing a "diff" of new data to determine whether a sector erase is required, and deferring the actual sector write until the client attempts to write to a different sector. (This last feature is why an explicit "flush" is required.) This approach (unlike mtdchar) provides a "transparent" mode for accessing the SPI ROM. Users of our character driver don't need to know they are writing a flash device, don't need to worry about erasing/rewriting sectors, etc. We could potentially rework our boot ROM management tool to support the mtdchar API, but it would be a modest pain, particularly since we still have to support the normal file mode without "erase" ioctls, etc., for use in the "cross-compiling" mode that we offer with this tool. And, another Tilera tool that stores data in the SPI ROM for bootup configuration would also have to be similarly reworked. And, if any of our customers have taken advantage of the part of the SPI ROM that is reserved for applications, they'd have to make that change as well. This seems like a problem. Plus, for what it's worth, it would make it impossible for us to use our tools on top of another OS, since they'd be dependent on a Linux-only interface. None of this is impossible to change, but my sense is that a basically trivial character device that makes use of the existing Tilera hypervisor's virtualization of the flash ROM should not be particularly problematic. It is true that it may not be worth grouping with other flash ROM character devices, since it has almost no "flash ROM" characteristics due to the hypervisor virtualization, but I will leave that up to Arnd. I'm happy to add some comments to the driver explaining what makes it different. I hope this adequately addresses your concerns. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-21 15:50 ` Eric Biederman 2011-05-23 20:10 ` Chris Metcalf @ 2011-05-28 15:13 ` Chris Metcalf 2011-05-28 21:23 ` Greg KH 2011-06-02 15:04 ` [PATCH v3] " Chris Metcalf 2 siblings, 1 reply; 27+ messages in thread From: Chris Metcalf @ 2011-05-28 15:13 UTC (permalink / raw) To: linux-kernel, Arnd Bergmann Cc: Greg KH, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody The first version of this patch proposed an arch/tile/drivers/ directory, but the consensus was that this was probably a poor choice for a place to group Tilera-specific drivers, and that in any case grouping by platform was discouraged, and grouping by function was preferred. This driver has no particular "peer" drivers it can be grouped with. It is sort of like an MTD driver for SPI ROM, but it doesn't group well with the other MTD devices since it relies on hypervisor virtualization to handle many of the irritating aspects of flash ROM management: sector awareness, background read for sub-sector writes, bit examination to determine whether a sector erase needs to be issued, etc. It is in fact more like an EEPROM driver, but the hypervisor virtualization does require a "flush" command if you wish to commit a sector write prior to writing to a different sector, and this is sufficiently different from generic I2C/SPI EEPROMs that as a result it doesn't group well with them either. The simple character device is already in use by a range of Tilera SPI ROM management tools, as well as by customers. In addition, using the simple character device actually simplifies the userspace tools, since they don't need to manage sector erase, background read, etc. This both simplifies the code (since we can uniformly manage plain files and the SPI ROM) as well as makes the user code portable to non-Linux platforms that don't offer the same MTD ioctls. Signed-off-by: Chris Metcalf <cmetcalf@tilera.com> --- arch/tile/include/hv/drv_srom_intf.h | 41 +++ drivers/char/Kconfig | 11 + drivers/char/Makefile | 2 + drivers/char/tile-srom.c | 453 ++++++++++++++++++++++++++++++++++ 4 files changed, 507 insertions(+), 0 deletions(-) create mode 100644 arch/tile/include/hv/drv_srom_intf.h create mode 100644 drivers/char/tile-srom.c diff --git a/arch/tile/include/hv/drv_srom_intf.h b/arch/tile/include/hv/drv_srom_intf.h new file mode 100644 index 0000000..6395faa --- /dev/null +++ b/arch/tile/include/hv/drv_srom_intf.h @@ -0,0 +1,41 @@ +/* + * Copyright 2011 Tilera Corporation. All Rights Reserved. + * + * 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, version 2. + * + * 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, GOOD TITLE or + * NON INFRINGEMENT. See the GNU General Public License for + * more details. + */ + +/** + * @file drv_srom_intf.h + * Interface definitions for the SPI Flash ROM driver. + */ + +#ifndef _SYS_HV_INCLUDE_DRV_SROM_INTF_H +#define _SYS_HV_INCLUDE_DRV_SROM_INTF_H + +/** Read this offset to get the total device size. */ +#define SROM_TOTAL_SIZE_OFF 0xF0000000 + +/** Read this offset to get the device sector size. */ +#define SROM_SECTOR_SIZE_OFF 0xF0000004 + +/** Read this offset to get the device page size. */ +#define SROM_PAGE_SIZE_OFF 0xF0000008 + +/** Write this offset to flush any pending writes. */ +#define SROM_FLUSH_OFF 0xF1000000 + +/** Write this offset, plus the byte offset of the start of a sector, to + * erase a sector. Any write data is ignored, but there must be at least + * one byte of write data. Only applies when the driver is in MTD mode. + */ +#define SROM_ERASE_OFF 0xF2000000 + +#endif /* _SYS_HV_INCLUDE_DRV_SROM_INTF_H */ diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index ad59b4e..d3c84c5 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -616,5 +616,16 @@ config MSM_SMD_PKT Enables userspace clients to read and write to some packet SMD ports via device interface for MSM chipset. +config TILE_SROM + bool "Character-device access via hypervisor to the Tilera SPI ROM" + depends on TILE + default y + ---help--- + This device provides character-level read-write access + to the SROM, typically via the "0", "1", "2", and "info" + devices in /dev/srom/. The Tilera hypervisor makes the + flash device appear much like a simple EEPROM, and knows + how to partition a single ROM for multiple purposes. + endmenu diff --git a/drivers/char/Makefile b/drivers/char/Makefile index 7a00672..32762ba 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -63,3 +63,5 @@ obj-$(CONFIG_RAMOOPS) += ramoops.o obj-$(CONFIG_JS_RTC) += js-rtc.o js-rtc-y = rtc.o + +obj-$(CONFIG_TILE_SROM) += tile-srom.o diff --git a/drivers/char/tile-srom.c b/drivers/char/tile-srom.c new file mode 100644 index 0000000..9e03826 --- /dev/null +++ b/drivers/char/tile-srom.c @@ -0,0 +1,453 @@ +/* + * Copyright 2011 Tilera Corporation. All Rights Reserved. + * + * 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, version 2. + * + * 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, GOOD TITLE or + * NON INFRINGEMENT. See the GNU General Public License for + * more details. + * + * SPI Flash ROM driver + * + * This source code is derived from code provided in "Linux Device + * Drivers" by Alessandro Rubini and Jonathan Corbet, published by + * O'Reilly & Associates. + */ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/kernel.h> /* printk() */ +#include <linux/slab.h> /* kmalloc() */ +#include <linux/fs.h> /* everything... */ +#include <linux/errno.h> /* error codes */ +#include <linux/types.h> /* size_t */ +#include <linux/proc_fs.h> +#include <linux/fcntl.h> /* O_ACCMODE */ +#include <linux/aio.h> +#include <linux/pagemap.h> +#include <linux/hugetlb.h> +#include <linux/uaccess.h> +#include <hv/hypervisor.h> +#include <linux/ioctl.h> +#include <linux/cdev.h> +#include <linux/delay.h> +#include <hv/drv_srom_intf.h> + +/* + * Size of our hypervisor I/O requests. We break up large transfers + * so that we don't spend large uninterrupted spans of time in the + * hypervisor. Erasing an SROM sector takes a significant fraction of + * a second, so if we allowed the user to, say, do one I/O to write the + * entire ROM, we'd get soft lockup timeouts, or worse. + */ +#define SROM_CHUNK_SIZE ((size_t)4096) + +/* + * When hypervisor is busy (e.g. erasing), poll the status periodically. + */ + +/* + * Interval to poll the state in msec + */ +#define SROM_WAIT_TRY_INTERVAL 20 + +/* + * Maximum times to poll the state + */ +#define SROM_MAX_WAIT_TRY_TIMES 1000 + +struct srom_dev { + struct cdev cdev; /* Character device structure */ + int hv_devhdl; /* Handle for hypervisor device */ + u32 size; /* Size of this device */ + char *info_string; /* String returned by the info device + if this is it; NULL otherwise */ + struct mutex srom_lock; /* Allow only one accessor at a time */ +}; + +static int srom_major; /* Dynamic major by default */ +static int srom_devs = 4; /* One per SROM partition plus one info file */ + +/* Minor number of the info file */ +static inline int srom_info_minor(void) +{ + return srom_devs - 1; +} + +module_param(srom_major, int, 0); +module_param(srom_devs, int, 0); +MODULE_AUTHOR("Tilera Corporation"); +MODULE_LICENSE("Dual BSD/GPL"); + +static struct srom_dev *srom_devices; /* allocated in srom_init */ + +/* + * Handle calling the hypervisor and managing EAGAIN/EBUSY. + */ + +static ssize_t _srom_read(int hv_devhdl, void *buf, + loff_t off, size_t count) +{ + int retval, retries = SROM_MAX_WAIT_TRY_TIMES; + for (;;) { + retval = hv_dev_pread(hv_devhdl, 0, (HV_VirtAddr)buf, + count, off); + if (retval >= 0) + return retval; + if (retval == HV_EAGAIN) + continue; + if (retval == HV_EBUSY && --retries > 0) { + msleep(SROM_WAIT_TRY_INTERVAL); + continue; + } + pr_err("_srom_read: error %d\n", retval); + return -EIO; + } +} + +static ssize_t _srom_write(int hv_devhdl, const void *buf, + loff_t off, size_t count) +{ + int retval, retries = SROM_MAX_WAIT_TRY_TIMES; + for (;;) { + retval = hv_dev_pwrite(hv_devhdl, 0, (HV_VirtAddr)buf, + count, off); + if (retval >= 0) + return retval; + if (retval == HV_EAGAIN) + continue; + if (retval == HV_EBUSY && --retries > 0) { + msleep(SROM_WAIT_TRY_INTERVAL); + continue; + } + pr_err("_srom_write: error %d\n", retval); + return -EIO; + } +} + +/** + * srom_open() - Device open routine. + * @inode: Inode for this device. + * @filp: File for this specific open of the device. + * + * Returns zero, or an error code. + */ +static int srom_open(struct inode *inode, struct file *filp) +{ + struct srom_dev *dev; + + /* Find the device */ + dev = container_of(inode->i_cdev, struct srom_dev, cdev); + + /* Now open the hypervisor device if we haven't already. */ + if (dev->hv_devhdl == 0) { + char buf[20]; + int instance = iminor(inode); + if (instance != srom_info_minor()) { + sprintf(buf, "srom/0/%d", instance); + dev->hv_devhdl = hv_dev_open((HV_VirtAddr)buf, 0); + if (dev->hv_devhdl > 0) { + dev->size = 0; + if (mutex_lock_interruptible(&dev->srom_lock)) + return -ERESTARTSYS; + _srom_read(dev->hv_devhdl, &dev->size, + SROM_TOTAL_SIZE_OFF, + sizeof(dev->size)); + mutex_unlock(&dev->srom_lock); + i_size_write(inode, dev->size); + } + } else { + u32 sector_size = 0; + u32 page_size = 0; + + sprintf(buf, "srom/0/0"); + dev->hv_devhdl = hv_dev_open((HV_VirtAddr)buf, 0); + if (dev->hv_devhdl > 0) { + static const int info_string_size = 80; + + if (mutex_lock_interruptible(&dev->srom_lock)) + return -ERESTARTSYS; + _srom_read(dev->hv_devhdl, §or_size, + SROM_SECTOR_SIZE_OFF, + sizeof(sector_size)); + _srom_read(dev->hv_devhdl, &page_size, + SROM_PAGE_SIZE_OFF, + sizeof(page_size)); + mutex_unlock(&dev->srom_lock); + + dev->info_string = kmalloc(info_string_size, + GFP_KERNEL); + snprintf(dev->info_string, info_string_size, + "sector_size: %d\npage_size: %d\n", + sector_size, page_size); + } + } + } + + /* If we tried and failed to open it, fail. */ + if (dev->hv_devhdl < 0) { + switch (dev->hv_devhdl) { + case HV_ENODEV: + return -ENODEV; + default: + return (ssize_t)dev->hv_devhdl; + } + } + + filp->private_data = dev; + + return 0; /* success */ +} + + +/** + * srom_release() - Device release routine. + * @inode: Inode for this device. + * @filp: File for this specific open of the device. + * + * Returns zero, or an error code. + */ +static int srom_release(struct inode *inode, struct file *filp) +{ + struct srom_dev *dev = filp->private_data; + int retval = 0; + char dummy; + + /* + * First we need to make sure we've flushed anything written to + * the ROM. + */ + if (mutex_lock_interruptible(&dev->srom_lock)) { + retval = -ERESTARTSYS; + filp->private_data = NULL; + return retval; + } + + if (dev->hv_devhdl > 0) + _srom_write(dev->hv_devhdl, &dummy, SROM_FLUSH_OFF, 1); + + mutex_unlock(&dev->srom_lock); + filp->private_data = NULL; + + return retval; +} + + +/** + * srom_read() - Read (control) data from the device. + * @filp: File for this specific open of the device. + * @buf: User's data buffer. + * @count: Number of bytes requested. + * @f_pos: File position. + * + * Returns number of bytes read, or an error code. + */ +static ssize_t srom_read(struct file *filp, char __user *buf, + size_t count, loff_t *f_pos) +{ + int retval = 0; + void *kernbuf; + struct srom_dev *dev = filp->private_data; + + if (dev->hv_devhdl < 0) + return -EINVAL; + + if (dev->info_string) { + int info_len = strlen(dev->info_string); + int bytes_avail = info_len - *f_pos; + int xfer_len = (bytes_avail < count) ? bytes_avail : count; + + if (xfer_len <= 0) + return 0; + + if (copy_to_user(buf, dev->info_string + *f_pos, xfer_len)) + return -EFAULT; + *f_pos += xfer_len; + return xfer_len; + } + + kernbuf = kmalloc(SROM_CHUNK_SIZE, GFP_KERNEL); + if (!kernbuf) + return -ENOMEM; + + if (mutex_lock_interruptible(&dev->srom_lock)) { + retval = -ERESTARTSYS; + kfree(kernbuf); + return retval; + } + + while (count) { + int hv_retval; + int bytes_this_pass = min(count, SROM_CHUNK_SIZE); + + hv_retval = _srom_read(dev->hv_devhdl, kernbuf, + *f_pos, bytes_this_pass); + if (hv_retval > 0) { + if (copy_to_user(buf, kernbuf, hv_retval) != 0) { + retval = -EFAULT; + break; + } + } else if (hv_retval <= 0) { + if (retval == 0) + retval = hv_retval; + break; + } + + retval += hv_retval; + *f_pos += hv_retval; + buf += hv_retval; + count -= hv_retval; + } + + mutex_unlock(&dev->srom_lock); + kfree(kernbuf); + + return retval; +} + +/** + * srom_write() - Write (control) data to the device. + * @filp: File for this specific open of the device. + * @buf: User's data buffer. + * @count: Number of bytes requested. + * @f_pos: File position. + * + * Returns number of bytes written, or an error code. + */ +static ssize_t srom_write(struct file *filp, const char __user *buf, + size_t count, loff_t *f_pos) +{ + int retval = 0; + void *kernbuf; + struct srom_dev *dev = filp->private_data; + + if (dev->hv_devhdl < 0 || dev->info_string) + return -EINVAL; + + kernbuf = kmalloc(SROM_CHUNK_SIZE, GFP_KERNEL); + if (!kernbuf) + return -ENOMEM; + + if (mutex_lock_interruptible(&dev->srom_lock)) { + retval = -ERESTARTSYS; + kfree(kernbuf); + return retval; + } + + while (count) { + int hv_retval; + int bytes_this_pass = min(count, SROM_CHUNK_SIZE); + + if (copy_from_user(kernbuf, buf, bytes_this_pass) != 0) { + retval = -EFAULT; + break; + } + + hv_retval = _srom_write(dev->hv_devhdl, kernbuf, + *f_pos, bytes_this_pass); + if (hv_retval <= 0) { + if (retval == 0) + retval = hv_retval; + break; + } + + retval += hv_retval; + *f_pos += hv_retval; + buf += hv_retval; + count -= hv_retval; + } + + mutex_unlock(&dev->srom_lock); + kfree(kernbuf); + + return retval; +} + + +/* + * The fops + */ +static const struct file_operations srom_fops = { + .owner = THIS_MODULE, + .llseek = generic_file_llseek, + .read = srom_read, + .write = srom_write, + .open = srom_open, + .release = srom_release, +}; + +/** + * srom_setup_cdev() - Set up a device instance in the cdev table. + * @dev: Per-device SROM state. + * @index: Device to set up. + */ +static void srom_setup_cdev(struct srom_dev *dev, int index) +{ + int err, devno = MKDEV(srom_major, index); + + cdev_init(&dev->cdev, &srom_fops); + dev->cdev.owner = THIS_MODULE; + dev->cdev.ops = &srom_fops; + err = cdev_add(&dev->cdev, devno, 1); + /* Fail gracefully if need be */ + if (err) + pr_notice("Error %d adding srom%d", err, index); +} + +/** srom_init() - Initialize the driver's module. */ +static int srom_init(void) +{ + int result, i; + dev_t dev = MKDEV(srom_major, 0); + + /* + * Register our major, and accept a dynamic number. + */ + if (srom_major) + result = register_chrdev_region(dev, srom_devs, "srom"); + else { + result = alloc_chrdev_region(&dev, 0, srom_devs, "srom"); + srom_major = MAJOR(dev); + } + if (result < 0) + return result; + + + /* + * Allocate the devices -- we can't have them static, as the number + * can be specified at load time. + */ + srom_devices = kzalloc(srom_devs * sizeof(struct srom_dev), + GFP_KERNEL); + if (!srom_devices) { + unregister_chrdev_region(dev, srom_devs); + return -ENOMEM; + } + for (i = 0; i < srom_devs; i++) { + srom_setup_cdev(srom_devices + i, i); + mutex_init(&srom_devices[i].srom_lock); + } + + return 0; /* succeed */ +} + +/** srom_cleanup() - Clean up the driver's module. */ +static void srom_cleanup(void) +{ + int i; + + for (i = 0; i < srom_devs; i++) { + cdev_del(&srom_devices[i].cdev); + kfree(srom_devices[i].info_string); + } + kfree(srom_devices); + unregister_chrdev_region(MKDEV(srom_major, 0), srom_devs); +} + +module_init(srom_init); +module_exit(srom_cleanup); -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-28 15:13 ` [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM Chris Metcalf @ 2011-05-28 21:23 ` Greg KH 2011-05-29 0:32 ` Chris Metcalf 0 siblings, 1 reply; 27+ messages in thread From: Greg KH @ 2011-05-28 21:23 UTC (permalink / raw) To: Chris Metcalf Cc: linux-kernel, Arnd Bergmann, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On Sat, May 28, 2011 at 11:13:54AM -0400, Chris Metcalf wrote: > --- /dev/null > +++ b/drivers/char/tile-srom.c > @@ -0,0 +1,453 @@ > +/* > + * Copyright 2011 Tilera Corporation. All Rights Reserved. > + * > + * 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, version 2. > + * > + * 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, GOOD TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for > + * more details. > + * > + * SPI Flash ROM driver > + * > + * This source code is derived from code provided in "Linux Device > + * Drivers" by Alessandro Rubini and Jonathan Corbet, published by > + * O'Reilly & Associates. > + */ LDD version 2? Wow, this driver is old. > +MODULE_LICENSE("Dual BSD/GPL"); Wait, where did the BSD come from? You just said it was GPL only above in the header of the file? > +/** > + * srom_setup_cdev() - Set up a device instance in the cdev table. > + * @dev: Per-device SROM state. > + * @index: Device to set up. > + */ > +static void srom_setup_cdev(struct srom_dev *dev, int index) > +{ > + int err, devno = MKDEV(srom_major, index); > + > + cdev_init(&dev->cdev, &srom_fops); > + dev->cdev.owner = THIS_MODULE; > + dev->cdev.ops = &srom_fops; > + err = cdev_add(&dev->cdev, devno, 1); > + /* Fail gracefully if need be */ > + if (err) > + pr_notice("Error %d adding srom%d", err, index); > +} As you are only using 1 minor device, why not just use a misc device instead? It's simpler, and you get the sysfs code for free, which you forgot to do, so your device node will never show up in userspace :( thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-28 21:23 ` Greg KH @ 2011-05-29 0:32 ` Chris Metcalf 2011-05-29 11:45 ` Greg KH 0 siblings, 1 reply; 27+ messages in thread From: Chris Metcalf @ 2011-05-29 0:32 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, Arnd Bergmann, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On 5/28/2011 5:23 PM, Greg KH wrote: > On Sat, May 28, 2011 at 11:13:54AM -0400, Chris Metcalf wrote: >> + * This source code is derived from code provided in "Linux Device >> + * Drivers" by Alessandro Rubini and Jonathan Corbet, published by >> + * O'Reilly & Associates. >> + */ > LDD version 2? Wow, this driver is old. The driver was originally written in 2007. LDD3 came out in 2005, right? I guess we were a little behind the curve :-) >> +MODULE_LICENSE("Dual BSD/GPL"); > Wait, where did the BSD come from? You just said it was GPL only above > in the header of the file? Good point. We have a boiler-plate auto-generated GPL license comment that we use for kernel sources. In general, we've tended toward licenses that are as available to the community as possible -- for example, the string code that we've written is under GPL in the kernel (and glibc) but under a BSD license in newlib, because we wrote it all from scratch and can do that. And, since we're hardware vendors, our general goal is to enable whatever kinds of software will sell chips. :-) My guess is that we should be using a consistent GPL-only license for these kinds of kernel drivers, because frankly, they aren't useful outside the context of Linux. I'll double-check with the rest of the software team, but I think we should probably just switch this over to a straight "GPL" license. > As you are only using 1 minor device, why not just use a misc device > instead? It's simpler, and you get the sysfs code for free, which you > forgot to do, so your device node will never show up in userspace :( Interesting; this appears to be a bug. We use 4 minors (see "srom_devs = 4" higher up). I'll fix this. We may have some other devices that would benefit from being recast as misc devices, so I'll look at our set of internal devices. Is there a good example of a character device that has multiple minors and also is registered with sysfs? -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-29 0:32 ` Chris Metcalf @ 2011-05-29 11:45 ` Greg KH 2011-05-29 12:18 ` Chris Metcalf 0 siblings, 1 reply; 27+ messages in thread From: Greg KH @ 2011-05-29 11:45 UTC (permalink / raw) To: Chris Metcalf Cc: linux-kernel, Arnd Bergmann, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On Sat, May 28, 2011 at 08:32:07PM -0400, Chris Metcalf wrote: > > As you are only using 1 minor device, why not just use a misc device > > instead? It's simpler, and you get the sysfs code for free, which you > > forgot to do, so your device node will never show up in userspace :( > > Interesting; this appears to be a bug. We use 4 minors (see "srom_devs = > 4" higher up). I'll fix this. We may have some other devices that would > benefit from being recast as misc devices, so I'll look at our set of > internal devices. This kind of implies that you didn't test this code, right? You might want to do that next time :) > Is there a good example of a character device that has multiple minors and > also is registered with sysfs? Lots of them, look all over the kernel, and in LDD3 there's even an outdated example of one I think. thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-29 11:45 ` Greg KH @ 2011-05-29 12:18 ` Chris Metcalf 2011-05-29 13:47 ` Greg KH 2011-05-29 15:45 ` Arnd Bergmann 0 siblings, 2 replies; 27+ messages in thread From: Chris Metcalf @ 2011-05-29 12:18 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, Arnd Bergmann, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On 5/29/2011 7:45 AM, Greg KH wrote: > On Sat, May 28, 2011 at 08:32:07PM -0400, Chris Metcalf wrote: >>> As you are only using 1 minor device, why not just use a misc device >>> instead? It's simpler, and you get the sysfs code for free, which you >>> forgot to do, so your device node will never show up in userspace :( >> Interesting; this appears to be a bug. We use 4 minors (see "srom_devs = >> 4" higher up). I'll fix this. We may have some other devices that would >> benefit from being recast as misc devices, so I'll look at our set of >> internal devices. > This kind of implies that you didn't test this code, right? You might > want to do that next time :) No, this bug has been in the code since day one (I just double-checked our SCM), and it has always worked fine. I'm looking into how this is possible now, but trust me, we've tested this aspect of the driver the whole time. :-) >> Is there a good example of a character device that has multiple minors and >> also is registered with sysfs? > Lots of them, look all over the kernel, and in LDD3 there's even an > outdated example of one I think. I didn't actually find many drivers that have both cdev_add with count>1, and mention sysfs or kobj, but char/raw.c is one that looks pretty clean. (I found only four others total: uio/uio.c, s390/char/vmlogrdr.c, staging/comedi/comedi_fops.c, and staging/vme/devices/vme_user.c.) -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-29 12:18 ` Chris Metcalf @ 2011-05-29 13:47 ` Greg KH 2011-05-29 15:45 ` Arnd Bergmann 1 sibling, 0 replies; 27+ messages in thread From: Greg KH @ 2011-05-29 13:47 UTC (permalink / raw) To: Chris Metcalf Cc: linux-kernel, Arnd Bergmann, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On Sun, May 29, 2011 at 08:18:36AM -0400, Chris Metcalf wrote: > >> Is there a good example of a character device that has multiple minors and > >> also is registered with sysfs? > > Lots of them, look all over the kernel, and in LDD3 there's even an > > outdated example of one I think. > > I didn't actually find many drivers that have both cdev_add with count>1, > and mention sysfs or kobj, but char/raw.c is one that looks pretty clean. > (I found only four others total: uio/uio.c, s390/char/vmlogrdr.c, > staging/comedi/comedi_fops.c, and staging/vme/devices/vme_user.c.) No, you don't need "raw" kobjects or sysfs at all. Just use the device_create() call. Hope this helps, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-29 12:18 ` Chris Metcalf 2011-05-29 13:47 ` Greg KH @ 2011-05-29 15:45 ` Arnd Bergmann 2011-05-29 18:23 ` Chris Metcalf 1 sibling, 1 reply; 27+ messages in thread From: Arnd Bergmann @ 2011-05-29 15:45 UTC (permalink / raw) To: Chris Metcalf Cc: Greg KH, linux-kernel, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On Sunday 29 May 2011 14:18:36 Chris Metcalf wrote: > On 5/29/2011 7:45 AM, Greg KH wrote: > > On Sat, May 28, 2011 at 08:32:07PM -0400, Chris Metcalf wrote: > >>> As you are only using 1 minor device, why not just use a misc device > >>> instead? It's simpler, and you get the sysfs code for free, which you > >>> forgot to do, so your device node will never show up in userspace :( > >> Interesting; this appears to be a bug. We use 4 minors (see "srom_devs = > >> 4" higher up). I'll fix this. We may have some other devices that would > >> benefit from being recast as misc devices, so I'll look at our set of > >> internal devices. > > This kind of implies that you didn't test this code, right? You might > > want to do that next time :) > > No, this bug has been in the code since day one (I just double-checked our > SCM), and it has always worked fine. I'm looking into how this is possible > now, but trust me, we've tested this aspect of the driver the whole time. :-) > AFAICT, the driver just calls cdev_add in a loop, adding one device add a time. This is actually a correct way to register multiple character devices, but simply passing the number of devices you want to add as the third argument would be simpler. On a related note, the number of devices you add is a module parameter, which seems a bit backwards, since the hypervisor should actually know how many devices there are. Can't you just ask the hypervisor? Arnd ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-29 15:45 ` Arnd Bergmann @ 2011-05-29 18:23 ` Chris Metcalf 0 siblings, 0 replies; 27+ messages in thread From: Chris Metcalf @ 2011-05-29 18:23 UTC (permalink / raw) To: Arnd Bergmann Cc: Greg KH, linux-kernel, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On 5/29/2011 11:45 AM, Arnd Bergmann wrote: > On Sunday 29 May 2011 14:18:36 Chris Metcalf wrote: >> On 5/29/2011 7:45 AM, Greg KH wrote: >>> On Sat, May 28, 2011 at 08:32:07PM -0400, Chris Metcalf wrote: >>>>> As you are only using 1 minor device, why not just use a misc device >>>>> instead? It's simpler, and you get the sysfs code for free, which you >>>>> forgot to do, so your device node will never show up in userspace :( >>>> Interesting; this appears to be a bug. We use 4 minors (see "srom_devs = >>>> 4" higher up). I'll fix this. We may have some other devices that would >>>> benefit from being recast as misc devices, so I'll look at our set of >>>> internal devices. >>> This kind of implies that you didn't test this code, right? You might >>> want to do that next time :) >> No, this bug has been in the code since day one (I just double-checked our >> SCM), and it has always worked fine. I'm looking into how this is possible >> now, but trust me, we've tested this aspect of the driver the whole time. :-) >> > AFAICT, the driver just calls cdev_add in a loop, adding one device > add a time. This is actually a correct way to register multiple > character devices, but simply passing the number of devices you > want to add as the third argument would be simpler. Good catch! Amusingly, I audited our other drivers that we haven't tried to push back yet, and did find one that actually did have this bug: it was doing cdev_add() without the surrounding loop, but without passing the right minor count from alloc_chrdev_region() (although in that case the default value was "1" in any case). I restructured the tile-srom init code to call cdev_add() once, using a single global "struct cdev". The "open" routine now uses "iminor()" instead of "container_of()" to get the proper srom_dev structure with the driver's per-partition info. > On a related note, the number of devices you add is a module parameter, > which seems a bit backwards, since the hypervisor should actually > know how many devices there are. Can't you just ask the hypervisor? Fair point. I restructured the init so we loop calling hv_dev_open() until the hypervisor says we've hit a bad partition number, and that's the number of partitions the Linux driver then supports. -- Chris Metcalf, Tilera Corp. http://www.tilera.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-05-21 15:50 ` Eric Biederman 2011-05-23 20:10 ` Chris Metcalf 2011-05-28 15:13 ` [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM Chris Metcalf @ 2011-06-02 15:04 ` Chris Metcalf 2011-06-10 16:41 ` Arnd Bergmann 2 siblings, 1 reply; 27+ messages in thread From: Chris Metcalf @ 2011-06-02 15:04 UTC (permalink / raw) To: linux-kernel, Arnd Bergmann Cc: Greg KH, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody The first version of this patch proposed an arch/tile/drivers/ directory, but the consensus was that this was probably a poor choice for a place to group Tilera-specific drivers, and that in any case grouping by platform was discouraged, and grouping by function was preferred. This version of the patch addresses various issues raised in the community, primarily the absence of sysfs integration. The sysfs integration now handles passing information on sector size, page size, and total partition size to userspace as well. In addition, we now use a single "struct cdev" to manage all the partition minor devices, and dynamically discover the correct number of partitions from the hypervisor rather than using a module_param with a default value. This driver has no particular "peer" drivers it can be grouped with. It is sort of like an MTD driver for SPI ROM, but it doesn't group well with the other MTD devices since it relies on hypervisor virtualization to handle many of the irritating aspects of flash ROM management: sector awareness, background read for sub-sector writes, bit examination to determine whether a sector erase needs to be issued, etc. It is in fact more like an EEPROM driver, but the hypervisor virtualization does require a "flush" command if you wish to commit a sector write prior to writing to a different sector, and this is sufficiently different from generic I2C/SPI EEPROMs that as a result it doesn't group well with them either. The simple character device is already in use by a range of Tilera SPI ROM management tools, as well as by customers. In addition, using the simple character device actually simplifies the userspace tools, since they don't need to manage sector erase, background read, etc. This both simplifies the code (since we can uniformly manage plain files and the SPI ROM) as well as makes the user code portable to non-Linux platforms that don't offer the same MTD ioctls. Signed-off-by: Chris Metcalf <cmetcalf@tilera.com> --- arch/tile/include/hv/drv_srom_intf.h | 41 +++ drivers/char/Kconfig | 11 + drivers/char/Makefile | 2 + drivers/char/tile-srom.c | 481 ++++++++++++++++++++++++++++++++++ 4 files changed, 535 insertions(+), 0 deletions(-) create mode 100644 arch/tile/include/hv/drv_srom_intf.h create mode 100644 drivers/char/tile-srom.c diff --git a/arch/tile/include/hv/drv_srom_intf.h b/arch/tile/include/hv/drv_srom_intf.h new file mode 100644 index 0000000..6395faa --- /dev/null +++ b/arch/tile/include/hv/drv_srom_intf.h @@ -0,0 +1,41 @@ +/* + * Copyright 2011 Tilera Corporation. All Rights Reserved. + * + * 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, version 2. + * + * 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, GOOD TITLE or + * NON INFRINGEMENT. See the GNU General Public License for + * more details. + */ + +/** + * @file drv_srom_intf.h + * Interface definitions for the SPI Flash ROM driver. + */ + +#ifndef _SYS_HV_INCLUDE_DRV_SROM_INTF_H +#define _SYS_HV_INCLUDE_DRV_SROM_INTF_H + +/** Read this offset to get the total device size. */ +#define SROM_TOTAL_SIZE_OFF 0xF0000000 + +/** Read this offset to get the device sector size. */ +#define SROM_SECTOR_SIZE_OFF 0xF0000004 + +/** Read this offset to get the device page size. */ +#define SROM_PAGE_SIZE_OFF 0xF0000008 + +/** Write this offset to flush any pending writes. */ +#define SROM_FLUSH_OFF 0xF1000000 + +/** Write this offset, plus the byte offset of the start of a sector, to + * erase a sector. Any write data is ignored, but there must be at least + * one byte of write data. Only applies when the driver is in MTD mode. + */ +#define SROM_ERASE_OFF 0xF2000000 + +#endif /* _SYS_HV_INCLUDE_DRV_SROM_INTF_H */ diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 49502bc..5afd610 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -616,5 +616,16 @@ config MSM_SMD_PKT Enables userspace clients to read and write to some packet SMD ports via device interface for MSM chipset. +config TILE_SROM + bool "Character-device access via hypervisor to the Tilera SPI ROM" + depends on TILE + default y + ---help--- + This device provides character-level read-write access + to the SROM, typically via the "0", "1", "2", and "info" + devices in /dev/srom/. The Tilera hypervisor makes the + flash device appear much like a simple EEPROM, and knows + how to partition a single ROM for multiple purposes. + endmenu diff --git a/drivers/char/Makefile b/drivers/char/Makefile index 7a00672..32762ba 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -63,3 +63,5 @@ obj-$(CONFIG_RAMOOPS) += ramoops.o obj-$(CONFIG_JS_RTC) += js-rtc.o js-rtc-y = rtc.o + +obj-$(CONFIG_TILE_SROM) += tile-srom.o diff --git a/drivers/char/tile-srom.c b/drivers/char/tile-srom.c new file mode 100644 index 0000000..cf3ee00 --- /dev/null +++ b/drivers/char/tile-srom.c @@ -0,0 +1,481 @@ +/* + * Copyright 2011 Tilera Corporation. All Rights Reserved. + * + * 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, version 2. + * + * 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, GOOD TITLE or + * NON INFRINGEMENT. See the GNU General Public License for + * more details. + * + * SPI Flash ROM driver + * + * This source code is derived from code provided in "Linux Device + * Drivers, Third Edition", by Jonathan Corbet, Alessandro Rubini, and + * Greg Kroah-Hartman, published by O'Reilly Media, Inc. + */ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/kernel.h> /* printk() */ +#include <linux/slab.h> /* kmalloc() */ +#include <linux/fs.h> /* everything... */ +#include <linux/errno.h> /* error codes */ +#include <linux/types.h> /* size_t */ +#include <linux/proc_fs.h> +#include <linux/fcntl.h> /* O_ACCMODE */ +#include <linux/aio.h> +#include <linux/pagemap.h> +#include <linux/hugetlb.h> +#include <linux/uaccess.h> +#include <linux/platform_device.h> +#include <hv/hypervisor.h> +#include <linux/ioctl.h> +#include <linux/cdev.h> +#include <linux/delay.h> +#include <hv/drv_srom_intf.h> + +/* + * Size of our hypervisor I/O requests. We break up large transfers + * so that we don't spend large uninterrupted spans of time in the + * hypervisor. Erasing an SROM sector takes a significant fraction of + * a second, so if we allowed the user to, say, do one I/O to write the + * entire ROM, we'd get soft lockup timeouts, or worse. + */ +#define SROM_CHUNK_SIZE ((size_t)4096) + +/* + * When hypervisor is busy (e.g. erasing), poll the status periodically. + */ + +/* + * Interval to poll the state in msec + */ +#define SROM_WAIT_TRY_INTERVAL 20 + +/* + * Maximum times to poll the state + */ +#define SROM_MAX_WAIT_TRY_TIMES 1000 + +struct srom_dev { + int hv_devhdl; /* Handle for hypervisor device */ + u32 total_size; /* Size of this device */ + u32 sector_size; /* Size of a sector */ + u32 page_size; /* Size of a page */ + struct mutex lock; /* Allow only one accessor at a time */ +}; + +static int srom_major; /* Dynamic major by default */ +module_param(srom_major, int, 0); +MODULE_AUTHOR("Tilera Corporation"); +MODULE_LICENSE("GPL"); + +static int srom_devs; /* Number of SROM partitions */ +static struct cdev srom_cdev; +static struct class *srom_class; +static struct srom_dev *srom_devices; + +/* + * Handle calling the hypervisor and managing EAGAIN/EBUSY. + */ + +static ssize_t _srom_read(int hv_devhdl, void *buf, + loff_t off, size_t count) +{ + int retval, retries = SROM_MAX_WAIT_TRY_TIMES; + for (;;) { + retval = hv_dev_pread(hv_devhdl, 0, (HV_VirtAddr)buf, + count, off); + if (retval >= 0) + return retval; + if (retval == HV_EAGAIN) + continue; + if (retval == HV_EBUSY && --retries > 0) { + msleep(SROM_WAIT_TRY_INTERVAL); + continue; + } + pr_err("_srom_read: error %d\n", retval); + return -EIO; + } +} + +static ssize_t _srom_write(int hv_devhdl, const void *buf, + loff_t off, size_t count) +{ + int retval, retries = SROM_MAX_WAIT_TRY_TIMES; + for (;;) { + retval = hv_dev_pwrite(hv_devhdl, 0, (HV_VirtAddr)buf, + count, off); + if (retval >= 0) + return retval; + if (retval == HV_EAGAIN) + continue; + if (retval == HV_EBUSY && --retries > 0) { + msleep(SROM_WAIT_TRY_INTERVAL); + continue; + } + pr_err("_srom_write: error %d\n", retval); + return -EIO; + } +} + +/** + * srom_open() - Device open routine. + * @inode: Inode for this device. + * @filp: File for this specific open of the device. + * + * Returns zero, or an error code. + */ +static int srom_open(struct inode *inode, struct file *filp) +{ + filp->private_data = &srom_devices[iminor(inode)]; + return 0; +} + + +/** + * srom_release() - Device release routine. + * @inode: Inode for this device. + * @filp: File for this specific open of the device. + * + * Returns zero, or an error code. + */ +static int srom_release(struct inode *inode, struct file *filp) +{ + struct srom_dev *srom = filp->private_data; + char dummy; + + /* Make sure we've flushed anything written to the ROM. */ + mutex_lock(&srom->lock); + if (srom->hv_devhdl >= 0) + _srom_write(srom->hv_devhdl, &dummy, SROM_FLUSH_OFF, 1); + mutex_unlock(&srom->lock); + + filp->private_data = NULL; + + return 0; +} + + +/** + * srom_read() - Read data from the device. + * @filp: File for this specific open of the device. + * @buf: User's data buffer. + * @count: Number of bytes requested. + * @f_pos: File position. + * + * Returns number of bytes read, or an error code. + */ +static ssize_t srom_read(struct file *filp, char __user *buf, + size_t count, loff_t *f_pos) +{ + int retval = 0; + void *kernbuf; + struct srom_dev *srom = filp->private_data; + + kernbuf = kmalloc(SROM_CHUNK_SIZE, GFP_KERNEL); + if (!kernbuf) + return -ENOMEM; + + if (mutex_lock_interruptible(&srom->lock)) { + retval = -ERESTARTSYS; + kfree(kernbuf); + return retval; + } + + while (count) { + int hv_retval; + int bytes_this_pass = min(count, SROM_CHUNK_SIZE); + + hv_retval = _srom_read(srom->hv_devhdl, kernbuf, + *f_pos, bytes_this_pass); + if (hv_retval > 0) { + if (copy_to_user(buf, kernbuf, hv_retval) != 0) { + retval = -EFAULT; + break; + } + } else if (hv_retval <= 0) { + if (retval == 0) + retval = hv_retval; + break; + } + + retval += hv_retval; + *f_pos += hv_retval; + buf += hv_retval; + count -= hv_retval; + } + + mutex_unlock(&srom->lock); + kfree(kernbuf); + + return retval; +} + +/** + * srom_write() - Write data to the device. + * @filp: File for this specific open of the device. + * @buf: User's data buffer. + * @count: Number of bytes requested. + * @f_pos: File position. + * + * Returns number of bytes written, or an error code. + */ +static ssize_t srom_write(struct file *filp, const char __user *buf, + size_t count, loff_t *f_pos) +{ + int retval = 0; + void *kernbuf; + struct srom_dev *srom = filp->private_data; + + kernbuf = kmalloc(SROM_CHUNK_SIZE, GFP_KERNEL); + if (!kernbuf) + return -ENOMEM; + + if (mutex_lock_interruptible(&srom->lock)) { + retval = -ERESTARTSYS; + kfree(kernbuf); + return retval; + } + + while (count) { + int hv_retval; + int bytes_this_pass = min(count, SROM_CHUNK_SIZE); + + if (copy_from_user(kernbuf, buf, bytes_this_pass) != 0) { + retval = -EFAULT; + break; + } + + hv_retval = _srom_write(srom->hv_devhdl, kernbuf, + *f_pos, bytes_this_pass); + if (hv_retval <= 0) { + if (retval == 0) + retval = hv_retval; + break; + } + + retval += hv_retval; + *f_pos += hv_retval; + buf += hv_retval; + count -= hv_retval; + } + + mutex_unlock(&srom->lock); + kfree(kernbuf); + + return retval; +} + +/* Provide our own implementation so we can use srom->total_size. */ +loff_t srom_llseek(struct file *filp, loff_t offset, int origin) +{ + struct srom_dev *srom = filp->private_data; + + if (mutex_lock_interruptible(&srom->lock)) + return -ERESTARTSYS; + + switch (origin) { + case SEEK_END: + offset += srom->total_size; + break; + case SEEK_CUR: + offset += filp->f_pos; + break; + } + + if (offset < 0 || offset > srom->total_size) { + offset = -EINVAL; + } else { + filp->f_pos = offset; + filp->f_version = 0; + } + + mutex_unlock(&srom->lock); + + return offset; +} + +static ssize_t total_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct srom_dev *srom = dev_get_drvdata(dev); + return sprintf(buf, "%u\n", srom->total_size); +} + +static ssize_t sector_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct srom_dev *srom = dev_get_drvdata(dev); + return sprintf(buf, "%u\n", srom->sector_size); +} + +static ssize_t page_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct srom_dev *srom = dev_get_drvdata(dev); + return sprintf(buf, "%u\n", srom->page_size); +} + +static struct device_attribute srom_dev_attrs[] = { + __ATTR(total_size, S_IRUGO, total_show, NULL), + __ATTR(sector_size, S_IRUGO, sector_show, NULL), + __ATTR(page_size, S_IRUGO, page_show, NULL), + __ATTR_NULL +}; + +static char *srom_devnode(struct device *dev, mode_t *mode) +{ + *mode = S_IRUGO | S_IWUSR; + return kasprintf(GFP_KERNEL, "srom/%s", dev_name(dev)); +} + +/* + * The fops + */ +static const struct file_operations srom_fops = { + .owner = THIS_MODULE, + .llseek = srom_llseek, + .read = srom_read, + .write = srom_write, + .open = srom_open, + .release = srom_release, +}; + +/** + * srom_setup_minor() - Initialize per-minor information. + * @srom: Per-device SROM state. + * @index: Device to set up. + */ +static int srom_setup_minor(struct srom_dev *srom, int index) +{ + struct device *dev; + int devhdl = srom->hv_devhdl; + + mutex_init(&srom->lock); + + if (_srom_read(devhdl, &srom->total_size, + SROM_TOTAL_SIZE_OFF, sizeof(srom->total_size)) < 0) + return -EIO; + if (_srom_read(devhdl, &srom->sector_size, + SROM_SECTOR_SIZE_OFF, sizeof(srom->sector_size)) < 0) + return -EIO; + if (_srom_read(devhdl, &srom->page_size, + SROM_PAGE_SIZE_OFF, sizeof(srom->page_size)) < 0) + return -EIO; + + dev = device_create(srom_class, &platform_bus, + MKDEV(srom_major, index), srom, "%d", index); + return IS_ERR(dev) ? PTR_ERR(dev) : 0; +} + +/** srom_init() - Initialize the driver's module. */ +static int srom_init(void) +{ + int result, i; + dev_t dev = MKDEV(srom_major, 0); + + /* + * Start with a plausible number of partitions; the krealloc() call + * below will yield about log(srom_devs) additional allocations. + */ + srom_devices = kzalloc(4 * sizeof(struct srom_dev), GFP_KERNEL); + + /* Discover the number of srom partitions. */ + for (i = 0; ; i++) { + int devhdl; + char buf[20]; + struct srom_dev *new_srom_devices = + krealloc(srom_devices, (i+1) * sizeof(struct srom_dev), + GFP_KERNEL | __GFP_ZERO); + if (!new_srom_devices) { + result = -ENOMEM; + goto fail_mem; + } + srom_devices = new_srom_devices; + sprintf(buf, "srom/0/%d", i); + devhdl = hv_dev_open((HV_VirtAddr)buf, 0); + if (devhdl < 0) { + if (devhdl != HV_ENODEV) + pr_notice("srom/%d: hv_dev_open failed: %d.\n", + i, devhdl); + break; + } + srom_devices[i].hv_devhdl = devhdl; + } + srom_devs = i; + + /* Bail out early if we have no partitions at all. */ + if (srom_devs == 0) { + result = -ENODEV; + goto fail_mem; + } + + /* Register our major, and accept a dynamic number. */ + if (srom_major) + result = register_chrdev_region(dev, srom_devs, "srom"); + else { + result = alloc_chrdev_region(&dev, 0, srom_devs, "srom"); + srom_major = MAJOR(dev); + } + if (result < 0) + goto fail_mem; + + /* Register a character device. */ + cdev_init(&srom_cdev, &srom_fops); + srom_cdev.owner = THIS_MODULE; + srom_cdev.ops = &srom_fops; + result = cdev_add(&srom_cdev, dev, srom_devs); + if (result < 0) + goto fail_chrdev; + + /* Create a sysfs class. */ + srom_class = class_create(THIS_MODULE, "srom"); + if (IS_ERR(srom_class)) { + result = PTR_ERR(srom_class); + goto fail_cdev; + } + srom_class->dev_attrs = srom_dev_attrs; + srom_class->devnode = srom_devnode; + + /* Do per-partition initialization */ + for (i = 0; i < srom_devs; i++) { + result = srom_setup_minor(srom_devices + i, i); + if (result < 0) + goto fail_class; + } + + return 0; + +fail_class: + for (i = 0; i < srom_devs; i++) + device_destroy(srom_class, MKDEV(srom_major, i)); + class_destroy(srom_class); +fail_cdev: + cdev_del(&srom_cdev); +fail_chrdev: + unregister_chrdev_region(dev, srom_devs); +fail_mem: + kfree(srom_devices); + return result; +} + +/** srom_cleanup() - Clean up the driver's module. */ +static void srom_cleanup(void) +{ + int i; + for (i = 0; i < srom_devs; i++) + device_destroy(srom_class, MKDEV(srom_major, i)); + class_destroy(srom_class); + cdev_del(&srom_cdev); + unregister_chrdev_region(MKDEV(srom_major, 0), srom_devs); + kfree(srom_devices); +} + +module_init(srom_init); +module_exit(srom_cleanup); -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v3] arch/tile: add hypervisor-based character driver for SPI flash ROM 2011-06-02 15:04 ` [PATCH v3] " Chris Metcalf @ 2011-06-10 16:41 ` Arnd Bergmann 0 siblings, 0 replies; 27+ messages in thread From: Arnd Bergmann @ 2011-06-10 16:41 UTC (permalink / raw) To: Chris Metcalf Cc: linux-kernel, Greg KH, Eric Biederman, Chris Wright, Benjamin Thery, Phil Carmody On Thursday 02 June 2011, Chris Metcalf wrote: > The first version of this patch proposed an arch/tile/drivers/ directory, > but the consensus was that this was probably a poor choice for a place to > group Tilera-specific drivers, and that in any case grouping by platform > was discouraged, and grouping by function was preferred. > > This version of the patch addresses various issues raised in the > community, primarily the absence of sysfs integration. The sysfs > integration now handles passing information on sector size, page size, > and total partition size to userspace as well. In addition, we now > use a single "struct cdev" to manage all the partition minor devices, > and dynamically discover the correct number of partitions from the > hypervisor rather than using a module_param with a default value. > > This driver has no particular "peer" drivers it can be grouped with. > It is sort of like an MTD driver for SPI ROM, but it doesn't group well > with the other MTD devices since it relies on hypervisor virtualization > to handle many of the irritating aspects of flash ROM management: sector > awareness, background read for sub-sector writes, bit examination to > determine whether a sector erase needs to be issued, etc. It is in fact > more like an EEPROM driver, but the hypervisor virtualization does require > a "flush" command if you wish to commit a sector write prior to writing > to a different sector, and this is sufficiently different from generic > I2C/SPI EEPROMs that as a result it doesn't group well with them either. > > The simple character device is already in use by a range of Tilera > SPI ROM management tools, as well as by customers. In addition, using > the simple character device actually simplifies the userspace tools, > since they don't need to manage sector erase, background read, etc. > This both simplifies the code (since we can uniformly manage plain files > and the SPI ROM) as well as makes the user code portable to non-Linux > platforms that don't offer the same MTD ioctls. > > Signed-off-by: Chris Metcalf <cmetcalf@tilera.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-20 23:39 ` Chris Metcalf 2011-05-21 3:21 ` Greg KH @ 2011-05-21 7:46 ` Eric Biederman 2011-05-21 8:32 ` Arnd Bergmann 1 sibling, 1 reply; 27+ messages in thread From: Eric Biederman @ 2011-05-21 7:46 UTC (permalink / raw) To: Chris Metcalf Cc: Arnd Bergmann, linux-kernel, Chris Wright, Greg Kroah-Hartman, Benjamin Thery, Phil Carmody On Fri, May 20, 2011 at 4:39 PM, Chris Metcalf <cmetcalf@tilera.com> wrote: > On 5/20/2011 6:40 PM, Eric Biederman wrote: >> Please do not make sysfs the direct access method to your device. >> I may be wrong but I don't think any other driver relies exclusively on sysfs. > > I'm basing my implementation on drivers/misc/eeprom/. All the drivers > there use the same sysfs model. > >> Certainly with the introduction of a flush you are introducing an completely >> different usage paradigm from current users and will need an entirely different >> set of tools. > > I don't think using my proposed implementation will be detectably different > for most user tools. The addition of the flush() method just allows my > implementation to defer the final sector's write until the device is closed > (sectors are in fact still written to hardware as one does seek() or > write() from one sector to another; only the "current" sector is cached by > the hypervisor). I suppose some third-party tool that kept the eeprom file > descriptor open indefinitely and did multiple writes to the same sector > might not work as expected with this implementation. But it seems hard to > imagine a use case for such a tool. > > The direct motivation for this case is to "impedance match" to the > hypervisor driver for this device, which handles sector management > internally, so the Linux device doesn't have to. Having a 'flush' method > avoids excessive re-writes of the same sector for certain access patterns. > The only alternatives that I see are to rewrite the tile userspace tools, > but they are the way they are because the current model gives good > consistency guarantees for writing the boot rom in the presence of > arbitrary failure modes; or, to add something like a delayed timer event > that allows the Linux driver to notify the hypervisor driver that writes > are likely complete and it can write out the last sector. Neither of these > are particularly attractive. What is wrong with an mtd driver? Looking a bit back into the conversation it appears clear that you are talking about something that resembles NOR flash with multiple sectors, etc. eeproms have random byte access and are typically 256 bytes. You devices doesn't sound anything like an eeprom. > I apologize for not including more of the back story in this email when > adding the cc's, by the way. Originally I proposed a straightforward > character device for this role. Arnd Bergmann encouraged me to look at > kernel precedents like drivers/char/eeprom/, which is why I converted this > driver to sysfs. The first post in this thread is here: > https://lkml.org/lkml/2011/5/4/415 . Since then I've come around to > believing that it's more valuable to group this driver with the other > eeprom drivers, rather than with the other paravirtualized tile drivers. Again this sounds most like an mtd driver to me, and arguably should be grouped there. If you have to worry about keeping sectors open or closed your hardware is very much not an eeprom. Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 7:46 ` [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver Eric Biederman @ 2011-05-21 8:32 ` Arnd Bergmann 2011-05-22 0:54 ` Mike Frysinger 0 siblings, 1 reply; 27+ messages in thread From: Arnd Bergmann @ 2011-05-21 8:32 UTC (permalink / raw) To: Eric Biederman Cc: Chris Metcalf, linux-kernel, Chris Wright, Greg Kroah-Hartman, Benjamin Thery, Phil Carmody On Saturday 21 May 2011 09:46:55 Eric Biederman wrote: > On Fri, May 20, 2011 at 4:39 PM, Chris Metcalf <cmetcalf@tilera.com> wrote: > > On 5/20/2011 6:40 PM, Eric Biederman wrote: > > The direct motivation for this case is to "impedance match" to the > > hypervisor driver for this device, which handles sector management > > internally, so the Linux device doesn't have to. Having a 'flush' method > > avoids excessive re-writes of the same sector for certain access patterns. > > The only alternatives that I see are to rewrite the tile userspace tools, > > but they are the way they are because the current model gives good > > consistency guarantees for writing the boot rom in the presence of > > arbitrary failure modes; or, to add something like a delayed timer event > > that allows the Linux driver to notify the hypervisor driver that writes > > are likely complete and it can write out the last sector. Neither of these > > are particularly attractive. > > What is wrong with an mtd driver? > > Looking a bit back into the conversation it appears clear that you are > talking about something that resembles NOR flash with multiple sectors, > etc. > > eeproms have random byte access and are typically 256 bytes. You devices > doesn't sound anything like an eeprom. MTD implies that you have low-level access to the NOR flash registers, which this one doesn't. It's certainly not the right Linux interface for a high-level flash device, and there is no precedent for this at all in Linux. We do have precedent for multiple interfaces that have the same purpose as this one: drivers/misc/eeprom/* drivers/char/ps3flash.c drivers/char/nwflash.c drivers/char/bfin-otp.c arch/powerpc/kernel/rtas_flash.c drivers/sbus/char/jsflash.c And then some more that I missed, plus the ones that use MTD. Arnd ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver 2011-05-21 8:32 ` Arnd Bergmann @ 2011-05-22 0:54 ` Mike Frysinger 0 siblings, 0 replies; 27+ messages in thread From: Mike Frysinger @ 2011-05-22 0:54 UTC (permalink / raw) To: Arnd Bergmann Cc: Eric Biederman, Chris Metcalf, linux-kernel, Chris Wright, Greg Kroah-Hartman, Benjamin Thery, Phil Carmody On Sat, May 21, 2011 at 01:32, Arnd Bergmann wrote: > On Saturday 21 May 2011 09:46:55 Eric Biederman wrote: >> What is wrong with an mtd driver? >> >> Looking a bit back into the conversation it appears clear that you are >> talking about something that resembles NOR flash with multiple sectors, >> etc. >> >> eeproms have random byte access and are typically 256 bytes. You devices >> doesn't sound anything like an eeprom. > > MTD implies that you have low-level access to the NOR flash registers, > which this one doesn't. It's certainly not the right Linux interface > for a high-level flash device, and there is no precedent for this at > all in Linux. > > We do have precedent for multiple interfaces that have the same > purpose as this one: > > drivers/misc/eeprom/* > drivers/char/ps3flash.c > drivers/char/nwflash.c > drivers/char/bfin-otp.c there is a new otp framework in the works that this driver would move to. but that wouldnt matter to the driver being proposed for the tile arch i dont think. -mike ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2011-06-10 16:41 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-05-04 19:10 [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver Chris Metcalf 2011-05-05 6:41 ` Arnd Bergmann 2011-05-06 19:37 ` Chris Metcalf 2011-05-20 18:05 ` Chris Metcalf 2011-05-20 18:46 ` Arnd Bergmann 2011-05-20 22:40 ` Eric Biederman 2011-05-20 23:39 ` Chris Metcalf 2011-05-21 3:21 ` Greg KH 2011-05-21 9:33 ` Arnd Bergmann 2011-05-21 13:52 ` Chris Metcalf 2011-05-21 15:02 ` Arnd Bergmann 2011-05-21 15:31 ` Chris Metcalf 2011-05-21 15:50 ` Eric Biederman 2011-05-23 20:10 ` Chris Metcalf 2011-05-28 15:13 ` [PATCH v2] arch/tile: add hypervisor-based character driver for SPI flash ROM Chris Metcalf 2011-05-28 21:23 ` Greg KH 2011-05-29 0:32 ` Chris Metcalf 2011-05-29 11:45 ` Greg KH 2011-05-29 12:18 ` Chris Metcalf 2011-05-29 13:47 ` Greg KH 2011-05-29 15:45 ` Arnd Bergmann 2011-05-29 18:23 ` Chris Metcalf 2011-06-02 15:04 ` [PATCH v3] " Chris Metcalf 2011-06-10 16:41 ` Arnd Bergmann 2011-05-21 7:46 ` [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver Eric Biederman 2011-05-21 8:32 ` Arnd Bergmann 2011-05-22 0:54 ` Mike Frysinger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.