On Sat, Sep 24, 2011 at 12:45 PM, Paul Walmsley <paul@xxxxxxxxx> wrote:
> On Fri, 23 Sep 2011, Munegowda, Keshava wrote:
>
>> On Thu, Sep 22, 2011 at 11:31 PM, Paul Walmsley <paul@xxxxxxxxx> wrote:
>> >
>> > On Thu, 22 Sep 2011, Keshava Munegowda wrote:
>> >> 4. usb_tll_hs hwmod of usbhs with the TLL base address and irq.
>> >>
>> >> Signed-off-by: Keshava Munegowda <keshava_mgowda@xxxxxx>
>> >> Reviewed-by: Partha Basak <parthab@xxxxxxxxxxxx>
>> >> ---
>> >> arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 271 ++++++++++++++++++++++++++++
>> >> 1 files changed, 271 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> >> index 59fdb9f..d79f728 100644
>> >> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> >> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>
>> >> +static struct omap_hwmod_ocp_if omap34xx_f128m_cfg__usb_host_hs = {
>> >> + .clk = "usbhost_120m_fck",
>> >
>> > This doesn't look right. This is an interface structure record, so it
>> > should be associated with an interface clock. Is the hardware really
>> > using the functional clock as the interface clock? Or, as seems more
>> > likely...
>>
>>
>> Agreed, how about:
>>
>> main clock: usbhost_120m_fck
>> optional f clock: usbhost_48m_fck
>
> Assuming the interface clock is enabled, which one of these clocks is
> needed for UHH register accesses to complete successfully?
it is usbhost_48m_fck ;
so,
main clock: usbhost_48m_fck
optional clock : usbhost_120m_fck
do you agree?
>
>> interface clock: usbhost_ick.
>>
>>
>>
>> >> + .user = OCP_USER_MPU,
>> >> + .flags = OCPIF_SWSUP_IDLE,
>> >
>> > ... is this just a hack? OCPIF_SWSUP_IDLE is intended to work around
>> > hardware autoidle bugs only. Are you sure this shouldn't be defined as an
>> > optional clock instead?
>>
>> yes ! it was causeing resets, hence you this flag.
>
> usbhost_120m_fck is not an interface clock. That is probably what was
> causing the problem you describe.
yes , you are right ! its not an interface clock.
>
>> >> +static struct omap_hwmod omap34xx_usb_host_hs_hwmod = {
>> >> + .name = "usb_host_hs",
>> >> + .class = &omap34xx_usb_host_hs_hwmod_class,
>> >> + .main_clk = "usbhost_ick",
>> >
>> > Is this really the main clock? The main clock is the clock that drives
>> > the register logic in the IP block. Looks to me, based on the integration
>> > document in the 34xx TRM vZR, that this module has a functional clock. In
>> > general, the only time that the main_clk should be an interface clock is
>> > when the clock is a combined interface and functional clock. The mailbox
>> > IP block is a classic example.
>>
>> As I mentioned above; I can make
>>
>> main clock: usbhost_120m_fck
>> optional f clock: usbhost_48m_fck
>> interface clock: usbhost_ick.
>>
>> do you agree?
>
> The interface clock should definitely be usbhost_ick, no doubt about it.
>
> But, as I mentioned above, to determine which functional clock should be
> the main clock, you first need to know which of those two clocks need to
> be enabled for register accesses to that module to succeed.
>
> I did this work a few years ago, by trial and error since it was largely
> undocumented. It was needed since the OMAP3 clk_enable() code waits for
> the PRCM to request that the IP block exit idle before returning.
> You might want to take a look at arch/arm/mach-omap2/clock3xxx_data.c.
>
>> >> +static struct omap_hwmod omap34xx_usb_tll_hs_hwmod = {
>> >> + .name = "usb_tll_hs",
>> >> + .class = &omap34xx_usb_tll_hs_hwmod_class,
>> >> + .mpu_irqs = omap34xx_usb_tll_hs_irqs,
>> >> + .main_clk = "usbtll_ick",
>> >
>> > Is this really the main clock? The main clock is the clock that drives
>> > the register logic in the IP block. Looks to me, based on the integration
>> > document in the 34xx TRM vZR, that this module has a functional clock.
>>
>> I can make
>>
>> main clock: usbtll_fck
>> interface clock: usbtll_ick.
>>
>> do you agree?
>
> Doesn't that make more sense?
yes, I will do this change.
keshava
--
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
