2008-10-13 09:09:48

by Bryan Wu

[permalink] [raw]
Subject: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

From: Javier Herrero <[email protected]>

Signed-off-by: Javier Herrero <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/net/dm9000.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
index f42c23f..c4737ca 100644
--- a/drivers/net/dm9000.c
+++ b/drivers/net/dm9000.c
@@ -188,35 +188,35 @@ iow(board_info_t * db, int reg, int value)

static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
{
- writesb(reg, data, count);
+ writesb((int)reg, data, count);
}

static void dm9000_outblk_16bit(void __iomem *reg, void *data, int count)
{
- writesw(reg, data, (count+1) >> 1);
+ writesw((int)reg, data, (count+1) >> 1);
}

static void dm9000_outblk_32bit(void __iomem *reg, void *data, int count)
{
- writesl(reg, data, (count+3) >> 2);
+ writesl((int)reg, data, (count+3) >> 2);
}

/* input block from chip to memory */

static void dm9000_inblk_8bit(void __iomem *reg, void *data, int count)
{
- readsb(reg, data, count);
+ readsb((int)reg, data, count);
}


static void dm9000_inblk_16bit(void __iomem *reg, void *data, int count)
{
- readsw(reg, data, (count+1) >> 1);
+ readsw((int)reg, data, (count+1) >> 1);
}

static void dm9000_inblk_32bit(void __iomem *reg, void *data, int count)
{
- readsl(reg, data, (count+3) >> 2);
+ readsl((int)reg, data, (count+3) >> 2);
}

/* dump block from chip to null */
--
1.5.6


2008-10-13 09:17:50

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, Oct 13, 2008 at 05:10:09PM +0800, Bryan Wu wrote:
> From: Javier Herrero <[email protected]>

No, your arch definitions of writesb and co are wrong if they
are not taking 'void __iomem *' arguments.

> Signed-off-by: Javier Herrero <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
> ---
> drivers/net/dm9000.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
> index f42c23f..c4737ca 100644
> --- a/drivers/net/dm9000.c
> +++ b/drivers/net/dm9000.c
> @@ -188,35 +188,35 @@ iow(board_info_t * db, int reg, int value)
>
> static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
> {
> - writesb(reg, data, count);
> + writesb((int)reg, data, count);
> }
>
> static void dm9000_outblk_16bit(void __iomem *reg, void *data, int count)
> {
> - writesw(reg, data, (count+1) >> 1);
> + writesw((int)reg, data, (count+1) >> 1);
> }
>
> static void dm9000_outblk_32bit(void __iomem *reg, void *data, int count)
> {
> - writesl(reg, data, (count+3) >> 2);
> + writesl((int)reg, data, (count+3) >> 2);
> }
>
> /* input block from chip to memory */
>
> static void dm9000_inblk_8bit(void __iomem *reg, void *data, int count)
> {
> - readsb(reg, data, count);
> + readsb((int)reg, data, count);
> }
>
>
> static void dm9000_inblk_16bit(void __iomem *reg, void *data, int count)
> {
> - readsw(reg, data, (count+1) >> 1);
> + readsw((int)reg, data, (count+1) >> 1);
> }
>
> static void dm9000_inblk_32bit(void __iomem *reg, void *data, int count)
> {
> - readsl(reg, data, (count+3) >> 2);
> + readsl((int)reg, data, (count+3) >> 2);
> }
>
> /* dump block from chip to null */
> --
> 1.5.6

--
--
Ben

Q: What's a light-year?
A: One-third less calories than a regular year.

2008-10-13 09:39:33

by Javier Herrero

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

Hi, Bryan,

So it seems that we must correct that in other place?

Regards,

Javier

Ben Dooks escribi?:
> On Mon, Oct 13, 2008 at 05:10:09PM +0800, Bryan Wu wrote:
>> From: Javier Herrero <[email protected]>
>
> No, your arch definitions of writesb and co are wrong if they
> are not taking 'void __iomem *' arguments.
>
>> Signed-off-by: Javier Herrero <[email protected]>
>> Signed-off-by: Bryan Wu <[email protected]>
>> ---
>> drivers/net/dm9000.c | 12 ++++++------
>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
>> index f42c23f..c4737ca 100644
>> --- a/drivers/net/dm9000.c
>> +++ b/drivers/net/dm9000.c
>> @@ -188,35 +188,35 @@ iow(board_info_t * db, int reg, int value)
>>
>> static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
>> {
>> - writesb(reg, data, count);
>> + writesb((int)reg, data, count);
>> }
>>
>> static void dm9000_outblk_16bit(void __iomem *reg, void *data, int count)
>> {
>> - writesw(reg, data, (count+1) >> 1);
>> + writesw((int)reg, data, (count+1) >> 1);
>> }
>>
>> static void dm9000_outblk_32bit(void __iomem *reg, void *data, int count)
>> {
>> - writesl(reg, data, (count+3) >> 2);
>> + writesl((int)reg, data, (count+3) >> 2);
>> }
>>
>> /* input block from chip to memory */
>>
>> static void dm9000_inblk_8bit(void __iomem *reg, void *data, int count)
>> {
>> - readsb(reg, data, count);
>> + readsb((int)reg, data, count);
>> }
>>
>>
>> static void dm9000_inblk_16bit(void __iomem *reg, void *data, int count)
>> {
>> - readsw(reg, data, (count+1) >> 1);
>> + readsw((int)reg, data, (count+1) >> 1);
>> }
>>
>> static void dm9000_inblk_32bit(void __iomem *reg, void *data, int count)
>> {
>> - readsl(reg, data, (count+3) >> 2);
>> + readsl((int)reg, data, (count+3) >> 2);
>> }
>>
>> /* dump block from chip to null */
>> --
>> 1.5.6
>

--
------------------------------------------------------------------------
Javier Herrero EMAIL: [email protected]
HV Sistemas S.L. PHONE: +34 949 336 806
Los Charcones, 17A FAX: +34 949 336 792
19170 El Casar - Guadalajara - Spain WEB: http://www.hvsistemas.com

2008-10-13 10:10:47

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, Oct 13, 2008 at 5:17 PM, Ben Dooks <[email protected]> wrote:
> On Mon, Oct 13, 2008 at 05:10:09PM +0800, Bryan Wu wrote:
>> From: Javier Herrero <[email protected]>
>
> No, your arch definitions of writesb and co are wrong if they
> are not taking 'void __iomem *' arguments.
>

Right, we should check the I/O functions and make them support "void
__iomem *" arguments.

Thanks
-Bryan

>> Signed-off-by: Javier Herrero <[email protected]>
>> Signed-off-by: Bryan Wu <[email protected]>
>> ---
>> drivers/net/dm9000.c | 12 ++++++------
>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
>> index f42c23f..c4737ca 100644
>> --- a/drivers/net/dm9000.c
>> +++ b/drivers/net/dm9000.c
>> @@ -188,35 +188,35 @@ iow(board_info_t * db, int reg, int value)
>>
>> static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
>> {
>> - writesb(reg, data, count);
>> + writesb((int)reg, data, count);
>> }
>>
>> static void dm9000_outblk_16bit(void __iomem *reg, void *data, int count)
>> {
>> - writesw(reg, data, (count+1) >> 1);
>> + writesw((int)reg, data, (count+1) >> 1);
>> }
>>
>> static void dm9000_outblk_32bit(void __iomem *reg, void *data, int count)
>> {
>> - writesl(reg, data, (count+3) >> 2);
>> + writesl((int)reg, data, (count+3) >> 2);
>> }
>>
>> /* input block from chip to memory */
>>
>> static void dm9000_inblk_8bit(void __iomem *reg, void *data, int count)
>> {
>> - readsb(reg, data, count);
>> + readsb((int)reg, data, count);
>> }
>>
>>
>> static void dm9000_inblk_16bit(void __iomem *reg, void *data, int count)
>> {
>> - readsw(reg, data, (count+1) >> 1);
>> + readsw((int)reg, data, (count+1) >> 1);
>> }
>>
>> static void dm9000_inblk_32bit(void __iomem *reg, void *data, int count)
>> {
>> - readsl(reg, data, (count+3) >> 2);
>> + readsl((int)reg, data, (count+3) >> 2);
>> }
>>
>> /* dump block from chip to null */
>> --
>> 1.5.6
>
> --
> --
> Ben
>
> Q: What's a light-year?
> A: One-third less calories than a regular year.
>
>

2008-10-13 10:11:46

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, Oct 13, 2008 at 5:28 PM, Javier Herrero <[email protected]> wrote:
> Hi, Bryan,
>
> So it seems that we must correct that in other place?
>

Yes, we need to check the I/O read_write functions.

-Bryan

> Regards,
>
> Javier
>
> Ben Dooks escribi?:
>>
>> On Mon, Oct 13, 2008 at 05:10:09PM +0800, Bryan Wu wrote:
>>>
>>> From: Javier Herrero <[email protected]>
>>
>> No, your arch definitions of writesb and co are wrong if they
>> are not taking 'void __iomem *' arguments.
>>
>>>
>>> Signed-off-by: Javier Herrero <[email protected]>
>>> Signed-off-by: Bryan Wu <[email protected]>
>>> ---
>>> drivers/net/dm9000.c | 12 ++++++------
>>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
>>> index f42c23f..c4737ca 100644
>>> --- a/drivers/net/dm9000.c
>>> +++ b/drivers/net/dm9000.c
>>> @@ -188,35 +188,35 @@ iow(board_info_t * db, int reg, int value)
>>> static void dm9000_outblk_8bit(void __iomem *reg, void *data, int
>>> count)
>>> {
>>> - writesb(reg, data, count);
>>> + writesb((int)reg, data, count);
>>> }
>>> static void dm9000_outblk_16bit(void __iomem *reg, void *data, int
>>> count)
>>> {
>>> - writesw(reg, data, (count+1) >> 1);
>>> + writesw((int)reg, data, (count+1) >> 1);
>>> }
>>> static void dm9000_outblk_32bit(void __iomem *reg, void *data, int
>>> count)
>>> {
>>> - writesl(reg, data, (count+3) >> 2);
>>> + writesl((int)reg, data, (count+3) >> 2);
>>> }
>>> /* input block from chip to memory */
>>> static void dm9000_inblk_8bit(void __iomem *reg, void *data, int count)
>>> {
>>> - readsb(reg, data, count);
>>> + readsb((int)reg, data, count);
>>> }
>>> static void dm9000_inblk_16bit(void __iomem *reg, void *data, int
>>> count)
>>> {
>>> - readsw(reg, data, (count+1) >> 1);
>>> + readsw((int)reg, data, (count+1) >> 1);
>>> }
>>> static void dm9000_inblk_32bit(void __iomem *reg, void *data, int
>>> count)
>>> {
>>> - readsl(reg, data, (count+3) >> 2);
>>> + readsl((int)reg, data, (count+3) >> 2);
>>> }
>>> /* dump block from chip to null */
>>> --
>>> 1.5.6
>>
>
> --
> ------------------------------------------------------------------------
> Javier Herrero EMAIL: [email protected]
> HV Sistemas S.L. PHONE: +34 949 336 806
> Los Charcones, 17A FAX: +34 949 336 792
> 19170 El Casar - Guadalajara - Spain WEB: http://www.hvsistemas.com
>
>

2008-10-13 10:14:32

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

From: Bryan Wu <[email protected]>
Date: Mon, 13 Oct 2008 17:10:09 +0800

> From: Javier Herrero <[email protected]>
>
> Signed-off-by: Javier Herrero <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>

This is not only inelegant, it'll likely break things on
64-bit MIPS or any other case where the upper 32-bit matter.

I'd rather see Blackfin fix it's I/O accessor prototypes
to take a proper __iomem pointer.

This patch won't be applied.

2008-10-13 10:24:49

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, Oct 13, 2008 at 05:52, Bryan Wu wrote:
> On Mon, Oct 13, 2008 at 5:28 PM, Javier Herrero wrote:
>> So it seems that we must correct that in other place?
>
> Yes, we need to check the I/O read_write functions.

and drop the CONFIG_BLACKFIN hacks in the dm90000 driver ... we
rewrite the write* functions to out* ... probably because we dont
provide any write* functions in our headers
-mike

2008-10-13 10:27:29

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, Oct 13, 2008 at 6:24 PM, Mike Frysinger <[email protected]> wrote:
> On Mon, Oct 13, 2008 at 05:52, Bryan Wu wrote:
>> On Mon, Oct 13, 2008 at 5:28 PM, Javier Herrero wrote:
>>> So it seems that we must correct that in other place?
>>
>> Yes, we need to check the I/O read_write functions.
>
> and drop the CONFIG_BLACKFIN hacks in the dm90000 driver ... we
> rewrite the write* functions to out* ... probably because we dont
> provide any write* functions in our headers
> -mike
>

No problem, I will take care of that.

-Bryan

2008-10-13 12:41:35

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, 13 Oct 2008 17:10:09 +0800
Bryan Wu <[email protected]> wrote:

> From: Javier Herrero <[email protected]>
>
> Signed-off-by: Javier Herrero <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
> ---
> drivers/net/dm9000.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
> index f42c23f..c4737ca 100644
> --- a/drivers/net/dm9000.c
> +++ b/drivers/net/dm9000.c
> @@ -188,35 +188,35 @@ iow(board_info_t * db, int reg, int value)
>
> static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
> {
> - writesb(reg, data, count);
> + writesb((int)reg, data, count);
> }

You lose the sparse checking for misuse by doing this.

Why not this instead?


--- a/drivers/net/dm9000.c 2008-10-13 05:29:15.000000000 -0700
+++ b/drivers/net/dm9000.c 2008-10-13 05:40:13.000000000 -0700
@@ -47,14 +47,6 @@
#define CARDNAME "dm9000"
#define DRV_VERSION "1.31"

-#ifdef CONFIG_BLACKFIN
-#define readsb insb
-#define readsw insw
-#define readsl insl
-#define writesb outsb
-#define writesw outsw
-#define writesl outsl
-#endif

/*
* Transmit timeout, default 5 seconds.
@@ -185,7 +177,40 @@ iow(board_info_t * db, int reg, int valu
}

/* routines for sending block to chip */
+#ifdef CONFIG_BLACKFIN
+static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
+{
+ outsb((unsigned long)reg, data, count);
+}

+static void dm9000_outblk_16bit(void __iomem *reg, void *data, int count)
+{
+ outsw((unsigned long)reg, data, (count+1) >> 1);
+}
+
+static void dm9000_outblk_32bit(void __iomem *reg, void *data, int count)
+{
+ outsl((unsigned long)reg, data, (count+3) >> 2);
+}
+
+/* input block from chip to memory */
+
+static void dm9000_inblk_8bit(void __iomem *reg, void *data, int count)
+{
+ insb((unsigned long)reg, data, count);
+}
+
+
+static void dm9000_inblk_16bit(void __iomem *reg, void *data, int count)
+{
+ insw((unsigned long)reg, data, (count+1) >> 1);
+}
+
+static void dm9000_inblk_32bit(void __iomem *reg, void *data, int count)
+{
+ insl((unsigned long)reg, data, (count+3) >> 2);
+}
+#else
static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
{
writesb(reg, data, count);
@@ -218,6 +243,7 @@ static void dm9000_inblk_32bit(void __io
{
readsl(reg, data, (count+3) >> 2);
}
+#endif

/* dump block from chip to null */

2008-10-13 23:19:07

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, Oct 13, 2008 at 03:01:50AM -0700, David Miller wrote:
> From: Bryan Wu <[email protected]>
> Date: Mon, 13 Oct 2008 17:10:09 +0800
>
> > From: Javier Herrero <[email protected]>
> >
> > Signed-off-by: Javier Herrero <[email protected]>
> > Signed-off-by: Bryan Wu <[email protected]>
>
> This is not only inelegant, it'll likely break things on
> 64-bit MIPS or any other case where the upper 32-bit matter.
>
> I'd rather see Blackfin fix it's I/O accessor prototypes
> to take a proper __iomem pointer.

How much would it hurt to provide blackfin wide
readsb and friends with the correct prototype and
remove the nasty blackfin hack in the dm9000.c driver
for good?

--
Ben

Q: What's a light-year?
A: One-third less calories than a regular year.

2008-10-14 08:50:36

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH 1/1] netdev: DM9000: Added typecasting to supress some warnings on Blackfin

On Mon, Oct 13, 2008 at 8:41 PM, Stephen Hemminger
<[email protected]> wrote:
> On Mon, 13 Oct 2008 17:10:09 +0800
> Bryan Wu <[email protected]> wrote:
>
>> From: Javier Herrero <[email protected]>
>>
>> Signed-off-by: Javier Herrero <[email protected]>
>> Signed-off-by: Bryan Wu <[email protected]>
>> ---
>> drivers/net/dm9000.c | 12 ++++++------
>> 1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
>> index f42c23f..c4737ca 100644
>> --- a/drivers/net/dm9000.c
>> +++ b/drivers/net/dm9000.c
>> @@ -188,35 +188,35 @@ iow(board_info_t * db, int reg, int value)
>>
>> static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
>> {
>> - writesb(reg, data, count);
>> + writesb((int)reg, data, count);
>> }
>
> You lose the sparse checking for misuse by doing this.
>
> Why not this instead?
>
>
> --- a/drivers/net/dm9000.c 2008-10-13 05:29:15.000000000 -0700
> +++ b/drivers/net/dm9000.c 2008-10-13 05:40:13.000000000 -0700
> @@ -47,14 +47,6 @@
> #define CARDNAME "dm9000"
> #define DRV_VERSION "1.31"
>
> -#ifdef CONFIG_BLACKFIN
> -#define readsb insb
> -#define readsw insw
> -#define readsl insl
> -#define writesb outsb
> -#define writesw outsw
> -#define writesl outsl
> -#endif
>
> /*
> * Transmit timeout, default 5 seconds.
> @@ -185,7 +177,40 @@ iow(board_info_t * db, int reg, int valu
> }
>
> /* routines for sending block to chip */
> +#ifdef CONFIG_BLACKFIN
> +static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
> +{
> + outsb((unsigned long)reg, data, count);
> +}
>

I added readsl/writesl to blackfin arch, so we don't need to mess up
the common DM9000 netdev driver here.

-Bryan

> +static void dm9000_outblk_16bit(void __iomem *reg, void *data, int count)
> +{
> + outsw((unsigned long)reg, data, (count+1) >> 1);
> +}
> +
> +static void dm9000_outblk_32bit(void __iomem *reg, void *data, int count)
> +{
> + outsl((unsigned long)reg, data, (count+3) >> 2);
> +}
> +
> +/* input block from chip to memory */
> +
> +static void dm9000_inblk_8bit(void __iomem *reg, void *data, int count)
> +{
> + insb((unsigned long)reg, data, count);
> +}
> +
> +
> +static void dm9000_inblk_16bit(void __iomem *reg, void *data, int count)
> +{
> + insw((unsigned long)reg, data, (count+1) >> 1);
> +}
> +
> +static void dm9000_inblk_32bit(void __iomem *reg, void *data, int count)
> +{
> + insl((unsigned long)reg, data, (count+3) >> 2);
> +}
> +#else
> static void dm9000_outblk_8bit(void __iomem *reg, void *data, int count)
> {
> writesb(reg, data, count);
> @@ -218,6 +243,7 @@ static void dm9000_inblk_32bit(void __io
> {
> readsl(reg, data, (count+3) >> 2);
> }
> +#endif
>
> /* dump block from chip to null */
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>