| Message ID | 6744862.JhM2B9tICl@diego (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Heiko, On Sun, Apr 19, 2015 at 12:30 AM, Heiko Stübner <heiko@sntech.de> wrote: > platform_get_irq() can return negative error values and we already test for > these. Therefore the variable holding this value should be signed to not > loose error values. > > Reported-by: David Binderman <dcb314@hotmail.com> > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index ccb0ce0..bde1c1e 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -104,7 +104,7 @@ struct vop { > /* lock vop irq reg */ > spinlock_t irq_lock; > > - unsigned int irq; > + int irq; > Hmm. Both enable_irq() and disable_irq(), etc) want irq as an unsigned int. The thing we want here is to detect a negative return value from platform_get_irq(). So, I'd slightly prefer changing to vop_bind() to something like this: int irq; ... irq = platform_get_irq() If (irq < 0) return irq; vop->irq = (unsigned int)irq; -Dan > > /* vop AHP clk */ > struct clk *hclk; > -- > 2.1.4 > > >
Hi Heiko, On Sun, Apr 19, 2015 at 12:30 AM, Heiko Stübner <heiko@sntech.de> wrote: > platform_get_irq() can return negative error values and we already test for > these. Therefore the variable holding this value should be signed to not > loose error values. > > Reported-by: David Binderman <dcb314@hotmail.com> > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index ccb0ce0..bde1c1e 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -104,7 +104,7 @@ struct vop { > /* lock vop irq reg */ > spinlock_t irq_lock; > > - unsigned int irq; > + int irq; Hmm. Both enable_irq() and disable_irq(), etc) want irq as an unsigned int. The thing we want here is to detect a negative return value from platform_get_irq(). So, I'd slightly prefer changing to vop_bind() to something like this: int irq; ... irq = platform_get_irq() If (irq < 0) return irq; vop->irq = (unsigned int)irq; But, either way, this patch is: Reviewed-By: Daniel Kurtz <djkurtz@chromium.org> Thanks for the fix. > > /* vop AHP clk */ > struct clk *hclk; > -- > 2.1.4 > >
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index ccb0ce0..bde1c1e 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -104,7 +104,7 @@ struct vop { /* lock vop irq reg */ spinlock_t irq_lock; - unsigned int irq; + int irq; /* vop AHP clk */ struct clk *hclk;
platform_get_irq() can return negative error values and we already test for these. Therefore the variable holding this value should be signed to not loose error values. Reported-by: David Binderman <dcb314@hotmail.com> Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)