Hi Lukasz,
On 5/26/25 4:24 PM, Lukasz Czechowski wrote:
Some of the onboard hubs require multiple power supplies, so extend
the driver to support them.
The implementation is inspired by the kernel driver, as introduced
by commit [1] in the v6.10 kernel.
[1]
https://github.com/torvalds/linux/commit/ec1848cd5df426f57a7f6a8a6b95b69259c52cfc
Signed-off-by: Lukasz Czechowski <[email protected]>
---
common/usb_onboard_hub.c | 65 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 49 insertions(+), 16 deletions(-)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c
index dcaa24d30ece..2d2114ea6d17 100644
--- a/common/usb_onboard_hub.c
+++ b/common/usb_onboard_hub.c
@@ -20,14 +20,18 @@
#define USB5744_CONFIG_REG_ACCESS 0x0037
#define USB5744_CONFIG_REG_ACCESS_LSB 0x99
+#define MAX_SUPPLIES 2
+
struct onboard_hub {
- struct udevice *vdd;
+ struct udevice *vdd[MAX_SUPPLIES];
struct gpio_desc *reset_gpio;
};
struct onboard_hub_data {
unsigned long reset_us;
unsigned long power_on_delay_us;
+ unsigned int num_supplies;
+ const char * const supply_names[MAX_SUPPLIES];
int (*init)(struct udevice *dev);
};
@@ -139,30 +143,53 @@ int usb_onboard_hub_reset(struct udevice *dev)
return 0;
}
+static int usb_onboard_hub_power_off(struct udevice *dev)
+{
+ struct onboard_hub_data *data =
+ (struct onboard_hub_data *)dev_get_driver_data(dev);
+ struct onboard_hub *hub = dev_get_priv(dev);
+ int ret = 0;
+ int i;
+
+ for (i = data->num_supplies - 1; i >= 0; i--)
+ if (hub->vdd[i])
+ ret |= regulator_set_enable_if_allowed(hub->vdd[i],
false);
+
I would suggest to print here the failing regulators which I find more
helpful than printing a global "something went wrong when powering off
regulators, here's the joined error code, if there are more than one
failing, good luck" kind of message in the caller.
+ return ret;
+}
+
static int usb_onboard_hub_probe(struct udevice *dev)
{
struct onboard_hub_data *data =
(struct onboard_hub_data *)dev_get_driver_data(dev);
struct onboard_hub *hub = dev_get_priv(dev);
+ unsigned int i;
int ret;
- ret = device_get_supply_regulator(dev, "vdd-supply", &hub->vdd);
- if (ret && ret != -ENOENT) {
- dev_err(dev, "can't get vdd-supply: %d\n", ret);
- return ret;
+ if (data->num_supplies > MAX_SUPPLIES) {
+ dev_err(dev, "invalid supplies number, max supported: %d\n",
MAX_SUPPLIES);
+ return -EINVAL;
}
- if (hub->vdd) {
- ret = regulator_set_enable_if_allowed(hub->vdd, true);
- if (ret && ret != -ENOSYS) {
- dev_err(dev, "can't enable vdd-supply: %d\n", ret);
- return ret;
+ for (i = 0; i < data->num_supplies; i++) {
+ ret = device_get_supply_regulator(dev, data->supply_names[i],
&hub->vdd[i]);
+ if (ret && ret != -ENOENT) {
Please check for -ENOSYS as well, like in 77b053502f39 ("usb:
onboard-hub: Fix return type for regulator APIs") (which is the reason
why your patch doesn't apply cleanly anymore as far as I could tell?).
Looks ok to me otherwise.
Cheers,
Quentin
