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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
