Hi Marek,

sorry for the late comments but I think the driver doesn't work as expected.

Am 10.04.2022 um 06:27 schrieb Marek Vasut:
This patch adds a driver for configuration of the Microchip USB251xB/xBi
USB 2.0 hub controller series with USB 2.0 upstream connectivity, SMBus
configuration interface and two to four USB 2.0 downstream ports.

This is ported from Linux as of Linux kernel commit
5c2b9c61ae5d8 ("usb: usb251xb: add boost-up property support")

Signed-off-by: Marek Vasut <ma...@denx.de>
Cc: Bin Meng <bmeng...@gmail.com>
Cc: Michal Simek <michal.si...@xilinx.com>
Cc: Simon Glass <s...@chromium.org>
---
  drivers/misc/Kconfig    |   9 +
  drivers/misc/Makefile   |   1 +
  drivers/misc/usb251xb.c | 605 ++++++++++++++++++++++++++++++++++++++++
  3 files changed, 615 insertions(+)
  create mode 100644 drivers/misc/usb251xb.c

diff --git a/drivers/misc/usb251xb.c b/drivers/misc/usb251xb.c
new file mode 100644
index 00000000000..077edc25045
--- /dev/null
+++ b/drivers/misc/usb251xb.c
@@ -0,0 +1,605 @@

[snip]

+static int usb251xb_of_to_plat(struct udevice *dev)
+{
+       struct usb251xb_data *data =
+               (struct usb251xb_data *)dev_get_driver_data(dev);
+       struct usb251xb *hub = dev_get_priv(dev);
+       char str[USB251XB_STRING_BUFSIZE / 2];
+       const char *cproperty_char;
+       u32 property_u32 = 0;
+       int len, err;
+
+       if (dev_read_bool(dev, "skip-config"))
+               hub->skip_config = 1;
+       else
+               hub->skip_config = 0;
+
+       err = gpio_request_by_name(dev, "reset-gpios", 0, &hub->gpio_reset,
+                                  GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
+       if (err && err != -ENOENT) {
+               dev_err(dev, "unable to request GPIO reset pin (%d)\n", err);
+               return err;
+       }
+
+       if (IS_ENABLED(CONFIG_DM_REGULATOR)) {
+               err = device_get_supply_regulator(dev, "vdd-supply",
+                                                 &hub->vdd);
+               if (err && err != -ENOENT) {
+                       dev_err(dev, "Warning: cannot get power supply\n");
+                       return err;
+               }
+       }

Shouldn't the gpio and regulator request be part of the probe?

+
+       if (dev_read_u32(dev, "vendor-id", &hub->vendor_id))
+               hub->vendor_id = USB251XB_DEF_VENDOR_ID;
+
+       if (dev_read_u32(dev, "product-id", &hub->product_id))
+               hub->product_id = data->product_id;
+
+       if (dev_read_u32(dev, "device-id", &hub->device_id))
+               hub->device_id = USB251XB_DEF_DEVICE_ID;
+

[snip]

+       if (dev_read_u32(dev, "language-id", &hub->lang_id))
+               hub->lang_id = USB251XB_DEF_LANGUAGE_ID;
+

This doesn't work because the ids are 16 bit [1,2] and the dev_read_u32 function checks the size.

+       if (!dev_read_u32(dev, "boost-up", &hub->boost_up))
+               hub->boost_up = USB251XB_DEF_BOOST_UP;

This looks like a 8 bit value [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/misc/usb251xb.c [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/usb/usb251xb.txt

Regards,
  Stefan

Reply via email to