| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0d3f12b9-d8a1-701e-df88-788faa064f20@ti.com>
Date: Wed, 18 Oct 2017 14:17:52 +0530
From: Kishon Vijay Abraham I <kishon@...com>
To: Niklas Cassel <niklas.cassel@...s.com>,
Bjorn Helgaas <helgaas@...nel.org>
CC: Jesper Nilsson <jespern@...s.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Jingoo Han <jingoohan1@...il.com>,
Xiaowei Song <songxiaowei@...ilicon.com>,
Peter Robinson <pbrobinson@...il.com>,
Tomasz Nowicki <tn@...ihalf.com>,
Gabriele Paoloni <gabriele.paoloni@...wei.com>,
Duc Dang <dhdang@....com>, <linux-arm-kernel@...s.com>,
<linux-pci@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 08/10] PCI: dwc: artpec6: add support for endpoint mode
Niklas,
On Wednesday 18 October 2017 01:45 PM, Niklas Cassel wrote:
> On 10/18/2017 10:03 AM, Kishon Vijay Abraham I wrote:
>> Hi Bjorn,
>>
>> On Tuesday 17 October 2017 05:13 AM, Bjorn Helgaas wrote:
>>> On Fri, Oct 13, 2017 at 06:09:11PM +0200, Niklas Cassel wrote:
>>>> Signed-off-by: Niklas Cassel <niklas.cassel@...s.com>
>>>> ---
>>>> .../devicetree/bindings/pci/axis,artpec6-pcie.txt | 3 +-
>>>> drivers/pci/dwc/Kconfig | 41 +++--
>>>> drivers/pci/dwc/Makefile | 4 +-
>>>> drivers/pci/dwc/pcie-artpec6.c | 202 ++++++++++++++++++++-
>>>> 4 files changed, 233 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
>>>> index 4e4aee4439ea..33eef7ae5a23 100644
>>>> --- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
>>>> +++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
>>>> @@ -4,7 +4,8 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
>>>> and thus inherits all the common properties defined in designware-pcie.txt.
>>>>
>>>> Required properties:
>>>> -- compatible: "axis,artpec6-pcie", "snps,dw-pcie"
>>>> +- compatible: "axis,artpec6-pcie", "snps,dw-pcie" for ARTPEC-6 in RC mode;
>>>> + "axis,artpec6-pcie-ep", "snps,dw-pcie" for ARTPEC-6 in EP mode;
>>>> - reg: base addresses and lengths of the PCIe controller (DBI),
>>>> the PHY controller, and configuration address space.
>>>> - reg-names: Must include the following entries:
>>>> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
>>>> index 22ec82fcdea2..e333283fb1ed 100644
>>>> --- a/drivers/pci/dwc/Kconfig
>>>> +++ b/drivers/pci/dwc/Kconfig
>>>> @@ -14,6 +14,36 @@ config PCIE_DW_EP
>>>> depends on PCI_ENDPOINT
>>>> select PCIE_DW
>>>>
>>>> +config PCIE_ARTPEC6
>>>> + bool "Axis ARTPEC-6 PCIe controller"
>>>> + depends on (PCI && PCI_MSI_IRQ_DOMAIN) || PCI_ENDPOINT
>>>> + depends on MACH_ARTPEC6
>>>> + help
>>>> + Say Y here to enable PCIe controller support on Axis ARTPEC-6
>>>> + SoCs. This PCIe controller uses the DesignWare core.
>>>> +
>>>> +if PCIE_ARTPEC6
>>>> +
>>>> +config PCIE_ARTPEC6_HOST
>>>> + bool "Axis ARTPEC-6 Host Mode"
>>>> + depends on PCI
>>>> + depends on PCI_MSI_IRQ_DOMAIN
>>>> + select PCIEPORTBUS
>>>> + select PCIE_DW_HOST
>>>> + help
>>>> + Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
>>>> + host mode.
>>>> +
>>>> +config PCIE_ARTPEC6_EP
>>>> + bool "Axis ARTPEC-6 Endpoint Mode"
>>>> + depends on PCI_ENDPOINT
>>>> + select PCIE_DW_EP
>>>> + help
>>>> + Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
>>>> + endpoint mode.
>>>> +
>>>> +endif
>>>> +
>>>> config PCI_DRA7XX
>>>> bool "TI DRA7xx PCIe controller"
>>>> depends on SOC_DRA7XX || COMPILE_TEST
>>>> @@ -148,17 +178,6 @@ config PCIE_ARMADA_8K
>>>> DesignWare hardware and therefore the driver re-uses the
>>>> DesignWare core functions to implement the driver.
>>>>
>>>> -config PCIE_ARTPEC6
>>>> - bool "Axis ARTPEC-6 PCIe controller"
>>>> - depends on PCI
>>>> - depends on MACH_ARTPEC6
>>>> - depends on PCI_MSI_IRQ_DOMAIN
>>>> - select PCIEPORTBUS
>>>> - select PCIE_DW_HOST
>>>> - help
>>>> - Say Y here to enable PCIe controller support on Axis ARTPEC-6
>>>> - SoCs. This PCIe controller uses the DesignWare core.
>>>> -
>>>> config PCIE_KIRIN
>>>> depends on OF && ARM64
>>>> bool "HiSilicon Kirin series SoCs PCIe controllers"
>>>
>>>> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
>>>> index c61be9738cce..ac98242b83a9 100644
>>>> --- a/drivers/pci/dwc/Makefile
>>>> +++ b/drivers/pci/dwc/Makefile
>>>> @@ -2,6 +2,9 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>>> obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>>>> obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>>>> obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>>>> +ifneq ($(filter y,$(CONFIG_PCIE_ARTPEC6_HOST) $(CONFIG_PCIE_ARTPEC6_EP)),)
>>>> + obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
>>>> +endif
>>>
>>> I see you're copying the DRA7XX style here, but I don't really
>>> understand it. I guess the idea is to build pcie-artpec6.o if either
>>> CONFIG_PCIE_ARTPEC6_HOST or CONFIG_PCIE_ARTPEC6_EP is set (or both).
>>>
>>> Is this really the simplest way to express that in Kconfig? Both the
>>> "if PCIE_ARTPEC6" and this ifneq thing are complicated and relatively
>>> unusual.
>>>
>>>> ifneq ($(filter y,$(CONFIG_PCI_DRA7XX_HOST) $(CONFIG_PCI_DRA7XX_EP)),)
>>>> obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>>> endif
>>>
>>>> +static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>>>
>>> Looks funny to have this dw_*() function in pcie-artpec6.c. Intended?
>>> Or is this something other endpoint drivers could/should share?
>>
>> yeah, dw_pcie_ep_reset_bar is already present in pcie-designware-ep.c which
>> should be made as library function to be used by other drivers. (pci-dra7xx.c
>> has also added something like above which should also be fixed).
>>
>
> Hello Kishon,
>
> I can remove the static keyword from dw_pcie_ep_reset_bar,
> move the declaration to pcie-designware.h,
> and make sure that both pcie-artpec6.c and pci-dra7xx.c uses it.
>
> Will do this in the next version of the patch series if you
> don't have any objection.
sure, thanks for doing it :-)
-Kishon
Powered by blists - more mailing lists