> -----Original Message----- > From: Cousson, Benoit > Sent: Wednesday, August 04, 2010 3:53 PM > To: Shilimkar, Santosh > Cc: G, Manjunath Kondaiah; Kevin Hilman; linux-omap@xxxxxxxxxxxxxxx; Paul > Walmsley; Tony Lindgren; Sawant, Anand; Nayak, Rajendra; Basak, Partha; > Varadarajan, Charulatha > Subject: Re: [PATCH 04/11] OMAP3: DMA: HWMOD: Add hwmod data structures > > On 8/4/2010 12:15 PM, Shilimkar, Santosh wrote: > >> From: Cousson, Benoit > >> Sent: Wednesday, August 04, 2010 3:38 PM > >> > >> On 8/4/2010 2:35 AM, G, Manjunath Kondaiah wrote: > >>> Kevin, > >>> > >>>> From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] > >>>> Sent: Wednesday, August 04, 2010 3:27 AM > >>>> > >>>> Manjunatha GK<manjugk@xxxxxx> writes: > >>>> > >>>>> This patch adds OMAP3 DMA hwmod structures. > >>>>> > >>>>> Signed-off-by: Manjunatha GK<manjugk@xxxxxx> > >>>> > >>>> [...] > >>>> > >>>>> +static struct omap_hwmod omap3xxx_dma_system_hwmod = { > >>>>> + .name = "dma", > >>>>> + .class =&omap3xxx_dma_hwmod_class, > >>>>> + .mpu_irqs = omap3xxx_dma_system_irqs, > >>>>> + .mpu_irqs_cnt = ARRAY_SIZE(omap3xxx_dma_system_irqs), > >>>>> + .main_clk = "l3_div_ck", > >>>>> + .prcm = { > >>>>> + .omap2 = { > >>>>> + /* .clkctrl_reg = NULL, */ > >>>>> + }, > >>>>> + }, > >>>> > >>>> Has this been tested? Without valid fields here the hwmod > >>>> will never be > >>>> enabled. > >>> > >>> This patch series has been tested on both omap3 and omap4. I didn't > find > >>> any entries for DMA module in prcm-common.h(for assigning .module_bit > >> etc) > >>> hence I commented this for clarification. > >> > >> There is no explicit PRCM control for the SDMA. You can just access the > >> standby status and change the sysconfig idle and standby settings. > >> It uses the L3 interface clock as functional clock. > >> You should still use HWMOD_NO_IDLEST status to prevent the enable to > >> crash. > >> > >> In your case, I think that you are probably never explicitly enabling > or > >> disabling the dma device due to the library kind of API and the fact > >> that the HW is managing is automatically, isn't it? > >> > >> We had the same kind of discussion for the hwspinlock. Since these IPs > >> are using interface clock as a functional clock, there is no need to > >> explicitly enable them. But we should, otherwise we have no way to > track > >> the usage of this IP and potentially to enable the power domain that > >> contains these IP. > >> > >> Each time a dma channel is requested, you should call the > >> pm_runtime_get, and pm_runtime_put() when the channel is not needed > >> anymore. > >> > > Request call can get called on multiple channels. > > So at the time of request a channel, other channels might be already > > in use. So logically " pm_runtime_put" should only be done if > > reserved channels are 0 > > The point is to "enable" the device when at least one channel is in use > and "disable" it when every channel are released. We do have only one > DMA IP that managed severals channels, since pm_runtime is doing > reference counting, we can call get for each channel request and put for > each release. > Did I miss something? We are saying same thing more or less :) Since it's one IP, and say have one clock, then doing per channel runtime get/put is redundant, No? -- 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
