On 2/8/2016 12:19 PM, Petr Kulhavy wrote:
[Changed Felipe's address to the kernel.org one, so thta he can read this
too. :-)]
He should also update the MAINTAINERS file with his new address.
He has submitted a patch to do that.
TI DA8xx MUSB driver equipped with DeviceTree support.
Tested with AM1808 board and USB2.0 (OTG) in host mode.
Have you notices "depends on BROKEN" in Kconfig, BTW?
Honestly, I don't see any "depends on BROKEN". However I do build with the
3.17 kernel.
And the patch is against 3.17? You should only submit patches against the
recent kernel. In this case, against the -next branch of Felipe's repo on
kernel.org.
Felipe wants the PHY specific parts moved to a PHY driver...
I was wondering about a PHY driver too. However the AM1808 has no standalone
PHY module like e.g. the AM335x does. The PHY together with the USB core are
integrated into a single block and there is only a little control through the
SYSCFG registers.
The CFGCHIP2 register controls the PHY in practice. The code manipulating
it should be moved to a PHY driver.
And that change has nothing to do with DT support - it's a structural change
of the da8xx USB and platform drivers.
That's why I didn't go for that.
OK, just thought I'd let you know.
Signed-off-by: Petr Kulhavy <petr@xxxxxxxxx>
---
.../devicetree/bindings/usb/da8xx-usb.txt | 63 ++++++++
drivers/usb/musb/da8xx.c | 162
+++++++++++++++++++++
include/linux/platform_data/usb-davinci.h | 3 +-
3 files changed, 227 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
new file mode 100644
index 0000000..69c0961
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -0,0 +1,63 @@
+TI DA8xx MUSB
+~~~~~~~~~~~~~
+
+Rquired properties:
+~~~~~~~~~~~~~~~~~~~~
+ - compatible : Should be "ti,da8xx-musb"
No way. You'l probably need to select a lowest common denominator model.
I don't remember offhand but I don't think DA850 (AM1808) is different from
DA830 (AM170x)...
So I'd suggest "ti,da830-musb".
I just did what the guys here on the mailing list told me. I'm getting a bit
confused now what is the right approach then...
Wildcards are not allowed in the "compatible" prop. People here told you
the same.
+
+ - interrupts: USB interrupt number
+
+ - interrupt-names: should be set to "mc"
+
+ - dr_mode: The USB operation mode. Should be one of "host", "peripheral"
or "otg".
+
+ - mentor,power : Specifies the maximum current in milli-ampers the
controller can
+ supply in host mode. The maximum configurable value is 510mA.
+
+ - mentor,num-eps : Valid only in host mode. Specifies the number of
target endpoints
+ supported by the controller. For DA8xx it is "5".
That number should actually be deducible from the "compatible" prop. I'm
not sure it's a good idea to specify this in DT... although TI have done
this once already, for OMAPs...
Even twice, in omap2430.c and musb_dsps.c. omap2430.c even parses
non-prefixed props, ew...
All the other MUSBs specify these parameters in the DT. Do you want to make an
exception?
I'd prefer doing it that way, sure.
[...]
@@ -134,6 +139,35 @@ static inline void phy_off(void)
__raw_writel(cfgchip2, CFGCHIP2);
}
+/* converts PHY refclk frequency in HZ into USB0REF_FREQ config value
It's Hz, no?
Nope, see the spruh82a.pdf
Refering me to the AM18xx TRM when I asked for just correctly naming the
SI unit is strange. :-)
[...]
@@ -527,6 +561,35 @@ static const struct platform_device_info
da8xx_dev_info = {
.dma_mask = DMA_BIT_MASK(32),
};
+static int get_musb_port_mode(struct device_node *np)
+{
+ enum usb_dr_mode mode;
+
+ mode = of_usb_get_dr_mode(np);
+ switch (mode) {
+ case USB_DR_MODE_HOST:
+ return MUSB_HOST;
+
+ case USB_DR_MODE_PERIPHERAL:
+ return MUSB_PERIPHERAL;
+
+ case USB_DR_MODE_OTG:
+ return MUSB_OTG;
+
+ default:
+ return MUSB_UNDEFINED;
+ }
+}
This is clearly MUSB generic code.
So what does it mean?
Define this function in musb_core.c.
[...]
@@ -560,6 +624,95 @@ static int da8xx_probe(struct platform_device *pdev)
glue->dev = &pdev->dev;
glue->clk = clk;
+ if (IS_ENABLED(CONFIG_OF) && np) {
+ struct musb_hdrc_config *config;
+ struct musb_hdrc_platform_data *data;
+ u32 phy20_refclock_freq, phy20_clkmux_cfg;
[...]
+ pdata->mode = get_musb_port_mode(np);
+ config->num_eps = get_int_prop(np, "mentor,num-eps");
+ config->ram_bits = get_int_prop(np, "mentor,ram-bits");
+ /* the "mentor,power" value is in milli-amps, whereas
+ * the mentor configuration is in 2mA units
+ */
+ pdata->power = get_int_prop(np, "mentor,power") / 2;
+ config->multipoint = of_property_read_bool(np,
+ "mentor,multipoint");
These props are MUSB generic. Parsing them in each glue separately
doesn't make much sense...
>
What do you suggest then?
musb_core.c again.
[...]
+ /* optional parameter reference clock frequency */
+ if (!of_property_read_u32(np, "ti,phy20-clkmux-cfg",
+ &phy20_clkmux_cfg)) {
+ u32 cfgchip2;
+
+ /*
+ * Select internal reference clock for USB 2.0 PHY
+ * and use it as a clock source for USB 1.1 PHY
+ * (this is the default setting anyway).
+ */
+
+ cfgchip2 = __raw_readl(
+ DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
That's why a PHY driver is needed. DA8XX_SYSCFG2_VIRT() shouldn't be used
outside arch/arm/mach-davinci/.
See above.
Why are you not using CFGCHIP2 macro in this file as the rest of the code
does?
Regards
Petr
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
