Re: [PATCH v8 4/9] mfd: Add binding document for NVIDIA Tegra XUSB

[Jon Hunter]

On 14/05/15 08:40, Lee Jones wrote:
> On Thu, 14 May 2015, Jon Hunter wrote:
> 
>> Hi Lee,
>>
>> On 13/05/15 15:39, Lee Jones wrote:
>>> On Mon, 04 May 2015, Andrew Bresticker wrote:
>>>
>>>> Add a binding document for the XUSB host complex on NVIDIA Tegra124
>>>> and later SoCs.  The XUSB host complex includes a mailbox for
>>>> communication with the XUSB micro-controller and an xHCI host-controller.
>>>>
>>>> Signed-off-by: Andrew Bresticker <abrestic@xxxxxxxxxxxx>
>>>> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
>>>> Cc: Pawel Moll <pawel.moll@xxxxxxx>
>>>> Cc: Mark Rutland <mark.rutland@xxxxxxx>
>>>> Cc: Ian Campbell <ijc+devicetree@xxxxxxxxxxxxxx>
>>>> Cc: Kumar Gala <galak@xxxxxxxxxxxxxx>
>>>> Cc: Samuel Ortiz <sameo@xxxxxxxxxxxxxxx>
>>>> Cc: Lee Jones <lee.jones@xxxxxxxxxx>
>>>> ---
>>>> Changes from v7:
>>>>  - Move non-shared resources into child nodes.
>>>> New for v7.
>>>> ---
>>>>  .../bindings/mfd/nvidia,tegra124-xusb.txt          | 37 ++++++++++++++++++++++
>>>>  1 file changed, 37 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt b/Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt
>>>> new file mode 100644
>>>> index 0000000..bc50110
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt
>>>> @@ -0,0 +1,37 @@
>>>> +NVIDIA Tegra XUSB host copmlex
>>>> +==============================
>>>> +
>>>> +The XUSB host complex on Tegra124 and later SoCs contains an xHCI host
>>>> +controller and a mailbox for communication with the XUSB micro-controller.
>>>> +
>>>> +Required properties:
>>>> +--------------------
>>>> + - compatible: For Tegra124, must contain "nvidia,tegra124-xusb".
>>>> +   Otherwise, must contain '"nvidia,<chip>-xusb", "nvidia,tegra124-xusb"'
>>>> +   where <chip> is tegra132.
>>>> + - reg: Must contain the base and length of the XUSB FPCI registers.
>>>> + - ranges: Bus address mapping for the XUSB block.  Can be empty since the
>>>> +   mapping is 1:1.
>>>> + - #address-cells: Must be 2.
>>>> + - #size-cells: Must be 2.
>>>> +
>>>> +Example:
>>>> +--------
>>>> +	usb@0,70098000 {
>>>> +		compatible = "nvidia,tegra124-xusb";
>>>> +		reg = <0x0 0x70098000 0x0 0x1000>;
>>>> +		ranges;
>>>> +
>>>> +		#address-cells = <2>;
>>>> +		#size-cells = <2>;
>>>> +
>>>> +		usb-host@0,70090000 {
>>>> +			compatible = "nvidia,tegra124-xhci";
>>>> +			...
>>>> +		};
>>>> +
>>>> +		mailbox {
>>>> +			compatible = "nvidia,tegra124-xusb-mbox";
>>>> +			...
>>>> +		};
>>>
>>> This doesn't appear to be a proper MFD.  I would have the USB and
>>> Mailbox devices probe seperately and use a phandle to point the USB
>>> device to its Mailbox.
>>>
>>> usb@xyz {
>>> 	mboxes = <&xusb-mailbox, [chan]>;
>>> };
>>>
>>
>> I am assuming that Andrew had laid it out like this to reflect the hw
>> structure. The mailbox and xhci controller are part of the xusb
>> sub-system and hence appear as child nodes. My understanding is that for
>> device-tree we want the device-tree structure to reflect the actual hw.
>> Is this not the case?
> 
> Yes, the DT files should reflect h/w.  I have requested to see what
> the memory map looks like, so I might provide a more appropriate
> solution to accepting a pretty pointless MFD.

For the xusb-host has memory from 0x7009000 - 0x7009ffff.

Within this range, we have this fpci range which is defined as 0x7009800
- 0x70098fff. This range is being shared between the mailbox and xhci
drivers. Looking at the drivers, we have ...

mailbox uses: 0x700980e0 - 0x700980f3 and 0x70098428 - 0x7009842b.
xhci uses:    0x70098000 - 0x700980cf and 0x70098800 - 0x70098803.

So it is a bit messy as they overlap. However, we could have ...

	xusb_mbox: mailbox {
		compatible = "nvidia,tegra124-xusb-mbox";
		reg = <0x0 0x700980e0 0x0 0x14>,
		      <0x0 0x70098428 0x0 0x4>;
		...
	};
	usb-host@0,70090000 {
		compatible = "nvidia,tegra124-xhci";
		reg = <0x0 0x70090000 0x0 0x8000>,		      	
		      <0x0 0x70098000 0x0 0x00d0>;
		      <0x0 0x70098800 0x0 0x0004>;
		      <0x0 0x70099000 0x0 0x1000>;
		...
	};

I believe that Thierry and Stephen said that they wished to avoid
multiple devices sharing the same memory ranges, and so we would need to
divvy up the memory map as above. However, I am not sure if this is an
ok thing to do.
	
> Two solutions spring to mind.  You can either call
> of_platform_populate() from the USB driver, as some already do:
> 
>   drivers/usb/dwc3/dwc3-exynos.c:
>     ret = of_platform_populate(node, NULL, NULL, dev);
>   drivers/usb/dwc3/dwc3-keystone.c:
>     error = of_platform_populate(node, NULL, NULL, dev);
>   drivers/usb/dwc3/dwc3-omap.c:
>     ret = of_platform_populate(node, NULL, NULL, dev);
>   drivers/usb/dwc3/dwc3-qcom.c:
>     ret = of_platform_populate(node, NULL, NULL, qdwc->dev);
>   drivers/usb/dwc3/dwc3-st.c:
>     ret = of_platform_populate(node, NULL, NULL, dev);
>   drivers/usb/musb/musb_am335x.c:
>     ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> 
> Or use the "simple-mfd", which is currently in -next:
> 
>   git show next/master:Documentation/devicetree/bindings/mfd/mfd.txt

That is nice. Sounds like the "simple-bus" style of device but for an
mfd. Based upon the above, let me know if you think we could use the
"simple-mfd"?

Cheers
Jon

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/