| Message ID | 20170627102851.15484-1-oohall@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, Jun 27, 2017 at 3:28 AM, Oliver O'Halloran <oohall@gmail.com> wrote: > struct device contains the ->of_node pointer so that devices can be > assoicated with the device-tree node that created them on DT platforms. > libnvdimm hides the struct device for regions and nvdimm devices inside > of an opaque structure so this patch adds accessors for each to allow > the of_nvdimm driver to set the of_node pointer. I'd rather go the other way and pass in the of_node to the bus and dimm registration routines. It's a generic property of the device so we should handle it like other generic device properties that get set at initialization time like 'attr_groups' in nvdimm_bus_descriptor, or a new parameter to nvdimm_create().
On Tue, Jul 11, 2017 at 9:53 AM, Dan Williams <dan.j.williams@intel.com> wrote: > On Tue, Jun 27, 2017 at 3:28 AM, Oliver O'Halloran <oohall@gmail.com> wrote: >> struct device contains the ->of_node pointer so that devices can be >> assoicated with the device-tree node that created them on DT platforms. >> libnvdimm hides the struct device for regions and nvdimm devices inside >> of an opaque structure so this patch adds accessors for each to allow >> the of_nvdimm driver to set the of_node pointer. > > I'd rather go the other way and pass in the of_node to the bus and > dimm registration routines. It's a generic property of the device so > we should handle it like other generic device properties that get set > at initialization time like 'attr_groups' in nvdimm_bus_descriptor, or > a new parameter to nvdimm_create(). Sure. I just figured it would be preferable to keep firmware specific details inside the firmware driver rather than adding #ifdef CONFIG_OF around the place. Do you have any objections to making nvdimm_create() take a descriptor structure rather than adding a parameter?
On Mon, Jul 10, 2017 at 9:38 PM, Oliver <oohall@gmail.com> wrote: > On Tue, Jul 11, 2017 at 9:53 AM, Dan Williams <dan.j.williams@intel.com> wrote: >> On Tue, Jun 27, 2017 at 3:28 AM, Oliver O'Halloran <oohall@gmail.com> wrote: >>> struct device contains the ->of_node pointer so that devices can be >>> assoicated with the device-tree node that created them on DT platforms. >>> libnvdimm hides the struct device for regions and nvdimm devices inside >>> of an opaque structure so this patch adds accessors for each to allow >>> the of_nvdimm driver to set the of_node pointer. >> >> I'd rather go the other way and pass in the of_node to the bus and >> dimm registration routines. It's a generic property of the device so >> we should handle it like other generic device properties that get set >> at initialization time like 'attr_groups' in nvdimm_bus_descriptor, or >> a new parameter to nvdimm_create(). > > Sure. I just figured it would be preferable to keep firmware specific > details inside the firmware driver rather than adding #ifdef CONFIG_OF > around the place. Do you have any objections to making nvdimm_create() > take a descriptor structure rather than adding a parameter? I don't see why we need "#ifdef CONFIG_OF". It's just a "struct of_node *" pointer that can be forward declared as "struct of_node;" we don't need the full definition. Yes, I'm fine with converting nvdimm_create() to a take a descriptor,
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c index f0d1b7e5de01..cbddac011181 100644 --- a/drivers/nvdimm/dimm_devs.c +++ b/drivers/nvdimm/dimm_devs.c @@ -227,6 +227,12 @@ struct nvdimm *to_nvdimm(struct device *dev) } EXPORT_SYMBOL_GPL(to_nvdimm); +struct device *to_nvdimm_dev(struct nvdimm *nvdimm) +{ + return &nvdimm->dev; +} +EXPORT_SYMBOL_GPL(to_nvdimm_dev); + struct nvdimm *nd_blk_region_to_dimm(struct nd_blk_region *ndbr) { struct nd_region *nd_region = &ndbr->nd_region; diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c index cbaab4210c39..6c3988135fd5 100644 --- a/drivers/nvdimm/region_devs.c +++ b/drivers/nvdimm/region_devs.c @@ -182,6 +182,12 @@ struct nd_region *to_nd_region(struct device *dev) } EXPORT_SYMBOL_GPL(to_nd_region); +struct device *to_nd_region_dev(struct nd_region *region) +{ + return ®ion->dev; +} +EXPORT_SYMBOL_GPL(to_nd_region_dev); + struct nd_blk_region *to_nd_blk_region(struct device *dev) { struct nd_region *nd_region = to_nd_region(dev); diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h index 550761477005..10fbc523ff95 100644 --- a/include/linux/libnvdimm.h +++ b/include/linux/libnvdimm.h @@ -139,6 +139,8 @@ struct nd_region *to_nd_region(struct device *dev); struct nd_blk_region *to_nd_blk_region(struct device *dev); struct nvdimm_bus_descriptor *to_nd_desc(struct nvdimm_bus *nvdimm_bus); struct device *to_nvdimm_bus_dev(struct nvdimm_bus *nvdimm_bus); +struct device *to_nvdimm_dev(struct nvdimm *nvdimm); +struct device *to_nd_region_dev(struct nd_region *region); const char *nvdimm_name(struct nvdimm *nvdimm); struct kobject *nvdimm_kobj(struct nvdimm *nvdimm); unsigned long nvdimm_cmd_mask(struct nvdimm *nvdimm);
struct device contains the ->of_node pointer so that devices can be assoicated with the device-tree node that created them on DT platforms. libnvdimm hides the struct device for regions and nvdimm devices inside of an opaque structure so this patch adds accessors for each to allow the of_nvdimm driver to set the of_node pointer. Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- drivers/nvdimm/dimm_devs.c | 6 ++++++ drivers/nvdimm/region_devs.c | 6 ++++++ include/linux/libnvdimm.h | 2 ++ 3 files changed, 14 insertions(+)