* [PATCH 09/15] ps3: Limit the number of regions per storage device
From: Andre Heider @ 2011-08-01 20:03 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
There can be only 8 regions, add a sanity check
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
arch/powerpc/include/asm/ps3stor.h | 1 +
arch/powerpc/platforms/ps3/device-init.c | 8 ++++++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/ps3stor.h b/arch/powerpc/include/asm/ps3stor.h
index 6fcaf71..d51e53c 100644
--- a/arch/powerpc/include/asm/ps3stor.h
+++ b/arch/powerpc/include/asm/ps3stor.h
@@ -25,6 +25,7 @@
#include <asm/ps3.h>
+#define PS3_STORAGE_MAX_REGIONS (8)
struct ps3_storage_region {
unsigned int id;
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 6c4b583..830d741 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -349,6 +349,14 @@ static int ps3_setup_storage_dev(const struct ps3_repository_device *repo,
return -ENODEV;
}
+ if (num_regions > PS3_STORAGE_MAX_REGIONS) {
+ pr_warning("%s:%u: device %u:%u reported %u regions, "
+ "limiting to %u\n", __func__, __LINE__,
+ num_regions, repo->bus_index, repo->dev_index,
+ PS3_STORAGE_MAX_REGIONS);
+ num_regions = PS3_STORAGE_MAX_REGIONS;
+ }
+
pr_debug("%s:%u: (%u:%u:%u): port %llu blk_size %llu num_blocks %llu "
"num_regions %u\n", __func__, __LINE__, repo->bus_index,
repo->dev_index, repo->dev_type, port, blk_size, num_blocks,
--
1.7.5.4
^ permalink raw reply related
* [PATCH 10/15] ps3stor_lib: Add support for multiple regions
From: Andre Heider @ 2011-08-01 20:03 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
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@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;
+ }
+
gendisk->major = ps3disk_major;
gendisk->first_minor = devidx * PS3DISK_MINORS;
gendisk->fops = &ps3disk_fops;
@@ -492,7 +502,8 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
devidx+'a');
priv->blocking_factor = dev->blk_size >> 9;
set_capacity(gendisk,
- dev->regions[dev->region_idx].size*priv->blocking_factor);
+ dev->regions[priv->region_idx].size *
+ priv->blocking_factor);
dev_info(&dev->sbd.core,
"%s is a %s (%llu MiB total, %lu MiB for OtherOS)\n",
diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
index b1e8659..47b1dc7 100644
--- a/drivers/char/ps3flash.c
+++ b/drivers/char/ps3flash.c
@@ -36,6 +36,7 @@
struct ps3flash_private {
struct mutex mutex; /* Bounce buffer mutex */
u64 chunk_sectors;
+ unsigned int region_idx; /* first accessible region */
int tag; /* Start sector of buffer, -1 if invalid */
bool dirty;
};
@@ -46,7 +47,8 @@ static int ps3flash_read_write_sectors(struct ps3_storage_device *dev,
u64 start_sector, int write)
{
struct ps3flash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
- u64 res = ps3stor_read_write_sectors(dev, dev->bounce_lpar,
+ u64 res = ps3stor_read_write_sectors(dev, priv->region_idx,
+ dev->bounce_lpar,
start_sector, priv->chunk_sectors,
write);
if (res) {
@@ -98,6 +100,7 @@ static int ps3flash_fetch(struct ps3_storage_device *dev, u64 start_sector)
static loff_t ps3flash_llseek(struct file *file, loff_t offset, int origin)
{
struct ps3_storage_device *dev = ps3flash_dev;
+ struct ps3flash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
loff_t res;
mutex_lock(&file->f_mapping->host->i_mutex);
@@ -106,7 +109,7 @@ static loff_t ps3flash_llseek(struct file *file, loff_t offset, int origin)
offset += file->f_pos;
break;
case 2:
- offset += dev->regions[dev->region_idx].size*dev->blk_size;
+ offset += dev->regions[priv->region_idx].size*dev->blk_size;
break;
}
if (offset < 0) {
@@ -136,7 +139,7 @@ static ssize_t ps3flash_read(char __user *userbuf, void *kernelbuf,
"%s:%u: Reading %zu bytes at position %lld to U0x%p/K0x%p\n",
__func__, __LINE__, count, *pos, userbuf, kernelbuf);
- size = dev->regions[dev->region_idx].size*dev->blk_size;
+ size = dev->regions[priv->region_idx].size*dev->blk_size;
if (*pos >= size || !count)
return 0;
@@ -205,7 +208,7 @@ static ssize_t ps3flash_write(const char __user *userbuf,
"%s:%u: Writing %zu bytes at position %lld from U0x%p/K0x%p\n",
__func__, __LINE__, count, *pos, userbuf, kernelbuf);
- size = dev->regions[dev->region_idx].size*dev->blk_size;
+ size = dev->regions[priv->region_idx].size*dev->blk_size;
if (*pos >= size || !count)
return 0;
@@ -359,6 +362,7 @@ static int __devinit ps3flash_probe(struct ps3_system_bus_device *_dev)
struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
struct ps3flash_private *priv;
int error;
+ unsigned int region_idx;
unsigned long tmp;
/* use static buffer, kmalloc cannot allocate 256 KiB */
@@ -391,14 +395,21 @@ static int __devinit ps3flash_probe(struct ps3_system_bus_device *_dev)
if (error)
goto fail_free_priv;
- tmp = dev->regions[dev->region_idx].start*dev->blk_size;
+ /* 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;
+ }
+
+ tmp = dev->regions[priv->region_idx].start*dev->blk_size;
if (tmp % FLASH_BLOCK_SIZE) {
dev_err(&dev->sbd.core,
"%s:%u region start %lu is not aligned\n", __func__,
__LINE__, tmp);
return -EINVAL;
}
- tmp = dev->regions[dev->region_idx].size*dev->blk_size;
+ tmp = dev->regions[priv->region_idx].size*dev->blk_size;
if (tmp % FLASH_BLOCK_SIZE) {
dev_err(&dev->sbd.core,
"%s:%u region size %lu is not aligned\n", __func__,
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
@@ -101,9 +101,8 @@ static int ps3stor_probe_access(struct ps3_storage_device *dev)
"%s:%u: checking accessibility of region %u\n",
__func__, __LINE__, i);
- dev->region_idx = i;
- res = ps3stor_read_write_sectors(dev, dev->bounce_lpar, 0, 1,
- 0);
+ res = ps3stor_read_write_sectors(dev, i, dev->bounce_lpar,
+ 0, 1, 0);
if (res) {
dev_dbg(&dev->sbd.core, "%s:%u: read failed, "
"region %u is not accessible\n", __func__,
@@ -117,6 +116,11 @@ static int ps3stor_probe_access(struct ps3_storage_device *dev)
/* We can access at least one region */
error = 0;
+
+ dev_info(&dev->sbd.core,
+ "Accessible region found: #%u start %llu size %llu\n",
+ i, dev->regions[i].start, dev->regions[i].size);
+
}
if (error)
return error;
@@ -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;
}
@@ -264,6 +261,7 @@ EXPORT_SYMBOL_GPL(ps3stor_teardown);
/**
* ps3stor_read_write_sectors - read/write from/to a storage device
* @dev: Pointer to a struct ps3_storage_device
+ * @region_idx: Index of the region to access
* @lpar: HV logical partition address
* @start_sector: First sector to read/write
* @sectors: Number of sectors to read/write
@@ -272,10 +270,11 @@ EXPORT_SYMBOL_GPL(ps3stor_teardown);
* Returns 0 for success, -1 in case of failure to submit the command, or
* an LV1 status value in case of other errors
*/
-u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev, u64 lpar,
+u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev,
+ unsigned int region_idx, u64 lpar,
u64 start_sector, u64 sectors, int write)
{
- unsigned int region_id = dev->regions[dev->region_idx].id;
+ unsigned int region_id = dev->regions[region_idx].id;
const char *op = write ? "write" : "read";
int res;
diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index cd178b9..68db03c 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -39,6 +39,7 @@
#define PS3ROM_MAX_SECTORS (BOUNCE_SIZE >> 9)
+#define PS3ROM_REGION_IDX 0
struct ps3rom_private {
struct ps3_storage_device *dev;
@@ -177,8 +178,9 @@ static int ps3rom_read_request(struct ps3_storage_device *dev,
__func__, __LINE__, sectors, start_sector);
res = lv1_storage_read(dev->sbd.dev_id,
- dev->regions[dev->region_idx].id, start_sector,
- sectors, 0, dev->bounce_lpar, &dev->tag);
+ dev->regions[PS3ROM_REGION_IDX].id,
+ start_sector, sectors, 0,
+ dev->bounce_lpar, &dev->tag);
if (res) {
dev_err(&dev->sbd.core, "%s:%u: read failed %d\n", __func__,
__LINE__, res);
@@ -200,8 +202,9 @@ static int ps3rom_write_request(struct ps3_storage_device *dev,
scsi_sg_copy_to_buffer(cmd, dev->bounce_buf, dev->bounce_size);
res = lv1_storage_write(dev->sbd.dev_id,
- dev->regions[dev->region_idx].id, start_sector,
- sectors, 0, dev->bounce_lpar, &dev->tag);
+ dev->regions[PS3ROM_REGION_IDX].id,
+ start_sector, sectors, 0,
+ dev->bounce_lpar, &dev->tag);
if (res) {
dev_err(&dev->sbd.core, "%s:%u: write failed %d\n", __func__,
__LINE__, res);
--
1.7.5.4
^ permalink raw reply related
* [PATCH 11/15] ps3disk: Provide a gendisk per accessible region
From: Andre Heider @ 2011-08-01 20:03 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
This changes the behavior to name the block devices for lpars
other than OtherOS. Instead of a single disk with an alphanumeric
suffix (/dev/ps3da), disks are now numeric, while each
accessible region gets its own alphanumeric suffix:
/dev/ps3d1a
/dev/ps3d1b
The old behavior for OtherOS is kept:
- only one region will be exposed as block device
- the block device name stays the same
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
drivers/block/ps3disk.c | 118 ++++++++++++++++++++++++++++++----------------
1 files changed, 77 insertions(+), 41 deletions(-)
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index 96e00ff..cba8b45 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -35,18 +35,19 @@
#define PS3DISK_MINORS 16
-#define PS3DISK_NAME "ps3d%c"
+#define PS3DISK_NAME_OTHEROS "ps3d%c"
+#define PS3DISK_NAME "ps3d%c%c"
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;
+ unsigned int devidx;
u64 raw_capacity;
unsigned char model[ATA_ID_PROD_LEN+1];
+ struct gendisk *gendisk[0]; /* Must be last */
};
@@ -126,7 +127,9 @@ 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[priv->region_idx].id;
+ unsigned int region_idx = MINOR(disk_devt(req->rq_disk)) &
+ (PS3DISK_MINORS - 1);
+ unsigned int region_id = dev->regions[region_idx].id;
#ifdef DEBUG
unsigned int n = 0;
@@ -410,6 +413,7 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
struct request_queue *queue;
struct gendisk *gendisk;
unsigned int region_idx;
+ unsigned int otheros = ps3_get_ss_laid() == PS3_SS_LAID_OTHEROS;
if (dev->blk_size < 512) {
dev_err(&dev->sbd.core,
@@ -430,7 +434,9 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
__set_bit(devidx, &ps3disk_mask);
mutex_unlock(&ps3disk_mask_mutex);
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ priv = kzalloc(sizeof(*priv) +
+ dev->num_regions * sizeof(struct gendisk),
+ GFP_KERNEL);
if (!priv) {
error = -ENOMEM;
goto fail;
@@ -450,6 +456,7 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
if (error)
goto fail_free_bounce;
+ priv->devidx = devidx;
ps3disk_identify(dev);
queue = blk_init_queue(ps3disk_request, &priv->lock);
@@ -475,45 +482,60 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
blk_queue_max_segments(queue, -1);
blk_queue_max_segment_size(queue, dev->bounce_size);
- gendisk = alloc_disk(PS3DISK_MINORS);
- if (!gendisk) {
- dev_err(&dev->sbd.core, "%s:%u: alloc_disk failed\n", __func__,
- __LINE__);
- error = -ENOMEM;
- goto fail_cleanup_queue;
- }
+ dev_info(&dev->sbd.core, "%s (%llu MiB)\n",
+ priv->model, priv->raw_capacity >> 11);
- priv->gendisk = gendisk;
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
- /* 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;
+ gendisk = alloc_disk(PS3DISK_MINORS * PS3_STORAGE_MAX_REGIONS);
+ if (!gendisk) {
+ dev_err(&dev->sbd.core, "%s:%u: alloc_disk failed\n",
+ __func__, __LINE__);
+ error = -ENOMEM;
+ goto fail_cleanup_queue;
}
- gendisk->major = ps3disk_major;
- gendisk->first_minor = devidx * PS3DISK_MINORS;
- gendisk->fops = &ps3disk_fops;
- gendisk->queue = queue;
- gendisk->private_data = dev;
- gendisk->driverfs_dev = &dev->sbd.core;
- snprintf(gendisk->disk_name, sizeof(gendisk->disk_name), PS3DISK_NAME,
- devidx+'a');
- priv->blocking_factor = dev->blk_size >> 9;
- set_capacity(gendisk,
- dev->regions[priv->region_idx].size *
- priv->blocking_factor);
-
- dev_info(&dev->sbd.core,
- "%s is a %s (%llu MiB total, %lu MiB for OtherOS)\n",
- gendisk->disk_name, priv->model, priv->raw_capacity >> 11,
- get_capacity(gendisk) >> 11);
-
- add_disk(gendisk);
+ priv->gendisk[region_idx] = gendisk;
+ gendisk->major = ps3disk_major;
+ gendisk->first_minor = devidx * PS3DISK_MINORS + region_idx;
+ gendisk->fops = &ps3disk_fops;
+ gendisk->queue = queue;
+ gendisk->private_data = dev;
+ gendisk->driverfs_dev = &dev->sbd.core;
+
+ if (otheros) {
+ /* keep the old block device name for OtherOS */
+ snprintf(gendisk->disk_name, sizeof(gendisk->disk_name),
+ PS3DISK_NAME_OTHEROS, 'a' + devidx);
+ } else {
+ snprintf(gendisk->disk_name, sizeof(gendisk->disk_name),
+ PS3DISK_NAME, '1' + devidx, 'a' + region_idx);
+ }
+
+ priv->blocking_factor = dev->blk_size >> 9;
+ set_capacity(gendisk, dev->regions[region_idx].size *
+ priv->blocking_factor);
+
+ dev_info(&dev->sbd.core,
+ "%s (%lu MiB region)\n",
+ gendisk->disk_name, get_capacity(gendisk) >> 11);
+
+ add_disk(gendisk);
+
+ /* keep old behavior for OtherOS - only one region */
+ if (otheros)
+ break;
+ }
+
return 0;
fail_cleanup_queue:
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++)
+ if (priv->gendisk[region_idx])
+ del_gendisk(priv->gendisk[region_idx]);
+
blk_cleanup_queue(queue);
fail_teardown:
ps3stor_teardown(dev);
@@ -533,14 +555,28 @@ static int ps3disk_remove(struct ps3_system_bus_device *_dev)
{
struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
struct ps3disk_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ unsigned int region_idx;
mutex_lock(&ps3disk_mask_mutex);
- __clear_bit(MINOR(disk_devt(priv->gendisk)) / PS3DISK_MINORS,
- &ps3disk_mask);
+ __clear_bit(priv->devidx, &ps3disk_mask);
mutex_unlock(&ps3disk_mask_mutex);
- del_gendisk(priv->gendisk);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ del_gendisk(priv->gendisk[region_idx]);
+ }
+
blk_cleanup_queue(priv->queue);
- put_disk(priv->gendisk);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ put_disk(priv->gendisk[region_idx]);
+ }
+
dev_notice(&dev->sbd.core, "Synchronizing disk cache\n");
ps3disk_sync_cache(dev);
ps3stor_teardown(dev);
--
1.7.5.4
^ permalink raw reply related
* [PATCH 12/15] ps3stor_lib: Add support for storage access flags
From: Andre Heider @ 2011-08-01 20:03 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
Users can now set the access flags in the region struct. This is
required for accessing the first region, or selecting an alternative
decryption key for the vflash partitions.
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
arch/powerpc/include/asm/ps3stor.h | 8 +++++++-
arch/powerpc/platforms/ps3/device-init.c | 1 +
drivers/block/ps3disk.c | 5 +++--
drivers/ps3/ps3stor_lib.c | 5 +++--
4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/ps3stor.h b/arch/powerpc/include/asm/ps3stor.h
index 9871c05..f29aa37 100644
--- a/arch/powerpc/include/asm/ps3stor.h
+++ b/arch/powerpc/include/asm/ps3stor.h
@@ -25,12 +25,18 @@
#include <asm/ps3.h>
-#define PS3_STORAGE_MAX_REGIONS (8)
+#define PS3_STORAGE_MAX_REGIONS (8)
+
+#define PS3_STORAGE_FLAG_DEFAULT (0)
+#define PS3_STORAGE_FLAG_SKIP_ACL (1 << 1)
+#define PS3_STORAGE_FLAG_ALT_KEY (1 << 2)
+#define PS3_STORAGE_FLAG_UNENCRYPTED (1 << 5)
struct ps3_storage_region {
unsigned int id;
u64 start;
u64 size;
+ u64 flags;
};
struct ps3_storage_device {
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 830d741..7a3dbf8 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -409,6 +409,7 @@ static int ps3_setup_storage_dev(const struct ps3_repository_device *repo,
p->regions[i].id = id;
p->regions[i].start = start;
p->regions[i].size = size;
+ p->regions[i].flags = PS3_STORAGE_FLAG_DEFAULT;
}
result = ps3_system_bus_device_register(&p->sbd);
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index cba8b45..30dae10 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -130,6 +130,7 @@ static int ps3disk_submit_request_sg(struct ps3_storage_device *dev,
unsigned int region_idx = MINOR(disk_devt(req->rq_disk)) &
(PS3DISK_MINORS - 1);
unsigned int region_id = dev->regions[region_idx].id;
+ u64 flags = dev->regions[region_idx].flags;
#ifdef DEBUG
unsigned int n = 0;
@@ -152,11 +153,11 @@ static int ps3disk_submit_request_sg(struct ps3_storage_device *dev,
ps3disk_scatter_gather(dev, req, 1);
res = lv1_storage_write(dev->sbd.dev_id, region_id,
- start_sector, sectors, 0,
+ start_sector, sectors, flags,
dev->bounce_lpar, &dev->tag);
} else {
res = lv1_storage_read(dev->sbd.dev_id, region_id,
- start_sector, sectors, 0,
+ start_sector, sectors, flags,
dev->bounce_lpar, &dev->tag);
}
if (res) {
diff --git a/drivers/ps3/ps3stor_lib.c b/drivers/ps3/ps3stor_lib.c
index 5bbc023..8bb54ac 100644
--- a/drivers/ps3/ps3stor_lib.c
+++ b/drivers/ps3/ps3stor_lib.c
@@ -275,6 +275,7 @@ u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev,
u64 start_sector, u64 sectors, int write)
{
unsigned int region_id = dev->regions[region_idx].id;
+ u64 flags = dev->regions[region_idx].flags;
const char *op = write ? "write" : "read";
int res;
@@ -283,10 +284,10 @@ u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev,
init_completion(&dev->done);
res = write ? lv1_storage_write(dev->sbd.dev_id, region_id,
- start_sector, sectors, 0, lpar,
+ start_sector, sectors, flags, lpar,
&dev->tag)
: lv1_storage_read(dev->sbd.dev_id, region_id,
- start_sector, sectors, 0, lpar,
+ start_sector, sectors, flags, lpar,
&dev->tag);
if (res) {
dev_dbg(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
--
1.7.5.4
^ permalink raw reply related
* [PATCH 13/15] ps3disk: Use region flags
From: Andre Heider @ 2011-08-01 20:03 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
Provide a set of default region flags and make them overwritable via
a module parameter array.
Set PS3_STORAGE_FLAG_SKIP_ACL for region 0, so it can be accessed
from the GameOS lpar.
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
drivers/block/ps3disk.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index 30dae10..483806e 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -39,6 +39,19 @@
#define PS3DISK_NAME "ps3d%c%c"
+static unsigned int region_flags[] = {
+ PS3_STORAGE_FLAG_SKIP_ACL, /* raw disk */
+ PS3_STORAGE_FLAG_DEFAULT, /* core os */
+ PS3_STORAGE_FLAG_DEFAULT, /* gameos pup */
+ PS3_STORAGE_FLAG_DEFAULT, /* n/a */
+ PS3_STORAGE_FLAG_DEFAULT, /* n/a */
+ PS3_STORAGE_FLAG_DEFAULT, /* n/a */
+ PS3_STORAGE_FLAG_DEFAULT, /* n/a */
+ PS3_STORAGE_FLAG_DEFAULT, /* n/a */
+};
+module_param_array(region_flags, uint, NULL, S_IRUGO);
+MODULE_PARM_DESC(region_flags, "Region flags");
+
struct ps3disk_private {
spinlock_t lock; /* Request queue spinlock */
struct request_queue *queue;
@@ -453,6 +466,9 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
goto fail_free_priv;
}
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++)
+ dev->regions[region_idx].flags = region_flags[region_idx];
+
error = ps3stor_setup(dev, ps3disk_interrupt);
if (error)
goto fail_free_bounce;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 14/15] ps3: Add a vflash driver for lpars other than OtherOS
From: Andre Heider @ 2011-08-01 20:03 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
This driver refuses to work on OtherOS, and hence complements the
ps3flash driver - which only works on OtherOS.
A gendisk for each accessible region is created, and a default set
of region flags is provided - overwritable via a module param array.
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
arch/powerpc/platforms/ps3/Kconfig | 16 +-
drivers/block/Makefile | 1 +
drivers/block/ps3vflash.c | 508 ++++++++++++++++++++++++++++++++++++
3 files changed, 524 insertions(+), 1 deletions(-)
create mode 100644 drivers/block/ps3vflash.c
diff --git a/arch/powerpc/platforms/ps3/Kconfig b/arch/powerpc/platforms/ps3/Kconfig
index 5eb956a..f8d865c 100644
--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -121,13 +121,27 @@ config PS3_FLASH
This support is required to access the PS3 FLASH ROM, which
contains the boot loader and some boot options.
- This driver only supports the deprecated OtherOS LPAR.
+ This driver only supports the deprecated OtherOS LPAR, select
+ PS3_VFLASH below for all other LPARs.
In general, all users will say Y or M.
As this driver needs a fixed buffer of 256 KiB of memory, it can
be disabled on the kernel command line using "ps3flash=off", to
not allocate this fixed buffer.
+config PS3_VFLASH
+ tristate "PS3 VFLASH Storage Driver"
+ depends on PPC_PS3 && BLOCK
+ select PS3_STORAGE
+ help
+ Include support for the PS3 virtual FLASH Storage.
+
+ This support is required to access the PS3 virtual FLASH ROM, which
+ contains the boot loader and some boot options.
+ This driver only supports LPARs other than the deprecated OtherOS,
+ select PS3_FLASH above to add support for FLASH ROM for the
+ OtherOS LPAR.
+
config PS3_VRAM
tristate "PS3 Video RAM Storage Driver"
depends on FB_PS3=y && BLOCK && m
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index 40528ba..8b02899 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_BLK_DEV_SWIM) += swim_mod.o
obj-$(CONFIG_BLK_DEV_FD) += floppy.o
obj-$(CONFIG_AMIGA_FLOPPY) += amiflop.o
obj-$(CONFIG_PS3_DISK) += ps3disk.o
+obj-$(CONFIG_PS3_VFLASH) += ps3vflash.o
obj-$(CONFIG_PS3_VRAM) += ps3vram.o
obj-$(CONFIG_ATARI_FLOPPY) += ataflop.o
obj-$(CONFIG_AMIGA_Z2RAM) += z2ram.o
diff --git a/drivers/block/ps3vflash.c b/drivers/block/ps3vflash.c
new file mode 100644
index 0000000..a59e9f7
--- /dev/null
+++ b/drivers/block/ps3vflash.c
@@ -0,0 +1,508 @@
+/*
+ * PS3 VFLASH Storage Driver
+ *
+ * Copyright (C) 2011 Andre Heider <a.heider@gmail.com>
+ *
+ * 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 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/ata.h>
+#include <linux/blkdev.h>
+#include <linux/slab.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+#include <asm/firmware.h>
+
+
+#define DEVICE_NAME "ps3vflash"
+
+#define BOUNCE_SIZE (64*1024)
+
+#define PS3VFLASH_MAX_DISKS 16
+#define PS3VFLASH_MINORS 16
+
+
+#define PS3VFLASH_NAME "ps3v%c%c"
+
+
+static unsigned int region_flags[] = {
+ PS3_STORAGE_FLAG_SKIP_ACL | PS3_STORAGE_FLAG_ALT_KEY, /* raw vflash */
+ PS3_STORAGE_FLAG_SKIP_ACL, /* lpar 1 */
+ PS3_STORAGE_FLAG_ALT_KEY, /* dev_flash */
+ PS3_STORAGE_FLAG_ALT_KEY, /* dev_flash2 */
+ PS3_STORAGE_FLAG_ALT_KEY, /* dev_flash3 */
+ PS3_STORAGE_FLAG_DEFAULT, /* otheros */
+ PS3_STORAGE_FLAG_SKIP_ACL, /* unknown */
+ PS3_STORAGE_FLAG_DEFAULT, /* n/a */
+};
+module_param_array(region_flags, uint, NULL, S_IRUGO);
+MODULE_PARM_DESC(region_flags, "Region flags");
+
+struct ps3vflash_private {
+ spinlock_t lock; /* Request queue spinlock */
+ struct request_queue *queue;
+ unsigned int blocking_factor;
+ struct request *req;
+ unsigned int devidx;
+ struct gendisk *gendisk[0]; /* Must be last */
+};
+
+
+#define LV1_STORAGE_ATA_FLUSH_CACHE (0x31)
+
+static int ps3vflash_major;
+
+
+static const struct block_device_operations ps3vflash_fops = {
+ .owner = THIS_MODULE,
+};
+
+
+static void ps3vflash_scatter_gather(struct ps3_storage_device *dev,
+ struct request *req, int gather)
+{
+ unsigned int offset = 0;
+ struct req_iterator iter;
+ struct bio_vec *bvec;
+ unsigned int i = 0;
+ size_t size;
+ void *buf;
+
+ rq_for_each_segment(bvec, req, iter) {
+ unsigned long flags;
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: bio %u: %u segs %u sectors from %lu\n",
+ __func__, __LINE__, i, bio_segments(iter.bio),
+ bio_sectors(iter.bio), iter.bio->bi_sector);
+
+ size = bvec->bv_len;
+ buf = bvec_kmap_irq(bvec, &flags);
+ if (gather)
+ memcpy(dev->bounce_buf+offset, buf, size);
+ else
+ memcpy(buf, dev->bounce_buf+offset, size);
+ offset += size;
+ flush_kernel_dcache_page(bvec->bv_page);
+ bvec_kunmap_irq(buf, &flags);
+ i++;
+ }
+}
+
+static int ps3vflash_submit_request_sg(struct ps3_storage_device *dev,
+ struct request *req)
+{
+ struct ps3vflash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ int write = rq_data_dir(req), res;
+ const char *op = write ? "write" : "read";
+ u64 start_sector, sectors;
+ unsigned int region_idx = MINOR(disk_devt(req->rq_disk)) &
+ (PS3VFLASH_MINORS - 1);
+ unsigned int region_id = dev->regions[region_idx].id;
+ u64 flags = dev->regions[region_idx].flags;
+
+#ifdef DEBUG
+ unsigned int n = 0;
+ struct bio_vec *bv;
+ struct req_iterator iter;
+
+ rq_for_each_segment(bv, req, iter)
+ n++;
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: %s req has %u bvecs for %u sectors\n",
+ __func__, __LINE__, op, n, blk_rq_sectors(req));
+#endif
+
+ start_sector = blk_rq_pos(req) * priv->blocking_factor;
+ sectors = blk_rq_sectors(req) * priv->blocking_factor;
+ dev_dbg(&dev->sbd.core, "%s:%u: %s %llu sectors starting at %llu\n",
+ __func__, __LINE__, op, sectors, start_sector);
+
+ if (write) {
+ ps3vflash_scatter_gather(dev, req, 1);
+
+ res = lv1_storage_write(dev->sbd.dev_id, region_id,
+ start_sector, sectors, flags,
+ dev->bounce_lpar, &dev->tag);
+ } else {
+ res = lv1_storage_read(dev->sbd.dev_id, region_id,
+ start_sector, sectors, flags,
+ dev->bounce_lpar, &dev->tag);
+ }
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
+ __LINE__, op, res);
+ __blk_end_request_all(req, -EIO);
+ return 0;
+ }
+
+ priv->req = req;
+ return 1;
+}
+
+static int ps3vflash_submit_flush_request(struct ps3_storage_device *dev,
+ struct request *req)
+{
+ struct ps3vflash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ u64 res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: flush request\n", __func__, __LINE__);
+
+ res = lv1_storage_send_device_command(dev->sbd.dev_id,
+ LV1_STORAGE_ATA_FLUSH_CACHE,
+ 0, 0, 0, 0, &dev->tag);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: sync cache failed 0x%llx\n",
+ __func__, __LINE__, res);
+ __blk_end_request_all(req, -EIO);
+ return 0;
+ }
+
+ priv->req = req;
+ return 1;
+}
+
+static void ps3vflash_do_request(struct ps3_storage_device *dev,
+ struct request_queue *q)
+{
+ struct request *req;
+
+ dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+ while ((req = blk_fetch_request(q))) {
+ if (req->cmd_flags & REQ_FLUSH) {
+ if (ps3vflash_submit_flush_request(dev, req))
+ break;
+ } else if (req->cmd_type == REQ_TYPE_FS) {
+ if (ps3vflash_submit_request_sg(dev, req))
+ break;
+ } else {
+ blk_dump_rq_flags(req, DEVICE_NAME " bad request");
+ __blk_end_request_all(req, -EIO);
+ continue;
+ }
+ }
+}
+
+static void ps3vflash_request(struct request_queue *q)
+{
+ struct ps3_storage_device *dev = q->queuedata;
+ struct ps3vflash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+
+ if (priv->req) {
+ dev_dbg(&dev->sbd.core, "%s:%u busy\n", __func__, __LINE__);
+ return;
+ }
+
+ ps3vflash_do_request(dev, q);
+}
+
+static irqreturn_t ps3vflash_interrupt(int irq, void *data)
+{
+ struct ps3_storage_device *dev = data;
+ struct ps3vflash_private *priv;
+ struct request *req;
+ int res, read, error;
+ u64 tag, status;
+ const char *op;
+
+ res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+
+ if (tag != dev->tag)
+ dev_err(&dev->sbd.core,
+ "%s:%u: tag mismatch, got %llx, expected %llx\n",
+ __func__, __LINE__, tag, dev->tag);
+
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%llx\n",
+ __func__, __LINE__, res, status);
+ return IRQ_HANDLED;
+ }
+
+ priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ req = priv->req;
+ if (!req) {
+ dev_dbg(&dev->sbd.core,
+ "%s:%u non-block layer request completed\n", __func__,
+ __LINE__);
+ dev->lv1_status = status;
+ complete(&dev->done);
+ return IRQ_HANDLED;
+ }
+
+ if (req->cmd_flags & REQ_FLUSH) {
+ read = 0;
+ op = "flush";
+ } else {
+ read = !rq_data_dir(req);
+ op = read ? "read" : "write";
+ }
+ if (status) {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s failed 0x%llx\n", __func__,
+ __LINE__, op, status);
+ error = -EIO;
+ } else {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s completed\n", __func__,
+ __LINE__, op);
+ error = 0;
+ if (read)
+ ps3vflash_scatter_gather(dev, req, 0);
+ }
+
+ spin_lock(&priv->lock);
+ __blk_end_request_all(req, error);
+ priv->req = NULL;
+ ps3vflash_do_request(dev, priv->queue);
+ spin_unlock(&priv->lock);
+
+ return IRQ_HANDLED;
+}
+
+static unsigned long ps3vflash_mask;
+
+static DEFINE_MUTEX(ps3vflash_mask_mutex);
+
+static int __devinit ps3vflash_probe(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct ps3vflash_private *priv;
+ int error;
+ unsigned int devidx;
+ struct request_queue *queue;
+ struct gendisk *gendisk;
+ u64 raw_capacity;
+ unsigned int region_idx;
+
+ if (dev->blk_size < 512) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: cannot handle block size %llu\n", __func__,
+ __LINE__, dev->blk_size);
+ return -EINVAL;
+ }
+
+ BUILD_BUG_ON(PS3VFLASH_MAX_DISKS > BITS_PER_LONG);
+ mutex_lock(&ps3vflash_mask_mutex);
+ devidx = find_first_zero_bit(&ps3vflash_mask, PS3VFLASH_MAX_DISKS);
+ if (devidx >= PS3VFLASH_MAX_DISKS) {
+ dev_err(&dev->sbd.core, "%s:%u: Too many disks\n", __func__,
+ __LINE__);
+ mutex_unlock(&ps3vflash_mask_mutex);
+ return -ENOSPC;
+ }
+ __set_bit(devidx, &ps3vflash_mask);
+ mutex_unlock(&ps3vflash_mask_mutex);
+
+ priv = kzalloc(sizeof(*priv) +
+ dev->num_regions * sizeof(struct gendisk),
+ GFP_KERNEL);
+ if (!priv) {
+ error = -ENOMEM;
+ goto fail;
+ }
+
+ ps3_system_bus_set_drvdata(_dev, priv);
+ spin_lock_init(&priv->lock);
+
+ dev->bounce_size = BOUNCE_SIZE;
+ dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
+ if (!dev->bounce_buf) {
+ error = -ENOMEM;
+ goto fail_free_priv;
+ }
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++)
+ dev->regions[region_idx].flags = region_flags[region_idx];
+
+ error = ps3stor_setup(dev, ps3vflash_interrupt);
+ if (error)
+ goto fail_free_bounce;
+
+ priv->devidx = devidx;
+
+ queue = blk_init_queue(ps3vflash_request, &priv->lock);
+ if (!queue) {
+ dev_err(&dev->sbd.core, "%s:%u: blk_init_queue failed\n",
+ __func__, __LINE__);
+ error = -ENOMEM;
+ goto fail_teardown;
+ }
+
+ priv->queue = queue;
+ queue->queuedata = dev;
+
+ blk_queue_bounce_limit(queue, BLK_BOUNCE_HIGH);
+
+ blk_queue_max_hw_sectors(queue, dev->bounce_size >> 9);
+ blk_queue_segment_boundary(queue, -1UL);
+ blk_queue_dma_alignment(queue, dev->blk_size-1);
+ blk_queue_logical_block_size(queue, dev->blk_size);
+
+ blk_queue_flush(queue, REQ_FLUSH);
+
+ blk_queue_max_segments(queue, -1);
+ blk_queue_max_segment_size(queue, dev->bounce_size);
+
+ if (test_bit(0, &dev->accessible_regions) == 0) {
+ raw_capacity = 0;
+ for (region_idx = 0; region_idx < dev->num_regions;
+ region_idx++)
+ if (test_bit(region_idx, &dev->accessible_regions))
+ raw_capacity += dev->regions[0].size;
+ } else {
+ raw_capacity = dev->regions[0].size;
+ }
+
+ dev_info(&dev->sbd.core, "%llu MiB\n", raw_capacity >> 11);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ gendisk = alloc_disk(PS3VFLASH_MINORS *
+ PS3_STORAGE_MAX_REGIONS);
+ if (!gendisk) {
+ dev_err(&dev->sbd.core, "%s:%u: alloc_disk failed\n",
+ __func__, __LINE__);
+ error = -ENOMEM;
+ goto fail_cleanup_queue;
+ }
+
+ priv->gendisk[region_idx] = gendisk;
+ gendisk->major = ps3vflash_major;
+ gendisk->first_minor = devidx * PS3VFLASH_MINORS + region_idx;
+ gendisk->fops = &ps3vflash_fops;
+ gendisk->queue = queue;
+ gendisk->private_data = dev;
+ gendisk->driverfs_dev = &dev->sbd.core;
+
+ snprintf(gendisk->disk_name, sizeof(gendisk->disk_name),
+ PS3VFLASH_NAME, '1' + devidx, 'a' + region_idx);
+
+ priv->blocking_factor = dev->blk_size >> 9;
+ set_capacity(gendisk, dev->regions[region_idx].size *
+ priv->blocking_factor);
+
+ dev_info(&dev->sbd.core,
+ "%s (%lu MiB region)\n",
+ gendisk->disk_name, get_capacity(gendisk) >> 11);
+
+ add_disk(gendisk);
+ }
+
+ return 0;
+
+fail_cleanup_queue:
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++)
+ if (priv->gendisk[region_idx])
+ del_gendisk(priv->gendisk[region_idx]);
+
+ blk_cleanup_queue(queue);
+fail_teardown:
+ ps3stor_teardown(dev);
+fail_free_bounce:
+ kfree(dev->bounce_buf);
+fail_free_priv:
+ kfree(priv);
+ ps3_system_bus_set_drvdata(_dev, NULL);
+fail:
+ mutex_lock(&ps3vflash_mask_mutex);
+ __clear_bit(devidx, &ps3vflash_mask);
+ mutex_unlock(&ps3vflash_mask_mutex);
+ return error;
+}
+
+static int ps3vflash_remove(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct ps3vflash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ unsigned int region_idx;
+
+ mutex_lock(&ps3vflash_mask_mutex);
+ __clear_bit(priv->devidx, &ps3vflash_mask);
+ mutex_unlock(&ps3vflash_mask_mutex);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ del_gendisk(priv->gendisk[region_idx]);
+ }
+
+ blk_cleanup_queue(priv->queue);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ put_disk(priv->gendisk[region_idx]);
+ }
+
+ ps3stor_teardown(dev);
+ kfree(dev->bounce_buf);
+ kfree(priv);
+ ps3_system_bus_set_drvdata(_dev, NULL);
+ return 0;
+}
+
+static struct ps3_system_bus_driver ps3vflash = {
+ .match_id = PS3_MATCH_ID_STOR_FLASH,
+ .core.name = DEVICE_NAME,
+ .core.owner = THIS_MODULE,
+ .probe = ps3vflash_probe,
+ .remove = ps3vflash_remove,
+ .shutdown = ps3vflash_remove,
+};
+
+
+static int __init ps3vflash_init(void)
+{
+ int error;
+
+ if (!firmware_has_feature(FW_FEATURE_PS3_LV1))
+ return -ENODEV;
+
+ if (ps3_get_ss_laid() == PS3_SS_LAID_OTHEROS)
+ return -ENODEV;
+
+ error = register_blkdev(0, DEVICE_NAME);
+ if (error <= 0) {
+ printk(KERN_ERR "%s:%u: register_blkdev failed %d\n", __func__,
+ __LINE__, error);
+ return error;
+ }
+ ps3vflash_major = error;
+
+ pr_info("%s:%u: registered block device major %d\n", __func__,
+ __LINE__, ps3vflash_major);
+
+ error = ps3_system_bus_driver_register(&ps3vflash);
+ if (error)
+ unregister_blkdev(ps3vflash_major, DEVICE_NAME);
+
+ return error;
+}
+
+static void __exit ps3vflash_exit(void)
+{
+ ps3_system_bus_driver_unregister(&ps3vflash);
+ unregister_blkdev(ps3vflash_major, DEVICE_NAME);
+}
+
+module_init(ps3vflash_init);
+module_exit(ps3vflash_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 VFLASH Storage Driver");
+MODULE_AUTHOR("Andre Heider");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_FLASH);
--
1.7.5.4
^ permalink raw reply related
* [PATCH 15/15] ps3: Add a NOR FLASH driver for PS3s without NAND
From: Andre Heider @ 2011-08-01 20:03 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
A gendisk for each accessible region is created, and a default set
of region flags is provided - overwritable via a module param array.
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
arch/powerpc/include/asm/ps3.h | 2 +
arch/powerpc/platforms/ps3/Kconfig | 15 +
arch/powerpc/platforms/ps3/device-init.c | 7 +
arch/powerpc/platforms/ps3/platform.h | 1 +
arch/powerpc/platforms/ps3/system-bus.c | 2 +
drivers/block/Makefile | 1 +
drivers/block/ps3nflash.c | 473 ++++++++++++++++++++++++++++++
7 files changed, 501 insertions(+), 0 deletions(-)
create mode 100644 drivers/block/ps3nflash.c
diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
index 136354a..fe84231 100644
--- a/arch/powerpc/include/asm/ps3.h
+++ b/arch/powerpc/include/asm/ps3.h
@@ -333,6 +333,7 @@ enum ps3_match_id {
PS3_MATCH_ID_SOUND = 9,
PS3_MATCH_ID_GPU = 10,
PS3_MATCH_ID_LPM = 11,
+ PS3_MATCH_ID_STOR_NFLASH = 12,
};
enum ps3_match_sub_id {
@@ -352,6 +353,7 @@ enum ps3_match_sub_id {
#define PS3_MODULE_ALIAS_GPU_FB "ps3:10:1"
#define PS3_MODULE_ALIAS_GPU_RAMDISK "ps3:10:2"
#define PS3_MODULE_ALIAS_LPM "ps3:11:0"
+#define PS3_MODULE_ALIAS_STOR_NFLASH "ps3:12:0"
enum ps3_system_bus_device_type {
PS3_DEVICE_TYPE_IOC0 = 1,
diff --git a/arch/powerpc/platforms/ps3/Kconfig b/arch/powerpc/platforms/ps3/Kconfig
index f8d865c..9e1f841 100644
--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -142,6 +142,21 @@ config PS3_VFLASH
select PS3_FLASH above to add support for FLASH ROM for the
OtherOS LPAR.
+config PS3_NFLASH
+ tristate "PS3 NFLASH Storage Driver"
+ depends on PPC_PS3 && BLOCK && EXPERT
+ select PS3_STORAGE
+ help
+ Include support for the PS3 NOR FLASH Storage.
+
+ This support is required to access the PS3 NOR flash, which can be
+ found on SKU models starting around October 2007.
+
+ The NOR flash contains essential boot loaders. Damaging this area
+ will render the console unbootable.
+
+ If unsure, say N.
+
config PS3_VRAM
tristate "PS3 Video RAM Storage Driver"
depends on FB_PS3=y && BLOCK && m
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 7a3dbf8..c2fbe33 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -601,6 +601,13 @@ static int ps3_setup_dynamic_device(const struct ps3_repository_device *repo)
__func__, __LINE__);
break;
+ case PS3_DEV_TYPE_STOR_NFLASH:
+ result = ps3_setup_storage_dev(repo, PS3_MATCH_ID_STOR_NFLASH);
+ if (result)
+ pr_debug("%s:%u ps3_setup_storage_dev failed\n",
+ __func__, __LINE__);
+ break;
+
default:
result = 0;
pr_debug("%s:%u: unsupported dev_type %u\n", __func__, __LINE__,
diff --git a/arch/powerpc/platforms/ps3/platform.h b/arch/powerpc/platforms/ps3/platform.h
index 1ba15b8..94b5048 100644
--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -87,6 +87,7 @@ enum ps3_dev_type {
PS3_DEV_TYPE_STOR_ROM = TYPE_ROM, /* 5 */
PS3_DEV_TYPE_SB_GPIO = 6,
PS3_DEV_TYPE_STOR_FLASH = TYPE_RBC, /* 14 */
+ PS3_DEV_TYPE_STOR_NFLASH = 254,
};
int ps3_repository_read_bus_str(unsigned int bus_index, const char *bus_str,
diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c
index 23083c3..810bbc5 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -174,6 +174,7 @@ int ps3_open_hv_device(struct ps3_system_bus_device *dev)
case PS3_MATCH_ID_STOR_DISK:
case PS3_MATCH_ID_STOR_ROM:
case PS3_MATCH_ID_STOR_FLASH:
+ case PS3_MATCH_ID_STOR_NFLASH:
return ps3_open_hv_device_sb(dev);
case PS3_MATCH_ID_SOUND:
@@ -212,6 +213,7 @@ int ps3_close_hv_device(struct ps3_system_bus_device *dev)
case PS3_MATCH_ID_STOR_DISK:
case PS3_MATCH_ID_STOR_ROM:
case PS3_MATCH_ID_STOR_FLASH:
+ case PS3_MATCH_ID_STOR_NFLASH:
return ps3_close_hv_device_sb(dev);
case PS3_MATCH_ID_SOUND:
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index 8b02899..ab2bb06 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_BLK_DEV_FD) += floppy.o
obj-$(CONFIG_AMIGA_FLOPPY) += amiflop.o
obj-$(CONFIG_PS3_DISK) += ps3disk.o
obj-$(CONFIG_PS3_VFLASH) += ps3vflash.o
+obj-$(CONFIG_PS3_NFLASH) += ps3nflash.o
obj-$(CONFIG_PS3_VRAM) += ps3vram.o
obj-$(CONFIG_ATARI_FLOPPY) += ataflop.o
obj-$(CONFIG_AMIGA_Z2RAM) += z2ram.o
diff --git a/drivers/block/ps3nflash.c b/drivers/block/ps3nflash.c
new file mode 100644
index 0000000..5a7127e
--- /dev/null
+++ b/drivers/block/ps3nflash.c
@@ -0,0 +1,473 @@
+/*
+ * PS3 NOR FLASH Storage Driver
+ *
+ * Copyright (C) 2011 Andre Heider <a.heider@gmail.com>
+ *
+ * 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 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/ata.h>
+#include <linux/blkdev.h>
+#include <linux/slab.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+#include <asm/firmware.h>
+
+
+#define DEVICE_NAME "ps3nflash"
+
+#define BOUNCE_SIZE (64*1024)
+
+#define PS3NFLASH_MAX_DISKS 16
+#define PS3NFLASH_MINORS 16
+
+
+#define PS3NFLASH_NAME "ps3n%c%c"
+
+
+static unsigned int region_flags[] = {
+ PS3_STORAGE_FLAG_SKIP_ACL,
+ PS3_STORAGE_FLAG_SKIP_ACL,
+ PS3_STORAGE_FLAG_SKIP_ACL,
+ PS3_STORAGE_FLAG_SKIP_ACL,
+ PS3_STORAGE_FLAG_DEFAULT,
+ PS3_STORAGE_FLAG_DEFAULT,
+ PS3_STORAGE_FLAG_DEFAULT,
+ PS3_STORAGE_FLAG_DEFAULT,
+};
+module_param_array(region_flags, uint, NULL, S_IRUGO);
+MODULE_PARM_DESC(region_flags, "Region flags");
+
+struct ps3nflash_private {
+ spinlock_t lock; /* Request queue spinlock */
+ struct request_queue *queue;
+ unsigned int blocking_factor;
+ struct request *req;
+ unsigned int devidx;
+ struct gendisk *gendisk[0]; /* Must be last */
+};
+
+static int ps3nflash_major;
+
+
+static const struct block_device_operations ps3nflash_fops = {
+ .owner = THIS_MODULE,
+};
+
+
+static void ps3nflash_scatter_gather(struct ps3_storage_device *dev,
+ struct request *req, int gather)
+{
+ unsigned int offset = 0;
+ struct req_iterator iter;
+ struct bio_vec *bvec;
+ unsigned int i = 0;
+ size_t size;
+ void *buf;
+
+ rq_for_each_segment(bvec, req, iter) {
+ unsigned long flags;
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: bio %u: %u segs %u sectors from %lu\n",
+ __func__, __LINE__, i, bio_segments(iter.bio),
+ bio_sectors(iter.bio), iter.bio->bi_sector);
+
+ size = bvec->bv_len;
+ buf = bvec_kmap_irq(bvec, &flags);
+ if (gather)
+ memcpy(dev->bounce_buf+offset, buf, size);
+ else
+ memcpy(buf, dev->bounce_buf+offset, size);
+ offset += size;
+ flush_kernel_dcache_page(bvec->bv_page);
+ bvec_kunmap_irq(buf, &flags);
+ i++;
+ }
+}
+
+static int ps3nflash_submit_request_sg(struct ps3_storage_device *dev,
+ struct request *req)
+{
+ struct ps3nflash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ int write = rq_data_dir(req), res;
+ const char *op = write ? "write" : "read";
+ u64 start_sector, sectors;
+ unsigned int region_idx = MINOR(disk_devt(req->rq_disk)) &
+ (PS3NFLASH_MINORS - 1);
+ unsigned int region_id = dev->regions[region_idx].id;
+ u64 flags = dev->regions[region_idx].flags;
+
+#ifdef DEBUG
+ unsigned int n = 0;
+ struct bio_vec *bv;
+ struct req_iterator iter;
+
+ rq_for_each_segment(bv, req, iter)
+ n++;
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: %s req has %u bvecs for %u sectors\n",
+ __func__, __LINE__, op, n, blk_rq_sectors(req));
+#endif
+
+ start_sector = blk_rq_pos(req) * priv->blocking_factor;
+ sectors = blk_rq_sectors(req) * priv->blocking_factor;
+ dev_dbg(&dev->sbd.core, "%s:%u: %s %llu sectors starting at %llu\n",
+ __func__, __LINE__, op, sectors, start_sector);
+
+ if (write) {
+ ps3nflash_scatter_gather(dev, req, 1);
+
+ res = lv1_storage_write(dev->sbd.dev_id, region_id,
+ start_sector, sectors, flags,
+ dev->bounce_lpar, &dev->tag);
+ } else {
+ res = lv1_storage_read(dev->sbd.dev_id, region_id,
+ start_sector, sectors, flags,
+ dev->bounce_lpar, &dev->tag);
+ }
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
+ __LINE__, op, res);
+ __blk_end_request_all(req, -EIO);
+ return 0;
+ }
+
+ priv->req = req;
+ return 1;
+}
+
+static void ps3nflash_do_request(struct ps3_storage_device *dev,
+ struct request_queue *q)
+{
+ struct request *req;
+
+ dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+ while ((req = blk_fetch_request(q))) {
+ if (req->cmd_type == REQ_TYPE_FS) {
+ if (ps3nflash_submit_request_sg(dev, req))
+ break;
+ } else {
+ blk_dump_rq_flags(req, DEVICE_NAME " bad request");
+ __blk_end_request_all(req, -EIO);
+ continue;
+ }
+ }
+}
+
+static void ps3nflash_request(struct request_queue *q)
+{
+ struct ps3_storage_device *dev = q->queuedata;
+ struct ps3nflash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+
+ if (priv->req) {
+ dev_dbg(&dev->sbd.core, "%s:%u busy\n", __func__, __LINE__);
+ return;
+ }
+
+ ps3nflash_do_request(dev, q);
+}
+
+static irqreturn_t ps3nflash_interrupt(int irq, void *data)
+{
+ struct ps3_storage_device *dev = data;
+ struct ps3nflash_private *priv;
+ struct request *req;
+ int res, read, error;
+ u64 tag, status;
+ const char *op;
+
+ res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+
+ if (tag != dev->tag)
+ dev_err(&dev->sbd.core,
+ "%s:%u: tag mismatch, got %llx, expected %llx\n",
+ __func__, __LINE__, tag, dev->tag);
+
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%llx\n",
+ __func__, __LINE__, res, status);
+ return IRQ_HANDLED;
+ }
+
+ priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ req = priv->req;
+ if (!req) {
+ dev_dbg(&dev->sbd.core,
+ "%s:%u non-block layer request completed\n", __func__,
+ __LINE__);
+ dev->lv1_status = status;
+ complete(&dev->done);
+ return IRQ_HANDLED;
+ }
+
+ read = !rq_data_dir(req);
+ op = read ? "read" : "write";
+
+ if (status) {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s failed 0x%llx\n", __func__,
+ __LINE__, op, status);
+ error = -EIO;
+ } else {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s completed\n", __func__,
+ __LINE__, op);
+ error = 0;
+ if (read)
+ ps3nflash_scatter_gather(dev, req, 0);
+ }
+
+ spin_lock(&priv->lock);
+ __blk_end_request_all(req, error);
+ priv->req = NULL;
+ ps3nflash_do_request(dev, priv->queue);
+ spin_unlock(&priv->lock);
+
+ return IRQ_HANDLED;
+}
+
+static unsigned long ps3nflash_mask;
+
+static DEFINE_MUTEX(ps3nflash_mask_mutex);
+
+static int __devinit ps3nflash_probe(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct ps3nflash_private *priv;
+ int error;
+ unsigned int devidx;
+ struct request_queue *queue;
+ struct gendisk *gendisk;
+ u64 raw_capacity;
+ unsigned int region_idx;
+
+ if (dev->blk_size < 512) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: cannot handle block size %llu\n", __func__,
+ __LINE__, dev->blk_size);
+ return -EINVAL;
+ }
+
+ BUILD_BUG_ON(PS3NFLASH_MAX_DISKS > BITS_PER_LONG);
+ mutex_lock(&ps3nflash_mask_mutex);
+ devidx = find_first_zero_bit(&ps3nflash_mask, PS3NFLASH_MAX_DISKS);
+ if (devidx >= PS3NFLASH_MAX_DISKS) {
+ dev_err(&dev->sbd.core, "%s:%u: Too many disks\n", __func__,
+ __LINE__);
+ mutex_unlock(&ps3nflash_mask_mutex);
+ return -ENOSPC;
+ }
+ __set_bit(devidx, &ps3nflash_mask);
+ mutex_unlock(&ps3nflash_mask_mutex);
+
+ priv = kzalloc(sizeof(*priv) +
+ dev->num_regions * sizeof(struct gendisk),
+ GFP_KERNEL);
+ if (!priv) {
+ error = -ENOMEM;
+ goto fail;
+ }
+
+ ps3_system_bus_set_drvdata(_dev, priv);
+ spin_lock_init(&priv->lock);
+
+ dev->bounce_size = BOUNCE_SIZE;
+ dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
+ if (!dev->bounce_buf) {
+ error = -ENOMEM;
+ goto fail_free_priv;
+ }
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++)
+ dev->regions[region_idx].flags = region_flags[region_idx];
+
+ error = ps3stor_setup(dev, ps3nflash_interrupt);
+ if (error)
+ goto fail_free_bounce;
+
+ priv->devidx = devidx;
+
+ queue = blk_init_queue(ps3nflash_request, &priv->lock);
+ if (!queue) {
+ dev_err(&dev->sbd.core, "%s:%u: blk_init_queue failed\n",
+ __func__, __LINE__);
+ error = -ENOMEM;
+ goto fail_teardown;
+ }
+
+ priv->queue = queue;
+ queue->queuedata = dev;
+
+ blk_queue_bounce_limit(queue, BLK_BOUNCE_HIGH);
+
+ blk_queue_max_hw_sectors(queue, dev->bounce_size >> 9);
+ blk_queue_segment_boundary(queue, -1UL);
+ blk_queue_dma_alignment(queue, dev->blk_size-1);
+ blk_queue_logical_block_size(queue, dev->blk_size);
+
+ blk_queue_flush(queue, REQ_FLUSH);
+
+ blk_queue_max_segments(queue, -1);
+ blk_queue_max_segment_size(queue, dev->bounce_size);
+
+ if (test_bit(0, &dev->accessible_regions) == 0) {
+ raw_capacity = 0;
+ for (region_idx = 0; region_idx < dev->num_regions;
+ region_idx++)
+ if (test_bit(region_idx, &dev->accessible_regions))
+ raw_capacity += dev->regions[0].size;
+ } else {
+ raw_capacity = dev->regions[0].size;
+ }
+
+ dev_info(&dev->sbd.core, "%llu MiB\n", raw_capacity >> 11);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ gendisk = alloc_disk(PS3NFLASH_MINORS *
+ PS3_STORAGE_MAX_REGIONS);
+ if (!gendisk) {
+ dev_err(&dev->sbd.core, "%s:%u: alloc_disk failed\n",
+ __func__, __LINE__);
+ error = -ENOMEM;
+ goto fail_cleanup_queue;
+ }
+
+ priv->gendisk[region_idx] = gendisk;
+ gendisk->major = ps3nflash_major;
+ gendisk->first_minor = devidx * PS3NFLASH_MINORS + region_idx;
+ gendisk->fops = &ps3nflash_fops;
+ gendisk->queue = queue;
+ gendisk->private_data = dev;
+ gendisk->driverfs_dev = &dev->sbd.core;
+
+ snprintf(gendisk->disk_name, sizeof(gendisk->disk_name),
+ PS3NFLASH_NAME, '1' + devidx, 'a' + region_idx);
+
+ priv->blocking_factor = dev->blk_size >> 9;
+ set_capacity(gendisk, dev->regions[region_idx].size *
+ priv->blocking_factor);
+
+ dev_info(&dev->sbd.core,
+ "%s (%lu MiB region)\n",
+ gendisk->disk_name, get_capacity(gendisk) >> 11);
+
+ add_disk(gendisk);
+ }
+
+ return 0;
+
+fail_cleanup_queue:
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++)
+ if (priv->gendisk[region_idx])
+ del_gendisk(priv->gendisk[region_idx]);
+
+ blk_cleanup_queue(queue);
+fail_teardown:
+ ps3stor_teardown(dev);
+fail_free_bounce:
+ kfree(dev->bounce_buf);
+fail_free_priv:
+ kfree(priv);
+ ps3_system_bus_set_drvdata(_dev, NULL);
+fail:
+ mutex_lock(&ps3nflash_mask_mutex);
+ __clear_bit(devidx, &ps3nflash_mask);
+ mutex_unlock(&ps3nflash_mask_mutex);
+ return error;
+}
+
+static int ps3nflash_remove(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct ps3nflash_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
+ unsigned int region_idx;
+
+ mutex_lock(&ps3nflash_mask_mutex);
+ __clear_bit(priv->devidx, &ps3nflash_mask);
+ mutex_unlock(&ps3nflash_mask_mutex);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ del_gendisk(priv->gendisk[region_idx]);
+ }
+
+ blk_cleanup_queue(priv->queue);
+
+ for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
+ if (test_bit(region_idx, &dev->accessible_regions) == 0)
+ continue;
+
+ put_disk(priv->gendisk[region_idx]);
+ }
+
+ ps3stor_teardown(dev);
+ kfree(dev->bounce_buf);
+ kfree(priv);
+ ps3_system_bus_set_drvdata(_dev, NULL);
+ return 0;
+}
+
+static struct ps3_system_bus_driver ps3nflash = {
+ .match_id = PS3_MATCH_ID_STOR_NFLASH,
+ .core.name = DEVICE_NAME,
+ .core.owner = THIS_MODULE,
+ .probe = ps3nflash_probe,
+ .remove = ps3nflash_remove,
+ .shutdown = ps3nflash_remove,
+};
+
+
+static int __init ps3nflash_init(void)
+{
+ int error;
+
+ if (!firmware_has_feature(FW_FEATURE_PS3_LV1))
+ return -ENODEV;
+
+ error = register_blkdev(0, DEVICE_NAME);
+ if (error <= 0) {
+ printk(KERN_ERR "%s:%u: register_blkdev failed %d\n", __func__,
+ __LINE__, error);
+ return error;
+ }
+ ps3nflash_major = error;
+
+ pr_info("%s:%u: registered block device major %d\n", __func__,
+ __LINE__, ps3nflash_major);
+
+ error = ps3_system_bus_driver_register(&ps3nflash);
+ if (error)
+ unregister_blkdev(ps3nflash_major, DEVICE_NAME);
+
+ return error;
+}
+
+static void __exit ps3nflash_exit(void)
+{
+ ps3_system_bus_driver_unregister(&ps3nflash);
+ unregister_blkdev(ps3nflash_major, DEVICE_NAME);
+}
+
+module_init(ps3nflash_init);
+module_exit(ps3nflash_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 NOR FLASH Storage Driver");
+MODULE_AUTHOR("Andre Heider");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_NFLASH);
--
1.7.5.4
^ permalink raw reply related
* Re: kvm PCI assignment & VFIO ramblings
From: Alex Williamson @ 2011-08-01 20:27 UTC (permalink / raw)
To: Avi Kivity
Cc: Alexey Kardashevskiy, kvm, Paul Mackerras, David Gibson,
Anthony Liguori, linux-pci@vger.kernel.org, linuxppc-dev
In-Reply-To: <4E356221.6010302@redhat.com>
On Sun, 2011-07-31 at 17:09 +0300, Avi Kivity wrote:
> On 07/30/2011 02:58 AM, Benjamin Herrenschmidt wrote:
> > Due to our paravirt nature, we don't need to masquerade the MSI-X table
> > for example. At all. If the guest configures crap into it, too bad, it
> > can only shoot itself in the foot since the host bridge enforce
> > validation anyways as I explained earlier. Because it's all paravirt, we
> > don't need to "translate" the interrupt vectors& addresses, the guest
> > will call hyercalls to configure things anyways.
>
> So, you have interrupt redirection? That is, MSI-x table values encode
> the vcpu, not pcpu?
>
> Alex, with interrupt redirection, we can skip this as well? Perhaps
> only if the guest enables interrupt redirection?
It's not clear to me how we could skip it. With VT-d, we'd have to
implement an emulated interrupt remapper and hope that the guest picks
unused indexes in the host interrupt remapping table before it could do
anything useful with direct access to the MSI-X table. Maybe AMD IOMMU
makes this easier? Thanks,
Alex
^ permalink raw reply
* Re: [Cbe-oss-dev] [PATCH 06/15] ps3flash: Fix region align checks
From: Geert Uytterhoeven @ 2011-08-01 20:29 UTC (permalink / raw)
To: Andre Heider; +Cc: Geoff Levand, cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-7-git-send-email-a.heider@gmail.com>
On Mon, Aug 1, 2011 at 22:02, Andre Heider <a.heider@gmail.com> wrote:
> The region fields used by the align checks are set in
> ps3stor_setup(), so move those after that call.
Are you sure?
Aren't they set in
arch/powerpc/platforms/ps3/device-init.c:ps3_setup_storage_dev()?
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> =C2=A0drivers/char/ps3flash.c | =C2=A0 30 +++++++++++++++---------------
> =C2=A01 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
> index 85c004a..69c734a 100644
> --- a/drivers/char/ps3flash.c
> +++ b/drivers/char/ps3flash.c
> @@ -360,21 +360,6 @@ static int __devinit ps3flash_probe(struct ps3_syste=
m_bus_device *_dev)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0int error;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned long tmp;
>
> - =C2=A0 =C2=A0 =C2=A0 tmp =3D dev->regions[dev->region_idx].start*dev->b=
lk_size;
> - =C2=A0 =C2=A0 =C2=A0 if (tmp % FLASH_BLOCK_SIZE) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_err(&dev->sbd.core=
,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 "%s:%u region start %lu is not aligned\n", __func__,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 __LINE__, tmp);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EINVAL;
> - =C2=A0 =C2=A0 =C2=A0 }
> - =C2=A0 =C2=A0 =C2=A0 tmp =3D dev->regions[dev->region_idx].size*dev->bl=
k_size;
> - =C2=A0 =C2=A0 =C2=A0 if (tmp % FLASH_BLOCK_SIZE) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_err(&dev->sbd.core=
,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 "%s:%u region size %lu is not aligned\n", __func__,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 __LINE__, tmp);
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EINVAL;
> - =C2=A0 =C2=A0 =C2=A0 }
> -
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* use static buffer, kmalloc cannot allocate =
256 KiB */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!ps3flash_bounce_buffer.address)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -ENODEV;
> @@ -405,6 +390,21 @@ static int __devinit ps3flash_probe(struct ps3_syste=
m_bus_device *_dev)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (error)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto fail_free_pri=
v;
>
> + =C2=A0 =C2=A0 =C2=A0 tmp =3D dev->regions[dev->region_idx].start*dev->b=
lk_size;
> + =C2=A0 =C2=A0 =C2=A0 if (tmp % FLASH_BLOCK_SIZE) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_err(&dev->sbd.core=
,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 "%s:%u region start %lu is not aligned\n", __func__,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 __LINE__, tmp);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EINVAL;
> + =C2=A0 =C2=A0 =C2=A0 }
> + =C2=A0 =C2=A0 =C2=A0 tmp =3D dev->regions[dev->region_idx].size*dev->bl=
k_size;
> + =C2=A0 =C2=A0 =C2=A0 if (tmp % FLASH_BLOCK_SIZE) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_err(&dev->sbd.core=
,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 "%s:%u region size %lu is not aligned\n", __func__,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 __LINE__, tmp);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EINVAL;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> =C2=A0 =C2=A0 =C2=A0 =C2=A0ps3flash_misc.parent =3D &dev->sbd.core;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D misc_register(&ps3flash_misc);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (error) {
Gr{oetje,eeting}s,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds
^ permalink raw reply
* Re: [Cbe-oss-dev] [PATCH 09/15] ps3: Limit the number of regions per storage device
From: Geert Uytterhoeven @ 2011-08-01 20:30 UTC (permalink / raw)
To: Andre Heider; +Cc: Geoff Levand, cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-10-git-send-email-a.heider@gmail.com>
On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
> There can be only 8 regions, add a sanity check
Why can there be only 8 regions?
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> =C2=A0arch/powerpc/include/asm/ps3stor.h =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =
=C2=A01 +
> =C2=A0arch/powerpc/platforms/ps3/device-init.c | =C2=A0 =C2=A08 ++++++++
> =C2=A02 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3stor.h b/arch/powerpc/include/as=
m/ps3stor.h
> index 6fcaf71..d51e53c 100644
> --- a/arch/powerpc/include/asm/ps3stor.h
> +++ b/arch/powerpc/include/asm/ps3stor.h
> @@ -25,6 +25,7 @@
>
> =C2=A0#include <asm/ps3.h>
>
> +#define PS3_STORAGE_MAX_REGIONS =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0(8)
>
> =C2=A0struct ps3_storage_region {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int id;
> diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/plat=
forms/ps3/device-init.c
> index 6c4b583..830d741 100644
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -349,6 +349,14 @@ static int ps3_setup_storage_dev(const struct ps3_re=
pository_device *repo,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -ENODEV;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> + =C2=A0 =C2=A0 =C2=A0 if (num_regions > PS3_STORAGE_MAX_REGIONS) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_warning("%s:%u: dev=
ice %u:%u reported %u regions, "
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0"limiting to %u\n", __func__, __LINE__,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0num_regions, repo->bus_index, repo->dev_index,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0PS3_STORAGE_MAX_REGIONS);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 num_regions =3D PS3_ST=
ORAGE_MAX_REGIONS;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> =C2=A0 =C2=A0 =C2=A0 =C2=A0pr_debug("%s:%u: (%u:%u:%u): port %llu blk_siz=
e %llu num_blocks %llu "
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "num_regions %u\n=
", __func__, __LINE__, repo->bus_index,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 repo->dev_index, =
repo->dev_type, port, blk_size, num_blocks,
Gr{oetje,eeting}s,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 10/15] ps3stor_lib: Add support for multiple regions
From: Geert Uytterhoeven @ 2011-08-01 20:35 UTC (permalink / raw)
To: Andre Heider; +Cc: Geoff Levand, cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-11-git-send-email-a.heider@gmail.com>
On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@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@gmail.com>
> ---
> =C2=A0arch/powerpc/include/asm/ps3stor.h | =C2=A0 =C2=A04 ++--
> =C2=A0drivers/block/ps3disk.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =
=C2=A0 15 +++++++++++++--
> =C2=A0drivers/char/ps3flash.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =
=C2=A0 23 +++++++++++++++++------
> =C2=A0drivers/ps3/ps3stor_lib.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=
=A0 25 ++++++++++++-------------
> =C2=A0drivers/scsi/ps3rom.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0| =C2=A0 11 +++++++----
> =C2=A05 files changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3stor.h b/arch/powerpc/include/as=
m/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 {
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int num_regions;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned long accessible_regions;
> - =C2=A0 =C2=A0 =C2=A0 unsigned int region_idx; =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* first accessible region */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct ps3_storage_region regions[0]; =C2=A0 /=
* Must be last */
> =C2=A0};
>
> @@ -63,7 +62,8 @@ static inline struct ps3_storage_device *to_ps3_storage=
_device(struct device *de
> =C2=A0extern int ps3stor_setup(struct ps3_storage_device *dev,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 irq_handler_t handler);
> =C2=A0extern void ps3stor_teardown(struct ps3_storage_device *dev);
> -extern u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev, u6=
4 lpar,
> +extern u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 unsigned int region=
_idx, u64 lpar,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0u64 start_sector=
, u64 sectors,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int write);
> =C2=A0extern 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 {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0spinlock_t lock; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0/* Request queue spinlock */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct request_queue *queue;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct gendisk *gendisk;
> + =C2=A0 =C2=A0 =C2=A0 unsigned int region_idx; =C2=A0 =C2=A0 =C2=A0 =C2=
=A0/* first accessible region */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int blocking_factor;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct request *req;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0u64 raw_capacity;
> @@ -125,7 +126,7 @@ static int ps3disk_submit_request_sg(struct ps3_stora=
ge_device *dev,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0int write =3D rq_data_dir(req), res;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0const char *op =3D write ? "write" : "read";
> =C2=A0 =C2=A0 =C2=A0 =C2=A0u64 start_sector, sectors;
> - =C2=A0 =C2=A0 =C2=A0 unsigned int region_id =3D dev->regions[dev->regio=
n_idx].id;
> + =C2=A0 =C2=A0 =C2=A0 unsigned int region_id =3D dev->regions[priv->regi=
on_idx].id;
>
> =C2=A0#ifdef DEBUG
> =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int n =3D 0;
> @@ -408,6 +409,7 @@ static int __devinit ps3disk_probe(struct ps3_system_=
bus_device *_dev)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int devidx;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct request_queue *queue;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0struct gendisk *gendisk;
> + =C2=A0 =C2=A0 =C2=A0 unsigned int region_idx;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (dev->blk_size < 512) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_err(&dev->sbd.=
core,
> @@ -482,6 +484,14 @@ static int __devinit ps3disk_probe(struct ps3_system=
_bus_device *_dev)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0priv->gendisk =3D gendisk;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* find first accessible region */
> + =C2=A0 =C2=A0 =C2=A0 for (region_idx =3D 0; region_idx < dev->num_regio=
ns; region_idx++)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (test_bit(region_id=
x, &dev->accessible_regions)) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 priv->region_idx =3D region_idx;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 break;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +
Why not
priv->region_idx =3D __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_d=
evice *dev)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0n =3D hweight_long(dev->accessible_regions);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0if (n > 1)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_info(&dev->sbd=
.core,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0"%s:%u: %lu accessible regions found. Only the first "
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0"one will be used\n",
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0"%s:%u: %lu accessible regions found\n",
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 __func__, __LINE__, n);
> - =C2=A0 =C2=A0 =C2=A0 dev->region_idx =3D __ffs(dev->accessible_regions)=
;
> - =C2=A0 =C2=A0 =C2=A0 dev_info(&dev->sbd.core,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"First accessibl=
e region has index %u start %llu size %llu\n",
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->region_idx,=
dev->regions[dev->region_idx].start,
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev->regions[dev=
->region_idx].size);
> -
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
> =C2=A0}
>
Same in the other drivers.
Gr{oetje,eeting}s,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds
^ permalink raw reply
* Re: [Cbe-oss-dev] [PATCH 06/15] ps3flash: Fix region align checks
From: Andre Heider @ 2011-08-01 20:56 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Geoff Levand, cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <CAMuHMdUgK7ZkaNA+DVg0_TL8fz+AfEOTZqW5WdOY5ybwSP=hcA@mail.gmail.com>
On Mon, Aug 1, 2011 at 10:29 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Aug 1, 2011 at 22:02, Andre Heider <a.heider@gmail.com> wrote:
>> The region fields used by the align checks are set in
>> ps3stor_setup(), so move those after that call.
>
> Are you sure?
> Aren't they set in
> arch/powerpc/platforms/ps3/device-init.c:ps3_setup_storage_dev()?
Hm right, unfortunate commit message... :)
dev->region_idx is set in ps3stor_probe_access(), which is called from
ps3stor_setup().
So the code always checked the first region, where it should check the
one beeing used.
Will fix the commit message.
Thanks
^ permalink raw reply
* Re: [Cbe-oss-dev] [PATCH 09/15] ps3: Limit the number of regions per storage device
From: Andre Heider @ 2011-08-01 20:58 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Geoff Levand, cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <CAMuHMdVq+WSjZKVZk1riEFX6NgW6Y+8EhCc0MqMtuDK5KAD7dw@mail.gmail.com>
On Mon, Aug 1, 2011 at 10:30 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
>> There can be only 8 regions, add a sanity check
>
> Why can there be only 8 regions?
I believe lv1 limits it to 8? I might be mistaken here, it mostly is a
check for the patches after this one
^ permalink raw reply
* Re: [Cbe-oss-dev] [PATCH 06/15] ps3flash: Fix region align checks
From: Geert Uytterhoeven @ 2011-08-01 21:00 UTC (permalink / raw)
To: Andre Heider; +Cc: Geoff Levand, cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <CAHsu+b-xtj8DvLS-v_5-QZ3Erc-9jpFd0rodeoc4RU9YPho3tw@mail.gmail.com>
On Mon, Aug 1, 2011 at 22:56, Andre Heider <a.heider@gmail.com> wrote:
> On Mon, Aug 1, 2011 at 10:29 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Aug 1, 2011 at 22:02, Andre Heider <a.heider@gmail.com> wrote:
>>> The region fields used by the align checks are set in
>>> ps3stor_setup(), so move those after that call.
>>
>> Are you sure?
>> Aren't they set in
>> arch/powerpc/platforms/ps3/device-init.c:ps3_setup_storage_dev()?
>
> Hm right, unfortunate commit message... :)
> dev->region_idx is set in ps3stor_probe_access(), which is called from
> ps3stor_setup().
> So the code always checked the first region, where it should check the
> one beeing used.
IC. You're right. That's indeed a bug.
Gr{oetje,eeting}s,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 10/15] ps3stor_lib: Add support for multiple regions
From: Andre Heider @ 2011-08-01 21:01 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Geoff Levand, cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <CAMuHMdXJWTw33ehLAPq00WjsebBHtQd5B9nM5Y-Z5JtevzmVqA@mail.gmail.com>
On Mon, Aug 1, 2011 at 10:35 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
>> =A0 =A0 =A0 =A0priv->gendisk =3D gendisk;
>> +
>> + =A0 =A0 =A0 /* find first accessible region */
>> + =A0 =A0 =A0 for (region_idx =3D 0; region_idx < dev->num_regions; regi=
on_idx++)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (test_bit(region_idx, &dev->accessible_=
regions)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 priv->region_idx =3D regio=
n_idx;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> +
>
> Why not
>
> priv->region_idx =3D __ffs(dev->accessible_regions);
Right, thanks. Will change that for v2
^ permalink raw reply
* [PATCH] drivers/misc: introduce Freescale Data Collection Manager driver
From: Timur Tabi @ 2011-08-01 21:48 UTC (permalink / raw)
To: arnd, grant.likely, linuxppc-dev, linux-kernel
The Data Collection Manager (DCM) is a feature of the FPGA on some Freescale
PowerPC reference boards that can read temperature, current, and voltage
settings from the sensors on those boards. This driver exposes the DCM via a
sysfs interface (/sys/devices/platform/fsl-ocm.0).
The DCM collects and tallies data over a period of time in the background,
without utilizing any resources on the host (CPU, memory, etc). The data is
summarized and made available when data collection stops. This allows power
consumption to be measured while the host is performing some tasks (usually
a benchmark).
Signed-off-by: Timur Tabi <timur@freescale.com>
---
Grant, could you please review the way I instantiate the platform driver and
call the .probe function?
Documentation/misc-devices/fsl_dcm.txt | 50 +++
drivers/misc/Kconfig | 14 +
drivers/misc/Makefile | 1 +
drivers/misc/fsl_dcm.c | 750 ++++++++++++++++++++++++++++++++
4 files changed, 815 insertions(+), 0 deletions(-)
create mode 100644 Documentation/misc-devices/fsl_dcm.txt
create mode 100644 drivers/misc/fsl_dcm.c
diff --git a/Documentation/misc-devices/fsl_dcm.txt b/Documentation/misc-devices/fsl_dcm.txt
new file mode 100644
index 0000000..d679db3
--- /dev/null
+++ b/Documentation/misc-devices/fsl_dcm.txt
@@ -0,0 +1,50 @@
+Freescale Data Collection Manager (DCM) device driver
+
+Inside the FPGA of some Freescale QorIQ (PowerPC) reference boards is a
+microprocessor called the General Purpose Processor (GSMA). Running on
+the GSMA is the Data Collection Manager (DCM), which is used to
+periodically read and tally voltage, current, and temperature measurements
+from the on-board sensors. You can use this feature to measure power
+consumption while running tests, without having the host CPU perform those
+measurements.
+
+Only some Freescale reference boards are supported. In most cases, the
+on-board dip switches need to be changed to enable the DCM.
+
+These are the boards that are currently supported:
+
+* P1022DS
+ Set SW9[7:8] to '10'.
+
+* P3060QDS
+ Set SW9[2] to 0 and SW9[8] to '1'.
+
+* P4080DS
+ Set SW9[7:8] to '10'.
+
+* P5020DS
+ Set SW9[7:8] to '10'.
+
+To use, first send "1" to the 'control' file:
+ echo 1 > /sys/devices/platform/fsl-dcm.0/control
+
+Then perform your benchmarks, when you are done, stop the monitoring:
+ echo 0 > /sys/devices/platform/fsl-dcm.0/control
+
+You can display the current status of data collection
+ cat /sys/devices/platform/fsl-dcm.0/control
+
+To view the results of data collection (after having been stopped):
+ cat /sys/devices/platform/fsl-dcm.0/result
+
+To change the sampling frequency to 10Hz (for example)
+ echo 10 > /sys/devices/platform/fsl-dcm.0/frequency
+
+Empirical evidence shows that the maximum sampling frequency is 48 Hz.
+Anything higher than that is unreliable.
+
+You can display the current sampling frequency:
+ cat /sys/devices/platform/fsl-dcm.0/frequency
+
+To display information about the DCM:
+ cat /sys/devices/platform/fsl-dcm.0/info
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 0a4d86c..93b4952 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -498,6 +498,20 @@ config USB_SWITCH_FSA9480
stereo and mono audio, video, microphone and UART data to use
a common connector port.
+config FSL_DCM
+ tristate "Freescale Data Collection Manager (DCM) driver"
+ depends on FSL_SOC && PPC_85xx
+ help
+ Inside the FPGA of some Freescale QorIQ (PowerPC) reference boards
+ is a microcontroller called the General Purpose Processor (GSMA).
+ Running on the GSMA is the Data Collection Manager (DCM), which is
+ used to periodically read and tally voltage, current, and temperature
+ measurements from the on-board sensors. You can use this feature to
+ measure power consumption while running tests, without having the
+ host CPU perform those measurements.
+
+ See Documentation/misc-devices/fsl_dcm.txt for more information.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 8f3efb6..f039c59 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -47,3 +47,4 @@ obj-$(CONFIG_AB8500_PWM) += ab8500-pwm.o
obj-y += lis3lv02d/
obj-y += carma/
obj-$(CONFIG_USB_SWITCH_FSA9480) += fsa9480.o
+obj-$(CONFIG_FSL_DCM) += fsl_dcm.o
diff --git a/drivers/misc/fsl_dcm.c b/drivers/misc/fsl_dcm.c
new file mode 100644
index 0000000..0e50e08
--- /dev/null
+++ b/drivers/misc/fsl_dcm.c
@@ -0,0 +1,750 @@
+/*
+ * Freescale Data Collection Manager (DCM) device driver
+ *
+ * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ *
+ * Inside the FPGA of some Freescale QorIQ (PowerPC) reference boards is a
+ * microprocessor called the General Purpose Processor (GSMA). Running on
+ * the GSMA is the Data Collection Manager (DCM), which is used to
+ * periodically read and tally voltage, current, and temperature measurements
+ * from the on-board sensors. You can use this feature to measure power
+ * consumption while running tests, without having the host CPU perform those
+ * measurements.
+ *
+ * See Documentation/misc-devices/fsl_dcm.txt for more information.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/of_platform.h>
+
+/* sysfs commands for 'control' */
+#define SYSFS_DCM_CMD_STOP 0
+#define SYSFS_DCM_CMD_START 1
+
+/* Crecords can be either voltage, current, or temperature */
+enum crecord_type {
+ CR_V, /* voltage */
+ CR_C, /* current */
+ CR_T /* temperature */
+};
+
+#define MAX_FREQUENCY 48 /* max freq (Hz) the DCM supports */
+#define DATA_ADDR 0x80 /* data address in DCM SRAM */
+
+struct crecord {
+ __be16 curr; /* last sample read */
+ __be16 max; /* maximum of all samples read */
+ __be16 qty1; /* number of samples read */
+ u8 qty2;
+ __be32 acc; /* sum of all samples read */
+} __packed;
+#define MAX_CRECORDS ((0x100 - DATA_ADDR) / sizeof(struct crecord))
+
+struct om_info {
+ __be16 version; /* DCM version number */
+ u8 prescale; /* prescale value used (should be 0) */
+ u8 timer; /* timer */
+ u8 count; /* number of CRECORDS */
+ __be16 address; /* address of CRECORD array in SRAM */
+ u8 res[3];
+} __packed;
+
+/**
+ * struct dcm_board - board-specific information
+ * @compatible: the 'compatible' property to search for
+ * @offsets: array of PIXIS address offsets (addr, data, dcmd, omsg, mack)
+ * @mask: enable mask for the OM_ENABLE command
+ * @convert_vout: board-specific function to convert a VOUT to millivolts
+ * @convert_iout: board-specific function to convert an IOUT to milliamps
+ * @convert_tout: board-specific function to convert a TOUT to degrees Celsius
+ * @ical: calibration factor for .convert_iout function
+ * @num: number of crecords (equal to the number of '1's in @mask)
+ * @names: array of names of each crecord
+ * @types: array of types of each crecord
+ */
+struct dcm_board {
+ const char *compatible;
+ unsigned int offsets[5];
+ u16 mask;
+ unsigned int (*convert_vout)(u16 vout);
+ unsigned int (*convert_iout)(u16 iout, unsigned int ical);
+ unsigned int (*convert_tout)(u16 tout);
+ unsigned int ical;
+ unsigned int num;
+ const char *names[MAX_CRECORDS];
+ enum crecord_type types[MAX_CRECORDS];
+};
+
+/* PIXIS register status bits */
+#define PX_OCMD_MSG (1 << 0)
+#define PX_OACK_ERR (1 << 1)
+#define PX_OACK_ACK (1 << 0) /* OACK is sometimes called MACK */
+
+/* DCM commands */
+#define OM_END 0x00
+#define OM_SETDLY 0x01
+#define OM_RST0 0x02
+#define OM_RST1 0x03
+#define OM_CHKDLY 0x04
+#define OM_PWR 0x05
+#define OM_WAKE 0x07
+#define OM_GETMEM 0x08
+#define OM_SETMEM 0x09
+#define OM_SCLR 0x10
+#define OM_START 0x11
+#define OM_STOP 0x12
+#define OM_GET 0x13
+#define OM_ENABLE 0x14
+#define OM_TIMER 0x15
+#define OM_SETV 0x30
+#define OM_INFO 0x31
+
+struct fsl_dcm_data {
+ struct device *dev;
+ const struct dcm_board *board;
+ void __iomem *base; /* PIXIS/QIXIS base address */
+ u8 __iomem *addr; /* SRAM address */
+ u8 __iomem *data; /* SRAM data */
+ u8 __iomem *ocmd; /* DCM command/status */
+ u8 __iomem *omsg; /* DCM message */
+ u8 __iomem *mack; /* DCM acknowledge */
+ struct crecord rec[MAX_CRECORDS];
+ u8 timer;
+ int running;
+};
+
+/*
+ * Converts a 16-bit VOUT value from the Zilker ZL6100 into a voltage value,
+ * in millivolts.
+ */
+static unsigned int voltage_from_zl6100(u16 vout)
+{
+ return (1000UL * vout) / (1 << 13);
+}
+
+/*
+ * Converts a 16-bit IOUT from the Texas Instruments INA220 chip into a
+ * current value, in milliamps. 'ical' is a board-specific calibration
+ * factor.
+ */
+static unsigned int current_from_ina220(u16 iout, unsigned int ical)
+{
+ unsigned long c;
+
+ /* milliamp = 1000 * ((iout / 100) / cal-factor) */
+ /* = (iout * 100000) / ical */
+ c = iout * 1000 * 100;
+ c /= ical;
+
+ return c;
+}
+
+/*
+ * Converts a 16-bit TOUT value from the sensor device into a temperature
+ * value, in degrees Celsius.
+ */
+static unsigned int temp_from_u16(u16 tout)
+{
+ return tout;
+}
+
+/*
+ * Write a byte to an address in SRAM
+ */
+static void write_sram(struct fsl_dcm_data *dcm, u8 offset, u8 v)
+{
+ out_8(dcm->addr, offset);
+ out_8(dcm->data, v);
+}
+
+/*
+ * Read a byte from an address in SRAM
+ */
+static u8 read_sram(struct fsl_dcm_data *dcm, u8 offset)
+{
+ out_8(dcm->addr, offset);
+
+ return in_8(dcm->data);
+}
+
+/*
+ * True TRUE if we can read/write SRAM, FALSE otherwise.
+ *
+ * If the SRAM is unavailable, it's probably because the DCM is busy with it.
+ */
+static int is_sram_available(struct fsl_dcm_data *dcm)
+{
+ u8 ack, cmd;
+
+ cmd = in_8(dcm->ocmd);
+ ack = in_8(dcm->mack);
+
+ if ((cmd & PX_OCMD_MSG) || (ack & PX_OACK_ACK)) {
+ dev_dbg(dcm->dev, "dcm is not ready (cmd=%02X mack=%02X)\n",
+ cmd, ack);
+ return 0;
+ }
+
+ return 1;
+}
+
+/*
+ * Loads and program into SRAM, then tells the DCM to run it, and then waits
+ * for it to finish.
+ */
+static int run_program(struct fsl_dcm_data *dcm, u8 addr, unsigned int len, ...)
+{
+ u8 v, n;
+ va_list args;
+
+ if (addr + len > 0xff) {
+ dev_err(dcm->dev, "address/length of %u/%u is out of bounds\n",
+ addr, len);
+ return 0;
+ }
+
+ /* load the program into SRAM */
+ va_start(args, len);
+ for (n = addr; n < addr + len; n++) {
+ v = va_arg(args, int);
+ write_sram(dcm, n, v);
+ }
+ va_end(args);
+
+ /* start the DCM */
+ out_8(dcm->omsg, addr);
+ out_8(dcm->ocmd, PX_OCMD_MSG);
+
+ /* wait for ack or error */
+ v = spin_event_timeout(in_8(dcm->mack) & (PX_OACK_ERR | PX_OACK_ACK),
+ 50000, 1000);
+ if ((!v) || (v & PX_OACK_ERR)) {
+ dev_err(dcm->dev, "timeout or error waiting for start ack\n");
+ return 0;
+ }
+
+ /* 4. allow the host to read SRAM */
+ out_8(dcm->ocmd, 0);
+
+ /* 5. wait for DCM to stop (ack == 0) or error (err == 1) */
+ spin_event_timeout(
+ ((v = in_8(dcm->mack)) & (PX_OACK_ERR | PX_OACK_ACK)) != PX_OACK_ACK,
+ 50000, 1000);
+
+ /* 6. check for error or timeout */
+ if (v & (PX_OACK_ERR | PX_OACK_ACK)) {
+ dev_err(dcm->dev, "timeout or error waiting for stop ack\n");
+ return 0;
+ }
+
+ return 1;
+}
+
+#define TRATE0 241120 /* t-rate if prescale==0, in millihertz */
+#define TRATE1 38759330 /* t-rate if prescale==1, in millihertz */
+
+/*
+ * Empirical tests show that any frequency higher than 48Hz is unreliable.
+ */
+static int set_dcm_frequency(struct fsl_dcm_data *dcm, unsigned long frequency)
+{
+ unsigned long timer;
+
+ if (!is_sram_available(dcm)) {
+ dev_err(dcm->dev, "dcm is busy\n");
+ return 0;
+ }
+
+ /* Restrict the frequency to a supported range. */
+ frequency = clamp_t(unsigned long, frequency, 1, MAX_FREQUENCY);
+
+ /* We only support prescale == 0 */
+ timer = TRATE0 / frequency;
+ dcm->timer = (timer / 1000) - 1;
+
+ return run_program(dcm, 0, 6, OM_TIMER, 0, dcm->timer, 0, 0, OM_END);
+}
+
+static int copy_from_sram(struct fsl_dcm_data *dcm, unsigned int addr,
+ void *buf, unsigned int len)
+{
+ u8 *p = buf;
+ unsigned int i;
+
+ if (addr + len > 0xff) {
+ dev_err(dcm->dev, "address/length of %u/%u is out of bounds\n",
+ addr, len);
+ return 0;
+ }
+
+ for (i = 0; i < len; i++)
+ p[i] = read_sram(dcm, addr + i);
+
+ return 1;
+}
+
+/*
+ * Tells the DCM which channels to collect data on.
+ */
+static int select_dcm_channels(struct fsl_dcm_data *dcm, u16 mask)
+{
+ if (!is_sram_available(dcm)) {
+ dev_err(dcm->dev, "dcm is busy\n");
+ return 0;
+ }
+
+ return run_program(dcm, 0, 4, OM_ENABLE,
+ ((mask >> 8) & 0xFF), mask & 0xFF, OM_END);
+}
+
+/*
+ * Tells the DCM to start data collection. If the DCM is currently running,
+ * it is restarted. Any currently collected data is cleared.
+ */
+static int start_data_collection(struct fsl_dcm_data *dcm)
+{
+ if (!is_sram_available(dcm)) {
+ dev_err(dcm->dev, "dcm is busy\n");
+ return 0;
+ }
+
+ if (dcm->running)
+ dev_dbg(dcm->dev, "restarting\n");
+
+ dcm->running = true;
+
+ return run_program(dcm, 0, 4, OM_STOP, OM_SCLR, OM_START, OM_END);
+}
+
+/*
+ * Tells the DCM to stop data collection. Collected data is copied from
+ * SRAM into a local buffer.
+ */
+static int stop_data_collection(struct fsl_dcm_data *dcm)
+{
+ if (!dcm->running) {
+ dev_dbg(dcm->dev, "dcm is already stopped\n");
+ return 1;
+ }
+
+ if (!is_sram_available(dcm)) {
+ dev_err(dcm->dev, "dcm is busy\n");
+ return 0;
+ }
+
+ if (!run_program(dcm, 0, 4, OM_STOP, OM_GET, DATA_ADDR, OM_END)) {
+ dev_err(dcm->dev, "could not stop monitoring\n");
+ return 0;
+ }
+
+ if (!copy_from_sram(dcm, DATA_ADDR, dcm->rec,
+ dcm->board->num * sizeof(struct crecord))) {
+ dev_err(dcm->dev, "could not copy sensor data\n");
+ return 0;
+ }
+
+ dcm->running = 0;
+ return 1;
+}
+
+ssize_t fsl_dcm_sysfs_control_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsl_dcm_data *dcm = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%s\n", dcm->running ? "running" : "stoppped");
+}
+
+ssize_t fsl_dcm_sysfs_control_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct fsl_dcm_data *dcm = dev_get_drvdata(dev);
+ unsigned long pm_cmd;
+ int ret;
+
+ ret = strict_strtoul(buf, 10, &pm_cmd);
+ if (ret)
+ return ret;
+
+ switch (pm_cmd) {
+ case SYSFS_DCM_CMD_START:
+ ret = start_data_collection(dcm);
+ if (!ret)
+ dev_err(dev, "failed to start power monitoring.\n");
+ break;
+ case SYSFS_DCM_CMD_STOP:
+ ret = stop_data_collection(dcm);
+ if (!ret)
+ dev_err(dev, "failed to stop power monitoring\n");
+ break;
+ default:
+ return -EIO;
+ }
+
+ return count;
+}
+
+/* Calculate the average, even if 'count' is zero */
+#define AVG(sum, count) ((sum) / ((count) ?: 1))
+
+static ssize_t fsl_dcm_sysfs_result(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsl_dcm_data *dcm = dev_get_drvdata(dev);
+ const struct dcm_board *board = dcm->board;
+ unsigned int i;
+ ssize_t len;
+
+ len = sprintf(buf,
+ "Name Num CURR MAX SUM Max Average\n"
+ "====== === ========== ========== =============== ======== ========\n");
+
+ for (i = 0; i < board->num; i++) {
+ unsigned int num, max, avg;
+ const char *unit;
+
+ num = (dcm->rec[i].qty1 << 8) | (dcm->rec[i].qty2);
+
+ switch (board->types[i]) {
+ case CR_V:
+ max = board->convert_vout(dcm->rec[i].max);
+ avg = board->convert_vout(AVG(dcm->rec[i].acc, num));
+ unit = "mV";
+ break;
+ case CR_C:
+ max = board->convert_iout(dcm->rec[i].max, board->ical);
+ avg = board->convert_iout(AVG(dcm->rec[i].acc, num),
+ board->ical);
+ unit = "mA";
+ break;
+ case CR_T:
+ max = board->convert_tout(dcm->rec[i].max);
+ avg = board->convert_tout(AVG(dcm->rec[i].acc, num));
+ unit = "C ";
+ break;
+ default:
+ continue;
+ }
+
+ len += sprintf(buf + len,
+ "%-6s %3u %5u/%04x %5u/%04x %8u/%06x %6d %s %6d %s\n",
+ board->names[i], num,
+ dcm->rec[i].curr, dcm->rec[i].curr,
+ dcm->rec[i].max, dcm->rec[i].max,
+ dcm->rec[i].acc, dcm->rec[i].acc,
+ max, unit, avg, unit);
+ }
+
+ return len;
+}
+
+ssize_t fsl_dcm_sysfs_info(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct fsl_dcm_data *dcm = dev_get_drvdata(dev);
+ struct om_info info;
+ ssize_t len;
+
+ if (!is_sram_available(dcm)) {
+ dev_err(dev, "dcm is busy\n");
+ return 0;
+ }
+
+ if (!run_program(dcm, 0, 2, OM_INFO, DATA_ADDR)) {
+ dev_err(dev, "could not run 'info' program\n");
+ return 0;
+ }
+
+ if (!copy_from_sram(dcm, DATA_ADDR, &info, sizeof(info))) {
+ dev_err(dev, "could not copy 'info' data\n");
+ return 0;
+ }
+
+ len = sprintf(buf, "DCM Version: %u\n", info.version);
+ len += sprintf(buf + len, "Prescale: %u\n", info.prescale);
+ len += sprintf(buf + len, "Timer: %u\n", info.timer);
+ len += sprintf(buf + len, "Number of CRECORDs: %u\n", info.count);
+ len += sprintf(buf + len, "CRECORD Address: %u\n", info.address);
+
+ return len;
+}
+
+ssize_t fsl_dcm_sysfs_frequency_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsl_dcm_data *dcm = dev_get_drvdata(dev);
+ unsigned long frequency;
+
+ frequency = TRATE0 / (dcm->timer + 1);
+
+ return sprintf(buf, "%lu Hz\n", frequency / 1000);
+}
+
+ssize_t fsl_dcm_sysfs_frequency_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct fsl_dcm_data *dcm = dev_get_drvdata(dev);
+ unsigned long frequency;
+ int ret;
+
+ ret = strict_strtoul(buf, 10, &frequency);
+ if (ret)
+ return ret;
+
+ set_dcm_frequency(dcm, frequency);
+
+ return count;
+}
+
+static DEVICE_ATTR(control, 0666, fsl_dcm_sysfs_control_show,
+ fsl_dcm_sysfs_control_store);
+static DEVICE_ATTR(result, 0444, fsl_dcm_sysfs_result, NULL);
+static DEVICE_ATTR(info, 0444, fsl_dcm_sysfs_info, NULL);
+static DEVICE_ATTR(frequency, 0666, fsl_dcm_sysfs_frequency_show,
+ fsl_dcm_sysfs_frequency_store);
+
+static const struct attribute_group fsl_dcm_attr_group = {
+ .attrs = (struct attribute *[]) {
+ &dev_attr_control.attr,
+ &dev_attr_result.attr,
+ &dev_attr_info.attr,
+ &dev_attr_frequency.attr,
+ NULL,
+ },
+};
+
+static int fsl_dcm_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct fsl_dcm_data *dcm;
+ int ret;
+
+ dcm = kzalloc(sizeof(struct fsl_dcm_data), GFP_KERNEL);
+ if (!dcm)
+ return -ENOMEM;
+
+ dcm->base = of_iomap(np, 0);
+ if (!dcm->base) {
+ dev_err(&pdev->dev, "could not map fpga node\n");
+ ret = -ENOMEM;
+ goto error_kzalloc;
+ }
+
+ dcm->dev = &pdev->dev;
+ dcm->board = pdev->dev.platform_data;
+
+ dcm->addr = dcm->base + dcm->board->offsets[0];
+ dcm->data = dcm->base + dcm->board->offsets[1];
+ dcm->ocmd = dcm->base + dcm->board->offsets[2];
+ dcm->omsg = dcm->base + dcm->board->offsets[3];
+ dcm->mack = dcm->base + dcm->board->offsets[4];
+
+ /* Check to make sure the DCM is enable and working */
+ if (!is_sram_available(dcm)) {
+ dev_err(&pdev->dev, "dcm is not responding\n");
+ ret = -ENODEV;
+ goto error_iomap;
+ }
+
+ dev_set_drvdata(&pdev->dev, dcm);
+
+ ret = sysfs_create_group(&pdev->dev.kobj, &fsl_dcm_attr_group);
+ if (ret) {
+ dev_err(&pdev->dev, "could not create sysfs group\n");
+ goto error_iomap;
+ }
+
+ if (!select_dcm_channels(dcm, dcm->board->mask)) {
+ dev_err(&pdev->dev, "could not set crecord mask\n");
+ ret = -ENODEV;
+ goto error_sysfs;
+ }
+
+ /* Set the timer to the fastest support rate. */
+ if (!set_dcm_frequency(dcm, MAX_FREQUENCY)) {
+ dev_err(&pdev->dev, "could not set frequency\n");
+ ret = -ENODEV;
+ goto error_sysfs;
+ }
+
+ return 0;
+
+error_sysfs:
+ sysfs_remove_group(&pdev->dev.kobj, &fsl_dcm_attr_group);
+
+error_iomap:
+ iounmap(dcm->base);
+
+error_kzalloc:
+ kfree(dcm);
+
+ return ret;
+}
+
+static int fsl_dcm_remove(struct platform_device *pdev)
+{
+ struct fsl_dcm_data *dcm = dev_get_drvdata(&pdev->dev);
+
+ stop_data_collection(dcm);
+
+ sysfs_remove_group(&pdev->dev.kobj, &fsl_dcm_attr_group);
+
+ iounmap(dcm->base);
+ kfree(dcm);
+
+ return 0;
+}
+
+static struct platform_driver fsl_dcm_platform_driver = {
+ .probe = fsl_dcm_probe,
+ .remove = fsl_dcm_remove,
+ .driver = {
+ .name = "fsl-dcm",
+ .owner = THIS_MODULE,
+ },
+};
+
+static const struct dcm_board dcm_types[] = {
+ {
+ "fsl,p1022ds-pixis",
+ { 0x0a, 0x0d, 0x14, 0x15, 0x18},
+ 0x155,
+ voltage_from_zl6100,
+ NULL,
+ temp_from_u16,
+ 0,
+ 5,
+ /* Current measurements are not reliable on this board */
+ {"Vdd", "OVdd", "S/XVdd", "GVdd", "CPU_Tj"},
+ {CR_V, CR_V, CR_V, CR_V, CR_T},
+ },
+ {
+ "fsl,p3060qds-pixis",
+ { 0x12, 0x13, 0x14, 0x15, 0x18},
+ 0x1ff,
+ voltage_from_zl6100,
+ current_from_ina220,
+ temp_from_u16,
+ 10328,
+ 9,
+ {"Vdd_CA", "Idd_CA", "Vdd_CB", "Idd_CB", "Vdd_PL", "Idd_PL",
+ "GVdd", "GIdd", "CPU_Tj"},
+ {CR_V, CR_C, CR_V, CR_C, CR_V, CR_C, CR_V, CR_C, CR_T},
+ },
+ {
+ "fsl,p4080ds-pixis",
+ { 0x0a, 0x0d, 0x14, 0x15, 0x18},
+ 0x155,
+ voltage_from_zl6100,
+ NULL,
+ temp_from_u16,
+ 0,
+ 5,
+ /* Current measurements are not reliable on this board */
+ {"Vdd_CA", "Vdd_CB", "Vdd_PL", "GVdd", "CPU_Tj"},
+ {CR_V, CR_V, CR_V, CR_V, CR_T},
+ },
+ {
+ "fsl,p5020ds-pixis",
+ { 0x0a, 0x0d, 0x14, 0x15, 0x18},
+ 0x1ff,
+ voltage_from_zl6100,
+ current_from_ina220,
+ temp_from_u16,
+ 21064,
+ 9,
+ {"Vdd_CA", "Idd_CA", "Vdd_CB", "Idd_CB", "Vdd_PL", "Idd_PL",
+ "GVdd", "GIdd", "CPU_Tj"},
+ {CR_V, CR_C, CR_V, CR_C, CR_V, CR_C, CR_V, CR_C, CR_T},
+ },
+};
+
+static int __init fsl_dcm_init(void)
+{
+ struct platform_device *pdev;
+ struct device_node *np = NULL;
+ unsigned int i;
+ int ret;
+
+ /* Look for a supported platform */
+ for (i = 0; i < ARRAY_SIZE(dcm_types); i++) {
+ np = of_find_compatible_node(NULL, NULL,
+ dcm_types[i].compatible);
+ if (np)
+ break;
+ }
+ if (!np) {
+ pr_debug("fsl-dcm: unsupported platform\n");
+ return -ENODEV;
+ }
+
+ /* We found a supported platform, so register a platform driver */
+ ret = platform_driver_register(&fsl_dcm_platform_driver);
+ if (ret) {
+ pr_err("fsl-dcm: could not register platform driver\n");
+ goto error_np;
+ }
+
+ /* We need to create a device and add the data for this platform */
+ pdev = platform_device_alloc(fsl_dcm_platform_driver.driver.name, 0);
+ if (!pdev) {
+ ret = -ENOMEM;
+ goto error_drv;
+ }
+
+ /* Pass the device_node pointer to the probe function */
+ pdev->dev.of_node = np;
+
+ /* Pass the DCM platform data */
+ ret = platform_device_add_data(pdev, &dcm_types[i],
+ sizeof(struct dcm_board));
+ if (ret) {
+ pr_err("fsl-dcm: could not register platform driver\n");
+ goto error_dev;
+ }
+
+ /* This will call the probe function */
+ ret = platform_device_add(pdev);
+ if (ret) {
+ pr_err("fsl-dcm: could not register platform driver\n");
+ goto error_dev;
+ }
+
+ of_node_put(np);
+
+ return 0;
+
+error_dev:
+ platform_device_unregister(pdev);
+
+error_drv:
+ platform_driver_unregister(&fsl_dcm_platform_driver);
+
+error_np:
+ of_node_put(np);
+
+ return ret;
+}
+
+static void __exit fsl_dcm_exit(void)
+{
+ platform_driver_unregister(&fsl_dcm_platform_driver);
+}
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("Freescale Data Collection Manager driver");
+MODULE_LICENSE("GPL v2");
+
+module_init(fsl_dcm_init);
+module_exit(fsl_dcm_exit);
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] drivers/misc: introduce Freescale Data Collection Manager driver
From: Tabi Timur-B04825 @ 2011-08-01 23:58 UTC (permalink / raw)
To: Mark Brown
Cc: linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org,
arnd@arndb.de
In-Reply-To: <20110801234645.GA15792@sirena.org.uk>
Mark Brown wrote:
> On Mon, Aug 01, 2011 at 04:48:54PM -0500, Timur Tabi wrote:
>> The Data Collection Manager (DCM) is a feature of the FPGA on some Frees=
cale
>> PowerPC reference boards that can read temperature, current, and voltage
>> settings from the sensors on those boards. This driver exposes the DCM =
via a
>> sysfs interface (/sys/devices/platform/fsl-ocm.0).
>
> This sounds like it should be a hwmon driver.
I didn't see any way to interface the hardware to the hwmon layer in a=20
manner that provides the information that our customers went using this=20
hardware.
>> The DCM collects and tallies data over a period of time in the backgroun=
d,
>> without utilizing any resources on the host (CPU, memory, etc). The dat=
a is
>> summarized and made available when data collection stops. This allows p=
ower
>> consumption to be measured while the host is performing some tasks (usua=
lly
>> a benchmark).
>
> Though this is a bit odd for the subsystem I don't think it's too far
> out of what other hwmon chips can do, some of them do have longer term
> stats than just instantaneous readings.
Can you show an example or some documentation? I couldn't find anything=20
remotely like that. I don't even see anything that lets me start/stop=20
monitoring of sensors.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* Re: [PATCH] drivers/misc: introduce Freescale Data Collection Manager driver
From: Mark Brown @ 2011-08-01 23:46 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, linux-kernel, arnd
In-Reply-To: <1312235334-15036-1-git-send-email-timur@freescale.com>
On Mon, Aug 01, 2011 at 04:48:54PM -0500, Timur Tabi wrote:
> The Data Collection Manager (DCM) is a feature of the FPGA on some Freescale
> PowerPC reference boards that can read temperature, current, and voltage
> settings from the sensors on those boards. This driver exposes the DCM via a
> sysfs interface (/sys/devices/platform/fsl-ocm.0).
This sounds like it should be a hwmon driver.
> The DCM collects and tallies data over a period of time in the background,
> without utilizing any resources on the host (CPU, memory, etc). The data is
> summarized and made available when data collection stops. This allows power
> consumption to be measured while the host is performing some tasks (usually
> a benchmark).
Though this is a bit odd for the subsystem I don't think it's too far
out of what other hwmon chips can do, some of them do have longer term
stats than just instantaneous readings.
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: Benjamin Herrenschmidt @ 2011-08-02 1:27 UTC (permalink / raw)
To: Avi Kivity
Cc: Alexey Kardashevskiy, kvm, Paul Mackerras,
linux-pci@vger.kernel.org, David Gibson, Alex Williamson,
Anthony Liguori, linuxppc-dev
In-Reply-To: <4E356221.6010302@redhat.com>
On Sun, 2011-07-31 at 17:09 +0300, Avi Kivity wrote:
> On 07/30/2011 02:58 AM, Benjamin Herrenschmidt wrote:
> > - Having a magic heuristic in libvirt to figure out those constraints is
> > WRONG. This reeks of XFree 4 PCI layer trying to duplicate the kernel
> > knowledge of PCI resource management and getting it wrong in many many
> > cases, something that took years to fix essentially by ripping it all
> > out. This is kernel knowledge and thus we need the kernel to expose in a
> > way or another what those constraints are, what those "partitionable
> > groups" are.
>
> How about a sysfs entry partition=<partition-id>? then libvirt knows not
> to assign devices from the same partition to different guests (and not
> to let the host play with them, either).
That would work. On POWER I also need to expose the way that such
partitions also mean shared iommu domain but that's probably doable.
It would be easy for me to implement it that way since I would just pass
down my PE#.
However, it seems to be a bit of the "smallest possible tweak" to get it
to work. We keep a completely orthogonal iommu domain handling for x86
and there is no link between them.
I still personally prefer a way to statically define the grouping, but
it looks like you guys don't agree... oh well.
> > The interface currently proposed for VFIO (and associated uiommu)
> > doesn't handle that problem at all. Instead, it is entirely centered
> > around a specific "feature" of the VTd iommu's for creating arbitrary
> > domains with arbitrary devices (tho those devices -do- have the same
> > constraints exposed above, don't try to put 2 legacy PCI devices behind
> > the same bridge into 2 different domains !), but the API totally ignores
> > the problem, leaves it to libvirt "magic foo" and focuses on something
> > that is both quite secondary in the grand scheme of things, and quite
> > x86 VTd specific in the implementation and API definition.
> >
> > Now, I'm not saying these programmable iommu domains aren't a nice
> > feature and that we shouldn't exploit them when available, but as it is,
> > it is too much a central part of the API.
>
> I have a feeling you'll be getting the same capabilities sooner or
> later, or you won't be able to make use of S/R IOV VFs.
I'm not sure why you mean. We can do SR/IOV just fine (well, with some
limitations due to constraints with how our MMIO segmenting works and
indeed some of those are being lifted in our future chipsets but
overall, it works).
In -theory-, one could do the grouping dynamically with some kind of API
for us as well. However the constraints are such that it's not
practical. Filtering on RID is based on number of bits to match in the
bus number and whether to match the dev and fn. So it's not arbitrary
(but works fine for SR-IOV).
The MMIO segmentation is a bit special too. There is a single MMIO
region in 32-bit space (size is configurable but that's not very
practical so for now we stick it to 1G) which is evenly divided into N
segments (where N is the number of PE# supported by the host bridge,
typically 128 with the current bridges).
Each segment goes through a remapping table to select the actual PE# (so
large BARs use consecutive segments mapped to the same PE#).
For SR-IOV we plan to not use the M32 region. We also have 64-bit MMIO
regions which act as some kind of "accordions", they are evenly divided
into segments in different PE# and there's several of them which we can
"move around" and typically use to map VF BARs.
> While we should
> support the older hardware, the interfaces should be designed with the
> newer hardware in mind.
Well, our newer hardware will relax some of our limitations, like the
way our 64-bit segments work (I didn't go into details but they have
some inconvenient size constraints that will be lifted), having more
PE#, supporting more MSI ports etc... but the basic scheme remains the
same. Oh and the newer IOMMU will support separate address spaces.
But as you said, we -do- need to support the older stuff.
> > My main point is that I don't want the "knowledge" here to be in libvirt
> > or qemu. In fact, I want to be able to do something as simple as passing
> > a reference to a PE to qemu (sysfs path ?) and have it just pickup all
> > the devices in there and expose them to the guest.
>
> Such magic is nice for a developer playing with qemu but in general less
> useful for a managed system where the various cards need to be exposed
> to the user interface anyway.
Right but at least the code that does that exposure can work top-down,
picking groups and exposing their content.
> > * IOMMU
> >
> > Now more on iommu. I've described I think in enough details how ours
> > work, there are others, I don't know what freescale or ARM are doing,
> > sparc doesn't quite work like VTd either, etc...
> >
> > The main problem isn't that much the mechanics of the iommu but really
> > how it's exposed (or not) to guests.
> >
> > VFIO here is basically designed for one and only one thing: expose the
> > entire guest physical address space to the device more/less 1:1.
>
> A single level iommu cannot be exposed to guests. Well, it can be
> exposed as an iommu that does not provide per-device mapping.
Well, x86 ones can't maybe but on POWER we can and must thanks to our
essentially paravirt model :-) Even if it' wasn't and we used trapping
of accesses to the table, it would work because in practice, even with
filtering, what we end up having is a per-device (or rather per-PE#
table).
> A two level iommu can be emulated and exposed to the guest. See
> http://support.amd.com/us/Processor_TechDocs/48882.pdf for an example.
What you mean 2-level is two passes through two trees (ie 6 or 8 levels
right ?). We don't have that and probably never will. But again, because
we have a paravirt interface to the iommu, it's less of an issue.
> > This means:
> >
> > - It only works with iommu's that provide complete DMA address spaces
> > to devices. Won't work with a single 'segmented' address space like we
> > have on POWER.
> >
> > - It requires the guest to be pinned. Pass-through -> no more swap
>
> Newer iommus (and devices, unfortunately) (will) support I/O page faults
> and then the requirement can be removed.
No. -Some- newer devices will. Out of these, a bunch will have so many
bugs in it it's not usable. Some never will. It's a mess really and I
wouldn't design my stuff based on those premises just yet. Making it
possible to support it for sure, having it in mind, but not making it
the fundation on which the whole API is designed.
> > - The guest cannot make use of the iommu to deal with 32-bit DMA
> > devices, thus a guest with more than a few G of RAM (I don't know the
> > exact limit on x86, depends on your IO hole I suppose), and you end up
> > back to swiotlb& bounce buffering.
>
> Is this a problem in practice?
Could be. It's an artificial limitation we don't need on POWER.
> > - It doesn't work for POWER server anyways because of our need to
> > provide a paravirt iommu interface to the guest since that's how pHyp
> > works today and how existing OSes expect to operate.
>
> Then you need to provide that same interface, and implement it using the
> real iommu.
Yes. Working on it. It's not very practical due to how VFIO interacts in
terms of APIs but solvable. Eventually, we'll make the iommu Hcalls
almost entirely real-mode for performance reasons.
> > - Performance sucks of course, the vfio map ioctl wasn't mean for that
> > and has quite a bit of overhead. However we'll want to do the paravirt
> > call directly in the kernel eventually ...
>
> Does the guest iomap each request? Why?
Not sure what you mean... the guest calls h-calls for every iommu page
mapping/unmapping, yes. So the performance of these is critical. So yes,
we'll eventually do it in kernel. We just haven't yet.
> Emulating the iommu in the kernel is of course the way to go if that's
> the case, still won't performance suck even then?
Well, we have HW on the field where we still beat intel on 10G
networking performances but heh, yeah, the cost of those h-calls is a
concern.
There are some new interfaces in pHyp that we'll eventually support that
allow to create additional iommu mappings in 64-bit space (the current
base mapping is 32-bit and 4K for backward compatibility) with larger
iommu page sizes.
This will eventually help. For guests backed with hugetlbfs we might be
able to map the whole guest in using 16M pages at the iommu level.
But on the other hand, the current method means that we can support
pass-through without losing overcommit & paging which is handy.
> > The QEMU side VFIO code hard wires various constraints that are entirely
> > based on various requirements you decided you have on x86 but don't
> > necessarily apply to us :-)
> >
> > Due to our paravirt nature, we don't need to masquerade the MSI-X table
> > for example. At all. If the guest configures crap into it, too bad, it
> > can only shoot itself in the foot since the host bridge enforce
> > validation anyways as I explained earlier. Because it's all paravirt, we
> > don't need to "translate" the interrupt vectors& addresses, the guest
> > will call hyercalls to configure things anyways.
>
> So, you have interrupt redirection? That is, MSI-x table values encode
> the vcpu, not pcpu?
Not exactly. The MSI-X address is a real PCI address to an MSI port and
the value is a real interrupt number in the PIC.
However, the MSI port filters by RID (using the same matching as PE#) to
ensure that only allowed devices can write to it, and the PIC has a
matching PE# information to ensure that only allowed devices can trigger
the interrupt.
As for the guest knowing what values to put in there (what port address
and interrupt source numbers to use), this is part of the paravirt APIs.
So the paravirt APIs handles the configuration and the HW ensures that
the guest cannot do anything else than what it's allowed to.
> Alex, with interrupt redirection, we can skip this as well? Perhaps
> only if the guest enables interrupt redirection?
>
> If so, it's not arch specific, it's interrupt redirection specific.
>
> > We don't need to prevent MMIO pass-through for small BARs at all. This
> > should be some kind of capability or flag passed by the arch. Our
> > segmentation of the MMIO domain means that we can give entire segments
> > to the guest and let it access anything in there (those segments are a
> > multiple of the page size always). Worst case it will access outside of
> > a device BAR within a segment and will cause the PE to go into error
> > state, shooting itself in the foot, there is no risk of side effect
> > outside of the guest boundaries.
>
> Does the BAR value contain the segment base address? Or is that added
> later?
It's a shared address space. With a basic configuration on p7ioc for
example we have MMIO going from 3G to 4G (PCI side addresses). BARs
contain the normal PCI address there. But that 1G is divided in 128
segments of equal size which can separately be assigned to PE#'s.
So BARs are allocated by firmware or the kernel PCI code so that devices
in different PEs don't share segments.
Of course there's always the risk that a device can be hacked via a
sideband access to BARs to move out of it's allocated segment. That
means that the guest owning that device won't be able to access it
anymore and can potentially disturb a guest or host owning whatever is
in that other segment.
The only way to enforce isolation here is to ensure that PE# are
entirely behind P2P bridges, since those would then ensure that even if
you put crap into your BARs you won't be able to walk over a neighbour.
I believe pHyp enforces that, for example, if you have a slot, all
devices & functions behind that slot pertain to the same PE# under pHyp.
That means you cannot put individual functions of a device into
different PE# with pHyp.
We plan to be a bit less restrictive here for KVM, assuming that if you
use a device that allows such a back-channel to the BARs, then it's your
problem to not trust such a device for virtualization. And most of the
time, you -will- have a P2P to protect you anyways.
The problem doesn't exist (or is assumed as non-existing) for SR-IOV
since in that case, the VFs are meant to be virtualized, so pHyp assumes
there is no such back-channel and it can trust them to be in different
PE#.
Cheers,
Ben.
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: Benjamin Herrenschmidt @ 2011-08-02 1:29 UTC (permalink / raw)
To: Alex Williamson
Cc: Alexey Kardashevskiy, kvm, Paul Mackerras,
linux-pci@vger.kernel.org, David Gibson, Anthony Liguori,
linuxppc-dev
In-Reply-To: <1312216847.2653.258.camel@bling.home>
On Mon, 2011-08-01 at 10:40 -0600, Alex Williamson wrote:
> On Sun, 2011-07-31 at 08:21 +1000, Benjamin Herrenschmidt wrote:
> > On Sat, 2011-07-30 at 09:58 +1000, Benjamin Herrenschmidt wrote:
> > > Hi folks !
> > >
> > > So I promised Anthony I would try to summarize some of the comments &
> > > issues we have vs. VFIO after we've tried to use it for PCI pass-through
> > > on POWER. It's pretty long, there are various items with more or less
> > > impact, some of it is easily fixable, some are API issues, and we'll
> > > probably want to discuss them separately, but for now here's a brain
> > > dump.
> > >
> > > David, Alexei, please make sure I haven't missed anything :-)
> >
> > And I think I have :-)
> >
> > * Config space
> >
> > VFIO currently handles that as a byte stream. It's quite gross to be
> > honest and it's not right. You shouldn't lose access size information
> > between guest and host when performing real accesses.
> >
> > Some config space registers can have side effects and not respecting
> > access sizes can be nasty.
>
> It's a bug, let's fix it.
Right. I was just trying to be exhaustive :-) If you don't beat us to
it, we'll eventually submit patches to fix it, we haven't fixed it yet
either, just something I noticed (because this byte-transport also makes
handling of endianess clumsly).
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] drivers/misc: introduce Freescale Data Collection Manager driver
From: Mark Brown @ 2011-08-02 1:50 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org,
arnd@arndb.de
In-Reply-To: <4E373D88.80006@freescale.com>
On Mon, Aug 01, 2011 at 11:58:00PM +0000, Tabi Timur-B04825 wrote:
> Mark Brown wrote:
> > On Mon, Aug 01, 2011 at 04:48:54PM -0500, Timur Tabi wrote:
> >> PowerPC reference boards that can read temperature, current, and voltage
> >> settings from the sensors on those boards. This driver exposes the DCM via a
> >> sysfs interface (/sys/devices/platform/fsl-ocm.0).
> > This sounds like it should be a hwmon driver.
> I didn't see any way to interface the hardware to the hwmon layer in a
> manner that provides the information that our customers went using this
> hardware.
Even if that's the case I can't see that it is a good reason for adding
a driver out of any subsystem with a completely non-standard interface.
> > Though this is a bit odd for the subsystem I don't think it's too far
> > out of what other hwmon chips can do, some of them do have longer term
> > stats than just instantaneous readings.
> Can you show an example or some documentation? I couldn't find anything
> remotely like that. I don't even see anything that lets me start/stop
> monitoring of sensors.
I'd expect that things like the _lowest, _highest and _average
attributes which a number of drivers have are what you're looking for.
At the very least it seems obvious how you might extend the interface if
some features you need are missing. The subsystem has fairly extensive
documentation in Documentation/hwmon.
^ permalink raw reply
* Re: [PATCH] drivers/misc: introduce Freescale Data Collection Manager driver
From: Tabi Timur-B04825 @ 2011-08-02 1:57 UTC (permalink / raw)
To: Mark Brown
Cc: linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org,
arnd@arndb.de
In-Reply-To: <20110802015050.GA17286@opensource.wolfsonmicro.com>
Mark Brown wrote:
> I'd expect that things like the _lowest, _highest and _average
> attributes which a number of drivers have are what you're looking for.
Yes, but then all I'm doing is presenting numbers that don't change to an=20
interface, simply on the basis that the numbers represent sensor values.
If I'm running a sensor application, I'm doing it to get real-time=20
monitoring of the sensors in my system. The DCM on our boards is not=20
capable of real-time results. So you're not actually "monitoring" the=20
hardware. The data from the DCM is available only *after* you stop=20
running the background process.
> At the very least it seems obvious how you might extend the interface if
> some features you need are missing. The subsystem has fairly extensive
> documentation in Documentation/hwmon.
I just don't see how it fits. Yes, I could do it, but then I'd end up=20
with something that doesn't make any sense. I would have to use a custom=20
interface to start monitoring and then another interface to stop it. Then=
=20
I would query the results use the hwmon interface, but the results would=20
be static. That just seems silly.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: Benjamin Herrenschmidt @ 2011-08-02 2:00 UTC (permalink / raw)
To: Alex Williamson
Cc: chrisw, Alexey Kardashevskiy, kvm, Paul Mackerras,
linux-pci@vger.kernel.org, qemu-devel, David Gibson, aafabbri,
iommu, Anthony Liguori, linuxppc-dev, benve
In-Reply-To: <1312225174.2653.352.camel@bling.home>
On Mon, 2011-08-01 at 12:59 -0600, Alex Williamson wrote:
> >
> > .../...
>
> I'll try to consolidate my reply to all the above here because there are
> too many places above to interject and make this thread even more
> difficult to respond to.
True, I should try to do the same :-)
> Much of what you're discussion above comes
> down to policy. Do we trust DisINTx? Do we trust multi-function
> devices? I have no doubt there are devices we can use as examples for
> each behaving badly. On x86 this is one of the reasons we have SR-IOV.
Right, that and having the ability to provide way more functions that
you would normally have.
> Besides splitting a single device into multiple, it makes sure each
> devices is actually virtualization friendly. POWER seems to add
> multiple layers of hardware so that you don't actually have to trust the
> device, which is a great value add for enterprise systems, but in doing
> so it mostly defeats the purpose and functionality of SR-IOV.
Well not entirely. A lot of what POWER does is also about isolation on
errors. This is going to be useful with and without SR-IOV. Also not all
devices are SR-IOV capable and there are plenty of situations where one
would want to pass-through devices that aren't, I don't see that as
disappearing tomorrow.
> How we present this in a GUI is largely irrelevant because something has
> to create a superset of what the hardware dictates (can I uniquely
> identify transactions from this device, can I protect other devices from
> it, etc.), the system policy (do I trust DisINTx, do I trust function
> isolation, do I require ACS) and mold that with what the user actually
> wants to assign. For the VFIO kernel interface, we should only be
> concerned with the first problem. Userspace is free to make the rest as
> simple or complete as it cares to. I argue for x86, we want device
> level granularity of assignment, but that also tends to be the typical
> case (when only factoring in hardware restrictions) due to our advanced
> iommus.
Well, POWER iommu's are advanced too ... just in a different way :-) x86
seems to be a lot less interested in robustness and reliability for
example :-)
I tend to agree that the policy decisions in general should be done by
the user, tho with appropriate information :-)
But some of them on our side are hard requirements imposed by how our
firmware or early kernel code assigned the PE's and we need to expose
that. It directly derives the sharing of iommu's too but then we -could-
have those different iommu's point to the same table in memory and
essentially mimmic the x86 domains. We chose not to. The segments are
too small in our current HW design for one and it means we lose the
isolation between devices which is paramount to getting the kind of
reliability and error handling we want to achieve.
> > > > Maybe something like /sys/devgroups ? This probably warrants involving
> > > > more kernel people into the discussion.
> > >
> > > I don't yet buy into passing groups to qemu since I don't buy into the
> > > idea of always exposing all of those devices to qemu. Would it be
> > > sufficient to expose iommu nodes in sysfs that link to the devices
> > > behind them and describe properties and capabilities of the iommu
> > > itself? More on this at the end.
> >
> > Well, iommu aren't the only factor. I mentioned shared interrupts (and
> > my unwillingness to always trust DisINTx),
>
> *userspace policy*
Maybe ... some of it yes. I suppose. You can always hand out to
userspace bigger guns to shoot itself in the foot. Not always very wise
but heh.
Some of these are hard requirements tho. And we have to make that
decision when we assign PE's at boot time.
> > there's also the MMIO
> > grouping I mentioned above (in which case it's an x86 -limitation- with
> > small BARs that I don't want to inherit, especially since it's based on
> > PAGE_SIZE and we commonly have 64K page size on POWER), etc...
>
> But isn't MMIO grouping effectively *at* the iommu?
No exactly. It's a different set of tables & registers in the host
bridge and essentially a different set of logic, tho it does hook into
the whole "shared PE# state" thingy to enforce isolation of all layers
on error.
> > So I'm not too fan of making it entirely look like the iommu is the
> > primary factor, but we -can-, that would be workable. I still prefer
> > calling a cat a cat and exposing the grouping for what it is, as I think
> > I've explained already above, tho.
>
> The trouble is the "group" analogy is more fitting to a partitionable
> system, whereas on x86 we can really mix-n-match devices across iommus
> fairly easily. The iommu seems to be the common point to describe these
> differences.
No. You can do that by throwing away isolation between those devices and
thus throwing away error isolation capabilities as well. I suppose if
you don't care about RAS... :-)
> > > > Now some of this can be fixed with tweaks, and we've started doing it
> > > > (we have a working pass-through using VFIO, forgot to mention that, it's
> > > > just that we don't like what we had to do to get there).
> > >
> > > This is a result of wanting to support *unmodified* x86 guests. We
> > > don't have the luxury of having a predefined pvDMA spec that all x86
> > > OSes adhere to.
> >
> > No but you could emulate a HW iommu no ?
>
> We can, but then we have to worry about supporting legacy, proprietary
> OSes that may not have support or may make use of it differently. As
> Avi mentions, hardware is coming the eases the "pin the whole guest"
> requirement and we may implement emulated iommus for the benefit of some
> guests.
That's a pipe dream :-) It will take a LONG time before a reasonable
proportion of devices does this in a reliable way I believe.
> > > The 32bit problem is unfortunate, but the priority use
> > > case for assigning devices to guests is high performance I/O, which
> > > usually entails modern, 64bit hardware. I'd like to see us get to the
> > > point of having emulated IOMMU hardware on x86, which could then be
> > > backed by VFIO, but for now guest pinning is the most practical and
> > > useful.
> >
> > For your current case maybe. It's just not very future proof imho.
> > Anyways, it's fixable, but the APIs as they are make it a bit clumsy.
>
> You expect more 32bit devices in the future?
Got knows what embedded ARM folks will come up with :-) I wouldn't
dismiss that completely. I do expect to have to deal with OHCI for a
while tho.
> > > > Also our next generation chipset may drop support for PIO completely.
> > > >
> > > > On the other hand, because PIO is just a special range of MMIO for us,
> > > > we can do normal pass-through on it and don't need any of the emulation
> > > > done qemu.
> > >
> > > Maybe we can add mmap support to PIO regions on non-x86.
> >
> > We have to yes. I haven't looked into it yet, it should be easy if VFIO
> > kernel side starts using the "proper" PCI mmap interfaces in kernel (the
> > same interfaces sysfs & proc use).
>
> Patches welcome.
Sure, we do plan to send patches for a lot of those things as we get
there, I'm just chosing to mention all the issues at once here and we
haven't go to fixing -that- just yet.
.../...
> > Right. We can slow map the ROM, or we can not care :-) At the end of the
> > day, what is the difference here between a "guest" under qemu and the
> > real thing bare metal on the machine ? IE. They have the same issue vs.
> > accessing the ROM. IE. I don't see why qemu should try to make it safe
> > to access it at any time while it isn't on a real machine. Since VFIO
> > resets the devices before putting them in guest space, they should be
> > accessible no ? (Might require a hard reset for some devices tho ... )
>
> My primary motivator for doing the ROM the way it's done today is that I
> get to push all the ROM handling off to QEMU core PCI code. The ROM for
> an assigned device is handled exactly like the ROM for an emulated
> device except it might be generated by reading it from the hardware.
> This gives us the benefit of things like rombar=0 if I want to hide the
> ROM or romfile=<file> if I want to load an ipxe image for a device that
> may not even have a physical ROM. Not to mention I don't have to
> special case ROM handling routines in VFIO. So it actually has little
> to do w/ making it safe to access the ROM at any time.
On the other hand, let's hope no device has side effects on the ROM and
expects to exploit them :-) Do we know how ROM/flash updates work for
devices in practice ? Do they expect to be able to write to the ROM BAR
or they always use a different MMIO based sideband access ?
> > In any case, it's not a big deal and we can sort it out, I'm happy to
> > fallback to slow map to start with and eventually we will support small
> > pages mappings on POWER anyways, it's a temporary limitation.
>
> Perhaps this could also be fixed in the generic QEMU PCI ROM support so
> it works for emulated devices too... code reuse paying off already ;)
Heh, I think emulation works.
> > > > * EEH
> > > >
> > > > This is the name of those fancy error handling & isolation features I
> > > > mentioned earlier. To some extent it's a superset of AER, but we don't
> > > > generally expose AER to guests (or even the host), it's swallowed by
> > > > firmware into something else that provides a superset (well mostly) of
> > > > the AER information, and allow us to do those additional things like
> > > > isolating/de-isolating, reset control etc...
> > > >
> > > > Here too, we'll need arch specific APIs through VFIO. Not necessarily a
> > > > huge deal, I mention it for completeness.
> > >
> > > We expect to do AER via the VFIO netlink interface, which even though
> > > its bashed below, would be quite extensible to supporting different
> > > kinds of errors.
> >
> > As could platform specific ioctls :-)
>
> Is qemu going to poll for errors?
I wouldn't mind eventfd + ioctl, I really don't like netlink :-) But
others might disagree with me here. However that's not really my
argument, see below...
> > I don't understand what the advantage of netlink is compared to just
> > extending your existing VFIO ioctl interface, possibly using children
> > fd's as we do for example with spufs but it's not a huge deal. It just
> > that netlink has its own gotchas and I don't like multi-headed
> > interfaces.
>
> We could do yet another eventfd that triggers the VFIO user to go call
> an ioctl to see what happened, but then we're locked into an ioctl
> interface for something that we may want to more easily extend over
> time. As I said, it feels like this is what netlink is for and the
> arguments against seem to be more gut reaction.
My argument here is we already have an fd open, ie, we already have a
communication open to vfio as a chardev, I don't like the idea of
creating -another- one.
> Hmm... it is. I added a pci_get_irq() that returns a
> platform/architecture specific translation of a PCI interrupt to it's
> resulting system interrupt. Implement this in your PCI root bridge.
> There's a notifier for when this changes, so vfio will check
> pci_get_irq() again, also to be implemented in the PCI root bridge code.
> And a notifier that gets registered with that system interrupt and gets
> notice for EOI... implemented in x86 ioapic, somewhere else for power.
Let's leave this one alone, we'll fix it a way or another and we can
discuss the patches when it comes down to it.
> > > The problem is
> > > that we have to disable INTx on an assigned device after it fires (VFIO
> > > does this automatically). If we don't do this, a non-responsive or
> > > malicious guest could sit on the interrupt, causing it to fire
> > > repeatedly as a DoS on the host. The only indication that we can rely
> > > on to re-enable INTx is when the guest CPU writes an EOI to the APIC.
> > > We can't just wait for device accesses because a) the device CSRs are
> > > (hopefully) direct mapped and we'd have to slow map them or attempt to
> > > do some kind of dirty logging to detect when they're accesses b) what
> > > constitutes an interrupt service is device specific.
> > >
> > > That means we need to figure out how PCI interrupt 'A' (or B...)
> > > translates to a GSI (Global System Interrupt - ACPI definition, but
> > > hopefully a generic concept). That GSI identifies a pin on an IOAPIC,
> > > which will also see the APIC EOI. And just to spice things up, the
> > > guest can change the PCI to GSI mappings via ACPI. I think the set of
> > > callbacks I've added are generic (maybe I left ioapic in the name), but
> > > yes they do need to be implemented for other architectures. Patches
> > > appreciated from those with knowledge of the systems and/or access to
> > > device specs. This is the only reason that I make QEMU VFIO only build
> > > for x86.
> >
> > Right, and we need to cook a similiar sauce for POWER, it's an area that
> > has to be arch specific (and in fact specific to the specific HW machine
> > being emulated), so we just need to find out what's the cleanest way for
> > the plaform to "register" the right callbacks here.
>
> Aside from the ioapic, I hope it's obvious hooks in the PCI root bridge
> emulation.
Yeah, we'll see, whatever we come up with and we discuss the details
then :-)
> Thanks,
> >
> > Well, I would map those "iommus" to PEs, so what remains is the path to
> > put all the "other" bits and pieces such as inform qemu of the location
> > and size of the MMIO segment(s) (so we can map the whole thing and not
> > bother with individual BARs) etc...
>
> My assumption is that PEs are largely defined by the iommus already.
> Are MMIO segments a property of the iommu too? Thanks,
Not exactly but it's all tied together. See my other replies.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] drivers/misc: introduce Freescale Data Collection Manager driver
From: Mark Brown @ 2011-08-02 2:26 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org,
arnd@arndb.de
In-Reply-To: <4E375998.1020901@freescale.com>
On Tue, Aug 02, 2011 at 01:57:45AM +0000, Tabi Timur-B04825 wrote:
> Mark Brown wrote:
> > I'd expect that things like the _lowest, _highest and _average
> > attributes which a number of drivers have are what you're looking for.
> Yes, but then all I'm doing is presenting numbers that don't change to an
> interface, simply on the basis that the numbers represent sensor values.
> If I'm running a sensor application, I'm doing it to get real-time
> monitoring of the sensors in my system. The DCM on our boards is not
> capable of real-time results. So you're not actually "monitoring" the
> hardware. The data from the DCM is available only *after* you stop
> running the background process.
Right, that seems to fit reasonably well with things like averages and
extremes.
> > At the very least it seems obvious how you might extend the interface if
> > some features you need are missing. The subsystem has fairly extensive
> > documentation in Documentation/hwmon.
> I just don't see how it fits. Yes, I could do it, but then I'd end up
> with something that doesn't make any sense. I would have to use a custom
> interface to start monitoring and then another interface to stop it. Then
The most obvious thing seems to be to use the existing _reset_history
stuff to trigger a restart. If the hardware is so incapable that it
can't cope with reads while active and needs to be reset to even pause
that's seems pretty rubbish, I'd have expected you could at least pause
measurement momentarily to do a read.
Perhaps integration as a PMU may make more sense? The general point
here is that this doesn't sound like it's doing something so odd that it
shouldn't even be trying to work within any sort of framework.
^ permalink raw reply
* Re: [PATCH v11 08/10] USB ppc4xx: Add Synopsys DWC OTG PCD interrupt function
From: Pratyush Anand @ 2011-08-02 4:25 UTC (permalink / raw)
To: tmarri; +Cc: Mark Miesfeld, greg, linux-usb, linuxppc-dev, Fushen Chen
In-Reply-To: <1301684732-17603-1-git-send-email-tmarri@apm.com>
Is somebody working on these patches?
If not, then is it possible to share last modification, so that I can
start work from there.
If this v11 was the last modification, and noone is working further,
then just confirm it. I will start working from here.
Regards
Pratyush
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox