Re: [PATCH 1/3] PCI: dwc: Handle host_init failures

[Joao Pinto]

ie_host_init(struct pcie_port *pp)
>  
>   if (IS_ENABLED(CONFIG_PCI_MSI))
>   dw_pcie_msi_init(pp);
> +
> + return 0;
>  }
>  
>  static const struct dw_pcie_host_ops dw_plat_pcie_host_ops = {
> diff --git a/drivers/pci/dwc/pcie-designware.h 
> b/drivers/pci/dwc/pcie-designware.h
> index b4d2a89f8e58..7366c8167404 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -134,7 +134,7 @@ struct dw_pcie_host_ops {
>unsigned int devfn, int where, int size, u32 *val);
>   int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
>unsigned int devfn, int where, int size, u32 val);
> - void (*host_init)(struct pcie_port *pp);
> + int (*host_init)(struct pcie_port *pp);
>   void (*msi_set_irq)(struct pcie_port *pp, int irq);
>   void (*msi_clear_irq)(struct pcie_port *pp, int irq);
>   phys_addr_t (*get_msi_addr)(struct pcie_port *pp);
> diff --git a/drivers/pci/dwc/pcie-kirin.c b/drivers/pci/dwc/pcie-kirin.c
> index 33fddb9f6739..0b0eb67f2658 100644
> --- a/drivers/pci/dwc/pcie-kirin.c
> +++ b/drivers/pci/dwc/pcie-kirin.c
> @@ -430,9 +430,11 @@ static int kirin_pcie_establish_link(struct pcie_port 
> *pp)
>   return 0;
>  }
>  
> -static void kirin_pcie_host_init(struct pcie_port *pp)
> +static int kirin_pcie_host_init(struct pcie_port *pp)
>  {
>   kirin_pcie_establish_link(pp);
> +
> + return 0;
>  }
>  
>  static struct dw_pcie_ops kirin_dw_pcie_ops = {
> diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
> index 68c5f2ab5bc8..d15657dc3990 100644
> --- a/drivers/pci/dwc/pcie-qcom.c
> +++ b/drivers/pci/dwc/pcie-qcom.c
> @@ -891,7 +891,7 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
>   return !!(val & PCI_EXP_LNKSTA_DLLLA);
>  }
>  
> -static void qcom_pcie_host_init(struct pcie_port *pp)
> +static int qcom_pcie_host_init(struct pcie_port *pp)
>  {
>   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>   struct qcom_pcie *pcie = to_qcom_pcie(pci);
> @@ -921,12 +921,14 @@ static void qcom_pcie_host_init(struct pcie_port *pp)
>   if (ret)
>   goto err;
>  
> - return;
> + return 0;
>  err:
>   qcom_ep_reset_assert(pcie);
>   phy_power_off(pcie->phy);
>  err_deinit:
>   pcie->ops->deinit(pcie);
> +
> + return ret;
>  }
>  
>  static int qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> diff --git a/drivers/pci/dwc/pcie-spear13xx.c 
> b/drivers/pci/dwc/pcie-spear13xx.c
> index 80897291e0fb..52000bc34600 100644
> --- a/drivers/pci/dwc/pcie-spear13xx.c
> +++ b/drivers/pci/dwc/pcie-spear13xx.c
> @@ -177,13 +177,15 @@ static int spear13xx_pcie_link_up(struct dw_pcie *pci)
>   return 0;
>  }
>  
> -static void spear13xx_pcie_host_init(struct pcie_port *pp)
> +static int spear13xx_pcie_host_init(struct pcie_port *pp)
>  {
>   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>   struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pci);
>  
>   spear13xx_pcie_establish_link(spear13xx_pcie);
>   spear13xx_pcie_enable_interrupts(spear13xx_pcie);
> +
> + return 0;
>  }
>  
>  static const struct dw_pcie_host_ops spear13xx_pcie_host_ops = {
> 

A step in the right direction :). In the future we should add host init
validation in the specific SoC drivers, like Layerscape and Qcom have, to assure
that any problem is treated properly in the core driver.

Acked-by: Joao Pinto <jpi...@synopsys.com>

Re: [PATCH 11/37] PCI: dwc: Split pcie-designware.c into host and core files

[Joao Pinto]

Às 11:30 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
> Hi Joao,
> 
> On Monday 16 January 2017 03:57 PM, Joao Pinto wrote:
>>
>> Hi,
>>
>> Às 5:21 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
>>> Hi Joao,
>>>
>>> On Friday 13 January 2017 10:19 PM, Joao Pinto wrote:
>>>> Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
>>>>> Split pcie-designware.c into pcie-designware-host.c that contains
>>>>> the host specific parts of the driver and pcie-designware.c that
>>>>> contains the parts used by both host driver and endpoint driver.
>>>>>
>>>>> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com>
>>>>> ---
>>>>>  drivers/pci/dwc/Makefile   |2 +-
>>>>>  drivers/pci/dwc/pcie-designware-host.c |  619 
>>>>> 
>>>>>  drivers/pci/dwc/pcie-designware.c  |  613 
>>>>> +--
>>>>>  drivers/pci/dwc/pcie-designware.h  |8 +
>>>>>  4 files changed, 634 insertions(+), 608 deletions(-)
>>>>>  create mode 100644 drivers/pci/dwc/pcie-designware-host.c
>>>>>
>>>>> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
>>>>> index 7d27c14..3b57e55 100644
>>>>> --- a/drivers/pci/dwc/Makefile
>>>>> +++ b/drivers/pci/dwc/Makefile
>>>>> @@ -1,4 +1,4 @@
>>>>
>>>> (snip...)
>>>>
>>>>> -static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>>>> -   int type, u64 cpu_addr, u64 pci_addr,
>>>>> -   u32 size)
>>>>> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>>>>> +u64 cpu_addr, u64 pci_addr, u32 size)
>>>>>  {
>>>>>   u32 retries, val;
>>>>>  
>>>>> @@ -186,220 +151,6 @@ static void dw_pcie_prog_outbound_atu(struct 
>>>>> dw_pcie *pci, int index,
>>>>>   dev_err(pci->dev, "iATU is not being enabled\n");
>>>>>  }
>>>>
>>>> Kishon, iATU only makes sense in The Root Complex (host), so it should be 
>>>> inside
>>>> the pcie-designware-host.
>>>
>>> That is not true. Outbound ATU should be programmed to access host side 
>>> buffers
>>> and inbound ATU should be programmed for the host to access EP mem space.
>>
>> Sorry, I was not clear enough. What I was trying to suggest is, since the ATU
>> programming is done by the host, wouldn't be better to include it in the
>> pcie-designware-host? It is just an architectural detail.
> 
> ATU programming is required in EP mode. See "[PATCH 24/37] PCI: dwc:
> designware: Add EP mode support" in this patch series.
> 
> Anything that's required by both EP mode and RC mode, I've placed in
> pcie-designware.c

Agreed!

> 
> Thanks
> Kishon
> 

Re: [PATCH 11/37] PCI: dwc: Split pcie-designware.c into host and core files

[Joao Pinto]

Hi,

Às 5:21 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
> Hi Joao,
> 
> On Friday 13 January 2017 10:19 PM, Joao Pinto wrote:
>> Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
>>> Split pcie-designware.c into pcie-designware-host.c that contains
>>> the host specific parts of the driver and pcie-designware.c that
>>> contains the parts used by both host driver and endpoint driver.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com>
>>> ---
>>>  drivers/pci/dwc/Makefile   |2 +-
>>>  drivers/pci/dwc/pcie-designware-host.c |  619 
>>> 
>>>  drivers/pci/dwc/pcie-designware.c  |  613 
>>> +--
>>>  drivers/pci/dwc/pcie-designware.h  |8 +
>>>  4 files changed, 634 insertions(+), 608 deletions(-)
>>>  create mode 100644 drivers/pci/dwc/pcie-designware-host.c
>>>
>>> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
>>> index 7d27c14..3b57e55 100644
>>> --- a/drivers/pci/dwc/Makefile
>>> +++ b/drivers/pci/dwc/Makefile
>>> @@ -1,4 +1,4 @@
>>
>> (snip...)
>>
>>> -static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>> - int type, u64 cpu_addr, u64 pci_addr,
>>> - u32 size)
>>> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>>> +  u64 cpu_addr, u64 pci_addr, u32 size)
>>>  {
>>> u32 retries, val;
>>>  
>>> @@ -186,220 +151,6 @@ static void dw_pcie_prog_outbound_atu(struct dw_pcie 
>>> *pci, int index,
>>> dev_err(pci->dev, "iATU is not being enabled\n");
>>>  }
>>
>> Kishon, iATU only makes sense in The Root Complex (host), so it should be 
>> inside
>> the pcie-designware-host.
> 
> That is not true. Outbound ATU should be programmed to access host side 
> buffers
> and inbound ATU should be programmed for the host to access EP mem space.

Sorry, I was not clear enough. What I was trying to suggest is, since the ATU
programming is done by the host, wouldn't be better to include it in the
pcie-designware-host? It is just an architectural detail.

> 
> Thanks
> Kishon
> 

Re: [PATCH 09/37] PCI: dwc: designware: Parse *num-lanes* property in dw_pcie_setup_rc

[Joao Pinto]

Hi,

Às 5:19 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
> Hi,
> 
> On Friday 13 January 2017 10:43 PM, Joao Pinto wrote:
>> Hi,
>>
>> Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
>>> *num-lanes* dt property is parsed in dw_pcie_host_init. However
>>> *num-lanes* property is applicable to both root complex mode and
>>> endpoint mode. As a first step, move the parsing of this property
>>> outside dw_pcie_host_init. This is in preparation for splitting
>>> pcie-designware.c to pcie-designware.c and pcie-designware-host.c
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com>
>>> ---
>>>  drivers/pci/dwc/pcie-designware.c |   18 +++---
>>>  drivers/pci/dwc/pcie-designware.h |1 -
>>>  2 files changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/pci/dwc/pcie-designware.c 
>>> b/drivers/pci/dwc/pcie-designware.c
>>> index 00a0fdc..89cdb6b 100644
>>> --- a/drivers/pci/dwc/pcie-designware.c
>>> +++ b/drivers/pci/dwc/pcie-designware.c
>>> @@ -551,10 +551,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>> }
>>> }
>>>  
>>> -   ret = of_property_read_u32(np, "num-lanes", >lanes);
>>> -   if (ret)
>>> -   pci->lanes = 0;
>>> -
>>> ret = of_property_read_u32(np, "num-viewport", >num_viewport);
>>> if (ret)
>>> pci->num_viewport = 2;
>>> @@ -751,18 +747,26 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 
>>> devfn,
>>>  
>>>  void dw_pcie_setup_rc(struct pcie_port *pp)
>>>  {
>>> +   int ret;
>>> +   u32 lanes;
>>> u32 val;
>>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>> +   struct device *dev = pci->dev;
>>> +   struct device_node *np = dev->of_node;
>>>  
>>> /* get iATU unroll support */
>>> pci->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pci);
>>> dev_dbg(pci->dev, "iATU unroll: %s\n",
>>> pci->iatu_unroll_enabled ? "enabled" : "disabled");
>>>  
>>> +   ret = of_property_read_u32(np, "num-lanes", );
>>> +   if (ret)
>>> +   lanes = 0;
>>
>> You moved from host_init to root complex setup function, which in my opinion 
>> did
>> not improve (in this scope).
>>
>> I suggest that instead of making so much intermediary patches, which is nice 
>> to
>> understand your development sequence, but hard to review. Wouldn't be better 
>> to
>> condense some of the patches? We would have a cloear vision of the final 
>> product :)
> 
> I thought the other way. If squashing patches is easier to review, I'll do it.

I understand. To break it in small pieces is good to understand clearly what is
done and how was done, but I would break too much. That's a personal opinion of
course, lets see what others say :).

Thanks,
Joao

> 
> Btw, thanks for reviewing.
> 
> Cheers
> Kishon
> 

Re: [PATCH 12/37] PCI: dwc: Create a new config symbol to enable pci dwc host

[Joao Pinto]

Hi Kishon,

Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Now that pci designware host has a separate file, create a new
> config symbol to select the host only driver. This is in preparation
> to enable endpoint support to designware driver.
> 
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  drivers/pci/dwc/Kconfig   |   26 +++---
>  drivers/pci/dwc/Makefile  |3 ++-
>  drivers/pci/dwc/pcie-designware.h |   29 +
>  3 files changed, 42 insertions(+), 16 deletions(-)
> 

You are already working in a base where dwc/ already exists. I know you made a
rename / re-structure patch for pci, but I think it was not yet accepted, right?
I don't see it in any of Bjorn' dev branches.

Thanks.

> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> index 8b08519..d0bdfb5 100644
> --- a/drivers/pci/dwc/Kconfig
> +++ b/drivers/pci/dwc/Kconfig
> @@ -3,13 +3,17 @@ menu "DesignWare PCI Core Support"
>  
>  config PCIE_DW
>   bool
> +
> +config PCIE_DW_HOST
> +bool
>   depends on PCI_MSI_IRQ_DOMAIN
> +select PCIE_DW
>  
>  config PCI_DRA7XX
>   bool "TI DRA7xx PCIe controller"
>   depends on OF && HAS_IOMEM && TI_PIPE3
>   depends on PCI_MSI_IRQ_DOMAIN
> - select PCIE_DW
> + select PCIE_DW_HOST
>   help
>Enables support for the PCIe controller in the DRA7xx SoC.  There
>are two instances of PCIe controller in DRA7xx.  This controller can
> @@ -18,7 +22,7 @@ config PCI_DRA7XX
>  config PCIE_DW_PLAT
>   bool "Platform bus based DesignWare PCIe Controller"
>   depends on PCI_MSI_IRQ_DOMAIN
> - select PCIE_DW
> + select PCIE_DW_HOST
>   ---help---
>This selects the DesignWare PCIe controller support. Select this if
>you have a PCIe controller on Platform bus.
> @@ -32,21 +36,21 @@ config PCI_EXYNOS
>   depends on SOC_EXYNOS5440 || COMPILE_TEST
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + select PCIE_DW_HOST
>  
>  config PCI_IMX6
>   bool "Freescale i.MX6 PCIe controller"
>   depends on SOC_IMX6Q || COMPILE_TEST
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + select PCIE_DW_HOST
>  
>  config PCIE_SPEAR13XX
>   bool "STMicroelectronics SPEAr PCIe controller"
>   depends on ARCH_SPEAR13XX || COMPILE_TEST
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + select PCIE_DW_HOST
>   help
> Say Y here if you want PCIe support on SPEAr13XX SoCs.
>  
> @@ -55,7 +59,7 @@ config PCI_KEYSTONE
>   depends on ARCH_KEYSTONE || COMPILE_TEST
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + select PCIE_DW_HOST
>   help
> Say Y here if you want to enable PCI controller support on Keystone
> SoCs. The PCI controller on Keystone is based on Designware hardware
> @@ -67,7 +71,7 @@ config PCI_LAYERSCAPE
>   depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
>   depends on PCI_MSI_IRQ_DOMAIN
>   select MFD_SYSCON
> - select PCIE_DW
> + select PCIE_DW_HOST
>   help
> Say Y here if you want PCIe controller support on Layerscape SoCs.
>  
> @@ -76,7 +80,7 @@ config PCI_HISI
>   bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + select PCIE_DW_HOST
>   help
> Say Y here if you want PCIe controller support on HiSilicon
> Hip05 and Hip06 SoCs
> @@ -86,7 +90,7 @@ config PCIE_QCOM
>   depends on (ARCH_QCOM || COMPILE_TEST) && OF
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + select PCIE_DW_HOST
>   help
> Say Y here to enable PCIe controller support on Qualcomm SoCs. The
> PCIe controller uses the Designware core plus Qualcomm-specific
> @@ -97,7 +101,7 @@ config PCIE_ARMADA_8K
>   depends on ARCH_MVEBU || COMPILE_TEST
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + select PCIE_DW_HOST
>   help
> Say Y here if you want to enable PCIe controller support on
> Armada-8K SoCs. The PCIe controller on Armada-8K is based on
> @@ -109,7 +113,7 @@ config PCIE_ARTPEC6
>   depends on MACH_ARTPEC6 || COMPILE_TEST
>   depends on PCI_MSI_IRQ_DOMAIN
>   select PCIEPORTBUS
> - select PCIE_DW
> + 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.
> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
> index 3b57e55..a2df13c 100644
> --- a/drivers/pci/dwc/Makefile
> +++ b/drivers/pci/dwc/Makefile
> @@ -1,4 +1,5 @@
> -obj-$(CONFIG_PCIE_DW) += pcie-designware.o 

Re: [PATCH 07/37] PCI: dwc: designware: Get device pointer at the start of dw_pcie_host_init

[Joao Pinto]

Hi!

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. Get device pointer at the beginning of
> dw_pcie_host_init instead of getting it all over dw_pcie_host_init.
> This is in preparation for splitting struct pcie_port into host and
> core structures (Once split pcie_port will not have device pointer).
> 

I think change should be condense with another patch to reduce the patch-set
number and to give it real meaning. I understand why you are breaking so much
the patch, it is has a lot of changes, but in my opinion is too much fragmented.

Thanks
Joao

Re: [PATCH 09/37] PCI: dwc: designware: Parse *num-lanes* property in dw_pcie_setup_rc

[Joao Pinto]

Hi,

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> *num-lanes* dt property is parsed in dw_pcie_host_init. However
> *num-lanes* property is applicable to both root complex mode and
> endpoint mode. As a first step, move the parsing of this property
> outside dw_pcie_host_init. This is in preparation for splitting
> pcie-designware.c to pcie-designware.c and pcie-designware-host.c
> 
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  drivers/pci/dwc/pcie-designware.c |   18 +++---
>  drivers/pci/dwc/pcie-designware.h |1 -
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware.c 
> b/drivers/pci/dwc/pcie-designware.c
> index 00a0fdc..89cdb6b 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -551,10 +551,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
>   }
>   }
>  
> - ret = of_property_read_u32(np, "num-lanes", >lanes);
> - if (ret)
> - pci->lanes = 0;
> -
>   ret = of_property_read_u32(np, "num-viewport", >num_viewport);
>   if (ret)
>   pci->num_viewport = 2;
> @@ -751,18 +747,26 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 
> devfn,
>  
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
> + int ret;
> + u32 lanes;
>   u32 val;
>   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct device *dev = pci->dev;
> + struct device_node *np = dev->of_node;
>  
>   /* get iATU unroll support */
>   pci->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pci);
>   dev_dbg(pci->dev, "iATU unroll: %s\n",
>   pci->iatu_unroll_enabled ? "enabled" : "disabled");
>  
> + ret = of_property_read_u32(np, "num-lanes", );
> + if (ret)
> + lanes = 0;

You moved from host_init to root complex setup function, which in my opinion did
not improve (in this scope).

I suggest that instead of making so much intermediary patches, which is nice to
understand your development sequence, but hard to review. Wouldn't be better to
condense some of the patches? We would have a cloear vision of the final 
product :)

Joao

> +
>   /* set the number of lanes */
>   val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
>   val &= ~PORT_LINK_MODE_MASK;
> - switch (pci->lanes) {
> + switch (lanes) {
>   case 1:
>   val |= PORT_LINK_MODE_1_LANES;
>   break;
> @@ -776,7 +780,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>   val |= PORT_LINK_MODE_8_LANES;
>   break;
>   default:
> - dev_err(pci->dev, "num-lanes %u: invalid value\n", pci->lanes);
> + dev_err(pci->dev, "num-lanes %u: invalid value\n", lanes);
>   return;
>   }
>   dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> @@ -784,7 +788,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>   /* set link width speed control register */
>   val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>   val &= ~PORT_LOGIC_LINK_WIDTH_MASK;
> - switch (pci->lanes) {
> + switch (lanes) {
>   case 1:
>   val |= PORT_LOGIC_LINK_WIDTH_1_LANES;
>   break;
> diff --git a/drivers/pci/dwc/pcie-designware.h 
> b/drivers/pci/dwc/pcie-designware.h
> index d4b3d43..491fbe3 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -148,7 +148,6 @@ struct dw_pcie_ops {
>  struct dw_pcie {
>   struct device   *dev;
>   void __iomem*dbi_base;
> - u32 lanes;
>   u32 num_viewport;
>   u8  iatu_unroll_enabled;
>   struct pcie_portpp;
> 

Re: [PATCH 05/37] PCI: dwc: Add platform_set_drvdata

[Joao Pinto]

Hi,

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Add platform_set_drvdata in all designware based drivers to store the
> private data structure of the driver so that dev_set_drvdata can be
> used to get back private data pointer in add_pcie_port/host_init.
> This is in preparation for splitting struct pcie_port into core and
> host only structures. After the split pcie_port will not be part of
> the driver's private data structure and *container_of* used now
> to get the private data pointer cannot be used.
> 
> Cc: Jingoo Han <jingooh...@gmail.com>
> Cc: Richard Zhu <hongxing@nxp.com>
> Cc: Lucas Stach <l.st...@pengutronix.de>
> Cc: Murali Karicheri <m-kariche...@ti.com>
> Cc: Minghuan Lian <minghuan.l...@freescale.com>
> Cc: Mingkai Hu <mingkai...@freescale.com>
> Cc: Roy Zang <tie-fei.z...@freescale.com>
> Cc: Thomas Petazzoni <thomas.petazz...@free-electrons.com>
> Cc: Niklas Cassel <niklas.cas...@axis.com>
> Cc: Jesper Nilsson <jesper.nils...@axis.com>
> Cc: Joao Pinto <joao.pi...@synopsys.com>
> Cc: Zhou Wang <wangzh...@hisilicon.com>
> Cc: Gabriele Paoloni <gabriele.paol...@huawei.com>
> Cc: Stanimir Varbanov <svarba...@mm-sol.com>
> Cc: Pratyush Anand <pratyush.an...@gmail.com>
> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com>
> ---
>  drivers/pci/dwc/pci-dra7xx.c   |3 ++-
>  drivers/pci/dwc/pci-exynos.c   |3 ++-
>  drivers/pci/dwc/pci-imx6.c |3 ++-
>  drivers/pci/dwc/pci-keystone.c |2 ++
>  drivers/pci/dwc/pci-layerscape.c   |2 ++
>  drivers/pci/dwc/pcie-armada8k.c|2 ++
>  drivers/pci/dwc/pcie-artpec6.c |2 ++
>  drivers/pci/dwc/pcie-designware-plat.c |2 ++
>  drivers/pci/dwc/pcie-hisi.c|2 ++
>  drivers/pci/dwc/pcie-qcom.c|2 ++
>  drivers/pci/dwc/pcie-spear13xx.c   |3 ++-
>  11 files changed, 22 insertions(+), 4 deletions(-)
> 

This is an example of a change that could be merged with another patch that
gives it more meaning, like I wrote in the review of patch 9/37.

Thanks,
Joao

Re: [PATCH 11/37] PCI: dwc: Split pcie-designware.c into host and core files

[Joao Pinto]

Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Split pcie-designware.c into pcie-designware-host.c that contains
> the host specific parts of the driver and pcie-designware.c that
> contains the parts used by both host driver and endpoint driver.
> 
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  drivers/pci/dwc/Makefile   |2 +-
>  drivers/pci/dwc/pcie-designware-host.c |  619 
> 
>  drivers/pci/dwc/pcie-designware.c  |  613 +--
>  drivers/pci/dwc/pcie-designware.h  |8 +
>  4 files changed, 634 insertions(+), 608 deletions(-)
>  create mode 100644 drivers/pci/dwc/pcie-designware-host.c
> 
> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
> index 7d27c14..3b57e55 100644
> --- a/drivers/pci/dwc/Makefile
> +++ b/drivers/pci/dwc/Makefile
> @@ -1,4 +1,4 @@

(snip...)

> -static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> -   int type, u64 cpu_addr, u64 pci_addr,
> -   u32 size)
> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> +u64 cpu_addr, u64 pci_addr, u32 size)
>  {
>   u32 retries, val;
>  
> @@ -186,220 +151,6 @@ static void dw_pcie_prog_outbound_atu(struct dw_pcie 
> *pci, int index,
>   dev_err(pci->dev, "iATU is not being enabled\n");
>  }

Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host.

>  
> -static struct irq_chip dw_msi_irq_chip = {
> - .name = "PCI-MSI",
> - .irq_enable = pci_msi_unmask_irq,
> - .irq_disable = pci_msi_mask_irq,
> - .irq_mask = pci_msi_mask_irq,
> - .irq_unmask = pci_msi_unmask_irq,
> -};
> -

(snip...)

> -
> -static const struct irq_domain_ops msi_domain_ops = {
> - .map = dw_pcie_msi_map,
> -};
> -
>  static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
>  {
>   u32 val;
> @@ -454,303 +192,11 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie 
> *pci)
>   return 0;
>  }

Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host.

(snip...)

> diff --git a/drivers/pci/dwc/pcie-designware.h 
> b/drivers/pci/dwc/pcie-designware.h
> index 491fbe3..808d17b 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -14,6 +14,10 @@
>  #ifndef _PCIE_DESIGNWARE_H
>  #define _PCIE_DESIGNWARE_H
>  
> +#include 
> +#include 
> +#include 
> +
>  /* Parameters for the waiting for link up routine */
>  #define LINK_WAIT_MAX_RETRIES10
>  #define LINK_WAIT_USLEEP_MIN 9
> @@ -167,4 +171,8 @@ struct dw_pcie {
>  void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val);
>  int dw_pcie_link_up(struct dw_pcie *pci);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> +int type, u64 cpu_addr, u64 pci_addr,
> +u32 size);
> +void dw_pcie_setup(struct dw_pcie *pci);


Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host as static.

>  #endif /* _PCIE_DESIGNWARE_H */
> 

Thanks,
Joao

Re: [PATCH 06/37] PCI: dwc: Rename cfg_read/cfg_write to read/write

[Joao Pinto]

Hi,

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. dw_pcie_cfg_read/dw_pcie_cfg_write doesn't do
> anything specific to access configuration space. It can be just renamed
> to dw_pcie_read/dw_pcie_write and used to read/write data to dbi space.
> This is in preparation for added endpoint support to linux kernel.
> 
> Cc: Jingoo Han <jingooh...@gmail.com>
> Cc: Murali Karicheri <m-kariche...@ti.com>
> Cc: Joao Pinto <joao.pi...@synopsys.com>
> Cc: Stanimir Varbanov <svarba...@mm-sol.com>
> Cc: Pratyush Anand <pratyush.an...@gmail.com>
> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com>
> ---
>  drivers/pci/dwc/pci-dra7xx.c  |   16 
>  drivers/pci/dwc/pci-exynos.c  |4 ++--
>  drivers/pci/dwc/pci-keystone-dw.c |4 ++--
>  drivers/pci/dwc/pcie-designware.c |   12 ++--
>  drivers/pci/dwc/pcie-designware.h |4 ++--
>  drivers/pci/dwc/pcie-qcom.c   |2 +-
>  drivers/pci/dwc/pcie-spear13xx.c  |   24 
>  7 files changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index aeeab74..38b0c9a 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -114,22 +114,22 @@ static int dra7xx_pcie_establish_link(struct 
> dra7xx_pcie *dra7xx)
>   }
>  
>   if (dra7xx->link_gen == 1) {
> - dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> -  4, );
> + dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> +  4, );
>   if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
>   reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
>   reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> -   PCI_EXP_LNKCAP, 4, reg);
> + dw_pcie_write(pp->dbi_base + exp_cap_off +
> +   PCI_EXP_LNKCAP, 4, reg);
>   }
>  
> - dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> -  2, );
> + dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> +  2, );
>   if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
>   reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
>   reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> -   PCI_EXP_LNKCTL2, 2, reg);
> + dw_pcie_write(pp->dbi_base + exp_cap_off +
> +   PCI_EXP_LNKCTL2, 2, reg);
>   }
>   }
>  
> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
> index c179e7a..e3fbff4 100644
> --- a/drivers/pci/dwc/pci-exynos.c
> +++ b/drivers/pci/dwc/pci-exynos.c
> @@ -429,7 +429,7 @@ static int exynos_pcie_rd_own_conf(struct pcie_port *pp, 
> int where, int size,
>   int ret;
>  
>   exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
> - ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> + ret = dw_pcie_read(pp->dbi_base + where, size, val);
>   exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
>   return ret;
>  }
> @@ -441,7 +441,7 @@ static int exynos_pcie_wr_own_conf(struct pcie_port *pp, 
> int where, int size,
>   int ret;
>  
>   exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
> - ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
> + ret = dw_pcie_write(pp->dbi_base + where, size, val);
>   exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
>   return ret;
>  }
> diff --git a/drivers/pci/dwc/pci-keystone-dw.c 
> b/drivers/pci/dwc/pci-keystone-dw.c
> index 9397c46..4875334 100644
> --- a/drivers/pci/dwc/pci-keystone-dw.c
> +++ b/drivers/pci/dwc/pci-keystone-dw.c
> @@ -444,7 +444,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct 
> pci_bus *bus,
>  
>   addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
>  
> - return dw_pcie_cfg_read(addr + where, size, val);
> + return dw_pcie_read(addr + where, size, val);
>  }
>  
>  int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> @@ -456,7 +456,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct 
> pci_bus *bus,
>  
>   addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
>  
> - return dw_pcie_cfg_write(addr + where, size, val);
&

Re: [PATCH 10/37] PCI: dwc: designware: Fix style errors in pcie-designware.c

[Joao Pinto]

 pcie_port *pp)
>   break;
>   case 0:
>   pp->cfg = win->res;
> - pp->cfg0_size = resource_size(pp->cfg)/2;
> - pp->cfg1_size = resource_size(pp->cfg)/2;
> + pp->cfg0_size = resource_size(pp->cfg) / 2;
> + pp->cfg1_size = resource_size(pp->cfg) / 2;
>   pp->cfg0_base = pp->cfg->start;
>   pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
>   break;
> @@ -615,7 +615,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  }
>  
>  static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> - u32 devfn, int where, int size, u32 *val)
> +  u32 devfn, int where, int size, u32 *val)
>  {
>   int ret, type;
>   u32 busdev, cfg_size;
> @@ -654,7 +654,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, 
> struct pci_bus *bus,
>  }
>  
>  static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> - u32 devfn, int where, int size, u32 val)
> +  u32 devfn, int where, int size, u32 val)
>  {
>   int ret, type;
>   u32 busdev, cfg_size;
> @@ -711,7 +711,7 @@ static int dw_pcie_valid_device(struct pcie_port *pp, 
> struct pci_bus *bus,
>  }
>  
>  static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> - int size, u32 *val)
> +int size, u32 *val)
>  {
>   struct pcie_port *pp = bus->sysdata;
>  
> @@ -727,7 +727,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 
> devfn, int where,
>  }
>  
>  static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
> - int where, int size, u32 val)
> +int where, int size, u32 val)
>  {
>   struct pcie_port *pp = bus->sysdata;
>  
> 

Always good to make clean up! Thanks!

Acked-By: Joao Pinto <jpi...@synopsys.com>

Re: [PATCH 02/37] PCI: dwc: designware: Add new *ops* for cpu addr fixup

[Joao Pinto]

Hi Kishon,

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Some platforms (like dra7xx) require only the least 28 bits of the
> corresponding 32 bit CPU address to be programmed in the address
> translation unit. This modified address is stored in io_base/mem_base/
> cfg0_base/cfg1_base in dra7xx_pcie_host_init. While this is okay for
> host mode where the address range is fixed, device mode requires
> different addresses to be programmed based on the host buffer address.
> Add a new ops to get the least 28 bits of the corresponding 32 bit
> CPU address and invoke it before programming the address translation
> unit.
> 
> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com>
> ---
>  drivers/pci/dwc/pcie-designware.c |3 +++
>  drivers/pci/dwc/pcie-designware.h |1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pcie-designware.c 
> b/drivers/pci/dwc/pcie-designware.c
> index bed1999..d68bc7b 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -195,6 +195,9 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port 
> *pp, int index,
>  {
>   u32 retries, val;
>  
> + if (pp->ops->cpu_addr_fixup)
> + cpu_addr = pp->ops->cpu_addr_fixup(cpu_addr);
> +
>   if (pp->iatu_unroll_enabled) {
>   dw_pcie_writel_unroll(pp, index, PCIE_ATU_UNR_LOWER_BASE,
>   lower_32_bits(cpu_addr));
> diff --git a/drivers/pci/dwc/pcie-designware.h 
> b/drivers/pci/dwc/pcie-designware.h
> index a567ea2..32f4602 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -54,6 +54,7 @@ struct pcie_port {
>  };
>  
>  struct pcie_host_ops {
> + u64 (*cpu_addr_fixup)(u64 cpu_addr);
>   u32 (*readl_rc)(struct pcie_port *pp, u32 reg);
>   void (*writel_rc)(struct pcie_port *pp, u32 reg, u32 val);
>   int (*rd_own_conf)(struct pcie_port *pp, int where, int size, u32 *val);
> 

I think this is an acceptable fixup, I am ok with it.

Reviewed-By: Joao Pinto <jpi...@synopsys.com>

Joao

Re: [PATCH 04/37] PCI: dwc: designware: Move the register defines to designware header file

[Joao Pinto]

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. Move the register defines and other macros from
> pcie-designware.c to pcie-designware.h. This is in preparation to
> split the pcie-designware.c file into designware core file and host
> specific file.
> 
> While at that also fix a checkpatch warning.
> 
> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com>
> ---
>  drivers/pci/dwc/pcie-designware.c |   70 
>  drivers/pci/dwc/pcie-designware.h |   71 
> +
>  2 files changed, 71 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware.c 
> b/drivers/pci/dwc/pcie-designware.c
> index d68bc7b..0b928dc 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -25,76 +25,6 @@
>  
>  #include "pcie-designware.h"

Make sense.

Reviewed-By: Joao Pinto <jpi...@synopsys.com>

Re: [PATCH v2] PCI: designware: add host_init error handling

[Joao Pinto]

Hi Srinivas!

Thanks for the update!

Acked-By: Joao Pinto <jpi...@synopsys.com>

Às 10:32 AM de 12/7/2016, Srinivas Kandagatla escreveu:
> This patch add support to return value from host_init() callback from drivers,
> so that the designware libary can handle or pass it to proper place. Issue 
> with
> void return type is that errors or error handling within host_init() callback
> are never know to designware code, which could go ahead and access registers
> even in error cases.
> 
> Typical case in qcom controller driver is to turn off clks in case of errors,
> if designware code continues to read/write register when clocks are turned off
> the board would reboot/lockup.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> ---
> Currently designware code does not have a way return errors generated
> as part of host_init() callback in controller drivers. This is an issue
> with controller drivers like qcom which turns off the clocks in error
> handling path. As the dw core is un aware of this would continue to
> access registers which faults resulting in board reboots/hangs.
> 
> There are two ways to solve this issue,
> one is remove error handling in the qcom controller host_init() function
> other is to handle error and pass back to dw core code which would then
> pass back to controller driver as part of dw_pcie_host_init() return value.
> 
> Second option seems more sensible and correct way to fix the issue,
> this patch does the same.
> 
> As part of this change to host_init() return type I had to patch other
> ihost controller drivers which use dw core. Most of the changes to other 
> drivers
> are to return proper error codes to upper layer.
> Only compile tested drivers.
> 
> Changes since RFC:
>   - Add error handling to other drivers as suggested by Joao Pinto
> 
>  drivers/pci/host/pci-dra7xx.c   | 10 --
>  drivers/pci/host/pci-exynos.c   | 10 --
>  drivers/pci/host/pci-imx6.c | 10 --
>  drivers/pci/host/pci-keystone.c | 10 --
>  drivers/pci/host/pci-layerscape.c   | 22 +-
>  drivers/pci/host/pcie-armada8k.c|  4 +++-
>  drivers/pci/host/pcie-designware-plat.c | 10 --
>  drivers/pci/host/pcie-designware.c  |  4 +++-
>  drivers/pci/host/pcie-designware.h  |  2 +-
>  drivers/pci/host/pcie-qcom.c|  5 +++--
>  drivers/pci/host/pcie-spear13xx.c   | 10 --
>  11 files changed, 71 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 9595fad..811f0f9 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -127,9 +127,10 @@ static void dra7xx_pcie_enable_interrupts(struct 
> dra7xx_pcie *dra7xx)
>  LEG_EP_INTERRUPTS);
>  }
>  
> -static void dra7xx_pcie_host_init(struct pcie_port *pp)
> +static int dra7xx_pcie_host_init(struct pcie_port *pp)
>  {
>   struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> + int ret;
>  
>   pp->io_base &= DRA7XX_CPU_TO_BUS_ADDR;
>   pp->mem_base &= DRA7XX_CPU_TO_BUS_ADDR;
> @@ -138,10 +139,15 @@ static void dra7xx_pcie_host_init(struct pcie_port *pp)
>  
>   dw_pcie_setup_rc(pp);
>  
> - dra7xx_pcie_establish_link(dra7xx);
> + ret = dra7xx_pcie_establish_link(dra7xx);
> + if (ret < 0)
> + return ret;
> +
>   if (IS_ENABLED(CONFIG_PCI_MSI))
>   dw_pcie_msi_init(pp);
>   dra7xx_pcie_enable_interrupts(dra7xx);
> +
> + return 0;
>  }
>  
>  static struct pcie_host_ops dra7xx_pcie_host_ops = {
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index f1c544b..c116fd9 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -458,12 +458,18 @@ static int exynos_pcie_link_up(struct pcie_port *pp)
>   return 0;
>  }
>  
> -static void exynos_pcie_host_init(struct pcie_port *pp)
> +static int exynos_pcie_host_init(struct pcie_port *pp)
>  {
>   struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> + int ret;
> +
> + ret = exynos_pcie_establish_link(exynos_pcie);
> + if (ret < 0)
> + return ret;
>  
> - exynos_pcie_establish_link(exynos_pcie);
>   exynos_pcie_enable_interrupts(exynos_pcie);
> +
> + return 0;
>  }
>  
>  static struct pcie_host_ops exynos_pcie_host_ops = {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index c8cefb0..1251e92 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-

Re: [RFC PATCH] PCI: designware: add host_init() error handling

[Joao Pinto]

Às 11:51 AM de 12/2/2016, Srinivas Kandagatla escreveu:
> 
> 
> On 02/12/16 10:32, Joao Pinto wrote:
>>
>> Hi Srinivas,
>>
>> Às 11:51 AM de 12/1/2016, Srinivas Kandagatla escreveu:
>>>  drivers/pci/host/pci-dra7xx.c   |  4 +++-
>>>  drivers/pci/host/pci-exynos.c   |  4 +++-
>>>  drivers/pci/host/pci-imx6.c |  4 +++-
>>>  drivers/pci/host/pci-keystone.c |  4 +++-
>>>  drivers/pci/host/pci-layerscape.c   | 12 
>>>  drivers/pci/host/pcie-armada8k.c|  4 +++-
>>>  drivers/pci/host/pcie-designware-plat.c |  4 +++-
>>>  drivers/pci/host/pcie-designware.c  |  4 +++-
>>>  drivers/pci/host/pcie-designware.h  |  2 +-
>>>  drivers/pci/host/pcie-qcom.c|  6 --
>>>  drivers/pci/host/pcie-spear13xx.c   |  4 +++-
>>>  11 files changed, 37 insertions(+), 15 deletions(-)
>>>
>>
>> Thanks for the patch!
>>
>> In my opinion your idea is good but only qcom driver is able to detect 
>> failure
>> in the specific host init routine, all others have a 'return 0' even if
>> something not well init. I would recomend that we take this issue a bit 
>> further
>> and add the error checking to all specific pci drivers in order to make them 
>> as
>> robust as qcom'.
> I totally agree with you, I can give this a go in next version.

Sure, but I think it would be better to finish now since we are on top of the
task. I can help you if you need.

Thanks Joao

> 
> Thanks,
> srini
> 
>>
>> Thanks,
>> Joao
>>

Re: [RFC PATCH] PCI: designware: add host_init() error handling

[Joao Pinto]

Hi Srinivas,

Às 11:51 AM de 12/1/2016, Srinivas Kandagatla escreveu:
>  drivers/pci/host/pci-dra7xx.c   |  4 +++-
>  drivers/pci/host/pci-exynos.c   |  4 +++-
>  drivers/pci/host/pci-imx6.c |  4 +++-
>  drivers/pci/host/pci-keystone.c |  4 +++-
>  drivers/pci/host/pci-layerscape.c   | 12 
>  drivers/pci/host/pcie-armada8k.c|  4 +++-
>  drivers/pci/host/pcie-designware-plat.c |  4 +++-
>  drivers/pci/host/pcie-designware.c  |  4 +++-
>  drivers/pci/host/pcie-designware.h  |  2 +-
>  drivers/pci/host/pcie-qcom.c|  6 --
>  drivers/pci/host/pcie-spear13xx.c   |  4 +++-
>  11 files changed, 37 insertions(+), 15 deletions(-)
> 

Thanks for the patch!

In my opinion your idea is good but only qcom driver is able to detect failure
in the specific host init routine, all others have a 'return 0' even if
something not well init. I would recomend that we take this issue a bit further
and add the error checking to all specific pci drivers in order to make them as
robust as qcom'.

Thanks,
Joao