Messages in this thread | | | Date | Sat, 29 Jan 2022 18:35:00 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH] staging: media: atomisp: Use BIT macro instead of left shifting |
| |
On Sat, Jan 29, 2022 at 12:38:04PM +0100, Moses Christopher Bollavarapu wrote:
It's a bit too many people in Cc list (I dropped more than a half in this reply). I would suggest to use my script [1] or look into it for a hint how to reduce it.
[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
> There is a BIT(nr) macro available in Linux Kernel, > which does the same thing.
In some cases it might fix a (theoretical) UB issue.
> Example: BIT(7) = (1UL << 7)
...
> + s_config->source.tpg.x_mask = BIT(4) - 1;
> + s_config->source.tpg.y_mask = BIT(4) - 1;
> + s_config->source.tpg.xy_mask = BIT(8) - 1;
For masks you may use GENMASK().
...
> - irq = irq & 1 << INTR_IIR; > + irq = irq & BIT(INTR_IIR);
It may be:
irq &= BIT(...);
...
> - irq = irq & 1 << INTR_IIR; > + irq = irq & BIT(INTR_IIR);
Ditto.
...
> - virt += (1 << PAGE_SHIFT); > + virt += BIT(PAGE_SHIFT);
This is weird. Shouldn't be as simple as
virt += PAGE_SIZE;
?
...
> enum ia_css_sp_sleep_mode { > SP_DISABLE_SLEEP_MODE = 0, > - SP_SLEEP_AFTER_FRAME = 1 << 0, > - SP_SLEEP_AFTER_IRQ = 1 << 1 > + SP_SLEEP_AFTER_FRAME = BIT(0),
> + SP_SLEEP_AFTER_IRQ = BIT(1)
While at it, add a comma here.
> };
...
> + IA_CSS_DEBUG_DUMP_ALL = BIT(14) /** Dump all device parameters */
Ditto.
...
On top of this don't forget to add
#include <linux/bits.h>
to the modified header files.
-- With Best Regards, Andy Shevchenko
| |