[PATCH] staging: dgnc: tty.c: updates uart_struct declaration for sparse

Lidza Louina lidza.louina at gmail.com
Tue Aug 27 20:46:23 UTC 2013


On Tue, Aug 27, 2013 at 2:39 PM, Dan Carpenter <dan.carpenter at oracle.com> wrote:
> Could you redo this one?
>
> On Tue, Aug 27, 2013 at 11:12:16AM -0400, Lidza Louina wrote:
>> This patch edits the type casts neo_uart_struct and
>> cls_uart_struct. A previous patch added the marker __iomem
>> to these structs. This patch ensures that the change to
>> the marker is consistent. This also removes these
>> sparse warnings:
>>
>> warning: incorrect type in assignment (different address spaces)
>>   expected struct neo_uart_struct [noderef] <asn:2>*ch_neo_uart
>>   got struct neo_uart_struct *<noident>
>> warning: incorrect type in assignment (different address spaces)
>>   expected struct cls_uart_struct [noderef] <asn:2>*ch_cls_uart
>>   got struct cls_uart_struct *<noident>
>>
>> Signed-off-by: Lidza Louina <lidza.louina at gmail.com>
>> ---
>>  drivers/staging/dgnc/dgnc_tty.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
>> index a54b829..a36fae8 100644
>> --- a/drivers/staging/dgnc/dgnc_tty.c
>> +++ b/drivers/staging/dgnc/dgnc_tty.c
>> @@ -403,9 +403,9 @@ int dgnc_tty_init(struct board_t *brd)
>>               ch->ch_pun.un_dev = i + 128;
>>
>>               if (brd->bd_uart_offset == 0x200)
>> -                     ch->ch_neo_uart = (struct neo_uart_struct *) ((ulong) vaddr + (brd->bd_uart_offset * i));
>> +                     ch->ch_neo_uart = (struct __iomem neo_uart_struct *) ((ulong) vaddr + (brd->bd_uart_offset * i));
>                                                                             ^        ^
>
> Don't put spaces here.  Leaving the space out helps the reader remember
> that casting is a high priority operation in C.
>
> Casting should be relatively uncommon in good code.  If you see it that
> means something complicated is going on and it's a red flag.  Here is
> how I imagine they came up with that code:
>
> Code #1:
>         ch->ch_neo_uart = vaddr + (brd->bd_uart_offset * i);
> Problem:  Compile warning.
>
> Code #2:
>         ch->ch_neo_uart = (struct __iomem neo_uart_struct *) vaddr + (brd->bd_uart_offset * i);
> Problem:  The pointer math doesn't work, because they forgot casting has
>           high precedence.
>
> Code #3:
>         ch->ch_neo_uart = (struct __iomem neo_uart_struct *) (ulong) vaddr + (brd->bd_uart_offset * i);
> Problem:  The pointer math doesn't work, because they forgot casting has
>           still has high precedence.  If only they had followed the no
>           spaces with casting rule, it would have been obvious.  :)
>
> Code #4:
>         ch->ch_neo_uart = (struct __iomem neo_uart_struct *) ((ulong) vaddr + (brd->bd_uart_offset * i));
> Problem:  It works but it's ugly as pants.
>
> Anyway, just declare vaddr like this:
>
>         void __iomem *vaddr;
>
> And remove all the casting:
>
>         ch->ch_neo_uart = vaddr + (brd->bd_uart_offset * i);

Alrighty, thank you. There are a lot of type casts in
this code. I'll work on them.


More information about the devel mailing list