* [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional
@ 2018-05-23 12:00 Christian Gmeiner
2018-05-23 12:00 ` [U-Boot] [PATCH 2/2] dm: pci: use a 1:1 mapping for bus <-> phy addresses Christian Gmeiner
2018-05-23 13:10 ` [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional Bin Meng
0 siblings, 2 replies; 6+ messages in thread
From: Christian Gmeiner @ 2018-05-23 12:00 UTC (permalink / raw)
To: u-boot
If we use u-boot as coreboot payload with a generic dts without
any ranges specified we fail in pci pre_probe and our pci bus
is not usable.
So convert decode_regions(..) into a void function and do the simple
error handling there.
Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
drivers/pci/pci-uclass.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 49be1ebdd7..416444a230 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -810,7 +810,7 @@ error:
return ret;
}
-static int decode_regions(struct pci_controller *hose, ofnode parent_node,
+static void decode_regions(struct pci_controller *hose, ofnode parent_node,
ofnode node)
{
int pci_addr_cells, addr_cells, size_cells;
@@ -820,8 +820,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
int i;
prop = ofnode_get_property(node, "ranges", &len);
- if (!prop)
- return -EINVAL;
+ if (!prop) {
+ debug("%s: Cannot decode regions\n", __func__);
+ return;
+ }
+
pci_addr_cells = ofnode_read_simple_addr_cells(node);
addr_cells = ofnode_read_simple_addr_cells(parent_node);
size_cells = ofnode_read_simple_size_cells(node);
@@ -876,7 +879,7 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
bd_t *bd = gd->bd;
if (!bd)
- return 0;
+ return;
for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
if (bd->bi_dram[i].size) {
@@ -901,13 +904,12 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
#endif
- return 0;
+ return;
}
static int pci_uclass_pre_probe(struct udevice *bus)
{
struct pci_controller *hose;
- int ret;
debug("%s, bus=%d/%s, parent=%s\n", __func__, bus->seq, bus->name,
bus->parent->name);
@@ -916,12 +918,7 @@ static int pci_uclass_pre_probe(struct udevice *bus)
/* For bridges, use the top-level PCI controller */
if (!device_is_on_pci_bus(bus)) {
hose->ctlr = bus;
- ret = decode_regions(hose, dev_ofnode(bus->parent),
- dev_ofnode(bus));
- if (ret) {
- debug("%s: Cannot decode regions\n", __func__);
- return ret;
- }
+ decode_regions(hose, dev_ofnode(bus->parent), dev_ofnode(bus));
} else {
struct pci_controller *parent_hose;
--
2.17.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] dm: pci: use a 1:1 mapping for bus <-> phy addresses
2018-05-23 12:00 [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional Christian Gmeiner
@ 2018-05-23 12:00 ` Christian Gmeiner
2018-05-23 16:33 ` Simon Glass
2018-05-23 13:10 ` [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional Bin Meng
1 sibling, 1 reply; 6+ messages in thread
From: Christian Gmeiner @ 2018-05-23 12:00 UTC (permalink / raw)
To: u-boot
If u-boot gets used as coreboot payload all pci resources got
assigned by coreboot. If a dts without any pci ranges gets used
the dm is not able to access pci device memory. To get things
working make use of a 1:1 mapping for bus <-> phy addresses.
This change makes it possible to get the e1000 u-boot driver
working on a sandybridge device where u-boot is used as coreboot
payload.
Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
drivers/pci/pci-uclass.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 416444a230..9cec619bb2 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -1175,6 +1175,11 @@ static int _dm_pci_bus_to_phys(struct udevice *ctlr,
struct pci_region *res;
int i;
+ if (hose->region_count == 0) {
+ *pa = bus_addr;
+ return 0;
+ }
+
for (i = 0; i < hose->region_count; i++) {
res = &hose->regions[i];
@@ -1238,6 +1243,11 @@ int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr,
ctlr = pci_get_controller(dev);
hose = dev_get_uclass_priv(ctlr);
+ if (hose->region_count == 0) {
+ *ba = phys_addr;
+ return 0;
+ }
+
for (i = 0; i < hose->region_count; i++) {
res = &hose->regions[i];
--
2.17.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional
2018-05-23 12:00 [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional Christian Gmeiner
2018-05-23 12:00 ` [U-Boot] [PATCH 2/2] dm: pci: use a 1:1 mapping for bus <-> phy addresses Christian Gmeiner
@ 2018-05-23 13:10 ` Bin Meng
2018-05-23 13:39 ` Christian Gmeiner
1 sibling, 1 reply; 6+ messages in thread
From: Bin Meng @ 2018-05-23 13:10 UTC (permalink / raw)
To: u-boot
Hi Christian,
On Wed, May 23, 2018 at 8:00 PM, Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
> If we use u-boot as coreboot payload with a generic dts without
> any ranges specified we fail in pci pre_probe and our pci bus
> is not usable.
>
What do you mean by "a generic dts"?
The coreboot payload needs to specify a dedicated dts for that board,
for example to build a coreboot payload for minnowmax, we need specify
minnowmax.dts in the Kconfig. README.x86 documents this.
> So convert decode_regions(..) into a void function and do the simple
> error handling there.
>
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
> drivers/pci/pci-uclass.c | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 49be1ebdd7..416444a230 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -810,7 +810,7 @@ error:
> return ret;
> }
>
> -static int decode_regions(struct pci_controller *hose, ofnode parent_node,
> +static void decode_regions(struct pci_controller *hose, ofnode parent_node,
> ofnode node)
> {
> int pci_addr_cells, addr_cells, size_cells;
> @@ -820,8 +820,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
> int i;
>
> prop = ofnode_get_property(node, "ranges", &len);
> - if (!prop)
> - return -EINVAL;
> + if (!prop) {
> + debug("%s: Cannot decode regions\n", __func__);
> + return;
> + }
> +
> pci_addr_cells = ofnode_read_simple_addr_cells(node);
> addr_cells = ofnode_read_simple_addr_cells(parent_node);
> size_cells = ofnode_read_simple_size_cells(node);
> @@ -876,7 +879,7 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
> bd_t *bd = gd->bd;
>
> if (!bd)
> - return 0;
> + return;
>
> for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> if (bd->bi_dram[i].size) {
> @@ -901,13 +904,12 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
> base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> #endif
>
> - return 0;
> + return;
> }
>
> static int pci_uclass_pre_probe(struct udevice *bus)
> {
> struct pci_controller *hose;
> - int ret;
>
> debug("%s, bus=%d/%s, parent=%s\n", __func__, bus->seq, bus->name,
> bus->parent->name);
> @@ -916,12 +918,7 @@ static int pci_uclass_pre_probe(struct udevice *bus)
> /* For bridges, use the top-level PCI controller */
> if (!device_is_on_pci_bus(bus)) {
> hose->ctlr = bus;
> - ret = decode_regions(hose, dev_ofnode(bus->parent),
> - dev_ofnode(bus));
> - if (ret) {
> - debug("%s: Cannot decode regions\n", __func__);
> - return ret;
> - }
> + decode_regions(hose, dev_ofnode(bus->parent), dev_ofnode(bus));
> } else {
> struct pci_controller *parent_hose;
>
> --
>
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional
2018-05-23 13:10 ` [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional Bin Meng
@ 2018-05-23 13:39 ` Christian Gmeiner
2018-06-06 13:19 ` Bin Meng
0 siblings, 1 reply; 6+ messages in thread
From: Christian Gmeiner @ 2018-05-23 13:39 UTC (permalink / raw)
To: u-boot
Hi Bin,
Am Mi., 23. Mai 2018 um 15:10 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
> Hi Christian,
> On Wed, May 23, 2018 at 8:00 PM, Christian Gmeiner
> <christian.gmeiner@gmail.com> wrote:
> > If we use u-boot as coreboot payload with a generic dts without
> > any ranges specified we fail in pci pre_probe and our pci bus
> > is not usable.
> >
> What do you mean by "a generic dts"?
> The coreboot payload needs to specify a dedicated dts for that board,
> for example to build a coreboot payload for minnowmax, we need specify
> minnowmax.dts in the Kconfig. README.x86 documents this.
Yeah.. so in my eyes a "generic dts" contains only this part regarding pci:
pci {
compatible = "pci-x86";
#address-cells = <0x3>;
#size-cells = <0x2>;
};
The important part is that it does not contain any ranges. Coreboot is the
one
who does the pci bus setup (assigning memory windows for devices etc). So I
do not want to know in u-boot at build time (ranges thing for pci) how the
pci
bus looks regarding addresses. My end goal is one generic u-boot payload
that
gets used for different FSP 2.0 based boards.
> > So convert decode_regions(..) into a void function and do the simple
> > error handling there.
> >
> > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> > ---
> > drivers/pci/pci-uclass.c | 21 +++++++++------------
> > 1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> > index 49be1ebdd7..416444a230 100644
> > --- a/drivers/pci/pci-uclass.c
> > +++ b/drivers/pci/pci-uclass.c
> > @@ -810,7 +810,7 @@ error:
> > return ret;
> > }
> >
> > -static int decode_regions(struct pci_controller *hose, ofnode
parent_node,
> > +static void decode_regions(struct pci_controller *hose, ofnode
parent_node,
> > ofnode node)
> > {
> > int pci_addr_cells, addr_cells, size_cells;
> > @@ -820,8 +820,11 @@ static int decode_regions(struct pci_controller
*hose, ofnode parent_node,
> > int i;
> >
> > prop = ofnode_get_property(node, "ranges", &len);
> > - if (!prop)
> > - return -EINVAL;
> > + if (!prop) {
> > + debug("%s: Cannot decode regions\n", __func__);
> > + return;
> > + }
> > +
> > pci_addr_cells = ofnode_read_simple_addr_cells(node);
> > addr_cells = ofnode_read_simple_addr_cells(parent_node);
> > size_cells = ofnode_read_simple_size_cells(node);
> > @@ -876,7 +879,7 @@ static int decode_regions(struct pci_controller
*hose, ofnode parent_node,
> > bd_t *bd = gd->bd;
> >
> > if (!bd)
> > - return 0;
> > + return;
> >
> > for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> > if (bd->bi_dram[i].size) {
> > @@ -901,13 +904,12 @@ static int decode_regions(struct pci_controller
*hose, ofnode parent_node,
> > base, size, PCI_REGION_MEM |
PCI_REGION_SYS_MEMORY);
> > #endif
> >
> > - return 0;
> > + return;
> > }
> >
> > static int pci_uclass_pre_probe(struct udevice *bus)
> > {
> > struct pci_controller *hose;
> > - int ret;
> >
> > debug("%s, bus=%d/%s, parent=%s\n", __func__, bus->seq,
bus->name,
> > bus->parent->name);
> > @@ -916,12 +918,7 @@ static int pci_uclass_pre_probe(struct udevice
*bus)
> > /* For bridges, use the top-level PCI controller */
> > if (!device_is_on_pci_bus(bus)) {
> > hose->ctlr = bus;
> > - ret = decode_regions(hose, dev_ofnode(bus->parent),
> > - dev_ofnode(bus));
> > - if (ret) {
> > - debug("%s: Cannot decode regions\n", __func__);
> > - return ret;
> > - }
> > + decode_regions(hose, dev_ofnode(bus->parent),
dev_ofnode(bus));
> > } else {
> > struct pci_controller *parent_hose;
> >
> > --
> >
> Regards,
> Bin
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] dm: pci: use a 1:1 mapping for bus <-> phy addresses
2018-05-23 12:00 ` [U-Boot] [PATCH 2/2] dm: pci: use a 1:1 mapping for bus <-> phy addresses Christian Gmeiner
@ 2018-05-23 16:33 ` Simon Glass
0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2018-05-23 16:33 UTC (permalink / raw)
To: u-boot
Hi Christian,
On 23 May 2018 at 06:00, Christian Gmeiner <christian.gmeiner@gmail.com> wrote:
> If u-boot gets used as coreboot payload all pci resources got
U-Boot
(please use this consistently)
> assigned by coreboot. If a dts without any pci ranges gets used
> the dm is not able to access pci device memory. To get things
> working make use of a 1:1 mapping for bus <-> phy addresses.
>
> This change makes it possible to get the e1000 u-boot driver
> working on a sandybridge device where u-boot is used as coreboot
> payload.
>
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
> drivers/pci/pci-uclass.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 416444a230..9cec619bb2 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -1175,6 +1175,11 @@ static int _dm_pci_bus_to_phys(struct udevice *ctlr,
> struct pci_region *res;
> int i;
>
> + if (hose->region_count == 0) {
Or just if (!hose->region_count)
Please add a comment as to the case you are covering here. How come
the mapping will be 1:1? Is that guaranteed?
> + *pa = bus_addr;
> + return 0;
> + }
> +
> for (i = 0; i < hose->region_count; i++) {
> res = &hose->regions[i];
>
> @@ -1238,6 +1243,11 @@ int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr,
> ctlr = pci_get_controller(dev);
> hose = dev_get_uclass_priv(ctlr);
>
> + if (hose->region_count == 0) {
> + *ba = phys_addr;
> + return 0;
> + }
> +
> for (i = 0; i < hose->region_count; i++) {
> res = &hose->regions[i];
>
> --
> 2.17.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional
2018-05-23 13:39 ` Christian Gmeiner
@ 2018-06-06 13:19 ` Bin Meng
0 siblings, 0 replies; 6+ messages in thread
From: Bin Meng @ 2018-06-06 13:19 UTC (permalink / raw)
To: u-boot
Hi Christian,
On Wed, May 23, 2018 at 9:39 PM, Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
> Hi Bin,
>
> Am Mi., 23. Mai 2018 um 15:10 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
>
>> Hi Christian,
>
>> On Wed, May 23, 2018 at 8:00 PM, Christian Gmeiner
>> <christian.gmeiner@gmail.com> wrote:
>> > If we use u-boot as coreboot payload with a generic dts without
>> > any ranges specified we fail in pci pre_probe and our pci bus
>> > is not usable.
>> >
>
>> What do you mean by "a generic dts"?
>
>
>> The coreboot payload needs to specify a dedicated dts for that board,
>> for example to build a coreboot payload for minnowmax, we need specify
>> minnowmax.dts in the Kconfig. README.x86 documents this.
>
>
> Yeah.. so in my eyes a "generic dts" contains only this part regarding pci:
>
> pci {
> compatible = "pci-x86";
> #address-cells = <0x3>;
> #size-cells = <0x2>;
> };
>
> The important part is that it does not contain any ranges. Coreboot is the
> one
> who does the pci bus setup (assigning memory windows for devices etc). So I
> do not want to know in u-boot at build time (ranges thing for pci) how the
> pci
> bus looks regarding addresses. My end goal is one generic u-boot payload
> that
> gets used for different FSP 2.0 based boards.
>
I see your point. Then they way we support coreboot might be changed.
We may create a generic U-Boot payload for coreboot targets.
Can you respin the patch to address these comments, like u-boot ->
U-Boot, adding detailed info to the commit message about this, and
update README.x86?
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-06 13:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23 12:00 [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional Christian Gmeiner
2018-05-23 12:00 ` [U-Boot] [PATCH 2/2] dm: pci: use a 1:1 mapping for bus <-> phy addresses Christian Gmeiner
2018-05-23 16:33 ` Simon Glass
2018-05-23 13:10 ` [U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional Bin Meng
2018-05-23 13:39 ` Christian Gmeiner
2018-06-06 13:19 ` Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.