Hi,
On 09/29/10 17:38, Sergei Shtylyov wrote:
> Hello.
>
> Igor Grinberg wrote:
>
>>> AM35x has musb interface and uses CPPI4.1 DMA engine.
>>> Current patch supports only PIO mode. DMA support can be
>>> added later once basic CPPI4.1 DMA patch is accepted.
>
>>> Also added USB_MUSB_AM35X which is required to differentiate musb ips
>>> between OMAP3x and AM35x. This config would be used to for below
>>> purposes,
>>> - Select am35x.c instead of omap2430.c for compilation
>>> at drivers/usb/musb directory. Please note there are
>>> significant differneces in these two files as musb ip
>>> in quite different on AM35x.
>>> - Select workaround codes applicable for AM35x musb issues.
>>> one such workaround is for bytewise read issue on AM35x.
>
>>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@xxxxxx>
>
> [...]
>
>>> diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
>>> new file mode 100644
>>> index 0000000..ee0c104
>>> --- /dev/null
>>> +++ b/drivers/usb/musb/am35x.c
>>> @@ -0,0 +1,510 @@
> [...]
>>> +int musb_platform_set_mode(struct musb *musb, u8 musb_mode)
>>> +{
>>> + u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
>>> +
>>> + devconf2 &= ~CONF2_OTGMODE;
>>> + switch (musb_mode) {
>>> +#ifdef CONFIG_USB_MUSB_HDRC_HCD
>>> + case MUSB_HOST: /* Force VBUS valid, ID = 0 */
>>> + devconf2 |= CONF2_FORCE_HOST;
>>> + break;
>>> +#endif
>>> +#ifdef CONFIG_USB_GADGET_MUSB_HDRC
>>> + case MUSB_PERIPHERAL: /* Force VBUS valid, ID = 1 */
>>> + devconf2 |= CONF2_FORCE_DEVICE;
>>> + break;
>>> +#endif
>>> +#ifdef CONFIG_USB_MUSB_OTG
>>> + case MUSB_OTG: /* Don't override the VBUS/ID comparators */
>>> + devconf2 |= CONF2_NO_OVERRIDE;
>
>> This does nothing, can be removed...
>
> Well, I think you should let it live -- for completeness...
Well, I actually don't like, how those bits defined in first place,
but I want am35xx musb support badly, so yeah, let it be...
>
>>> +int musb_platform_exit(struct musb *musb)
>>> +{
>>> + struct clk *otg_fck;
>>> +
>>> + if (is_host_enabled(musb))
>>> + del_timer_sync(&otg_workaround);
>>> +
>>> + phy_off();
>>> +
>>> + otg_put_transceiver(musb->xceiv);
>>> + usb_nop_xceiv_unregister();
>>> +
>>> + if (musb->set_clock)
>>> + musb->set_clock(musb->clock, 0);
>>> + else
>>> + clk_disable(musb->clock);
>>> +
>>> + otg_fck = clk_get(musb->controller, "fck");
>>> + if (IS_ERR(otg_fck)) {
>>> + DBG(2, "clk_get() failed for otg_fck.\n");
>>> + } else {
>>> + clk_put(otg_fck);
>>> + clk_put(otg_fck);
>>> + clk_disable(otg_fck);
>
>> I think the order should be:
>> clk_disable(...);
>> clk_put(...);
>
> Right...
>
>> And of course, it should be put only once... ;)
>
> clk_get() is called twice, here and in musb_platform_init(), and so is clk_put().
Right, it should be stored in a static variable or a structure
(as you state in your reply), so there won't be any need for putting it twice.
>
> WBR, Sergei
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Igor.
--
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
