[RFC] SQUASHME: pmem: Split up pmem_probe from pmem_alloc

Commit Message

Boaz Harrosh March 31, 2015, 2:21 p.m. UTC

On 03/31/2015 01:25 PM, Boaz Harrosh wrote:
<>
> 
> And one last issue. I have some configuration "hardness" with the
> memmap=nn!aa Kernel command line API, it was better for me with the
> pmem map= module param. Will you be OK if I split pmem_probe() into
> calling pmem_alloc(addr, length), so I can keep an out-of-tree patch
> that adds the map= parameter to pmem?
> 

Hi Christoph.

Is this too much ugly for you? The reason I need it is because
I would like to keep out-of-tree a patch that adds back the
map= module param so to have more fine grain control of my
pmem devices.

[And also to have mapping control per pmem device. For example
 one device can be pages-mapped another uncached ioremap, 3rd
 write through mapping, and so on]

In the patch if pmem loads without map= it will load like
yours, but if map= is not empty it will not call platform_driver_register()
and will manually load devices as before.

I can still do this, of course. But with this here split patch
it will be easier to maintain.

Signed-off-by: Boaz Harrosh <[e-mail]>
---
 drivers/block/pmem.c | 48 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 15 deletions(-)

Comments

Christoph Hellwig March 31, 2015, 4:10 p.m. UTC | #1

On Tue, Mar 31, 2015 at 05:21:15PM +0300, Boaz Harrosh wrote:
> -static int pmem_probe(struct platform_device *pdev)
> +static int pmem_alloc(struct resource *res, struct device *dev,
> +		      struct pmem_device **o_pmem)
>  {

please return the pmem device or an ERR_PTR() here.

Except for that it looks fine for me, and it's exactly what we'd export
for your PCIe device which would get an almost trivial pci_driver wrapper
around it.

Patch

diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c
index 62cc9d0..0fc5c66 100644
--- a/drivers/block/pmem.c
+++ b/drivers/block/pmem.c
@@ -193,20 +193,13 @@  static void pmem_unmapmem(struct pmem_device *pmem)
 
 #endif /* !CONFIG_BLK_DEV_PMEM_USE_PAGES */
 
-static int pmem_probe(struct platform_device *pdev)
+static int pmem_alloc(struct resource *res, struct device *dev,
+		      struct pmem_device **o_pmem)
 {
 	struct pmem_device *pmem;
 	struct gendisk *disk;
-	struct resource *res;
 	int idx, err;
 
-	if (WARN_ON(pdev->num_resources > 1))
-		return -ENXIO;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (unlikely(!res))
-		return -ENXIO;
-
 	pmem = kzalloc(sizeof(*pmem), GFP_KERNEL);
 	if (unlikely(!pmem))
 		return -ENOMEM;
@@ -240,13 +233,12 @@  static int pmem_probe(struct platform_device *pdev)
 	disk->queue		= pmem->pmem_queue;
 	disk->flags		= GENHD_FL_EXT_DEVT;
 	sprintf(disk->disk_name, "pmem%d", idx);
-	disk->driverfs_dev = &pdev->dev;
+	disk->driverfs_dev = dev;
 	set_capacity(disk, pmem->size >> 9);
 	pmem->pmem_disk = disk;
 
 	add_disk(disk);
-
-	platform_set_drvdata(pdev, pmem);
+	*o_pmem = pmem;
 	return 0;
 
 out_free_queue:
@@ -255,19 +247,45 @@  out_unmap:
 	pmem_unmapmem(pmem);
 out_free_dev:
 	kfree(pmem);
+	*o_pmem = NULL;
 	return err;
 }
 
-static int pmem_remove(struct platform_device *pdev)
+static void pmem_free(struct pmem_device *pmem)
 {
-	struct pmem_device *pmem = platform_get_drvdata(pdev);
-
 	del_gendisk(pmem->pmem_disk);
 	put_disk(pmem->pmem_disk);
 	blk_cleanup_queue(pmem->pmem_queue);
 	pmem_unmapmem(pmem);
 	kfree(pmem);
+}
+
+static int pmem_probe(struct platform_device *pdev)
+{
+	struct pmem_device *pmem;
+	struct resource *res;
+	int err;
+
+	if (WARN_ON(pdev->num_resources > 1))
+		return -ENXIO;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (unlikely(!res))
+		return -ENXIO;
+
+	err = pmem_alloc(res, &pdev->dev, &pmem);
+	if (unlikely(err))
+		return err;
+
+	platform_set_drvdata(pdev, pmem);
+	return 0;
+}
+
+static int pmem_remove(struct platform_device *pdev)
+{
+	struct pmem_device *pmem = platform_get_drvdata(pdev);
 
+	pmem_free(pmem);
 	return 0;
 }