Hi Tom,

On 16.06.21 г. 20:37 ч., Tom Rini wrote:
> On Wed, Jun 16, 2021 at 08:25:28PM +0300, Ivaylo Dimitrov wrote:
>> Hi,
>>
>> On 16.06.21 г. 15:13 ч., Tom Rini wrote:
>>> On Wed, Jun 16, 2021 at 08:10:08AM -0400, Tom Rini wrote:
>>>> On Wed, Jun 16, 2021 at 09:02:16AM +0300, Ivaylo Dimitrov wrote:
>>>>> Hi,
>>>>>
>>>>> On 15.06.21 г. 15:34 ч., Tom Rini wrote:
>>>>>> On Tue, Jun 15, 2021 at 08:40:30AM +0300, Ivaylo Dimitrov wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 22.05.21 г. 0:36 ч., Pali Rohár wrote:
>>>>>>>> On Friday 21 May 2021 10:44:18 Tom Rini wrote:
>>>>>>>>> On Wed, May 19, 2021 at 11:52:03AM -0400, Tom Rini wrote:
>>>>>>>>>> On Wed, May 19, 2021 at 03:27:48PM +0200, Pali Rohár wrote:
>>>>>>>>>>
>>>>>>>>>>> On Tuesday 18 May 2021 21:26:40 Tom Rini wrote:
>>>>>>>>>>>> This board has not been converted to CONFIG_DM_USB by the deadline.
>>>>>>>>>>>> Remove it.
>>>>>>>>>>>
>>>>>>>>>>> I'm very disappointed that you want to remove Nokia N900 from U-Boot.
>>>>>>>>>>>
>>>>>>>>>>> I was waiting waiting half of year because other developers did not
>>>>>>>>>>> react to issues which were introduced and neither to patches which I
>>>>>>>>>>> sent (+ trying to remind open issues). And also I was waiting another
>>>>>>>>>>> half of year until other N900 related patches were merged. So the whole
>>>>>>>>>>> slowdown was not caused by me, why it is taking so long.
>>>>>>>>>>>
>>>>>>>>>>> Now there is still one N900 DM related patch waiting for review. I'm
>>>>>>>>>>> converting code step by step.
>>>>>>>>>>>
>>>>>>>>>>> So the ball is not on my side.
>>>>>>>>>>
>>>>>>>>>> So, what patch(es) need to be applied to get DM_USB enabled? Thanks.
>>>>>>>>>
>>>>>>>>> I don't see any open patches from you that look related to enabling
>>>>>>>>> DM_USB on the platform. If you want to disable USB on the platform for
>>>>>>>>> now instead, that's fine too.
>>>>>>>>
>>>>>>>
>>>>>>> I tried to migrate the latest master to DM_USB, but unfortunately the
>>>>>>> results are pretty much sad - adding OF_CONTROL (which is a prerequisite to
>>>>>>> have DM_USB IIUC) and OF_BOARD (so binary to be compiled), adds ~100k to the
>>>>>>> size of the u-boot binary, so it becomes 370284 bytes. Given that we have
>>>>>>> less than 256k of storage space for the u-boot, the produced binary cannot
>>>>>>> be used on n900 the same way current (no DM_USB) binary is used.
>>>>>>>
>>>>>>> As I see it, there are not much options left - u-boot on N900 is not SPL, so
>>>>>>> we can't use OF_PLATDATA, which in turn always links libfdt.
>>>>>>> Also, if I read the code (usb-uclass.c) correctly, enabling DM_USB requires
>>>>>>> the board to be converted to DT and this is way bigger change.
>>>>>>>
>>>>>>> Please advice on how to proceed.
>>>>>>
>>>>>> Please post your WIP patches, thanks.
>>>>>>
>>>>>
>>>>> Sorry, I am not sure I understand what patches you want me to post:
>>>>>
>>>>> WDT patch has already been sent couple of months ago -
>>>>> https://lists.denx.de/pipermail/u-boot/2021-March/443868.html
>>>>> Do you want it to be rebased and resend?
>>>>>
>>>>> DM_USB, I just started writing one and I immediately hit the OF_CONTROL
>>>>> requirement. Enabling OF_CONTROL requires a full blown migration to DT,
>>>>> which is a huge task and not really equal to "Please update the board to use
>>>>> CONFIG_DM_USB...". Without OF_CONTROL, I simply get link failures:
>>>>>
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> DWARF error: could not find abbrev number 3998
>>>>> /tmp/cc0BOqms.ltrans0.ltrans.o: in function `usb_child_post_bind':
>>>>> <artificial>:(.text+0x5672): undefined reference to
>>>>> `ofnode_read_u32_default'
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> <artificial>:(.text+0x568c): undefined reference to
>>>>> `ofnode_read_u32_default'
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> /tmp/cc0BOqms.ltrans0.ltrans.o: in function `usb_scan_device':
>>>>> <artificial>:(.text+0x6c84): undefined reference to `ofnode_first_subnode'
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> <artificial>:(.text+0x6c96): undefined reference to `ofnode_read_u32'
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> <artificial>:(.text+0x6ca4): undefined reference to `ofnode_next_subnode'
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> /tmp/cc0BOqms.ltrans0.ltrans.o:(.u_boot_list_2_uclass_driver_2_usb+0x8):
>>>>> undefined reference to `dm_scan_fdt_dev'
>>>>> /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld:
>>>>> /tmp/cc0BOqms.ltrans0.ltrans.o:(.u_boot_list_2_uclass_driver_2_usb_hub+0x8):
>>>>> undefined reference to `dm_scan_fdt_dev'
>>>>>
>>>>> Fixing those requires enabling of OF_CONTROL and this in turn means the
>>>>> board must be migrated to DT, unless I am missing something. That's why my
>>>>> "please advice..." stance.
>>>>
>>>> Please post the patches that bring you to the above link errors, yes,
>>>> thanks.
>>>
>>> To be clearer, finish up a patch that completes the migration but is too
>>> large to install on the hardware so that others can take a look.
>>
>> I am not sure I understand that - a patch that completes the migration to
>> DM_USB cannot be done ATM as the binary does not link without OF_CONTROL.
>> And I am not going to enable OF_CONTROL as this means I will have to migrate
>> everything to DT. That's why I enable DM_USB only - see reply to the other
>> mail.
>
> My advise is to provide a linkable but not runnable (or, only runnable
> in QEMU, the problem is the fixed layout of the actual device flash,
> right?) patch so that we can figure out how and what needs to be tuned
> where so that we can see what to do about this platform.
>

What about the following patch (once you confirm I am on the right
track, I will send a proper patch as well). Enabling thumb is a must,
otherwise binary does not fit. With the below changes code compiles and
runs on a real HW:

diff --git a/common/usb_hub.c b/common/usb_hub.c
index ba11a188ca..c6b042a684 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -956,7 +956,9 @@ U_BOOT_DRIVER(usb_generic_hub) = {
  UCLASS_DRIVER(usb_hub) = {
      .id        = UCLASS_USB_HUB,
      .name        = "usb_hub",
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
      .post_bind    = dm_scan_fdt_dev,
+#endif
      .post_probe    = usb_hub_post_probe,
      .child_pre_probe    = usb_child_pre_probe,
      .per_child_auto    = sizeof(struct usb_device),
diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig
index 3548aab0e1..daaf6690ef 100644
--- a/configs/nokia_rx51_defconfig
+++ b/configs/nokia_rx51_defconfig
@@ -1,5 +1,5 @@
  CONFIG_ARM=y
-# CONFIG_SYS_THUMB_BUILD is not set
+CONFIG_SYS_THUMB_BUILD=y
  CONFIG_ARCH_OMAP2PLUS=y
  CONFIG_SYS_TEXT_BASE=0x80008000
  CONFIG_NR_DRAM_BANKS=2
@@ -61,6 +61,10 @@ CONFIG_CONS_INDEX=3
  CONFIG_SYS_NS16550=y
  CONFIG_SPI=y
  CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_OF_LIBFDT=y
+CONFIG_OF_LIBFDT_OVERLAY=n
+CONFIG_EFI_LOADER=n
  CONFIG_USB_MUSB_UDC=y
  CONFIG_USB_OMAP3=y
  CONFIG_CFB_CONSOLE=y
diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 5edd4e4135..e7213143c4 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -2,7 +2,7 @@
  #
  # Copyright (c) 2013 Google, Inc

-obj-y    += device.o fdtaddr.o lists.o root.o uclass.o util.o
+obj-y    += device.o fdtaddr.o lists.o root.o uclass.o util.o ofnode.o
  obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
  obj-$(CONFIG_DEVRES) += devres.o
  obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE)    += device-remove.o
@@ -15,6 +15,6 @@ obj-$(CONFIG_$(SPL_)OF_LIVE) += of_access.o of_addr.o
  ifndef CONFIG_DM_DEV_READ_INLINE
  obj-$(CONFIG_OF_CONTROL) += read.o
  endif
-obj-$(CONFIG_OF_CONTROL) += of_extra.o ofnode.o read_extra.o
+obj-$(CONFIG_OF_CONTROL) += of_extra.o read_extra.o

ccflags-$(CONFIG_DM_DEBUG) += -DDEBUG
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index e3b616c326..71dc578550 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -770,6 +770,7 @@ int usb_detect_change(void)

  static int usb_child_post_bind(struct udevice *dev)
  {
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
      struct usb_dev_plat *plat = dev_get_parent_plat(dev);
      int val;

@@ -787,7 +788,7 @@ static int usb_child_post_bind(struct udevice *dev)
          plat->id.match_flags |= USB_DEVICE_ID_MATCH_INT_CLASS;
          plat->id.bInterfaceClass = val;
      }
-
+#endif
      return 0;
  }

@@ -848,7 +849,9 @@ UCLASS_DRIVER(usb) = {
      .id        = UCLASS_USB,
      .name        = "usb",
      .flags        = DM_UC_FLAG_SEQ_ALIAS,
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
      .post_bind    = dm_scan_fdt_dev,
+#endif
      .priv_auto    = sizeof(struct usb_uclass_priv),
      .per_child_auto    = sizeof(struct usb_device),
      .per_device_auto    = sizeof(struct usb_bus_priv),