USB & hwmon on am33xx

USB & hwmon on am33xx

[Sebastian Andrzej Siewior]

Hi Paul,

I'm slowly losing my mind with hwmod.

arch/arm/boot/dts/am33xx.dtsi has the ti,musb-am33xx node.

That one has "usb_otg_hs" as hwmod property.
The entry for it arch/arm/mach-omap2/omap_hwmod_33xx_data.c uses
AM33XX_CM_PER_USB0_CLKCTRL_OFFSET (0x1c) as the clk. The TRM only
mentions this one, i.e. no USB1.

Now I have the following logical devices here:
- usb0 instance + its glue code for irqs
- usb1 instance + its glue code for irqs
- two phy instances, one for USB0 and one USB1
- one dma engine which serves both usb instances.

Shouldn't I have for each device (usb0/1, phy0/1, dma) a hwmod entry?
Or is it enough to use the same entry for each device?

Sebastian

Re: USB & hwmon on am33xx

[Mugunthan V N]

On 7/9/2013 10:02 PM, Sebastian Andrzej Siewior wrote:
> Hi Paul,
>
> I'm slowly losing my mind with hwmod.
>
> arch/arm/boot/dts/am33xx.dtsi has the ti,musb-am33xx node.
>
> That one has "usb_otg_hs" as hwmod property.
> The entry for it arch/arm/mach-omap2/omap_hwmod_33xx_data.c uses
> AM33XX_CM_PER_USB0_CLKCTRL_OFFSET (0x1c) as the clk. The TRM only
> mentions this one, i.e. no USB1.
>
> Now I have the following logical devices here:
> - usb0 instance + its glue code for irqs
> - usb1 instance + its glue code for irqs
> - two phy instances, one for USB0 and one USB1
> - one dma engine which serves both usb instances.
>
> Shouldn't I have for each device (usb0/1, phy0/1, dma) a hwmod entry?
> Or is it enough to use the same entry for each device?
Yes, you need hwmod entry for each of the above device. Since USB has only only
clock ctrl register, have one as parrent and make other  devices as child node.
you can take CPSW as an example.

Regards
Mugunthan V N

Re: USB & hwmon on am33xx

[Sebastian Andrzej Siewior]

On 07/09/2013 08:08 PM, Mugunthan V N wrote:
> Yes, you need hwmod entry for each of the above device. Since USB has only only
> clock ctrl register, have one as parrent and make other  devices as child node.
> you can take CPSW as an example.

So there is not some kind of ref counting if I use this entry twice?
What about the PWM as an example? It has two child nodes and also only
one clock register.

> Regards
> Mugunthan V N

Sebastian

Re: USB & hwmon on am33xx

[Mugunthan V N]

On 7/10/2013 1:31 AM, Sebastian Andrzej Siewior wrote:
> On 07/09/2013 08:08 PM, Mugunthan V N wrote:
>> Yes, you need hwmod entry for each of the above device. Since USB has only only
>> clock ctrl register, have one as parrent and make other  devices as child node.
>> you can take CPSW as an example.
> So there is not some kind of ref counting if I use this entry twice?
> What about the PWM as an example? It has two child nodes and also only
> one clock register.
>
Yes, you can take PWM as example.

Regards
Mugunthan V N

Re: USB & hwmon on am33xx

[Paul Walmsley]

Hello Sebastian,

On Tue, 9 Jul 2013, Sebastian Andrzej Siewior wrote:

> I'm slowly losing my mind with hwmod.

Well, look on the bright side, its days are numbered...

> arch/arm/boot/dts/am33xx.dtsi has the ti,musb-am33xx node.
> 
> That one has "usb_otg_hs" as hwmod property.
> The entry for it arch/arm/mach-omap2/omap_hwmod_33xx_data.c uses
> AM33XX_CM_PER_USB0_CLKCTRL_OFFSET (0x1c) as the clk. The TRM only
> mentions this one, i.e. no USB1.
> 
> Now I have the following logical devices here:
> - usb0 instance + its glue code for irqs
> - usb1 instance + its glue code for irqs
> - two phy instances, one for USB0 and one USB1
> - one dma engine which serves both usb instances.
> 
> Shouldn't I have for each device (usb0/1, phy0/1, dma) a hwmod entry?
> Or is it enough to use the same entry for each device?

When hwmod was designed, it followed OMAP architectural conventions, which 
was that devices didn't share enable/disable register bitfields.  AM33xx 
seems not to have followed that convention.  This is not completely 
surprising since it was designed by a different group of people.

But when hwmod support was added for AM33xx, no one added code to deal 
with the AM33xx shared-bitfield model.  And I did not realize that this 
situation existed.

So ideally, what should happen is that the AM33xx guys should add code to 
deal with the problem.  Maybe by adding a hwmod flag to indicate that the 
IP block shares an enable/disable bitfield with other IP blocks, and that 
the hwmod code should find the other devices, and then implement some kind 
of use-counting for that case?


- Paul