lkml.org 
[lkml]   [2010]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] Add a common struct clk
From
Date
Hi MyungJoo,

> > +int clk_enable(struct clk *clk)
> > +{
> > + int ret = 0;
> > +
> > + if (!clk->ops->enable)
> > + return 0;
>
> Wouldn't it be better (safer?) to check "clk" and "clk->ops" before
> accessing clk->ops->enable?
> For example,
>
> if (IS_ERR_OR_NULL(clk))
> return -EINVAL;
>
> if (!clk->ops || !clk->ops->enable)
> return 0;
>
> Or, do you think it'd be better not to check and save some time?
>
> Anyway, if we intend to check the input, the patch for the patch
> including kernel/clk.c would be...

I think that we should leave it as-is; although it may be 'safer', it
may mean more subtle bugs can arise because we've been handed an invalid
clock pointer.

I'd rather the code oops on first usage so that the developer realises
that something is broken, rather than fail with an error code.

BTW - nice work on the samsung implementation, I will check it out soon.

Cheers,


Jeremy



\
 
 \ /
  Last update: 2010-07-12 04:21    [W:0.060 / U:1.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site