Re: [RFC] AM35xx glue code for M-USB driver

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

 



Hi,

Rolf Peukert <rolf.peukert@xxxxxxx> writes:
> Hi Felipe,
>
> On 07.10.2015 18:22, Felipe Balbi wrote:
> [...]
>>>>> 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.
>>>
>>> I think so. I'll double-check that.
>> 
>> cool, thanks.
>
> The ID pin was not connected, it's tied to ground now, but still the
> same behaviour. But I can keep it from entering the musb_try_idle
> function by deactivating the vbus timeout. This can be done from
> userspace, so we don't need the additional if-statements in the code.

okay, eventually we want things to work without relying on userspace
though. For now, it should be enough to do what you're doing so we focus
on other issues.

>>> [...]
>>>>> +	/* 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.
>
> OK, if everything is declared in the device tree, we don't need to set
> default values here.

right.

> Regarding the four am35x_... functions from omap_phy_internal.c, it's
> not easy to move them over to am35x.c.
> While they just set a few bits in some system control module registers,
> they call functions from control.c to access the SCM. The control.c
> functions are not exported either (and use some static variables and
> also local include files from mach-omap2).

yeah, there's still quite a bit left to clean up for those devices and,
unfortunately, I don't even have them available so I can't help much :-s

>>> [...]
>>>>> +	phy_clk = clk_get(&pdev->dev, "hsotgusb_fck");
>>>>
>>>> why did you change the clock name ? That shouldn't be necessary.
>>>
>>> I did get "failed to get PHY clock" and "failed to get clock"
>>> messages.
>> 
>> right, this a bug elsewhere. Drivers shouldn't care about the exact
>> clock name ;-)
>> 
>
> The corresponding clock declaration seems to be in
> drivers/clk/ti/clk-3xxx.c:
>
> 	DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
> 	DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
>
> When I add two more lines there,
>
> 	DT_CLK("5c040000.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
> 	DT_CLK("5c040000.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
>
> the "ick" and "fck" clocks are found. It doesn't work without the
> address in the device name, and not with just "musb-am35x" either.

right, you need to pass the correct device name. This alone is a bug fix
worth sending. But send this two-line change as a single patch, a single
bug fix with nothing else in it.

> Still, all other device names in that file look simpler, and I'm
> wondering if I forgot a proper name declaration somewhere else?

no, you're correct. Those other devices are likely broken on DT boots
too, dunno for sure.

-- 
balbi

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux