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.