Re: [PATCH v3 1/3] powerpc/nvram: move generic code for nvram and pstore

[Hari Bathini <hbathini@linux.vnet.ibm.com>]

On 01/15/2015 03:58 AM, Michael Ellerman wrote:
> On Wed, 2015-01-14 at 23:35 +0530, Hari Bathini wrote:
>> On 01/14/2015 10:01 AM, Michael Ellerman wrote:
>>
>>> On Wed, 2014-12-24 at 17:28 +0530, Hari Bathini wrote:
>>>> With minor checks, we can move most of the code for nvram
>>>> under pseries to a common place to be re-used by other
>>>> powerpc platforms like powernv. This patch moves such
>>>> common code to arch/powerpc/kernel/nvram_64.c file.
>>> As I said in my reply to the previous version:
>>>
>>>      ... you need to keep in mind that it is very common for us to build kernels
>>>      with both POWERNV=y and PSERIES=y.
>>>      
>>>      So you need to make sure you're only using CONFIG_PPC_PSERIES to protect things
>>>      that are optional on pseries. Not things that we *shouldn't* be doing on
>>>      powernv.
>> we could as well do away with the PPC_PSERIES flag in a couple of
>> places in
>> arch/powerpc/kernel/nvram_64.c, but doing that will unnecessarily add
>> few extra variables for !PPC_PSERIES case.
> Yep. I'm happy for them to be there, I just want you to explain in the
> changelog that you've thought about the PSERIES=y POWERNV=y case and why the
> code makes sense for that configuration.
>
>>> Please explain in your commit message how you have dealt with that.
>> Sure. Will update the changelog
> Thanks.
>   
>>> Also, you broke the build for every config that doesn't have
>>> CONFIG_PPC_PSERIES, all 95 of them. This is pasemi_defconfig for example:
>> My bad! clobbering_unread_rtas_event should have been static inline
>> while defining under !PPC_PSERIES
> Correct.
>
> Please make sure you test build at least some of the other configurations in
> future. I realise it's too time consuming to build all of them, but ideally for
> every config symbol you use in your patch you need to build a kernel config
> where that symbol =y and =n (and =m if it's tristate).

Sure, Michael. I will keep this in mind :)

Thanks
Hari

> cheers
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev