Hi Felipe & Sebastian,
On 11/24/2011 12:02 PM, Felipe Balbi wrote:
> From: Sebastian Andrzej Siewior<bigeasy@xxxxxxxxxxxxx>
>
> Trivial patch, no functional changes.
>
> [ balbi@xxxxxx : fixed up Documentation based on what
> we actually want to achieve ]
>
> Signed-off-by: Felipe Balbi<balbi@xxxxxx>
> ---
> Documentation/devicetree/bindings/usb/dwc3.txt | 66 ++++++++++++++++++++++++
> drivers/usb/dwc3/dwc3-omap.c | 11 +++--
> 2 files changed, 73 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt
>
> 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.
> +
> +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.
> + - 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
> +
> +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>.
> + reg =< 0x4a020000 0x10000>;
> + utmi_mode = "hardware";
> + range;
ranges;
> +
> + 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.
> + compatible = "synopsys,dwc3";
> + interrupt-parent =< &intc>;
> + interupts =< 92 4>;
typo
> + 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.
> +
> +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.
Regards,
Benoit
--
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
