From: Benoit Cousson <b-cousson@ti.com>
To: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>,
Peter Ujfalusi <peter.ujfalusi@ti.com>,
Tony Lindgren <tony@atomide.com>,
linux-omap@vger.kernel.org, Kevin Hilman <khilman@ti.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: omap_hwmod: Fix up resource names when booted with devicetree
Date: Thu, 6 Sep 2012 09:37:29 +0200 [thread overview]
Message-ID: <504852B9.5000403@ti.com> (raw)
In-Reply-To: <5047B079.6040209@ti.com>
Hi Vaibhav,
On 09/05/2012 10:05 PM, Vaibhav Hiremath wrote:
> On 9/5/2012 7:57 PM, Benoit Cousson wrote:
>> Hi Paul,
>>
>> On 08/24/2012 06:20 PM, Peter Ujfalusi wrote:
>>> Hi Paul,
>>>
>>> On 08/24/2012 06:38 PM, Paul Walmsley wrote:
>>>> Do we need both this one and your '[PATCH] driver core: Check if r->name
>>>> is valid in platform_get_resource_byname()' ? Or would that second patch
>>>> be enough? Is the crash happening in the platform_get_resource_byname()
>>>> iterator?
>>>
>>> The crash happens in platform_get_resource_byname(). What I see as a problem
>>> that when we boot without DT the r->name is configured for the hwmods. If we
>>> boot with DT we discard the resources created by the OF (which also have the
>>> r->name configured). We replace the resources from hwmods but we do not fix up
>>> the resource names (which is done in other cases).
>>> I have sent the patch for the drivers core as well since I think it is a good
>>> practice anyway to check for NULL pointer before strcmp().
>>>
>>> Either is good, but IMHO we should fix this in omap_hwmod (at least).
>>
>> Yes, clearly we do have a corner case today due to the mix of hwmod /
>> DTS resource management.
>> - Legacy boot + hwmod resources is OK
>> - DTS boot with DTS resources will be OK
>> - DTS boot with hwmod resources is not OK since the resource name will
>> not be populated in the case resources are not named :-(
>>
>> If you are OK, I'll take that patch along with Vaibhav one to handle
>> properly the resources from DTS long with the DTS patches I'm queuing
>> for 3.7.
>>
>
> Benoit,
> Note that this patch will add device name to all resources, since none
> of the resource will have name property and may break drivers;
> especially after my patch which respects DT resources.
So you do have to add the name in the DTS if the driver is expecting that.
> Also I just read the thread you started sometime back on _byname api,
> not sure that was the conclusion, but it seems like it is not
> recommended to use _byname. The driver should be fixed while converting
> to DT.
No the DT name for reg and interrupt was accepted and Peter is taking advantage on that.
The only missing binding is the DMA now.
Here is what Peter did.
compatible = "ti,omap4-mcpdm";
reg = <0x40132000 0x7f>, /* MPU private access */
<0x49032000 0x7f>; /* L3 Interconnect */
+ reg-names = "mpu", "dma";
interrupts = <0 112 0x4>;
interrupt-parent = <&gic>;
ti,hwmods = "mcpdm";
Regards,
Benoit
WARNING: multiple messages have this Message-ID (diff)
From: b-cousson@ti.com (Benoit Cousson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: omap_hwmod: Fix up resource names when booted with devicetree
Date: Thu, 6 Sep 2012 09:37:29 +0200 [thread overview]
Message-ID: <504852B9.5000403@ti.com> (raw)
In-Reply-To: <5047B079.6040209@ti.com>
Hi Vaibhav,
On 09/05/2012 10:05 PM, Vaibhav Hiremath wrote:
> On 9/5/2012 7:57 PM, Benoit Cousson wrote:
>> Hi Paul,
>>
>> On 08/24/2012 06:20 PM, Peter Ujfalusi wrote:
>>> Hi Paul,
>>>
>>> On 08/24/2012 06:38 PM, Paul Walmsley wrote:
>>>> Do we need both this one and your '[PATCH] driver core: Check if r->name
>>>> is valid in platform_get_resource_byname()' ? Or would that second patch
>>>> be enough? Is the crash happening in the platform_get_resource_byname()
>>>> iterator?
>>>
>>> The crash happens in platform_get_resource_byname(). What I see as a problem
>>> that when we boot without DT the r->name is configured for the hwmods. If we
>>> boot with DT we discard the resources created by the OF (which also have the
>>> r->name configured). We replace the resources from hwmods but we do not fix up
>>> the resource names (which is done in other cases).
>>> I have sent the patch for the drivers core as well since I think it is a good
>>> practice anyway to check for NULL pointer before strcmp().
>>>
>>> Either is good, but IMHO we should fix this in omap_hwmod (at least).
>>
>> Yes, clearly we do have a corner case today due to the mix of hwmod /
>> DTS resource management.
>> - Legacy boot + hwmod resources is OK
>> - DTS boot with DTS resources will be OK
>> - DTS boot with hwmod resources is not OK since the resource name will
>> not be populated in the case resources are not named :-(
>>
>> If you are OK, I'll take that patch along with Vaibhav one to handle
>> properly the resources from DTS long with the DTS patches I'm queuing
>> for 3.7.
>>
>
> Benoit,
> Note that this patch will add device name to all resources, since none
> of the resource will have name property and may break drivers;
> especially after my patch which respects DT resources.
So you do have to add the name in the DTS if the driver is expecting that.
> Also I just read the thread you started sometime back on _byname api,
> not sure that was the conclusion, but it seems like it is not
> recommended to use _byname. The driver should be fixed while converting
> to DT.
No the DT name for reg and interrupt was accepted and Peter is taking advantage on that.
The only missing binding is the DMA now.
Here is what Peter did.
compatible = "ti,omap4-mcpdm";
reg = <0x40132000 0x7f>, /* MPU private access */
<0x49032000 0x7f>; /* L3 Interconnect */
+ reg-names = "mpu", "dma";
interrupts = <0 112 0x4>;
interrupt-parent = <&gic>;
ti,hwmods = "mcpdm";
Regards,
Benoit
next prev parent reply other threads:[~2012-09-06 7:37 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-23 13:54 [PATCH] ARM: omap_hwmod: Fix up resource names when booted with devicetree Peter Ujfalusi
2012-08-23 13:54 ` Peter Ujfalusi
2012-08-24 15:38 ` Paul Walmsley
2012-08-24 15:38 ` Paul Walmsley
2012-08-24 16:20 ` Peter Ujfalusi
2012-08-24 16:20 ` Peter Ujfalusi
2012-09-05 14:27 ` Benoit Cousson
2012-09-05 14:27 ` Benoit Cousson
2012-09-05 14:46 ` Benoit Cousson
2012-09-05 14:46 ` Benoit Cousson
2012-09-05 14:46 ` Benoit Cousson
2012-09-05 14:46 ` Benoit Cousson
2012-09-05 20:05 ` Vaibhav Hiremath
2012-09-05 20:05 ` Vaibhav Hiremath
2012-09-06 7:37 ` Benoit Cousson [this message]
2012-09-06 7:37 ` Benoit Cousson
2012-09-06 8:28 ` Hiremath, Vaibhav
2012-09-06 8:28 ` Hiremath, Vaibhav
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=504852B9.5000403@ti.com \
--to=b-cousson@ti.com \
--cc=hvaibhav@ti.com \
--cc=khilman@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-omap@vger.kernel.org \
--cc=paul@pwsan.com \
--cc=peter.ujfalusi@ti.com \
--cc=tony@atomide.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.