Re: [PATCH v4 7/9] serial: sc16is7xx: fix regression with GPIO configuration

[Andy Shevchenko]

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!

> > 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