Re: [PATCH v2] usb: gadget: mv: Add USB 3.0 device driver for Marvell PXA2128 chip.

[Felipe Balbi <balbi@xxxxxx>]

On Mon, May 14, 2012 at 06:11:11PM +0800, Yu Xu wrote:
> Hi Balbi,
> 
> 
> >> +             dev_err(&dev->dev, "failed to allocate memory for u3d\n");
> >> +             retval = -ENOMEM;
> >> +             goto err_alloc_private;
> >> +     }
> >> +
> >> +     spin_lock_init(&u3d->lock);
> >> +
> >> +     platform_set_drvdata(dev, u3d);
> >> +
> >> +     u3d->dev = dev;
> >> +     u3d->pdata = dev->dev.platform_data;
> >
> > I believe pdata could be freed after init, no ?
> >
> Actually, pdata is still used after init, such as pdata->phy_init/phy_deinit.

k, fair enough.

> >> +     u3d->clk = clk_get(&dev->dev, pdata->clkname[0]);
> >> +     if (IS_ERR(u3d->clk)) {
> >> +             retval = PTR_ERR(u3d->clk);
> >> +             goto err_get_clk;
> >> +     }
> >> +
> >> +     r = platform_get_resource_byname(u3d->dev, IORESOURCE_MEM, "capregs");
> >> +     if (r == NULL) {
> >> +             dev_err(&dev->dev, "no I/O memory resource defined\n");
> >> +             retval = -ENODEV;
> >> +             goto err_get_cap_regs;
> >> +     }
> >> +
> >> +     u3d->cap_regs = (struct mv_u3d_cap_regs __iomem *)
> >> +             ioremap(r->start, resource_size(r));
> >> +     if (u3d->cap_regs == NULL) {
> >> +             dev_err(&dev->dev, "failed to map I/O memory\n");
> >> +             retval = -EBUSY;
> >> +             goto err_map_cap_regs;
> >> +     } else {
> >> +             dev_dbg(&dev->dev, "cap_regs address: 0x%x/0x%x\n",
> >> +                     (unsigned int)r->start, (unsigned int)u3d->cap_regs);
> >> +     }
> >> +
> >> +     u3d->phy_regs = (u32)u3d->cap_regs + USB3_PHY_OFFSET;
> >
> > PHY shouldn't be handled by this driver. You need to write a PHY driver
> > for controlling the PHY, no ?
> >
> Yes, I'll have another patch for PHY.

Then, why even add this here ?

> >> +     /* we will access controller register, so enable the u3d controller */
> >> +     clk_enable(u3d->clk);
> >> +
> >> +     if (pdata->phy_init) {
> >> +             retval = pdata->phy_init(u3d->phy_regs);
> >> +             if (retval) {
> >> +                     dev_err(&dev->dev, "init phy error %d\n", retval);
> >> +                     goto err_u3d_enable;
> >> +             }
> >> +     }
> >> +
> >> +     u3d->op_regs = (struct mv_u3d_op_regs __iomem *)((u32)u3d->cap_regs
> >> +             + USB3_OP_REGS_OFFSET);
> >> +
> >> +     u3d->vuc_regs = (struct mv_u3d_vuc_regs __iomem *)((u32)u3d->cap_regs
> >> +             + readl(&u3d->cap_regs->vuoff));
> >> +
> >> +     u3d->max_eps = 16;
> >> +
> >> +     /*
> >> +      * some platform will use usb to download image, it may not disconnect
> >> +      * usb gadget before loading kernel. So first stop u3d here.
> >> +      */
> >> +     u3d_stop(u3d);
> >
> > can't you simply softreset the IP ? Do you have a softreset bit ?
> >
> Sorry, there is no such bit to easily reset:(

too bad :-(

-- 
balbi

Attachment: signature.asc
Description: Digital signature