| Message ID | 1355852048-23188-4-git-send-email-linux@prisktech.co.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2012-12-18 at 21:39 +0300, Dan Carpenter wrote: > On Wed, Dec 19, 2012 at 06:34:05AM +1300, Tony Prisk wrote: > > Resend to include mailing lists. > > > > Replace IS_ERR_OR_NULL with IS_ERR on clk_get results. > > > > The original code is correct. clk_get() can return NULL depending > on the .config. > > regards, > dan carpenter Thanks for than Dan, Arguably that seems like an incorrect behaviour on the part of the clock subsystem given that the 'proper' function has kernel doc: * Returns a struct clk corresponding to the clock producer, or * valid IS_ERR() condition containing errno. Therefore the 'empty' version should adhere to the same rules, and not return NULL. I've cc'd Mike Turquette as well for his thoughts. Regards Tony P
On Tue, 2012-12-18 at 22:03 +0300, Dan Carpenter wrote: > I don't care either way, but being different from the documentation > is less bad than crashing which is what your patch does. Please > be more careful in the future. > > regards, > dan carpenter Critism accepted. Given that the driver depends on (PLAT_SAMSUNG || ARCH_MULTIPLATFORM), and multiplatform select COMMON_CLK and PLAT_SAMSUNG is selected only by ARCH_S3C64XX which has HAVE_CLK I didn't see a problem here. Your point is still valid and I will sort it out with Mike first. Regards Tony P
On Wed, 2012-12-19 at 08:11 +1300, Tony Prisk wrote: > On Tue, 2012-12-18 at 22:03 +0300, Dan Carpenter wrote: > > I don't care either way, but being different from the documentation > > is less bad than crashing which is what your patch does. Please > > be more careful in the future. > > > > regards, > > dan carpenter > > Critism accepted. > > Given that the driver depends on (PLAT_SAMSUNG || ARCH_MULTIPLATFORM), > and multiplatform select COMMON_CLK and PLAT_SAMSUNG is selected only by > ARCH_S3C64XX which has HAVE_CLK I didn't see a problem here. > > Your point is still valid and I will sort it out with Mike first. > > Regards > Tony P Actually, I was wrong here - PLAT_SAMSUNG is selectable via PLAT_S3C24XX || ARCH_S3C64XX || PLAT_S5P but all these options (or the options that select them) seem to select a clock system as well, hence still no reason for a crash. Regards Tony P
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 2c115f8..557ef2f 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2167,27 +2167,27 @@ static int __devinit hdmi_resources_init(struct hdmi_context *hdata) /* get clocks, power */ res->hdmi = clk_get(dev, "hdmi"); - if (IS_ERR_OR_NULL(res->hdmi)) { + if (IS_ERR(res->hdmi)) { DRM_ERROR("failed to get clock 'hdmi'\n"); goto fail; } res->sclk_hdmi = clk_get(dev, "sclk_hdmi"); - if (IS_ERR_OR_NULL(res->sclk_hdmi)) { + if (IS_ERR(res->sclk_hdmi)) { DRM_ERROR("failed to get clock 'sclk_hdmi'\n"); goto fail; } res->sclk_pixel = clk_get(dev, "sclk_pixel"); - if (IS_ERR_OR_NULL(res->sclk_pixel)) { + if (IS_ERR(res->sclk_pixel)) { DRM_ERROR("failed to get clock 'sclk_pixel'\n"); goto fail; } res->sclk_hdmiphy = clk_get(dev, "sclk_hdmiphy"); - if (IS_ERR_OR_NULL(res->sclk_hdmiphy)) { + if (IS_ERR(res->sclk_hdmiphy)) { DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n"); goto fail; } res->hdmiphy = clk_get(dev, "hdmiphy"); - if (IS_ERR_OR_NULL(res->hdmiphy)) { + if (IS_ERR(res->hdmiphy)) { DRM_ERROR("failed to get clock 'hdmiphy'\n"); goto fail; }
Resend to include mailing lists. Replace IS_ERR_OR_NULL with IS_ERR on clk_get results. Signed-off-by: Tony Prisk <linux@prisktech.co.nz> CC: Inki Dae <inki.dae@samsung.com> CC: Joonyoung Shim <jy0922.shim@samsung.com> CC: Seung-Woo Kim <sw0312.kim@samsung.com> CC: Kyungmin Park <kyungmin.park@samsung.com> CC: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/exynos/exynos_hdmi.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)