Re: [PATCH 04/11] OMAP3: DMA: HWMOD: Add hwmod data structures

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

 



On 8/4/2010 12:27 PM, Shilimkar, Santosh wrote:
-----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 several 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 :)

That was my impression as well, but I was not sure :-)

Since it's one IP, and say have one clock, then doing per channel runtime
get/put is redundant, No?

If the DMA driver is already counting the number of channel in use, you're right we can avoid that. It is already the case today?

Benoit

--
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


[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