Messages in this thread | ![/](/images/icornerl.gif) | | From | Andy Shevchenko <> | Date | Wed, 13 Dec 2023 16:43:31 +0200 | Subject | Re: [PATCH 1/7] serial: sc16is7xx: fix snprintf format specifier in sc16is7xx_regmap_name() |
| |
On Tue, Dec 12, 2023 at 10:03 PM Hugo Villeneuve <hugo@hugovil.com> wrote: > On Thu, 7 Dec 2023 20:24:45 +0200 > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Thu, Dec 7, 2023 at 7:52 PM Hugo Villeneuve <hugo@hugovil.com> 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
| ![\](/images/icornerr.gif) |