| Message ID | 1367227499-543-1-git-send-email-sachin.kamat@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/29/2013 11:24 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> > --- > drivers/media/platform/s5p-tv/hdmi_drv.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c b/drivers/media/platform/s5p-tv/hdmi_drv.c > index 4e86626..b3344cb 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)); > } I think this patch is incomplete. You need to ensure all the clock pointers are initially set to an invalid value. This is currently done with memsetting whole hdmi_resource structure to 0. But now some ERR_PTR() value needs to be used instead. Same applies to your subsequent patch. Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, On 1 May 2013 15:02, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote: > On 04/29/2013 11:24 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> >> --- >> drivers/media/platform/s5p-tv/hdmi_drv.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c >> b/drivers/media/platform/s5p-tv/hdmi_drv.c >> index 4e86626..b3344cb 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)); >> } > > > I think this patch is incomplete. You need to ensure all the clock pointers > are initially set to an invalid value. This is currently done with > memsetting > whole hdmi_resource structure to 0. But now some ERR_PTR() value needs to be > used instead. Same applies to your subsequent patch. Yes you are right. Initialised the clocks to invalid value and sent v2 of both these patches.
diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c b/drivers/media/platform/s5p-tv/hdmi_drv.c index 4e86626..b3344cb 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)); }
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> --- drivers/media/platform/s5p-tv/hdmi_drv.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)