Re: [PATCH 2/2] powerpc/pseries: Add driver for PAPR SCM regions

On Sat, Oct 13, 2018 at 5:08 AM Michael Ellerman [off-list ref] wrote:

Dan Williams [off-list ref] writes:

quoted

On Tue, Oct 9, 2018 at 11:21 PM Oliver O'Halloran [off-list ref] wrote:

quoted

Adds a driver that implements support for enabling and accessing PAPR
SCM regions. Unfortunately due to how the PAPR interface works we can't
use the existing of_pmem driver (yet) because:

...

quoted

quoted

+
+static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
+{
+       struct device *dev = &p->pdev->dev;
+       struct nd_mapping_desc mapping;
+       struct nd_region_desc ndr_desc;
+       unsigned long dimm_flags;
+
+       p->bus_desc.ndctl = papr_scm_ndctl;
+       p->bus_desc.module = THIS_MODULE;
+       p->bus_desc.of_node = p->pdev->dev.of_node;
+       p->bus_desc.attr_groups = bus_attr_groups;
+       p->bus_desc.provider_name = kstrdup(p->pdev->name, GFP_KERNEL);
+
+       if (!p->bus_desc.provider_name)
+               return -ENOMEM;
+
+       p->bus = nvdimm_bus_register(NULL, &p->bus_desc);
+       if (!p->bus) {
+               dev_err(dev, "Error creating nvdimm bus %pOF\n", p->dn);
+               return -ENXIO;
+       }
+
+       dimm_flags = 0;
+       set_bit(NDD_ALIASING, &dimm_flags);
+
+       p->nvdimm = nvdimm_create(p->bus, p, papr_scm_dimm_groups,
+                               dimm_flags, PAPR_SCM_DIMM_CMD_MASK, 0, NULL);

Looks good, although I'm just about to push out commits that change
this function signature to take a 'security_ops' pointer. If you need
a stable branch to base this on, let me know.

...

quoted

Other than that looks ok to me:

Acked-by: Dan Williams <redacted>

...just the matter of what to do about function signature change.

Yeah that's a bit of a bother.

The ideal for me would be that you put the commit that changes the
signature by itself in a branch based on 4.19-rc3 (or earlier), and then
we could both just merge that.

But not sure if that will work with whatever else you're trying to sync
up with.

How about this, I'll move the new signature to an 'advanced':

    __nvdimm_create()

...and make the existing:

   nvdimm_create()

...a simple wrapper around the new functionality. That way no matter
what merge order we should be ok.