Re: [PATCH] arch/tile: add arch/tile/drivers/ directory with SROM driver

From: Arnd Bergmann
Date: Thu May 05 2011 - 02:41:51 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/