| Message ID | 1367471009-7103-1-git-send-email-sachin.kamat@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Hi Sachin, On 05/02/2013 07:03 AM, Sachin Kamat wrote: > NULL check on clocks obtained using common clock APIs should not > be done. Use IS_ERR only. > > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> > --- > Changes since v1: > Initialised clocks to invalid value. > --- > drivers/media/platform/s5p-tv/hdmi_drv.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c b/drivers/media/platform/s5p-tv/hdmi_drv.c > index 4e86626..ed1a695 100644 > --- a/drivers/media/platform/s5p-tv/hdmi_drv.c > +++ b/drivers/media/platform/s5p-tv/hdmi_drv.c > @@ -765,15 +765,15 @@ static void hdmi_resources_cleanup(struct hdmi_device *hdev) > regulator_bulk_free(res->regul_count, res->regul_bulk); > /* kfree is NULL-safe */ > kfree(res->regul_bulk); > - if (!IS_ERR_OR_NULL(res->hdmiphy)) > + if (!IS_ERR(res->hdmiphy)) > clk_put(res->hdmiphy); > - if (!IS_ERR_OR_NULL(res->sclk_hdmiphy)) > + if (!IS_ERR(res->sclk_hdmiphy)) > clk_put(res->sclk_hdmiphy); > - if (!IS_ERR_OR_NULL(res->sclk_pixel)) > + if (!IS_ERR(res->sclk_pixel)) > clk_put(res->sclk_pixel); > - if (!IS_ERR_OR_NULL(res->sclk_hdmi)) > + if (!IS_ERR(res->sclk_hdmi)) > clk_put(res->sclk_hdmi); > - if (!IS_ERR_OR_NULL(res->hdmi)) > + if (!IS_ERR(res->hdmi)) > clk_put(res->hdmi); > memset(res, 0, sizeof(*res)); Shouldn't this memset be removed not ? Then res->regul_count would need to be set to 0. > } > @@ -793,8 +793,14 @@ static int hdmi_resources_init(struct hdmi_device *hdev) > dev_dbg(dev, "HDMI resource init\n"); > > memset(res, 0, sizeof(*res)); This could be replaced with res->regul_count = 0; > - /* get clocks, power */ > > + res->hdmi = ERR_PTR(-EINVAL); You could remove this line, as res->sclk_hdmi will be set by clk_get() right below. > + res->sclk_hdmi = ERR_PTR(-EINVAL); > + res->sclk_pixel = ERR_PTR(-EINVAL); > + res->sclk_hdmiphy = ERR_PTR(-EINVAL); > + res->hdmiphy = ERR_PTR(-EINVAL); > + > + /* get clocks, power */ > res->hdmi = clk_get(dev, "hdmi"); > if (IS_ERR(res->hdmi)) { > dev_err(dev, "failed to get clock 'hdmi'\n"); > Regards, Sylwester
diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c b/drivers/media/platform/s5p-tv/hdmi_drv.c index 4e86626..ed1a695 100644 --- a/drivers/media/platform/s5p-tv/hdmi_drv.c +++ b/drivers/media/platform/s5p-tv/hdmi_drv.c @@ -765,15 +765,15 @@ static void hdmi_resources_cleanup(struct hdmi_device *hdev) regulator_bulk_free(res->regul_count, res->regul_bulk); /* kfree is NULL-safe */ kfree(res->regul_bulk); - if (!IS_ERR_OR_NULL(res->hdmiphy)) + if (!IS_ERR(res->hdmiphy)) clk_put(res->hdmiphy); - if (!IS_ERR_OR_NULL(res->sclk_hdmiphy)) + if (!IS_ERR(res->sclk_hdmiphy)) clk_put(res->sclk_hdmiphy); - if (!IS_ERR_OR_NULL(res->sclk_pixel)) + if (!IS_ERR(res->sclk_pixel)) clk_put(res->sclk_pixel); - if (!IS_ERR_OR_NULL(res->sclk_hdmi)) + if (!IS_ERR(res->sclk_hdmi)) clk_put(res->sclk_hdmi); - if (!IS_ERR_OR_NULL(res->hdmi)) + if (!IS_ERR(res->hdmi)) clk_put(res->hdmi); memset(res, 0, sizeof(*res)); } @@ -793,8 +793,14 @@ static int hdmi_resources_init(struct hdmi_device *hdev) dev_dbg(dev, "HDMI resource init\n"); memset(res, 0, sizeof(*res)); - /* get clocks, power */ + res->hdmi = ERR_PTR(-EINVAL); + res->sclk_hdmi = ERR_PTR(-EINVAL); + res->sclk_pixel = ERR_PTR(-EINVAL); + res->sclk_hdmiphy = ERR_PTR(-EINVAL); + res->hdmiphy = ERR_PTR(-EINVAL); + + /* get clocks, power */ res->hdmi = clk_get(dev, "hdmi"); if (IS_ERR(res->hdmi)) { dev_err(dev, "failed to get clock 'hdmi'\n");
NULL check on clocks obtained using common clock APIs should not be done. Use IS_ERR only. Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> --- Changes since v1: Initialised clocks to invalid value. --- drivers/media/platform/s5p-tv/hdmi_drv.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)