linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/2] usb: misc: add support for microchip,usb2244 USB-SD controller
@ 2021-11-22  6:28 Piyush Mehta
  2021-11-22  6:28 ` [PATCH V2 1/2] dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller Piyush Mehta
  2021-11-22  6:28 ` [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller Piyush Mehta
  0 siblings, 2 replies; 10+ messages in thread
From: Piyush Mehta @ 2021-11-22  6:28 UTC (permalink / raw)
  To: gregkh, robh+dt, mka, ravisadineni, stern, alcooperx, michal.simek
  Cc: piyush.mehta, linux-usb, devicetree, linux-kernel, git, sgoud

Microchip's USB224x family of Hi-Speed USB 2.0 flash media card
controllers provide an ultra-fast interface between a USB host
controller and flash media cards.

This patch adds a GPIO based usb-sd reset for USB2244 USB2 ultra
fast SD controller. This usb2244 driver trigger sd reset signal
after soft reset or core Reset. The SD needs to be resetted after
completion of phy initialization. After the toggling of gpio,
controller gets out form reset.

As part of the reset, sets the direction of the pin to output before
toggling the pin. Delay of microseconds is added in between low and
high to meet the setup and hold time requirement of the reset.
---
Changes for V2:
- Update reset polarity, make reset ACTIVE LOW in the dt-binding document
  and usb2244 driver.
- Fix WARNING: msleep < 20ms can sleep for up to 20ms by changing msleep to
  usleep_range()
- Addressed Rob Herring review comments
  - Added usbsd node under the usb controller (usb0) node.
  - Remove Warning: decoded text below may be mangled, UTF-8 assumed.	

Review comments:
Link: https://lore.kernel.org/lkml/20211024180628.2992108-3-piyush.mehta@xilinx.com/t/#m2e146078276051984a144ba074c6cf4eee07b4d0
---
Piyush Mehta (2):
  dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller
  usb: misc: usb244: add support for USB2 ultra fast sd controller

 .../devicetree/bindings/usb/microchip,usb2244.yaml | 47 +++++++++++++++
 drivers/usb/misc/Kconfig                           | 10 ++++
 drivers/usb/misc/Makefile                          |  1 +
 drivers/usb/misc/usb2244.c                         | 69 ++++++++++++++++++++++
 4 files changed, 127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
 create mode 100644 drivers/usb/misc/usb2244.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V2 1/2] dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller
  2021-11-22  6:28 [PATCH V2 0/2] usb: misc: add support for microchip,usb2244 USB-SD controller Piyush Mehta
@ 2021-11-22  6:28 ` Piyush Mehta
  2021-11-30  2:11   ` Rob Herring
  2021-11-22  6:28 ` [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller Piyush Mehta
  1 sibling, 1 reply; 10+ messages in thread
From: Piyush Mehta @ 2021-11-22  6:28 UTC (permalink / raw)
  To: gregkh, robh+dt, mka, ravisadineni, stern, alcooperx, michal.simek
  Cc: piyush.mehta, linux-usb, devicetree, linux-kernel, git, sgoud

Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
provides an ultra-fast interface between a USB host controller and flash
media cards.

Add dt-bindings documentation for Microchip's usb2244 Controller.
USB224x is a USB 2.0 compliant ultra fast USB 2.0 multi-format,
SD/MMC, and MS Flash Media Controllers.

Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
---
Changes for V2:
- Update reset polarity, make reset ACTIVE LOW in the dt-binding document.
- Added usbsd node under the usb controller (usb0) node.
- Remove Warning: decoded text below may be mangled, UTF-8 assumed.

Review comments:
Link: https://lore.kernel.org/lkml/CAL_JsqKu6vr3iCz1G7MtK6gyqAvn4s4mpuLOwPzJDEmyZeROig@mail.gmail.com/
---
 .../devicetree/bindings/usb/microchip,usb2244.yaml | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb2244.yaml

diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
new file mode 100644
index 0000000..1854313
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/usb/microchip,usb2244.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Bindings for the Microchip USB2244 Ultra Fast USB-SD Controller
+
+description:
+  Microchip USB224x is a USB 2.0 compliant, Hi-Speed bulk only mass
+  storage class peripheral controller intended for reading and writing
+  to popular flash media from the xDPicture Card, Memory Stick (MS),
+  Secure Digital (SD), and MultiMediaCard (MMC) families.
+
+  USB224x is a flash media card reader solution fully compliant with the
+  USB 2.0 specification.
+
+maintainers:
+  - Piyush Mehta <piyush.mehta@xilinx.com>
+
+properties:
+  compatible:
+    const: microchip,usb2244
+
+  reset-gpios:
+    maxItems: 1
+    description:
+      The phandle and specifier for the GPIO that controls the RESET line of
+      flash media controller.
+
+required:
+  - compatible
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    usb0 {
+        usbsd {
+            compatible = "microchip,usb2244";
+            reset-gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
+        };
+    };
+
+...
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller
  2021-11-22  6:28 [PATCH V2 0/2] usb: misc: add support for microchip,usb2244 USB-SD controller Piyush Mehta
  2021-11-22  6:28 ` [PATCH V2 1/2] dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller Piyush Mehta
@ 2021-11-22  6:28 ` Piyush Mehta
  2021-11-22  7:02   ` Greg KH
  1 sibling, 1 reply; 10+ messages in thread
From: Piyush Mehta @ 2021-11-22  6:28 UTC (permalink / raw)
  To: gregkh, robh+dt, mka, ravisadineni, stern, alcooperx, michal.simek
  Cc: piyush.mehta, linux-usb, devicetree, linux-kernel, git, sgoud

Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
provides an ultra-fast interface between a USB host controller and flash
media cards.

This patch adds a GPIO based usb-sd reset for USB2244 USB2 ultra fast
SD controller. This usb2244 driver trigger sd reset signal after soft
reset or core Reset. The SD needs to be resetted after completion of
phy initialization. After the toggling of gpio, controller gets out
form reset. USB2244 is a simple platform device driver.

As part of the reset, sets the direction of the pin to output before
toggling the pin. Delay of microseconds is added in between high and
low to meet the setup and hold time requirement of the reset.

Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
---
Changes for V2:
- Update reset polarity, make reset ACTIVE LOW in the usb2244 driver.
- Fix WARNING: msleep < 20ms can sleep for up to 20ms by changing msleep to
  usleep_range()
---
 drivers/usb/misc/Kconfig   | 10 +++++++
 drivers/usb/misc/Makefile  |  1 +
 drivers/usb/misc/usb2244.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 drivers/usb/misc/usb2244.c

diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 8f11443..e1c66a2 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -215,6 +215,16 @@ config USB_ISIGHTFW
 	  driver beforehand. Tools for doing so are available at
 	  http://bersace03.free.fr
 
+config USB_USB2244
+	tristate "Microchip USB2244 Ultra Fast USB 2.0 SD driver"
+	depends on GPIOLIB
+	help
+	  Say Y or M here if you want to reset Microchip USB2244 Ultra Fast
+	  USB 2.0 SD controller.
+	  This option enables support for Microchip USB2244 Ultra Fast USB 2.0
+	  SD controller. This driver reset the gpio pin makes controller out of
+	  reset.
+
 config USB_YUREX
 	tristate "USB YUREX driver support"
 	help
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 5f4e598..5b4af7d 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
 obj-$(CONFIG_USB_TEST)			+= usbtest.o
 obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
 obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
+obj-$(CONFIG_USB_USB2244)		+= usb2244.o
 obj-$(CONFIG_USB_USS720)		+= uss720.o
 obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
 obj-$(CONFIG_USB_YUREX)			+= yurex.o
diff --git a/drivers/usb/misc/usb2244.c b/drivers/usb/misc/usb2244.c
new file mode 100644
index 0000000..5a868c2
--- /dev/null
+++ b/drivers/usb/misc/usb2244.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Microchip USB2244 Ultra Fast USB 2.0 Multi-Format,
+ * SD/MMC, and MS Flash Media Controllers
+ *
+ * Copyright (c) 2021 Xilinx, Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+
+struct usb2244 {
+	struct gpio_desc *reset_gpio;
+};
+
+static int usb2244_init_hw(struct device *dev, struct usb2244 *data)
+{
+	data = devm_kzalloc(dev, sizeof(struct usb2244), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(data->reset_gpio)) {
+		dev_err_probe(dev, PTR_ERR(data->reset_gpio),
+			      "Failed to request reset GPIO %ld, errcode",
+			      PTR_ERR(data->reset_gpio));
+		return PTR_ERR(data->reset_gpio);
+	}
+
+	/* Toggle RESET_N to reset the hub. */
+	gpiod_set_value_cansleep(data->reset_gpio, 1);
+	usleep_range(5, 10);
+	gpiod_set_value_cansleep(data->reset_gpio, 0);
+	usleep_range(5000, 6000);
+
+	return 0;
+}
+
+static int usb2244_probe(struct platform_device *pdev)
+{
+	struct usb2244 *data = NULL;
+
+	/* Trigger gpio reset to the hub. */
+	return usb2244_init_hw(&pdev->dev, data);
+}
+
+static const struct of_device_id usb2244_of_match[] = {
+	{ .compatible = "microchip,usb2244", },
+	{ }
+};
+
+static struct platform_driver usb2244_driver = {
+	.driver = {
+		.name = "microchip,usb2244",
+		.of_match_table	= usb2244_of_match,
+	},
+	.probe = usb2244_probe,
+};
+
+module_platform_driver(usb2244_driver);
+
+MODULE_AUTHOR("Piyush Mehta <piyush.mehta@xilinx.com>");
+MODULE_DESCRIPTION("USB2244 Ultra Fast SD-Controller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller
  2021-11-22  6:28 ` [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller Piyush Mehta
@ 2021-11-22  7:02   ` Greg KH
  2021-11-22 11:02     ` Michal Simek
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2021-11-22  7:02 UTC (permalink / raw)
  To: Piyush Mehta
  Cc: robh+dt, mka, ravisadineni, stern, alcooperx, michal.simek,
	linux-usb, devicetree, linux-kernel, git, sgoud

On Mon, Nov 22, 2021 at 11:58:34AM +0530, Piyush Mehta wrote:
> Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
> provides an ultra-fast interface between a USB host controller and flash
> media cards.
> 
> This patch adds a GPIO based usb-sd reset for USB2244 USB2 ultra fast
> SD controller. This usb2244 driver trigger sd reset signal after soft
> reset or core Reset. The SD needs to be resetted after completion of
> phy initialization. After the toggling of gpio, controller gets out
> form reset. USB2244 is a simple platform device driver.
> 
> As part of the reset, sets the direction of the pin to output before
> toggling the pin. Delay of microseconds is added in between high and
> low to meet the setup and hold time requirement of the reset.
> 
> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> ---
> Changes for V2:
> - Update reset polarity, make reset ACTIVE LOW in the usb2244 driver.
> - Fix WARNING: msleep < 20ms can sleep for up to 20ms by changing msleep to
>   usleep_range()
> ---
>  drivers/usb/misc/Kconfig   | 10 +++++++
>  drivers/usb/misc/Makefile  |  1 +
>  drivers/usb/misc/usb2244.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++

This isn't really a USB driver, so maybe drivers/misc/ instead?

>  3 files changed, 80 insertions(+)
>  create mode 100644 drivers/usb/misc/usb2244.c
> 
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 8f11443..e1c66a2 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -215,6 +215,16 @@ config USB_ISIGHTFW
>  	  driver beforehand. Tools for doing so are available at
>  	  http://bersace03.free.fr
>  
> +config USB_USB2244
> +	tristate "Microchip USB2244 Ultra Fast USB 2.0 SD driver"
> +	depends on GPIOLIB
> +	help
> +	  Say Y or M here if you want to reset Microchip USB2244 Ultra Fast
> +	  USB 2.0 SD controller.
> +	  This option enables support for Microchip USB2244 Ultra Fast USB 2.0
> +	  SD controller. This driver reset the gpio pin makes controller out of
> +	  reset.

Module name?

> +
>  config USB_YUREX
>  	tristate "USB YUREX driver support"
>  	help
> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> index 5f4e598..5b4af7d 100644
> --- a/drivers/usb/misc/Makefile
> +++ b/drivers/usb/misc/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
>  obj-$(CONFIG_USB_TEST)			+= usbtest.o
>  obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
>  obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
> +obj-$(CONFIG_USB_USB2244)		+= usb2244.o
>  obj-$(CONFIG_USB_USS720)		+= uss720.o
>  obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
>  obj-$(CONFIG_USB_YUREX)			+= yurex.o
> diff --git a/drivers/usb/misc/usb2244.c b/drivers/usb/misc/usb2244.c
> new file mode 100644
> index 0000000..5a868c2
> --- /dev/null
> +++ b/drivers/usb/misc/usb2244.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the Microchip USB2244 Ultra Fast USB 2.0 Multi-Format,
> + * SD/MMC, and MS Flash Media Controllers
> + *
> + * Copyright (c) 2021 Xilinx, Inc.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/platform_device.h>
> +
> +struct usb2244 {
> +	struct gpio_desc *reset_gpio;
> +};

Why is this structure needed?

> +
> +static int usb2244_init_hw(struct device *dev, struct usb2244 *data)
> +{
> +	data = devm_kzalloc(dev, sizeof(struct usb2244), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(data->reset_gpio)) {
> +		dev_err_probe(dev, PTR_ERR(data->reset_gpio),
> +			      "Failed to request reset GPIO %ld, errcode",
> +			      PTR_ERR(data->reset_gpio));
> +		return PTR_ERR(data->reset_gpio);
> +	}
> +
> +	/* Toggle RESET_N to reset the hub. */
> +	gpiod_set_value_cansleep(data->reset_gpio, 1);
> +	usleep_range(5, 10);
> +	gpiod_set_value_cansleep(data->reset_gpio, 0);
> +	usleep_range(5000, 6000);

Why do you need a kernel driver for this at all?  Why not just toggle
the pin from userspace?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller
  2021-11-22  7:02   ` Greg KH
@ 2021-11-22 11:02     ` Michal Simek
  2021-11-22 11:07       ` Greg KH
  2021-11-30  2:14       ` Rob Herring
  0 siblings, 2 replies; 10+ messages in thread
From: Michal Simek @ 2021-11-22 11:02 UTC (permalink / raw)
  To: Greg KH, Piyush Mehta
  Cc: robh+dt, mka, ravisadineni, stern, alcooperx, michal.simek,
	linux-usb, devicetree, linux-kernel, git, sgoud


nit: Just spot typo in subject. It is usb2244.

On 11/22/21 08:02, Greg KH wrote:
> On Mon, Nov 22, 2021 at 11:58:34AM +0530, Piyush Mehta wrote:
>> Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
>> provides an ultra-fast interface between a USB host controller and flash
>> media cards.
>>
>> This patch adds a GPIO based usb-sd reset for USB2244 USB2 ultra fast
>> SD controller. This usb2244 driver trigger sd reset signal after soft
>> reset or core Reset. The SD needs to be resetted after completion of
>> phy initialization. After the toggling of gpio, controller gets out
>> form reset. USB2244 is a simple platform device driver.
>>
>> As part of the reset, sets the direction of the pin to output before
>> toggling the pin. Delay of microseconds is added in between high and
>> low to meet the setup and hold time requirement of the reset.
>>
>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
>> ---
>> Changes for V2:
>> - Update reset polarity, make reset ACTIVE LOW in the usb2244 driver.
>> - Fix WARNING: msleep < 20ms can sleep for up to 20ms by changing msleep to
>>    usleep_range()
>> ---
>>   drivers/usb/misc/Kconfig   | 10 +++++++
>>   drivers/usb/misc/Makefile  |  1 +
>>   drivers/usb/misc/usb2244.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> 
> This isn't really a USB driver, so maybe drivers/misc/ instead?
> 
>>   3 files changed, 80 insertions(+)
>>   create mode 100644 drivers/usb/misc/usb2244.c
>>
>> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
>> index 8f11443..e1c66a2 100644
>> --- a/drivers/usb/misc/Kconfig
>> +++ b/drivers/usb/misc/Kconfig
>> @@ -215,6 +215,16 @@ config USB_ISIGHTFW
>>   	  driver beforehand. Tools for doing so are available at
>>   	  http://bersace03.free.fr
>>   
>> +config USB_USB2244
>> +	tristate "Microchip USB2244 Ultra Fast USB 2.0 SD driver"
>> +	depends on GPIOLIB
>> +	help
>> +	  Say Y or M here if you want to reset Microchip USB2244 Ultra Fast
>> +	  USB 2.0 SD controller.
>> +	  This option enables support for Microchip USB2244 Ultra Fast USB 2.0
>> +	  SD controller. This driver reset the gpio pin makes controller out of
>> +	  reset.
> 
> Module name?
> 
>> +
>>   config USB_YUREX
>>   	tristate "USB YUREX driver support"
>>   	help
>> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
>> index 5f4e598..5b4af7d 100644
>> --- a/drivers/usb/misc/Makefile
>> +++ b/drivers/usb/misc/Makefile
>> @@ -21,6 +21,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
>>   obj-$(CONFIG_USB_TEST)			+= usbtest.o
>>   obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
>>   obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
>> +obj-$(CONFIG_USB_USB2244)		+= usb2244.o
>>   obj-$(CONFIG_USB_USS720)		+= uss720.o
>>   obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
>>   obj-$(CONFIG_USB_YUREX)			+= yurex.o
>> diff --git a/drivers/usb/misc/usb2244.c b/drivers/usb/misc/usb2244.c
>> new file mode 100644
>> index 0000000..5a868c2
>> --- /dev/null
>> +++ b/drivers/usb/misc/usb2244.c
>> @@ -0,0 +1,69 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for the Microchip USB2244 Ultra Fast USB 2.0 Multi-Format,
>> + * SD/MMC, and MS Flash Media Controllers
>> + *
>> + * Copyright (c) 2021 Xilinx, Inc.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/platform_device.h>
>> +
>> +struct usb2244 {
>> +	struct gpio_desc *reset_gpio;
>> +};
> 
> Why is this structure needed?
> 
>> +
>> +static int usb2244_init_hw(struct device *dev, struct usb2244 *data)
>> +{
>> +	data = devm_kzalloc(dev, sizeof(struct usb2244), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>> +	if (IS_ERR(data->reset_gpio)) {
>> +		dev_err_probe(dev, PTR_ERR(data->reset_gpio),
>> +			      "Failed to request reset GPIO %ld, errcode",
>> +			      PTR_ERR(data->reset_gpio));
>> +		return PTR_ERR(data->reset_gpio);
>> +	}
>> +
>> +	/* Toggle RESET_N to reset the hub. */
>> +	gpiod_set_value_cansleep(data->reset_gpio, 1);
>> +	usleep_range(5, 10);
>> +	gpiod_set_value_cansleep(data->reset_gpio, 0);
>> +	usleep_range(5000, 6000);
> 
> Why do you need a kernel driver for this at all?  Why not just toggle
> the pin from userspace?

It is usb-sd convertor. If you have rootfs on SD you need to get the 
chip out of reset to be able to access init. There is no way how to do 
it via userspace.
Maybe there could be a different way how to do it via different driver 
to toggle the reset. It is dwc3 -> usb-hub(usb5744) -> usb-sd(usb2244) 
+ usb phys for sd2.0 and it should be also reset in the right order.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller
  2021-11-22 11:02     ` Michal Simek
@ 2021-11-22 11:07       ` Greg KH
  2021-11-30  2:14       ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Greg KH @ 2021-11-22 11:07 UTC (permalink / raw)
  To: Michal Simek
  Cc: Piyush Mehta, robh+dt, mka, ravisadineni, stern, alcooperx,
	linux-usb, devicetree, linux-kernel, git, sgoud

On Mon, Nov 22, 2021 at 12:02:52PM +0100, Michal Simek wrote:
> 
> nit: Just spot typo in subject. It is usb2244.
> 
> On 11/22/21 08:02, Greg KH wrote:
> > On Mon, Nov 22, 2021 at 11:58:34AM +0530, Piyush Mehta wrote:
> > > Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
> > > provides an ultra-fast interface between a USB host controller and flash
> > > media cards.
> > > 
> > > This patch adds a GPIO based usb-sd reset for USB2244 USB2 ultra fast
> > > SD controller. This usb2244 driver trigger sd reset signal after soft
> > > reset or core Reset. The SD needs to be resetted after completion of
> > > phy initialization. After the toggling of gpio, controller gets out
> > > form reset. USB2244 is a simple platform device driver.
> > > 
> > > As part of the reset, sets the direction of the pin to output before
> > > toggling the pin. Delay of microseconds is added in between high and
> > > low to meet the setup and hold time requirement of the reset.
> > > 
> > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> > > ---
> > > Changes for V2:
> > > - Update reset polarity, make reset ACTIVE LOW in the usb2244 driver.
> > > - Fix WARNING: msleep < 20ms can sleep for up to 20ms by changing msleep to
> > >    usleep_range()
> > > ---
> > >   drivers/usb/misc/Kconfig   | 10 +++++++
> > >   drivers/usb/misc/Makefile  |  1 +
> > >   drivers/usb/misc/usb2244.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> > 
> > This isn't really a USB driver, so maybe drivers/misc/ instead?
> > 
> > >   3 files changed, 80 insertions(+)
> > >   create mode 100644 drivers/usb/misc/usb2244.c
> > > 
> > > diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> > > index 8f11443..e1c66a2 100644
> > > --- a/drivers/usb/misc/Kconfig
> > > +++ b/drivers/usb/misc/Kconfig
> > > @@ -215,6 +215,16 @@ config USB_ISIGHTFW
> > >   	  driver beforehand. Tools for doing so are available at
> > >   	  http://bersace03.free.fr
> > > +config USB_USB2244
> > > +	tristate "Microchip USB2244 Ultra Fast USB 2.0 SD driver"
> > > +	depends on GPIOLIB
> > > +	help
> > > +	  Say Y or M here if you want to reset Microchip USB2244 Ultra Fast
> > > +	  USB 2.0 SD controller.
> > > +	  This option enables support for Microchip USB2244 Ultra Fast USB 2.0
> > > +	  SD controller. This driver reset the gpio pin makes controller out of
> > > +	  reset.
> > 
> > Module name?
> > 
> > > +
> > >   config USB_YUREX
> > >   	tristate "USB YUREX driver support"
> > >   	help
> > > diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> > > index 5f4e598..5b4af7d 100644
> > > --- a/drivers/usb/misc/Makefile
> > > +++ b/drivers/usb/misc/Makefile
> > > @@ -21,6 +21,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
> > >   obj-$(CONFIG_USB_TEST)			+= usbtest.o
> > >   obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
> > >   obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
> > > +obj-$(CONFIG_USB_USB2244)		+= usb2244.o
> > >   obj-$(CONFIG_USB_USS720)		+= uss720.o
> > >   obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
> > >   obj-$(CONFIG_USB_YUREX)			+= yurex.o
> > > diff --git a/drivers/usb/misc/usb2244.c b/drivers/usb/misc/usb2244.c
> > > new file mode 100644
> > > index 0000000..5a868c2
> > > --- /dev/null
> > > +++ b/drivers/usb/misc/usb2244.c
> > > @@ -0,0 +1,69 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Driver for the Microchip USB2244 Ultra Fast USB 2.0 Multi-Format,
> > > + * SD/MMC, and MS Flash Media Controllers
> > > + *
> > > + * Copyright (c) 2021 Xilinx, Inc.
> > > + */
> > > +
> > > +#include <linux/delay.h>
> > > +#include <linux/err.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +struct usb2244 {
> > > +	struct gpio_desc *reset_gpio;
> > > +};
> > 
> > Why is this structure needed?
> > 
> > > +
> > > +static int usb2244_init_hw(struct device *dev, struct usb2244 *data)
> > > +{
> > > +	data = devm_kzalloc(dev, sizeof(struct usb2244), GFP_KERNEL);
> > > +	if (!data)
> > > +		return -ENOMEM;
> > > +
> > > +	data->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > +	if (IS_ERR(data->reset_gpio)) {
> > > +		dev_err_probe(dev, PTR_ERR(data->reset_gpio),
> > > +			      "Failed to request reset GPIO %ld, errcode",
> > > +			      PTR_ERR(data->reset_gpio));
> > > +		return PTR_ERR(data->reset_gpio);
> > > +	}
> > > +
> > > +	/* Toggle RESET_N to reset the hub. */
> > > +	gpiod_set_value_cansleep(data->reset_gpio, 1);
> > > +	usleep_range(5, 10);
> > > +	gpiod_set_value_cansleep(data->reset_gpio, 0);
> > > +	usleep_range(5000, 6000);
> > 
> > Why do you need a kernel driver for this at all?  Why not just toggle
> > the pin from userspace?
> 
> It is usb-sd convertor. If you have rootfs on SD you need to get the chip
> out of reset to be able to access init. There is no way how to do it via
> userspace.

Ah, ok, that makes more sense.

So it belongs in drivers/misc/ along with the other "enable some USB
hardware" platform drivers that we have in there today.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller
  2021-11-22  6:28 ` [PATCH V2 1/2] dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller Piyush Mehta
@ 2021-11-30  2:11   ` Rob Herring
  2021-11-30 10:23     ` Michal Simek
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2021-11-30  2:11 UTC (permalink / raw)
  To: Piyush Mehta
  Cc: gregkh, mka, ravisadineni, stern, alcooperx, michal.simek,
	linux-usb, devicetree, linux-kernel, git, sgoud

On Mon, Nov 22, 2021 at 11:58:33AM +0530, Piyush Mehta wrote:
> Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
> provides an ultra-fast interface between a USB host controller and flash
> media cards.
> 
> Add dt-bindings documentation for Microchip's usb2244 Controller.
> USB224x is a USB 2.0 compliant ultra fast USB 2.0 multi-format,
> SD/MMC, and MS Flash Media Controllers.
> 
> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> ---
> Changes for V2:
> - Update reset polarity, make reset ACTIVE LOW in the dt-binding document.
> - Added usbsd node under the usb controller (usb0) node.
> - Remove Warning: decoded text below may be mangled, UTF-8 assumed.
> 
> Review comments:
> Link: https://lore.kernel.org/lkml/CAL_JsqKu6vr3iCz1G7MtK6gyqAvn4s4mpuLOwPzJDEmyZeROig@mail.gmail.com/
> ---
>  .../devicetree/bindings/usb/microchip,usb2244.yaml | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
> new file mode 100644
> index 0000000..1854313
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/usb/microchip,usb2244.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Bindings for the Microchip USB2244 Ultra Fast USB-SD Controller
> +
> +description:
> +  Microchip USB224x is a USB 2.0 compliant, Hi-Speed bulk only mass
> +  storage class peripheral controller intended for reading and writing
> +  to popular flash media from the xDPicture Card, Memory Stick (MS),
> +  Secure Digital (SD), and MultiMediaCard (MMC) families.
> +
> +  USB224x is a flash media card reader solution fully compliant with the
> +  USB 2.0 specification.
> +
> +maintainers:
> +  - Piyush Mehta <piyush.mehta@xilinx.com>
> +
> +properties:
> +  compatible:
> +    const: microchip,usb2244
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description:
> +      The phandle and specifier for the GPIO that controls the RESET line of
> +      flash media controller.
> +
> +required:
> +  - compatible
> +  - reset-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    usb0 {
> +        usbsd {
> +            compatible = "microchip,usb2244";
> +            reset-gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
> +        };

This is not how the USB device binding works. See usb-device.yaml.

> +    };
> +
> +...
> -- 
> 2.7.4
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller
  2021-11-22 11:02     ` Michal Simek
  2021-11-22 11:07       ` Greg KH
@ 2021-11-30  2:14       ` Rob Herring
  2021-11-30 10:42         ` Michal Simek
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2021-11-30  2:14 UTC (permalink / raw)
  To: Michal Simek
  Cc: Greg KH, Piyush Mehta, mka, ravisadineni, stern, alcooperx,
	linux-usb, devicetree, linux-kernel, git, sgoud

On Mon, Nov 22, 2021 at 12:02:52PM +0100, Michal Simek wrote:
> 
> nit: Just spot typo in subject. It is usb2244.
> 
> On 11/22/21 08:02, Greg KH wrote:
> > On Mon, Nov 22, 2021 at 11:58:34AM +0530, Piyush Mehta wrote:
> > > Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
> > > provides an ultra-fast interface between a USB host controller and flash
> > > media cards.
> > > 
> > > This patch adds a GPIO based usb-sd reset for USB2244 USB2 ultra fast
> > > SD controller. This usb2244 driver trigger sd reset signal after soft
> > > reset or core Reset. The SD needs to be resetted after completion of
> > > phy initialization. After the toggling of gpio, controller gets out
> > > form reset. USB2244 is a simple platform device driver.
> > > 
> > > As part of the reset, sets the direction of the pin to output before
> > > toggling the pin. Delay of microseconds is added in between high and
> > > low to meet the setup and hold time requirement of the reset.
> > > 
> > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> > > ---
> > > Changes for V2:
> > > - Update reset polarity, make reset ACTIVE LOW in the usb2244 driver.
> > > - Fix WARNING: msleep < 20ms can sleep for up to 20ms by changing msleep to
> > >    usleep_range()
> > > ---
> > >   drivers/usb/misc/Kconfig   | 10 +++++++
> > >   drivers/usb/misc/Makefile  |  1 +
> > >   drivers/usb/misc/usb2244.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> > 
> > This isn't really a USB driver, so maybe drivers/misc/ instead?
> > 
> > >   3 files changed, 80 insertions(+)
> > >   create mode 100644 drivers/usb/misc/usb2244.c
> > > 
> > > diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> > > index 8f11443..e1c66a2 100644
> > > --- a/drivers/usb/misc/Kconfig
> > > +++ b/drivers/usb/misc/Kconfig
> > > @@ -215,6 +215,16 @@ config USB_ISIGHTFW
> > >   	  driver beforehand. Tools for doing so are available at
> > >   	  http://bersace03.free.fr
> > > +config USB_USB2244
> > > +	tristate "Microchip USB2244 Ultra Fast USB 2.0 SD driver"
> > > +	depends on GPIOLIB
> > > +	help
> > > +	  Say Y or M here if you want to reset Microchip USB2244 Ultra Fast
> > > +	  USB 2.0 SD controller.
> > > +	  This option enables support for Microchip USB2244 Ultra Fast USB 2.0
> > > +	  SD controller. This driver reset the gpio pin makes controller out of
> > > +	  reset.
> > 
> > Module name?
> > 
> > > +
> > >   config USB_YUREX
> > >   	tristate "USB YUREX driver support"
> > >   	help
> > > diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> > > index 5f4e598..5b4af7d 100644
> > > --- a/drivers/usb/misc/Makefile
> > > +++ b/drivers/usb/misc/Makefile
> > > @@ -21,6 +21,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
> > >   obj-$(CONFIG_USB_TEST)			+= usbtest.o
> > >   obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
> > >   obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
> > > +obj-$(CONFIG_USB_USB2244)		+= usb2244.o
> > >   obj-$(CONFIG_USB_USS720)		+= uss720.o
> > >   obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
> > >   obj-$(CONFIG_USB_YUREX)			+= yurex.o
> > > diff --git a/drivers/usb/misc/usb2244.c b/drivers/usb/misc/usb2244.c
> > > new file mode 100644
> > > index 0000000..5a868c2
> > > --- /dev/null
> > > +++ b/drivers/usb/misc/usb2244.c
> > > @@ -0,0 +1,69 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Driver for the Microchip USB2244 Ultra Fast USB 2.0 Multi-Format,
> > > + * SD/MMC, and MS Flash Media Controllers
> > > + *
> > > + * Copyright (c) 2021 Xilinx, Inc.
> > > + */
> > > +
> > > +#include <linux/delay.h>
> > > +#include <linux/err.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +struct usb2244 {
> > > +	struct gpio_desc *reset_gpio;
> > > +};
> > 
> > Why is this structure needed?
> > 
> > > +
> > > +static int usb2244_init_hw(struct device *dev, struct usb2244 *data)
> > > +{
> > > +	data = devm_kzalloc(dev, sizeof(struct usb2244), GFP_KERNEL);
> > > +	if (!data)
> > > +		return -ENOMEM;
> > > +
> > > +	data->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > +	if (IS_ERR(data->reset_gpio)) {
> > > +		dev_err_probe(dev, PTR_ERR(data->reset_gpio),
> > > +			      "Failed to request reset GPIO %ld, errcode",
> > > +			      PTR_ERR(data->reset_gpio));
> > > +		return PTR_ERR(data->reset_gpio);
> > > +	}
> > > +
> > > +	/* Toggle RESET_N to reset the hub. */
> > > +	gpiod_set_value_cansleep(data->reset_gpio, 1);
> > > +	usleep_range(5, 10);
> > > +	gpiod_set_value_cansleep(data->reset_gpio, 0);
> > > +	usleep_range(5000, 6000);
> > 
> > Why do you need a kernel driver for this at all?  Why not just toggle
> > the pin from userspace?
> 
> It is usb-sd convertor. If you have rootfs on SD you need to get the chip
> out of reset to be able to access init. There is no way how to do it via
> userspace.

Then by the bootloader...

> Maybe there could be a different way how to do it via different driver to
> toggle the reset. It is dwc3 -> usb-hub(usb5744) -> usb-sd(usb2244) + usb
> phys for sd2.0 and it should be also reset in the right order.

Otherwise, you need to define the whole USB hierarchy to add the GPIO 
line to the USB device. Unfortunately, getting all that to work in the 
kernel is not a solved problem. It's the same issue on all the 
'discoverable' buses.

Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 1/2] dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller
  2021-11-30  2:11   ` Rob Herring
@ 2021-11-30 10:23     ` Michal Simek
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2021-11-30 10:23 UTC (permalink / raw)
  To: Rob Herring, Piyush Mehta
  Cc: gregkh, mka, ravisadineni, stern, alcooperx, michal.simek,
	linux-usb, devicetree, linux-kernel, git, sgoud



On 11/30/21 03:11, Rob Herring wrote:
> On Mon, Nov 22, 2021 at 11:58:33AM +0530, Piyush Mehta wrote:
>> Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
>> provides an ultra-fast interface between a USB host controller and flash
>> media cards.
>>
>> Add dt-bindings documentation for Microchip's usb2244 Controller.
>> USB224x is a USB 2.0 compliant ultra fast USB 2.0 multi-format,
>> SD/MMC, and MS Flash Media Controllers.
>>
>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
>> ---
>> Changes for V2:
>> - Update reset polarity, make reset ACTIVE LOW in the dt-binding document.
>> - Added usbsd node under the usb controller (usb0) node.
>> - Remove Warning: decoded text below may be mangled, UTF-8 assumed.
>>
>> Review comments:
>> Link: https://lore.kernel.org/lkml/CAL_JsqKu6vr3iCz1G7MtK6gyqAvn4s4mpuLOwPzJDEmyZeROig@mail.gmail.com/
>> ---
>>   .../devicetree/bindings/usb/microchip,usb2244.yaml | 47 ++++++++++++++++++++++
>>   1 file changed, 47 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml b/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
>> new file mode 100644
>> index 0000000..1854313
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb2244.yaml
>> @@ -0,0 +1,47 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/usb/microchip,usb2244.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Bindings for the Microchip USB2244 Ultra Fast USB-SD Controller
>> +
>> +description:
>> +  Microchip USB224x is a USB 2.0 compliant, Hi-Speed bulk only mass
>> +  storage class peripheral controller intended for reading and writing
>> +  to popular flash media from the xDPicture Card, Memory Stick (MS),
>> +  Secure Digital (SD), and MultiMediaCard (MMC) families.
>> +
>> +  USB224x is a flash media card reader solution fully compliant with the
>> +  USB 2.0 specification.
>> +
>> +maintainers:
>> +  - Piyush Mehta <piyush.mehta@xilinx.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: microchip,usb2244
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +    description:
>> +      The phandle and specifier for the GPIO that controls the RESET line of
>> +      flash media controller.
>> +
>> +required:
>> +  - compatible
>> +  - reset-gpios
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +    usb0 {
>> +        usbsd {
>> +            compatible = "microchip,usb2244";
>> +            reset-gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
>> +        };
> 
> This is not how the USB device binding works. See usb-device.yaml.
> 

Is this fine?

device@1 {
     compatible = "usb424,2240";
     reg = <1>;
     reset-gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
};

If yes, this means that we should use usb driver instead of platform driver.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller
  2021-11-30  2:14       ` Rob Herring
@ 2021-11-30 10:42         ` Michal Simek
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2021-11-30 10:42 UTC (permalink / raw)
  To: Rob Herring, Michal Simek
  Cc: Greg KH, Piyush Mehta, mka, ravisadineni, stern, alcooperx,
	linux-usb, devicetree, linux-kernel, git, sgoud



On 11/30/21 03:14, Rob Herring wrote:
> On Mon, Nov 22, 2021 at 12:02:52PM +0100, Michal Simek wrote:
>>
>> nit: Just spot typo in subject. It is usb2244.
>>
>> On 11/22/21 08:02, Greg KH wrote:
>>> On Mon, Nov 22, 2021 at 11:58:34AM +0530, Piyush Mehta wrote:
>>>> Microchip's USB224x family of Hi-Speed USB 2.0 flash media card controllers
>>>> provides an ultra-fast interface between a USB host controller and flash
>>>> media cards.
>>>>
>>>> This patch adds a GPIO based usb-sd reset for USB2244 USB2 ultra fast
>>>> SD controller. This usb2244 driver trigger sd reset signal after soft
>>>> reset or core Reset. The SD needs to be resetted after completion of
>>>> phy initialization. After the toggling of gpio, controller gets out
>>>> form reset. USB2244 is a simple platform device driver.
>>>>
>>>> As part of the reset, sets the direction of the pin to output before
>>>> toggling the pin. Delay of microseconds is added in between high and
>>>> low to meet the setup and hold time requirement of the reset.
>>>>
>>>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
>>>> ---
>>>> Changes for V2:
>>>> - Update reset polarity, make reset ACTIVE LOW in the usb2244 driver.
>>>> - Fix WARNING: msleep < 20ms can sleep for up to 20ms by changing msleep to
>>>>     usleep_range()
>>>> ---
>>>>    drivers/usb/misc/Kconfig   | 10 +++++++
>>>>    drivers/usb/misc/Makefile  |  1 +
>>>>    drivers/usb/misc/usb2244.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
>>>
>>> This isn't really a USB driver, so maybe drivers/misc/ instead?
>>>
>>>>    3 files changed, 80 insertions(+)
>>>>    create mode 100644 drivers/usb/misc/usb2244.c
>>>>
>>>> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
>>>> index 8f11443..e1c66a2 100644
>>>> --- a/drivers/usb/misc/Kconfig
>>>> +++ b/drivers/usb/misc/Kconfig
>>>> @@ -215,6 +215,16 @@ config USB_ISIGHTFW
>>>>    	  driver beforehand. Tools for doing so are available at
>>>>    	  http://bersace03.free.fr
>>>> +config USB_USB2244
>>>> +	tristate "Microchip USB2244 Ultra Fast USB 2.0 SD driver"
>>>> +	depends on GPIOLIB
>>>> +	help
>>>> +	  Say Y or M here if you want to reset Microchip USB2244 Ultra Fast
>>>> +	  USB 2.0 SD controller.
>>>> +	  This option enables support for Microchip USB2244 Ultra Fast USB 2.0
>>>> +	  SD controller. This driver reset the gpio pin makes controller out of
>>>> +	  reset.
>>>
>>> Module name?
>>>
>>>> +
>>>>    config USB_YUREX
>>>>    	tristate "USB YUREX driver support"
>>>>    	help
>>>> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
>>>> index 5f4e598..5b4af7d 100644
>>>> --- a/drivers/usb/misc/Makefile
>>>> +++ b/drivers/usb/misc/Makefile
>>>> @@ -21,6 +21,7 @@ obj-$(CONFIG_USB_LEGOTOWER)		+= legousbtower.o
>>>>    obj-$(CONFIG_USB_TEST)			+= usbtest.o
>>>>    obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)    += ehset.o
>>>>    obj-$(CONFIG_USB_TRANCEVIBRATOR)	+= trancevibrator.o
>>>> +obj-$(CONFIG_USB_USB2244)		+= usb2244.o
>>>>    obj-$(CONFIG_USB_USS720)		+= uss720.o
>>>>    obj-$(CONFIG_USB_SEVSEG)		+= usbsevseg.o
>>>>    obj-$(CONFIG_USB_YUREX)			+= yurex.o
>>>> diff --git a/drivers/usb/misc/usb2244.c b/drivers/usb/misc/usb2244.c
>>>> new file mode 100644
>>>> index 0000000..5a868c2
>>>> --- /dev/null
>>>> +++ b/drivers/usb/misc/usb2244.c
>>>> @@ -0,0 +1,69 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Driver for the Microchip USB2244 Ultra Fast USB 2.0 Multi-Format,
>>>> + * SD/MMC, and MS Flash Media Controllers
>>>> + *
>>>> + * Copyright (c) 2021 Xilinx, Inc.
>>>> + */
>>>> +
>>>> +#include <linux/delay.h>
>>>> +#include <linux/err.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_platform.h>
>>>> +#include <linux/gpio/consumer.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> +struct usb2244 {
>>>> +	struct gpio_desc *reset_gpio;
>>>> +};
>>>
>>> Why is this structure needed?
>>>
>>>> +
>>>> +static int usb2244_init_hw(struct device *dev, struct usb2244 *data)
>>>> +{
>>>> +	data = devm_kzalloc(dev, sizeof(struct usb2244), GFP_KERNEL);
>>>> +	if (!data)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	data->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>>>> +	if (IS_ERR(data->reset_gpio)) {
>>>> +		dev_err_probe(dev, PTR_ERR(data->reset_gpio),
>>>> +			      "Failed to request reset GPIO %ld, errcode",
>>>> +			      PTR_ERR(data->reset_gpio));
>>>> +		return PTR_ERR(data->reset_gpio);
>>>> +	}
>>>> +
>>>> +	/* Toggle RESET_N to reset the hub. */
>>>> +	gpiod_set_value_cansleep(data->reset_gpio, 1);
>>>> +	usleep_range(5, 10);
>>>> +	gpiod_set_value_cansleep(data->reset_gpio, 0);
>>>> +	usleep_range(5000, 6000);
>>>
>>> Why do you need a kernel driver for this at all?  Why not just toggle
>>> the pin from userspace?
>>
>> It is usb-sd convertor. If you have rootfs on SD you need to get the chip
>> out of reset to be able to access init. There is no way how to do it via
>> userspace.
> 
> Then by the bootloader...

Bootloader is doing it because it needs to get access there too. And it 
also need to do in our case initialization of usb5744 usb hub via i2c to 
start working. And description is taken from DT.
When you get to OS it takes the same DT and start to do the job again. 
It means reset usb core, reset usb hub, reset usb-sd bridge and reset 
usb ULPI.
I don't think it is good way to let bootloader do it and Linux will just 
rely on it. Bootloader should be decouple from OS that's why IMHO 
initialization on both sides is necessary.

> 
>> Maybe there could be a different way how to do it via different driver to
>> toggle the reset. It is dwc3 -> usb-hub(usb5744) -> usb-sd(usb2244) + usb
>> phys for sd2.0 and it should be also reset in the right order.
> 
> Otherwise, you need to define the whole USB hierarchy to add the GPIO
> line to the USB device. Unfortunately, getting all that to work in the
> kernel is not a solved problem. It's the same issue on all the
> 'discoverable' buses.

We have to define that hierarchy anyway for bootloader to do all resets.
And structure is fixed.
This is what we have defined. (I have updated that 2244 usb-sd convertor).
usb2244 is connected to usb5744 hub port1. Not sure if this is correct 
description because maybe it should be child of it.

Can you please check this structure?


usb0: usb0@ff9d0000 {
	#address-cells = <2>;
	#size-cells = <2>;
	compatible = "xlnx,zynqmp-dwc3";
	reg = <0x0 0xff9d0000 0x0 0x100>;
	clock-names = "bus_clk", "ref_clk";
	power-domains = <&zynqmp_firmware PD_USB_0>;
	resets = <&zynqmp_reset ZYNQMP_RESET_USB0_CORERESET>,
		 <&zynqmp_reset ZYNQMP_RESET_USB0_HIBERRESET>,
		 <&zynqmp_reset ZYNQMP_RESET_USB0_APB>;
	reset-names = "usb_crst", "usb_hibrst", "usb_apbrst";
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usb0_default>;
	phy-names = "usb3-phy";
	phys = <&psgtr 2 PHY_TYPE_USB3 0 2>;
	reset-gpios = <&slg7xl45106 0 GPIO_ACTIVE_LOW>;
  	clocks = <&zynqmp_clk USB0_BUS_REF>, <&zynqmp_clk USB3_DUAL_REF>;
  	assigned-clocks = <&zynqmp_clk USB0_BUS_REF>, <&zynqmp_clk 
USB3_DUAL_REF>;
	assigned-clock-rates = <250000000>, <20000000>;
	dr-mode = "host";

	usbhub0_2_0: hub@1 {
		i2c-bus = <&usbhub_i2c0>;
		compatible = "usb424:2744";
		reg = <1>;
		reset-gpios = <&slg7xl45106 3 GPIO_ACTIVE_LOW>;
		companion-hub = <&usbhub0_3_0>;
	};

	usbhub0_3_0: hub@2 {
		i2c-bus = <&usbhub_i2c0>;
		compatible = "usb424:5744";
		reg = <2>;
		reset-gpios = <&slg7xl45106 3 GPIO_ACTIVE_LOW>;
		companion-hub = <&usbhub0_2_0>;
	};

	/* connected to port 1, Should it be reg 1? */
	usb2244: device@3 {
		compatible = "usb424:2244";
		reset-gpios = <&slg7xl45106 2 GPIO_ACTIVE_LOW>;
		reg = <3>;
	};
		dwc3_0: dwc3@fe200000 {
		compatible = "snps,dwc3";
		reg = <0x0 0xfe200000 0x0 0x40000>;
		interrupt-parent = <&gic>;
		interrupt-names = "dwc_usb3", "otg", "hiber";
		interrupts = <0 65 4>, <0 69 4>, <0 75 4>;
		...
	};
};

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-11-30 10:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22  6:28 [PATCH V2 0/2] usb: misc: add support for microchip,usb2244 USB-SD controller Piyush Mehta
2021-11-22  6:28 ` [PATCH V2 1/2] dt-bindings: usb: misc: Add binding for Microchip usb2244 Controller Piyush Mehta
2021-11-30  2:11   ` Rob Herring
2021-11-30 10:23     ` Michal Simek
2021-11-22  6:28 ` [PATCH V2 2/2] usb: misc: usb244: add support for USB2 ultra fast sd controller Piyush Mehta
2021-11-22  7:02   ` Greg KH
2021-11-22 11:02     ` Michal Simek
2021-11-22 11:07       ` Greg KH
2021-11-30  2:14       ` Rob Herring
2021-11-30 10:42         ` Michal Simek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).