Message ID | 20180312135333.11421-1-mario.six@gdsys.cc |
---|---|
State | Accepted |
Delegated to: | Simon Glass |
Headers | show |
Series | [U-Boot,v2] core: ofnode: Fix translation for #size-cells == 0 | expand |
On 12 March 2018 at 07:53, Mario Six <mario.six@gdsys.cc> wrote: > Commit 286ede6 ("drivers: core: Add translation in live tree case") made > dev_get_addr always use proper bus translations for addresses read from > the device tree. But this leads to problems with certain busses, e.g. > I2C busses, which run into an error during translation, and hence stop > working. > > It turns out that of_translate_address() and fdt_translate_address() > stop the address translation with an error when they're asked to > translate addresses for busses where #size-cells == 0 (comment from > drivers/core/of_addr.c): > > * Note: We consider that crossing any level with #size-cells == 0 to mean > * that translation is impossible (that is we are not dealing with a value > * that can be mapped to a cpu physical address). This is not really specified > * that way, but this is traditionally the way IBM at least do things > > To fix this case, we check in both the live-tree and non-live tree-case, > whether the bus of the device whose address is about to be translated > has size-cell size zero. If this is the case, we just read the address > as a plain integer and return it, and only apply bus translations if the > size-cell size if greater than zero. > > Signed-off-by: Mario Six <mario.six@gdsys.cc> > Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com> > Reported-by: Martin Fuzzey <mfuzzey@parkeon.com> > Fixes: 286ede6 ("drivers: core: Add translation in live tree case") > --- > > v1 -> v2: > * Add unit tests for address translation as suggested by Simon Glass > > --- > arch/sandbox/dts/test.dts | 48 +++++++++++++++++++++++++++++++++++++++++++++++ > drivers/core/fdtaddr.c | 17 +++++++++++------ > drivers/core/ofnode.c | 5 ++++- > include/dm/uclass-id.h | 1 + > test/dm/test-fdt.c | 43 ++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 107 insertions(+), 7 deletions(-) > Reviewed-by: Simon Glass <sjg@chromium.org>
On 20 March 2018 at 01:58, Simon Glass <sjg@chromium.org> wrote: > On 12 March 2018 at 07:53, Mario Six <mario.six@gdsys.cc> wrote: >> Commit 286ede6 ("drivers: core: Add translation in live tree case") made >> dev_get_addr always use proper bus translations for addresses read from >> the device tree. But this leads to problems with certain busses, e.g. >> I2C busses, which run into an error during translation, and hence stop >> working. >> >> It turns out that of_translate_address() and fdt_translate_address() >> stop the address translation with an error when they're asked to >> translate addresses for busses where #size-cells == 0 (comment from >> drivers/core/of_addr.c): >> >> * Note: We consider that crossing any level with #size-cells == 0 to mean >> * that translation is impossible (that is we are not dealing with a value >> * that can be mapped to a cpu physical address). This is not really specified >> * that way, but this is traditionally the way IBM at least do things >> >> To fix this case, we check in both the live-tree and non-live tree-case, >> whether the bus of the device whose address is about to be translated >> has size-cell size zero. If this is the case, we just read the address >> as a plain integer and return it, and only apply bus translations if the >> size-cell size if greater than zero. >> >> Signed-off-by: Mario Six <mario.six@gdsys.cc> >> Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com> >> Reported-by: Martin Fuzzey <mfuzzey@parkeon.com> >> Fixes: 286ede6 ("drivers: core: Add translation in live tree case") >> --- >> >> v1 -> v2: >> * Add unit tests for address translation as suggested by Simon Glass >> >> --- >> arch/sandbox/dts/test.dts | 48 +++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/core/fdtaddr.c | 17 +++++++++++------ >> drivers/core/ofnode.c | 5 ++++- >> include/dm/uclass-id.h | 1 + >> test/dm/test-fdt.c | 43 ++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 107 insertions(+), 7 deletions(-) >> > > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot-dm, thanks!
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index b0f0ca8f19..06d0e8ce85 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -27,6 +27,10 @@ testfdt3 = "/b-test"; testfdt5 = "/some-bus/c-test@5"; testfdt8 = "/a-test"; + fdt_dummy0 = "/translation-test@8000/dev@0,0"; + fdt_dummy1 = "/translation-test@8000/dev@1,100"; + fdt_dummy2 = "/translation-test@8000/dev@2,200"; + fdt_dummy3 = "/translation-test@8000/noxlatebus@3,300/dev@42"; usb0 = &usb_0; usb1 = &usb_1; usb2 = &usb_2; @@ -487,6 +491,50 @@ reg = <9 1>; }; }; + + translation-test@8000 { + compatible = "simple-bus"; + reg = <0x8000 0x4000>; + + #address-cells = <0x2>; + #size-cells = <0x1>; + + ranges = <0 0x0 0x8000 0x1000 + 1 0x100 0x9000 0x1000 + 2 0x200 0xA000 0x1000 + 3 0x300 0xB000 0x1000 + >; + + dev@0,0 { + compatible = "denx,u-boot-fdt-dummy"; + reg = <0 0x0 0x1000>; + }; + + dev@1,100 { + compatible = "denx,u-boot-fdt-dummy"; + reg = <1 0x100 0x1000>; + + }; + + dev@2,200 { + compatible = "denx,u-boot-fdt-dummy"; + reg = <2 0x200 0x1000>; + }; + + + noxlatebus@3,300 { + compatible = "simple-bus"; + reg = <3 0x300 0x1000>; + + #address-cells = <0x1>; + #size-cells = <0x0>; + + dev@42 { + compatible = "denx,u-boot-fdt-dummy"; + reg = <0x42>; + }; + }; + }; }; #include "sandbox_pmic.dtsi" diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index 3847dd836e..9a3b4c312a 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -49,12 +49,17 @@ fdt_addr_t devfdt_get_addr_index(struct udevice *dev, int index) reg += index * (na + ns); - /* - * Use the full-fledged translate function for complex - * bus setups. - */ - addr = fdt_translate_address((void *)gd->fdt_blob, - dev_of_offset(dev), reg); + if (ns) { + /* + * Use the full-fledged translate function for complex + * bus setups. + */ + addr = fdt_translate_address((void *)gd->fdt_blob, + dev_of_offset(dev), reg); + } else { + /* Non translatable if #size-cells == 0 */ + addr = fdt_read_number(reg, na); + } } else { /* * Use the "simple" translate function for less complex diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 98f4b539ea..b2f6ffe385 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -200,13 +200,16 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int index) uint flags; u64 size; int na; + int ns; prop_val = of_get_address(ofnode_to_np(node), index, &size, &flags); if (!prop_val) return FDT_ADDR_T_NONE; - if (IS_ENABLED(CONFIG_OF_TRANSLATE)) { + ns = of_n_size_cells(ofnode_to_np(node)); + + if (IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0) { return of_translate_address(ofnode_to_np(node), prop_val); } else { na = of_n_addr_cells(ofnode_to_np(node)); diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 07fabc3ce6..d28fb3e23f 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -19,6 +19,7 @@ enum uclass_id { UCLASS_TEST_FDT, UCLASS_TEST_BUS, UCLASS_TEST_PROBE, + UCLASS_TEST_DUMMY, UCLASS_SPI_EMUL, /* sandbox SPI device emulator */ UCLASS_I2C_EMUL, /* sandbox I2C device emulator */ UCLASS_PCI_EMUL, /* sandbox PCI device emulator */ diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 920ccbf016..0d11bfdb2f 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -419,3 +419,46 @@ static int dm_test_first_next_ok_device(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_first_next_ok_device, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static const struct udevice_id fdt_dummy_ids[] = { + { .compatible = "denx,u-boot-fdt-dummy", }, + { } +}; + +UCLASS_DRIVER(fdt_dummy) = { + .name = "fdt_dummy", + .id = UCLASS_TEST_DUMMY, + .flags = DM_UC_FLAG_SEQ_ALIAS, +}; + +U_BOOT_DRIVER(fdt_dummy_drv) = { + .name = "fdt_dummy_drv", + .of_match = fdt_dummy_ids, + .id = UCLASS_TEST_DUMMY, +}; + +static int dm_test_fdt_translation(struct unit_test_state *uts) +{ + struct udevice *dev; + + /* Some simple translations */ + ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 0, true, &dev)); + ut_asserteq_str("dev@0,0", dev->name); + ut_asserteq(0x8000, dev_read_addr(dev)); + + ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 1, true, &dev)); + ut_asserteq_str("dev@1,100", dev->name); + ut_asserteq(0x9000, dev_read_addr(dev)); + + ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 2, true, &dev)); + ut_asserteq_str("dev@2,200", dev->name); + ut_asserteq(0xA000, dev_read_addr(dev)); + + /* No translation for busses with #size-cells == 0 */ + ut_assertok(uclass_find_device_by_seq(UCLASS_TEST_DUMMY, 3, true, &dev)); + ut_asserteq_str("dev@42", dev->name); + ut_asserteq(0x42, dev_read_addr(dev)); + + return 0; +} +DM_TEST(dm_test_fdt_translation, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);