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.
>> + 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.
>> + /* 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:(
>> + writel(0xFFFFFFFF, &u3d->vuc_regs->intrcause);
>> +
>> + size = u3d->max_eps * sizeof(struct ep_context) * 2;
>> + size = (size + EP_CONTEXT_ALIGNMENT - 1) & ~(EP_CONTEXT_ALIGNMENT - 1);
>> + u3d->ep_context = dma_alloc_coherent(&dev->dev, size,
>> + &u3d->ep_context_dma, GFP_KERNEL);
>> + if (u3d->ep_context == NULL) {
>> + dev_err(&dev->dev, "allocate ep context memory failed\n");
>> + retval = -ENOMEM;
>> + goto err_alloc_ep_context;
>> + }
>> + u3d->ep_context_size = size;
>> +
>> + /* create TRB dma_pool resource */
>> + u3d->trb_pool = dma_pool_create("u3d_trb",
>> + &dev->dev,
>> + sizeof(struct trb_hw),
>> + TRB_ALIGNMENT,
>
> this is too generic name. Prepend with something like MV_U3D_
>
>> + DMA_BOUNDARY);
>
> likewise.
>
>> +
>> + if (!u3d->trb_pool) {
>> + retval = -ENOMEM;
>> + goto err_alloc_trb_pool;
>> + }
>> +
>> + size = u3d->max_eps * sizeof(struct mv_ep) * 2;
>> + u3d->eps = kzalloc(size, GFP_KERNEL);
>> + if (u3d->eps == NULL) {
>
> !u3d->eps
>
> --
> balbi
Thanks for your review and comments! I'll fix them.
Thanks,
Yu Xu
--
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
