>>-----Original Message----- >>From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>Sent: Thursday, December 10, 2009 10:36 PM >>To: Gopinath, Thara >>Cc: linux-omap@xxxxxxxxxxxxxxx; Paul Walmsley >>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep >>dependencies. >> >>"Gopinath, Thara" <thara@xxxxxx> writes: >> >>>>>-----Original Message----- >>>>>From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>>>>Sent: Thursday, December 10, 2009 4:39 AM >>>>>To: Gopinath, Thara >>>>>Cc: linux-omap@xxxxxxxxxxxxxxx; Paul Walmsley >>>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the >>sleep >>>>>dependencies. >>>>> >>>>>Thara Gopinath <thara@xxxxxx> writes: >>>>> >>>>>> Some clock nodes like wdt1_fck, sr1_fck, sr2_fck etc do not have a >>>>>> clock domain associated . For such nodes accessing the clock domain pointers >>>>>> in _add_initiator_dep and _del_initiator_dep, will lead to null pointer >>>>>> defreferencing crash. Adding support in these functions to check for >>>>>> existence of clkdm pointer before trying to acess it. Even if tomorrow >>>>>> we correct all the clock nodes to have an associated clock domain, checking >>>>>> for the existence of the pointer is a good programming practice. >>>>>> >>>>>> Signed-off-by: Thara Gopinath <thara@xxxxxx> >>>>>> Cc: Paul Walmsley <paul@xxxxxxxxx> >>>>>> --- >>>>>> This patch depends on http://patchwork.kernel.org/patch/63383/ >>>>> >>>>>I think clocks without associated clockdomains should be handled with >>>>>a flag instead of a check for a NULL pointer. >>>>> >>>>>Your current approach will silently fail if someone forgets to add >>>>>a clockdomain to a clock that should have one. >>> >>> Are you suggesting adding a flag to the clock node in case it does >>> not have an associated clockdomain? >> >>Yes, or to the hwmod. >> >>> I am using your tree origin/pm-wip/omap_device branch. On this I see >>> a commit (e89a95db7095d998991c564a756a33ee0ec722c5) in which a >>> warning is printed in omap2_init_clk_clkdm if there is no >>> clockdomain associated with a clock node. >> >>This is to check for the case of a missing clkdm when there should be >>one. You're trying to solve the problem of when there is no >>clockdomain for a given clock. >> >>> My idea was this warning plus the check so as to avoid the system >>> crash would suffice. >> >>I still think this should be flagged explicitly in the clock/hwmod, >>then check using the flag instead of of using NULL pointer check. Two points. First , if there is a flag it should be in clock node. Hwmod structure should not bother with whether the clock node has a clockdomain or not. Second null pointer checks are there every where in the kernel and generally is a good programming practice. >> >>For readability, it makes it more clear that this is checking for some >>exceptions and not simply doing error checking. >> >>Kevin -- 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
