Hi Andy,
Le mer., mars 16 2022 at 16:47:24 +0200, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> a écrit :
On Wed, Mar 16, 2022 at 4:11 AM Paul Cercueil <paul@xxxxxxxxxxxxxxx>
wrote:
By being a bit smarter about how the SoC version checks are
performed,
it is possible to have all the code paths that correspond to SoCs
disabled in the kernel config automatically marked as dead code by
the
compiler, and therefore garbage-collected.
With this patch, when compiling a kernel that only targets the
JZ4760
for instance, the driver is now about 4.5 KiB smaller.
...
+static const u32 enabled_socs =
If you make it unsigned long, it would be easier to switch to bitmap
APIs if needed in the future.
Sure, I can make it a unsigned long. But we'd need a lot more entries
to justify a switch to bitmap APIs...
+ IS_ENABLED(CONFIG_MACH_JZ4730) << ID_JZ4730 |
+ IS_ENABLED(CONFIG_MACH_JZ4740) << ID_JZ4740 |
+ IS_ENABLED(CONFIG_MACH_JZ4725B) << ID_JZ4725B |
+ IS_ENABLED(CONFIG_MACH_JZ4750) << ID_JZ4750 |
+ IS_ENABLED(CONFIG_MACH_JZ4755) << ID_JZ4755 |
+ IS_ENABLED(CONFIG_MACH_JZ4760) << ID_JZ4760 |
+ IS_ENABLED(CONFIG_MACH_JZ4770) << ID_JZ4770 |
+ IS_ENABLED(CONFIG_MACH_JZ4775) << ID_JZ4775 |
+ IS_ENABLED(CONFIG_MACH_JZ4780) << ID_JZ4780 |
+ IS_ENABLED(CONFIG_MACH_X1000) << ID_X1000 |
+ IS_ENABLED(CONFIG_MACH_X1500) << ID_X1500 |
+ IS_ENABLED(CONFIG_MACH_X1830) << ID_X1830 |
+ IS_ENABLED(CONFIG_MACH_X2000) << ID_X2000 |
+ IS_ENABLED(CONFIG_MACH_X2100) << ID_X2100;
...
+is_soc_or_above(const struct ingenic_pinctrl *jzpc, enum
jz_version version)
+{
+ return (enabled_socs >> version) &&
+ (!(enabled_socs & GENMASK((unsigned int)version -
1, 0))
Why casting? Why not use BIT()?
Casting just to be explicit about what we're doing here - I don't like
doing arithmetic on enums. But I can certainly remove it.
But these two lines seem like a very interesting way to reinvent the
test_bit().
I don't use BIT() or test_bit() because I am not checking a bit, but a
mask:
- (enabled_socs >> version) checks that the config supports either
(version) or anything above;
- !(enabled_socs & GENMASK(version - 1, 0)) checks that the config does
not support anything below. If true, the actual version check can be
skipped and the operation is a compile-time constant, and GCC will trim
the dead branches accordingly.
Cheers,
-Paul
If I'm mistaken, this all version code needs a good comment.
+ || jzpc->info->version >= version);
+}
--
With Best Regards,
Andy Shevchenko
