Hi Omar,
On Friday 23 December 2011 16:53:58 Ramirez Luna, Omar wrote:
> On Mon, Dec 19, 2011 at 10:11 AM, Felipe Contreras wrote:
> > On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar wrote:
> >> On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras wrote:
> >>> Are you sure you are not missing something like:
> >>>
> >>> .clk = "cam_ick",
> >>
> >> I believe in this case cam_ick is used as the main clock as it
> >> supplies both functional and interface.
> >
> > Are you sure?
>
> As sure as 4.7.4.1.7 CAM Power Domain ;), if someone else could
> clarify would be great to avoid the "are you sure" discussion.
I don't know where the ISP IOMMU fonctional and interface clocks come from,
sorry. Shouldn't it be possible to find someone inside TI who can provide that
information ? :-)
> >>>> +/* isp mmu slave ports */
> >>>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
> >>>> + &omap3xxx_l4_core__isp_mmu,
> >>>> +};
> >>>> +
> >>>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
> >>>> + .name = "isp_mmu",
> >>>> + .class = &omap3xxx_mmu_hwmod_class,
> >>>> + .mpu_irqs = omap3xxx_isp_mmu_irqs,
> >>>> + .main_clk = "cam_ick",
> >>>
> >>> It's not "cam_fck"?
> >>
> >> AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
> >> both ick/fck according to TRM.
> >
> > Then maybe the code is wrong.
> >
> > Look at the OMAP34xx documentation, it says that:
> >
> > CAM_L3_ICLK -> CAM_FCLK
> > CAM_L4_ICLK -> CAM_ICLK
> > CAM_MCLK -> CAM_MCLK
> > CSI2_96M_FCLK -> CSI2_96M_FCLK
> >
> > CAM_FCLK
> > Functional clock (L3 interconnect clock domain)
> > Functional clock domain.
> >
> > CAM_ICLK
> > Interface clock (L4 interconnect clock domain)
> > Interface clock domain.
>
> "The camera subsystem interface is clocked with the L3 and L4 clocks
> (CAM_L3_ICLK and CAM_L4_ICLK, respectively). CAM_L3_ICLK is also used
> as the main functional clock. The functional clock (CAM_MCLK) is
> provided by DPLL4 to supply the external sensor."
>
> Either CAM subsystem section or PRCM - CAM PWRDM is ambiguous or wrong.
I haven't checked how the clocks are used in details (I started working on the
OMAP3 ISP driver after the clock handling code was in place), but I can at
least confirm that CAM_L3_ICLK is the main functional clock. I would be a bit
surprised if CAM_L3_ICLK was used as the interface clock as well.
Once again, you should be able to find clarification on this subject inside
TI.
> ...
>
> > Looks like the driver is manually calling clk_get() and clk_put() for
> > the "i3_ick", where I guess the clock framework is supposed to do
> > that... It's almost as if somebody forgot a dependency somewhere.
>
> Would be good to clarify the intentions to keep the code as it is.
Once proper clock dependencies will be in place I'll be glad to remove the
direct l3_ick if needed.
> >>>> + .dev_attr = &isp_mmu_dev_attr,
> >>>> + .slaves = omap3xxx_isp_mmu_slaves,
> >>>> + .slaves_cnt = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
> >>>> + .flags = HWMOD_NO_IDLEST,
> >>>> +};
> >>>
> >>> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
> >>> .clk = "foo_ick". Maybe that explains the irq issues you get.
> >>
> >> I see irq issues with iva hwmod because tidspbridge doesn't use iommu
> >> API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
> >> implementation there will be some conflicts.
> >>
> >> I didn't see isp issues though, but I didn't went more than
> >> booting/enabling with isp mmu.
> >
> > This is what you said:
> > Removed clk handling during interrupt, given that in order to receive
> > one, the device should be powered on in advance.
>
> Yes, you should receive an interrupt if the clock is enabled and the
> iommu is being used, hence the part inside in the ISR to enable the
> clocks was removed.
>
> > I'm not sure how this clock stuff works, but I'm guessing the device
> > is supposed to go to sleep at some points in time, and with your patch
> > "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
> > is loaded, unless I'm missing something.
>
> The device should be able to be put to sleep at anytime when it is NOT
> being used. AFAIK there is no mechanism for the main processor (the
> one running the kernel) to know when the other iommus are not being
> used, given that they are in independent processors/subsystems, at
> least for the ones in the DSP or M3 processors. Once the user releases
> its iommu resource it means it is no longer using it, at that point
> the device can be put to sleep.
How should the OMAP3 ISP driver proceed to make sure that its IOMMU is clocked
off when it doesn't need it ?
--
Regards,
Laurent Pinchart
--
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
