On Wed, 31 May 2023 00:56:57 +0300 Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Tue, May 30, 2023 at 6:36 PM Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote: > > On Tue, 30 May 2023 01:38:17 +0300 > > andy.shevchenko@xxxxxxxxx wrote: > > > Mon, May 29, 2023 at 10:07:09AM -0400, Hugo Villeneuve kirjoitti: > > ... > > > > GENMASK() > > > > Ok done, altough even if in general I like the bit manipulation macros because they make the code easier to read/understand, I find it less obvious by using GENMASK in this case IMMO. > > GENMASK() was introduced to increase code robustness: > 1) to make sure the bits mentioned are correct > 2) to check the bit boundary. > > ... > > > > > + of_property_for_each_u32(dev->of_node, "nxp,modem-control-line-ports", > > > > + prop, p, u) { > > > > + if (u >= devtype->nr_uart) > > > > + continue; > > > > + > > > > + /* Use GPIO lines as modem control lines */ > > > > + if (u == 0) > > > > + mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_A_BIT; > > > > + else if (u == 1) > > > > + mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_B_BIT; > > > > + } > > > > > > Can we use device properties, please? > > > > I have converted this section to use device_property_count_u32() and device_property_read_u32_array(). Is that Ok? > > Yes, thank you! Hi Andy, now that I am using the device property API, I think I no longer have the need to test for "if (dev->of_node)" before reading the new property "nxp,modem-control-line-ports"? If that is the case, I will leave the test "if (dev->of_node)" only for the "irda-mode-ports" property. The pseudo code woulk look like this: if (dev->of_node) { struct property *prop; const __be32 *p; u32 u; of_property_for_each_u32(dev->of_node, "irda-mode-ports", prop, p, u) if (u < devtype->nr_uart) s->p[u].irda_mode = true; } /* Read "nxp,modem-control-line-ports" using device property API. */ sc16is7xx_setup_mctrl_ports(dev); Thank you, Hugo. > > > If you think about backporting to the earlier kernels (w/o properties in use in > > > this driver), perhaps an additional followup for that? > > > > I am not sure what you mean by this? > > If the device property API was not yet available for this fix being > backported to the old enough kernel we have to use old OF stuff. In > that case the device property conversion needs to be done in a > separate change. > > -- > With Best Regards, > Andy Shevchenko >