On 28.03.2025 09:51:43, Biju Das wrote: > > Yes, it starts at bit 16, but at which bit does it end? > > > > The GENMASK() helps to document the register names. Of course is works if you replace the FIELD_PREP > > with a left shift, but you are replacing some meaningful information about the register name, register > > start bit and end bit by just a start bit value. See? You just lost the register name and end bit > > info. > > > > FIELD_PREP() is not only about doing the correct shift but also about documenting that you are putting > > the value into a specific register. > > > > When reading: > > > > FIELD_PREP(RCANFD_GERFL_EEF0_7, gpriv->channels_mask) > > > > I immediately understand that you are putting the gpriv->channels_mask value into the GERFL_EEF0_7 > > register and I can look at the datasheet for details about that GERFL_EEF0_7 if I want to. > > Gen4 has 8 channels (GENMASK(16, 23) > G3E has 6 channels (GENMASK(16, 21) > V4M has 4 channels (GENMASK(16, 19) > V3H_2 has 3 channels (GENMASK(16,18) > All other SoCs has 2 channels (GENMASK(16,17) > > So you mean, I should replace RCANFD_GERFL_EEF0_7 with a > > Generic one RCANFD_GERFL_EEF(x) GENMASK(16, 16 + (x) - 1) to handle > this differences > > Where x is the number of supported channels(info->max_channels) > > and then use > > FIELD_PREP(RCANFD_GERFL_EEF(x), gpriv->channels_mask) The mask for FIELD_PREP must be compile time constant. Geert recently posted a series to add non constant helpers: | https://lore.kernel.org/all/1824412519cb8791ab428065116927ee7b77cf35.1738329459.git.geert+renesas@xxxxxxxxx/ It seems it wasn't applied yet... Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
