Hello.
On 2/9/2016 3:23 PM, Petr Kulhavy wrote:
Adds DT support for the TI DA830 and DA850 USB driver.
Signed-off-by: Petr Kulhavy <[email protected]>
---
drivers/usb/musb/da8xx.c | 136 +++++++++++++++++++++++++++++++++++++++++++
drivers/usb/musb/musb_core.c | 24 ++++++++
drivers/usb/musb/musb_core.h | 2 +
3 files changed, 162 insertions(+)
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b03d3b8..6573600 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -1,6 +1,9 @@
/*
* Texas Instruments DA8xx/OMAP-L1x "glue layer"
*
+ * DT support
+ * Copyright (c) 2016 Petr Kulhavy, Barix AG <[email protected]>
+ *
Could you place this after MV's copyright?
* Copyright (c) 2008-2009 MontaVista Software, Inc. <[email protected]>
*
* Based on the DaVinci "glue layer" code.
@@ -36,6 +39,7 @@
#include <mach/da8xx.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/of_platform.h>
#include "musb_core.h"
@@ -134,6 +138,35 @@ static inline void phy_off(void)
__raw_writel(cfgchip2, CFGCHIP2);
}
+/* converts PHY refclk frequency in Hz into USB0REF_FREQ config value
+ * on unsupported frequency returns -1
+ */
+static inline int phy_refclk_cfg(int frequency)
+{
+ switch (frequency) {
+ case 12000000:
+ return 0x01;
There's a macro for this.
+ case 13000000:
+ return 0x06;
+ case 19200000:
+ return 0x05;
+ case 20000000:
+ return 0x08;
+ case 24000000:
+ return 0x02;
And for this.
+ case 26000000:
+ return 0x07;
+ case 38400000:
+ return 0x05;
+ case 40000000:
+ return 0x09;
+ case 48000000:
+ return 0x03;
And for this.
+ default:
+ return -1;
I suggest that you first add the macros for the ones not #define'd (in a
separate patch).
[...]
@@ -515,6 +551,90 @@ static int da8xx_probe(struct platform_device *pdev)
glue->dev = &pdev->dev;
glue->clk = clk;
+ if (IS_ENABLED(CONFIG_OF) && np) {
+ const struct of_device_id *match;
+ const struct musb_hdrc_config *matched_config;
+ struct musb_hdrc_config *config;
+ struct musb_hdrc_platform_data *data;
+ u32 phy20_refclock_freq, phy20_clkmux_cfg;
+ u32 cfgchip2;
+ unsigned power_ma;
+ int ret;
+
+ match = of_match_device(da8xx_id_table, &pdev->dev);
+ if (!match) {
+ ret = -ENODEV;
+ goto err5;
+ }
+ matched_config = match->data;
There's no dire need for initializing the DT device data yet (and there
will hardly be one I think).
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ ret = -ENOMEM;
+ goto err5;
+ }
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
Why duplicate the platform data?!
+ if (!data) {
+ ret = -ENOMEM;
+ goto err5;
+ }
+
+ config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
Why not just use the static config?!
+ if (!config) {
+ ret = -ENOMEM;
+ goto err5;
+ }
+
+ config->num_eps = matched_config->num_eps;
+ config->ram_bits = matched_config->ram_bits;
+ config->multipoint = matched_config->multipoint;
+ pdata->config = config;
+ pdata->board_data = data;
What?! Why store a pointer to self?!
+ pdata->mode = musb_get_dr_mode(&pdev->dev);
+
+ ret = of_property_read_u32(np, "mentor,power", &power_ma);
I told you this is MUSB generic prop and so should be parsed in musb_core.c.
+ if (ret)
+ power_ma = 0;
+
+ /* the "mentor,power" value is in milli-amps, whereas
milli-Ampers, no?
+ * the mentor configuration is in 2mA units
+ */
+ pdata->power = power_ma / 2;
+
+ /* optional parameter reference clock frequency
+ * true = use PLL, false = use external clock pin
+ */
+ phy20_clkmux_cfg =
+ of_property_read_bool(np, "ti,phy20-clkmux-pll") ?
+ CFGCHIP2_USB2PHYCLKMUX : 0;
No dire need for this variable...
+
+ cfgchip2 = __raw_readl(CFGCHIP2);
+ cfgchip2 &= ~CFGCHIP2_USB2PHYCLKMUX;
+ cfgchip2 |= phy20_clkmux_cfg;
+ __raw_writel(cfgchip2, CFGCHIP2);
+
+ /* optional parameter reference clock frequency */
+ if (!of_property_read_u32(np, "ti,phy20-refclock-frequency",
Actually, this one smells like mandatory prop... U-Boot doesn't program
CFGCHIP2, so REFFREQ is left 0.
+ &phy20_refclock_freq)) {
+ int phy20_refclk_cfg;
+
+ phy20_refclk_cfg = phy_refclk_cfg(phy20_refclock_freq);
+ if (phy20_refclk_cfg < 0) {
+ dev_err(&pdev->dev,
+ "invalid PHY clock frequency %u Hz\n",
+ phy20_refclock_freq);
+ ret = -EINVAL;
+ goto err5;
+ }
+
+ cfgchip2 = __raw_readl(CFGCHIP2);
+ cfgchip2 &= ~CFGCHIP2_REFFREQ;
+ cfgchip2 |= phy20_refclk_cfg;
+ __raw_writel(cfgchip2, CFGCHIP2);
+ }
+ }
+
pdata->platform_ops = &da8xx_ops;
glue->phy = usb_phy_generic_register();
@@ -582,11 +702,27 @@ static int da8xx_remove(struct platform_device *pdev)
return 0;
}
+static const struct musb_hdrc_config da830_config = {
+ .ram_bits = 10,
+ .num_eps = 5,
+ .multipoint = 1,
+
+static const struct of_device_id da8xx_id_table[] = {
+ {
+ .compatible = "ti,da830-musb",
+ .data = &da830_config,
I don't think you need to init. 'data' at this stage, keep it simple.
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_id_table);
+
[...]
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index c3791a0..284bf66 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -100,6 +100,7 @@
#include <linux/io.h>
#include <linux/dma-mapping.h>
#include <linux/usb.h>
+#include <linux/usb/of.h>
#include "musb_core.h"
@@ -129,6 +130,29 @@ static inline struct musb *dev_to_musb(struct device *dev)
return dev_get_drvdata(dev);
}
+enum musb_mode musb_get_dr_mode(struct device *dev)
I'd call it just musb_get_mode()...
+{
+ enum usb_dr_mode mode;
+
+ mode = usb_get_dr_mode(dev);
+ 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;
+ }
+}
+EXPORT_SYMBOL_GPL(musb_get_dr_mode);
+
+
+
One empty line is enough. :-)
And please add this function in a separate (preceding) patch.
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
