From: Nathan Fontenot <nfont@linux•vnet.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: linuxppc-dev@ozlabs•org
Subject: Re: [PATCH v3 1/12] Create a powerpc update_devicetree interface
Date: Tue, 23 Apr 2013 13:46:45 -0500 [thread overview]
Message-ID: <5176D715.4060606@linux.vnet.ibm.com> (raw)
In-Reply-To: <1366676147.2886.2.camel@pasglop>
On 04/22/2013 07:15 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2013-04-22 at 13:30 -0500, Nathan Fontenot wrote:
>
>> This patch exposes a method for updating the device tree via
>> ppc_md.update_devicetree that takes a single 32-bit value as a parameter.
>> For pseries platforms this is the existing pseries_devicetree_update routine
>> which is updated to take the new parameter which is a scope value
>> to indicate the the reason for making the rtas calls. This parameter is
>> required by the ibm,update-nodes/ibm,update-properties RTAS calls, and
>> the appropriate value is contained within the RTAS event for PRRN
>> notifications. In pseries_devicetree_update() it was previously
>> hard-coded to 1, the scope value for partition migration.
>
> I think that's too much abstraction.... (see below)
>
> Also you add this helper:
>
>> Index: powerpc/arch/powerpc/kernel/rtas.c
>> ===================================================================
>> --- powerpc.orig/arch/powerpc/kernel/rtas.c 2013-03-08 19:23:06.000000000 -0600
>> +++ powerpc/arch/powerpc/kernel/rtas.c 2013-04-17 13:02:29.000000000 -0500
>> @@ -1085,3 +1085,13 @@
>> timebase = 0;
>> arch_spin_unlock(&timebase_lock);
>> }
>> +
>> +int update_devicetree(s32 scope)
>> +{
>> + int rc = 0;
>> +
>> + if (ppc_md.update_devicetree)
>> + rc = ppc_md.update_devicetree(scope);
>> +
>> + return rc;
>> +}
>
> But then don't use it afaik (you call directly ppc_md.update_... from
> prrn_work_fn().
>
> In the end, the caller (PRRN stuff), while in rtasd, is really pseries
> specific and the resulting update_device_tree() as well, so I don't
> think we need the ppc_md. hook in the middle with that "oddball" scope
> parameter which is not defined outside of pseries specific areas.
>
> In this case, it might be better to make sure the PRRN related stuff in
> rtasd is inside an ifdef CONFIG_PPC_PSERIES and have it call directly
> into pseries_update_devicetree().
>
> It makes the code somewhat easier to follow and I doubt anybody else
> will ever use that specific hook, at least not in its current form. If
> we need an abstraction later, we can add one then.
>
ok, good. I was not crazy about using ppc_md to do this and if you're fine
with putting the pseries specific stuff in ifdef CONFIG_PPC_PSERIES I'll
update the code to do that.
Question concerning rtas code. There is quite a bit of pseries specific
pieces in the general powerpc/kernel directory. Has there been, or should
there be, any effort to move that to the pseries directory?
-Nathan
next prev parent reply other threads:[~2013-04-23 18:47 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-22 17:54 [PATCH v3 0/12] NUMA CPU Reconfiguration using PRRN Nathan Fontenot
2013-04-22 18:30 ` [PATCH v3 1/12] Create a powerpc update_devicetree interface Nathan Fontenot
2013-04-23 0:15 ` Benjamin Herrenschmidt
2013-04-23 18:46 ` Nathan Fontenot [this message]
2013-04-23 20:54 ` Benjamin Herrenschmidt
2013-04-22 18:31 ` [PATCH v3 2/12] Correct buffer parsing in update-properties Nathan Fontenot
2013-04-22 18:33 ` [PATCH v3 3/12] Add PRRN event handler Nathan Fontenot
2013-04-22 18:35 ` [PATCH v3 4/12] Move architecture vector definitions to prom.h Nathan Fontenot
2013-04-23 0:18 ` Benjamin Herrenschmidt
2013-04-22 18:38 ` [PATCH v3 5/12] Update firmware_has_feature() to check architecture bits Nathan Fontenot
2013-04-23 1:50 ` Stephen Rothwell
2013-04-23 18:56 ` Nathan Fontenot
2013-04-23 1:52 ` Stephen Rothwell
2013-04-22 18:40 ` [PATCH v3 6/12] Update numa.c to use updated firmware_has_feature() Nathan Fontenot
2013-04-22 18:41 ` [PATCH v3 7/12] Use stop machine to update cpu maps Nathan Fontenot
2013-04-23 0:24 ` Benjamin Herrenschmidt
2013-04-23 18:58 ` Nathan Fontenot
2013-04-22 18:44 ` [PATCH v3 8/12] " Nathan Fontenot
2013-04-22 18:45 ` [PATCH v3 9/12] Update NUMA VDSO information Nathan Fontenot
2013-04-22 18:46 ` [PATCH v3 10/12] Re-enable Virtual Private Home Node capabilities Nathan Fontenot
2013-04-22 18:47 ` [PATCH v3 11/12] Enable PRRN Event handling Nathan Fontenot
2013-04-22 18:47 ` [PATCH v3 12/12] Add /proc interface to control topology updates Nathan Fontenot
2013-04-23 2:00 ` Stephen Rothwell
2013-04-23 2:49 ` Michael Ellerman
2013-04-23 18:59 ` Nathan Fontenot
2013-04-23 2:02 ` Stephen Rothwell
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=5176D715.4060606@linux.vnet.ibm.com \
--to=nfont@linux$(echo .)vnet.ibm.com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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