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, 16 Feb 2016 15:54:49 +0800 [thread overview]
Message-ID: <56C2D5C9.7090009@huawei.com> (raw)
In-Reply-To: <20160203111602.GA1234@leverpostej>
Hi Mark:
sorry for late response because of chinese new year.
? 2016/2/3 19:16, Mark Rutland ??:
> On Wed, Feb 03, 2016 at 10:31:52AM +0800, majun (F) wrote:
>>
>>
>> ? 2016/2/2 19:43, Mark Rutland ??:
>>> On Tue, Feb 02, 2016 at 06:25:53PM +0800, majun (F) wrote:
>>>> 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.
>>>
>>> >From the above, it sounds like the DT representation is not an accurate
>>> representation of the hardware. We should describe the _whole_ mbigen,
>>> not portions thereof. Trying to describe it piecemeal leads to problems
>>> like this one.
>>>
>>> I don't understand the rationale for describing the mbigen as separate
>>> nodes. Above you mention that we "need to define a device node for each
>>> device", but I don't see why that's necessary. Why do you believe we
>>> need an mbigen node per client device?
>>>
>>> As far as I can tell, we should be able to map an arbitrary
>>> interrupt-specifier to a particular MSI (complete with device id) within
>>> the mbigen driver. We just need to define the full set of MSIs the
>>> mbigen owns.
>>>
>>
>> mbigen device is a interrupt controller, it is also a ITS device for ITS module.
>> So, we need to register the each mbigen device to ITS with a unique ID.
>> Based on the current MSI/ITS driver, I can't register whole mbigen module which
>> contains several mbigen devices at one time. Because they have different device ID.
>
> I don't follow.
>
> You can describe this by having a top-level mbigen node featuring a reg,
> with a sub-node for each mbigen module with an appropriate msi-parent,
> e.g.
>
> mbigen {
> reg = < ... ... >;
>
> #interrupt-cells = <2>;
>
> #address-cells = <1>; /* module index */
>
> module at 0 {
> reg = <0>;
> msi-parent = <&its 0>;
> };
>
> module at 1 {
> reg = <1>;
> reg = <&its 1>;
> };
> };
>
>
> That clearly does not require the reg to be duplicated, and encodes the
> information you want. The infrastructure for handling that might not
> exist yet, but that is a Linux issue that we can fix.
>
> Marc, thoughts?
>
> I take it all interrupts within a module share the same device id?
>
>> I don't know whether this explanation is clear or not.
>> I think Marc can explain this well.
>>
>> Marc, would you please help me explain this? or, do you have any opinion about this ?
>>
>>>> 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?
>>>
>>> As above, I think we've mis-described the hardware. Rather than making
>>> things simpler, this has resulted in problems like this one.
>>>
>>> We should not map a reg region more than once. Even if it's technically
>>> possible to do so, I do not believe that would be the right solution
>>> here. Implicitly sharing resources (e.g. portions of the mbigen control
>>> registers) is inevitably going to lead to more problems later on.
>>
>> Because we only need to write 1 into corresponding bit of status
>> register to clear interrupt status during runtime,I think we don't
>> need to worry about this.
>
> That might be currently true, but I doubt that will remain true in
> future. Presumably there are other control registers in the mbigen which
> are shared between modules.
>
I'm sure there is nothing to control except the status register even in future.
Thanks
MaJun
> I think we do need to worry about this.
>
> Mark.
>
> .
>
prev parent reply other threads:[~2016-02-16 7:54 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)
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) [this message]
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=56C2D5C9.7090009@huawei.com \
--to=majun258@huawei$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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