RE: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-09 Thread Bharat Kumar Gogada 
> On 09/10/15 06:11, Bharat Kumar Gogada wrote:
>  +struct nwl_msi {  /* struct nwl_msi - MSI information
> >> */
>  +  struct msi_controller chip; /* chip: MSI controller */
> >>>
>  We're moving away from msi_controller altogether, as the kernel now
>  has all the necessary infrastructure to do this properly.
> >>>
> >>> Our current GIC version does not have separate msi controller (we
> >>> are not using GICv2m or GICv3), so is it necessary to have separate
> >>> msi controller node ? Please give me clarity on this.
> >>
> >> This has nothing to do with the version of the GIC you are using
> >> (XGene doesn't have GICv2m or v3 either). This is about reducing code
> >> duplication and having something that we can maintain. See also
> >> https://lkml.org/lkml/2015/9/20/193 for yet another example.
> >>
> >> I still plan to kill msi_controller, and I'd like to avoid more
> >> dependencies with it. MSI domains are the way to do it.
> >>
> > Sorry previously I haven't configured my email client properly so resending.
> 
> Thanks for doing so, much appreciated.
> 
> > Since we don't have separate MSI controller, and our PCIe controller
> > is handling MSI, is it necessary to create a separate MSI controller
> > node because we don't have any 'reg' space.
> 
> No, your PCI controller can perfectly be part of the PCIe node.
You meant 'msi-controller' property to be part of PCIe node?

> 
> > Please let me know whether we require a separate msi file as suggested
> > in your previous comments to separate MSI controller and PCIE
> > controller in two files, if we don't have separate node. If we do not
> > need a separate node do we need to embed MSI controller child node  in
> > PCIe controller node itself, and what properties does this child node
> > will require other than 'interrupts'.
> 
> If you want to keep them in the same file, please at least have two separate
> patches. These are two different functions, and they should be reviewed
> separately.
> 
What I meant is if we don't have separate msi node do we need separate file? 
If you meant msi controller to be part of same node then we will use single 
file and will
try to have two separate patches.

Thanks,
Bharat

> It will help everyone to understand your code, and speed up the reviewing
> process.
> 
> Thanks,
> 
>   M.
> --
> Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-09 Thread Marc Zyngier 
On 09/10/15 09:51, Bharat Kumar Gogada wrote:
>> On 09/10/15 06:11, Bharat Kumar Gogada wrote:
>> +struct nwl_msi {  /* struct nwl_msi - MSI information
 */
>> +  struct msi_controller chip; /* chip: MSI controller */
>
>> We're moving away from msi_controller altogether, as the kernel now
>> has all the necessary infrastructure to do this properly.
>
> Our current GIC version does not have separate msi controller (we
> are not using GICv2m or GICv3), so is it necessary to have separate
> msi controller node ? Please give me clarity on this.

 This has nothing to do with the version of the GIC you are using
 (XGene doesn't have GICv2m or v3 either). This is about reducing code
 duplication and having something that we can maintain. See also
 https://lkml.org/lkml/2015/9/20/193 for yet another example.

 I still plan to kill msi_controller, and I'd like to avoid more
 dependencies with it. MSI domains are the way to do it.

>>> Sorry previously I haven't configured my email client properly so resending.
>>
>> Thanks for doing so, much appreciated.
>>
>>> Since we don't have separate MSI controller, and our PCIe controller
>>> is handling MSI, is it necessary to create a separate MSI controller
>>> node because we don't have any 'reg' space.
>>
>> No, your PCI controller can perfectly be part of the PCIe node.
> You meant 'msi-controller' property to be part of PCIe node?

Yeah, sorry. Too early, not enough coffee.

>>
>>> Please let me know whether we require a separate msi file as suggested
>>> in your previous comments to separate MSI controller and PCIE
>>> controller in two files, if we don't have separate node. If we do not
>>> need a separate node do we need to embed MSI controller child node  in
>>> PCIe controller node itself, and what properties does this child node
>>> will require other than 'interrupts'.
>>
>> If you want to keep them in the same file, please at least have two separate
>> patches. These are two different functions, and they should be reviewed
>> separately.
>>
> What I meant is if we don't have separate msi node do we need separate file? 

That's up to you. Nodes and source code files don't have to match at all.

> If you meant msi controller to be part of same node then we will use single 
> file and will
> try to have two separate patches.

That's fine by me.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-09 Thread Marc Zyngier 
On 09/10/15 06:11, Bharat Kumar Gogada wrote:
 +struct nwl_msi {  /* struct nwl_msi - MSI information
>> */
 +  struct msi_controller chip; /* chip: MSI controller */
>>>
 We're moving away from msi_controller altogether, as the kernel now
 has all the necessary infrastructure to do this properly.
>>>
>>> Our current GIC version does not have separate msi controller (we are
>>> not using GICv2m or GICv3), so is it necessary to have separate msi
>>> controller node ? Please give me clarity on this.
>>
>> This has nothing to do with the version of the GIC you are using (XGene
>> doesn't have GICv2m or v3 either). This is about reducing code duplication
>> and having something that we can maintain. See also
>> https://lkml.org/lkml/2015/9/20/193 for yet another example.
>>
>> I still plan to kill msi_controller, and I'd like to avoid more dependencies 
>> with
>> it. MSI domains are the way to do it.
>>
> Sorry previously I haven't configured my email client properly so resending.

Thanks for doing so, much appreciated.

> Since we don't have separate MSI controller, and our PCIe controller
> is handling MSI, is it necessary to create a separate MSI controller
> node because we don't have any 'reg' space.

No, your PCI controller can perfectly be part of the PCIe node.

> Please let me know whether we require a separate msi file as
> suggested in your previous comments to separate MSI controller and
> PCIE controller in two files, if we don't have separate node. If we
> do not need a separate node do we need to embed MSI controller child
> node  in PCIe controller node itself, and what properties does this
> child node will require other than 'interrupts'.

If you want to keep them in the same file, please at least have two
separate patches. These are two different functions, and they should be
reviewed separately.

It will help everyone to understand your code, and speed up the
reviewing process.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-09 Thread Arnd Bergmann 
On Friday 09 October 2015 09:10:17 Marc Zyngier wrote:
> > Please let me know whether we require a separate msi file as
> > suggested in your previous comments to separate MSI controller and
> > PCIE controller in two files, if we don't have separate node. If we
> > do not need a separate node do we need to embed MSI controller child
> > node  in PCIe controller node itself, and what properties does this
> > child node will require other than 'interrupts'.
> 
> If you want to keep them in the same file, please at least have two
> separate patches. These are two different functions, and they should be
> reviewed separately.
> 
> It will help everyone to understand your code, and speed up the
> reviewing process.

Agreed. Also, the part we really want is to have the PCI controller
code decoupled enough from the MSI code that it will not require
further changes once Xilinx comes out with a chip that has a modern
GIC in it and the same PCIe host.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-09 Thread Marc Zyngier 
On 09/10/15 14:47, Bharat Kumar Gogada wrote:
>> Hi Bharat,
 Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP.
>>> 
>>> +/* SSPL ERROR */ +#define SLVERR   0x02 
>>> +#define DECERR
>>> 0x03 + +struct nwl_msi {/* struct nwl_msi - MSI 
>>> information
>> */
>>> +   struct msi_controller chip; /* chip: MSI controller */
>> 
>> We're moving away from msi_controller altogether, as the kernel now
>> has all the necessary infrastructure to do this properly.
>> 
>> Please convert this driver to msi domains (see
>> drivers/pci/host/pci-xgene- msi.c or the gic-v2m driver as examples
>> of how this is being done).
> 
> As suggested I have gone through the code in pci-gene-msi.c for msi
> domains, and have gone through IRQ Domain documentation. I mainly
> observe when we have more than one interrupt controller involved 
> between device and cpu we use "Hierarchy IRQ domain" (parent and
> child msi domains). But in our case we don't have any such Hierarchy
> and we have single interrupt controller. In such case is it necessary
> to use multiple domains in software which actually isn't case in
> hardware.

This is not optional. The generic MSI domain is stacked on top of the
one that drives the HW. Which is exactly the XGene case. Your case is
not different in any way, and the exact same methods apply:

MSI -> RandomHW -> Interrupt controller (GIC)

We can argue about it as long as you want, but there is no way we're
adding more msi_controller madness to the tree. It took us a long time,
but we now have a generic way to do MSIs in the kernel, and you need to
use it, not sidestep it.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-09 Thread Bharat Kumar Gogada 
> Hi Bharat,
>> > Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP.
> >
> > +/* SSPL ERROR */
> > +#define SLVERR 0x02
> > +#define DECERR 0x03
> > +
> > +struct nwl_msi {   /* struct nwl_msi - MSI information
> */
> > +   struct msi_controller chip; /* chip: MSI controller */
> 
> We're moving away from msi_controller altogether, as the kernel now has all
> the necessary infrastructure to do this properly.
> 
> Please convert this driver to msi domains (see drivers/pci/host/pci-xgene-
> msi.c or the gic-v2m driver as examples of how this is being done).

As suggested I have gone through the code in pci-gene-msi.c for msi domains, 
and have gone through IRQ Domain documentation. 
I mainly observe when we have more than one interrupt controller involved
between device and cpu we use "Hierarchy IRQ domain" (parent and child msi 
domains). But in our case we 
don't have any such Hierarchy and we have single interrupt controller.
In such case is it necessary to use multiple domains in software which actually 
isn't case in hardware.

Thanks 
Bharat
> 
> > +   DECLARE_BITMAP(used, INT_PCI_MSI_NR);   /* used: Declare
> Bitmap
> > +   for MSI */
> > +   struct irq_domain *domain;  /* domain: IRQ domain pointer */
> > +   unsigned long pages;/* pages: MSI pages */
> 
> Overall, I think it would make sense to split the PCIe controller driver and 
> the
> MSI controller (in separate files as well).
> 
> Please keep me posted on the updates.
> 
> Thanks,
> 
>   M.
> --
> Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-08 Thread Bharat Kumar Gogada 
On 06/10/15 17:27, Bharat Kumar Gogada wrote:
> Subject: Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx 
> NWL PCIe Host Controller

[...]

>> +struct nwl_msi {/* struct nwl_msi - MSI information */
>> +struct msi_controller chip; /* chip: MSI controller */
> 
>> We're moving away from msi_controller altogether, as the kernel now 
>> has all the necessary infrastructure to do this properly.
> 
> Our current GIC version does not have separate msi controller (we are 
> not using GICv2m or GICv3), so is it necessary to have separate msi 
> controller node ? Please give me clarity on this.

This has nothing to do with the version of the GIC you are using (XGene doesn't 
have GICv2m or v3 either). This is about reducing code duplication and having 
something that we can maintain. See also
https://lkml.org/lkml/2015/9/20/193 for yet another example.

I still plan to kill msi_controller, and I'd like to avoid more dependencies 
with it. MSI domains are the way to do it.

Since we don't have separate MSI controller, and our PCIe controller is 
handling MSI, is it necessary to create a separate MSI controller node because 
we don't have any 'reg' space. 
Please let me know whether we require a separate msi file as suggested in your 
previous comments to separate MSI controller and PCIE controller in two files, 
if we don't have separate node. 
If we do not need a separate node do we need to embed MSI controller child node 
 in PCIe controller node itself, and what properties does this child node will 
require other than 'interrupts'.

Bharat

Thanks,

M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-08 Thread Bharat Kumar Gogada 
> >> +struct nwl_msi {  /* struct nwl_msi - MSI information
> */
> >> +  struct msi_controller chip; /* chip: MSI controller */
> >
> >> We're moving away from msi_controller altogether, as the kernel now
> >> has all the necessary infrastructure to do this properly.
> >
> > Our current GIC version does not have separate msi controller (we are
> > not using GICv2m or GICv3), so is it necessary to have separate msi
> > controller node ? Please give me clarity on this.
>
> This has nothing to do with the version of the GIC you are using (XGene
> doesn't have GICv2m or v3 either). This is about reducing code duplication
> and having something that we can maintain. See also
> https://lkml.org/lkml/2015/9/20/193 for yet another example.
>
> I still plan to kill msi_controller, and I'd like to avoid more dependencies 
> with
> it. MSI domains are the way to do it.
>
Sorry previously I haven't configured my email client properly so resending.

Since we don't have separate MSI controller, and our PCIe controller is 
handling MSI, is it necessary to create a separate MSI controller node because 
we don't have any 'reg' space.
Please let me know whether we require a separate msi file as suggested in your 
previous comments to separate MSI controller and PCIE controller in two files, 
if we don't have separate node.
If we do not need a separate node do we need to embed MSI controller child node 
 in PCIe controller node itself, and what properties does this child node will 
require other than 'interrupts'.

Bharat
> Thanks,
>
>   M.
> --
> Jazz is not dead. It just smells funny...


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-06 Thread Bharat Kumar Gogada 
Subject: Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe 
Host Controller

Hi Bharat,

On 06/10/15 16:44, Bharat Kumar Gogada wrote:
> Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP.
> 
> Signed-off-by: Bharat Kumar Gogada <bhara...@xilinx.com>
> Signed-off-by: Ravi Kiran Gummaluri <rgum...@xilinx.com>
> ---
> Added interrupt-map, interrupt-map-mask properties
> ---
>  .../devicetree/bindings/pci/xilinx-nwl-pcie.txt|   56 ++
>  drivers/pci/host/Kconfig   |9 +
>  drivers/pci/host/Makefile  |1 +
>  drivers/pci/host/pcie-xilinx-nwl.c | 1029 
> 
>  4 files changed, 1095 insertions(+), 0 deletions(-)  create mode 
> 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
>  create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt 
> b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> new file mode 100644
> index 000..81006ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> @@ -0,0 +1,56 @@
> +* Xilinx NWL PCIe Root Port Bridge DT description
> +
> +Required properties:
> +- compatible: Should contain "xlnx,nwl-pcie-2.11"
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +- reg: Should contain Bridge, PCIe Controller registers location,
> + configuration sapce, and length
> +- reg-names: Must include the following entries:
> + "breg": bridge registers
> + "pcireg": PCIe controller registers
> + "cfg": configuration space region
> +- device_type: must be "pci"
> +- interrupts: Should contain NWL PCIe interrupt
> +- interrupt-names: Must include the following entries:
> + "misc": interrupt asserted when miscellaneous is recieved
> + "intx": interrupt that is asserted when an legacy interrupt is received
> + "msi_1, msi_0": interrupt asserted when msi is recieved
> +- interrupt-map-mask and interrupt-map: standard PCI properties to define the
> + mapping of the PCI interface to interrupt numbers.
> +- ranges: ranges for the PCI memory regions (I/O space region is not
> + supported by hardware)
> + Please refer to the standard PCI bus binding document for a more
> + detailed explanation
> +
> +Optional properties:
> +- xlnx,msi-fifo: To enable MSI FIFO mode
> + - This feature can be used to queue multiple MSI interrupts
> +
> +Example:
> +
> +nwl_pcie: pcie@fd0e {
> + compatible = "xlnx,nwl-pcie-2.11";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + device_type = "pci";
> + interrupt-parent = <>;
> + interrupts = < 0 118 4
> +0 116 4
> +0 115 4  // MSI_1 [63...32]
> +0 114 4 >;   // MSI_0 [31...0]
> + interrupt-names = "misc", "intx", "msi_1", "msi_0";
> + interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> + interrupt-map = <0x0 0x0 0x0 0x1  0x0 116 0x4
> + 0x0 0x0 0x0 0x2  0x0 116 0x4
> + 0x0 0x0 0x0 0x3  0x0 116 0x4
> + 0x0 0x0 0x0 0x4  0x0 116 0x4>;
> + reg = <0x0 0xfd0e 0x1000
> +0x0 0xfd48 0x1000
> +0x0 0xE000 0x100>;
> + reg-names = "breg", "pcireg", "cfg";
> + ranges = <0x0200 0x 0xE100 0x 0xE100 0 
> +0x0F00>; };

Please move this to a separate patch. This is big enough, and hard enough to 
review.

> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index 
> c132bdd..7f5f35e 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -15,6 +15,15 @@ config PCI_MVEBU
>   depends on ARCH_MVEBU || ARCH_DOVE
>   depends on OF
> 
> +config PCIE_XILINX_NWL
> + bool "NWL PCIe Core && PCI_MSI"
> + depends on ARCH_ZYNQMP
> + help
> +  Say 'Y' here if you want kernel to support for Xilinx
> +  NWL PCIe controller.The controller can act as Root Port
> +  or End Point.The current option selection will only

Add a space after the dot.

> +  support root port enabling.
> +
>  config PCIE_DW
>   bool
&g

Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-06 Thread Marc Zyngier 
Hi Bharat,

On 06/10/15 16:44, Bharat Kumar Gogada wrote:
> Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP.
> 
> Signed-off-by: Bharat Kumar Gogada 
> Signed-off-by: Ravi Kiran Gummaluri 
> ---
> Added interrupt-map, interrupt-map-mask properties
> ---
>  .../devicetree/bindings/pci/xilinx-nwl-pcie.txt|   56 ++
>  drivers/pci/host/Kconfig   |9 +
>  drivers/pci/host/Makefile  |1 +
>  drivers/pci/host/pcie-xilinx-nwl.c | 1029 
> 
>  4 files changed, 1095 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
>  create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt 
> b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> new file mode 100644
> index 000..81006ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> @@ -0,0 +1,56 @@
> +* Xilinx NWL PCIe Root Port Bridge DT description
> +
> +Required properties:
> +- compatible: Should contain "xlnx,nwl-pcie-2.11"
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> + interrupt source. The value must be 1.
> +- reg: Should contain Bridge, PCIe Controller registers location,
> + configuration sapce, and length
> +- reg-names: Must include the following entries:
> + "breg": bridge registers
> + "pcireg": PCIe controller registers
> + "cfg": configuration space region
> +- device_type: must be "pci"
> +- interrupts: Should contain NWL PCIe interrupt
> +- interrupt-names: Must include the following entries:
> + "misc": interrupt asserted when miscellaneous is recieved
> + "intx": interrupt that is asserted when an legacy interrupt is received
> + "msi_1, msi_0": interrupt asserted when msi is recieved
> +- interrupt-map-mask and interrupt-map: standard PCI properties to define the
> + mapping of the PCI interface to interrupt numbers.
> +- ranges: ranges for the PCI memory regions (I/O space region is not
> + supported by hardware)
> + Please refer to the standard PCI bus binding document for a more
> + detailed explanation
> +
> +Optional properties:
> +- xlnx,msi-fifo: To enable MSI FIFO mode
> + - This feature can be used to queue multiple MSI interrupts
> +
> +Example:
> +
> +nwl_pcie: pcie@fd0e {
> + compatible = "xlnx,nwl-pcie-2.11";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + device_type = "pci";
> + interrupt-parent = <>;
> + interrupts = < 0 118 4
> +0 116 4
> +0 115 4  // MSI_1 [63...32]
> +0 114 4 >;   // MSI_0 [31...0]
> + interrupt-names = "misc", "intx", "msi_1", "msi_0";
> + interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> + interrupt-map = <0x0 0x0 0x0 0x1  0x0 116 0x4
> + 0x0 0x0 0x0 0x2  0x0 116 0x4
> + 0x0 0x0 0x0 0x3  0x0 116 0x4
> + 0x0 0x0 0x0 0x4  0x0 116 0x4>;
> + reg = <0x0 0xfd0e 0x1000
> +0x0 0xfd48 0x1000
> +0x0 0xE000 0x100>;
> + reg-names = "breg", "pcireg", "cfg";
> + ranges = <0x0200 0x 0xE100 0x 0xE100 0 
> 0x0F00>;
> +};

Please move this to a separate patch. This is big enough, and hard
enough to review.

> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index c132bdd..7f5f35e 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -15,6 +15,15 @@ config PCI_MVEBU
>   depends on ARCH_MVEBU || ARCH_DOVE
>   depends on OF
> 
> +config PCIE_XILINX_NWL
> + bool "NWL PCIe Core && PCI_MSI"
> + depends on ARCH_ZYNQMP
> + help
> +  Say 'Y' here if you want kernel to support for Xilinx
> +  NWL PCIe controller.The controller can act as Root Port
> +  or End Point.The current option selection will only

Add a space after the dot.

> +  support root port enabling.
> +
>  config PCIE_DW
>   bool
> 
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 140d66f..6a56df7 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
>  obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>  obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
> +obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
>  obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
>  obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
>  obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> diff --git a/drivers/pci/host/pcie-xilinx-nwl.c 
>

Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL PCIe Host Controller

2015-10-06 Thread Marc Zyngier 
On 06/10/15 17:27, Bharat Kumar Gogada wrote:
> Subject: Re: [PATCH v3] PCI: Xilinx-NWL-PCIe: Added support for Xilinx NWL 
> PCIe Host Controller

[...]

Please use an email client that does proper quoting - I cannot see what
you are replying to. Or at least annotate your answers so that I can
spot them.

>> +struct nwl_msi {/* struct nwl_msi - MSI information */
>> +struct msi_controller chip; /* chip: MSI controller */
> 
>> We're moving away from msi_controller altogether, as the kernel now
>> has all the necessary infrastructure to do this properly.
> 
> Our current GIC version does not have separate msi controller (we are
> not using GICv2m or GICv3), so is it necessary to have separate msi
> controller node ? Please give me clarity on this.

This has nothing to do with the version of the GIC you are using (XGene
doesn't have GICv2m or v3 either). This is about reducing code
duplication and having something that we can maintain. See also
https://lkml.org/lkml/2015/9/20/193 for yet another example.

I still plan to kill msi_controller, and I'd like to avoid more
dependencies with it. MSI domains are the way to do it.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html