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