Re: [PATCH V2 1/1] serial: 8250_pci: add RS485 for F81504/508/512
On Tue, 28 Jul 2015 11:59:24 +0800, Peter Hung wrote: > Add RS485 control for Fintek F81504/508/512 > > F81504/508/512 can control their RTS with H/W mode. > PCI configuration space for each port is 0x40 + idx * 8 + 7. > > When it set with 0x01, it's configured with RS232 mode. > RTS is controlled by MCR. > > When it set with 0x11, it's configured with RS485 mode. > RTS is controlled by H/W, RTS low with idle & RX, high with TX. > > When it set with 0x31, it's configured with RS485 mode. > RTS is controlled by H/W, RTS high with idle & RX, low with TX. > > We will force 0x01 on pci_fintek_setup(). > > Changelog: > V2 > 1. change direct bit operation with meaningful define name. > 2. due to F81504 series only support SER_RS485_ENABLED & >SER_RS485_RTS_ON_SEND. We'll clean non-support area of >struct serial_rs485. > 3. change control method of SER_RS485_RTS_ON_SEND. In our >reference circuit, the transceiver default mode needed >in rx mode with RTS logic high, tx mode with RTS logic low. > >If user set to SER_RS485_ENABLED(default), we should set >reg with 0x31. if SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND >will set reg to 0x11. > > Signed-off-by: Peter Hung Looks better, thanks. For future postings please put the changelog below the ---, we don't need it in the logs. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/1] serial: 8250_pci: add RS485 for F81504/508/512
On Tue, 28 Jul 2015 11:59:24 +0800, Peter Hung wrote: Add RS485 control for Fintek F81504/508/512 F81504/508/512 can control their RTS with H/W mode. PCI configuration space for each port is 0x40 + idx * 8 + 7. When it set with 0x01, it's configured with RS232 mode. RTS is controlled by MCR. When it set with 0x11, it's configured with RS485 mode. RTS is controlled by H/W, RTS low with idle RX, high with TX. When it set with 0x31, it's configured with RS485 mode. RTS is controlled by H/W, RTS high with idle RX, low with TX. We will force 0x01 on pci_fintek_setup(). Changelog: V2 1. change direct bit operation with meaningful define name. 2. due to F81504 series only support SER_RS485_ENABLED SER_RS485_RTS_ON_SEND. We'll clean non-support area of struct serial_rs485. 3. change control method of SER_RS485_RTS_ON_SEND. In our reference circuit, the transceiver default mode needed in rx mode with RTS logic high, tx mode with RTS logic low. If user set to SER_RS485_ENABLED(default), we should set reg with 0x31. if SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND will set reg to 0x11. Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com Looks better, thanks. For future postings please put the changelog below the ---, we don't need it in the logs. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] serial: 8250_pci: add RS485 for F81504/508/512
On Fri, 24 Jul 2015 13:55:39 +0800, Peter Hung wrote:
> Add RS485 control for Fintek F81504/508/512
>
> F81504/508/512 can control their RTS with H/W mode.
> PCI configuration space for each port is 0x40 + idx * 8 + 7.
>
> When it set with 0x01, it's configured with RS232 mode.
> RTS is controlled by MCR.
>
> When it set with 0x11, it's configured with RS485 mode.
> RTS is controlled by H/W, RTS high with idle & RX, low with TX.
>
> When it set with 0x31, it's configured with RS485 mode.
> RTS is controlled by H/W, RTS low with idle & RX, high with TX.
>
> We will force 0x01 on pci_fintek_setup().
>
> Signed-off-by: Peter Hung
> ---
> drivers/tty/serial/8250/8250_pci.c | 44
> ++
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_pci.c
> b/drivers/tty/serial/8250/8250_pci.c
> index e55f18b..36280fa 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1685,11 +1685,43 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
> return ret;
> }
>
> +static int pci_fintek_rs485_config(struct uart_port *port,
> +struct serial_rs485 *rs485)
> +{
> + u8 setting;
> + u8 *index = (u8 *) port->private_data;
> + struct pci_dev *pci_dev = container_of(port->dev, struct pci_dev,
> + dev);
This looks misaligned. 'dev' should be aligned with opening
parenthesis.
> +
> + pci_read_config_byte(pci_dev, 0x40 + 8 * *index + 7, );
> +
> + if (rs485->flags & SER_RS485_ENABLED) {
> + /* Enable RTS H/W control mode */
> + setting |= BIT(4);
Please add defines with the bit names.
> +
> + if (rs485->flags & SER_RS485_RTS_ON_SEND) {
> + /* RTS driving high on TX */
> + setting |= BIT(5);
> + } else {
> + /* RTS driving low on TX */
> + setting &= ~BIT(5);
> + }
> + } else {
> + /* Disable RTS H/W control mode */
> + setting &= ~(BIT(4) | BIT(5));
> + }
Please make sure you correct the rs485 configuration with what you can
actually support. Look at 8250_lpc18xx.c as an example. In your case
when the function returns it should have SER_RS485_ENABLED and one of
SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND set and nothing else
(or be completely zeroed if SER_RS485_ENABLED was not set).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] serial: 8250_pci: add RS485 for F81504/508/512
On Fri, 24 Jul 2015 13:55:39 +0800, Peter Hung wrote:
Add RS485 control for Fintek F81504/508/512
F81504/508/512 can control their RTS with H/W mode.
PCI configuration space for each port is 0x40 + idx * 8 + 7.
When it set with 0x01, it's configured with RS232 mode.
RTS is controlled by MCR.
When it set with 0x11, it's configured with RS485 mode.
RTS is controlled by H/W, RTS high with idle RX, low with TX.
When it set with 0x31, it's configured with RS485 mode.
RTS is controlled by H/W, RTS low with idle RX, high with TX.
We will force 0x01 on pci_fintek_setup().
Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com
---
drivers/tty/serial/8250/8250_pci.c | 44
++
1 file changed, 44 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_pci.c
b/drivers/tty/serial/8250/8250_pci.c
index e55f18b..36280fa 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1685,11 +1685,43 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
return ret;
}
+static int pci_fintek_rs485_config(struct uart_port *port,
+struct serial_rs485 *rs485)
+{
+ u8 setting;
+ u8 *index = (u8 *) port-private_data;
+ struct pci_dev *pci_dev = container_of(port-dev, struct pci_dev,
+ dev);
This looks misaligned. 'dev' should be aligned with opening
parenthesis.
+
+ pci_read_config_byte(pci_dev, 0x40 + 8 * *index + 7, setting);
+
+ if (rs485-flags SER_RS485_ENABLED) {
+ /* Enable RTS H/W control mode */
+ setting |= BIT(4);
Please add defines with the bit names.
+
+ if (rs485-flags SER_RS485_RTS_ON_SEND) {
+ /* RTS driving high on TX */
+ setting |= BIT(5);
+ } else {
+ /* RTS driving low on TX */
+ setting = ~BIT(5);
+ }
+ } else {
+ /* Disable RTS H/W control mode */
+ setting = ~(BIT(4) | BIT(5));
+ }
Please make sure you correct the rs485 configuration with what you can
actually support. Look at 8250_lpc18xx.c as an example. In your case
when the function returns it should have SER_RS485_ENABLED and one of
SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND set and nothing else
(or be completely zeroed if SER_RS485_ENABLED was not set).
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH linux-next 1/4] ARM: at91/dt: add new DT properties for Atmel usart
On Tue, 2 Jun 2015 16:18:21 +0200, Cyrille Pitchen wrote: > +- atmel,rts-low-threshold: when the RX FIFO level, ie the number of data > + available to be read from the RX FIFO, crosses down this threshold the RTS > + line is driven to low level to tell the remote peer that it can (re)start > + sending new data. > +- atmel,rts-high-threshold: when the RX FIFO level crosses up this threshold, > + the RTS line is driven to high level to tell the remote peer that it should > + stop sending new data. Maintainers, are there any guidelines for what is appropriate to put into DT? The parameters above look like they could be programmed at runtime through some user space API like sysfs. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH linux-next 1/4] ARM: at91/dt: add new DT properties for Atmel usart
On Tue, 2 Jun 2015 16:18:21 +0200, Cyrille Pitchen wrote: +- atmel,rts-low-threshold: when the RX FIFO level, ie the number of data + available to be read from the RX FIFO, crosses down this threshold the RTS + line is driven to low level to tell the remote peer that it can (re)start + sending new data. +- atmel,rts-high-threshold: when the RX FIFO level crosses up this threshold, + the RTS line is driven to high level to tell the remote peer that it should + stop sending new data. Maintainers, are there any guidelines for what is appropriate to put into DT? The parameters above look like they could be programmed at runtime through some user space API like sysfs. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] tty: serial: fsl_lpuart: Add support for RS-485
On Fri, 29 May 2015 13:35:54 +0530, Bhuvanchandra DV wrote:
> Enable Vybrid's build-in support for RS-485 auto RTS for
> controlling line direction of RS-485 transceiver driver.
>
> Signed-off-by: Bhuvanchandra DV
> ---
> drivers/tty/serial/fsl_lpuart.c | 60
> +
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 357f623..c553b14 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -820,6 +820,50 @@ static unsigned int lpuart32_tx_empty(struct uart_port
> *port)
> TIOCSER_TEMT : 0;
> }
>
> +static int lpuart_config_rs485(struct uart_port *port,
> + struct serial_rs485 *rs485)
> +{
> + struct lpuart_port *sport = container_of(port,
> + struct lpuart_port, port);
> + u8 modem = readb(sport->port.membase + UARTMODEM) &
> + ~(UARTMODEM_TXRTSPOL | UARTMODEM_TXRTSE);
Please put empty line between variables and code.
> + writeb(modem, sport->port.membase + UARTMODEM);
> +
> + if (rs485->flags & SER_RS485_ENABLED) {
> + /* Enable auto RS-485 RTS mode */
> + modem |= UARTMODEM_TXRTSE;
> +
> + /*
> + * RTS needs to be logic HIGH either during transer _or_ after
> + * transfer, other variants are not supported by the hardware.
> + */
Indentation is off here. '*' should be aligned.
> + if (!(rs485->flags & (SER_RS485_RTS_ON_SEND |
> + SER_RS485_RTS_AFTER_SEND)))
and here - things should be aligned on the containing bracket (SER_
under SER_ here).
> + rs485->flags |= SER_RS485_RTS_ON_SEND;
> +
> + if (rs485->flags & SER_RS485_RTS_ON_SEND &&
> + rs485->flags & SER_RS485_RTS_AFTER_SEND)
and here - same bracket rule (rs485-> under rs485->)
> + rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
> +
> + /*
> + * The hardware defaults to RTS logic HIGH while transfer.
> + * Switch polarity in case RTS shall be logic HIGH
> + * after transfer.
> + * Note: UART is assumed to be active high.
> + */
> + if (rs485->flags & SER_RS485_RTS_ON_SEND)
> + modem &= ~UARTMODEM_TXRTSPOL;
UARTMODEM_TXRTSPOL was already clear when you read modem.
> + else if (rs485->flags & SER_RS485_RTS_AFTER_SEND)
> + modem |= UARTMODEM_TXRTSPOL;
> + }
> +
> + /* Store the new configuration */
> + sport->port.rs485 = *rs485;
> +
> + writeb(modem, sport->port.membase + UARTMODEM);
> + return 0;
> +}
> +
> static unsigned int lpuart_get_mctrl(struct uart_port *port)
> {
> return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
[...]
> @@ -1802,6 +1854,14 @@ static int lpuart_probe(struct platform_device *pdev)
> dev_info(sport->port.dev, "DMA rx channel request failed, "
> "operating without rx DMA\n");
>
> + if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time")) {
> + sport->port.rs485.flags |= SER_RS485_ENABLED;
> + sport->port.rs485.flags |= SER_RS485_RTS_ON_SEND;
> + writeb(UARTMODEM_TXRTSE, sport->port.membase + UARTMODEM);
> + } else {
> + sport->port.rs485.flags &= ~SER_RS485_ENABLED;
Why the need to clear the flag? sport was kzalloc'ed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] tty: serial: fsl_lpuart: remove RTS/CTS control from set/get_mctrl
On Fri, 29 May 2015 13:35:53 +0530, Bhuvanchandra DV wrote: > The LPUART does not provide manual control of RTS/CTS signals, > those can only be controlled by the hardware directly. Therefore > manual control of those signals through mctrl can not be provided. > The current implementation enables/disables the automatic control, > which is not what mctrl should do, hence remove the incorrect > implementation. > > Signed-off-by: Bhuvanchandra DV Now that the functions do nothing there is probably no point in keeping separate lpuart/lpuart32 variants. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] tty: serial: fsl_lpuart: remove RTS/CTS control from set/get_mctrl
On Fri, 29 May 2015 13:35:53 +0530, Bhuvanchandra DV wrote: The LPUART does not provide manual control of RTS/CTS signals, those can only be controlled by the hardware directly. Therefore manual control of those signals through mctrl can not be provided. The current implementation enables/disables the automatic control, which is not what mctrl should do, hence remove the incorrect implementation. Signed-off-by: Bhuvanchandra DV bhuvanchandra...@toradex.com Now that the functions do nothing there is probably no point in keeping separate lpuart/lpuart32 variants. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] tty: serial: fsl_lpuart: Add support for RS-485
On Fri, 29 May 2015 13:35:54 +0530, Bhuvanchandra DV wrote:
Enable Vybrid's build-in support for RS-485 auto RTS for
controlling line direction of RS-485 transceiver driver.
Signed-off-by: Bhuvanchandra DV bhuvanchandra...@toradex.com
---
drivers/tty/serial/fsl_lpuart.c | 60
+
1 file changed, 60 insertions(+)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 357f623..c553b14 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -820,6 +820,50 @@ static unsigned int lpuart32_tx_empty(struct uart_port
*port)
TIOCSER_TEMT : 0;
}
+static int lpuart_config_rs485(struct uart_port *port,
+ struct serial_rs485 *rs485)
+{
+ struct lpuart_port *sport = container_of(port,
+ struct lpuart_port, port);
+ u8 modem = readb(sport-port.membase + UARTMODEM)
+ ~(UARTMODEM_TXRTSPOL | UARTMODEM_TXRTSE);
Please put empty line between variables and code.
+ writeb(modem, sport-port.membase + UARTMODEM);
+
+ if (rs485-flags SER_RS485_ENABLED) {
+ /* Enable auto RS-485 RTS mode */
+ modem |= UARTMODEM_TXRTSE;
+
+ /*
+ * RTS needs to be logic HIGH either during transer _or_ after
+ * transfer, other variants are not supported by the hardware.
+ */
Indentation is off here. '*' should be aligned.
+ if (!(rs485-flags (SER_RS485_RTS_ON_SEND |
+ SER_RS485_RTS_AFTER_SEND)))
and here - things should be aligned on the containing bracket (SER_
under SER_ here).
+ rs485-flags |= SER_RS485_RTS_ON_SEND;
+
+ if (rs485-flags SER_RS485_RTS_ON_SEND
+ rs485-flags SER_RS485_RTS_AFTER_SEND)
and here - same bracket rule (rs485- under rs485-)
+ rs485-flags = ~SER_RS485_RTS_AFTER_SEND;
+
+ /*
+ * The hardware defaults to RTS logic HIGH while transfer.
+ * Switch polarity in case RTS shall be logic HIGH
+ * after transfer.
+ * Note: UART is assumed to be active high.
+ */
+ if (rs485-flags SER_RS485_RTS_ON_SEND)
+ modem = ~UARTMODEM_TXRTSPOL;
UARTMODEM_TXRTSPOL was already clear when you read modem.
+ else if (rs485-flags SER_RS485_RTS_AFTER_SEND)
+ modem |= UARTMODEM_TXRTSPOL;
+ }
+
+ /* Store the new configuration */
+ sport-port.rs485 = *rs485;
+
+ writeb(modem, sport-port.membase + UARTMODEM);
+ return 0;
+}
+
static unsigned int lpuart_get_mctrl(struct uart_port *port)
{
return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
[...]
@@ -1802,6 +1854,14 @@ static int lpuart_probe(struct platform_device *pdev)
dev_info(sport-port.dev, DMA rx channel request failed,
operating without rx DMA\n);
+ if (of_property_read_bool(np, linux,rs485-enabled-at-boot-time)) {
+ sport-port.rs485.flags |= SER_RS485_ENABLED;
+ sport-port.rs485.flags |= SER_RS485_RTS_ON_SEND;
+ writeb(UARTMODEM_TXRTSE, sport-port.membase + UARTMODEM);
+ } else {
+ sport-port.rs485.flags = ~SER_RS485_ENABLED;
Why the need to clear the flag? sport was kzalloc'ed.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Thu, 14 May 2015 14:45:47 +0530, ram kiran wrote: > > I know little about kbuild but I'm worried that someone doing oldconfig > > can still get SERIAL_SC16IS7XX selected while saying no to all the > > others. > > > > Other option would be to swap the names between SERIAL_SC16IS7XX and > > SERIAL_SC16IS7XX_SELECT, oldconfig would run smoother. > I think, with the above, there would need a configuration change for sure. > It should be okay, as I2C is default Y. Exactly, but with what you proposed we need a configuration change as well, no? SERIAL_SC16IS7XX_SELECT is new so users would have to know that it's what SERIAL_SC16IS7XX used to be. > Swap names would need Makefile changes, i was just thinking to avoid this. > obj-$(CONFIG_SERIAL_SC16IS7XX) += sc16is7xx.o > would be > obj-$(CONFIG_SERIAL_SC16IS7XX_SELECT) += sc16is7xx.o > > I think its some that need not be there. Do suggest.. Perhaps *_SELECT is not the best name then but we could use something like *_CORE or *_BASE. Changes to the Makefile are not user-visible so no worries. It would be nice if people who run oldconfig by default got the same behaviour as they did so far (i2c if SC16IS7XX was enabled in previous config). I think with names swapped and modification of Makefile we would get exactly that. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Thu, 14 May 2015 14:45:47 +0530, ram kiran wrote: > > I know little about kbuild but I'm worried that someone doing oldconfig > > can still get SERIAL_SC16IS7XX selected while saying no to all the > > others. > > > > Other option would be to swap the names between SERIAL_SC16IS7XX and > > SERIAL_SC16IS7XX_SELECT, oldconfig would run smoother. > I think, with the above, there would need a configuration change for sure. > It should be okay, as I2C is default Y. Exactly, but with what you proposed we need a configuration change as well, no? SERIAL_SC16IS7XX_SELECT is new so users would have to know that it's what SERIAL_SC16IS7XX used to be. > Swap names would need Makefile changes, i was just thinking to avoid this. > obj-$(CONFIG_SERIAL_SC16IS7XX) += sc16is7xx.o > would be > obj-$(CONFIG_SERIAL_SC16IS7XX_SELECT) += sc16is7xx.o > > I think its some that need not be there. Do suggest.. Perhaps *_SELECT is not the best name then but we could use something like *_CORE or *_BASE. Changes to the Makefile are not user-visible so no worries. It would be nice if people who run oldconfig by default got the same behaviour as they did so far (i2c if SC16IS7XX was enabled in previous config). I think with names swapped and modification of Makefile we would get exactly that. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Thu, 14 May 2015 13:16:20 +0530, ram kiran wrote: > > On Wed, 13 May 2015 16:27:58 +0530, ram.i hcltech 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: ram.i hcltech > >> --- > >> > >> Changes in v2: > >> -Added seprate flags for i2c and spi > >> -Added space in the comments lines > >> -Added MODULE_ALIAS for spi interface > >> --- > >> drivers/tty/serial/Kconfig | 27 +++-- > >> drivers/tty/serial/sc16is7xx.c | 69 > >> +- > >> 2 files changed, 92 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > >> index f8120c1..8c505b2 100644 > >> --- a/drivers/tty/serial/Kconfig > >> +++ b/drivers/tty/serial/Kconfig > >> @@ -1181,13 +1181,34 @@ config SERIAL_SCCNXP_CONSOLE > >> > To avoid error or warning on build, i think this would be the probable > solution. > I thinking to go with this, any comments on this please. > > config SERIAL_SC16IS7XX > bool > > config SERIAL_SC16IS7XX_SELECT > tristate "SC16IS7xx serial support" > select SERIAL_CORE > depends on I2C || SPI_MASTER > select REGMAP_I2C if I2C > select REGMAP_SPI if 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_SELECT > select SERIAL_SC16IS7XX > default y > help > Enable SC16IS7xx driver on I2C bus. > > config SERIAL_SC16IS7XX_SPI > bool "SC16IS7xx for spi interface" > depends on SERIAL_SC16IS7XX_SELECT > select SERIAL_SC16IS7XX > help > Enable SC16IS7xx driver on SPI bus. > This looks quite elegant! Should we aslo make SERIAL_SC16IS7XX depend on SERIAL_SC16IS7XX_I2C || SERIAL_SC16IS7XX_SPI? Would that work? I know little about kbuild but I'm worried that someone doing oldconfig can still get SERIAL_SC16IS7XX selected while saying no to all the others. Other option would be to swap the names between SERIAL_SC16IS7XX and SERIAL_SC16IS7XX_SELECT, oldconfig would run smoother. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Thu, 14 May 2015 13:16:20 +0530, ram kiran wrote: On Wed, 13 May 2015 16:27:58 +0530, ram.i hcltech 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: ram.i hcltech indrakanti_...@hotmail.com --- Changes in v2: -Added seprate flags for i2c and spi -Added space in the comments lines -Added MODULE_ALIAS for spi interface --- drivers/tty/serial/Kconfig | 27 +++-- drivers/tty/serial/sc16is7xx.c | 69 +- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index f8120c1..8c505b2 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -1181,13 +1181,34 @@ config SERIAL_SCCNXP_CONSOLE To avoid error or warning on build, i think this would be the probable solution. I thinking to go with this, any comments on this please. config SERIAL_SC16IS7XX bool config SERIAL_SC16IS7XX_SELECT tristate SC16IS7xx serial support select SERIAL_CORE depends on I2C || SPI_MASTER select REGMAP_I2C if I2C select REGMAP_SPI if 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_SELECT select SERIAL_SC16IS7XX default y help Enable SC16IS7xx driver on I2C bus. config SERIAL_SC16IS7XX_SPI bool SC16IS7xx for spi interface depends on SERIAL_SC16IS7XX_SELECT select SERIAL_SC16IS7XX help Enable SC16IS7xx driver on SPI bus. This looks quite elegant! Should we aslo make SERIAL_SC16IS7XX depend on SERIAL_SC16IS7XX_I2C || SERIAL_SC16IS7XX_SPI? Would that work? I know little about kbuild but I'm worried that someone doing oldconfig can still get SERIAL_SC16IS7XX selected while saying no to all the others. Other option would be to swap the names between SERIAL_SC16IS7XX and SERIAL_SC16IS7XX_SELECT, oldconfig would run smoother. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Thu, 14 May 2015 14:45:47 +0530, ram kiran wrote: I know little about kbuild but I'm worried that someone doing oldconfig can still get SERIAL_SC16IS7XX selected while saying no to all the others. Other option would be to swap the names between SERIAL_SC16IS7XX and SERIAL_SC16IS7XX_SELECT, oldconfig would run smoother. I think, with the above, there would need a configuration change for sure. It should be okay, as I2C is default Y. Exactly, but with what you proposed we need a configuration change as well, no? SERIAL_SC16IS7XX_SELECT is new so users would have to know that it's what SERIAL_SC16IS7XX used to be. Swap names would need Makefile changes, i was just thinking to avoid this. obj-$(CONFIG_SERIAL_SC16IS7XX) += sc16is7xx.o would be obj-$(CONFIG_SERIAL_SC16IS7XX_SELECT) += sc16is7xx.o I think its some that need not be there. Do suggest.. Perhaps *_SELECT is not the best name then but we could use something like *_CORE or *_BASE. Changes to the Makefile are not user-visible so no worries. It would be nice if people who run oldconfig by default got the same behaviour as they did so far (i2c if SC16IS7XX was enabled in previous config). I think with names swapped and modification of Makefile we would get exactly that. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Thu, 14 May 2015 14:45:47 +0530, ram kiran wrote: I know little about kbuild but I'm worried that someone doing oldconfig can still get SERIAL_SC16IS7XX selected while saying no to all the others. Other option would be to swap the names between SERIAL_SC16IS7XX and SERIAL_SC16IS7XX_SELECT, oldconfig would run smoother. I think, with the above, there would need a configuration change for sure. It should be okay, as I2C is default Y. Exactly, but with what you proposed we need a configuration change as well, no? SERIAL_SC16IS7XX_SELECT is new so users would have to know that it's what SERIAL_SC16IS7XX used to be. Swap names would need Makefile changes, i was just thinking to avoid this. obj-$(CONFIG_SERIAL_SC16IS7XX) += sc16is7xx.o would be obj-$(CONFIG_SERIAL_SC16IS7XX_SELECT) += sc16is7xx.o I think its some that need not be there. Do suggest.. Perhaps *_SELECT is not the best name then but we could use something like *_CORE or *_BASE. Changes to the Makefile are not user-visible so no worries. It would be nice if people who run oldconfig by default got the same behaviour as they did so far (i2c if SC16IS7XX was enabled in previous config). I think with names swapped and modification of Makefile we would get exactly that. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Wed, 13 May 2015 16:27:58 +0530, ram.i hcltech 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: ram.i hcltech > --- > > Changes in v2: > -Added seprate flags for i2c and spi > -Added space in the comments lines > -Added MODULE_ALIAS for spi interface > --- > drivers/tty/serial/Kconfig | 27 +++-- > drivers/tty/serial/sc16is7xx.c | 69 > +- > 2 files changed, 92 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > index f8120c1..8c505b2 100644 > --- a/drivers/tty/serial/Kconfig > +++ b/drivers/tty/serial/Kconfig > @@ -1181,13 +1181,34 @@ config SERIAL_SCCNXP_CONSOLE > > config SERIAL_SC16IS7XX > tristate "SC16IS7xx serial support" > - depends on I2C Please keep the dependency like this: depends on I2C || SPI_MASTER (or SPI, I don't know what's the difference. SPI seems fine.) > select SERIAL_CORE > - select REGMAP_I2C if I2C > help > This selects support for SC16IS7xx serial ports. > Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752, > - SC16IS760 and SC16IS762. > + SC16IS760 and SC16IS762, over i2c or spi. > + select at least one of the i2c or spi interface. I would phrase the help message like this: 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" Please add "default y" to minimize oldconfig pain for those already using the driver. > + depends on SERIAL_SC16IS7XX=y Why =y? > + depends on I2C > + select REGMAP_I2C if I2C > + help > + to enable i2c interface for SC16IS7XX, say Y, > + Otherwise, for i2c say N. > + this would make the driver to interface over SPI and I2C would > + be diabled. I would phrase it simply like this: Enable SC16IS7xx driver on I2C bus. > +config SERIAL_SC16IS7XX_SPI > + bool "SC16IS7xx for spi interface" > + depends on SERIAL_SC16IS7XX > + depends on SPI_MASTER Right now it is possible to select the driver without any bus-specific option being set. I don't see an easy way to avoid this but please make sure that there are no build failures/warnings in this scenario. You should also extend the binding information to include the new SPI interface (Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt) Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] sc16is7xx: spi interface is added
On Wed, 13 May 2015 16:27:58 +0530, ram.i hcltech 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: ram.i hcltech indrakanti_...@hotmail.com --- Changes in v2: -Added seprate flags for i2c and spi -Added space in the comments lines -Added MODULE_ALIAS for spi interface --- drivers/tty/serial/Kconfig | 27 +++-- drivers/tty/serial/sc16is7xx.c | 69 +- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index f8120c1..8c505b2 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -1181,13 +1181,34 @@ config SERIAL_SCCNXP_CONSOLE config SERIAL_SC16IS7XX tristate SC16IS7xx serial support - depends on I2C Please keep the dependency like this: depends on I2C || SPI_MASTER (or SPI, I don't know what's the difference. SPI seems fine.) select SERIAL_CORE - select REGMAP_I2C if I2C help This selects support for SC16IS7xx serial ports. Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752, - SC16IS760 and SC16IS762. + SC16IS760 and SC16IS762, over i2c or spi. + select at least one of the i2c or spi interface. I would phrase the help message like this: 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 Please add default y to minimize oldconfig pain for those already using the driver. + depends on SERIAL_SC16IS7XX=y Why =y? + depends on I2C + select REGMAP_I2C if I2C + help + to enable i2c interface for SC16IS7XX, say Y, + Otherwise, for i2c say N. + this would make the driver to interface over SPI and I2C would + be diabled. I would phrase it simply like this: Enable SC16IS7xx driver on I2C bus. +config SERIAL_SC16IS7XX_SPI + bool SC16IS7xx for spi interface + depends on SERIAL_SC16IS7XX + depends on SPI_MASTER Right now it is possible to select the driver without any bus-specific option being set. I don't see an easy way to avoid this but please make sure that there are no build failures/warnings in this scenario. You should also extend the binding information to include the new SPI interface (Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt) Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rt2x00: BUG: remove double loop on REGISTER_BUSY_COUNT
On Fri, 4 Apr 2014 10:19:09 +0200, Stanislaw Gruszka wrote: > On Thu, Apr 03, 2014 at 05:37:01PM +0200, Jakub Kiciński wrote: > > On Thu, 3 Apr 2014 16:12:07 +0200, Richard Genoud wrote: > > > rt2x00usb_register_read_lock() calls rt2x00usb_vendor_req_buff_lock() > > > that calls rt2x00usb_vendor_request() which is already looping up to > > > REGISTER_BUSY_COUNT times. > > > > > > So this loop is not needed. > > > > Not true. rt2x00usb_vendor_request() busy-waits for usb_control_msg() > > to succeed, rt2x00usb_register_read_lock() busy-waits for the register > > field itself to become 0. > > Yeah, but still we are looping REGISTER_BUSY_COUNT*REGISTER_BUSY_COUNT > what seems to be far too long. Yes, the busy waiting itself takes roughly 1s (100*100*100us) and then there are transfer times, so it might be too long indeed. Vendor driver waits only 10 * 5ms in RTUSB_VendorRequest() so rt2x00usb_vendor_request() seems like a better place to cut down the number of loops. Alternatively we could make rt2x00usb_regbusy_read() check the retval from rt2x00usb_vendor_request() and exit early? -- kuba -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rt2x00: BUG: remove double loop on REGISTER_BUSY_COUNT
On Fri, 4 Apr 2014 10:19:09 +0200, Stanislaw Gruszka wrote: On Thu, Apr 03, 2014 at 05:37:01PM +0200, Jakub Kiciński wrote: On Thu, 3 Apr 2014 16:12:07 +0200, Richard Genoud wrote: rt2x00usb_register_read_lock() calls rt2x00usb_vendor_req_buff_lock() that calls rt2x00usb_vendor_request() which is already looping up to REGISTER_BUSY_COUNT times. So this loop is not needed. Not true. rt2x00usb_vendor_request() busy-waits for usb_control_msg() to succeed, rt2x00usb_register_read_lock() busy-waits for the register field itself to become 0. Yeah, but still we are looping REGISTER_BUSY_COUNT*REGISTER_BUSY_COUNT what seems to be far too long. Yes, the busy waiting itself takes roughly 1s (100*100*100us) and then there are transfer times, so it might be too long indeed. Vendor driver waits only 10 * 5ms in RTUSB_VendorRequest() so rt2x00usb_vendor_request() seems like a better place to cut down the number of loops. Alternatively we could make rt2x00usb_regbusy_read() check the retval from rt2x00usb_vendor_request() and exit early? -- kuba -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rt2x00: BUG: remove double loop on REGISTER_BUSY_COUNT
On Thu, 3 Apr 2014 16:12:07 +0200, Richard Genoud wrote: > rt2x00usb_register_read_lock() calls rt2x00usb_vendor_req_buff_lock() > that calls rt2x00usb_vendor_request() which is already looping up to > REGISTER_BUSY_COUNT times. > > So this loop is not needed. Not true. rt2x00usb_vendor_request() busy-waits for usb_control_msg() to succeed, rt2x00usb_register_read_lock() busy-waits for the register field itself to become 0. Also, how would this be a BUG? -- kuba -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rt2x00: BUG: remove double loop on REGISTER_BUSY_COUNT
On Thu, 3 Apr 2014 16:12:07 +0200, Richard Genoud wrote: rt2x00usb_register_read_lock() calls rt2x00usb_vendor_req_buff_lock() that calls rt2x00usb_vendor_request() which is already looping up to REGISTER_BUSY_COUNT times. So this loop is not needed. Not true. rt2x00usb_vendor_request() busy-waits for usb_control_msg() to succeed, rt2x00usb_register_read_lock() busy-waits for the register field itself to become 0. Also, how would this be a BUG? -- kuba -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
