Re: [PATCH] microblaze: Don't mark arch_kgdb_ops as const.

[Michal Simek]

Hi Graeme,

On 06/23/2013 11:16 PM, Graeme Smecher wrote:
> Hi Michal,
> 
> On 08/06/13 01:58 PM, Graeme Smecher wrote:
>> Hi all,
>>
>> On 08/06/13 09:52 AM, Graeme Smecher wrote:
>>> Other architectures don't do it, and it conflicts with the extern'd definition
>>> in include/linux/kgdb.h.
>>>
>>> Signed-off-by: Graeme Smecher<gsmecher@xxxxxxxxxxxxxxxxxxx>
>>> CC: Michal Simek<monstr@xxxxxxxxx>
>>> CC:linux-kernel@xxxxxxxxxxxxxxx
>>> ---
>>>   arch/microblaze/kernel/kgdb.c |    2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/microblaze/kernel/kgdb.c b/arch/microblaze/kernel/kgdb.c
>>> index 8adc9244..09a5e82 100644
>>> --- a/arch/microblaze/kernel/kgdb.c
>>> +++ b/arch/microblaze/kernel/kgdb.c
>>> @@ -141,7 +141,7 @@ void kgdb_arch_exit(void)
>>>   /*
>>>    * Global data
>>>    */
>>> -const struct kgdb_arch arch_kgdb_ops = {
>>> +struct kgdb_arch arch_kgdb_ops = {
>>>   #ifdef __MICROBLAZEEL__
>>>       .gdb_bpt_instr = {0x18, 0x00, 0x0c, 0xba}, /* brki r16, 0x18 */
>>>   #else
>>
>> It's worth noting that this patch is flagged by checkpatch:
>>
>>    WARNING: struct kgdb_arch should normally be const
>>    #25: FILE: arch/microblaze/kernel/kgdb.c:144:
>>    +struct kgdb_arch arch_kgdb_ops = {
>>
>>    total: 0 errors, 1 warnings, 8 lines checked
>>
>>
>> However, without it, my kernel fails to compile:
>>
>>       CC      arch/microblaze/kernel/kgdb.o
>>    arch/microblaze/kernel/kgdb.c:144:24: error: conflicting type
>>    qualifiers for 'arch_kgdb_ops'
>>    include/linux/kgdb.h:284:26: note: previous declaration of
>>    'arch_kgdb_ops' was here
>>    make[1]: *** [arch/microblaze/kernel/kgdb.o] Error 1
>>    make: *** [arch/microblaze/kernel] Error 2
>>
>>
>> The checkpatch test was a response to this (rejected) patch series:
>>
>> http://lkml.indiana.edu/hypermail/linux/kernel/0912.1/02659.html
>>
>> I'm the lucky victim, here, since I'm using gcc 4.6.4 (which notices the conflicting qualifiers), while other Microblaze users are still working with gcc 4.1.2 (which didn't care.) All other architectures (including mn10300, arc, and hexagon, which were accepted after the checkpatch commit) ignore the checkpatch rule and declare arch_kgdb_ops non-const.
>>
>> In any case, I wanted to point out (a) that the patch fails checkpatch, (b) that I'm submitting it anyway, and (c) that I'm utterly ambivalent about how the struct is declared, provided I can compile working kernels on Microblaze.
> 
> Can you give me an update on including this patch in linux-2.6-microblaze? I realize the "const" was added as a result of a checkpatch.pl complaint (see 6bd55f0bbaebb79b39e147aa864401fd0c94db82), but it seems like a non-compiling kernel (even for trivial reasons) is worse than a noisy Perl script.

Sorry for delay.
Yes, you are right. I will extend description to connect it with that cleanup patch
and will also create follow up to enable kgdb in defconfig to be sure that it is tested regularly.

Applied.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

Attachment: signature.asc
Description: OpenPGP digital signature