[RFC PATCH] irq/mbigen:Fix the problem of IO remap for duplicated physical address in mbigen driver

[majun (F)]

在 2016/2/2 19:43, Mark Rutland 写道:
> On Tue, Feb 02, 2016 at 06:25:53PM +0800, majun (F) wrote:
>> 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 at 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.
> 
>>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 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.

Thanks!
Ma Jun


> 
> Mark.
> 
> .
>