linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: majun258@huawei.com (majun (F))
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] irq/mbigen:Fix the problem of IO remap for duplicated physical address in mbigen driver
Date: Tue, 2 Feb 2016 18:25:53 +0800	[thread overview]
Message-ID: <56B08431.3000200@huawei.com> (raw)
In-Reply-To: <20160201132557.GG674@leverpostej>

Hi Mark:

? 2016/2/1 21:25, Mark Rutland ??:
> On Mon, Feb 01, 2016 at 09:06:38PM +0800, majun (F) wrote:
>>
>>
>> ? 2016/2/1 19:57, Mark Rutland ??:
>>> On Mon, Feb 01, 2016 at 07:44:54PM +0800, MaJun wrote:
>>>> From: Ma Jun <majun258@huawei.com>
>>>>
>>>> For mbigen module, there is a special case that more than one mbigen 
>>>> device nodes use the same reg definition in DTS when these devices
>>>> exist in the same mbigen hardware module. 
>>>>
>>>> mbigen_dev1:intc_dev1 {
>>>> 	...
>>>> 	reg = <0x0 0xc0080000 0x0 0x10000>;
>>>> 	...
>>>> };
>>>>
>>>> mbigen_dev2:intc_dev2 {
>>>> 	...
>>>> 	reg = <0x0 0xc0080000 0x0 0x10000>;
>>>> 	...
>>>> };
>>>
>>> This doesn't sound right. If they exist in the same place, and have the
>>> same reg, they _are_ the same device.
>>>
>>> You'll need to explain this better.
>>>
>>
>> For a mbigen hardware module which connecte with several devices,
>> because these devices has different device ID,
>> I need to define a device node for each device in DTS file.
>>
>> 	mbigen
>> 	|
>> ------------------
>> | 	| 	|
>> dev1 	dev2 	dev3
>>
>> Because of register layout,the registers related with these devices
>> are put together, I can't split them clearly.
>> So, I only make these devices which connect with
>> same mbigen hardware module usethe same address.
> 
> This sounds like we've either mis-described the mbigen in the binding,
> or we're mis-describing the relationship with the devices.
> 

Sorry, I didn't express myself clearly.

For example, a mbigen hardware module can support several peripheral devices
with different device ID.

|-----------------------|-
|	mbigen		|
|-----------------------|-
 |       |        |
dev1    dev2     dev3

To simplify the mbigen drvier,
I didn't use the whole mbigen module as a mbgien device, but use
the register collections(vector, trigger type,status etc.) corresponding
to a peripheral device as a mbigen device.
So, mbigen device is a logical conception or logical device.

Now, a mbigen hardware module contains several logical mbigen device.

-------------------------------
|mgn_dev1  mgn_dev2  mgn_dev3 |
|-----------------------------|
   |          |        |
dev1	    dev2      dev3

So,mgn_dev1, mgn_dev2 and mgn_dev3 exist in same mbigen hardware module,
and,they use the same reg address region,that is adress of mbigen hardware module.

For this case, I think the question is can we map an reg address
region more than one time?

Thanks!

> It should not be necessary to describe the same set of registers
> repeatedly.
> 
>>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> -	mgn_chip->base = devm_ioremap_resource(&pdev->dev, res);
>>>> +	mgn_chip->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>>>
>>> These two behaving differently doesn't seem correct either...
>>
>> The problem is caused by memory region check fail because of
>> duplicated address in devm_ioremap_resource().
> 
> The check is sensible. Using devm_ioremap rather than
> devm_ioremap_resource to avoid the check is not the correct solution.
> 
>> For mbigen special case, there is no necessary to do this check.
> 
> I'm not sure I fully agree.
> 
> Regardless, this is not the right solution; it is fragile at best.
> 
> Thanks,
> Mark.
> 
> .
> 

  reply	other threads:[~2016-02-02 10:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-01 11:44 [RFC PATCH] irq/mbigen:Fix the problem of IO remap for duplicated physical address in mbigen driver MaJun
2016-02-01 11:57 ` Mark Rutland
2016-02-01 13:06   ` majun (F)
2016-02-01 13:25     ` Mark Rutland
2016-02-02 10:25       ` majun (F) [this message]
2016-02-02 11:43         ` Mark Rutland
2016-02-03  2:31           ` majun (F)
2016-02-03 11:16             ` Mark Rutland
2016-02-16  7:54               ` majun (F)

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=56B08431.3000200@huawei.com \
    --to=majun258@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).