On Tue, Aug 27, 2013 at 2:39 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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@xxxxxxxxx> >> --- >> 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. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
