RE: [PATCH v4 1/2] sc16is7xx: spi interface is added

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Wed, 20 May 2015 17:54:43 +0530, Rama Kiran Kumar Indrakanti wrote:
>> spi interface for sc16is7xx is added along with Kconfig flag
>> to enable spi or i2c, thus in a instance we can have either
>> spi or i2c or both, in sync to the hw.
>>
>> Signed-off-by: Rama Kiran Kumar Indrakanti <indrakanti_ram@xxxxxxxxxxx>
>> ---
>>
>> Changes in v2:
>>  -Added seprate flags for i2c and spi
>>  -Added space in the comments lines
>>  -Added MODULE_ALIAS for spi interface
>>
>> Changes in v3:
>>  -Added Kconfig to avoid build errors
>>  -Modified Makefile to refelect Kconfig changes
>>
>> Changes in v4:
>>  -Change to Kconig to avoid build errors
>> ---
>>  drivers/tty/serial/Kconfig     | 34 ++++++++++++++++++--
>>  drivers/tty/serial/Makefile    |  2 +-
>>  drivers/tty/serial/sc16is7xx.c | 70 +++++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 101 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index f8120c1..67edd34 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -1179,15 +1179,43 @@ config SERIAL_SCCNXP_CONSOLE
>>      help
>>        Support for console on SCCNXP serial ports.
>>
>> +config SERIAL_SC16IS7XX_CORE
>> +    bool
>
> bool is wrong, it won't be possible to build as module.  Use tristate.

Yes, you are right.
But making a tristate would cause module build issues when both
the buses are selected together.

As module_spi_device and module_i2c_device would ultimately
#define to module_driver.

Which would cause duplication issue.

To avoid this, build error i would then need to make either one i2c or spi
bus interface to be tristate.

Thus making this something like this: where the serial is made tristate.

config SERIAL_SC16IS7XX_CORE
        tristate

config SERIAL_SC16IS7XX
        tristate "SC16IS7xx serial support"
        select SERIAL_CORE
        depends on I2C || SPI_MASTER
        help
          This selects support for SC16IS7xx serial ports.
          Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752,
          SC16IS760 and SC16IS762. Select supported buses using options below.

config SERIAL_SC16IS7XX_I2C
        bool "SC16IS7xx for I2C interface"
        depends on SERIAL_SC16IS7XX
        depends on I2C
        select SERIAL_SC16IS7XX_CORE
        select REGMAP_I2C if I2C
        default y
        help
          Enable SC16IS7xx driver on I2C bus,
          If required say y, and say n to i2c if not required,
          Enabled by default to support oldconfig.
          You must select at least one bus for the driver to be built.

config SERIAL_SC16IS7XX_SPI
        tristate "SC16IS7xx for spi interface"
        depends on SERIAL_SC16IS7XX
        depends on SPI_MASTER
        select SERIAL_SC16IS7XX_CORE
        select REGMAP_SPI if SPI_MASTER
        help
          Enable SC16IS7xx driver on SPI bus,
          If required say y, and say n to spi if not required,
          This is additional support to exsisting driver.
          You must select at least one bus for the driver to be built.

>
>> +    depends on SERIAL_SC16IS7XX_I2C || SERIAL_SC16IS7XX_SPI
>
> Remove this line.  It's no longer needed.

This is line is for makes sure that the driver is not build if any of the interfaces
are not selected.
This counters the comment below:
>
>> +
>>  config SERIAL_SC16IS7XX
>>      tristate "SC16IS7xx serial support"
>> -    depends on I2C
>>      select SERIAL_CORE
>> -    select REGMAP_I2C if I2C
>> +    depends on I2C || SPI_MASTER
>>      help
>>        This selects support for SC16IS7xx serial ports.
>>        Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752,
>> -      SC16IS760 and SC16IS762.
>> +      SC16IS760 and SC16IS762. Select supported buses using options below.
>> +
>> +config SERIAL_SC16IS7XX_I2C
>> +    bool "SC16IS7xx for I2C interface"
>> +    depends on SERIAL_SC16IS7XX
>> +    depends on I2C
>> +    select SERIAL_SC16IS7XX_CORE
>
> You need to say:
>
> select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
>

yes, this would help but a bit overkill, as we already said depends on
SERIAL_SC16IS7XX, and once we have the depends clear to go

> Otherwise SERIAL_SC16IS7XX_CORE core will always be built into the
> kernel and it won't be possible to build the driver as a module.
>
>> +    select REGMAP_I2C if I2C
>> +    default y
>> +    help
>> +      Enable SC16IS7xx driver on I2C bus,
>> +      If required say y, and say n to i2c if not required,
>> +      Enabled by default to support oldconfig.
>> +      You must select at least one bus for the driver to be built.
>> +
>> +config SERIAL_SC16IS7XX_SPI
>> +    bool "SC16IS7xx for spi interface"
>> +    depends on SERIAL_SC16IS7XX
>> +    depends on SPI_MASTER
>> +    select SERIAL_SC16IS7XX_CORE
>
> Same here.
>
>> +    select REGMAP_SPI if SPI_MASTER
>> +    help
>> +      Enable SC16IS7xx driver on SPI bus,
>> +      If required say y, and say n to spi if not required,
>> +      This is additional support to existing driver.
>> +      You must select at least one bus for the driver to be built.
>>
>>  config SERIAL_BFIN_SPORT
>>      tristate "Blackfin SPORT emulate UART"
> [...]
>> +module_spi_driver(sc16is7xx_spi_uart_driver);
>> +MODULE_ALIAS("spi:sc16is7xx");
>> +#endif
>> +
>> +#ifdef CONFIG_SERIAL_SC16IS7XX_I2C
>>  static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
>>                             const struct i2c_device_id *id)
>>  {
>> @@ -1255,7 +1323,7 @@ static struct i2c_driver sc16is7xx_i2c_uart_driver = {
>>  };
>>  module_i2c_driver(sc16is7xx_i2c_uart_driver);
>>  MODULE_ALIAS("i2c:sc16is7xx");
>> -
>> +#endif
>
> Minor nit - there is no need to remove the empty line.

Yeah, would take care.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


 		 	   		  --
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux