[Cbe-oss-dev] [PATCH 10/15] ps3stor_lib: Add support for multiple regions

[Geert Uytterhoeven]

On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider at gmail.com> wrote:
> Users (ps3disk, ps3flash and ps3rom) retain the old behavior. That is:
> they still only provide access to the first accessible region.
>
> Signed-off-by: Andre Heider <a.heider at gmail.com>
> ---
>  arch/powerpc/include/asm/ps3stor.h |    4 ++--
>  drivers/block/ps3disk.c            |   15 +++++++++++++--
>  drivers/char/ps3flash.c            |   23 +++++++++++++++++------
>  drivers/ps3/ps3stor_lib.c          |   25 ++++++++++++-------------
>  drivers/scsi/ps3rom.c              |   11 +++++++----
>  5 files changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3stor.h b/arch/powerpc/include/asm/ps3stor.h
> index d51e53c..9871c05 100644
> --- a/arch/powerpc/include/asm/ps3stor.h
> +++ b/arch/powerpc/include/asm/ps3stor.h
> @@ -51,7 +51,6 @@ struct ps3_storage_device {
>
>        unsigned int num_regions;
>        unsigned long accessible_regions;
> -       unsigned int region_idx;                /* first accessible region */
>        struct ps3_storage_region regions[0];   /* Must be last */
>  };
>
> @@ -63,7 +62,8 @@ static inline struct ps3_storage_device *to_ps3_storage_device(struct device *de
>  extern int ps3stor_setup(struct ps3_storage_device *dev,
>                         irq_handler_t handler);
>  extern void ps3stor_teardown(struct ps3_storage_device *dev);
> -extern u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev, u64 lpar,
> +extern u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev,
> +                                     unsigned int region_idx, u64 lpar,
>                                      u64 start_sector, u64 sectors,
>                                      int write);
>  extern u64 ps3stor_send_command(struct ps3_storage_device *dev, u64 cmd,
> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
> index 8e1ce2e..96e00ff 100644
> --- a/drivers/block/ps3disk.c
> +++ b/drivers/block/ps3disk.c
> @@ -42,6 +42,7 @@ struct ps3disk_private {
>        spinlock_t lock;                /* Request queue spinlock */
>        struct request_queue *queue;
>        struct gendisk *gendisk;
> +       unsigned int region_idx;        /* first accessible region */
>        unsigned int blocking_factor;
>        struct request *req;
>        u64 raw_capacity;
> @@ -125,7 +126,7 @@ static int ps3disk_submit_request_sg(struct ps3_storage_device *dev,
>        int write = rq_data_dir(req), res;
>        const char *op = write ? "write" : "read";
>        u64 start_sector, sectors;
> -       unsigned int region_id = dev->regions[dev->region_idx].id;
> +       unsigned int region_id = dev->regions[priv->region_idx].id;
>
>  #ifdef DEBUG
>        unsigned int n = 0;
> @@ -408,6 +409,7 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
>        unsigned int devidx;
>        struct request_queue *queue;
>        struct gendisk *gendisk;
> +       unsigned int region_idx;
>
>        if (dev->blk_size < 512) {
>                dev_err(&dev->sbd.core,
> @@ -482,6 +484,14 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
>        }
>
>        priv->gendisk = gendisk;
> +
> +       /* find first accessible region */
> +       for (region_idx = 0; region_idx < dev->num_regions; region_idx++)
> +               if (test_bit(region_idx, &dev->accessible_regions)) {
> +                       priv->region_idx = region_idx;
> +                       break;
> +               }
> +

Why not

priv->region_idx = __ffs(dev->accessible_regions);

like the original code in ps3stor_probe_access() used? Cfr. the code
you removed:

> diff --git a/drivers/ps3/ps3stor_lib.c b/drivers/ps3/ps3stor_lib.c
> index af0afa1..5bbc023 100644
> --- a/drivers/ps3/ps3stor_lib.c
> +++ b/drivers/ps3/ps3stor_lib.c
> @@ -124,15 +128,8 @@ static int ps3stor_probe_access(struct ps3_storage_device *dev)
>        n = hweight_long(dev->accessible_regions);
>        if (n > 1)
>                dev_info(&dev->sbd.core,
> -                        "%s:%u: %lu accessible regions found. Only the first "
> -                        "one will be used\n",
> +                        "%s:%u: %lu accessible regions found\n",
>                         __func__, __LINE__, n);
> -       dev->region_idx = __ffs(dev->accessible_regions);
> -       dev_info(&dev->sbd.core,
> -                "First accessible region has index %u start %llu size %llu\n",
> -                dev->region_idx, dev->regions[dev->region_idx].start,
> -                dev->regions[dev->region_idx].size);
> -
>        return 0;
>  }
>

Same in the other drivers.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds