Re: [PATCH 1/7] serial: sc16is7xx: fix snprintf format specifier in sc16is7xx_regmap_name()

[Andy Shevchenko]

On Tue, Dec 12, 2023 at 10:03 PM Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote:
> On Thu, 7 Dec 2023 20:24:45 +0200
> Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> > On Thu, Dec 7, 2023 at 7:52 PM Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote:

...

> > While at it, can you look at the following items to improve?
> > - sc16is7xx_alloc_line() can be updated to use IDA framework
> > - move return xxx; to the default cases in a few functions
> > - if (div > 0xffff) { --> if (div >= BIT(16)) { as it better shows why
> > the limit is that (we have only 16 bits for the divider)
> > - do {} while (0) in the sc16is7xx_port_irq, WTH?!
> > - while (1) { -- do { } while (keep_polling); in sc16is7xx_irq()
> > - use in_range() in sc16is7xx_setup_mctrl_ports() ? (maybe not, dunno)
> > - for (i--; i >= 0; i--) { --> while (i--) {
> > - use spi_get_device_match_data() and i2c_get_match_data()
> > - 15000000 --> 15 * HZ_PER_MHZ ?
> > - dropping MODULE_ALIAS (and fix the ID tables, _if_ needed)
> > - split the code to the core / main + SPI + I2C glue drivers
> >
> > * These just come on the first glance at the code, perhaps there is
> > more room to improve.
>
> Hi Andy,
> just to let you know that I have implemented almost all of the fixes /
> improvements. I will submit them once V2 of this current series
> lands in Greg's next tree.

Hooray!

> However, for sc16is7xx_alloc_line(), I looked at using the IDA framework
> but it doesn't seem possible because there is no IDA function
> to search if a bit is set, which is a needed functionality.

It can be done via trying to get it, but probably it's uglier than
current behaviour. Okay, let's leave it as is for now.

-- 
With Best Regards,
Andy Shevchenko