From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb0-x234.google.com (mail-yb0-x234.google.com [IPv6:2607:f8b0:4002:c09::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 60C3B21D147B4 for ; Tue, 11 Jul 2017 00:34:40 -0700 (PDT) Received: by mail-yb0-x234.google.com with SMTP id p207so35315605yba.2 for ; Tue, 11 Jul 2017 00:36:26 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20170627102851.15484-1-oohall@gmail.com> From: Dan Williams Date: Tue, 11 Jul 2017 00:36:24 -0700 Message-ID: Subject: Re: [RFC 1/4] libnvdimm: add to_{nvdimm,nd_region}_dev() List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Oliver Cc: linuxppc-dev , "linux-nvdimm@lists.01.org" List-ID: On Mon, Jul 10, 2017 at 9:38 PM, Oliver wrote: > On Tue, Jul 11, 2017 at 9:53 AM, Dan Williams wrote: >> On Tue, Jun 27, 2017 at 3:28 AM, Oliver O'Halloran 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, _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb0-x232.google.com (mail-yb0-x232.google.com [IPv6:2607:f8b0:4002:c09::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3x6DRJ09hvzDqb4 for ; Tue, 11 Jul 2017 17:36:27 +1000 (AEST) Received: by mail-yb0-x232.google.com with SMTP id p207so35315587yba.2 for ; Tue, 11 Jul 2017 00:36:27 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20170627102851.15484-1-oohall@gmail.com> From: Dan Williams Date: Tue, 11 Jul 2017 00:36:24 -0700 Message-ID: Subject: Re: [RFC 1/4] libnvdimm: add to_{nvdimm,nd_region}_dev() To: Oliver Cc: linuxppc-dev , "linux-nvdimm@lists.01.org" Content-Type: text/plain; charset="UTF-8" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, Jul 10, 2017 at 9:38 PM, Oliver wrote: > On Tue, Jul 11, 2017 at 9:53 AM, Dan Williams wrote: >> On Tue, Jun 27, 2017 at 3:28 AM, Oliver O'Halloran 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,