Re: [PATCH ] mlx4_core: Enhance device capabilities flags debug printouts

Yuval Shaia
2015-01-25T10:36:30Z

> >> >- mlx4_dbg(dev, " %s\n", fname[i]);
> >> >+ if (fname[i])
> >> >+ mlx4_dbg(dev, " (%c) %s\n", (flags & (1LL << i)) ?
> >> >+ '*' : ' ', fname[i]);
> >> > }
> >>
> >> I think that the strings should be displayed only if both driver and
> >> FW supports them. You could add a QUERY_DEV_CAP_SUPPORTED_FLAGS
> >> which ors all the supported flags in device.h and check if (flags &
> >> QUERY_DEV_CAP_SUPPORTED_FLAGS) & (1LL << i)).
But that would add an extra place to maintain when new feature will be added to driver, isn't it?
At present, when new feature is added to driver enum and string array needs to be updated.
With this new approach, string array is full, need only to add new enum and to update QUERY_DEV_CAP_SUPPORTED_FLAGS.
Again, two places in the code to change.
Up to you folks, which one you like better?
I like the new approach, it is less error prone.
> >>
> >> >
> >> > static void dump_dev_cap_flags2(struct mlx4_dev *dev, u64 flags)
> >> >@@ -144,8 +172,9 @@ static void dump_dev_cap_flags2(struct mlx4_dev *dev, u64 flags)
> >> > int i;
> >> >
> >> > for (i =; i < ARRAY_SIZE(fname); ++i)
> >> >- if (fname[i] && (flags & (1LL << i)))
> >> >- mlx4_dbg(dev, " %s\n", fname[i]);
> >> >+ if (fname[i])
> >> >+ mlx4_dbg(dev, " (%c) %s\n", (flags & (1LL << i)) ?
> >> >+ '*' : ' ', fname[i]);
> >> > }