All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Andreas Larsson <andreas@gaisler.com>
Cc: linux-can@vger.kernel.org, software@gaisler.com
Subject: Re: [PATCH v2] can: sja1000: Make sja1000_of_platform selectable and compilable on SPARC
Date: Fri, 05 Oct 2012 11:19:55 +0200	[thread overview]
Message-ID: <506EA63B.1060109@pengutronix.de> (raw)
In-Reply-To: <506EA57A.3020002@gaisler.com>

[-- 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 --]

  reply	other threads:[~2012-10-05  9:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2012-11-12 17:52       ` Andreas Larsson
2012-11-22 11:30         ` Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=506EA63B.1060109@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=andreas@gaisler.com \
    --cc=linux-can@vger.kernel.org \
    --cc=software@gaisler.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.