Re: [PATCH] powerpc: Add support to access the flash on SLOF based systems

[Milton Miller <miltonm@bga.com>]

On Sun Jan 11 at 02:31:22 EST in 2009, Adrian Reber wrote:
> This adds support for a simple character device to access the
> flash for SLOF based systems like the PowerStation, QS2x and
> PXCAB. In the SLOF git there is a user space program with
> which the content of the flash for SLOF based systems can
> be displayed and modified. This can be used to add a Linux
> image to the flash and then directly boot the kernel from the
> flash.
>
> Signed-off-by: Adrian Reber <adrian at lisas.de>
> ---
>
> This is based on the mmio NVRAM driver. I am not sure how useful this
> is for anybody else but I am posting it anyway, hoping to get some
> feedback. Also hoping it can be included at one point.


Normally such drivers are written and mtd drivers.

If slof were not an of implementation I would just say put the right 
properties on the node in the device tree, but the kernel should adapt 
to real OF.  It should be easy to write a driver to hook up a mtd 
platform device if this is a direct mapped flash.


> +
> +static void __iomem *slof_flash_start;
> +static long slof_flash_len;
> +static DEFINE_SPINLOCK(slof_flash_lock);
> +
> +
> +static ssize_t slof_flash_read(struct file *file, char __user *buf,
> +                              size_t count, loff_t *ppos)
> +{
> +       unsigned long flags;
> +       char *tmp;
> +       int rc;
> +
> +       spin_lock_irqsave(&slof_flash_lock, flags);
> +
> +       memcpy_fromio(tmp, slof_flash_start + *ppos, count);
> +
> +       spin_unlock_irqrestore(&slof_flash_lock, flags);
> +

Why do you need a spinlock?  Why does it need to be irq safe?

This decision is also driving the malloc of the temporary buffer, and
you are intentionally returning a short read to userspace.


> +
> +const struct file_operations slof_flash_fops = {
> +       .owner = THIS_MODULE,
> +       .llseek = slof_flash_llseek,
> +       .read = slof_flash_read,
> +};
> +

You mentioned userspace reflashing the image, but this driver seems to
be read only access.

> +static struct miscdevice slof_flash_dev = {
> +       SLOF_FLASH_MINOR,
> +       "slof_flash",
> +       &slof_flash_fops
> +};
> +
> +
> +static int __init slof_flash_init(void)
> +{
> +       struct device_node *slof_flash;
> +       struct device_node *compatible;
> +       struct resource r;
> +       int rc;
> +       unsigned long slof_flash_addr;
> +       /* SLOF is known to run on systems with following values
> +        * for the compatible property: */
> +       char *compstrs[] = {"IBM,Bimini", "IBM,JS21", "IBM,JS20", 
> "IBM,CBEA" };
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(compstrs); i++) {
> +               compatible = of_find_compatible_node(NULL, NULL, 
> compstrs[i]);
> +
> +               if (compatible)
> +                       break;
> +       }


Can you identify slof from the information in the /openprom node?  I 
don't think all js20 and 21 use slof, although the IBM provided 
firmware may also work with this driver.


> +
> +       /* not a system with a SLOF flash */
> +       if (!compatible)
> +               return -ENODEV;
> +
> +       of_node_put(compatible);
> +
> +       slof_flash = of_find_node_by_type(NULL, "flash");
> +       if (!slof_flash) {
> +               printk(KERN_WARNING "SLOF FLASH: "
> +                      "no flash node found in device-tree\n");
> +               return -ENODEV;
> +       }
> +       rc = of_address_to_resource(slof_flash, 0, &r);
> +       if (rc) {
> +               printk(KERN_WARNING "SLOF FLASH: "
> +                      "failed to get address (err %d)\n", rc);
> +               goto out;
> +       }
> +
> +       slof_flash_addr =  r.start;
> +       slof_flash_len = r.end - r.start + 1;
> +
> +       if ((slof_flash_len <= 0) || (!slof_flash_addr)) {
> +               printk(KERN_WARNING "SLOF FLASH: address or length is 
> 0\n");
> +               rc = -EIO;
> +               goto out;
> +       }

Why are these warnings?   again, debug is more approprate

> +
> +       slof_flash_start = ioremap(slof_flash_addr, slof_flash_len);
> +       if (!slof_flash_start) {
> +               printk(KERN_WARNING "SLOF FLASH: failed to ioremap\n");
> +               rc = -ENOMEM;
> +               goto out;
> +       }
> +
> +       printk(KERN_INFO "SLOF FLASH: %luk at 0x%lx mapped to %p\n",
> +              slof_flash_len >> 10, slof_flash_addr, 
> slof_flash_start);

This looks to be a debug message at most.

> +
> +       rc = misc_register(&slof_flash_dev);

And as I said, this should be a mtd driver.

Thanks,
milton