Hi, (please sure you also Cc linux-...@vger.kernel.org for MUSB patches)
Rolf Peukert <rolf.peuk...@imms.de> writes:
> Hi,
>
> we're running a 4.x kernel on a board with an AM3505 CPU and would like
> to use device trees. The AM35xx glue code for the M-USB controller
> didn't support configuration from a DT yet. I now got it working
> somehow, but I'm not sure if I'm doing it the correct way. So this post
> is not meant as a patch submission, but more as a request for comments
> or help.
>
> In the older kernel we were using before (3.2.x), some data structures
> were set up in the respective board file and then passed to the function
> am35x_probe(). For the use with DT, I added the function
> am35x_get_config, which sets up these data structures with default
> values and then tries to read settings from the DT.
> The device .compatible declaration is now "ti,am35x-musb", so it's
> separate from "ti,omap3-musb" (as used in omap2430.c).
>
> Now the two things I'm not so sure about:
>
> a) We needed to set pointers to machine-specific functions like
> am35x_musb_clear_irq etc. These functions are implemented in
> arch/arm/mach-omap2/omap_phy_internal.c, and declared in
> arch/arm/mach-omap2/usb.h.
> As this header file can't be included easily from am35x.c, I moved the
> declarations to include/linux/platform_data/usb-omap.h. I hope that's
> OK, but would be happy about suggestions.
Yeah, those functions should not be defined under arch/arm/mach-omap2,
they need to be moved to drivers/usb/musb/am35x.c. That PHY power stuff
also needs to move to some system controller driver of some sort.
> b) The M-USB port on our board is wired as host only. If a device is
> unplugged, the driver normally goes into some idle state and waits for
> an ID signal change. But on our board that never happens.
did you route ID pin anywhere ? I hope you tied it to ground.
> So I added two checks for MUSB_OTG mode to support our hardware. Then
if your HW is host-only, why are you messing around with OTG ?
> I noticed similar code was removed from this file three years ago
> (commit 032ec49f5351e9cb242b1a1c367d14415043ab95), and I don't know if
> these lines might break something on other hardware.
>
> Best regards,
> Rolf
>
> ---
> Index: linux4/drivers/usb/musb/am35x.c
> ===================================================================
> --- linux4.orig/drivers/usb/musb/am35x.c
> +++ linux4/drivers/usb/musb/am35x.c
> @@ -188,6 +188,10 @@ static void am35x_musb_try_idle(struct m
> {
> static unsigned long last_timer;
>
> + /* do not try if not in OTG mode */
> + if (musb->port_mode != MUSB_OTG)
> + return;
peripheral-only and host-only configurations are valid, you should not
bail out unless OTG.
> @@ -323,8 +327,9 @@ eoi:
> musb_writel(reg_base, USB_END_OF_INTR_REG, 0);
> }
>
> - /* Poll for ID change */
> - if (musb->xceiv->otg->state == OTG_STATE_B_IDLE)
> + /* Poll for ID change (in OTG mode only) */
> + if (musb->port_mode == MUSB_OTG &&
> + musb->xceiv->otg->state == OTG_STATE_B_IDLE)
no, this is wrong too.
> @@ -458,6 +463,70 @@ static const struct platform_device_info
> .dma_mask = DMA_BIT_MASK(32),
> };
>
> +static struct musb_hdrc_platform_data *
> +am35x_get_config(struct platform_device *pdev)
> +{
> + struct musb_hdrc_platform_data *pdata;
> + struct omap_musb_board_data *bdata;
> + struct musb_hdrc_config *config;
> + struct device_node *np;
> + int val, ret;
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + goto err_out;
> +
> + bdata = devm_kzalloc(&pdev->dev, sizeof(*bdata), GFP_KERNEL);
> + if (!bdata)
> + goto err_pdata;
> +
> + config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
> + if (!config)
> + goto err_bdata;
> +
> + /* Set defaults */
> + config->num_eps = 16;
> + config->ram_bits = 12;
> + config->multipoint = true;
all of these are board-specific.
> +
> + bdata->interface_type = MUSB_INTERFACE_UTMI;
> + bdata->mode = MUSB_OTG;
> + bdata->power = 500;
> + bdata->extvbus = 1;
so are these.
> + bdata->set_phy_power = am35x_musb_phy_power;
> + bdata->clear_irq = am35x_musb_clear_irq;
> + bdata->set_mode = am35x_set_mode;
> + bdata->reset = am35x_musb_reset;
> +
> + pdata->mode = MUSB_OTG;
> + pdata->power = 250;
also mode and power.
> + pdata->board_data = bdata;
> + pdata->config = config;
> +
> + /* Read settings from device tree */
> + np = pdev->dev.of_node;
> + if (np) {
> + of_property_read_u32(np, "mode", (u32 *)&pdata->mode);
> + of_property_read_u32(np, "interface-type",
> + (u32 *)&bdata->interface_type);
> + of_property_read_u32(np, "num-eps", (u32 *)&config->num_eps);
> + of_property_read_u32(np, "ram-bits", (u32 *)&config->ram_bits);
> + of_property_read_u32(np, "power", (u32 *)&pdata->power);
> +
> + ret = of_property_read_u32(np, "multipoint", &val);
> + if (!ret && val)
> + config->multipoint = true;
> + }
> + return pdata;
> +
> +err_bdata:
> + kfree(bdata);
> +err_pdata:
> + kfree(pdata);
> +err_out:
> + return NULL;
> +}
> +
> static int am35x_probe(struct platform_device *pdev)
> {
> struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev);
> @@ -475,14 +544,20 @@ static int am35x_probe(struct platform_d
> goto err0;
> }
>
> - phy_clk = clk_get(&pdev->dev, "fck");
> + if (!pdata) {
> + pdata = am35x_get_config(pdev);
> + if (!pdata)
> + goto err1;
> + }
> +
> + phy_clk = clk_get(&pdev->dev, "hsotgusb_fck");
why did you change the clock name ? That shouldn't be necessary.
> if (IS_ERR(phy_clk)) {
> dev_err(&pdev->dev, "failed to get PHY clock\n");
> ret = PTR_ERR(phy_clk);
> goto err3;
> }
>
> - clk = clk_get(&pdev->dev, "ick");
> + clk = clk_get(&pdev->dev, "hsotgusb_ick");
nor here.
--
balbi
signature.asc
Description: PGP signature
