From: B31939@freescale•com (Lian Minghuan-B31939)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2] PCI: designware: Add support 4 ATUs assignment
Date: Thu, 13 Nov 2014 18:02:11 +0800 [thread overview]
Message-ID: <546481A3.5050700@freescale.com> (raw)
In-Reply-To: <1415809929.2876.5.camel@pengutronix.de>
Hi Lucas,
Please see my comments inline.
Thanks,
Minghuan
On 2014?11?13? 00:32, Lucas Stach wrote:
> Am Mittwoch, den 12.11.2014, 21:53 +0530 schrieb Srikanth Thokala:
>> Hi Minghuan,
>>
>> On Wed, Nov 12, 2014 at 3:39 PM, Lian Minghuan-B31939
>> <B31939@freescale•com> wrote:
>>> Hi Srikanth,
>>>
>>> please see my comments inline.
>>>
>>> Thanks,
>>> Minghuan
>>>
>>>
>>> On 2014?11?12? 17:01, Srikanth Thokala wrote:
>>>> Hi Minghuan,
>>>>
>>>> On Wed, Nov 12, 2014 at 12:44 PM, Lian Minghuan-B31939
>>>> <B31939@freescale•com> wrote:
>>>>> Hi Srikanth,
>>>>>
>>>>> Thanks for your comments, please see my reply inline.
>>>>>
>>>>>
>>>>> On 2014?11?12? 14:22, Srikanth Thokala wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Tue, Nov 11, 2014 at 10:37 AM, Minghuan Lian
>>>>>> <Minghuan.Lian@freescale•com> wrote:
>>>>>>> Currently, pcie-designware.c only supports two ATUs, ATU0 is used
>>>>>>> for CFG0 and MEM, ATU1 is used for CFG1 and IO. There is a conflict
>>>>>>> when MEM and CFG0 are accessed simultaneously. The patch adds
>>>>>>> 'num-atus' property to PCIe dts node to describe the number of
>>>>>>> PCIe controller's ATUs. If num_atus is bigger than or equal to 4,
>>>>>>> we will change ATUs assignment: ATU0 for CFG0, ATU1 for CFG1,
>>>>>>> ATU2 for MEM, ATU3 for IO.
>>>>>>>
>>>>>>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale•com>
>>>>>>> ---
>>>>>>> change log:
>>>>>>> v1-v2:
>>>>>>> 1. add the default value to property num-atus description
>>>>>>> 2. Use atu_idx[] instead of single values
>>>>>>> 3. initialize num_atus to 2
>>>>>>>
>>>>>>> .../devicetree/bindings/pci/designware-pcie.txt | 1 +
>>>>>>> drivers/pci/host/pcie-designware.c | 53
>>>>>>> ++++++++++++++++------
>>>>>>> drivers/pci/host/pcie-designware.h | 9 ++++
>>>>>>> 3 files changed, 50 insertions(+), 13 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>>>>> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>>>>> index 9f4faa8..64d0533 100644
>>>>>>> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
>>>>>>> @@ -26,3 +26,4 @@ Optional properties:
>>>>>>> - bus-range: PCI bus numbers covered (it is recommended for new
>>>>>>> devicetrees to
>>>>>>> specify this property, to keep backwards compatibility a range of
>>>>>>> 0x00-0xff
>>>>>>> is assumed if not present)
>>>>>>> +- num-atus: number of ATUs. The default value is 2 if not present.
>>>>>>> diff --git a/drivers/pci/host/pcie-designware.c
>>>>>>> b/drivers/pci/host/pcie-designware.c
>>>>>>> index dfed00a..46a609d 100644
>>>>>>> --- a/drivers/pci/host/pcie-designware.c
>>>>>>> +++ b/drivers/pci/host/pcie-designware.c
>>>>>>> @@ -48,6 +48,8 @@
>>>>>>> #define PCIE_ATU_VIEWPORT 0x900
>>>>>>> #define PCIE_ATU_REGION_INBOUND (0x1 << 31)
>>>>>>> #define PCIE_ATU_REGION_OUTBOUND (0x0 << 31)
>>>>>>> +#define PCIE_ATU_REGION_INDEX3 (0x3 << 0)
>>>>>>> +#define PCIE_ATU_REGION_INDEX2 (0x2 << 0)
>>>>>>> #define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
>>>>>>> #define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
>>>>>>> #define PCIE_ATU_CR1 0x904
>>>>>>> @@ -346,7 +348,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>>>>>>> struct of_pci_range range;
>>>>>>> struct of_pci_range_parser parser;
>>>>>>> struct resource *cfg_res;
>>>>>>> - u32 val, na, ns;
>>>>>>> + u32 num_atus = 2, val, na, ns;
>>>>>> I think this can be u8?
>>>>> [Minghuan] I define num-atus like this: num-atus = <6> (our controller
>>>>> supports 6 outbound ATUs)
>>>>> So, num_atus should be u32 type.
>>>>> If we use u8 type to define num_atus, the dts node should be changed to
>>>>> num_atus = /bits/ 8 <8>.
>>>>> I prefer the previous definition num-atus = <6> which is more simple and
>>>>> clear.
>>>> Yes, I agree. But as it holds only 6 possible distinct values, I
>>>> prefer to use u8.
>>> [Minghuan] PCIe Designware IP supports more than 6 ATUs. But our
>>> current PCIe controller only supports 6 outbound ATUs and 6 inbound ATUs.
>>> The next PCIe controller may supports more ATUs. I think u32 can be better
>>> compatible with hardware upgrade.
>>>
>>> I inquired dts, almost all dts property use u32 type.
>> I don't think this property will have values > 255, but if you think
>> so you could
>> use u16 and then u32.
>>
> Using a smaller type complicates the DT for little to no benefit. I
> think it's ok to use u32 here, which is a common standard for integer
> values in DT.
>
> Though this discussion lead me to the question if we even need to have
> this property in the DT at all. Isn't this a property that is fixed for
> a specific silicon implementation of the DW core? In that case we could
> just infer the number of ATUs from the DT compatible, so this should
> probably just be added to struct pcie_port and properly initialized by
> the SoC glue drivers.
[Minghuan] As far as I know, exynos implements only 2 ATUs, this is why
pcie-designware only supports 2 ATU. iMX implements 4 ATUs and LS1021A
implements 6 ATUs.
> Regards,
> Lucas
>
next prev parent reply other threads:[~2014-11-13 10:02 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-11 5:07 [PATCH v2] PCI: designware: Add support 4 ATUs assignment Minghuan Lian
2014-11-12 6:22 ` Srikanth Thokala
2014-11-12 7:14 ` Lian Minghuan-B31939
2014-11-12 9:01 ` Srikanth Thokala
2014-11-12 10:09 ` Lian Minghuan-B31939
2014-11-12 16:23 ` Srikanth Thokala
2014-11-12 16:32 ` Lucas Stach
2014-11-13 10:02 ` Lian Minghuan-B31939 [this message]
2014-11-13 10:20 ` Lucas Stach
2014-11-13 10:52 ` Lian Minghuan-B31939
2014-11-13 11:09 ` Lucas Stach
2014-11-14 8:47 ` Lian Minghuan-B31939
2014-11-14 10:02 ` Lucas Stach
2014-11-14 11:30 ` Mingkai.Hu at freescale.com
2014-11-14 11:42 ` Lucas Stach
2014-11-17 2:58 ` Lian Minghuan-B31939
2014-11-17 10:25 ` Lucas Stach
2014-11-14 9:36 ` Lian Minghuan-B31939
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=546481A3.5050700@freescale.com \
--to=b31939@freescale$(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