[U-Boot] [PATCH 1/2] dm/pci: Change the first CFG read to Vendor ID in enumeration

[U-Boot] [PATCH 1/2] dm/pci: Change the first CFG read to Vendor ID in enumeration

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

As the PCIe specification recommend reading the Vendor ID register
to determine if a Function is present, read the Vendor ID of a
non-existent Function must not result in system error, so we'd better
make the first CFG read to Vendor ID instead of Header Type register
in the PCIe enumeration.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 drivers/pci/pci-uclass.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index eb118f3496..c8cb9cdad9 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -748,16 +748,19 @@ int pci_bind_bus_devices(struct udevice *bus)
 			found_multi = false;
 		if (PCI_FUNC(bdf) && !found_multi)
 			continue;
+
 		/* Check only the first access, we don't expect problems */
-		ret = pci_bus_read_config(bus, bdf, PCI_HEADER_TYPE,
-					  &header_type, PCI_SIZE_8);
+		ret = pci_bus_read_config(bus, bdf, PCI_VENDOR_ID, &vendor,
+					  PCI_SIZE_16);
 		if (ret)
 			goto error;
-		pci_bus_read_config(bus, bdf, PCI_VENDOR_ID, &vendor,
-				    PCI_SIZE_16);
+
 		if (vendor == 0xffff || vendor == 0x0000)
 			continue;
 
+		pci_bus_read_config(bus, bdf, PCI_HEADER_TYPE,
+				    &header_type, PCI_SIZE_8);
+
 		if (!PCI_FUNC(bdf))
 			found_multi = header_type & 0x80;
 
-- 
2.17.1

[U-Boot] [PATCH 2/2] pci: Change the first CFG read to Vendor ID in enumeration

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

As the PCIe specification recommend reading the Vendor ID register
to determine if a Function is present, read the Vendor ID of a
non-existent Function must not result in system error, so we'd better
make the first CFG read to Vendor ID instead of Header Type register
in the PCIe enumeration.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
 drivers/pci/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 221054593e..590f7b123a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -361,13 +361,13 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 		if (PCI_FUNC(dev) && !found_multi)
 			continue;
 
-		pci_hose_read_config_byte(hose, dev, PCI_HEADER_TYPE, &header_type);
-
 		pci_hose_read_config_word(hose, dev, PCI_VENDOR_ID, &vendor);
 
 		if (vendor == 0xffff || vendor == 0x0000)
 			continue;
 
+		pci_hose_read_config_byte(hose, dev, PCI_HEADER_TYPE, &header_type);
+
 		if (!PCI_FUNC(dev))
 			found_multi = header_type & 0x80;
 
-- 
2.17.1

[U-Boot] [PATCH 1/2] dm/pci: Change the first CFG read to Vendor ID in enumeration

[Bin Meng]

On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote:
>
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>
> As the PCIe specification recommend reading the Vendor ID register
> to determine if a Function is present, read the Vendor ID of a
> non-existent Function must not result in system error, so we'd better
> make the first CFG read to Vendor ID instead of Header Type register
> in the PCIe enumeration.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/pci-uclass.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 2/2] pci: Change the first CFG read to Vendor ID in enumeration

[Bin Meng]

On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote:
>
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>
> As the PCIe specification recommend reading the Vendor ID register
> to determine if a Function is present, read the Vendor ID of a
> non-existent Function must not result in system error, so we'd better
> make the first CFG read to Vendor ID instead of Header Type register
> in the PCIe enumeration.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
>  drivers/pci/pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Simon, do we need fix the non-DM version or adding new "capability" to
the non-DM drivers? I guess the answer is no and we should set some
deadline for PCI DM conversion too?

Regards,
Bin

[U-Boot] [PATCH 1/2] dm/pci: Change the first CFG read to Vendor ID in enumeration

[Z.q. Hou]

Hi Bin,

Thanks a lot for your review!

Thanks,
Zhiqiang

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: 2018年10月8日 16:53
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Simon Glass
> <sjg@chromium.org>; Christian Gmeiner <christian.gmeiner@gmail.com>;
> Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>; Bernhard Messerklinger
> <bernhard.messerklinger@br-automation.com>; Masahiro Yamada
> <yamada.masahiro@socionext.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/2] dm/pci: Change the first CFG read to Vendor ID in
> enumeration
> 
> On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com>
> wrote:
> >
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > As the PCIe specification recommend reading the Vendor ID register to
> > determine if a Function is present, read the Vendor ID of a
> > non-existent Function must not result in system error, so we'd better
> > make the first CFG read to Vendor ID instead of Header Type register
> > in the PCIe enumeration.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/pci-uclass.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 2/2] pci: Change the first CFG read to Vendor ID in enumeration

[Simon Glass]

Hi Bin,

On 8 October 2018 at 02:54, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote:
>>
>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>
>> As the PCIe specification recommend reading the Vendor ID register
>> to determine if a Function is present, read the Vendor ID of a
>> non-existent Function must not result in system error, so we'd better
>> make the first CFG read to Vendor ID instead of Header Type register
>> in the PCIe enumeration.
>>
>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>> ---
>>  drivers/pci/pci.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>
> Simon, do we need fix the non-DM version or adding new "capability" to
> the non-DM drivers? I guess the answer is no and we should set some
> deadline for PCI DM conversion too?

We should not fix or add new features to non-DM PCI. No one should be
using it at this point.

I agree about the deadline. Do you want to send a patch?

Regards,
Simon

[U-Boot] [PATCH 2/2] pci: Change the first CFG read to Vendor ID in enumeration

[Bin Meng]

Hi Simon,

On Tue, Oct 9, 2018 at 11:41 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On 8 October 2018 at 02:54, Bin Meng <bmeng.cn@gmail.com> wrote:
> > On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote:
> >>
> >> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >>
> >> As the PCIe specification recommend reading the Vendor ID register
> >> to determine if a Function is present, read the Vendor ID of a
> >> non-existent Function must not result in system error, so we'd better
> >> make the first CFG read to Vendor ID instead of Header Type register
> >> in the PCIe enumeration.
> >>
> >> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >> ---
> >>  drivers/pci/pci.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >
> > Simon, do we need fix the non-DM version or adding new "capability" to
> > the non-DM drivers? I guess the answer is no and we should set some
> > deadline for PCI DM conversion too?
>
> We should not fix or add new features to non-DM PCI. No one should be
> using it at this point.
>
> I agree about the deadline. Do you want to send a patch?
>

Actually I am not sure how to send a patch for such kind of deadline
stuff :-) Please go ahead.

Regards,
Bin

[U-Boot] [PATCH 2/2] pci: Change the first CFG read to Vendor ID in enumeration

[Z.q. Hou]

Hi Simon and Bin,

Do I need to send the standalone patch 1/2 in version 2?

Thanks,
Zhiqiang

> -----Original Message-----
> From: sjg at google.com <sjg@google.com> On Behalf Of Simon Glass
> Sent: 2018年10月9日 11:41
> To: Bin Meng <bmeng.cn@gmail.com>
> Cc: Z.q. Hou <zhiqiang.hou@nxp.com>; U-Boot Mailing List
> <u-boot@lists.denx.de>; Christian Gmeiner <christian.gmeiner@gmail.com>;
> Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>; Bernhard Messerklinger
> <bernhard.messerklinger@br-automation.com>; Masahiro Yamada
> <yamada.masahiro@socionext.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 2/2] pci: Change the first CFG read to Vendor ID in
> enumeration
> 
> Hi Bin,
> 
> On 8 October 2018 at 02:54, Bin Meng <bmeng.cn@gmail.com> wrote:
> > On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com>
> wrote:
> >>
> >> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >>
> >> As the PCIe specification recommend reading the Vendor ID register to
> >> determine if a Function is present, read the Vendor ID of a
> >> non-existent Function must not result in system error, so we'd better
> >> make the first CFG read to Vendor ID instead of Header Type register
> >> in the PCIe enumeration.
> >>
> >> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >> ---
> >>  drivers/pci/pci.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >
> > Simon, do we need fix the non-DM version or adding new "capability" to
> > the non-DM drivers? I guess the answer is no and we should set some
> > deadline for PCI DM conversion too?
> 
> We should not fix or add new features to non-DM PCI. No one should be using
> it at this point.
> 
> I agree about the deadline. Do you want to send a patch?
> 
> Regards,
> Simon

[U-Boot] [PATCH 2/2] pci: Change the first CFG read to Vendor ID in enumeration

[Simon Glass]

On 9 October 2018 at 02:07, Z.q. Hou <zhiqiang.hou@nxp.com> wrote:
> Hi Simon and Bin,
>
> Do I need to send the standalone patch 1/2 in version 2?

I don't think that is needed, no.

- SImon

>
> Thanks,
> Zhiqiang
>
>> -----Original Message-----
>> From: sjg at google.com <sjg@google.com> On Behalf Of Simon Glass
>> Sent: 2018年10月9日 11:41
>> To: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Z.q. Hou <zhiqiang.hou@nxp.com>; U-Boot Mailing List
>> <u-boot@lists.denx.de>; Christian Gmeiner <christian.gmeiner@gmail.com>;
>> Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>; Bernhard Messerklinger
>> <bernhard.messerklinger@br-automation.com>; Masahiro Yamada
>> <yamada.masahiro@socionext.com>; Mingkai Hu <mingkai.hu@nxp.com>;
>> M.h. Lian <minghuan.lian@nxp.com>
>> Subject: Re: [PATCH 2/2] pci: Change the first CFG read to Vendor ID in
>> enumeration
>>
>> Hi Bin,
>>
>> On 8 October 2018 at 02:54, Bin Meng <bmeng.cn@gmail.com> wrote:
>> > On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com>
>> wrote:
>> >>
>> >> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>> >>
>> >> As the PCIe specification recommend reading the Vendor ID register to
>> >> determine if a Function is present, read the Vendor ID of a
>> >> non-existent Function must not result in system error, so we'd better
>> >> make the first CFG read to Vendor ID instead of Header Type register
>> >> in the PCIe enumeration.
>> >>
>> >> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>> >> ---
>> >>  drivers/pci/pci.c | 4 ++--
>> >>  1 file changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >
>> > Simon, do we need fix the non-DM version or adding new "capability" to
>> > the non-DM drivers? I guess the answer is no and we should set some
>> > deadline for PCI DM conversion too?
>>
>> We should not fix or add new features to non-DM PCI. No one should be using
>> it at this point.
>>
>> I agree about the deadline. Do you want to send a patch?
>>
>> Regards,
>> Simon

[U-Boot] [PATCH 1/2] dm/pci: Change the first CFG read to Vendor ID in enumeration

[sjg at google.com]

Hi Bin,

Thanks a lot for your review!

Thanks,
Zhiqiang

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: 2018年10月8日 16:53
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Simon Glass
> <sjg@chromium.org>; Christian Gmeiner <christian.gmeiner@gmail.com>;
> Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi>; Bernhard Messerklinger
> <bernhard.messerklinger@br-automation.com>; Masahiro Yamada
> <yamada.masahiro@socionext.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> M.h. Lian <minghuan.lian@nxp.com>
> Subject: Re: [PATCH 1/2] dm/pci: Change the first CFG read to Vendor ID in
> enumeration
>
> On Mon, Oct 8, 2018 at 4:36 PM Zhiqiang Hou <Zhiqiang.Hou@nxp.com>
> wrote:
> >
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > As the PCIe specification recommend reading the Vendor ID register to
> > determine if a Function is present, read the Vendor ID of a
> > non-existent Function must not result in system error, so we'd better
> > make the first CFG read to Vendor ID instead of Header Type register
> > in the PCIe enumeration.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> >  drivers/pci/pci-uclass.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Applied to u-boot-dm/master, thanks!