Re: [PATCH 1/1] Drivers: USB: DA8xx MUSB: added DT support

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

 



Hello.

[Changed Felipe's address to the kernel.org one, so thta he can read this too. :-)]

On 02/05/2016 08:34 PM, Petr Kulhavy wrote:

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? Felipe wants the PHY specific parts moved to a PHY driver...

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
+~~~~~~~~~~~~~
+
+Required 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)...

+
+ - reg: offset and length of the usbss register sets

   Sets?! How many?! (If more than one, you better have "reg-names" as well.)

+
+ - 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...

+
+ - mentor,multipoint : Valid only in host mode. Enables addressing of USB hubs,
+     which is normally something you want and therefore should be set to "1".
+     If set to "0" only point-to-point communication is enabled, i.e. only a single
+     device can be attached.

   Same here.

+
+ - mentor,ram-bits : This 2-base logarithm value defines the RAM FIFO size of the controller.
+     The FIFO size is calculated in 32-bit words. E.g. if your controller has 4kiB of RAM FIFO
+     this value should be set to "10": 2^10 = 1024 words of 32-bits, i.e. 4096 bytes.
+     For the DA8xx controller this value should be set to 10.

   And here.

+
+
+Optional properties:
+
+ - ti,phy20-clkmux-cfg: Integer. Defines the USB 2.0 PHY reference clock source.
+     Supported values: "0" for external pin, "1" for internal PLL.

   Boolean prop looks more promising in this case.

+
+ - ti,phy20-refclock-frequency : Integer. Defines the USB 2.0 PHY reference clock input
+     frequency in Hz in case the clock is generated by the internal PLL.
+     Supported values are 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 38.4MHz, 40MHz, 48MHz

   Ugh. At least these both are something board specific indeed...

+
+
+Example:
+
+	usb20: usb@1e00000 {
+		compatible = "ti,da8xx-musb";
+		ti,hwmods = "usb_otg_hs";

   We need this on AM1808 too now?

[...]
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 4e13fe2..281d503 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
[...]
@@ -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?

[...]
@@ -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.

[...]
@@ -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...

[...]
+		/* 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/.

+		/* optional parameter reference clock frequency */
+		if (!of_property_read_u32(np, "ti,phy20-refclock-frequency",
+			&phy20_refclock_freq)) {
+			u32 cfgchip2;
+			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;
+			}
+
+			/*
+			 * Set up USB clock/mode in the CFGCHIP2 register.
+			 */
+			cfgchip2 = __raw_readl(
+				DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+			/* USB2.0 PHY reference clock is 24 MHz */

   You've just read this freq. out of DT, why this comment?

+			cfgchip2 &= ~CFGCHIP2_REFFREQ;
+			cfgchip2 |=  phy20_refclk_cfg;
+
+			__raw_writel(cfgchip2,
+				     DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+		}
+	}
+
  	pdata->platform_ops		= &da8xx_ops;

  	glue->phy = usb_phy_generic_register();
[...]

MBR, Sergei

--
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