designware: missing *config* reg space

designware: missing *config* reg space

[Fabio Estevam]

Hi,

I am running linux-next 20140805 on a imx6q-sabresd board and noticed
the following error:

root@freescale /$ dmesg | grep pci
[    0.168095] imx6q-pcie 1ffc000.pcie: missing *config* reg space
[    0.501019] imx6q-pcie 1ffc000.pcie: phy link never came up
[    0.501579] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00

This 'missing *config* reg space' was introduced by:

commit 4dd964df36d0e548e1806ec2ec275b62d4dc46e8
Author: Kishon Vijay Abraham I <kishon@ti.com>
Date:   Thu Jul 17 14:30:40 2014 +0530

    PCI: designware: Look for configuration space in 'reg', not 'ranges'

    The configuration address space has so far been specified in *ranges*,
    however it should be specified in *reg* making it a platform MEM resource.
    Hence used 'platform_get_resource_*' API to get configuration address space
    in the designware driver.

    Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Acked-by: Mohit Kumar <mohit.kumar@st.com>
    Acked-by: Jingoo Han <jg1.han@samsung.com>
    Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
    Cc: Marek Vasut <marex@denx.de>
    Cc: Arnd Bergmann <arnd@arndb.de>

Wouldn't this break dt compatibility?

Re: designware: missing *config* reg space

[Pratyush Anand]

Hi,

On Wed, Aug 06, 2014 at 02:26:10AM +0800, Fabio Estevam wrote:
> Hi,
> 
> I am running linux-next 20140805 on a imx6q-sabresd board and noticed
> the following error:
> 
> root@freescale /$ dmesg | grep pci
> [    0.168095] imx6q-pcie 1ffc000.pcie: missing *config* reg space
> [    0.501019] imx6q-pcie 1ffc000.pcie: phy link never came up
> [    0.501579] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
> 
> This 'missing *config* reg space' was introduced by:

dev_err printing the above message should be dev_dbg until all the
platform specifies its configuration space definition through *reg*
instead of *ranges*.

> commit 4dd964df36d0e548e1806ec2ec275b62d4dc46e8
> Author: Kishon Vijay Abraham I <kishon@ti.com>
> Date:   Thu Jul 17 14:30:40 2014 +0530
> 
>     PCI: designware: Look for configuration space in 'reg', not 'ranges'
> 
>     The configuration address space has so far been specified in *ranges*,
>     however it should be specified in *reg* making it a platform MEM resource.
>     Hence used 'platform_get_resource_*' API to get configuration address space
>     in the designware driver.
> 
>     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     Acked-by: Mohit Kumar <mohit.kumar@st.com>
>     Acked-by: Jingoo Han <jg1.han@samsung.com>
>     Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>     Cc: Marek Vasut <marex@denx.de>
>     Cc: Arnd Bergmann <arnd@arndb.de>
> 
> Wouldn't this break dt compatibility?

In spite of the above missing *config* message, your system should
work fine. This patch should not affect any older platform.They will
assign their cfg space through restype == 0.

~Pratyush

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

Re: designware: missing *config* reg space

[Lucas Stach]

Am Mittwoch, den 06.08.2014, 10:02 +0530 schrieb Pratyush Anand:
> Hi,
> 
> On Wed, Aug 06, 2014 at 02:26:10AM +0800, Fabio Estevam wrote:
> > Hi,
> > 
> > I am running linux-next 20140805 on a imx6q-sabresd board and noticed
> > the following error:
> > 
> > root@freescale /$ dmesg | grep pci
> > [    0.168095] imx6q-pcie 1ffc000.pcie: missing *config* reg space
> > [    0.501019] imx6q-pcie 1ffc000.pcie: phy link never came up
> > [    0.501579] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
> > 
> > This 'missing *config* reg space' was introduced by:
> 
> dev_err printing the above message should be dev_dbg until all the
> platform specifies its configuration space definition through *reg*
> instead of *ranges*.
> 
No it's completely right. Printing errors actually triggers people to
fix their DTs.

> > commit 4dd964df36d0e548e1806ec2ec275b62d4dc46e8
> > Author: Kishon Vijay Abraham I <kishon@ti.com>
> > Date:   Thu Jul 17 14:30:40 2014 +0530
> > 
> >     PCI: designware: Look for configuration space in 'reg', not 'ranges'
> > 
> >     The configuration address space has so far been specified in *ranges*,
> >     however it should be specified in *reg* making it a platform MEM resource.
> >     Hence used 'platform_get_resource_*' API to get configuration address space
> >     in the designware driver.
> > 
> >     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >     Acked-by: Mohit Kumar <mohit.kumar@st.com>
> >     Acked-by: Jingoo Han <jg1.han@samsung.com>
> >     Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >     Cc: Marek Vasut <marex@denx.de>
> >     Cc: Arnd Bergmann <arnd@arndb.de>
> > 
> > Wouldn't this break dt compatibility?
> 
> In spite of the above missing *config* message, your system should
> work fine. This patch should not affect any older platform.They will
> assign their cfg space through restype == 0.
> 
As Pratyush said things should work fine despite this warning. I'll send
a DT patch out to Shawn today to get rid of this warning.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: designware: missing *config* reg space

[Jingoo Han]

On Wednesday, August 06, 2014 5:39 PM, Lucas Stach wrote:
> 
> Am Mittwoch, den 06.08.2014, 10:02 +0530 schrieb Pratyush Anand:
> > On Wed, Aug 06, 2014 at 02:26:10AM +0800, Fabio Estevam wrote:
> > > Hi,
> > >
> > > I am running linux-next 20140805 on a imx6q-sabresd board and noticed
> > > the following error:
> > >
> > > root@freescale /$ dmesg | grep pci
> > > [    0.168095] imx6q-pcie 1ffc000.pcie: missing *config* reg space
> > > [    0.501019] imx6q-pcie 1ffc000.pcie: phy link never came up
> > > [    0.501579] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
> > >
> > > This 'missing *config* reg space' was introduced by:
> >
> > dev_err printing the above message should be dev_dbg until all the
> > platform specifies its configuration space definition through *reg*
> > instead of *ranges*.
> >
> No it's completely right. Printing errors actually triggers people to
> fix their DTs.

I agree with Lucas Stach's opinion.
Printing errors is necessary to let people change to fix the DT.
According to the DT spec, using *ranges* is wrong. So, dev_err()
can be used.

> 
> > > commit 4dd964df36d0e548e1806ec2ec275b62d4dc46e8
> > > Author: Kishon Vijay Abraham I <kishon@ti.com>
> > > Date:   Thu Jul 17 14:30:40 2014 +0530
> > >
> > >     PCI: designware: Look for configuration space in 'reg', not 'ranges'
> > >
> > >     The configuration address space has so far been specified in *ranges*,
> > >     however it should be specified in *reg* making it a platform MEM resource.
> > >     Hence used 'platform_get_resource_*' API to get configuration address space
> > >     in the designware driver.
> > >
> > >     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> > >     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > >     Acked-by: Mohit Kumar <mohit.kumar@st.com>
> > >     Acked-by: Jingoo Han <jg1.han@samsung.com>
> > >     Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > >     Cc: Marek Vasut <marex@denx.de>
> > >     Cc: Arnd Bergmann <arnd@arndb.de>
> > >
> > > Wouldn't this break dt compatibility?
> >
> > In spite of the above missing *config* message, your system should
> > work fine. This patch should not affect any older platform.They will
> > assign their cfg space through restype == 0.
> >
> As Pratyush said things should work fine despite this warning. I'll send
> a DT patch out to Shawn today to get rid of this warning.

Good! :-)

Best regards,
Jingoo Han

> 
> Regards,
> Lucas
> --
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: designware: missing *config* reg space

[Fabio Estevam]

Jingoo,

On Thu, Aug 7, 2014 at 12:01 AM, Jingoo Han <jg1.han@samsung.com> wrote:

> I agree with Lucas Stach's opinion.
> Printing errors is necessary to let people change to fix the DT.
> According to the DT spec, using *ranges* is wrong. So, dev_err()
> can be used.

Could you please point me to a dts that uses 'config' in the PCI node,
so I could use it as reference?

Thanks

Re: designware: missing *config* reg space

[Jingoo Han]

On Thursday, August 07, 2014 12:12 PM, Fabio Estevam wrote:
> On Thu, Aug 7, 2014 at 12:01 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> 
> > I agree with Lucas Stach's opinion.
> > Printing errors is necessary to let people change to fix the DT.
> > According to the DT spec, using *ranges* is wrong. So, dev_err()
> > can be used.
> 
> Could you please point me to a dts that uses 'config' in the PCI node,
> so I could use it as reference?

Kishon already sent the patch to use 'config' in the PCI node.
As far as I know, there is no objections. Please refer to the
following links.

[RESEND PATCH 8/8] ARM: dts: dra7: Add dt data for PCIe controller
(http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/272131.html)

[PATCH v3 3/4] Documentation: pci: ti: Add dt binding documentation for PCIe in DRA7xx
(http://www.spinics.net/lists/linux-pci/msg32988.html)

Best regards,
Jingoo Han

> 
> Thanks