* [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller
@ 2022-07-27 9:37 Alexander Stein
2022-07-27 9:38 ` [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support Alexander Stein
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alexander Stein @ 2022-07-27 9:37 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Matthias Kaehlcke
Cc: Alexander Stein, linux-usb, devicetree
The TI USB8041 is a USB 3.0 hub controller with 4 ports.
This initial version of the binding only describes USB related aspects
of the USB8041, it does not cover the option of connecting the controller
as an i2c slave.
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Thanks Matthias for your review.
Changes in v3:
* Put myself as maintainer
* Removed quotes around description
Changes in v2:
* Removed 'items' from compatible, it's just en enum now
* Rename reset-gpio to reset-gpios
* Use 'items' for reset-gpios
* Adjust description of vdd-supply
* Sorted required list
* Adjusted example
.../devicetree/bindings/usb/ti,usb8041.yaml | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ti,usb8041.yaml
diff --git a/Documentation/devicetree/bindings/usb/ti,usb8041.yaml b/Documentation/devicetree/bindings/usb/ti,usb8041.yaml
new file mode 100644
index 000000000000..e04fbd8ab0b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,usb8041.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/ti,usb8041.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Binding for the TI USB8041 USB 3.0 hub controller
+
+maintainers:
+ - Alexander Stein <alexander.stein@ew.tq-group.com>
+
+allOf:
+ - $ref: usb-device.yaml#
+
+properties:
+ compatible:
+ enum:
+ - usb451,8140
+ - usb451,8142
+
+ reg: true
+
+ reset-gpios:
+ items:
+ - description: GPIO specifier for GRST# pin.
+
+ vdd-supply:
+ description:
+ VDD power supply to the hub
+
+ peer-hub:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ phandle to the peer hub on the controller.
+
+required:
+ - compatible
+ - reg
+ - peer-hub
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ usb {
+ dr_mode = "host";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* 2.0 hub on port 1 */
+ hub_2_0: hub@1 {
+ compatible = "usb451,8142";
+ reg = <1>;
+ peer-hub = <&hub_3_0>;
+ reset-gpios = <&gpio1 11 GPIO_ACTIVE_LOW>;
+ };
+
+ /* 3.0 hub on port 2 */
+ hub_3_0: hub@2 {
+ compatible = "usb451,8140";
+ reg = <2>;
+ peer-hub = <&hub_2_0>;
+ reset-gpios = <&gpio1 11 GPIO_ACTIVE_LOW>;
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support
2022-07-27 9:37 [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Alexander Stein
@ 2022-07-27 9:38 ` Alexander Stein
2022-07-27 12:40 ` Greg Kroah-Hartman
2022-07-27 16:32 ` Matthias Kaehlcke
2022-07-27 9:38 ` [PATCH v3 3/3] usb: misc: onboard_usb_hub: Add TI USB8041 hub support Alexander Stein
2022-07-27 9:56 ` [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Krzysztof Kozlowski
2 siblings, 2 replies; 8+ messages in thread
From: Alexander Stein @ 2022-07-27 9:38 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Matthias Kaehlcke
Cc: Alexander Stein, linux-usb, devicetree
Despite default reset upon probe, release reset line after powering up
the hub and assert reset again before powering down.
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Thanks Matthias for your review.
Changes in v3:
* Require platform data unconditionally
* Removed unecessary checks for that reason
* Merged power_stable_time into reset_duration (no difference for now)
* Rename 'reset_duration' to 'reset_us'
* Renamed platform structure to onboard_hub_pdata
* Renamed device struct field to pdata as well
Changes in v2:
* Use device specific sleep times, if present
* Use fsleep instead of usleep_range
drivers/usb/misc/onboard_usb_hub.c | 28 ++++++++++++++++++++++++++++
drivers/usb/misc/onboard_usb_hub.h | 16 ++++++++++++----
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
index 6b9b949d17d3..2aa1db528b31 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -7,6 +7,7 @@
#include <linux/device.h>
#include <linux/export.h>
+#include <linux/gpio/consumer.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/list.h>
@@ -38,6 +39,8 @@ struct usbdev_node {
struct onboard_hub {
struct regulator *vdd;
struct device *dev;
+ const struct onboard_hub_pdata *pdata;
+ struct gpio_desc *reset_gpio;
bool always_powered_in_suspend;
bool is_powered_on;
bool going_away;
@@ -56,6 +59,9 @@ static int onboard_hub_power_on(struct onboard_hub *hub)
return err;
}
+ fsleep(hub->pdata->reset_us);
+ gpiod_set_value_cansleep(hub->reset_gpio, 0);
+
hub->is_powered_on = true;
return 0;
@@ -65,6 +71,11 @@ static int onboard_hub_power_off(struct onboard_hub *hub)
{
int err;
+ if (hub->reset_gpio) {
+ gpiod_set_value_cansleep(hub->reset_gpio, 1);
+ fsleep(hub->pdata->reset_us);
+ }
+
err = regulator_disable(hub->vdd);
if (err) {
dev_err(hub->dev, "failed to disable regulator: %d\n", err);
@@ -219,6 +230,7 @@ static void onboard_hub_attach_usb_driver(struct work_struct *work)
static int onboard_hub_probe(struct platform_device *pdev)
{
+ const struct of_device_id *of_id;
struct device *dev = &pdev->dev;
struct onboard_hub *hub;
int err;
@@ -227,10 +239,26 @@ static int onboard_hub_probe(struct platform_device *pdev)
if (!hub)
return -ENOMEM;
+ of_id = of_match_device(onboard_hub_match, &pdev->dev);
+ if (!of_id)
+ return -ENODEV;
+
+ hub->pdata = of_id->data;
+ if (!hub->pdata)
+ return -EINVAL;
+
hub->vdd = devm_regulator_get(dev, "vdd");
if (IS_ERR(hub->vdd))
return PTR_ERR(hub->vdd);
+ hub->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(hub->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(hub->reset_gpio), "failed to get reset GPIO\n");
+
+ if (hub->reset_gpio)
+ fsleep(hub->pdata->reset_us);
+
hub->dev = dev;
mutex_init(&hub->lock);
INIT_LIST_HEAD(&hub->udev_list);
diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
index d3a5b6938582..01d067db81f2 100644
--- a/drivers/usb/misc/onboard_usb_hub.h
+++ b/drivers/usb/misc/onboard_usb_hub.h
@@ -6,11 +6,19 @@
#ifndef _USB_MISC_ONBOARD_USB_HUB_H
#define _USB_MISC_ONBOARD_USB_HUB_H
+struct onboard_hub_pdata {
+ unsigned long reset_us; /* reset pulse width in us */
+};
+
+static const struct onboard_hub_pdata realtek_rts5411_data = {
+ .reset_us = 0,
+};
+
static const struct of_device_id onboard_hub_match[] = {
- { .compatible = "usbbda,411" },
- { .compatible = "usbbda,5411" },
- { .compatible = "usbbda,414" },
- { .compatible = "usbbda,5414" },
+ { .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
+ { .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
+ { .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
+ { .compatible = "usbbda,5414", .data = &realtek_rts5411_data, },
{}
};
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] usb: misc: onboard_usb_hub: Add TI USB8041 hub support
2022-07-27 9:37 [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Alexander Stein
2022-07-27 9:38 ` [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support Alexander Stein
@ 2022-07-27 9:38 ` Alexander Stein
2022-07-27 9:56 ` [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Krzysztof Kozlowski
2 siblings, 0 replies; 8+ messages in thread
From: Alexander Stein @ 2022-07-27 9:38 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Matthias Kaehlcke
Cc: Alexander Stein, linux-usb, devicetree
This is a 4-port 3.0 USB hub.
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Thanks Matthias for your review.
Changes in v3:
* Removed now unneeded power_stable_time
* Adjust for reset_us rename
Changes in v2:
* Add devtype data containing waiting times
drivers/usb/misc/onboard_usb_hub.c | 3 +++
drivers/usb/misc/onboard_usb_hub.h | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
index 2aa1db528b31..276174c20e7a 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -338,6 +338,7 @@ static struct platform_driver onboard_hub_driver = {
/************************** USB driver **************************/
#define VENDOR_ID_REALTEK 0x0bda
+#define VENDOR_ID_TI 0x0451
/*
* Returns the onboard_hub platform device that is associated with the USB
@@ -415,6 +416,8 @@ static const struct usb_device_id onboard_hub_id_table[] = {
{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
{ USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 */
{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 */
+ { USB_DEVICE(VENDOR_ID_TI, 0x8140) }, /* TI USB8041 3.0 */
+ { USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 */
{}
};
MODULE_DEVICE_TABLE(usb, onboard_hub_id_table);
diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
index 01d067db81f2..290afc2f504f 100644
--- a/drivers/usb/misc/onboard_usb_hub.h
+++ b/drivers/usb/misc/onboard_usb_hub.h
@@ -14,7 +14,13 @@ static const struct onboard_hub_pdata realtek_rts5411_data = {
.reset_us = 0,
};
+static const struct onboard_hub_pdata ti_tusb8041_data = {
+ .reset_us = 3000,
+};
+
static const struct of_device_id onboard_hub_match[] = {
+ { .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
+ { .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller
2022-07-27 9:37 [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Alexander Stein
2022-07-27 9:38 ` [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support Alexander Stein
2022-07-27 9:38 ` [PATCH v3 3/3] usb: misc: onboard_usb_hub: Add TI USB8041 hub support Alexander Stein
@ 2022-07-27 9:56 ` Krzysztof Kozlowski
2022-07-27 20:28 ` Matthias Kaehlcke
2 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-27 9:56 UTC (permalink / raw)
To: Alexander Stein, Greg Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Matthias Kaehlcke
Cc: linux-usb, devicetree
On 27/07/2022 11:37, Alexander Stein wrote:
> The TI USB8041 is a USB 3.0 hub controller with 4 ports.
>
> This initial version of the binding only describes USB related aspects
> of the USB8041, it does not cover the option of connecting the controller
> as an i2c slave.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support
2022-07-27 9:38 ` [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support Alexander Stein
@ 2022-07-27 12:40 ` Greg Kroah-Hartman
2022-07-27 16:32 ` Matthias Kaehlcke
1 sibling, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2022-07-27 12:40 UTC (permalink / raw)
To: Alexander Stein
Cc: Rob Herring, Krzysztof Kozlowski, Matthias Kaehlcke, linux-usb,
devicetree
On Wed, Jul 27, 2022 at 11:38:00AM +0200, Alexander Stein wrote:
> Despite default reset upon probe, release reset line after powering up
> the hub and assert reset again before powering down.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Thanks Matthias for your review.
>
> Changes in v3:
> * Require platform data unconditionally
> * Removed unecessary checks for that reason
> * Merged power_stable_time into reset_duration (no difference for now)
> * Rename 'reset_duration' to 'reset_us'
> * Renamed platform structure to onboard_hub_pdata
> * Renamed device struct field to pdata as well
>
> Changes in v2:
> * Use device specific sleep times, if present
> * Use fsleep instead of usleep_range
>
> drivers/usb/misc/onboard_usb_hub.c | 28 ++++++++++++++++++++++++++++
> drivers/usb/misc/onboard_usb_hub.h | 16 ++++++++++++----
> 2 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> index 6b9b949d17d3..2aa1db528b31 100644
> --- a/drivers/usb/misc/onboard_usb_hub.c
> +++ b/drivers/usb/misc/onboard_usb_hub.c
> @@ -7,6 +7,7 @@
>
> #include <linux/device.h>
> #include <linux/export.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/list.h>
> @@ -38,6 +39,8 @@ struct usbdev_node {
> struct onboard_hub {
> struct regulator *vdd;
> struct device *dev;
> + const struct onboard_hub_pdata *pdata;
> + struct gpio_desc *reset_gpio;
> bool always_powered_in_suspend;
> bool is_powered_on;
> bool going_away;
> @@ -56,6 +59,9 @@ static int onboard_hub_power_on(struct onboard_hub *hub)
> return err;
> }
>
> + fsleep(hub->pdata->reset_us);
> + gpiod_set_value_cansleep(hub->reset_gpio, 0);
> +
> hub->is_powered_on = true;
>
> return 0;
> @@ -65,6 +71,11 @@ static int onboard_hub_power_off(struct onboard_hub *hub)
> {
> int err;
>
> + if (hub->reset_gpio) {
> + gpiod_set_value_cansleep(hub->reset_gpio, 1);
> + fsleep(hub->pdata->reset_us);
> + }
> +
> err = regulator_disable(hub->vdd);
> if (err) {
> dev_err(hub->dev, "failed to disable regulator: %d\n", err);
> @@ -219,6 +230,7 @@ static void onboard_hub_attach_usb_driver(struct work_struct *work)
>
> static int onboard_hub_probe(struct platform_device *pdev)
> {
> + const struct of_device_id *of_id;
> struct device *dev = &pdev->dev;
> struct onboard_hub *hub;
> int err;
> @@ -227,10 +239,26 @@ static int onboard_hub_probe(struct platform_device *pdev)
> if (!hub)
> return -ENOMEM;
>
> + of_id = of_match_device(onboard_hub_match, &pdev->dev);
> + if (!of_id)
> + return -ENODEV;
> +
> + hub->pdata = of_id->data;
> + if (!hub->pdata)
> + return -EINVAL;
> +
> hub->vdd = devm_regulator_get(dev, "vdd");
> if (IS_ERR(hub->vdd))
> return PTR_ERR(hub->vdd);
>
> + hub->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(hub->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(hub->reset_gpio), "failed to get reset GPIO\n");
> +
> + if (hub->reset_gpio)
> + fsleep(hub->pdata->reset_us);
> +
> hub->dev = dev;
> mutex_init(&hub->lock);
> INIT_LIST_HEAD(&hub->udev_list);
> diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> index d3a5b6938582..01d067db81f2 100644
> --- a/drivers/usb/misc/onboard_usb_hub.h
> +++ b/drivers/usb/misc/onboard_usb_hub.h
> @@ -6,11 +6,19 @@
> #ifndef _USB_MISC_ONBOARD_USB_HUB_H
> #define _USB_MISC_ONBOARD_USB_HUB_H
>
> +struct onboard_hub_pdata {
> + unsigned long reset_us; /* reset pulse width in us */
> +};
> +
> +static const struct onboard_hub_pdata realtek_rts5411_data = {
> + .reset_us = 0,
> +};
> +
> static const struct of_device_id onboard_hub_match[] = {
> - { .compatible = "usbbda,411" },
> - { .compatible = "usbbda,5411" },
> - { .compatible = "usbbda,414" },
> - { .compatible = "usbbda,5414" },
> + { .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
> + { .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
> + { .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
> + { .compatible = "usbbda,5414", .data = &realtek_rts5411_data, },
> {}
> };
>
> --
> 2.25.1
>
This no longer applies to my tree due to some new devices being added.
Can you rebase against the usb-testing branch of the usb.git tree and
resend?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support
2022-07-27 9:38 ` [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support Alexander Stein
2022-07-27 12:40 ` Greg Kroah-Hartman
@ 2022-07-27 16:32 ` Matthias Kaehlcke
2022-07-27 17:16 ` Matthias Kaehlcke
1 sibling, 1 reply; 8+ messages in thread
From: Matthias Kaehlcke @ 2022-07-27 16:32 UTC (permalink / raw)
To: Alexander Stein
Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, linux-usb,
devicetree
Hi Alexander,
On Wed, Jul 27, 2022 at 11:38:00AM +0200, Alexander Stein wrote:
> Despite default reset upon probe, release reset line after powering up
> the hub and assert reset again before powering down.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Thanks Matthias for your review.
>
> Changes in v3:
> * Require platform data unconditionally
> * Removed unecessary checks for that reason
> * Merged power_stable_time into reset_duration (no difference for now)
> * Rename 'reset_duration' to 'reset_us'
> * Renamed platform structure to onboard_hub_pdata
> * Renamed device struct field to pdata as well
>
> Changes in v2:
> * Use device specific sleep times, if present
> * Use fsleep instead of usleep_range
>
> drivers/usb/misc/onboard_usb_hub.c | 28 ++++++++++++++++++++++++++++
> drivers/usb/misc/onboard_usb_hub.h | 16 ++++++++++++----
> 2 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> index 6b9b949d17d3..2aa1db528b31 100644
> --- a/drivers/usb/misc/onboard_usb_hub.c
> +++ b/drivers/usb/misc/onboard_usb_hub.c
> @@ -7,6 +7,7 @@
>
> #include <linux/device.h>
> #include <linux/export.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/list.h>
> @@ -38,6 +39,8 @@ struct usbdev_node {
> struct onboard_hub {
> struct regulator *vdd;
> struct device *dev;
> + const struct onboard_hub_pdata *pdata;
> + struct gpio_desc *reset_gpio;
> bool always_powered_in_suspend;
> bool is_powered_on;
> bool going_away;
> @@ -56,6 +59,9 @@ static int onboard_hub_power_on(struct onboard_hub *hub)
> return err;
> }
>
> + fsleep(hub->pdata->reset_us);
> + gpiod_set_value_cansleep(hub->reset_gpio, 0);
> +
> hub->is_powered_on = true;
>
> return 0;
> @@ -65,6 +71,11 @@ static int onboard_hub_power_off(struct onboard_hub *hub)
> {
> int err;
>
> + if (hub->reset_gpio) {
> + gpiod_set_value_cansleep(hub->reset_gpio, 1);
> + fsleep(hub->pdata->reset_us);
Is this delay here actually needed? There is a delay in onboard_hub_power_on(),
before de-asserting the reset, isn't that enough?
> + }
> +
> err = regulator_disable(hub->vdd);
> if (err) {
> dev_err(hub->dev, "failed to disable regulator: %d\n", err);
> @@ -219,6 +230,7 @@ static void onboard_hub_attach_usb_driver(struct work_struct *work)
>
> static int onboard_hub_probe(struct platform_device *pdev)
> {
> + const struct of_device_id *of_id;
> struct device *dev = &pdev->dev;
> struct onboard_hub *hub;
> int err;
> @@ -227,10 +239,26 @@ static int onboard_hub_probe(struct platform_device *pdev)
> if (!hub)
> return -ENOMEM;
>
> + of_id = of_match_device(onboard_hub_match, &pdev->dev);
> + if (!of_id)
> + return -ENODEV;
> +
> + hub->pdata = of_id->data;
> + if (!hub->pdata)
> + return -EINVAL;
> +
> hub->vdd = devm_regulator_get(dev, "vdd");
> if (IS_ERR(hub->vdd))
> return PTR_ERR(hub->vdd);
>
> + hub->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(hub->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(hub->reset_gpio), "failed to get reset GPIO\n");
> +
> + if (hub->reset_gpio)
> + fsleep(hub->pdata->reset_us);
Same question here: onboard_hub_power_on() is called a few lines below and
has a delay before de-asserting the reset. Is the delay here really needed?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support
2022-07-27 16:32 ` Matthias Kaehlcke
@ 2022-07-27 17:16 ` Matthias Kaehlcke
0 siblings, 0 replies; 8+ messages in thread
From: Matthias Kaehlcke @ 2022-07-27 17:16 UTC (permalink / raw)
To: Alexander Stein
Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, linux-usb,
devicetree
On Wed, Jul 27, 2022 at 09:32:03AM -0700, Matthias Kaehlcke wrote:
> Hi Alexander,
>
> On Wed, Jul 27, 2022 at 11:38:00AM +0200, Alexander Stein wrote:
> > Despite default reset upon probe, release reset line after powering up
> > the hub and assert reset again before powering down.
> >
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Thanks Matthias for your review.
> >
> > Changes in v3:
> > * Require platform data unconditionally
> > * Removed unecessary checks for that reason
> > * Merged power_stable_time into reset_duration (no difference for now)
> > * Rename 'reset_duration' to 'reset_us'
> > * Renamed platform structure to onboard_hub_pdata
> > * Renamed device struct field to pdata as well
> >
> > Changes in v2:
> > * Use device specific sleep times, if present
> > * Use fsleep instead of usleep_range
> >
> > drivers/usb/misc/onboard_usb_hub.c | 28 ++++++++++++++++++++++++++++
> > drivers/usb/misc/onboard_usb_hub.h | 16 ++++++++++++----
> > 2 files changed, 40 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> > index 6b9b949d17d3..2aa1db528b31 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.c
> > +++ b/drivers/usb/misc/onboard_usb_hub.c
> > @@ -7,6 +7,7 @@
> >
> > #include <linux/device.h>
> > #include <linux/export.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/init.h>
> > #include <linux/kernel.h>
> > #include <linux/list.h>
> > @@ -38,6 +39,8 @@ struct usbdev_node {
> > struct onboard_hub {
> > struct regulator *vdd;
> > struct device *dev;
> > + const struct onboard_hub_pdata *pdata;
> > + struct gpio_desc *reset_gpio;
> > bool always_powered_in_suspend;
> > bool is_powered_on;
> > bool going_away;
> > @@ -56,6 +59,9 @@ static int onboard_hub_power_on(struct onboard_hub *hub)
> > return err;
> > }
> >
> > + fsleep(hub->pdata->reset_us);
> > + gpiod_set_value_cansleep(hub->reset_gpio, 0);
> > +
> > hub->is_powered_on = true;
> >
> > return 0;
> > @@ -65,6 +71,11 @@ static int onboard_hub_power_off(struct onboard_hub *hub)
> > {
> > int err;
> >
> > + if (hub->reset_gpio) {
> > + gpiod_set_value_cansleep(hub->reset_gpio, 1);
> > + fsleep(hub->pdata->reset_us);
>
> Is this delay here actually needed? There is a delay in onboard_hub_power_on(),
> before de-asserting the reset, isn't that enough?
>
> > + }
> > +
> > err = regulator_disable(hub->vdd);
> > if (err) {
> > dev_err(hub->dev, "failed to disable regulator: %d\n", err);
> > @@ -219,6 +230,7 @@ static void onboard_hub_attach_usb_driver(struct work_struct *work)
> >
> > static int onboard_hub_probe(struct platform_device *pdev)
> > {
> > + const struct of_device_id *of_id;
> > struct device *dev = &pdev->dev;
> > struct onboard_hub *hub;
> > int err;
> > @@ -227,10 +239,26 @@ static int onboard_hub_probe(struct platform_device *pdev)
> > if (!hub)
> > return -ENOMEM;
> >
> > + of_id = of_match_device(onboard_hub_match, &pdev->dev);
> > + if (!of_id)
> > + return -ENODEV;
> > +
> > + hub->pdata = of_id->data;
> > + if (!hub->pdata)
> > + return -EINVAL;
> > +
> > hub->vdd = devm_regulator_get(dev, "vdd");
> > if (IS_ERR(hub->vdd))
> > return PTR_ERR(hub->vdd);
> >
> > + hub->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > + GPIOD_OUT_HIGH);
> > + if (IS_ERR(hub->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(hub->reset_gpio), "failed to get reset GPIO\n");
> > +
> > + if (hub->reset_gpio)
> > + fsleep(hub->pdata->reset_us);
>
> Same question here: onboard_hub_power_on() is called a few lines below and
> has a delay before de-asserting the reset. Is the delay here really needed?
I didn't notice that you had already posted v4 when I replied, let's keep the
discussion on that version for simplicity.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller
2022-07-27 9:56 ` [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Krzysztof Kozlowski
@ 2022-07-27 20:28 ` Matthias Kaehlcke
0 siblings, 0 replies; 8+ messages in thread
From: Matthias Kaehlcke @ 2022-07-27 20:28 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Alexander Stein, Greg Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, linux-usb, devicetree
Hi Alexander,
On Wed, Jul 27, 2022 at 11:56:55AM +0200, Krzysztof Kozlowski wrote:
> On 27/07/2022 11:37, Alexander Stein wrote:
> > The TI USB8041 is a USB 3.0 hub controller with 4 ports.
> >
> > This initial version of the binding only describes USB related aspects
> > of the USB8041, it does not cover the option of connecting the controller
> > as an i2c slave.
> >
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
I noticed that you didn't include the binding changes in v4. Please
make sure to include them until the patch has landed in a maintainer
tree.
Thanks
Matthias
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-07-27 20:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-27 9:37 [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Alexander Stein
2022-07-27 9:38 ` [PATCH v3 2/3] usb: misc: onboard_usb_hub: Add reset-gpio support Alexander Stein
2022-07-27 12:40 ` Greg Kroah-Hartman
2022-07-27 16:32 ` Matthias Kaehlcke
2022-07-27 17:16 ` Matthias Kaehlcke
2022-07-27 9:38 ` [PATCH v3 3/3] usb: misc: onboard_usb_hub: Add TI USB8041 hub support Alexander Stein
2022-07-27 9:56 ` [PATCH v3 1/3] dt-bindings: usb: Add binding for TI USB8041 hub controller Krzysztof Kozlowski
2022-07-27 20:28 ` Matthias Kaehlcke
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.