From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-by2on0118.outbound.protection.outlook.com ([207.46.100.118]:2537 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754505AbaKNIqo (ORCPT ); Fri, 14 Nov 2014 03:46:44 -0500 Message-ID: <5465C19D.3060801@freescale.com> Date: Fri, 14 Nov 2014 16:47:25 +0800 From: Lian Minghuan-B31939 MIME-Version: 1.0 To: Lucas Stach CC: Srikanth Thokala , Minghuan Lian , "linux-pci@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Zang Roy-R61911 , Hu Mingkai-B21284 , "Bjorn Helgaas" Subject: Re: [PATCH v2] PCI: designware: Add support 4 ATUs assignment References: <1415682444-24911-1-git-send-email-Minghuan.Lian@freescale.com> <546308DD.40109@freescale.com> <546331E7.5060003@freescale.com> <1415809929.2876.5.camel@pengutronix.de> <546481A3.5050700@freescale.com> <1415874015.2420.5.camel@pengutronix.de> <54648D61.6050704@freescale.com> <1415876988.2420.7.camel@pengutronix.de> In-Reply-To: <1415876988.2420.7.camel@pengutronix.de> Content-Type: text/plain; charset="utf-8"; format=flowed Sender: linux-pci-owner@vger.kernel.org List-ID: Hi Lucas and all, On 2014年11月13日 19:09, Lucas Stach wrote: > Am Donnerstag, den 13.11.2014, 18:52 +0800 schrieb Lian Minghuan-B31939: >> Hi Lucas, >> >> On 2014年11月13日 18:20, Lucas Stach wrote: >>> Am Donnerstag, den 13.11.2014, 18:02 +0800 schrieb Lian Minghuan-B31939: >>>> 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, >>> [...] >>> >>>>> 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. >>>> >>> Right so we don't need an additional property in the DT at all. The >>> number of ATUs is fixed for a specific core compatible and can be passed >>> in by the respective exynos, imx and ls1021 glue drivers. >>> >>> You may ask the Keystone and Spear maintainers to get the correct number >>> of ATUs for those implementations. >>> >>> Regards, >>> Lucas >> [Minghuan] Yes. This a way that specific core driver passes the ATU >> number to >> pci-designware. But I perfer to adding dts node for the following reasons: >> 1. ATU number is hardware attribute, so it can be added to DTS. > But it is a duplication of information that can be inferred from the DT > compatible alone, which is usually frowned upon. > > Also in contrast to the num-lanes property I don't see a use-case to > reduce the number of used ATUs in a specific system, so num-atus is > basically fixed for a specific implementation. [Minghuan] 4ATUs provides better support for multiple-function and SR-IOV PCIe devices. Can anyone tell me if there is the company will increase ATUs number? If yes, we should avoid the following code: if (of_device_is_compatible(pcie->dev->of_node, "xxx,r1-pcie")) atu_num = 2; if (of_device_is_compatible(pcie->dev->of_node, "xxx,r2-pcie")) atu_num = 4; >> 2. That pci-designware common code parses the 'num-atus' can avoid every >> specific controller driver to define and pass num-atus, so can reduce >> code size and simplify the specific controller driver implementation. >> > I don't think the code reduction matters here and the simplification is > minimal. > > Regards, > Lucas > From mboxrd@z Thu Jan 1 00:00:00 1970 From: B31939@freescale.com (Lian Minghuan-B31939) Date: Fri, 14 Nov 2014 16:47:25 +0800 Subject: [PATCH v2] PCI: designware: Add support 4 ATUs assignment In-Reply-To: <1415876988.2420.7.camel@pengutronix.de> References: <1415682444-24911-1-git-send-email-Minghuan.Lian@freescale.com> <546308DD.40109@freescale.com> <546331E7.5060003@freescale.com> <1415809929.2876.5.camel@pengutronix.de> <546481A3.5050700@freescale.com> <1415874015.2420.5.camel@pengutronix.de> <54648D61.6050704@freescale.com> <1415876988.2420.7.camel@pengutronix.de> Message-ID: <5465C19D.3060801@freescale.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Lucas and all, On 2014?11?13? 19:09, Lucas Stach wrote: > Am Donnerstag, den 13.11.2014, 18:52 +0800 schrieb Lian Minghuan-B31939: >> Hi Lucas, >> >> On 2014?11?13? 18:20, Lucas Stach wrote: >>> Am Donnerstag, den 13.11.2014, 18:02 +0800 schrieb Lian Minghuan-B31939: >>>> 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, >>> [...] >>> >>>>> 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. >>>> >>> Right so we don't need an additional property in the DT at all. The >>> number of ATUs is fixed for a specific core compatible and can be passed >>> in by the respective exynos, imx and ls1021 glue drivers. >>> >>> You may ask the Keystone and Spear maintainers to get the correct number >>> of ATUs for those implementations. >>> >>> Regards, >>> Lucas >> [Minghuan] Yes. This a way that specific core driver passes the ATU >> number to >> pci-designware. But I perfer to adding dts node for the following reasons: >> 1. ATU number is hardware attribute, so it can be added to DTS. > But it is a duplication of information that can be inferred from the DT > compatible alone, which is usually frowned upon. > > Also in contrast to the num-lanes property I don't see a use-case to > reduce the number of used ATUs in a specific system, so num-atus is > basically fixed for a specific implementation. [Minghuan] 4ATUs provides better support for multiple-function and SR-IOV PCIe devices. Can anyone tell me if there is the company will increase ATUs number? If yes, we should avoid the following code: if (of_device_is_compatible(pcie->dev->of_node, "xxx,r1-pcie")) atu_num = 2; if (of_device_is_compatible(pcie->dev->of_node, "xxx,r2-pcie")) atu_num = 4; >> 2. That pci-designware common code parses the 'num-atus' can avoid every >> specific controller driver to define and pass num-atus, so can reduce >> code size and simplify the specific controller driver implementation. >> > I don't think the code reduction matters here and the simplification is > minimal. > > Regards, > Lucas >