[PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Thara Gopinath]

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@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
This patch depends on http://patchwork.kernel.org/patch/63383/

 arch/arm/mach-omap2/omap_hwmod.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 18e6478..3edc387 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -314,8 +314,10 @@ static int _add_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
 	if (!oh->_clk)
 		return -EINVAL;
 
-	return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
+	if (oh->_clk->clkdm)
+		return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
 				  init_oh->_clk->clkdm->pwrdm.ptr);
+	return 0;
 }
 
 /**
@@ -335,8 +337,10 @@ static int _del_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
 	if (!oh->_clk)
 		return -EINVAL;
 
-	return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
+	if (oh->_clk->clkdm)
+		return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
 				  init_oh->_clk->clkdm->pwrdm.ptr);
+	return 0;
 }
 
 /**
-- 
1.5.4.7

Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Kevin Hilman]

Thara Gopinath <thara@ti.com> 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@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> ---
> 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.

Kevin


>  arch/arm/mach-omap2/omap_hwmod.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 18e6478..3edc387 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -314,8 +314,10 @@ static int _add_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
>  	if (!oh->_clk)
>  		return -EINVAL;
>  
> -	return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
> +	if (oh->_clk->clkdm)
> +		return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>  				  init_oh->_clk->clkdm->pwrdm.ptr);
> +	return 0;
>  }
>  
>  /**
> @@ -335,8 +337,10 @@ static int _del_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
>  	if (!oh->_clk)
>  		return -EINVAL;
>  
> -	return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
> +	if (oh->_clk->clkdm)
> +		return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>  				  init_oh->_clk->clkdm->pwrdm.ptr);
> +	return 0;
>  }
>  
>  /**
> -- 
> 1.5.4.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, December 10, 2009 4:39 AM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep
>>dependencies.
>>
>>Thara Gopinath <thara@ti.com> 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@ti.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> ---
>>> 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?
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. My idea was this warning plus the check so as to avoid the system crash would suffice.
>>
>>Kevin
>>
>>
>>>  arch/arm/mach-omap2/omap_hwmod.c |    8 ++++++--
>>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index 18e6478..3edc387 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -314,8 +314,10 @@ static int _add_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod
>>*init_oh)
>>>  	if (!oh->_clk)
>>>  		return -EINVAL;
>>>
>>> -	return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>>> +	if (oh->_clk->clkdm)
>>> +		return pwrdm_add_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>>>  				  init_oh->_clk->clkdm->pwrdm.ptr);
>>> +	return 0;
>>>  }
>>>
>>>  /**
>>> @@ -335,8 +337,10 @@ static int _del_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod
>>*init_oh)
>>>  	if (!oh->_clk)
>>>  		return -EINVAL;
>>>
>>> -	return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>>> +	if (oh->_clk->clkdm)
>>> +		return pwrdm_del_sleepdep(oh->_clk->clkdm->pwrdm.ptr,
>>>  				  init_oh->_clk->clkdm->pwrdm.ptr);
>>> +	return 0;
>>>  }
>>>
>>>  /**
>>> --
>>> 1.5.4.7
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Thursday, December 10, 2009 4:39 AM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep
>>>dependencies.
>>>
>>>Thara Gopinath <thara@ti.com> 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@ti.com>
>>>> Cc: Paul Walmsley <paul@pwsan.com>
>>>> ---
>>>> 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

RE: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, December 10, 2009 10:36 PM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep
>>dependencies.
>>
>>"Gopinath, Thara" <thara@ti.com> writes:
>>
>>>>>-----Original Message-----
>>>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>Sent: Thursday, December 10, 2009 4:39 AM
>>>>>To: Gopinath, Thara
>>>>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the
>>sleep
>>>>>dependencies.
>>>>>
>>>>>Thara Gopinath <thara@ti.com> 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@ti.com>
>>>>>> Cc: Paul Walmsley <paul@pwsan.com>
>>>>>> ---
>>>>>> 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.

Agreed. We could add a flag and base the check on the flag. But still if some clock node does not have a clkdomain and forgets to put this flag, the system will still crash. So have have both the checks ??

>>
>>Kevin

Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Thursday, December 10, 2009 10:36 PM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep
>>>dependencies.
>>>
>>>"Gopinath, Thara" <thara@ti.com> writes:
>>>
>>>>>>-----Original Message-----
>>>>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>>Sent: Thursday, December 10, 2009 4:39 AM
>>>>>>To: Gopinath, Thara
>>>>>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>>>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the
>>>sleep
>>>>>>dependencies.
>>>>>>
>>>>>>Thara Gopinath <thara@ti.com> 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@ti.com>
>>>>>>> Cc: Paul Walmsley <paul@pwsan.com>
>>>>>>> ---
>>>>>>> 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.
>
> Agreed. We could add a flag and base the check on the flag. But
> still if some clock node does not have a clkdomain and forgets to
> put this flag, the system will still crash. 

Yes, but with a clear message as to why.

> So have have both the checks ??

I don't think so, because the check you added will allow the system to
continue in a known non-working state.

Kevin

RE: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, December 10, 2009 10:36 PM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep
>>dependencies.
>>
>>"Gopinath, Thara" <thara@ti.com> writes:
>>
>>>>>-----Original Message-----
>>>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>Sent: Thursday, December 10, 2009 4:39 AM
>>>>>To: Gopinath, Thara
>>>>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the
>>sleep
>>>>>dependencies.
>>>>>
>>>>>Thara Gopinath <thara@ti.com> 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@ti.com>
>>>>>> Cc: Paul Walmsley <paul@pwsan.com>
>>>>>> ---
>>>>>> 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

Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep dependencies.

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Thursday, December 10, 2009 10:36 PM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the sleep
>>>dependencies.
>>>
>>>"Gopinath, Thara" <thara@ti.com> writes:
>>>
>>>>>>-----Original Message-----
>>>>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>>Sent: Thursday, December 10, 2009 4:39 AM
>>>>>>To: Gopinath, Thara
>>>>>>Cc: linux-omap@vger.kernel.org; Paul Walmsley
>>>>>>Subject: Re: [PATCH] OMAP3: hwmod: check for clkdomain pointer before accesing it to change the
>>>sleep
>>>>>>dependencies.
>>>>>>
>>>>>>Thara Gopinath <thara@ti.com> 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@ti.com>
>>>>>>> Cc: Paul Walmsley <paul@pwsan.com>
>>>>>>> ---
>>>>>>> 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. 

OK

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

Of course, NULL pointer checks are commonly used to catch errors.  My
point is is that there are two things we're trying to catch here, and
I was hoping to distinguish between the two:

1) errors: clocks that *have* a clockdomain but it's missing from the
   struct clk:

2) exceptions: clocks that do not have an associated clockdomain and
   need special treatment

IMHO, for readability, these should not be handled with the same NULL
pointer check.  Using NULL pointer check to catch (1) is perfectly OK
(and what I did with my WARN patch.)  

My point (and my opinion) is only that (2) should not be handled by
the same NULL pointer check, but rather by using a flag to indicate
the special case.

Kevin