From: Michael Bringmann <mwb@linux.vnet.ibm.com>
To: Michael Ellerman <mpe@ellerman.id.au>,
Tyrel Datwyler <turtle.in.the.kernel@gmail.com>,
"open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)"
<linuxppc-dev@lists.ozlabs.org>,
linux-kernel@vger.kernel.org
Cc: Juliet Kim <minkim@us.ibm.com>,
Thomas Falcon <tlfalcon@linux.vnet.ibm.com>,
Tyrel Datwyler <tyreld@linux.vnet.ibm.com>,
nathanl@linux.vnet.ibm.com
Subject: Re: [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info
Date: Tue, 29 Jan 2019 10:21:09 -0600 [thread overview]
Message-ID: <f03794ac-eb81-6280-3141-4c07ecd12331@linux.vnet.ibm.com> (raw)
In-Reply-To: <87a7jjstbu.fsf@concordia.ellerman.id.au>
On 1/29/19 3:31 AM, Michael Ellerman wrote:
> Tyrel Datwyler <turtle.in.the.kernel@gmail.com> writes:
>> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>>> Define interface to acquire arch-specific drc info to match against
>>> hotpluggable devices. The current implementation exposes several
>>> pseries-specific dynamic memory properties in generic kernel code.
>>> This patch set provides an interface to pull that code out of the
>>> generic kernel.
>>>
>>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>>> ---
>>> include/linux/topology.h | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>>> index cb0775e..df97f5f 100644
>>> --- a/include/linux/topology.h
>>> +++ b/include/linux/topology.h
>>> @@ -44,6 +44,15 @@
>>
>> As far as I know pseries is the only platform that uses DR connectors, and I
>> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
>> that this is really generic enough to belong in topology.h.
>
> Right. This does not belong in include/linux.
>
>> If anything I would
>> suggest putting this in an include in arch/powerpc/include/ named something like
>> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
>> that want/need to use this functionality.
>
> Yeah that would make more sense.
If you see no objection to referencing a powerpc-specific function from
the code ...
>
> Using "arch" in the name is wrong, it's pseries specific so
> pseries_find_drc_match() would be more appropriate.
>
>>> +int arch_find_drc_match(struct device_node *dn,
>>> + bool (*usercb)(struct device_node *dn,
>>> + u32 drc_index, char *drc_name,
>>> + char *drc_type, u32 drc_power_domain,
>>> + void *data),
>>> + char *opt_drc_type, char *opt_drc_name,
>>> + bool match_drc_index, bool ck_php_type,
>>> + void *data);
>
> This function signature is kind of insane.
>
> You end with calls like:
>
> + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
> + NULL, NULL, false, true, NULL);
>
> Which is impossible to parse.
>
> I feel like maybe this isn't the right level of abstraction.
...
I had already been considering simplifying the interface for these
calls to something like the following:
int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
char *drc_type)
{
return pseries_find_drc_match(dn, drc_type, drc_name);
}
...
int rpaphp_add_slot(struct device_node *dn)
{
if (!dn->name || strcmp(dn->name, "pci"))
return 0;
return pseries_add_drc_slot(dn, rpaphp_add_slot_cb);
}
...
Further details would be hidden within the pseries code.
>
> cheers
Regards
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
next prev parent reply other threads:[~2019-01-29 16:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-14 20:49 [RFC 0/6] powerpc/pseries: Refactor code to centralize drcinfo parsing Michael Bringmann
2018-12-14 20:50 ` [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info Michael Bringmann
2019-01-25 0:04 ` Tyrel Datwyler
2019-01-25 16:09 ` Michael Bringmann
2019-01-28 18:23 ` Michael Bringmann
2019-01-29 9:25 ` Michael Ellerman
2019-01-29 9:31 ` Michael Ellerman
2019-01-29 16:21 ` Michael Bringmann [this message]
2019-01-25 0:10 ` Tyrel Datwyler
2019-01-25 16:11 ` Michael Bringmann
2018-12-14 20:50 ` [RFC 2/6] pseries/drcinfo: Fix bug parsing ibm,drc-info Michael Bringmann
2018-12-14 20:51 ` [RFC 3/6] pseries/drcinfo: Pseries impl of arch_find_drc_info Michael Bringmann
2019-01-25 0:04 ` Tyrel Datwyler
2019-01-25 16:10 ` Michael Bringmann
2018-12-14 20:51 ` [RFC 4/6] powerpc/pseries: Use common drcinfo parsing Michael Bringmann
2018-12-14 20:51 ` [RFC 5/6] powerpc/pci/hotplug: " Michael Bringmann
2019-01-15 0:28 ` Bjorn Helgaas
2019-01-22 19:58 ` Bjorn Helgaas
2019-01-25 0:29 ` Tyrel Datwyler
2019-01-25 16:12 ` Michael Bringmann
2018-12-14 20:51 ` [RFC 6/6] powerpc: Enable support for ibm, drc-info devtree property Michael Bringmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f03794ac-eb81-6280-3141-4c07ecd12331@linux.vnet.ibm.com \
--to=mwb@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=minkim@us.ibm.com \
--cc=mpe@ellerman.id.au \
--cc=nathanl@linux.vnet.ibm.com \
--cc=tlfalcon@linux.vnet.ibm.com \
--cc=turtle.in.the.kernel@gmail.com \
--cc=tyreld@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).