* [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
@ 2012-04-18 4:08 jhbird.choi
2012-04-19 0:12 ` Kukjin Kim
2012-06-21 0:18 ` Kukjin Kim
0 siblings, 2 replies; 9+ messages in thread
From: jhbird.choi @ 2012-04-18 4:08 UTC (permalink / raw)
To: linux-samsung-soc; +Cc: Kukjin Kim, Jonghwan Choi
From: Jonghwan Choi <jhbird.choi@samsung.com>
On the error condition clk_get() returns ERR_PTR().
Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
---
.../arm/plat-samsung/include/plat/watchdog-reset.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-samsung/include/plat/watchdog-reset.h b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
index f19aff1..bc4db9b 100644
--- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
+++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
@@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
__raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
- if (s3c2410_wdtclk)
+ if (!IS_ERR(s3c2410_wdtclk))
clk_enable(s3c2410_wdtclk);
/* put initial values into count and data */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
2012-04-18 4:08 [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL jhbird.choi
@ 2012-04-19 0:12 ` Kukjin Kim
2012-04-19 8:25 ` Sylwester Nawrocki
2012-06-21 0:18 ` Kukjin Kim
1 sibling, 1 reply; 9+ messages in thread
From: Kukjin Kim @ 2012-04-19 0:12 UTC (permalink / raw)
To: jhbird.choi; +Cc: linux-samsung-soc, Kukjin Kim, Jonghwan Choi
jhbird.choi@gmail.com wrote:
> From: Jonghwan Choi<jhbird.choi@samsung.com>
>
> On the error condition clk_get() returns ERR_PTR().
>
> Signed-off-by: Jonghwan Choi<jhbird.choi@samsung.com>
> ---
> .../arm/plat-samsung/include/plat/watchdog-reset.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/include/plat/watchdog-reset.h b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> index f19aff1..bc4db9b 100644
> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
>
> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
>
> - if (s3c2410_wdtclk)
> + if (!IS_ERR(s3c2410_wdtclk))
Yeah, right. BTW don't we need to check NULL here?
> clk_enable(s3c2410_wdtclk);
>
> /* put initial values into count and data */
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
2012-04-19 0:12 ` Kukjin Kim
@ 2012-04-19 8:25 ` Sylwester Nawrocki
0 siblings, 0 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2012-04-19 8:25 UTC (permalink / raw)
To: Kukjin Kim
Cc: jhbird.choi, linux-samsung-soc, Jonghwan Choi,
'linux-arm-kernel'
On 04/19/2012 02:12 AM, Kukjin Kim wrote:
>> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
>>
>> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
>>
>> - if (s3c2410_wdtclk)
>> + if (!IS_ERR(s3c2410_wdtclk))
>
> Yeah, right. BTW don't we need to check NULL here?
It might make sense to check for NULL as well, but only if it happens
that a clock entry is ever added to clkdev with null struct clk_lookup:clk
member. This in turn seems impossible with the current clock code at
plat-samsung/clock.c.
>> clk_enable(s3c2410_wdtclk);
>>
>> /* put initial values into count and data */
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
@ 2012-04-19 8:25 ` Sylwester Nawrocki
0 siblings, 0 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2012-04-19 8:25 UTC (permalink / raw)
To: linux-arm-kernel
On 04/19/2012 02:12 AM, Kukjin Kim wrote:
>> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
>>
>> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
>>
>> - if (s3c2410_wdtclk)
>> + if (!IS_ERR(s3c2410_wdtclk))
>
> Yeah, right. BTW don't we need to check NULL here?
It might make sense to check for NULL as well, but only if it happens
that a clock entry is ever added to clkdev with null struct clk_lookup:clk
member. This in turn seems impossible with the current clock code at
plat-samsung/clock.c.
>> clk_enable(s3c2410_wdtclk);
>>
>> /* put initial values into count and data */
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
2012-04-19 8:25 ` Sylwester Nawrocki
@ 2012-04-19 9:03 ` Russell King - ARM Linux
-1 siblings, 0 replies; 9+ messages in thread
From: Russell King - ARM Linux @ 2012-04-19 9:03 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Kukjin Kim, jhbird.choi, linux-samsung-soc, Jonghwan Choi,
'linux-arm-kernel'
On Thu, Apr 19, 2012 at 10:25:36AM +0200, Sylwester Nawrocki wrote:
> On 04/19/2012 02:12 AM, Kukjin Kim wrote:
> >> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> >> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> >> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
> >>
> >> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
> >>
> >> - if (s3c2410_wdtclk)
> >> + if (!IS_ERR(s3c2410_wdtclk))
> >
> > Yeah, right. BTW don't we need to check NULL here?
>
> It might make sense to check for NULL as well, but only if it happens
> that a clock entry is ever added to clkdev with null struct clk_lookup:clk
> member.
Not quite the correct answer. The correct answer is:
Drivers have no business interpreting anything but IS_ERR() values from
clk_get() as errors. Everything else they _MUST_ assume is valid for
the rest of the clk API.
The clue: struct clk is an opaque cookie as far as drivers are concerned.
The only interpretation drivers are allowed to make is that IS_ERR()
values indicate an error. Everything else is potentially valid.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
@ 2012-04-19 9:03 ` Russell King - ARM Linux
0 siblings, 0 replies; 9+ messages in thread
From: Russell King - ARM Linux @ 2012-04-19 9:03 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 19, 2012 at 10:25:36AM +0200, Sylwester Nawrocki wrote:
> On 04/19/2012 02:12 AM, Kukjin Kim wrote:
> >> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> >> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> >> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
> >>
> >> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
> >>
> >> - if (s3c2410_wdtclk)
> >> + if (!IS_ERR(s3c2410_wdtclk))
> >
> > Yeah, right. BTW don't we need to check NULL here?
>
> It might make sense to check for NULL as well, but only if it happens
> that a clock entry is ever added to clkdev with null struct clk_lookup:clk
> member.
Not quite the correct answer. The correct answer is:
Drivers have no business interpreting anything but IS_ERR() values from
clk_get() as errors. Everything else they _MUST_ assume is valid for
the rest of the clk API.
The clue: struct clk is an opaque cookie as far as drivers are concerned.
The only interpretation drivers are allowed to make is that IS_ERR()
values indicate an error. Everything else is potentially valid.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
2012-04-19 9:03 ` Russell King - ARM Linux
@ 2012-04-20 10:25 ` Sylwester Nawrocki
-1 siblings, 0 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2012-04-20 10:25 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Kukjin Kim, jhbird.choi, linux-samsung-soc, Jonghwan Choi,
'linux-arm-kernel'
On 04/19/2012 11:03 AM, Russell King - ARM Linux wrote:
> On Thu, Apr 19, 2012 at 10:25:36AM +0200, Sylwester Nawrocki wrote:
>> On 04/19/2012 02:12 AM, Kukjin Kim wrote:
>>>> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>>>> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>>>> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
>>>>
>>>> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
>>>>
>>>> - if (s3c2410_wdtclk)
>>>> + if (!IS_ERR(s3c2410_wdtclk))
>>>
>>> Yeah, right. BTW don't we need to check NULL here?
>>
>> It might make sense to check for NULL as well, but only if it happens
>> that a clock entry is ever added to clkdev with null struct clk_lookup:clk
>> member.
>
> Not quite the correct answer. The correct answer is:
>
> Drivers have no business interpreting anything but IS_ERR() values from
> clk_get() as errors. Everything else they _MUST_ assume is valid for
> the rest of the clk API.
>
> The clue: struct clk is an opaque cookie as far as drivers are concerned.
> The only interpretation drivers are allowed to make is that IS_ERR()
> values indicate an error. Everything else is potentially valid.
Thanks for the clarification. I stand corrected.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
@ 2012-04-20 10:25 ` Sylwester Nawrocki
0 siblings, 0 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2012-04-20 10:25 UTC (permalink / raw)
To: linux-arm-kernel
On 04/19/2012 11:03 AM, Russell King - ARM Linux wrote:
> On Thu, Apr 19, 2012 at 10:25:36AM +0200, Sylwester Nawrocki wrote:
>> On 04/19/2012 02:12 AM, Kukjin Kim wrote:
>>>> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>>>> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
>>>> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
>>>>
>>>> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe */
>>>>
>>>> - if (s3c2410_wdtclk)
>>>> + if (!IS_ERR(s3c2410_wdtclk))
>>>
>>> Yeah, right. BTW don't we need to check NULL here?
>>
>> It might make sense to check for NULL as well, but only if it happens
>> that a clock entry is ever added to clkdev with null struct clk_lookup:clk
>> member.
>
> Not quite the correct answer. The correct answer is:
>
> Drivers have no business interpreting anything but IS_ERR() values from
> clk_get() as errors. Everything else they _MUST_ assume is valid for
> the rest of the clk API.
>
> The clue: struct clk is an opaque cookie as far as drivers are concerned.
> The only interpretation drivers are allowed to make is that IS_ERR()
> values indicate an error. Everything else is potentially valid.
Thanks for the clarification. I stand corrected.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL
2012-04-18 4:08 [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL jhbird.choi
2012-04-19 0:12 ` Kukjin Kim
@ 2012-06-21 0:18 ` Kukjin Kim
1 sibling, 0 replies; 9+ messages in thread
From: Kukjin Kim @ 2012-06-21 0:18 UTC (permalink / raw)
To: jhbird.choi, linux-samsung-soc; +Cc: 'Jonghwan Choi'
jhbird.choi@gmail.com wrote:
>
> From: Jonghwan Choi <jhbird.choi@samsung.com>
>
> On the error condition clk_get() returns ERR_PTR().
>
> Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
> ---
> .../arm/plat-samsung/include/plat/watchdog-reset.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> index f19aff1..bc4db9b 100644
> --- a/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> +++ b/arch/arm/plat-samsung/include/plat/watchdog-reset.h
> @@ -25,7 +25,7 @@ static inline void arch_wdt_reset(void)
>
> __raw_writel(0, S3C2410_WTCON); /* disable watchdog, to be safe
> */
>
> - if (s3c2410_wdtclk)
> + if (!IS_ERR(s3c2410_wdtclk))
> clk_enable(s3c2410_wdtclk);
>
> /* put initial values into count and data */
> --
> 1.7.0.4
Applied, thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-06-21 0:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-18 4:08 [PATCH] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL jhbird.choi
2012-04-19 0:12 ` Kukjin Kim
2012-04-19 8:25 ` Sylwester Nawrocki
2012-04-19 8:25 ` Sylwester Nawrocki
2012-04-19 9:03 ` Russell King - ARM Linux
2012-04-19 9:03 ` Russell King - ARM Linux
2012-04-20 10:25 ` Sylwester Nawrocki
2012-04-20 10:25 ` Sylwester Nawrocki
2012-06-21 0:18 ` Kukjin Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.