Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH] serial: 8250: Add CAP_MINI, set for bcm2835aux
From: Eric Anholt @ 2017-05-19 18:09 UTC (permalink / raw)
  To: Phil Elwell, Greg Kroah-Hartman, Stefan Wahren, linux-serial,
	linux-rpi-kernel, linux-arm-kernel
In-Reply-To: <8d9c605a-bff7-607c-2c5b-4ec75e68ac2d@raspberrypi.org>


[-- Attachment #1.1: Type: text/plain, Size: 820 bytes --]

Phil Elwell <phil@raspberrypi.org> writes:

> The AUX/mini-UART in the BCM2835 family of procesors is a cut-down
> 8250 clone. In particular it is lacking support for the following
> features: CSTOPB PARENB PARODD CMSPAR CS5 CS6
>
> Add a new capability (UART_CAP_MINI) that exposes the restrictions to
> the user of the termios API by turning off the unsupported features in
> the request.
>
> N.B. It is almost possible to automatically discover the missing
> features by reading back the LCR register, but the CSIZE bits don't
> cooperate (contrary to the documentation, both bits are significant,
> but CS5 and CS6 are mapped to CS7) and the code is much longer.
>
> See: https://github.com/raspberrypi/linux/issues/1561
>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] serial: 8250: Add CAP_MINI, set for bcm2835aux
From: Phil Elwell @ 2017-05-19 15:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Eric Anholt, Stefan Wahren,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

The AUX/mini-UART in the BCM2835 family of procesors is a cut-down
8250 clone. In particular it is lacking support for the following
features: CSTOPB PARENB PARODD CMSPAR CS5 CS6

Add a new capability (UART_CAP_MINI) that exposes the restrictions to
the user of the termios API by turning off the unsupported features in
the request.

N.B. It is almost possible to automatically discover the missing
features by reading back the LCR register, but the CSIZE bits don't
cooperate (contrary to the documentation, both bits are significant,
but CS5 and CS6 are mapped to CS7) and the code is much longer.

See: https://github.com/raspberrypi/linux/issues/1561

Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
---
 drivers/tty/serial/8250/8250.h            | 3 +++
 drivers/tty/serial/8250/8250_bcm2835aux.c | 2 +-
 drivers/tty/serial/8250/8250_port.c       | 6 ++++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ce8d4ff..b2bdc35 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -81,6 +81,9 @@ struct serial8250_config {
 #define UART_CAP_HFIFO	(1 << 14)	/* UART has a "hidden" FIFO */
 #define UART_CAP_RPM	(1 << 15)	/* Runtime PM is active while idle */
 #define UART_CAP_IRDA	(1 << 16)	/* UART supports IrDA line discipline */
+#define UART_CAP_MINI	(1 << 17)	/* Mini UART on BCM283X family lacks:
+					 * STOP PARITY EPAR SPAR WLEN5 WLEN6
+					 */
 
 #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
 #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index e10f124..a23c7da 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -39,7 +39,7 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 
 	/* initialize data */
 	spin_lock_init(&data->uart.port.lock);
-	data->uart.capabilities = UART_CAP_FIFO;
+	data->uart.capabilities = UART_CAP_FIFO | UART_CAP_MINI;
 	data->uart.port.dev = &pdev->dev;
 	data->uart.port.regshift = 2;
 	data->uart.port.type = PORT_16550;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 09a65a3..dd218a1 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2584,6 +2584,12 @@ static unsigned int serial8250_get_baud_rate(struct uart_port *port,
 	unsigned long flags;
 	unsigned int baud, quot, frac = 0;
 
+	if (up->capabilities & UART_CAP_MINI) {
+		termios->c_cflag &= ~(CSTOPB | PARENB | PARODD | CMSPAR);
+		if ((termios->c_cflag & CSIZE) == CS5 ||
+		    (termios->c_cflag & CSIZE) == CS6)
+			termios->c_cflag = (termios->c_cflag & ~CSIZE) | CS7;
+	}
 	cval = serial8250_compute_lcr(up, termios->c_cflag);
 
 	baud = serial8250_get_baud_rate(port, termios, old);
-- 
1.9.1

^ permalink raw reply related

* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-19 15:07 UTC (permalink / raw)
  To: Nikita Yushchenko
  Cc: A.S. Dong, linux-serial@vger.kernel.org, Andy Duan,
	gregkh@linuxfoundation.org, Y.B. Lu, linux-kernel@vger.kernel.org,
	stefan@agner.ch, Mingkai Hu, jslaby@suse.com,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <045d1888-4a23-cf73-ab4c-009e29fde974@cogentembedded.com>

On Wed, May 17, 2017 at 11:04:04AM +0300, Nikita Yushchenko wrote:
> Hi
> 
> My view of your statement is:
> - you currently assume only a few cases for this driver - builtin UART
> in vf610, ls1012a and imx7,
> - in each of these cases, all lpuart instances share same endian, thus
> having that in global var works for these cases,
> - having that in global var makes it possible for you to write less
> lines of code
> 
> My complain is:
> - in Linux, we are trying to keep drivers generic,
> - in Linux, having less lines of code has never been sufficient to break
> basic data structure consistency,
> - having driver to keep per-device capability in global var is a clear
> case of breaking consistency.
> 

Yes, i do understand your concern and i absolutely agree with the rule
you mentioned.

> 
> >>> That's for special case, normally we wouldn't do that.
> >>
> >> For me this "special case" looks like "let's break data structure
> >> consistency to reuse several lines of code".
> >>
> >> With code snippets you show, it looks even worse: you assign same global
> >> variable in several places for different uses. 
> > 
> > If you mean lpuart_is_be, it's not for different uses.
> > The purpose is the same to align the correct endian but in two places.
> 
> _probe() routine called for device X alters state already in use for
> device Y.
> 

Okay, you're saying two different types of devices appeared in one SoC.

> > 
> >> implicitly assuming that
> >> it is for same device. Which can be true in your current system, but not
> >> elsewhere (e.g. why not having lpuart programmed into fpga)?
> >>
> > 
> > Sorry, What issues for fpga?
> 
> Connect FPGA to IMX7 based system and program LS1012a version of lpuart
> core into it.  Have your console on system UART broken at time when
> driver gets registered.
> 

Well, theoretically it may happen.

> 
> > 
> >> Alternative solution could be - have separate write path for earlycon.
> > 
> > It looks to me having the same issue with a separate write patch
> > for earlycon as we still need distinguish Little or Big endian
> > for Layerscape and IMX.
> > 
> >> At a glance, it is dozen lines of code.
> > 
> > Would you please show some sample code?
> 
> Do not reuse lpuart32_console_putchar() in earlycon code.
> 
> Have two sets of early_setup/early_write/putchar - for BE and
> defaut-endian earlycon. And in these putchar's do not use
> lpuart_(read|write).
> 

Isn't that introducing another consistency break after fix one
consistency break?

If doing that, we then have two register read/write APIs.
One for normal driver operation by dynamically checking lpuart_is_be
property to distinguish the endian difference problem.
Another is specifically implemented for only early console read/write
and use hardcoded way to read/write register directly instead of using
the standard API lpuart32_read/write, like follows:
e.g.
lpuart32_le_console_write() {
	 writel();
}

lpuart32_be_console_write() {
	 iowrite32be()
}
This also makes the driver a bit strange and ugly.

It looks to me both way are trade offs and the later one seems sacrifice
more. And i doubt if it's really necessary for probably a no real gain
purpose as the FPGA you mentioned is a theoretical case and less
possibility to exist.

I'm still wondering how about keep using the exist way and adding more
information in code to explain why use a global var?

Regards
Dong Aisheng

> 
> As far as I can see, fsl_lpuart.c already has two drivers in one -
> there is separate set of routines for 8bit and 32bit cases.
> And those routines that are common, have if blocks that separate cases.
> I think these drivers will be cleaner if separated.
> However that's completely different story.

^ permalink raw reply

* Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding
From: Rob Herring @ 2017-05-19 12:37 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Jakub Kicinski, Michael Heimpold, Mark Rutland,
	Greg Kroah-Hartman, Jiri Slaby,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <53509108-fac2-8e8d-9279-ed54e8c1af3b-eS4NqCHxEME@public.gmane.org>

On Fri, May 19, 2017 at 2:13 AM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> Hi Rob,
>
> Am 12.05.2017 um 08:43 schrieb Jakub Kicinski:
>> On Fri, 12 May 2017 06:15:52 +0000, Michael Heimpold wrote:
>>> Hi,
>>>
>>> Zitat von Jakub Kicinski <kubakici-5tc4TXWwyLM@public.gmane.org>:
>>>
>>>> On Thu, 11 May 2017 21:12:22 +0200, Michael Heimpold wrote:
>>>>> Am Mittwoch, 10. Mai 2017, 10:53:26 CEST schrieb Stefan Wahren:
>>>>>> This merges the serdev binding for the QCA7000 UART driver (Ethernet over
>>>>>> UART) into the existing document.
>>>>>>
>>>>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/net/qca-qca7000.txt        | 32
>>>>>> ++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt
>>>>>> b/Documentation/devicetree/bindings/net/qca-qca7000.txt index
>>>>>> a37f656..08364c3 100644
>>>>>> --- a/Documentation/devicetree/bindings/net/qca-qca7000.txt
>>>>>> +++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt
>>>>>> @@ -54,3 +54,35 @@ ssp2: spi@80014000 {
>>>>>>           local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
>>>>>>   };
>>>>>>  };
>>>>>> +
>>>>>> +(b) Ethernet over UART
>>>>>> +
>>>>>> +In order to use the QCA7000 as UART slave it must be defined as
>>>>> a child of
>>>>>> a +UART master in the device tree. It is possible to preconfigure the UART
>>>>>> +settings of the QCA7000 firmware, but it's not possible to change them
>>>>>> during +runtime.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible        : Should be "qca,qca7000-uart"
>>>>> I already discussed this with Stefan off-list a little bit, but I would like
>>>>> to bring this to a broader audience: I'm not sure whether the compatible
>>>>> should contain the "-uart" suffix, because the hardware chip is the
>>>>> very same
>>>>> QCA7000 chip which can also be used with SPI protocol.
>>>>> The only difference is the loaded firmware within the chip which can either
>>>>> speak SPI or UART protocol (but not both at the same time - due to shared
>>>>> pins). So the hardware design decides which interface type is used.
>>>>>
>>>>> At the moment, this patch series adds a dedicated driver for the UART
>>>>> protocol, in parallel to the existing SPI driver. So a different compatible
>>>>> string is needed here to match against the new driver.
>>>>>
>>>>> An alternative approach would be to re-use the existing compatible string
>>>>> "qca,qca7000" for both, the SPI and UART protocol, because a "smarter"
>>>>> (combined) driver would detect which protocol to use. For example the driver
>>>>> could check for spi-cpha and/or spi-cpol which are required for SPI
>>>>> protocol:
>>>>> if these exists the driver could assume that SPI must be used, if both are
>>>>> missing then UART protocol should be used.
>>>>> (This way it would not be necessary to check whether the node is a child of
>>>>> a SPI or UART master node - but maybe this is even easier - I don't know)
>>>>>
>>>>> Or in shorter words: my concern is that while "qca7000-uart" describes the
>>>>> hardware, it's too closely coupled to the driver implementation. Having
>>>>> some feedback of the experts would be nice :-)
>>>> I'm no expert, but devices which can do both I2C and SPI are quite
>>>> common, and they usually have the same compatible string for both
>>>> buses.
>>> do you have an example driver at hand? I only found GPIO mcp23s08 driver,
>>> which can handle both I2C and SPI chips, but there are different compatible
>>> strings used to distinguish several chip models.
>> I think drivers/tty/serial/sc16is7xx.c has the same strings, and some
>> Kconfig magic to work when either bus is enabled in .config.
>>
>> Quick grep shows there are couple more potential ones to look at:
>>
>> $ find . -name Kconfig | xargs grep -n 'SPI_MASTER.* I2C'
>> ./drivers/tty/serial/Kconfig:1208:        depends on (SPI_MASTER && !I2C) || I2C
>> ./drivers/mfd/Kconfig:327:    depends on (SPI_MASTER || I2C)
>> ./drivers/iio/dac/Kconfig:10: depends on (SPI_MASTER && I2C!=m) || I2C
>> ./drivers/iio/dac/Kconfig:34: depends on (SPI_MASTER && I2C!=m) || I2C
>> ./drivers/iio/dac/Kconfig:57: depends on (SPI_MASTER && I2C!=m) || I2C
>> ./drivers/gpio/Kconfig:1231:  depends on (SPI_MASTER && !I2C) || I2C
>> $ find . -name Kconfig | xargs grep -n 'I2C.*||.*SPI_MASTER'
>> ./drivers/mfd/Kconfig:1094:   depends on (I2C=y || SPI_MASTER=y)
>> ./drivers/iio/gyro/Kconfig:55:        depends on (I2C || SPI_MASTER)
>> ./drivers/iio/gyro/Kconfig:107:       depends on (I2C || SPI_MASTER) && SYSFS
>> ./drivers/iio/accel/Kconfig:153:      depends on (I2C || SPI_MASTER) && SYSFS
>> ./drivers/iio/pressure/Kconfig:20:    depends on (I2C || SPI_MASTER)
>> ./drivers/iio/pressure/Kconfig:161:   depends on (I2C || SPI_MASTER) && SYSFS
>> ./drivers/iio/magnetometer/Kconfig:118:       depends on (I2C || SPI_MASTER) && SYSFS
>>
>> drivers/mfd/mc13xxx-*.c seems to have the same strings.  The iio/dac drivers
>> don't support DT but do share names.  The MCP GPIO chip you mention indeed has
>> different product names based on the bus it's made for (0 vs s in the middle
>> of the name), so I gather less relevant case?  drivers/iio/pressure/bmp280-*.c
>> has the same strings, if I'm looking correctly... I didn't look at the others.
>
> are you okay with the suggestion to use the compatible "qca,qca7000" for
> both drivers?

I believe I said I was fine either way.

> Should we mark "qca,qca7000-spi" as deprecated?

Yes.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
From: Dong Aisheng @ 2017-05-19 11:50 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Dong Aisheng, linux-serial, linux-kernel, linux-arm-kernel,
	gregkh, jslaby, fugang.duan, Mingkai.Hu, yangbo.lu
In-Reply-To: <90fa8850f9424712074217dff03b229e@agner.ch>

On Wed, May 17, 2017 at 10:35:43AM -0700, Stefan Agner wrote:
> On 2017-05-16 20:47, Dong Aisheng wrote:
> > On Mon, May 15, 2017 at 10:06:41AM -0700, Stefan Agner wrote:
> >> On 2017-05-15 00:48, Dong Aisheng wrote:
> >> > On new LPUART versions, the oversampling ratio for the receiver can be
> >> > changed from 4x (00011) to 32x (11111) which could help us get a more
> >> > accurate baud rate divider.
> >> >
> >> > The idea is to use the best OSR (over-sampling rate) possible.
> >> > Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> >> > Loop to find the best OSR value possible, one that generates minimum
> >> > baud diff iterate through the rest of the supported values of OSR.
> >> >
> >> > Currently only i.MX7ULP is using it.
> >> >
> >> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> > Cc: Jiri Slaby <jslaby@suse.com>
> >> > Cc: Stefan Agner <stefan@agner.ch>
> >> > Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> >> > Cc: Yangbo Lu <yangbo.lu@nxp.com>
> >> > Acked-by: Fugang Duan <fugang.duan@nxp.com>
> >> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> >> > ---
> >> >  drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
> >> >  1 file changed, 79 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> >> > index 107d0911..bda4b0c 100644
> >> > --- a/drivers/tty/serial/fsl_lpuart.c
> >> > +++ b/drivers/tty/serial/fsl_lpuart.c
> >> > @@ -140,6 +140,8 @@
> >> >  #define UARTBAUD_SBNS		0x00002000
> >> >  #define UARTBAUD_SBR		0x00000000
> >> >  #define UARTBAUD_SBR_MASK	0x1fff
> >> > +#define UARTBAUD_OSR_MASK       0x1f
> >> > +#define UARTBAUD_OSR_SHIFT      24
> >> >
> >> >  #define UARTSTAT_LBKDIF		0x80000000
> >> >  #define UARTSTAT_RXEDGIF	0x40000000
> >> > @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
> >> > struct ktermios *termios,
> >> >  }
> >> >
> >> >  static void
> >> > +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> >> > +{
> >> > +	u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> >> > +	u32 clk = sport->port.uartclk;
> >> > +
> >> > +	/*
> >> > +	 * The idea is to use the best OSR (over-sampling rate) possible.
> >> > +	 * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> >> > +	 * Loop to find the best OSR value possible, one that generates minimum
> >> > +	 * baud_diff iterate through the rest of the supported values of OSR.
> >> > +	 *
> >> > +	 * Calculation Formula:
> >> > +	 *  Baud Rate = baud clock / ((OSR+1) × SBR)
> >> > +	 */
> >> > +	baud_diff = baudrate;
> >> > +	osr = 0;
> >> > +	sbr = 0;
> >> > +
> >> > +	for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> >> > +		/* calculate the temporary sbr value  */
> >> > +		tmp_sbr = (clk / (baudrate * tmp_osr));
> >> > +		if (tmp_sbr == 0)
> >> > +			tmp_sbr = 1;
> >> > +
> >> > +		/*
> >> > +		 * calculate the baud rate difference based on the temporary
> >> > +		 * osr and sbr values
> >> > +		 */
> >> > +		tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
> >> > +
> >> > +		/* select best values between sbr and sbr+1 */
> >> > +		tmp = clk / (tmp_osr * (tmp_sbr + 1));
> >> > +		if (tmp_diff > (baudrate - tmp)) {
> >> > +			tmp_diff = baudrate - tmp;
> >> > +			tmp_sbr++;
> >> > +		}
> >> > +
> >> > +		if (tmp_diff <= baud_diff) {
> >> > +			baud_diff = tmp_diff;
> >> > +			osr = tmp_osr;
> >> > +			sbr = tmp_sbr;
> >> > +		}
> >> > +	}
> >> > +
> >> > +	/* handle buadrate outside acceptable rate */
> >> > +	if (baud_diff > ((baudrate / 100) * 3))
> >> > +		dev_warn(sport->port.dev,
> >> > +			 "unacceptable baud rate difference of more than 3%%\n");
> >> > +
> >> > +	tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> >> > +
> >> > +	if ((osr > 3) && (osr < 8))
> >> > +		tmp |= UARTBAUD_BOTHEDGE;
> >> > +
> >> > +	tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
> >> > +	tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
> >> > +
> >> > +	tmp &= ~UARTBAUD_SBR_MASK;
> >> > +	tmp |= sbr & UARTBAUD_SBR_MASK;
> >> > +
> >> > +	tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> >> > +
> >> > +	lpuart32_write(tmp, sport->port.membase + UARTBAUD);
> >> > +}
> >> > +
> >> > +static void
> >> >  lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
> >> >  		   struct ktermios *old)
> >> >  {
> >> > @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
> >> > struct ktermios *termios,
> >> >  	lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
> >> >  			sport->port.membase + UARTCTRL);
> >> >
> >> > -	sbr = sport->port.uartclk / (16 * baud);
> >> > -	bd &= ~UARTBAUD_SBR_MASK;
> >> > -	bd |= sbr & UARTBAUD_SBR_MASK;
> >> > -	bd |= UARTBAUD_BOTHEDGE;
> >> > -	bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> >> > -	lpuart32_write(bd, sport->port.membase + UARTBAUD);
> >> > +	if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
> >>
> >> Shouldn't we be consequent here and also use a flag in the soc data
> >> instead of of_device_is_compatible...?
> >>
> > 
> > The original purpose is that this is a temporary code and supposed will
> > be deleted later once LS platforms confirmed the new baud setting API
> > works for them as well.
> > 
> > That's why i did not make it a property, as i stated in the cover letter.
> 
> Ok, I see that is a good reason to not define a new feature property
> now... 
> 
> But, if you are reasonable sure it should work, I am inclined to say
> just enable it for LS1021a so it also really gets tested...
> 

Okay, i'll take this suggestion to get things started.

Thanks

Regards
Dong Aisheng

^ permalink raw reply

* [PATCH 2/2] arm64: dts: Add Mediatek SoC MT2712 and evaluation board dts and Makefile
From: YT Shen @ 2017-05-19 11:09 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger
  Cc: Mark Rutland, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Mars Cheng,
	YT Shen, devicetree, linux-kernel, linux-serial, linux-arm-kernel,
	linux-mediatek, srv_heupstream
In-Reply-To: <1495192186-22480-1-git-send-email-yt.shen@mediatek.com>

This adds basic chip support for Mediatek 2712

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/Makefile       |   1 +
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts |  44 +++++++
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi   | 172 ++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712e.dtsi

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index 9fbfd32..fcc0604 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
new file mode 100644
index 0000000..40b0c91
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: YT Shen <yt.shen@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/dts-v1/;
+#include "mt2712e.dtsi"
+
+/ {
+	model = "MediaTek MT2712 evaluation board";
+	compatible = "mediatek,mt2712-evb", "mediatek,mt2712";
+
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+		serial3 = &uart3;
+		serial4 = &uart4;
+		serial5 = &uart5;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0 0x40000000 0 0x80000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,921600n1 initrd=0x45000000,90M";
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
+
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
new file mode 100644
index 0000000..40747a9
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -0,0 +1,172 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: YT Shen <yt.shen@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "mediatek,mt2712";
+	interrupt-parent = <&sysirq>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+				core1 {
+					cpu = <&cpu1>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&cpu2>;
+				};
+			};
+		};
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x000>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x001>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu@200 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0x200>;
+			enable-method = "psci";
+		};
+	};
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+	uart_clk: dummy26m {
+		compatible = "fixed-clock";
+		clock-frequency = <26000000>;
+		#clock-cells = <0>;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_PPI 13
+			      (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14
+			      (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11
+			      (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10
+			      (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	soc {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		ranges;
+
+		uart5: serial@1000f000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x1000f000 0 0x400>;
+			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&uart_clk>;
+			status = "disabled";
+		};
+
+		sysirq: intpol-controller@10220a80 {
+			compatible = "mediatek,mt2712-sysirq",
+				     "mediatek,mt6577-sysirq";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			reg = <0 0x10220a80 0 0x40>;
+		};
+
+		gic: interrupt-controller@10510000 {
+			compatible = "arm,gic-400";
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			reg = <0 0x10510000 0 0x1000>,
+			      <0 0x10520000 0 0x1000>,
+			      <0 0x10540000 0 0x2000>,
+			      <0 0x10560000 0 0x2000>;
+			interrupts = <GIC_PPI 9
+				(GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
+		};
+
+		uart0: serial@11002000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11002000 0 0x400>;
+			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&uart_clk>;
+			status = "disabled";
+		};
+
+		uart1: serial@11003000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11003000 0 0x400>;
+			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&uart_clk>;
+			status = "disabled";
+		};
+
+		uart2: serial@11004000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11004000 0 0x400>;
+			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&uart_clk>;
+			status = "disabled";
+		};
+
+		uart3: serial@11005000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11005000 0 0x400>;
+			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&uart_clk>;
+			status = "disabled";
+		};
+
+		uart4: serial@11019000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11019000 0 0x400>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&uart_clk>;
+			status = "disabled";
+		};
+	};
+};
+
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/2] Document: DT: Add bindings for Mediatek MT2712 SoC Platform
From: YT Shen @ 2017-05-19 11:09 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger
  Cc: Mark Rutland, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Mars Cheng,
	YT Shen, devicetree, linux-kernel, linux-serial, linux-arm-kernel,
	linux-mediatek, srv_heupstream
In-Reply-To: <1495192186-22480-1-git-send-email-yt.shen@mediatek.com>

This adds dt-binding documentation for Mediatek MT2712.
Only include very basic items: cpu, gic and uart.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 Documentation/devicetree/bindings/arm/mediatek.txt                    | 4 ++++
 .../devicetree/bindings/interrupt-controller/mediatek,sysirq.txt      | 1 +
 Documentation/devicetree/bindings/serial/mtk-uart.txt                 | 1 +
 3 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.txt b/Documentation/devicetree/bindings/arm/mediatek.txt
index c860b24..3161651 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek.txt
@@ -7,6 +7,7 @@ Required root node property:
 
 compatible: Must contain one of
    "mediatek,mt2701"
+   "mediatek,mt2712"
    "mediatek,mt6580"
    "mediatek,mt6589"
    "mediatek,mt6592"
@@ -23,6 +24,9 @@ Supported boards:
 - Evaluation board for MT2701:
     Required root node properties:
       - compatible = "mediatek,mt2701-evb", "mediatek,mt2701";
+- Evaluation board for MT2712:
+    Required root node properties:
+      - compatible = "mediatek,mt2712-evb", "mediatek,mt2712";
 - Evaluation board for MT6580:
     Required root node properties:
       - compatible = "mediatek,mt6580-evbp1", "mediatek,mt6580";
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
index a89c03b..653adb5 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
@@ -16,6 +16,7 @@ Required properties:
 	"mediatek,mt6580-sysirq"
 	"mediatek,mt6577-sysirq"
 	"mediatek,mt2701-sysirq"
+	"mediatek,mt2712-sysirq"
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Use the same format as specified by GIC in arm,gic.txt.
 - interrupt-parent: phandle of irq parent for sysirq. The parent must
diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index 0015c72..5f88e0d 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -3,6 +3,7 @@
 Required properties:
 - compatible should contain:
   * "mediatek,mt2701-uart" for MT2701 compatible UARTS
+  * "mediatek,mt2712-uart" for MT2712 compatible UARTS
   * "mediatek,mt6580-uart" for MT6580 compatible UARTS
   * "mediatek,mt6582-uart" for MT6582 compatible UARTS
   * "mediatek,mt6589-uart" for MT6589 compatible UARTS
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/2] Add basic support for Mediatek MT2712 SoC
From: YT Shen @ 2017-05-19 11:09 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger
  Cc: Mark Rutland, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Mars Cheng,
	YT Shen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w

MT2712 is a SoC based on 64bit ARMv8 architecture.
MT2712 share many HW IP with MT8173.  This patchset was tested
on MT2712 evaluation board, and boot to shell ok.

This series contains document bindings, device tree including
interrupt, uart.

YT Shen (2):
  Document: DT: Add bindings for Mediatek MT2712 SoC Platform
  arm64: dts: Add Mediatek SoC MT2712 and evaluation board dts and
    Makefile

 Documentation/devicetree/bindings/arm/mediatek.txt |   4 +
 .../interrupt-controller/mediatek,sysirq.txt       |   1 +
 .../devicetree/bindings/serial/mtk-uart.txt        |   1 +
 arch/arm64/boot/dts/mediatek/Makefile              |   1 +
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts        |  44 ++++++
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi          | 172 +++++++++++++++++++++
 6 files changed, 223 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712e.dtsi

-- 
1.9.1

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

^ permalink raw reply

* Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding
From: Stefan Wahren @ 2017-05-19  7:13 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jakub Kicinski, Michael Heimpold, Mark Rutland,
	Greg Kroah-Hartman, Jiri Slaby,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170511234342.575d9226-68UzVGuGftmUSpRRplVxJ1aTQe2KTcn/@public.gmane.org>

Hi Rob,

Am 12.05.2017 um 08:43 schrieb Jakub Kicinski:
> On Fri, 12 May 2017 06:15:52 +0000, Michael Heimpold wrote:
>> Hi,
>>
>> Zitat von Jakub Kicinski <kubakici-5tc4TXWwyLM@public.gmane.org>:
>>
>>> On Thu, 11 May 2017 21:12:22 +0200, Michael Heimpold wrote:  
>>>> Am Mittwoch, 10. Mai 2017, 10:53:26 CEST schrieb Stefan Wahren:  
>>>>> This merges the serdev binding for the QCA7000 UART driver (Ethernet over
>>>>> UART) into the existing document.
>>>>>
>>>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>>>> ---
>>>>>  .../devicetree/bindings/net/qca-qca7000.txt        | 32
>>>>> ++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt
>>>>> b/Documentation/devicetree/bindings/net/qca-qca7000.txt index
>>>>> a37f656..08364c3 100644
>>>>> --- a/Documentation/devicetree/bindings/net/qca-qca7000.txt
>>>>> +++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt
>>>>> @@ -54,3 +54,35 @@ ssp2: spi@80014000 {
>>>>>  		local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
>>>>>  	};
>>>>>  };
>>>>> +
>>>>> +(b) Ethernet over UART
>>>>> +
>>>>> +In order to use the QCA7000 as UART slave it must be defined as    
>>>> a child of  
>>>>> a +UART master in the device tree. It is possible to preconfigure the UART
>>>>> +settings of the QCA7000 firmware, but it's not possible to change them
>>>>> during +runtime.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible        : Should be "qca,qca7000-uart"  
>>>> I already discussed this with Stefan off-list a little bit, but I would like
>>>> to bring this to a broader audience: I'm not sure whether the compatible
>>>> should contain the "-uart" suffix, because the hardware chip is the  
>>>> very same
>>>> QCA7000 chip which can also be used with SPI protocol.
>>>> The only difference is the loaded firmware within the chip which can either
>>>> speak SPI or UART protocol (but not both at the same time - due to shared
>>>> pins). So the hardware design decides which interface type is used.
>>>>
>>>> At the moment, this patch series adds a dedicated driver for the UART
>>>> protocol, in parallel to the existing SPI driver. So a different compatible
>>>> string is needed here to match against the new driver.
>>>>
>>>> An alternative approach would be to re-use the existing compatible string
>>>> "qca,qca7000" for both, the SPI and UART protocol, because a "smarter"
>>>> (combined) driver would detect which protocol to use. For example the driver
>>>> could check for spi-cpha and/or spi-cpol which are required for SPI  
>>>> protocol:
>>>> if these exists the driver could assume that SPI must be used, if both are
>>>> missing then UART protocol should be used.
>>>> (This way it would not be necessary to check whether the node is a child of
>>>> a SPI or UART master node - but maybe this is even easier - I don't know)
>>>>
>>>> Or in shorter words: my concern is that while "qca7000-uart" describes the
>>>> hardware, it's too closely coupled to the driver implementation. Having
>>>> some feedback of the experts would be nice :-)  
>>> I'm no expert, but devices which can do both I2C and SPI are quite
>>> common, and they usually have the same compatible string for both
>>> buses.  
>> do you have an example driver at hand? I only found GPIO mcp23s08 driver,
>> which can handle both I2C and SPI chips, but there are different compatible
>> strings used to distinguish several chip models.
> I think drivers/tty/serial/sc16is7xx.c has the same strings, and some
> Kconfig magic to work when either bus is enabled in .config.
>
> Quick grep shows there are couple more potential ones to look at:
>
> $ find . -name Kconfig | xargs grep -n 'SPI_MASTER.* I2C' 
> ./drivers/tty/serial/Kconfig:1208:        depends on (SPI_MASTER && !I2C) || I2C
> ./drivers/mfd/Kconfig:327:	depends on (SPI_MASTER || I2C)
> ./drivers/iio/dac/Kconfig:10:	depends on (SPI_MASTER && I2C!=m) || I2C
> ./drivers/iio/dac/Kconfig:34:	depends on (SPI_MASTER && I2C!=m) || I2C
> ./drivers/iio/dac/Kconfig:57:	depends on (SPI_MASTER && I2C!=m) || I2C
> ./drivers/gpio/Kconfig:1231:	depends on (SPI_MASTER && !I2C) || I2C
> $ find . -name Kconfig | xargs grep -n 'I2C.*||.*SPI_MASTER' 
> ./drivers/mfd/Kconfig:1094:	depends on (I2C=y || SPI_MASTER=y)
> ./drivers/iio/gyro/Kconfig:55:	depends on (I2C || SPI_MASTER)
> ./drivers/iio/gyro/Kconfig:107:	depends on (I2C || SPI_MASTER) && SYSFS
> ./drivers/iio/accel/Kconfig:153:	depends on (I2C || SPI_MASTER) && SYSFS
> ./drivers/iio/pressure/Kconfig:20:	depends on (I2C || SPI_MASTER)
> ./drivers/iio/pressure/Kconfig:161:	depends on (I2C || SPI_MASTER) && SYSFS
> ./drivers/iio/magnetometer/Kconfig:118:	depends on (I2C || SPI_MASTER) && SYSFS
>
> drivers/mfd/mc13xxx-*.c seems to have the same strings.  The iio/dac drivers
> don't support DT but do share names.  The MCP GPIO chip you mention indeed has
> different product names based on the bus it's made for (0 vs s in the middle 
> of the name), so I gather less relevant case?  drivers/iio/pressure/bmp280-*.c 
> has the same strings, if I'm looking correctly... I didn't look at the others.

are you okay with the suggestion to use the compatible "qca,qca7000" for
both drivers?

Should we mark "qca,qca7000-spi" as deprecated?

Regards
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 5/6] gpio-exar/8250-exar: Make set of exported GPIOs configurable
From: Andy Shevchenko @ 2017-05-18 17:43 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <357209721396d57cb871ae69d72308b1a7c9cbd7.1495119548.git.jan.kiszka@siemens.com>

On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On the SIMATIC, IOT2040 only a single pin is exportable as GPIO, the
> rest is required to operate the UART. To allow modeling this case,
> expand the platform device data structure to specify a (consecutive) pin
> subset for exporting by the gpio-exar driver.

> +       unsigned int first_gpio;

Perhaps pin?
Or shift?

Because first_gpio a bit confusing with Linux side of GPIO.

> -       unsigned int bank = offset / 8;
> -       unsigned int addr;
> +       struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
> +       unsigned int bank, addr;
>
> +       offset += exar_gpio->first_gpio;
> +       bank = offset / 8;

Can't we instead do something like the following:

struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
unsigned int bank = (offset + exar_gpio->pin) / 8;
unsigned int line = (offset + exar_gpio->pin) % 8;


> +       pdata.first_gpio = first_gpio;
> +       pdata.ngpio = ngpio;

Still thinking about device properties ("ngpios" and something like
"exar8250,gpio-start").

> +       unsigned int first_gpio;
> +       unsigned int ngpio;

u16 ?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Andy Shevchenko @ 2017-05-18 17:41 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <65189e05-e300-bbe3-660d-4fdd6d4ec8c1@siemens.com>

On Thu, May 18, 2017 at 7:39 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-18 18:33, Andy Shevchenko wrote:
>> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

>>> +static bool is_iot2040;
>>
>> No, please, use driver data of DMI and hide this in corresponding structure.
>> Or even assign  port->port.rs485_config in the callback function.
>>
>> Moreover, can't you use port->port.rs485_config != NULL instead?
>
> There are two cases to be handled on IOT2040: the setting of the
> rs485_config and the different setup of the GPIOs, but the latter at a
> specific point in the initialization only. So I don't see yet how
> driver_data could come into play and help.

struct exar_iot2040_setup {
...rs485_config();
...setup_gpio();
};

struct exar_iot2040_setup iot2040_setup = {
...
};

DMI:

.driver_data = (void *)&iot2040_setup;

Above is just unfinished proposal, since I have noticed your new mail.
So, it seems we are on the same page.

One thing, I still would consider to use device properties instead of
platform data (with consideration of MFD framework usage).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-18 17:23 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <f3959f33-b82a-e64a-c04d-bfa7ab486402@siemens.com>

On 2017-05-18 18:58, Jan Kiszka wrote:
> On 2017-05-18 18:39, Jan Kiszka wrote:
>> On 2017-05-18 18:33, Andy Shevchenko wrote:
>>> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> This implements the setup of RS232 and the switch-over to RS485 or RS422
>>>> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
>>>> to switch between the different modes. The external logic is controlled
>>>> via MPIO pins of the EXAR controller.
>>>>
>>>> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
>>>> an LED.
>>>>
>>>> As the XR17V352 used on the IOT2040 is not equipped with an external
>>>> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
>>>> subdevice IDs. Thus, we have to check via DMI for the target platform.
>>>>
>>>> Co-developed with Sascha Weisenberger.
>>>
>>> Few nits below and one comment that should be addressed.
>>>
>>>> +#define UART_EXAR_RS485_DLY(x) (x << 4)
>>>
>>> ((x) << 4)
>>
>> Yep.
>>
>>>
>>>> +static bool is_iot2040;
>>>
>>> No, please, use driver data of DMI and hide this in corresponding structure.
>>> Or even assign  port->port.rs485_config in the callback function.
>>>
>>> Moreover, can't you use port->port.rs485_config != NULL instead?
>>
>> There are two cases to be handled on IOT2040: the setting of the
>> rs485_config and the different setup of the GPIOs, but the latter at a
>> specific point in the initialization only. So I don't see yet how
>> driver_data could come into play and help.
>>
> 
> OK, got - hacking...

It looks like this now (will resent as part of v3, just waiting for
further comments):

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index d9c52288d6c9..01826c094515 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -9,6 +9,7 @@
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License.
  */
+#include <linux/dmi.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -61,8 +62,50 @@
 #define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
 #define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
 
+#define UART_EXAR_RS485_DLY(x)	((x) << 4)
+
+/*
+ * IOT2040 MPIO wiring semantics:
+ *
+ * MPIO		Port	Function
+ * ----		----	--------
+ * 0		2 	Mode bit 0
+ * 1		2	Mode bit 1
+ * 2		2	Terminate bus
+ * 3		-	<reserved>
+ * 4		3	Mode bit 0
+ * 5		3	Mode bit 1
+ * 6		3	Terminate bus
+ * 7		-	<reserved>
+ * 8		2	Enable
+ * 9		3	Enable
+ * 10		-	Red LED
+ * 11..15	-	<unused>
+ */
+
+/* IOT2040 MPIOs 0..7 */
+#define IOT2040_UART_MODE_RS232		0x01
+#define IOT2040_UART_MODE_RS485		0x02
+#define IOT2040_UART_MODE_RS422		0x03
+#define IOT2040_UART_TERMINATE_BUS	0x04
+
+#define IOT2040_UART1_MASK		0x0f
+#define IOT2040_UART2_SHIFT		4
+
+#define IOT2040_UARTS_DEFAULT_MODE	0x11	/* both RS232 */
+#define IOT2040_UARTS_GPIO_LO_MODE	0x88	/* reserved pins as input */
+
+/* IOT2040 MPIOs 8..15 */
+#define IOT2040_UARTS_ENABLE		0x03
+#define IOT2040_UARTS_GPIO_HI_MODE	0xF8	/* enable & LED as outputs */
+
 struct exar8250;
 
+struct exar8250_platform {
+	int (*rs485_config)(struct uart_port *, struct serial_rs485 *);
+	int (*register_gpio)(struct pci_dev *, struct uart_8250_port *);
+};
+
 /**
  * struct exar8250_board - board information
  * @num_ports: number of serial ports
@@ -189,7 +232,7 @@ static void setup_gpio(u8 __iomem *p)
 }
 
 static void *
-xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
+__xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
 		       unsigned int ngpio)
 {
 	struct platform_device *pdev;
@@ -212,17 +255,113 @@ xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
 	return pdev;
 }
 
+static int xr17v35x_register_gpio(struct pci_dev *pcidev,
+				  struct uart_8250_port *port)
+{
+	port->port.private_data = __xr17v35x_register_gpio(pcidev, 0, 16);
+
+	return 0;
+}
+
+static int iot2040_rs485_config(struct uart_port *port,
+				struct serial_rs485 *rs485)
+{
+	bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
+	u8 __iomem *p = port->membase;
+	u8 mask = IOT2040_UART1_MASK;
+	u8 mode, value;
+
+	if (is_rs485) {
+		if (rs485->flags & SER_RS485_RX_DURING_TX)
+			mode = IOT2040_UART_MODE_RS422;
+		else
+			mode = IOT2040_UART_MODE_RS485;
+
+		if (rs485->flags & SER_RS485_TERMINATE_BUS)
+			mode |= IOT2040_UART_TERMINATE_BUS;
+	} else {
+		mode = IOT2040_UART_MODE_RS232;
+	}
+
+	if (port->line == 3) {
+		mask <<= IOT2040_UART2_SHIFT;
+		mode <<= IOT2040_UART2_SHIFT;
+	}
+
+	value = readb(p + UART_EXAR_MPIOLVL_7_0);
+	value &= ~mask;
+	value |= mode;
+	writeb(value, p + UART_EXAR_MPIOLVL_7_0);
+
+	value = readb(p + UART_EXAR_FCTR);
+	if (is_rs485)
+		value |= UART_FCTR_EXAR_485;
+	else
+		value &= ~UART_FCTR_EXAR_485;
+	writeb(value, p + UART_EXAR_FCTR);
+
+	if (is_rs485)
+		writeb(UART_EXAR_RS485_DLY(4), p + UART_MSR);
+
+	return 0;
+}
+
+static int iot2040_register_gpio(struct pci_dev *pcidev,
+			      struct uart_8250_port *port)
+{
+	u8 __iomem *p = port->port.membase;
+
+	writeb(IOT2040_UARTS_DEFAULT_MODE, p + UART_EXAR_MPIOLVL_7_0);
+	writeb(IOT2040_UARTS_GPIO_LO_MODE, p + UART_EXAR_MPIOSEL_7_0);
+	writeb(IOT2040_UARTS_ENABLE, p + UART_EXAR_MPIOLVL_15_8);
+	writeb(IOT2040_UARTS_GPIO_HI_MODE, p + UART_EXAR_MPIOSEL_15_8);
+
+	port->port.private_data = __xr17v35x_register_gpio(pcidev, 10, 1);
+
+	return 0;
+}
+
+static const struct exar8250_platform exar8250_default_platform = {
+	.register_gpio = xr17v35x_register_gpio,
+};
+
+static const struct exar8250_platform iot2040_platform = {
+	.rs485_config = iot2040_rs485_config,
+	.register_gpio = iot2040_register_gpio,
+};
+
+static const struct dmi_system_id exar_platforms[] = {
+	{
+		.ident = "IOT2040",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
+			DMI_MATCH(DMI_BOARD_ASSET_TAG, "6ES7647-0AA00-1YA2"),
+		},
+		.driver_data = (void *)&iot2040_platform,
+	}
+};
+
 static int
 pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		   struct uart_8250_port *port, int idx)
 {
 	const struct exar8250_board *board = priv->board;
+	const struct exar8250_platform *platform;
+	const struct dmi_system_id *dmi_match;
 	unsigned int offset = idx * 0x400;
 	unsigned int baud = 7812500;
 	u8 __iomem *p;
 	int ret;
 
+	dmi_match = dmi_first_match(exar_platforms);
+	if (dmi_match)
+		platform = dmi_match->driver_data;
+	else
+		platform = &exar8250_default_platform;
+
 	port->port.uartclk = baud * 16;
+	port->port.rs485_config = platform->rs485_config;
+
 	/*
 	 * Setup the uart clock for the devices on expansion slot to
 	 * half the clock speed of the main chip (which is 125MHz)
@@ -245,10 +384,10 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		/* Setup Multipurpose Input/Output pins. */
 		setup_gpio(p);
 
-		port->port.private_data = xr17v35x_register_gpio(pcidev, 0, 16);
+		ret = platform->register_gpio(pcidev, port);
 	}
 
-	return 0;
+	return ret;
 }
 
 static void pci_xr17v35x_exit(struct pci_dev *pcidev)
-- 
2.12.

^ permalink raw reply related

* Re: [PATCH v2 1/6] gpio: exar: Fix passing in of parent PCI device
From: Andy Shevchenko @ 2017-05-18 17:14 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <62280d24a7cf7c0be849c1186c909c850f4d2cc6.1495119548.git.jan.kiszka@siemens.com>

On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This fixes reloading of the GPIO driver for the same platform device
> instance as created by the exar UART driver: First of all, the driver
> sets drvdata to its own value during probing and does not restore the
> original value on exit. But this won't help anyway as the core clears
> drvdata after the driver left.
>
> Use stable platform_data instead.

Okay, basically what we are trying to do here is to reinvent part of
MFD framework.

I'd like to hear Linus' and others opinions if it worth to use it instead.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-18 16:58 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <65189e05-e300-bbe3-660d-4fdd6d4ec8c1@siemens.com>

On 2017-05-18 18:39, Jan Kiszka wrote:
> On 2017-05-18 18:33, Andy Shevchenko wrote:
>> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> This implements the setup of RS232 and the switch-over to RS485 or RS422
>>> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
>>> to switch between the different modes. The external logic is controlled
>>> via MPIO pins of the EXAR controller.
>>>
>>> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
>>> an LED.
>>>
>>> As the XR17V352 used on the IOT2040 is not equipped with an external
>>> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
>>> subdevice IDs. Thus, we have to check via DMI for the target platform.
>>>
>>> Co-developed with Sascha Weisenberger.
>>
>> Few nits below and one comment that should be addressed.
>>
>>> +#define UART_EXAR_RS485_DLY(x) (x << 4)
>>
>> ((x) << 4)
> 
> Yep.
> 
>>
>>> +static bool is_iot2040;
>>
>> No, please, use driver data of DMI and hide this in corresponding structure.
>> Or even assign  port->port.rs485_config in the callback function.
>>
>> Moreover, can't you use port->port.rs485_config != NULL instead?
> 
> There are two cases to be handled on IOT2040: the setting of the
> rs485_config and the different setup of the GPIOs, but the latter at a
> specific point in the initialization only. So I don't see yet how
> driver_data could come into play and help.
> 

OK, got - hacking...

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-18 16:39 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75VewiaiYq6esGTiTV8kG30krhuXLQgLFGn10jmjkooXGPA@mail.gmail.com>

On 2017-05-18 18:33, Andy Shevchenko wrote:
> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> This implements the setup of RS232 and the switch-over to RS485 or RS422
>> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
>> to switch between the different modes. The external logic is controlled
>> via MPIO pins of the EXAR controller.
>>
>> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
>> an LED.
>>
>> As the XR17V352 used on the IOT2040 is not equipped with an external
>> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
>> subdevice IDs. Thus, we have to check via DMI for the target platform.
>>
>> Co-developed with Sascha Weisenberger.
> 
> Few nits below and one comment that should be addressed.
> 
>> +#define UART_EXAR_RS485_DLY(x) (x << 4)
> 
> ((x) << 4)

Yep.

> 
>> +static bool is_iot2040;
> 
> No, please, use driver data of DMI and hide this in corresponding structure.
> Or even assign  port->port.rs485_config in the callback function.
> 
> Moreover, can't you use port->port.rs485_config != NULL instead?

There are two cases to be handled on IOT2040: the setting of the
rs485_config and the different setup of the GPIOs, but the latter at a
specific point in the initialization only. So I don't see yet how
driver_data could come into play and help.

> 
>> +
>>  struct exar8250;
>>
>>  /**
>> @@ -212,6 +252,82 @@ xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
>>         return pdev;
>>  }
>>
>> +static int iot2040_rs485_config(struct uart_port *port,
>> +                               struct serial_rs485 *rs485)
>> +{
>> +       u8 __iomem *p = port->membase;
>> +       u8 mask = IOT2040_UART1_MASK;
>> +       u8 mode, value;
> 
>> +       bool is_rs485 = false;
>> +
>> +       if (rs485->flags & SER_RS485_ENABLED) {
>> +               is_rs485 = true;
> 
> bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
> 
> if (is_rs485) {
> ...
> } else {
> ...
> }
> 

OK.

>> +       return 0;
>> +}
> 
>> +static int iot2040_match(const struct dmi_system_id *dmi)
>> +{
>> +       is_iot2040 = true;
>> +       return 1;
>> +}
> 
> See above.

See above.

Jan

> 
>> +
>> +static const struct dmi_system_id exar_platforms[] = {
>> +       {
>> +               .callback = iot2040_match,
>> +               .ident = "IOT2040",
>> +               .matches = {
>> +                       DMI_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
>> +                       DMI_MATCH(DMI_BOARD_ASSET_TAG, "6ES7647-0AA00-1YA2"),
>> +               },
>> +       }
>> +};
> 


^ permalink raw reply

* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Andy Shevchenko @ 2017-05-18 16:33 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <f0d2245527bc5700be7a4ac707a81db798cb5519.1495119548.git.jan.kiszka@siemens.com>

On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This implements the setup of RS232 and the switch-over to RS485 or RS422
> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
> to switch between the different modes. The external logic is controlled
> via MPIO pins of the EXAR controller.
>
> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
> an LED.
>
> As the XR17V352 used on the IOT2040 is not equipped with an external
> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
> subdevice IDs. Thus, we have to check via DMI for the target platform.
>
> Co-developed with Sascha Weisenberger.

Few nits below and one comment that should be addressed.

> +#define UART_EXAR_RS485_DLY(x) (x << 4)

((x) << 4)

> +static bool is_iot2040;

No, please, use driver data of DMI and hide this in corresponding structure.
Or even assign  port->port.rs485_config in the callback function.

Moreover, can't you use port->port.rs485_config != NULL instead?

> +
>  struct exar8250;
>
>  /**
> @@ -212,6 +252,82 @@ xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
>         return pdev;
>  }
>
> +static int iot2040_rs485_config(struct uart_port *port,
> +                               struct serial_rs485 *rs485)
> +{
> +       u8 __iomem *p = port->membase;
> +       u8 mask = IOT2040_UART1_MASK;
> +       u8 mode, value;

> +       bool is_rs485 = false;
> +
> +       if (rs485->flags & SER_RS485_ENABLED) {
> +               is_rs485 = true;

bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);

if (is_rs485) {
...
} else {
...
}

> +       return 0;
> +}

> +static int iot2040_match(const struct dmi_system_id *dmi)
> +{
> +       is_iot2040 = true;
> +       return 1;
> +}

See above.

> +
> +static const struct dmi_system_id exar_platforms[] = {
> +       {
> +               .callback = iot2040_match,
> +               .ident = "IOT2040",
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
> +                       DMI_MATCH(DMI_BOARD_ASSET_TAG, "6ES7647-0AA00-1YA2"),
> +               },
> +       }
> +};

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Petr Mladek @ 2017-05-18 15:49 UTC (permalink / raw)
  To: Aleksey Makarov
  Cc: Sergey Senozhatsky, Sabrina Dubroca, linux-serial, linux-kernel,
	Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
	Robin Murphy, Steven Rostedt, Nair, Jayachandran,
	Sergey Senozhatsky
In-Reply-To: <7d312633-d040-240d-ec06-ed296b5251ae@linaro.org>

On Sun 2017-05-14 23:37:50, Aleksey Makarov wrote:
> 
> 
> On 05/12/2017 03:57 PM, Petr Mladek wrote:
> >On Thu 2017-05-11 17:41:58, Sergey Senozhatsky wrote:
> >>On (05/11/17 17:24), Sergey Senozhatsky wrote:
> >>>On (05/09/17 10:29), Sabrina Dubroca wrote:
> >>>[..]
> >>>>That's caused a change of behavior in my qemu setup, with this cmdline
> >>>>
> >>>>     root=/dev/sda1 console=ttyS1 console=ttyS0
> >>>>
> >>>>Before, the kernel logs appeared on ttyS1, and I logged in with ttyS0
> >>>>(with my setup, ttyS1 is a file and ttyS0 is unix socket). Now, the
> >>>>kernel logs go to ttyS0. I need to swap the two console= parameters to
> >>>>restore behavior.
> >>>>
> >>>>There might be some other problem (in qemu?) though, because adding
> >>>>console=tty0 anywhere on that cmdline makes the logs appear on both
> >>>>tty0 and one ttyS* (but only one of them, and the ordering of the
> >>>>ttyS* matters).
> >>>
> 
> The problem is that when we are registering a new console,
> we walk over the `console_cmdline` list and match _only one_ of
> the specified consoles, contrary to what stated in
> Documentation/admin-guide/serial-console.rst:
> 
> 	You can specify multiple console= options on the kernel
> 	command line. Output will appear on *all* of them.

and from the other mail:

> Last mentioned 'console=' (preferred console) is the console that
> should become /dev/console.  Its driver is returned by console_device().
> In other respects the last mentioned console is not special,
> so I believe it is irrelevant to the report.

Thanks a lot for explanation. I missed these pieces.

But this also means that your commit cf39bf58afdaabc0b
("printk: fix double printing with earlycon") helps only
when the duplicate of the boot console is defined as the preferred
one.

Well, the reverse order of searching the console_cmdline
might help also in other cases but it is weird approach.


> I emphasized all here.  Moreover, it is impossible to fix this
> without deep reworking of all the console framework.

IMHO, the same is true also for fixing the bug with double
printing correctly. The current fix helps in some situations
but it might break others. The question is how many people will
see the good and "bad" effects.

BTW: I wonder if we really need to add consoles defined
by ACPI SPCR table into the console_cmdline array. I am even
more curious when seeing the following code in
drivers/acpi/spcr.c:

int __init parse_spcr(bool earlycon)
{
[...]
	if (earlycon)
		setup_earlycon(opts);

	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
}

It seems that we setup (register) the early console before
we add it to the console_cmdline array. Do we really need
to call add_preferred_console() for these early consoles?

If we do not call add_preferred_console() here, it should fix
the duplicate output as well. Or do I still miss something?

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH 3/4] tty/serdev: add serdev registration interface
From: Greg Kroah-Hartman @ 2017-05-18 15:39 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Rob Herring, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <20170518153101.GA3657@localhost>

On Thu, May 18, 2017 at 05:31:01PM +0200, Johan Hovold wrote:
> On Thu, May 18, 2017 at 04:43:42PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Apr 11, 2017 at 07:07:30PM +0200, Johan Hovold wrote:
> > > Add a new interface for registering a serdev controller and clients, and
> > > a helper function to deregister serdev devices (or a tty device) that
> > > were previously registered using the new interface.
> > > 
> > > Once every driver currently using the tty_port_register_device() helpers
> > > have been vetted and converted to use the new serdev registration
> > > interface (at least for deregistration), we can move serdev registration
> > > to the current helpers and get rid of the serdev-specific functions.
> > > 
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  drivers/tty/serdev/serdev-ttyport.c |  6 ++-
> > >  drivers/tty/tty_port.c              | 76 +++++++++++++++++++++++++++++++++++++
> > >  include/linux/serdev.h              |  7 +++-
> > >  include/linux/tty.h                 |  9 +++++
> > >  4 files changed, 94 insertions(+), 4 deletions(-)
> > 
> > This patch doesn't apply
> 
> Probably because it was generated against v4.11-rc which did not have
> the write_buf wrapper which you just reverted (after which the patch
> seems to apply just fine again).
> 
> I'll submit a rebased v2 of the two remaining patches anyway.
> 
> > and should it really be a 4.12-final patch?
> 
> I think so. serdev was enabled in 4.11 by hooking into the tty layer at
> the wrong place, which is what this series addresses. I had hoped to get
> at least the revert-patch into 4.11-final, and the rest into 4.12, to
> avoid disabling something which had once been enabled.
> 
> The remaining two patches only re-enables serdev for a subset of tty
> drivers (for which it is safe to do so) by using the new tty-port
> registration functions in serial-core only.
> 
> In effect we'd up with serdev disabled in 4.11 and enabled for
> serial-core in 4.12 (which is the first release that has driver using
> serdev).
> 
> Does that make sense?

Ok, fair enough, now queued up, thanks.

greg k-h

^ permalink raw reply

* [PATCH v2 2/2] serial: enable serdev support
From: Johan Hovold @ 2017-05-18 15:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
In-Reply-To: <20170518153301.28885-1-johan@kernel.org>

Enable serdev support by using the new device-registration helpers.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/tty/serial/serial_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index bc6caea6099f..13bfd5dcffce 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2782,7 +2782,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
 	 * Register the port whether it's detected or not.  This allows
 	 * setserial to be used to alter this port's parameters.
 	 */
-	tty_dev = tty_port_register_device_attr(port, drv->tty_driver,
+	tty_dev = tty_port_register_device_attr_serdev(port, drv->tty_driver,
 			uport->line, uport->dev, port, uport->tty_groups);
 	if (likely(!IS_ERR(tty_dev))) {
 		device_set_wakeup_capable(tty_dev, 1);
@@ -2845,7 +2845,7 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
 	/*
 	 * Remove the devices from the tty layer
 	 */
-	tty_unregister_device(drv->tty_driver, uport->line);
+	tty_port_unregister_device(port, drv->tty_driver, uport->line);
 
 	tty = tty_port_tty_get(port);
 	if (tty) {
-- 
2.13.0

^ permalink raw reply related

* [PATCH v2 1/2] tty/serdev: add serdev registration interface
From: Johan Hovold @ 2017-05-18 15:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
In-Reply-To: <20170518153301.28885-1-johan@kernel.org>

Add a new interface for registering a serdev controller and clients, and
a helper function to deregister serdev devices (or a tty device) that
were previously registered using the new interface.

Once every driver currently using the tty_port_register_device() helpers
have been vetted and converted to use the new serdev registration
interface (at least for deregistration), we can move serdev registration
to the current helpers and get rid of the serdev-specific functions.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/tty/serdev/serdev-ttyport.c |  6 ++-
 drivers/tty/tty_port.c              | 75 +++++++++++++++++++++++++++++++++++++
 include/linux/serdev.h              |  7 +++-
 include/linux/tty.h                 |  9 +++++
 4 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 013efffd2e82..d0a021c93986 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -250,16 +250,18 @@ struct device *serdev_tty_port_register(struct tty_port *port,
 	return ERR_PTR(ret);
 }
 
-void serdev_tty_port_unregister(struct tty_port *port)
+int serdev_tty_port_unregister(struct tty_port *port)
 {
 	struct serdev_controller *ctrl = port->client_data;
 	struct serport *serport = serdev_controller_get_drvdata(ctrl);
 
 	if (!serport)
-		return;
+		return -ENODEV;
 
 	serdev_controller_remove(ctrl);
 	port->client_ops = NULL;
 	port->client_data = NULL;
 	serdev_controller_put(ctrl);
+
+	return 0;
 }
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 88dac3b79369..4fb3165384c4 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -16,6 +16,7 @@
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <linux/serdev.h>
 
 static int tty_port_default_receive_buf(struct tty_port *port,
 					const unsigned char *p,
@@ -136,6 +137,80 @@ struct device *tty_port_register_device_attr(struct tty_port *port,
 }
 EXPORT_SYMBOL_GPL(tty_port_register_device_attr);
 
+/**
+ * tty_port_register_device_attr_serdev - register tty or serdev device
+ * @port: tty_port of the device
+ * @driver: tty_driver for this device
+ * @index: index of the tty
+ * @device: parent if exists, otherwise NULL
+ * @drvdata: driver data for the device
+ * @attr_grp: attribute group for the device
+ *
+ * Register a serdev or tty device depending on if the parent device has any
+ * defined serdev clients or not.
+ */
+struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
+		struct tty_driver *driver, unsigned index,
+		struct device *device, void *drvdata,
+		const struct attribute_group **attr_grp)
+{
+	struct device *dev;
+
+	tty_port_link_device(port, driver, index);
+
+	dev = serdev_tty_port_register(port, device, driver, index);
+	if (PTR_ERR(dev) != -ENODEV) {
+		/* Skip creating cdev if we registered a serdev device */
+		return dev;
+	}
+
+	return tty_register_device_attr(driver, index, device, drvdata,
+			attr_grp);
+}
+EXPORT_SYMBOL_GPL(tty_port_register_device_attr_serdev);
+
+/**
+ * tty_port_register_device_serdev - register tty or serdev device
+ * @port: tty_port of the device
+ * @driver: tty_driver for this device
+ * @index: index of the tty
+ * @device: parent if exists, otherwise NULL
+ *
+ * Register a serdev or tty device depending on if the parent device has any
+ * defined serdev clients or not.
+ */
+struct device *tty_port_register_device_serdev(struct tty_port *port,
+		struct tty_driver *driver, unsigned index,
+		struct device *device)
+{
+	return tty_port_register_device_attr_serdev(port, driver, index,
+			device, NULL, NULL);
+}
+EXPORT_SYMBOL_GPL(tty_port_register_device_serdev);
+
+/**
+ * tty_port_unregister_device - deregister a tty or serdev device
+ * @port: tty_port of the device
+ * @driver: tty_driver for this device
+ * @index: index of the tty
+ *
+ * If a tty or serdev device is registered with a call to
+ * tty_port_register_device_serdev() then this function must be called when
+ * the device is gone.
+ */
+void tty_port_unregister_device(struct tty_port *port,
+		struct tty_driver *driver, unsigned index)
+{
+	int ret;
+
+	ret = serdev_tty_port_unregister(port);
+	if (ret == 0)
+		return;
+
+	tty_unregister_device(driver, index);
+}
+EXPORT_SYMBOL_GPL(tty_port_unregister_device);
+
 int tty_port_alloc_xmit_buf(struct tty_port *port)
 {
 	/* We may sleep in get_zeroed_page() */
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index e2a225bf716d..e69402d4a8ae 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -308,7 +308,7 @@ struct tty_driver;
 struct device *serdev_tty_port_register(struct tty_port *port,
 					struct device *parent,
 					struct tty_driver *drv, int idx);
-void serdev_tty_port_unregister(struct tty_port *port);
+int serdev_tty_port_unregister(struct tty_port *port);
 #else
 static inline struct device *serdev_tty_port_register(struct tty_port *port,
 					   struct device *parent,
@@ -316,7 +316,10 @@ static inline struct device *serdev_tty_port_register(struct tty_port *port,
 {
 	return ERR_PTR(-ENODEV);
 }
-static inline void serdev_tty_port_unregister(struct tty_port *port) {}
+static inline int serdev_tty_port_unregister(struct tty_port *port)
+{
+	return -ENODEV;
+}
 #endif /* CONFIG_SERIAL_DEV_CTRL_TTYPORT */
 
 #endif /*_LINUX_SERDEV_H */
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d07cd2105a6c..eccb4ec30a8a 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -558,6 +558,15 @@ extern struct device *tty_port_register_device_attr(struct tty_port *port,
 		struct tty_driver *driver, unsigned index,
 		struct device *device, void *drvdata,
 		const struct attribute_group **attr_grp);
+extern struct device *tty_port_register_device_serdev(struct tty_port *port,
+		struct tty_driver *driver, unsigned index,
+		struct device *device);
+extern struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
+		struct tty_driver *driver, unsigned index,
+		struct device *device, void *drvdata,
+		const struct attribute_group **attr_grp);
+extern void tty_port_unregister_device(struct tty_port *port,
+		struct tty_driver *driver, unsigned index);
 extern int tty_port_alloc_xmit_buf(struct tty_port *port);
 extern void tty_port_free_xmit_buf(struct tty_port *port);
 extern void tty_port_destroy(struct tty_port *port);
-- 
2.13.0

^ permalink raw reply related

* [PATCH v2 0/2] serdev: fix broken lifetime assumptions
From: Johan Hovold @ 2017-05-18 15:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Jiri Slaby, linux-serial, linux-kernel, Johan Hovold

This series fixes a number of issues with the new serdev code, which was
based on incorrect tty-port lifetime assumptions.

The first patch in v1 of this series disables serdev support by reverting the
patch which hooked into the tty layer in a broken way that leads to crashes and
leaks when deregistering devices. That one has now been applied to tty-linus
with a stable tag (for v4.11).

The remaining two patches again re-enables serdev (first enabled in 4.11, but
soon reverted in stable) for a subset of tty drivers by adding adding a new
interface for registering serdev devices and using that in serial core only.

More details can be found in the individual commit messages.

Johan


Changes in v2
 - rebase on tty-linus
 - drop first two patches that have already been applied
 - remove a stray newline in patch 1/2
 - add Rob's reviewed-by tag


Johan Hovold (2):
  tty/serdev: add serdev registration interface
  serial: enable serdev support

 drivers/tty/serdev/serdev-ttyport.c |  6 ++-
 drivers/tty/serial/serial_core.c    |  4 +-
 drivers/tty/tty_port.c              | 75 +++++++++++++++++++++++++++++++++++++
 include/linux/serdev.h              |  7 +++-
 include/linux/tty.h                 |  9 +++++
 5 files changed, 95 insertions(+), 6 deletions(-)

-- 
2.13.0

^ permalink raw reply

* Re: [PATCH 3/4] tty/serdev: add serdev registration interface
From: Johan Hovold @ 2017-05-18 15:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Johan Hovold, Rob Herring, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <20170518144342.GA10503@kroah.com>

On Thu, May 18, 2017 at 04:43:42PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 11, 2017 at 07:07:30PM +0200, Johan Hovold wrote:
> > Add a new interface for registering a serdev controller and clients, and
> > a helper function to deregister serdev devices (or a tty device) that
> > were previously registered using the new interface.
> > 
> > Once every driver currently using the tty_port_register_device() helpers
> > have been vetted and converted to use the new serdev registration
> > interface (at least for deregistration), we can move serdev registration
> > to the current helpers and get rid of the serdev-specific functions.
> > 
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/tty/serdev/serdev-ttyport.c |  6 ++-
> >  drivers/tty/tty_port.c              | 76 +++++++++++++++++++++++++++++++++++++
> >  include/linux/serdev.h              |  7 +++-
> >  include/linux/tty.h                 |  9 +++++
> >  4 files changed, 94 insertions(+), 4 deletions(-)
> 
> This patch doesn't apply

Probably because it was generated against v4.11-rc which did not have
the write_buf wrapper which you just reverted (after which the patch
seems to apply just fine again).

I'll submit a rebased v2 of the two remaining patches anyway.

> and should it really be a 4.12-final patch?

I think so. serdev was enabled in 4.11 by hooking into the tty layer at
the wrong place, which is what this series addresses. I had hoped to get
at least the revert-patch into 4.11-final, and the rest into 4.12, to
avoid disabling something which had once been enabled.

The remaining two patches only re-enables serdev for a subset of tty
drivers (for which it is safe to do so) by using the new tty-port
registration functions in serial-core only.

In effect we'd up with serdev disabled in 4.11 and enabled for
serial-core in 4.12 (which is the first release that has driver using
serdev).

Does that make sense?

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH v2 4/6] gpio: exar: Fix reading of directions and values
From: Andy Shevchenko @ 2017-05-18 15:28 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
	Linux Kernel Mailing List, linux-serial@vger.kernel.org,
	linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <ace08e630ab57d08817c5e8669094c10a7c667ad.1495119548.git.jan.kiszka@siemens.com>

On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> First, the logic for translating a register bit to the return code of
> exar_get_direction and exar_get_value were wrong. And second, there was
> a flip regarding the register bank in exar_get_direction.
>

FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/gpio/gpio-exar.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
> index 4ed1f8bdeda7..d8b6296c11de 100644
> --- a/drivers/gpio/gpio-exar.c
> +++ b/drivers/gpio/gpio-exar.c
> @@ -69,7 +69,7 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
>         value = readb(exar_gpio->regs + reg);
>         mutex_unlock(&exar_gpio->lock);
>
> -       return !!value;
> +       return value;
>  }
>
>  static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
> @@ -79,7 +79,7 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
>         int val;
>
>         addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
> -       val = exar_get(chip, addr) >> (offset % 8);
> +       val = exar_get(chip, addr) & BIT(offset % 8);
>
>         return !!val;
>  }
> @@ -90,8 +90,8 @@ static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
>         unsigned int addr;
>         int val;
>
> -       addr = bank ? EXAR_OFFSET_MPIOLVL_LO : EXAR_OFFSET_MPIOLVL_HI;
> -       val = exar_get(chip, addr) >> (offset % 8);
> +       addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
> +       val = exar_get(chip, addr) & BIT(offset % 8);
>
>         return !!val;
>  }
> --
> 2.12.0
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1495119548.git.jan.kiszka@siemens.com>

This implements the setup of RS232 and the switch-over to RS485 or RS422
for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
to switch between the different modes. The external logic is controlled
via MPIO pins of the EXAR controller.

Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
an LED.

As the XR17V352 used on the IOT2040 is not equipped with an external
EEPROM, it cannot present itself as IOT2040-variant via subvendor/
subdevice IDs. Thus, we have to check via DMI for the target platform.

Co-developed with Sascha Weisenberger.

Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/tty/serial/8250/8250_exar.c | 136 ++++++++++++++++++++++++++++++++++--
 1 file changed, 131 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index d9c52288d6c9..5da6f1c27b50 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -9,6 +9,7 @@
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License.
  */
+#include <linux/dmi.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -61,6 +62,45 @@
 #define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
 #define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
 
+#define UART_EXAR_RS485_DLY(x)	(x << 4)
+
+/*
+ * IOT2040 MPIO wiring semantics:
+ *
+ * MPIO		Port	Function
+ * ----		----	--------
+ * 0		2 	Mode bit 0
+ * 1		2	Mode bit 1
+ * 2		2	Terminate bus
+ * 3		-	<reserved>
+ * 4		3	Mode bit 0
+ * 5		3	Mode bit 1
+ * 6		3	Terminate bus
+ * 7		-	<reserved>
+ * 8		2	Enable
+ * 9		3	Enable
+ * 10		-	Red LED
+ * 11..15	-	<unused>
+ */
+
+/* IOT2040 MPIOs 0..7 */
+#define IOT2040_UART_MODE_RS232		0x01
+#define IOT2040_UART_MODE_RS485		0x02
+#define IOT2040_UART_MODE_RS422		0x03
+#define IOT2040_UART_TERMINATE_BUS	0x04
+
+#define IOT2040_UART1_MASK		0x0f
+#define IOT2040_UART2_SHIFT		4
+
+#define IOT2040_UARTS_DEFAULT_MODE	0x11	/* both RS232 */
+#define IOT2040_UARTS_GPIO_LO_MODE	0x88	/* reserved pins as input */
+
+/* IOT2040 MPIOs 8..15 */
+#define IOT2040_UARTS_ENABLE		0x03
+#define IOT2040_UARTS_GPIO_HI_MODE	0xF8	/* enable & LED as outputs */
+
+static bool is_iot2040;
+
 struct exar8250;
 
 /**
@@ -212,6 +252,82 @@ xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
 	return pdev;
 }
 
+static int iot2040_rs485_config(struct uart_port *port,
+				struct serial_rs485 *rs485)
+{
+	u8 __iomem *p = port->membase;
+	u8 mask = IOT2040_UART1_MASK;
+	u8 mode, value;
+	bool is_rs485 = false;
+
+	if (rs485->flags & SER_RS485_ENABLED) {
+		is_rs485 = true;
+		if (rs485->flags & SER_RS485_RX_DURING_TX)
+			mode = IOT2040_UART_MODE_RS422;
+		else
+			mode = IOT2040_UART_MODE_RS485;
+
+		if (rs485->flags & SER_RS485_TERMINATE_BUS)
+			mode |= IOT2040_UART_TERMINATE_BUS;
+	} else {
+		mode = IOT2040_UART_MODE_RS232;
+	}
+
+	if (port->line == 3) {
+		mask <<= IOT2040_UART2_SHIFT;
+		mode <<= IOT2040_UART2_SHIFT;
+	}
+
+	value = readb(p + UART_EXAR_MPIOLVL_7_0);
+	value &= ~mask;
+	value |= mode;
+	writeb(value, p + UART_EXAR_MPIOLVL_7_0);
+
+	value = readb(p + UART_EXAR_FCTR);
+	if (is_rs485)
+		value |= UART_FCTR_EXAR_485;
+	else
+		value &= ~UART_FCTR_EXAR_485;
+	writeb(value, p + UART_EXAR_FCTR);
+
+	if (is_rs485)
+		writeb(UART_EXAR_RS485_DLY(4), p + UART_MSR);
+
+	return 0;
+}
+
+static int iot2040_setup_gpio(struct pci_dev *pcidev,
+			      struct uart_8250_port *port)
+{
+	u8 __iomem *p = port->port.membase;
+
+	writeb(IOT2040_UARTS_DEFAULT_MODE, p + UART_EXAR_MPIOLVL_7_0);
+	writeb(IOT2040_UARTS_GPIO_LO_MODE, p + UART_EXAR_MPIOSEL_7_0);
+	writeb(IOT2040_UARTS_ENABLE, p + UART_EXAR_MPIOLVL_15_8);
+	writeb(IOT2040_UARTS_GPIO_HI_MODE, p + UART_EXAR_MPIOSEL_15_8);
+
+	port->port.private_data = xr17v35x_register_gpio(pcidev, 10, 1);
+
+	return 0;
+}
+
+static int iot2040_match(const struct dmi_system_id *dmi)
+{
+	is_iot2040 = true;
+	return 1;
+}
+
+static const struct dmi_system_id exar_platforms[] = {
+	{
+		.callback = iot2040_match,
+		.ident = "IOT2040",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
+			DMI_MATCH(DMI_BOARD_ASSET_TAG, "6ES7647-0AA00-1YA2"),
+		},
+	}
+};
+
 static int
 pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		   struct uart_8250_port *port, int idx)
@@ -222,7 +338,13 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 	u8 __iomem *p;
 	int ret;
 
+	dmi_check_system(exar_platforms);
+
 	port->port.uartclk = baud * 16;
+
+	if (is_iot2040)
+		port->port.rs485_config = iot2040_rs485_config;
+
 	/*
 	 * Setup the uart clock for the devices on expansion slot to
 	 * half the clock speed of the main chip (which is 125MHz)
@@ -241,14 +363,18 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 	writeb(128, p + UART_EXAR_TXTRG);
 	writeb(128, p + UART_EXAR_RXTRG);
 
-	if (idx == 0) {
-		/* Setup Multipurpose Input/Output pins. */
-		setup_gpio(p);
+	if (idx != 0)
+		return 0;
+
+	/* Setup Multipurpose Input/Output pins. */
+	setup_gpio(p);
 
+	if (is_iot2040)
+		ret = iot2040_setup_gpio(pcidev, port);
+	else
 		port->port.private_data = xr17v35x_register_gpio(pcidev, 0, 16);
-	}
 
-	return 0;
+	return ret;
 }
 
 static void pci_xr17v35x_exit(struct pci_dev *pcidev)
-- 
2.12.0


^ permalink raw reply related

* [PATCH v2 5/6] gpio-exar/8250-exar: Make set of exported GPIOs configurable
From: Jan Kiszka @ 2017-05-18 14:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
  Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1495119548.git.jan.kiszka@siemens.com>

On the SIMATIC, IOT2040 only a single pin is exportable as GPIO, the
rest is required to operate the UART. To allow modeling this case,
expand the platform device data structure to specify a (consecutive) pin
subset for exporting by the gpio-exar driver.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/gpio/gpio-exar.c                | 28 +++++++++++++++++++---------
 drivers/tty/serial/8250/8250_exar.c     |  7 +++++--
 include/linux/platform_data/gpio-exar.h |  3 +++
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index d8b6296c11de..e0698a1ae012 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -32,6 +32,7 @@ struct exar_gpio_chip {
 	int index;
 	void __iomem *regs;
 	char name[20];
+	unsigned int first_gpio;
 };
 
 static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
@@ -52,9 +53,11 @@ static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
 static int exar_set_direction(struct gpio_chip *chip, int direction,
 			      unsigned int offset)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
 	exar_update(chip, addr, direction, offset % 8);
 	return 0;
@@ -74,10 +77,12 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
 
 static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 	int val;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
 	val = exar_get(chip, addr) & BIT(offset % 8);
 
@@ -86,10 +91,12 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
 
 static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 	int val;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
 	val = exar_get(chip, addr) & BIT(offset % 8);
 
@@ -99,9 +106,11 @@ static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
 static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
 			   int value)
 {
-	unsigned int bank = offset / 8;
-	unsigned int addr;
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	unsigned int bank, addr;
 
+	offset += exar_gpio->first_gpio;
+	bank = offset / 8;
 	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
 	exar_update(chip, addr, value, offset % 8);
 }
@@ -154,9 +163,10 @@ static int gpio_exar_probe(struct platform_device *pdev)
 	exar_gpio->gpio_chip.get = exar_get_value;
 	exar_gpio->gpio_chip.set = exar_set_value;
 	exar_gpio->gpio_chip.base = -1;
-	exar_gpio->gpio_chip.ngpio = 16;
+	exar_gpio->gpio_chip.ngpio = pdata->ngpio;
 	exar_gpio->regs = p;
 	exar_gpio->index = index;
+	exar_gpio->first_gpio = pdata->first_gpio;
 
 	ret = devm_gpiochip_add_data(&pdev->dev,
 				     &exar_gpio->gpio_chip, exar_gpio);
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 0a806daaff8b..d9c52288d6c9 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -189,7 +189,8 @@ static void setup_gpio(u8 __iomem *p)
 }
 
 static void *
-xr17v35x_register_gpio(struct pci_dev *pcidev)
+xr17v35x_register_gpio(struct pci_dev *pcidev, unsigned int first_gpio,
+		       unsigned int ngpio)
 {
 	struct platform_device *pdev;
 	struct gpio_exar_pdata pdata;
@@ -199,6 +200,8 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
 		return NULL;
 
 	pdata.parent = pcidev;
+	pdata.first_gpio = first_gpio;
+	pdata.ngpio = ngpio;
 
 	if (platform_device_add_data(pdev, &pdata, sizeof(pdata)) < 0 ||
 	    platform_device_add(pdev) < 0) {
@@ -242,7 +245,7 @@ pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
 		/* Setup Multipurpose Input/Output pins. */
 		setup_gpio(p);
 
-		port->port.private_data = xr17v35x_register_gpio(pcidev);
+		port->port.private_data = xr17v35x_register_gpio(pcidev, 0, 16);
 	}
 
 	return 0;
diff --git a/include/linux/platform_data/gpio-exar.h b/include/linux/platform_data/gpio-exar.h
index 1a13e9ddaf7d..1754f5a2842d 100644
--- a/include/linux/platform_data/gpio-exar.h
+++ b/include/linux/platform_data/gpio-exar.h
@@ -17,6 +17,9 @@ struct pci_dev;
 
 struct gpio_exar_pdata {
 	struct pci_dev *parent;
+
+	unsigned int first_gpio;
+	unsigned int ngpio;
 };
 
 #endif /* __GPIO_EXAR_PDATA_H */
-- 
2.12.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox