| Message ID | 20200516212330.13633-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm: Include internal header for managed function declarations | expand |
On Sat, May 16, 2020 at 10:23:28PM +0100, Chris Wilson wrote: > drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’: > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] > __builtin_constant_p((l) > (h)), (l) > (h), 0))) > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Took me a while to spot where this goes boom, kinda wonder whether we should have an if (WARN_ON(!connector_count)) return -EINVAL; somewhere in here. Just for belts&suspenders approach. Anyway this looks good. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_client_modeset.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c > index 7443114bd713..6e9530df0737 100644 > --- a/drivers/gpu/drm/drm_client_modeset.c > +++ b/drivers/gpu/drm/drm_client_modeset.c > @@ -563,7 +563,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, > struct drm_client_offset *offsets, > bool *enabled, int width, int height) > { > - unsigned int count = min_t(unsigned int, connector_count, BITS_PER_LONG); > + const int count = min_t(unsigned int, connector_count, BITS_PER_LONG); > unsigned long conn_configured, conn_seq, mask; > struct drm_device *dev = client->dev; > int i, j; > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoting Daniel Vetter (2020-05-18 15:47:44) > On Sat, May 16, 2020 at 10:23:28PM +0100, Chris Wilson wrote: > > drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’: > > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] > > __builtin_constant_p((l) > (h)), (l) > (h), 0))) > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Took me a while to spot where this goes boom, kinda wonder whether we > should have an > > if (WARN_ON(!connector_count)) > return -EINVAL; Atm, drm_client_firmware_config() is called only if connector_count!=0. But if count==0, we would hit goto retry indefinitely, seems like that would be worth a WARN. -Chris
On Sat, 16 May 2020 at 22:23, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’: > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] > __builtin_constant_p((l) > (h)), (l) > (h), 0))) > Hmm this seems like a hack around a macro bug. I'll send a fix for the macro in a few minutes. With that you don't have to tinker to make it unsigned, although const will be appreciated. -Emil
Quoting Emil Velikov (2020-05-18 17:23:21) > On Sat, 16 May 2020 at 22:23, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’: > > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] > > __builtin_constant_p((l) > (h)), (l) > (h), 0))) > > > Hmm this seems like a hack around a macro bug. I'll send a fix for the > macro in a few minutes. > With that you don't have to tinker to make it unsigned, although const > will be appreciated. Streams crossed. I added Daniel's suggestion for a WARN_ON to make it clear that we should never pass connectors_count==0 to drm_client_firmware_config -Chris
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index 7443114bd713..6e9530df0737 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -563,7 +563,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, struct drm_client_offset *offsets, bool *enabled, int width, int height) { - unsigned int count = min_t(unsigned int, connector_count, BITS_PER_LONG); + const int count = min_t(unsigned int, connector_count, BITS_PER_LONG); unsigned long conn_configured, conn_seq, mask; struct drm_device *dev = client->dev; int i, j;
drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’: ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] __builtin_constant_p((l) > (h)), (l) > (h), 0))) Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/drm_client_modeset.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)