diff mbox

[U-Boot,2/6] dm: pci: Correct primary/secondary/subordinate bus number assignment

Message ID BLU436-SMTP119C5643AF40BB3897C39CEBF870@phx.gbl
State Accepted
Delegated to: Simon Glass
Headers show

Commit Message

Bin Meng July 18, 2015, 4:20 p.m. UTC
In driver model, each pci bridge device has its own hose structure.
hose->first_busno points to the bridge device's device number, so
we should not substract hose->first_busno before programming the
bridge device's primary/secondary/subordinate bus number registers.

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

 drivers/pci/pci_auto.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Simon Glass July 20, 2015, 1:59 a.m. UTC | #1
Hi Bin,

On 18 July 2015 at 10:20, Bin Meng <bmeng.cn@gmail.com> wrote:
> In driver model, each pci bridge device has its own hose structure.
> hose->first_busno points to the bridge device's device number, so
> we should not substract hose->first_busno before programming the
> bridge device's primary/secondary/subordinate bus number registers.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/pci/pci_auto.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Acked-by: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>

I'm not happy with how each PCI bridge has its own hose structure. I
think that was a mistake. It just creates problems.

I'm thinking we should instead have a UCLASS_PCI which is strictly for
the PCI controller, and a new UCLASS_PCI_BRIDGE for bridges.

What do you think?

Regards,
Simon
Bin Meng July 20, 2015, 2:38 a.m. UTC | #2
Hi Simon,

On Mon, Jul 20, 2015 at 9:59 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 18 July 2015 at 10:20, Bin Meng <bmeng.cn@gmail.com> wrote:
>> In driver model, each pci bridge device has its own hose structure.
>> hose->first_busno points to the bridge device's device number, so
>> we should not substract hose->first_busno before programming the
>> bridge device's primary/secondary/subordinate bus number registers.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  drivers/pci/pci_auto.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>
> Acked-by: Simon Glass <sjg@chromium.org>
> Tested-by: Simon Glass <sjg@chromium.org>
>
> I'm not happy with how each PCI bridge has its own hose structure. I
> think that was a mistake. It just creates problems.
>
> I'm thinking we should instead have a UCLASS_PCI which is strictly for
> the PCI controller, and a new UCLASS_PCI_BRIDGE for bridges.
>
> What do you think?
>

I guess that may help. During the debug, I feel the dm pci codes are
really not that intuitive than the previous non-dm version. It took me
sometime to figure out where is the problem.

Regards,
Bin
Simon Glass July 20, 2015, 3:01 p.m. UTC | #3
On 19 July 2015 at 19:59, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 18 July 2015 at 10:20, Bin Meng <bmeng.cn@gmail.com> wrote:
>> In driver model, each pci bridge device has its own hose structure.
>> hose->first_busno points to the bridge device's device number, so
>> we should not substract hose->first_busno before programming the
>> bridge device's primary/secondary/subordinate bus number registers.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  drivers/pci/pci_auto.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>
> Acked-by: Simon Glass <sjg@chromium.org>
> Tested-by: Simon Glass <sjg@chromium.org>
>
Applied to u-boot-x86, thanks!

[snip]
Simon Glass July 21, 2015, 9:24 p.m. UTC | #4
Hi Bin,

On 19 July 2015 at 20:38, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Mon, Jul 20, 2015 at 9:59 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Bin,
>>
>> On 18 July 2015 at 10:20, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> In driver model, each pci bridge device has its own hose structure.
>>> hose->first_busno points to the bridge device's device number, so
>>> we should not substract hose->first_busno before programming the
>>> bridge device's primary/secondary/subordinate bus number registers.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>
>>>  drivers/pci/pci_auto.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>> Tested-by: Simon Glass <sjg@chromium.org>
>>
>> I'm not happy with how each PCI bridge has its own hose structure. I
>> think that was a mistake. It just creates problems.
>>
>> I'm thinking we should instead have a UCLASS_PCI which is strictly for
>> the PCI controller, and a new UCLASS_PCI_BRIDGE for bridges.
>>
>> What do you think?
>>
>
> I guess that may help. During the debug, I feel the dm pci codes are
> really not that intuitive than the previous non-dm version. It took me
> sometime to figure out where is the problem.

Agreed. Part of the problem I think is that it is still compatible
with the old code.

We should be able to do things like pci_read_config16(struct udevice *dev, ...)

I guess we could move to separate things more.

Regards,
Simon
diff mbox

Patch

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index e034ed1..ef6dc4f 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -224,10 +224,15 @@  void pciauto_prescan_setup_bridge(struct pci_controller *hose,
 	prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
 
 	/* Configure bus number registers */
+#ifdef CONFIG_DM_PCI
+	pci_hose_write_config_byte(hose, dev, PCI_PRIMARY_BUS, PCI_BUS(dev));
+	pci_hose_write_config_byte(hose, dev, PCI_SECONDARY_BUS, sub_bus);
+#else
 	pci_hose_write_config_byte(hose, dev, PCI_PRIMARY_BUS,
 				   PCI_BUS(dev) - hose->first_busno);
 	pci_hose_write_config_byte(hose, dev, PCI_SECONDARY_BUS,
 				   sub_bus - hose->first_busno);
+#endif
 	pci_hose_write_config_byte(hose, dev, PCI_SUBORDINATE_BUS, 0xff);
 
 	if (pci_mem) {
@@ -295,8 +300,12 @@  void pciauto_postscan_setup_bridge(struct pci_controller *hose,
 	struct pci_region *pci_io = hose->pci_io;
 
 	/* Configure bus number registers */
+#ifdef CONFIG_DM_PCI
+	pci_hose_write_config_byte(hose, dev, PCI_SUBORDINATE_BUS, sub_bus);
+#else
 	pci_hose_write_config_byte(hose, dev, PCI_SUBORDINATE_BUS,
 				   sub_bus - hose->first_busno);
+#endif
 
 	if (pci_mem) {
 		/* Round memory allocator to 1MB boundary */