Re: [PATCH 13/34] usb: dwc3: omap: fix dwc3 binding match table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/24/2011 04:58 PM, Cousson, Benoit wrote:
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
new file mode 100644
index 0000000..c936863
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -0,0 +1,66 @@
+Synopsys DesignWareCores SuperSpeed USB 3.0 Controller
+
+The device node for a USB3 Wrapper that is part of an OMAP5 SoC is as described
+in the document "Open Firmware Recommended Practice: Universal Serial Bus" with
+the following modifications and additions:

You might want to separate the generic dwc3 part from the OMAP integration.

This binding should focus on the generic core only and another one should be added for the OMAP wrapper.

The binding mentions Omap specific things due to the glue layer. The
generic pieces are what remains if you take it out :) Should they come
up another ARM with this SoC or a Mips or something then it would need
a different binding due to the wrapper layer which probably turns out
different. The remaining pieces should be re-used.

If I did net get your point please try again :)

+
+Required properties:
+	- compatible : should be "ti,omap5-dwc3" or "ti,omap5430-dwc3".

In theory you should avoid having 2 compatible entries here. So far we are using the omap3- omap4- prefix only because it is enough to specify accurately the IP version.

omap5 should be enough. In case there are some workaround/bugs required
for the 5430 I have that property already in DT and need only to add it
to the driver. Should omap6 remain compatible with omap5 then there
will no change and we reuse the binding.

+	- reg : the first address and length contains the address of DRD. The
+		second address and length contains the address of the wrapper.
+	- interrupts :<   a b>   where 'a' is the interrupt number and 'b' is a
+		field that represents an encoding of the sense and level
+		information for the interrupt line.
+	- child device : The child Synopsys DesignWareCore USB 3.0 DRD Controller
+
+Optional properties:
+	- utmi_mode : UTMI mode of the wrapper Interrupts (SW or HW). Refer to
+		OMAP5 TRM for more details (for TI wrapper only)

If this is TI only and not something that can be used by other vendor, it should be prefixed with "ti,"
: ti,utmi_mode

This is wrapper only property, and it should be ti prefixed.

+
+Example of hardware mode OMAP5:
+
+	usb@4a020000 {
+		#address-cells =<   1>;
+		#size-cells =<   1>;
+		compatible = "ti,omap5430-dwc3", "ti,omap5-dwc3";
+		interrupt-parent =<   &intc>;
+		interrupts =<   93 4>;

In the latest GIC binding, there is a third cell for the GIC:

The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI
interrupts.

It should be now<0 93 4>.

okay, will update.

+		reg =<   0x4a020000 0x10000>;
+		utmi_mode = "hardware";
+		range;

ranges;

k.

+
+		usb@4a030000 {
+			#address-cells =<   3>;
+			#size-cells =<   3>;

Not needed and not correct in this case.
This is needed for bus node to specify how many cells are used for address and size in the list of entries for their children. So in that case you have 3 entries with 1 address-cell and 1 size-cell.

Anyway, the information is already retrieved from the parent node. And since there is no child, there is no need for that.

Will look into this.


+			compatible = "synopsys,dwc3";
+			interrupt-parent =<   &intc>;
+			interupts =<   92 4>;

typo

yes.


+			reg =<	0x4a030000 0x8000       // First address is xHCI
+				0x4a03c100 0x600        // Second is Global
+				0x4a03c700 0x500>;     // Third is Device

You should take advantage of the soon to come reg-names property here to be able to use the get_resource_byname API.

+		};
+	};

I'm just wondering if we should use a parent/child relationship or instead just have 2 sibling nodes?

usbss {
	usb-wrapper {
		compatible = "ti,omap5-dwc3";
	};

	usb {
		compatible = "synopsys,dwc3";
	};
}

That being said, the parent/child will avoid a third node.

Why want you to do this?

+
+Example of software mode OMAP5:
+
+	usb@4a020000 {
+		#address-cells =<   1>;
+		#size-cells =<   1>;
+		compatible = "ti,omap5430-dwc3", "ti,omap5-dwc3";
+		interrupt-parent =<   &intc>;
+		interrupts =<   93 4>;
+		reg =<   0x4a020000 0x10000 ;
+		utmi_mode = "software";
+		range;
+
+		usb@4a030000 {
+			#address-cells =<   3>;
+			#size-cells =<   3>;
+			compatible = "synopsys,dwc3";
+			interrupt-parent =<   &intc>;
+			interupts =<   92 4>;
+			reg =<	0x4a030000 0x8000       // First address is xHCI
+				0x4a03c100 0x600        // Second is Global
+				0x4a03c700 0x500>;     // Third is Device
+		};
+	};

The previous comments apply here as well.

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 7bcf677..c9f3e61 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -367,20 +367,23 @@ static int __devexit dwc3_omap_remove(struct platform_device *pdev)
   	return 0;
   }

-static const struct of_device_id of_dwc3_matach[] = {
+static const struct of_device_id of_dwc3_match[] = {
   	{
-		"ti,dwc3",
+		"ti,omap5430-dwc3"
+	},
+	{
+		"ti,omap5-dwc3"

Just use one entry here, all the other SoCs that will use the exact same wrapper will have to use the proper compatible value.
You will need several entries if the same driver is supposed to handle two different version of the IPs.

This was the plan, the 5430 entry should not be there.

Regards,
Benoit

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux