[PATCH] usb: onboard-hub: Update the bind function based on "peer_hub" property
Add the bool variable "peer_hub" and set this only for the hubs which have the "peer-hub" property in their DT nodes. Skip the bind function for usb hubs which don't have "peer-hub" property.
Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is already probed") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com --- common/usb_onboard_hub.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index 6f28036e09..0a8fac38ee 100644 --- a/common/usb_onboard_hub.c +++ b/common/usb_onboard_hub.c @@ -26,6 +26,7 @@ struct onboard_hub {
struct onboard_hub_data { unsigned long reset_us; + bool peer_hub; unsigned long power_on_delay_us; int (*init)(struct udevice *dev); }; @@ -178,10 +179,15 @@ err:
static int usb_onboard_hub_bind(struct udevice *dev) { + struct onboard_hub_data *data = + (struct onboard_hub_data *)dev_get_driver_data(dev); struct ofnode_phandle_args phandle; const void *fdt = gd->fdt_blob; int ret, off;
+ if (!data->peer_hub) + return 0; + ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, &phandle); if (ret) { dev_err(dev, "peer-hub not specified\n"); @@ -212,12 +218,14 @@ static int usb_onboard_hub_remove(struct udevice *dev) }
static const struct onboard_hub_data usb2514_data = { + .peer_hub = false, .power_on_delay_us = 500, .reset_us = 1, };
static const struct onboard_hub_data usb5744_data = { .init = usb5744_i2c_init, + .peer_hub = true, .power_on_delay_us = 1000, .reset_us = 5, };
On 12/3/24 05:49, Venkatesh Yadav Abbarapu wrote:
Add the bool variable "peer_hub" and set this only for the hubs which have the "peer-hub" property in their DT nodes. Skip the bind function for usb hubs which don't have "peer-hub" property.
Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is already probed") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
common/usb_onboard_hub.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index 6f28036e09..0a8fac38ee 100644 --- a/common/usb_onboard_hub.c +++ b/common/usb_onboard_hub.c @@ -26,6 +26,7 @@ struct onboard_hub {
struct onboard_hub_data { unsigned long reset_us;
- bool peer_hub; unsigned long power_on_delay_us; int (*init)(struct udevice *dev); };
@@ -178,10 +179,15 @@ err:
static int usb_onboard_hub_bind(struct udevice *dev) {
struct onboard_hub_data *data =
(struct onboard_hub_data *)dev_get_driver_data(dev);struct ofnode_phandle_args phandle; const void *fdt = gd->fdt_blob; int ret, off;
if (!data->peer_hub)
return 0;ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, &phandle); if (ret) { dev_err(dev, "peer-hub not specified\n");
@@ -212,12 +218,14 @@ static int usb_onboard_hub_remove(struct udevice *dev) }
static const struct onboard_hub_data usb2514_data = {
.peer_hub = false, .power_on_delay_us = 500, .reset_us = 1, };
static const struct onboard_hub_data usb5744_data = { .init = usb5744_i2c_init,
.peer_hub = true, .power_on_delay_us = 1000, .reset_us = 5, };
I don't think this is correct solution. I think if optional property peer-hub is not present in DT bind should return 0 and that should fix issue on stm board where only one hub is present.
Thanks, Michal
On 12/3/24 2:44 PM, Michal Simek wrote:
On 12/3/24 05:49, Venkatesh Yadav Abbarapu wrote:
Add the bool variable "peer_hub" and set this only for the hubs which have the "peer-hub" property in their DT nodes. Skip the bind function for usb hubs which don't have "peer-hub" property.
Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is already probed") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
common/usb_onboard_hub.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index 6f28036e09..0a8fac38ee 100644 --- a/common/usb_onboard_hub.c +++ b/common/usb_onboard_hub.c @@ -26,6 +26,7 @@ struct onboard_hub { struct onboard_hub_data { unsigned long reset_us; + bool peer_hub; unsigned long power_on_delay_us; int (*init)(struct udevice *dev); }; @@ -178,10 +179,15 @@ err: static int usb_onboard_hub_bind(struct udevice *dev) { + struct onboard_hub_data *data = + (struct onboard_hub_data *)dev_get_driver_data(dev); struct ofnode_phandle_args phandle; const void *fdt = gd->fdt_blob; int ret, off; + if (!data->peer_hub) + return 0;
ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, &phandle); if (ret) { dev_err(dev, "peer-hub not specified\n"); @@ -212,12 +218,14 @@ static int usb_onboard_hub_remove(struct udevice *dev) } static const struct onboard_hub_data usb2514_data = { + .peer_hub = false, .power_on_delay_us = 500, .reset_us = 1, }; static const struct onboard_hub_data usb5744_data = { .init = usb5744_i2c_init, + .peer_hub = true, .power_on_delay_us = 1000, .reset_us = 5, };
I don't think this is correct solution. I think if optional property peer-hub is not present in DT bind should return 0 and that should fix issue on stm board where only one hub is present.
Shouldn't the 'peer-hub' property be mandatory for this hub ?
On 12/3/24 17:24, Marek Vasut wrote:
On 12/3/24 2:44 PM, Michal Simek wrote:
On 12/3/24 05:49, Venkatesh Yadav Abbarapu wrote:
Add the bool variable "peer_hub" and set this only for the hubs which have the "peer-hub" property in their DT nodes. Skip the bind function for usb hubs which don't have "peer-hub" property.
Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is already probed") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
common/usb_onboard_hub.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index 6f28036e09..0a8fac38ee 100644 --- a/common/usb_onboard_hub.c +++ b/common/usb_onboard_hub.c @@ -26,6 +26,7 @@ struct onboard_hub { struct onboard_hub_data { unsigned long reset_us; + bool peer_hub; unsigned long power_on_delay_us; int (*init)(struct udevice *dev); }; @@ -178,10 +179,15 @@ err: static int usb_onboard_hub_bind(struct udevice *dev) { + struct onboard_hub_data *data = + (struct onboard_hub_data *)dev_get_driver_data(dev); struct ofnode_phandle_args phandle; const void *fdt = gd->fdt_blob; int ret, off; + if (!data->peer_hub) + return 0;
ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, &phandle); if (ret) { dev_err(dev, "peer-hub not specified\n"); @@ -212,12 +218,14 @@ static int usb_onboard_hub_remove(struct udevice *dev) } static const struct onboard_hub_data usb2514_data = { + .peer_hub = false, .power_on_delay_us = 500, .reset_us = 1, }; static const struct onboard_hub_data usb5744_data = { .init = usb5744_i2c_init, + .peer_hub = true, .power_on_delay_us = 1000, .reset_us = 5, };
I don't think this is correct solution. I think if optional property peer-hub is not present in DT bind should return 0 and that should fix issue on stm board where only one hub is present.
Shouldn't the 'peer-hub' property be mandatory for this hub ?
You can use this hub with usb2.0 only or with usb3.0 only. It means peer-hub is correctly optional property. It is just saying when it is there hub should be configured only once not twice.
It means right fix is when peer-hub property is missing bind should just pass because it is just hub which don't use another ID.
Thanks, Michal
On 12/3/24 6:02 PM, Michal Simek wrote:
On 12/3/24 17:24, Marek Vasut wrote:
On 12/3/24 2:44 PM, Michal Simek wrote:
On 12/3/24 05:49, Venkatesh Yadav Abbarapu wrote:
Add the bool variable "peer_hub" and set this only for the hubs which have the "peer-hub" property in their DT nodes. Skip the bind function for usb hubs which don't have "peer-hub" property.
Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is already probed") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
common/usb_onboard_hub.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index 6f28036e09..0a8fac38ee 100644 --- a/common/usb_onboard_hub.c +++ b/common/usb_onboard_hub.c @@ -26,6 +26,7 @@ struct onboard_hub { struct onboard_hub_data { unsigned long reset_us; + bool peer_hub; unsigned long power_on_delay_us; int (*init)(struct udevice *dev); }; @@ -178,10 +179,15 @@ err: static int usb_onboard_hub_bind(struct udevice *dev) { + struct onboard_hub_data *data = + (struct onboard_hub_data *)dev_get_driver_data(dev); struct ofnode_phandle_args phandle; const void *fdt = gd->fdt_blob; int ret, off; + if (!data->peer_hub) + return 0;
ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, &phandle); if (ret) { dev_err(dev, "peer-hub not specified\n"); @@ -212,12 +218,14 @@ static int usb_onboard_hub_remove(struct udevice *dev) } static const struct onboard_hub_data usb2514_data = { + .peer_hub = false, .power_on_delay_us = 500, .reset_us = 1, }; static const struct onboard_hub_data usb5744_data = { .init = usb5744_i2c_init, + .peer_hub = true, .power_on_delay_us = 1000, .reset_us = 5, };
I don't think this is correct solution. I think if optional property peer-hub is not present in DT bind should return 0 and that should fix issue on stm board where only one hub is present.
Shouldn't the 'peer-hub' property be mandatory for this hub ?
You can use this hub with usb2.0 only or with usb3.0 only. It means peer-hub is correctly optional property. It is just saying when it is there hub should be configured only once not twice.
It means right fix is when peer-hub property is missing bind should just pass because it is just hub which don't use another ID.
OK
On 12/3/24 05:49, Venkatesh Yadav Abbarapu wrote:
Add the bool variable "peer_hub" and set this only for the hubs which have the "peer-hub" property in their DT nodes. Skip the bind function for usb hubs which don't have "peer-hub" property.
Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is already probed") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
common/usb_onboard_hub.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index 6f28036e09..0a8fac38ee 100644 --- a/common/usb_onboard_hub.c +++ b/common/usb_onboard_hub.c @@ -26,6 +26,7 @@ struct onboard_hub {
struct onboard_hub_data { unsigned long reset_us;
- bool peer_hub; unsigned long power_on_delay_us; int (*init)(struct udevice *dev);
}; @@ -178,10 +179,15 @@ err:
static int usb_onboard_hub_bind(struct udevice *dev) {
struct onboard_hub_data *data =
(struct onboard_hub_data *)dev_get_driver_data(dev);struct ofnode_phandle_args phandle; const void *fdt = gd->fdt_blob; int ret, off;
if (!data->peer_hub)
return 0;ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, &phandle); if (ret) { dev_err(dev, "peer-hub not specified\n");
@@ -212,12 +218,14 @@ static int usb_onboard_hub_remove(struct udevice *dev) }
static const struct onboard_hub_data usb2514_data = {
- .peer_hub = false, .power_on_delay_us = 500, .reset_us = 1,
};
static const struct onboard_hub_data usb5744_data = { .init = usb5744_i2c_init,
- .peer_hub = true, .power_on_delay_us = 1000, .reset_us = 5,
};
Tested on STM32MP157c-DK2 board
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Tested-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
On 12/3/24 5:49 AM, Venkatesh Yadav Abbarapu wrote:
Add the bool variable "peer_hub" and set this only for the hubs which have the "peer-hub" property in their DT nodes. Skip the bind function for usb hubs which don't have "peer-hub" property.
Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is already probed") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
common/usb_onboard_hub.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index 6f28036e09..0a8fac38ee 100644 --- a/common/usb_onboard_hub.c +++ b/common/usb_onboard_hub.c @@ -26,6 +26,7 @@ struct onboard_hub {
struct onboard_hub_data { unsigned long reset_us;
- bool peer_hub; unsigned long power_on_delay_us;
Move the bool here , so it is not between the two delay descriptors.
int (*init)(struct udevice *dev); }; @@ -178,10 +179,15 @@ err:
static int usb_onboard_hub_bind(struct udevice *dev) {
- struct onboard_hub_data *data =
(struct onboard_hub_data *)dev_get_driver_data(dev);
Is the cast necessary ?
[...]
participants (4)
-
Marek Vasut -
Michal Simek -
Patrice CHOTARD -
Venkatesh Yadav Abbarapu