On Tue, Apr 2, 2024 at 8:45 PM Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote:
>
> From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
>
> Split the common code from sc16is7xx driver and move the I2C and SPI bus
> parts into interface-specific source files.
>
> sc16is7xx becomes the core functions which can support multiple bus
> interfaces like I2C and SPI.
>
> No functional change intended.
...
> -config SERIAL_SC16IS7XX_CORE
> - tristate
> -
> config SERIAL_SC16IS7XX
> tristate "SC16IS7xx serial support"
> select SERIAL_CORE
> - depends on (SPI_MASTER && !I2C) || I2C
> + depends on SPI_MASTER || I2C
Is it?
> help
> Core driver for NXP SC16IS7xx serial ports.
> Supported ICs are:
> @@ -1042,22 +1039,18 @@ config SERIAL_SC16IS7XX
> drivers below.
>
> config SERIAL_SC16IS7XX_I2C
> - bool "SC16IS7xx for I2C interface"
> + tristate "SC16IS7xx for I2C interface"
> depends on SERIAL_SC16IS7XX
> depends on I2C
> - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> - select REGMAP_I2C if I2C
> - default y
> + select REGMAP_I2C
> help
> - Enable SC16IS7xx driver on I2C bus,
> - enabled by default to support oldconfig.
> + Enable SC16IS7xx driver on I2C bus.
>
> config SERIAL_SC16IS7XX_SPI
> - bool "SC16IS7xx for spi interface"
> + tristate "SC16IS7xx for SPI interface"
> depends on SERIAL_SC16IS7XX
> depends on SPI_MASTER
> - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> - select REGMAP_SPI if SPI_MASTER
> + select REGMAP_SPI
> help
> Enable SC16IS7xx driver on SPI bus.
Hmm... What I was thinking about is more like dropping
the SERIAL_SC16IS7XX and having I2C/SPI to select the core.
See many examples under drivers/iio on how it's done.
...
> +EXPORT_SYMBOL_GPL(sc16is74x_devtype);
Is it namespaced? Please make sure we are not polluting the global
namespace with these.
...
> +#ifndef _SC16IS7XX_H_
> +#define _SC16IS7XX_H_
> +
> +#include <linux/device.h>
Not used (by this file).
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +#include <linux/serial_core.h>
Not used.
> +#include <linux/types.h>
> +extern const struct of_device_id __maybe_unused sc16is7xx_dt_ids[];
No __maybe_unused. Just have it always be added.
> +const char *sc16is7xx_regmap_name(u8 port_id);
> +
> +unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id);
> +
> +int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
> + struct regmap *regmaps[], int irq);
> +void sc16is7xx_remove(struct device *dev);
Will require forward declaration
#include ...
struct device;
> +#endif /* _SC16IS7XX_H_ */
...
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
Follow the IWYU principle (include what you use).
...
> + return dev_err_probe(&i2c->dev, -ENODEV, "Failed to match device\n");
+ dev_printk.h
...
> +static int __init sc16is7xx_i2c_init(void)
> +{
> + return i2c_add_driver(&sc16is7xx_i2c_driver);
> +}
> +module_init(sc16is7xx_i2c_init);
> +
> +static void __exit sc16is7xx_i2c_exit(void)
> +{
> + i2c_del_driver(&sc16is7xx_i2c_driver);
> +}
> +module_exit(sc16is7xx_i2c_exit);
This is now module_i2c_driver().
...
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("SC16IS7xx I2C interface driver");
+ MODULE_IMPORT_NS()
...
> +++ b/drivers/tty/serial/sc16is7xx_spi.c
Similar/same comments as per i2c counterpart.
--
With Best Regards,
Andy Shevchenko
