Hi Sylwester, On 1 May 2013 15:02, Sylwester Nawrocki <sylvester.nawrocki@xxxxxxxxx> 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@xxxxxxxxxx> >> --- >> 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. -- With warm regards, Sachin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html