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 <[email protected]>
> ---
> 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);
regards,
dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
