All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
@ 2012-10-02 13:13 Andreas Larsson
  2012-10-04 12:49 ` Marc Kleine-Budde
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Larsson @ 2012-10-02 13:13 UTC (permalink / raw)
  To: linux-can; +Cc: software

Signed-off-by: Andreas Larsson <andreas@gaisler.com>
---
 drivers/net/can/sja1000/Kconfig               |    2 +-
 drivers/net/can/sja1000/sja1000_of_platform.c |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index 03df9a8..44abee4 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -21,7 +21,7 @@ config CAN_SJA1000_PLATFORM
 
 config CAN_SJA1000_OF_PLATFORM
 	tristate "Generic OF Platform Bus based SJA1000 driver"
-	depends on PPC_OF
+	depends on OF
 	---help---
 	  This driver adds support for the SJA1000 chips connected to
 	  the OpenFirmware "platform bus" found on embedded systems with
diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
index f2683eb..73d964e 100644
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ b/drivers/net/can/sja1000/sja1000_of_platform.c
@@ -42,6 +42,8 @@
 #include <linux/can/dev.h>
 
 #include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <asm/prom.h>
 
 #include "sja1000.h"
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-02 13:13 [PATCH] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC Andreas Larsson
@ 2012-10-04 12:49 ` Marc Kleine-Budde
  2012-10-04 13:43   ` Andreas Larsson
  0 siblings, 1 reply; 12+ messages in thread
From: Marc Kleine-Budde @ 2012-10-04 12:49 UTC (permalink / raw)
  To: Andreas Larsson; +Cc: linux-can, software

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

On 10/02/2012 03:13 PM, Andreas Larsson wrote:
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

On sparc32 (and others) this fails to compile with:

> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_read_reg':
> linux/drivers/net/can/sja1000/sja1000_of_platform.c:64:2: error: implicit declaration of function 'in_8' [-Werror=implicit-function-declaration]
> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_write_reg':
> linux/drivers/net/can/sja1000/sja1000_of_platform.c:70:2: error: implicit declaration of function 'out_8' [-Werror=implicit-function-declaration]

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-04 12:49 ` Marc Kleine-Budde
@ 2012-10-04 13:43   ` Andreas Larsson
  2012-10-04 13:59     ` [PATCH v2] " Andreas Larsson
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Larsson @ 2012-10-04 13:43 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can, software

On 10/04/2012 02:49 PM, Marc Kleine-Budde wrote:
> On 10/02/2012 03:13 PM, Andreas Larsson wrote:
>> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
>
> On sparc32 (and others) this fails to compile with:
>
>> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_read_reg':
>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:64:2: error: implicit declaration of function 'in_8' [-Werror=implicit-function-declaration]
>> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_write_reg':
>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:70:2: error: implicit declaration of function 'out_8' [-Werror=implicit-function-declaration]

Oh, right, sorry about that. Thank's for the input. I was testing on top 
of a local patch providing that for sparc. I'll send a new patch 
changing the I/O functions to ioread8 and iowrite8.

Cheers,
Andreas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-04 13:43   ` Andreas Larsson
@ 2012-10-04 13:59     ` Andreas Larsson
  2012-10-04 14:42       ` Marc Kleine-Budde
  2012-11-12 17:52       ` Andreas Larsson
  0 siblings, 2 replies; 12+ messages in thread
From: Andreas Larsson @ 2012-10-04 13:59 UTC (permalink / raw)
  To: linux-can; +Cc: software

Signed-off-by: Andreas Larsson <andreas@gaisler.com>
---
 drivers/net/can/sja1000/Kconfig               |    2 +-
 drivers/net/can/sja1000/sja1000_of_platform.c |    6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index 03df9a8..44abee4 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -21,7 +21,7 @@ config CAN_SJA1000_PLATFORM
 
 config CAN_SJA1000_OF_PLATFORM
 	tristate "Generic OF Platform Bus based SJA1000 driver"
-	depends on PPC_OF
+	depends on OF
 	---help---
 	  This driver adds support for the SJA1000 chips connected to
 	  the OpenFirmware "platform bus" found on embedded systems with
diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
index f2683eb..e45258d 100644
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ b/drivers/net/can/sja1000/sja1000_of_platform.c
@@ -42,6 +42,8 @@
 #include <linux/can/dev.h>
 
 #include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <asm/prom.h>
 
 #include "sja1000.h"
@@ -59,13 +61,13 @@ MODULE_LICENSE("GPL v2");
 
 static u8 sja1000_ofp_read_reg(const struct sja1000_priv *priv, int reg)
 {
-	return in_8(priv->reg_base + reg);
+	return ioread8(priv->reg_base + reg);
 }
 
 static void sja1000_ofp_write_reg(const struct sja1000_priv *priv,
 				  int reg, u8 val)
 {
-	out_8(priv->reg_base + reg, val);
+	iowrite8(val, priv->reg_base + reg);
 }
 
 static int __devexit sja1000_ofp_remove(struct platform_device *ofdev)
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-04 13:59     ` [PATCH v2] " Andreas Larsson
@ 2012-10-04 14:42       ` Marc Kleine-Budde
  2012-10-04 15:12         ` Marc Kleine-Budde
  2012-11-12 17:52       ` Andreas Larsson
  1 sibling, 1 reply; 12+ messages in thread
From: Marc Kleine-Budde @ 2012-10-04 14:42 UTC (permalink / raw)
  To: Andreas Larsson; +Cc: linux-can, software

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

On 10/04/2012 03:59 PM, Andreas Larsson wrote:
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

Looks better. However on sparc and sparc64, I'm seeing this warning.
From my point of view a false positive.

> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_remove':
> linux/include/linux/ioport.h:165:18: warning: 'res.end' is used uninitialized in this function [-Wuninitialized]
> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note: 'res.end' was declared here
> linux/include/linux/ioport.h:165:2: warning: 'res.start' is used uninitialized in this function [-Wuninitialized]
> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note: 'res.start' was declared here

I'm using these compilers:
http://kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/x86_64-gcc-4.6.3-nolibc_sparc-linux.tar.xz
http://kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/x86_64-gcc-4.6.3-nolibc_sparc64-linux.tar.xz

Can you please compile your kernel with these and test if this works:

    while true; do rmmod sja1000_of_platform; modprobe sja1000_of_platform; done

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-04 14:42       ` Marc Kleine-Budde
@ 2012-10-04 15:12         ` Marc Kleine-Budde
  2012-10-05  9:01           ` Andreas Larsson
  0 siblings, 1 reply; 12+ messages in thread
From: Marc Kleine-Budde @ 2012-10-04 15:12 UTC (permalink / raw)
  To: Andreas Larsson; +Cc: linux-can, software

[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]

On 10/04/2012 04:42 PM, Marc Kleine-Budde wrote:
> On 10/04/2012 03:59 PM, Andreas Larsson wrote:
>> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
> 
> Looks better. However on sparc and sparc64, I'm seeing this warning.
> From my point of view a false positive.
> 
>> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_remove':
>> linux/include/linux/ioport.h:165:18: warning: 'res.end' is used uninitialized in this function [-Wuninitialized]
>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note: 'res.end' was declared here
>> linux/include/linux/ioport.h:165:2: warning: 'res.start' is used uninitialized in this function [-Wuninitialized]
>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note: 'res.start' was declared here

Looking at the preprocessed code, the compiler is right:

> # 30 "/home/frogger/pengutronix/socketcan/linux/include/linux/of_address.h"
> static inline __attribute__((always_inline)) __attribute__((no_instrument_function)) int of_address_to_resource(struct device_node *dev, int index,
>       struct resource *r)
> {
>  return -22;
> }

http://lxr.free-electrons.com/source/include/linux/of_address.h#L29

Why is CONFIG_OF_ADDRESS not set in my .config?

http://lxr.free-electrons.com/source/drivers/of/Kconfig#L43

What a U+1F4A9,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-04 15:12         ` Marc Kleine-Budde
@ 2012-10-05  9:01           ` Andreas Larsson
  2012-10-05  9:05             ` Marc Kleine-Budde
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Larsson @ 2012-10-05  9:01 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can, software

On 10/04/2012 05:12 PM, Marc Kleine-Budde wrote:
> On 10/04/2012 04:42 PM, Marc Kleine-Budde wrote:
>> On 10/04/2012 03:59 PM, Andreas Larsson wrote:
>>> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
>>
>> Looks better. However on sparc and sparc64, I'm seeing this warning.
>>  From my point of view a false positive.
>>
>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_remove':
>>> linux/include/linux/ioport.h:165:18: warning: 'res.end' is used uninitialized in this function [-Wuninitialized]
>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note: 'res.end' was declared here
>>> linux/include/linux/ioport.h:165:2: warning: 'res.start' is used uninitialized in this function [-Wuninitialized]
>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note: 'res.start' was declared here
>
> Looking at the preprocessed code, the compiler is right:
>
>> # 30 "/home/frogger/pengutronix/socketcan/linux/include/linux/of_address.h"
>> static inline __attribute__((always_inline)) __attribute__((no_instrument_function)) int of_address_to_resource(struct device_node *dev, int index,
>>        struct resource *r)
>> {
>>   return -22;
>> }
>
> http://lxr.free-electrons.com/source/include/linux/of_address.h#L29
>
> Why is CONFIG_OF_ADDRESS not set in my .config?

In drivers/of/Kconfig, OF_ADDRESS depends on !SPARC as sparc handles a 
bunch of open firmware things differently. There is a sparc-specific 
implementations of of_address_to_resource.

I'll send in a patch changing of_address.h so that 
of_address_to_resource gets declared extern for sparc instead of an 
empty inline function. That should solve the problem as far as I see it.

There is a similar thing with CONFIG_OF_IRQ, of_irq.h and 
irq_of_parse_and_map, but there irq_of_parse_and_map is declared outside 
of #ifdef CONFIG_OF_IRQ because of the sparc specific implementation.

Cheers,
Andreas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-05  9:01           ` Andreas Larsson
@ 2012-10-05  9:05             ` Marc Kleine-Budde
  2012-10-05  9:16               ` Andreas Larsson
  0 siblings, 1 reply; 12+ messages in thread
From: Marc Kleine-Budde @ 2012-10-05  9:05 UTC (permalink / raw)
  To: Andreas Larsson; +Cc: linux-can, software

[-- Attachment #1: Type: text/plain, Size: 2705 bytes --]

On 10/05/2012 11:01 AM, Andreas Larsson wrote:
> On 10/04/2012 05:12 PM, Marc Kleine-Budde wrote:
>> On 10/04/2012 04:42 PM, Marc Kleine-Budde wrote:
>>> On 10/04/2012 03:59 PM, Andreas Larsson wrote:
>>>> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
>>>
>>> Looks better. However on sparc and sparc64, I'm seeing this warning.
>>>  From my point of view a false positive.
>>>
>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function
>>>> 'sja1000_ofp_remove':
>>>> linux/include/linux/ioport.h:165:18: warning: 'res.end' is used
>>>> uninitialized in this function [-Wuninitialized]
>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note:
>>>> 'res.end' was declared here
>>>> linux/include/linux/ioport.h:165:2: warning: 'res.start' is used
>>>> uninitialized in this function [-Wuninitialized]
>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note:
>>>> 'res.start' was declared here
>>
>> Looking at the preprocessed code, the compiler is right:
>>
>>> # 30
>>> "/home/frogger/pengutronix/socketcan/linux/include/linux/of_address.h"
>>> static inline __attribute__((always_inline))
>>> __attribute__((no_instrument_function)) int
>>> of_address_to_resource(struct device_node *dev, int index,
>>>        struct resource *r)
>>> {
>>>   return -22;
>>> }
>>
>> http://lxr.free-electrons.com/source/include/linux/of_address.h#L29
>>
>> Why is CONFIG_OF_ADDRESS not set in my .config?
> 
> In drivers/of/Kconfig, OF_ADDRESS depends on !SPARC as sparc handles a
> bunch of open firmware things differently. There is a sparc-specific
> implementations of of_address_to_resource.
> 
> I'll send in a patch changing of_address.h so that
> of_address_to_resource gets declared extern for sparc instead of an
> empty inline function. That should solve the problem as far as I see it.
> 
> There is a similar thing with CONFIG_OF_IRQ, of_irq.h and
> irq_of_parse_and_map, but there irq_of_parse_and_map is declared outside
> of #ifdef CONFIG_OF_IRQ because of the sparc specific implementation.

I've noticed that, too. With the patch you posted the driver cannot
work, as the header files provide just no-ops of the of functions.
Please test your patch before posting :).

I'm preparing a patch to integrate the OF handling into the existing
platform driver. I'll send you a note when I have something to test.

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-05  9:05             ` Marc Kleine-Budde
@ 2012-10-05  9:16               ` Andreas Larsson
  2012-10-05  9:19                 ` Marc Kleine-Budde
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Larsson @ 2012-10-05  9:16 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can, software

On 10/05/2012 11:05 AM, Marc Kleine-Budde wrote:
> On 10/05/2012 11:01 AM, Andreas Larsson wrote:
>> On 10/04/2012 05:12 PM, Marc Kleine-Budde wrote:
>>> On 10/04/2012 04:42 PM, Marc Kleine-Budde wrote:
>>>> On 10/04/2012 03:59 PM, Andreas Larsson wrote:
>>>>> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
>>>>
>>>> Looks better. However on sparc and sparc64, I'm seeing this warning.
>>>>   From my point of view a false positive.
>>>>
>>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function
>>>>> 'sja1000_ofp_remove':
>>>>> linux/include/linux/ioport.h:165:18: warning: 'res.end' is used
>>>>> uninitialized in this function [-Wuninitialized]
>>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note:
>>>>> 'res.end' was declared here
>>>>> linux/include/linux/ioport.h:165:2: warning: 'res.start' is used
>>>>> uninitialized in this function [-Wuninitialized]
>>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note:
>>>>> 'res.start' was declared here
>>>
>>> Looking at the preprocessed code, the compiler is right:
>>>
>>>> # 30
>>>> "/home/frogger/pengutronix/socketcan/linux/include/linux/of_address.h"
>>>> static inline __attribute__((always_inline))
>>>> __attribute__((no_instrument_function)) int
>>>> of_address_to_resource(struct device_node *dev, int index,
>>>>         struct resource *r)
>>>> {
>>>>    return -22;
>>>> }
>>>
>>> http://lxr.free-electrons.com/source/include/linux/of_address.h#L29
>>>
>>> Why is CONFIG_OF_ADDRESS not set in my .config?
>>
>> In drivers/of/Kconfig, OF_ADDRESS depends on !SPARC as sparc handles a
>> bunch of open firmware things differently. There is a sparc-specific
>> implementations of of_address_to_resource.
>>
>> I'll send in a patch changing of_address.h so that
>> of_address_to_resource gets declared extern for sparc instead of an
>> empty inline function. That should solve the problem as far as I see it.
>>
>> There is a similar thing with CONFIG_OF_IRQ, of_irq.h and
>> irq_of_parse_and_map, but there irq_of_parse_and_map is declared outside
>> of #ifdef CONFIG_OF_IRQ because of the sparc specific implementation.
>
> I've noticed that, too. With the patch you posted the driver cannot
> work, as the header files provide just no-ops of the of functions.
> Please test your patch before posting :).

My test branch is pre commit a850a7554442f08d3e910c6eeb4ee216868dda1e 
that introduced the empty functions and the dependency on 
CONFIG_OF_ADDRESS in of_address.h. So there everything worked fine.

Makes it clear that it is time to rebase the set of external patches 
that I need to run my hardware.

> I'm preparing a patch to integrate the OF handling into the existing
> platform driver. I'll send you a note when I have something to test.

Great!

cheers
Andreas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-05  9:16               ` Andreas Larsson
@ 2012-10-05  9:19                 ` Marc Kleine-Budde
  0 siblings, 0 replies; 12+ messages in thread
From: Marc Kleine-Budde @ 2012-10-05  9:19 UTC (permalink / raw)
  To: Andreas Larsson; +Cc: linux-can, software

[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]

On 10/05/2012 11:16 AM, Andreas Larsson wrote:
> On 10/05/2012 11:05 AM, Marc Kleine-Budde wrote:
>> On 10/05/2012 11:01 AM, Andreas Larsson wrote:
>>> On 10/04/2012 05:12 PM, Marc Kleine-Budde wrote:
>>>> On 10/04/2012 04:42 PM, Marc Kleine-Budde wrote:
>>>>> On 10/04/2012 03:59 PM, Andreas Larsson wrote:
>>>>>> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
>>>>>
>>>>> Looks better. However on sparc and sparc64, I'm seeing this warning.
>>>>>   From my point of view a false positive.
>>>>>
>>>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c: In function
>>>>>> 'sja1000_ofp_remove':
>>>>>> linux/include/linux/ioport.h:165:18: warning: 'res.end' is used
>>>>>> uninitialized in this function [-Wuninitialized]
>>>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note:
>>>>>> 'res.end' was declared here
>>>>>> linux/include/linux/ioport.h:165:2: warning: 'res.start' is used
>>>>>> uninitialized in this function [-Wuninitialized]
>>>>>> linux/drivers/net/can/sja1000/sja1000_of_platform.c:78:18: note:
>>>>>> 'res.start' was declared here
>>>>
>>>> Looking at the preprocessed code, the compiler is right:
>>>>
>>>>> # 30
>>>>> "/home/frogger/pengutronix/socketcan/linux/include/linux/of_address.h"
>>>>> static inline __attribute__((always_inline))
>>>>> __attribute__((no_instrument_function)) int
>>>>> of_address_to_resource(struct device_node *dev, int index,
>>>>>         struct resource *r)
>>>>> {
>>>>>    return -22;
>>>>> }
>>>>
>>>> http://lxr.free-electrons.com/source/include/linux/of_address.h#L29
>>>>
>>>> Why is CONFIG_OF_ADDRESS not set in my .config?
>>>
>>> In drivers/of/Kconfig, OF_ADDRESS depends on !SPARC as sparc handles a
>>> bunch of open firmware things differently. There is a sparc-specific
>>> implementations of of_address_to_resource.
>>>
>>> I'll send in a patch changing of_address.h so that
>>> of_address_to_resource gets declared extern for sparc instead of an
>>> empty inline function. That should solve the problem as far as I see it.
>>>
>>> There is a similar thing with CONFIG_OF_IRQ, of_irq.h and
>>> irq_of_parse_and_map, but there irq_of_parse_and_map is declared outside
>>> of #ifdef CONFIG_OF_IRQ because of the sparc specific implementation.
>>
>> I've noticed that, too. With the patch you posted the driver cannot
>> work, as the header files provide just no-ops of the of functions.
>> Please test your patch before posting :).
> 
> My test branch is pre commit a850a7554442f08d3e910c6eeb4ee216868dda1e
> that introduced the empty functions and the dependency on
> CONFIG_OF_ADDRESS in of_address.h. So there everything worked fine.

But that commit is aka v3.4-rc1~172^2~18, which means it was included in
v3.4 and this is way old.

> Makes it clear that it is time to rebase the set of external patches
> that I need to run my hardware.

Sounds like a plan.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-10-04 13:59     ` [PATCH v2] " Andreas Larsson
  2012-10-04 14:42       ` Marc Kleine-Budde
@ 2012-11-12 17:52       ` Andreas Larsson
  2012-11-22 11:30         ` Marc Kleine-Budde
  1 sibling, 1 reply; 12+ messages in thread
From: Andreas Larsson @ 2012-11-12 17:52 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: software, linux-can, Wolfgang Grandegger

On 10/04/2012 03:59 PM, Andreas Larsson wrote:
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

Hi!

As of v3.7-rc5, this patch now works properly (of_address_to_resource is now 
once again available for sparc). It would be nice to have it applied, unless the 
work on integrating sja1000_of_platform into sja1000_platform will be applied 
soon (and work for sparc). I'll be happy to test new versions on that initiative 
if that is the way you want to go.

It would be nice to get sja1000 to work on sparc soon in some way or another.

Cheers,
Andreas


> ---
>   drivers/net/can/sja1000/Kconfig               |    2 +-
>   drivers/net/can/sja1000/sja1000_of_platform.c |    6 ++++--
>   2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
> index 03df9a8..44abee4 100644
> --- a/drivers/net/can/sja1000/Kconfig
> +++ b/drivers/net/can/sja1000/Kconfig
> @@ -21,7 +21,7 @@ config CAN_SJA1000_PLATFORM
>
>   config CAN_SJA1000_OF_PLATFORM
>   	tristate "Generic OF Platform Bus based SJA1000 driver"
> -	depends on PPC_OF
> +	depends on OF
>   	---help---
>   	  This driver adds support for the SJA1000 chips connected to
>   	  the OpenFirmware "platform bus" found on embedded systems with
> diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
> index f2683eb..e45258d 100644
> --- a/drivers/net/can/sja1000/sja1000_of_platform.c
> +++ b/drivers/net/can/sja1000/sja1000_of_platform.c
> @@ -42,6 +42,8 @@
>   #include <linux/can/dev.h>
>
>   #include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
>   #include <asm/prom.h>
>
>   #include "sja1000.h"
> @@ -59,13 +61,13 @@ MODULE_LICENSE("GPL v2");
>
>   static u8 sja1000_ofp_read_reg(const struct sja1000_priv *priv, int reg)
>   {
> -	return in_8(priv->reg_base + reg);
> +	return ioread8(priv->reg_base + reg);
>   }
>
>   static void sja1000_ofp_write_reg(const struct sja1000_priv *priv,
>   				  int reg, u8 val)
>   {
> -	out_8(priv->reg_base + reg, val);
> +	iowrite8(val, priv->reg_base + reg);
>   }
>
>   static int __devexit sja1000_ofp_remove(struct platform_device *ofdev)
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
  2012-11-12 17:52       ` Andreas Larsson
@ 2012-11-22 11:30         ` Marc Kleine-Budde
  0 siblings, 0 replies; 12+ messages in thread
From: Marc Kleine-Budde @ 2012-11-22 11:30 UTC (permalink / raw)
  To: Andreas Larsson; +Cc: software, linux-can, Wolfgang Grandegger

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

On 11/12/2012 06:52 PM, Andreas Larsson wrote:
> As of v3.7-rc5, this patch now works properly (of_address_to_resource is
> now once again available for sparc). It would be nice to have it
> applied, unless the work on integrating sja1000_of_platform into
> sja1000_platform will be applied soon (and work for sparc). I'll be
> happy to test new versions on that initiative if that is the way you
> want to go.
> 
> It would be nice to get sja1000 to work on sparc soon in some way or
> another.

For now, I've applied the patch to linux-can-next/master. But in the
short term I'll try to give the my other patch another try and merge the
of_platform driver to the existing platform driver. I hope you can test
on sparc then. :)

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-11-22 19:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-02 13:13 [PATCH] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC Andreas Larsson
2012-10-04 12:49 ` Marc Kleine-Budde
2012-10-04 13:43   ` Andreas Larsson
2012-10-04 13:59     ` [PATCH v2] " Andreas Larsson
2012-10-04 14:42       ` Marc Kleine-Budde
2012-10-04 15:12         ` Marc Kleine-Budde
2012-10-05  9:01           ` Andreas Larsson
2012-10-05  9:05             ` Marc Kleine-Budde
2012-10-05  9:16               ` Andreas Larsson
2012-10-05  9:19                 ` Marc Kleine-Budde
2012-11-12 17:52       ` Andreas Larsson
2012-11-22 11:30         ` Marc Kleine-Budde

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.