* Re: [PATCH v6 00/10] serial/gpio: exar: Fixes and support for IOT2000
From: Linus Walleij @ 2017-06-29 9:51 UTC (permalink / raw)
To: Jan Kiszka
Cc: Alexandre Courbot, Greg Kroah-Hartman, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <d52a1a65-842c-9c73-17f7-5574fcefab2a@siemens.com>
On Fri, Jun 23, 2017 at 10:09 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-06-21 08:29, Jan Kiszka wrote:
>> On 2017-06-20 13:38, Linus Walleij wrote:
>>> On Tue, Jun 20, 2017 at 10:54 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2017-06-20 10:19, Linus Walleij wrote:
>>>>> On Fri, Jun 9, 2017 at 8:33 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>
>>>>>> This makes the gpio-exar driver usable, which was prevented by a number
>>>>>> of fatal bugs, and adds support for the SIMATIC IOT2040 to the 8250-exar
>>>>>> driver and, indirectly, to gpio-exar as well. It's a cross-subsystem
>>>>>> series, so I'm also cross-posting to the serial and gpio lists.
>>>>>>
>>>>>> Changes in v6:
>>>>>
>>>>> I merged some of the patches, that applied. Let's see if they survive
>>>>> in linux-next, else I guess we need to fix this in the -rcs or for the next
>>>>> kernel cycle.
>>>>
>>>> Weird that things did not apply. I just did a cherry-pick for all those
>>>> 10 patches on top of 5c996b7eb52c, and that went smoothly. Please let me
>>>> know which baseline is needed, and I will rebase.
>>>
>>> This was on the "devel" branch of my GPIO tree:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=devel
>>
>> Again, all (missing) patches applied absolutely cleanly for me, see
>>
>> http://git.kiszka.org/?p=linux.git;a=shortlog;h=refs/heads/queues/gpio-iot2000
>
> I've rebased over devel and noticed along this that you will have to
> include "serial: uapi: Add support for bus termination" from Greg's
> tty-next for the last patch to avoid breaking the build. Result pushed to
>
> http://git.kiszka.org/?p=linux.git;a=shortlog;h=refs/heads/queues/gpio-iot2000
>
> If you want me to resend anything, just let me know. Would be great to
> have the remaining pieces lined up in time for the next merge window.
I could just pull it in, but then I want a clear indication that Greg and
say Andy are happy with this.
The serial patches are missing Gregs explicit ACK and the platform
patch could use Andy's ACK. The last patch adding the
IOT2040 is so obviously out of my GPIO territory that I want
Greg 100% aligned with this.
I am sorry that it is troublesome when things cross subsystem
boundaries, I already stretched it a bit with the things I queued
up because it annoys me too, but this is the process we have :/
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 17/20] dt-bindings: serial: stm32: add dma using note
From: Rob Herring @ 2017-06-28 22:26 UTC (permalink / raw)
To: Bich HEMON
Cc: Greg Kroah-Hartman, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, Jiri Slaby,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1498481318-1894-18-git-send-email-bich.hemon-qxv4g6HH51o@public.gmane.org>
On Mon, Jun 26, 2017 at 12:49:16PM +0000, Bich HEMON wrote:
> From: Bich Hemon <bich.hemon-qxv4g6HH51o@public.gmane.org>
>
> Signed-off-by: Gerald Baeza <gerald.baeza-qxv4g6HH51o@public.gmane.org>
> ---
> .../devicetree/bindings/serial/st,stm32-usart.txt | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
Sounds like broken DMA to me.
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 14/20] dt-bindings: serial: document option wake-up interrupt for STM32 USART
From: Rob Herring @ 2017-06-28 18:10 UTC (permalink / raw)
To: Bich HEMON
Cc: Greg Kroah-Hartman, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, Jiri Slaby,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1498481318-1894-15-git-send-email-bich.hemon-qxv4g6HH51o@public.gmane.org>
On Mon, Jun 26, 2017 at 12:49:15PM +0000, Bich HEMON wrote:
> From: Bich Hemon <bich.hemon-qxv4g6HH51o@public.gmane.org>
>
> Introduce new compatibles for "st,stm32h7-usart" and "st,stm32h7-uart".
> This new compatible allow to use optional wake-up interrupt.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> ---
> .../devicetree/bindings/serial/st,stm32-usart.txt | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/st,stm32-usart.txt b/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
> index a229b14..3b42138 100644
> --- a/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
> +++ b/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
> @@ -2,11 +2,14 @@
>
> Required properties:
> - compatible: Can be either "st,stm32-usart", "st,stm32-uart",
> -"st,stm32f7-usart" or "st,stm32f7-uart" depending on whether
> -the device supports synchronous mode and is compatible with
> -stm32(f4) or stm32f7.
> +"st,stm32f7-usart", "st,stm32f7-uart", "st,stm32h7-usart" or
> +st,stm32h7-uart depending on whether the device supports synchronous
> +mode and is compatible with stm32(f4), stm32f7 or stm32h7.
Please reformat as 1 valid set of compatibles per line.
> - reg: The address and length of the peripheral registers space
> -- interrupts: The interrupt line of the USART instance
> +- interrupts or interrupts-extended: Must contain first, the interrupt
> + line specifier for the USART instance. An optional wake-up interrupt
> + specifier can be added. Then, -extended variant may be needed to specify
> + interrupts parent controllers.
Just describe the interrupts property and what the interrupts are.
interrupts-extended is implied if it is necessary.
> - clocks: The input clock of the USART instance
>
> Optional properties:
> @@ -53,3 +56,12 @@ usart1: serial@40011000 {
> <&dma2 7 4 0x414 0x0>;
> dma-names = "rx", "tx";
> };
> +
> +uart4: serial@40010000 {
> + compatible = "st,stm32h7-uart";
> + reg = <0x40010000 0x400>;
> + interrupts-extended = <&intc GIC_SPI 52 IRQ_TYPE_NONE>,
> + <&aiec 30 1>;
> + clocks = <&rcc UART4_K>;
> + status = "disabled";
> +};
> --
> 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 02/20] dt-bindings: serial: each stm32 usart needs an alias
From: Rob Herring @ 2017-06-28 18:06 UTC (permalink / raw)
To: Bich HEMON
Cc: Greg Kroah-Hartman, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, Jiri Slaby,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1498481318-1894-3-git-send-email-bich.hemon-qxv4g6HH51o@public.gmane.org>
On Mon, Jun 26, 2017 at 12:49:09PM +0000, Bich HEMON wrote:
> From: Bich Hemon <bich.hemon-qxv4g6HH51o@public.gmane.org>
>
> Each usart controller should have an alias correctly
> numbered in "aliases" node.
This is board specific typically and doesn't need to be in the binding
doc.
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 v3 9/9] of: document rs485 bindings for Atmel USART, Freescale UARTs and OMAP UART
From: Rob Herring @ 2017-06-28 18:04 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: kernel, devicetree, linux-arm-kernel, linux-serial
In-Reply-To: <20170626090803.10981-10-u.kleine-koenig@pengutronix.de>
On Mon, Jun 26, 2017 at 11:08:03AM +0200, Uwe Kleine-König wrote:
> The drivers support the rs485 binding described in rs485.txt, this commit
> just makes that explicit.
"dt-bindings: serial: ..." for the subject if you respin the series.
Otherwise,
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 2/9] dt-bindings: serial/rs485: make rs485-rts-delay optional
From: Rob Herring @ 2017-06-28 16:48 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170626090803.10981-3-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Mon, Jun 26, 2017 at 11:07:56AM +0200, Uwe Kleine-König wrote:
> There are a few device trees that specify one of the already optional
> properties without also having the up to now required property
> rs485-rts-delay. Additionally there is no technical reason to require
> rs485-rts-delay and that's also what most drivers implement.
>
> So give existing users and implementers a blessing and document
> rs485-rts-delay as optional.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Documentation/devicetree/bindings/serial/rs485.txt | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Johan Hovold @ 2017-06-28 15:35 UTC (permalink / raw)
To: Stuart Longland
Cc: Oliver Neukum, Johan Hovold, Alan Cox,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ae2907c2-cb0f-61e4-c6f2-af3dcec93ace-3e+Fe6x+DsgJbe36r25VNhCuuivNXqWP@public.gmane.org>
On Wed, Jun 28, 2017 at 08:07:54AM +1000, Stuart Longland wrote:
> This beats the official Moxa solution: their Windows driver wants a
> reboot of the entire machine to switch between serial standards.
Interesting approach.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Johan Hovold @ 2017-06-28 15:34 UTC (permalink / raw)
To: Oliver Neukum
Cc: Stuart Longland, Johan Hovold, Alan Cox,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1498565331.30390.5.camel-IBi9RG/b67k@public.gmane.org>
On Tue, Jun 27, 2017 at 02:08:51PM +0200, Oliver Neukum wrote:
> Am Dienstag, den 27.06.2017, 19:43 +1000 schrieb Stuart Longland:
> > Maybe a good start would be a "standard" option (referring to the
> > physical signalling standard, TTL/RS-232/RS-422/RS-485), that lists the
> > available standards when read and shows the "selected" standard in
> > brackets (like the 'trigger' option of the LEDs sysfs interface)… so for
> > this case:
> >
> > # cat /sys/class/tty/ttyUSB0/standard
> > [rs232] rs422 rs485 rs485fd
> >
> > and to select 4-wire ("full duplex") RS-485, one does:
> > # echo rs485fd > /sys/class/tty/ttyUSB0/standard
>
> This looks like something that people will put into udev.
> So the switch will be done via udev, but the user not necessarily
> started via udev. Looks like a race to me.
Sure, but since changing the electrical interface arguably should be a
privileged operation, using an ioctl for this would not solve the race
when the interface is changed from an init-script and a non-privileged
application opens the port.
We also have the easy device-tree case, where such parameters could be
parsed at probe and all would be good. This could be used to handle some
semi-static cases involving USB devices eventually too.
So this is mostly an issue for systems not using OF and for hot-plugging,
where udev could race with the application.
The latter could of course always check that the desired mode has been
set before opening the port, but then I guess we're in some sense back
at encoding the physical setup in the application.
> And what do you if the interface is already opened and the sysfs
> interface is used?
Refuse switching mode and returning -EBUSY?
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Johan Hovold @ 2017-06-28 15:16 UTC (permalink / raw)
To: Stuart Longland
Cc: Johan Hovold, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Alan Cox
In-Reply-To: <42601356-656d-57d0-d366-a0933780b8f9-3e+Fe6x+DsgJbe36r25VNhCuuivNXqWP@public.gmane.org>
On Tue, Jun 27, 2017 at 07:43:28PM +1000, Stuart Longland wrote:
> On 27/06/17 19:04, Johan Hovold wrote:
> >> What's the "proper" way to configure such ports for RS-422 and/or 4-wire
> >> RS-485? Is there an example driver in the tree that I can use as a
> >> guide to porting Moxa's mode-switch code over to the ti_usb_3410_5052
> >> driver?
> >
> > I'm afraid not. We have the TIOCSRS485 ioctl, but that doesn't really
> > fit here (currently) as we're not just enabling 2-wire-rs485 style
> > signalling (using RTS to control the tranceiver), but are also
> > configuring the electrical interface (e.g. which pins on the DB9
> > connector are being used for which functions).
> >
> > This question is raised every now and then; typically when a new
> > driver is being merged (e.g. mxuport or when support for your device was
> > added), but we have yet to decide on the proper interface.
> >
> > Extending the TIOCSRS485 ioctl has been discussed, and one alternative
> > could be a sysfs-based interface as this would typically be something
> > you'd want to configure at boot and before opening the device (and
> > toggling DTR).
>
> I did consider sysfs this morning… this would make a lot of sense.
> These things being USB, it is possible that a udev trigger could put it
> in the appropriate mode when they are plugged in. Sounds a much better
> option than re-purposing other fields.
Unless there are too many attributes to set at least, otherwise a single
ioctl might still be still be more convenient. Note that there is room
for extending the TIOCSRS485 ioctl.
> > Either way, we need do determine the right level of abstraction as some
> > of the related properties are fairly device dependent (master/slave
> > mode, terminating resistors, auto direction, echo, etc).
> >
> > For the record, note that we do have the mxser driver, and a few more
> > old drivers, implementing custom ioctls for this, but we really should
> > not be adding more of those.
>
> Yeah, I won't be adding ioctls… it makes the interface too
> hardware-specific. If anything, the ioctls might be a hint to what
> enumerated properties to expose on sysfs, then the ioctl interface can
> be gradually phased out.
>
> I'll have a look at mxser. At this point I'm thinking gathering a
> listing of what devices support what configuration options would be
> worthwhile. I know the UPort 1150 supports 2-wire RS-485, 4-wire
> RS-485, RS-422 and RS-232… the UPort 1100 drops support for RS-232… I
> don't think either offer termination resistance options.
There are more Moxa devices with rs485-support, including those managed
by the mxuport driver. Some Edgeport devices apparently support some of
those more exotic settings like termination, see
https://marc.info/?l=linux-usb&m=129798628514104&w=2
The above thread also has some discussion about extending the TIOCSRS485
ioctl.
Other USB serial devices that have some support for rs485 include cp210x
(using a gpio pin to control the tranceiver), and some Fintek devices.
Also note that the vendor driver for UPort 1150 treats rs422 and
rs485-4w the same and really only has three distinct modes (rs232, and
rs485 with receiver enabled or disabled).
> Maybe a good start would be a "standard" option (referring to the
> physical signalling standard, TTL/RS-232/RS-422/RS-485), that lists the
> available standards when read and shows the "selected" standard in
> brackets (like the 'trigger' option of the LEDs sysfs interface)… so for
> this case:
>
> # cat /sys/class/tty/ttyUSB0/standard
> [rs232] rs422 rs485 rs485fd
Yeah, I once suggested something like this for Moxa, but I think we need
to at least consider the various parameters available first if we are to
come up with a generic interface.
> and to select 4-wire ("full duplex") RS-485, one does:
> # echo rs485fd > /sys/class/tty/ttyUSB0/standard
>
> A hypothetical dongle that supports pin re-mapping might have
> properties: rxd, txd, rts, cts, dtr, dsr, cd, ri, gnd (for RS-232; maybe
> a, b and gnd for RS-485 or txa, txb, rxa, rxb, gnd for
> RS-422/full-duplex-RS-485) that gives the pin numbers/labels:
>
> cd: [1] 2 3 4 5 6 7 8 9
> rx: 1 [2] 3 4 5 6 7 8 9
> tx: 1 2 [3] 4 5 6 7 8 9
> … etc …
This I think might be taking it too far at this stage though. :)
> and termination resistance (separate options for RX and TX) might be a
> selection of off, and the available resistances. (e.g. "50", "75",
> "120", "1k" … whatever the device supports.)
A binary setting here might suffice.
> I'm open to ideas here, I'm not sure how many configurable serial
> interfaces there are out there (USB or otherwise), so if people watching
> the list can provide some examples of these devices, this will be a big
> help.
I mention a few above, and there are some old ones like rocket and mxser
that have a custom ioctl for this.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Bjørn Mork @ 2017-06-28 14:59 UTC (permalink / raw)
To: Oliver Neukum
Cc: Greg KH, Johan Hovold, Stuart Longland, Alan Cox,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1498653709.10278.17.camel-IBi9RG/b67k@public.gmane.org>
Oliver Neukum <oneukum-IBi9RG/b67k@public.gmane.org> writes:
> Am Mittwoch, den 28.06.2017, 14:01 +0200 schrieb Greg KH:
>> On Wed, Jun 28, 2017 at 11:19:09AM +0200, Oliver Neukum wrote:
>> >
>> > 1) module option
>>
>> No, just no, sorry.
>
> I am sorry, but this is just ideological an answer. This is rather
> specialized hardware, so it will need a system specifically configured
> for this task. No distro will configure this on the fly.
Using two identical adapters in different modes is at least
theoretically possible, isn't it?
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* [PATCH RESEND] serial: imx: Enable RTSD only when needed
From: Romain Perier @ 2017-06-28 13:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Romain Perier, linux-arm-kernel, linux-serial, Nandor Han
From: Nandor Han <nandor.han@ge.com>
Currently, this IRQ is always enabled. Some devices might mux these pins
to other I/Os, like I2C. This could lead to spurious interrupts.
This commit makes this IRQ optional, by using the field have_rtscts.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
drivers/tty/serial/imx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 7327477..5437b34 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1302,7 +1302,9 @@ static int imx_startup(struct uart_port *port)
imx_enable_dma(sport);
temp = readl(sport->port.membase + UCR1);
- temp |= UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN;
+ temp |= UCR1_RRDYEN | UCR1_UARTEN;
+ if (sport->have_rtscts)
+ temp |= UCR1_RTSDEN;
writel(temp, sport->port.membase + UCR1);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Uwe Kleine-König @ 2017-06-28 13:04 UTC (permalink / raw)
To: Phil Elwell
Cc: Peter Hurley, Greg Kroah-Hartman, Yegor Yefremov,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Jan Kiszka, Andy Shevchenko
In-Reply-To: <95a13280-e9f6-50f5-bb0c-8bcdefd0bf12-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hello,
On Wed, Jun 28, 2017 at 12:52:46PM +0100, Phil Elwell wrote:
> On 28/06/2017 11:57, Uwe Kleine-König wrote:
> > nitpick: The order of lines in the Sob area matter and you should add
> > your Sob at the end. So as it was you adding the ack by Eric and Andy,
> > it should look as follows:
> >
> > Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> > Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
>
> Really? I thought the submitter went first and the final merger went
> last, with other reviewers in-between, like this:
>
> commit 6df765dca378bddf994cfd2044acafa501bd800f
> Author: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Date: Wed May 24 21:38:46 2017 +0200
>
> serial: imx: ensure UCR3 and UFCR are setup correctly
>
> [...]
>
> Fixes: e61c38d85b73 ("serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Acked-by: Mika Penttilä <mika.penttila-MRsr7dthA9VWk0Htik3J/w@public.gmane.org>
> Tested-by: Mika Penttilä <mika.penttila-MRsr7dthA9VWk0Htik3J/w@public.gmane.org>
> Acked-by: Steve Twiss <stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> Tested-by: Steve Twiss <stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Right, I would expect that it was Greg who added the Acked and Tested-by
tags when reading this.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Oliver Neukum @ 2017-06-28 12:41 UTC (permalink / raw)
To: Greg KH
Cc: Johan Hovold, Stuart Longland, Alan Cox,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170628120120.GA10502-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
Am Mittwoch, den 28.06.2017, 14:01 +0200 schrieb Greg KH:
> On Wed, Jun 28, 2017 at 11:19:09AM +0200, Oliver Neukum wrote:
> >
> > Am Mittwoch, den 28.06.2017, 08:07 +1000 schrieb Stuart Longland:
> >
> > Hi,
> >
> > >
> > > >
> > > > >
> > > > > and to select 4-wire ("full duplex") RS-485, one does:
> > > > > # echo rs485fd > /sys/class/tty/ttyUSB0/standard
> > > > This looks like something that people will put into udev.
> > > > So the switch will be done via udev, but the user not necessarily
> > > > started via udev. Looks like a race to me.
> > >
> > > It is a race, but unless a daemon is watching for the very moment that a
> > > particular serial interface appears, it's one that init scripts are very
> > > likely to win.
> >
> > That is precisely what a demon might do.
> >
> > >
> > > Short of having a flag that blocks reads/writes until a given sysfs file
> > > is written to say "I'm ready, go ahead"… there aren't that many options
> > > I can think of.
> >
> > 1) module option
>
> No, just no, sorry.
>
Hi,
I am sorry, but this is just ideological an answer. This is rather
specialized hardware, so it will need a system specifically configured
for this task. No distro will configure this on the fly.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Andy Shevchenko @ 2017-06-28 12:01 UTC (permalink / raw)
To: Uwe Kleine-König, Phil Elwell
Cc: Peter Hurley, Greg Kroah-Hartman, Yegor Yefremov,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Jan Kiszka
In-Reply-To: <20170628105744.42bchlftl6khvsxd-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Wed, 2017-06-28 at 12:57 +0200, Uwe Kleine-König wrote:
> Hello,
>
> On Wed, Jun 28, 2017 at 10:41:25AM +0100, Phil Elwell wrote:
> > Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> > Acked-by: Eric Anholt <eric@anholt.net>
> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > Changes in v2:
> > Add review tags.
>
> nitpick: The order of lines in the Sob area matter and you should add
> your Sob at the end. So as it was you adding the ack by Eric and Andy,
> it should look as follows:
>
> Acked-by: Eric Anholt <eric@anholt.net>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>
> . Note that this comment is very picky, quite some people don't get it
> right and I never saw a maintainer refuse a patch because of this. So
> this probably doesn't warrant a v3 :-)
Yeah, it's easily learned when one is using
% git commit -s [--amend]
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
^ permalink raw reply
* Re: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Greg KH @ 2017-06-28 12:01 UTC (permalink / raw)
To: Oliver Neukum
Cc: Stuart Longland, Johan Hovold, Alan Cox,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1498641549.10278.5.camel-IBi9RG/b67k@public.gmane.org>
On Wed, Jun 28, 2017 at 11:19:09AM +0200, Oliver Neukum wrote:
> Am Mittwoch, den 28.06.2017, 08:07 +1000 schrieb Stuart Longland:
>
> Hi,
>
> > >> and to select 4-wire ("full duplex") RS-485, one does:
> > >> # echo rs485fd > /sys/class/tty/ttyUSB0/standard
> > > This looks like something that people will put into udev.
> > > So the switch will be done via udev, but the user not necessarily
> > > started via udev. Looks like a race to me.
> >
> > It is a race, but unless a daemon is watching for the very moment that a
> > particular serial interface appears, it's one that init scripts are very
> > likely to win.
>
> That is precisely what a demon might do.
>
> > Short of having a flag that blocks reads/writes until a given sysfs file
> > is written to say "I'm ready, go ahead"… there aren't that many options
> > I can think of.
>
> 1) module option
No, just no, sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Andy Shevchenko @ 2017-06-28 12:00 UTC (permalink / raw)
To: Phil Elwell, Uwe Kleine-König
Cc: Peter Hurley, Greg Kroah-Hartman, Yegor Yefremov,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Jan Kiszka
In-Reply-To: <95a13280-e9f6-50f5-bb0c-8bcdefd0bf12-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
On Wed, 2017-06-28 at 12:52 +0100, Phil Elwell wrote:
> On 28/06/2017 11:57, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Wed, Jun 28, 2017 at 10:41:25AM +0100, Phil Elwell wrote:
> > > Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> > > Acked-by: Eric Anholt <eric@anholt.net>
> > > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > > Changes in v2:
> > > Add review tags.
> >
> > nitpick: The order of lines in the Sob area matter and you should
> > add
> > your Sob at the end. So as it was you adding the ack by Eric and
> > Andy,
> > it should look as follows:
> >
> > Acked-by: Eric Anholt <eric@anholt.net>
> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>
> Really? I thought the submitter went first and the final merger went
> last, with other reviewers in-between, like this:
There is a difference between two, i.e. who has added tags in your case
and below one? I guess whoever adds them, adds before their own SoB tag.
>
> commit 6df765dca378bddf994cfd2044acafa501bd800f
> Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Date: Wed May 24 21:38:46 2017 +0200
>
> serial: imx: ensure UCR3 and UFCR are setup correctly
>
> [...]
>
> Fixes: e61c38d85b73 ("serial: imx: setup DCEDTE early and
> ensure DCD and RI irqs to be off")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.d
> e>
> Acked-by: Mika Penttilä <mika.penttila@nextfour.com>
> Tested-by: Mika Penttilä <mika.penttila@nextfour.com>
> Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> > . Note that this comment is very picky, quite some people don't get
> > it
> > right and I never saw a maintainer refuse a patch because of this.
> > So
> > this probably doesn't warrant a v3 :-)
> >
> > Best regards
> > Uwe
> >
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
^ permalink raw reply
* Re: [PATCH v2] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Phil Elwell @ 2017-06-28 11:52 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Peter Hurley, Greg Kroah-Hartman, Yegor Yefremov,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Jan Kiszka, Andy Shevchenko
In-Reply-To: <20170628105744.42bchlftl6khvsxd-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 28/06/2017 11:57, Uwe Kleine-König wrote:
> Hello,
>
> On Wed, Jun 28, 2017 at 10:41:25AM +0100, Phil Elwell wrote:
>> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>> Acked-by: Eric Anholt <eric@anholt.net>
>> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>> Changes in v2:
>> Add review tags.
>
> nitpick: The order of lines in the Sob area matter and you should add
> your Sob at the end. So as it was you adding the ack by Eric and Andy,
> it should look as follows:
>
> Acked-by: Eric Anholt <eric@anholt.net>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
Really? I thought the submitter went first and the final merger went
last, with other reviewers in-between, like this:
commit 6df765dca378bddf994cfd2044acafa501bd800f
Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Wed May 24 21:38:46 2017 +0200
serial: imx: ensure UCR3 and UFCR are setup correctly
[...]
Fixes: e61c38d85b73 ("serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Mika Penttilä <mika.penttila@nextfour.com>
Tested-by: Mika Penttilä <mika.penttila@nextfour.com>
Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>
Tested-by: Steve Twiss <stwiss.opensource@diasemi.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> . Note that this comment is very picky, quite some people don't get it
> right and I never saw a maintainer refuse a patch because of this. So
> this probably doesn't warrant a v3 :-)
>
> Best regards
> Uwe
>
_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
^ permalink raw reply
* Re: [PATCH v2] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Uwe Kleine-König @ 2017-06-28 10:57 UTC (permalink / raw)
To: Phil Elwell
Cc: Peter Hurley, Greg Kroah-Hartman, Yegor Yefremov,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Jan Kiszka, Andy Shevchenko
In-Reply-To: <1498642885-8063-1-git-send-email-phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hello,
On Wed, Jun 28, 2017 at 10:41:25AM +0100, Phil Elwell wrote:
> Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> Changes in v2:
> Add review tags.
nitpick: The order of lines in the Sob area matter and you should add
your Sob at the end. So as it was you adding the ack by Eric and Andy,
it should look as follows:
Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
. Note that this comment is very picky, quite some people don't get it
right and I never saw a maintainer refuse a patch because of this. So
this probably doesn't warrant a v3 :-)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v2] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Phil Elwell @ 2017-06-28 9:41 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andy Shevchenko, Peter Hurley, Yegor Yefremov,
Jan Kiszka, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
The BCM2835 MINI UART has non-standard THRE semantics. Conventionally
the bit means that the FIFO is empty (although there may still be a
byte in the transmit register), but on 2835 it indicates that the FIFO
is not full. This causes interrupts after every byte is transmitted,
with the FIFO providing some interrupt latency tolerance.
A consequence of this difference is that the usual strategy of writing
multiple bytes into the TX FIFO after checking THRE once is unsafe.
In the worst case of 7 bytes in the FIFO, writing 8 bytes loses all
but the first since by then the FIFO is full.
There is an HFIFO ("Hidden FIFO") capability that causes the transmit
loop to terminate when both THRE and TEMT are set, i.e. when the TX
block is completely idle. This is unnecessarily cautious, potentially
causing gaps in transmission.
Add a new conditional to the transmit loop, predicated on CAP_MINI,
that exits when THRE is no longer set (the FIFO is full). This allows
the FIFO to fill quickly but subsequent writes are paced by the
transmission rate.
Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
Changes in v2:
Add review tags.
drivers/tty/serial/8250/8250_port.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 4c620be..a5fe0e6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1764,6 +1764,10 @@ void serial8250_tx_chars(struct uart_8250_port *up)
if ((up->capabilities & UART_CAP_HFIFO) &&
(serial_in(up, UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)
break;
+ /* The BCM2835 MINI UART THRE bit is really a not-full bit. */
+ if ((up->capabilities & UART_CAP_MINI) &&
+ !(serial_in(up, UART_LSR) & UART_LSR_THRE))
+ break;
} while (--count > 0);
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
--
1.9.1
^ permalink raw reply related
* Re: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Oliver Neukum @ 2017-06-28 9:19 UTC (permalink / raw)
To: Stuart Longland, Johan Hovold
Cc: Alan Cox, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ae2907c2-cb0f-61e4-c6f2-af3dcec93ace-3e+Fe6x+DsgJbe36r25VNhCuuivNXqWP@public.gmane.org>
Am Mittwoch, den 28.06.2017, 08:07 +1000 schrieb Stuart Longland:
Hi,
> >> and to select 4-wire ("full duplex") RS-485, one does:
> >> # echo rs485fd > /sys/class/tty/ttyUSB0/standard
> > This looks like something that people will put into udev.
> > So the switch will be done via udev, but the user not necessarily
> > started via udev. Looks like a race to me.
>
> It is a race, but unless a daemon is watching for the very moment that a
> particular serial interface appears, it's one that init scripts are very
> likely to win.
That is precisely what a demon might do.
> Short of having a flag that blocks reads/writes until a given sysfs file
> is written to say "I'm ready, go ahead"… there aren't that many options
> I can think of.
1) module option
2) ioctl
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Phil Elwell @ 2017-06-28 9:17 UTC (permalink / raw)
To: Stefan Wahren, Greg Kroah-Hartman, Andy Shevchenko, Peter Hurley
Cc: Jan Kiszka, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Yegor Yefremov
In-Reply-To: <af5786eb-786b-5a50-6b4d-f67b8daf1c8a-eS4NqCHxEME@public.gmane.org>
On 28/06/2017 09:54, Stefan Wahren wrote:
> Hi Phil,
>
> Am 28.06.2017 um 10:42 schrieb Phil Elwell:
>> The BCM2835 MINI UART has non-standard THRE semantics. Conventionally
>> the bit means that the FIFO is empty (although there may still be a
>> byte in the transmit register), but on 2835 it indicates that the FIFO
>> is not full. This causes interrupts after every byte is transmitted,
>> with the FIFO providing some interrupt latency tolerance.
>>
>> A consequence of this difference is that the usual strategy of writing
>> multiple bytes into the TX FIFO after checking THRE once is unsafe.
>> In the worst case of 7 bytes in the FIFO, writing 8 bytes loses all
>> but the first since by then the FIFO is full.
>>
>> There is an HFIFO ("Hidden FIFO") capability that causes the transmit
>> loop to terminate when both THRE and TEMT are set, i.e. when the TX
>> block is completely idle. This is unnecessarily cautious, potentially
>> causing gaps in transmission.
>>
>> Add a new conditional to the transmit loop, predicated on CAP_MINI,
>> that exits when THRE is no longer set (the FIFO is full). This allows
>> the FIFO to fill quickly but subsequent writes are paced by the
>> transmission rate.
>>
>> Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
>> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>> Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> ---
>> drivers/tty/serial/8250/8250_port.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>
> please increase the version of the patch and add your changelog below
> this line, so Greg has the chance to apply the right fix.
But I've seen people reprimanded for posting a new version where the only difference
is in the review history, with claims that it doesn't make the merger's job easier.
I'll repost anyway.
> Also i assume that Andy wanted to suggest you to add the fixes tag.
This patch doesnt fix a bug in previous commit, it adds support for broken hardware,
so I don't think the "Fixes:" tag applies.
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index 4c620be..a5fe0e6 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -1764,6 +1764,10 @@ void serial8250_tx_chars(struct uart_8250_port *up)
>> if ((up->capabilities & UART_CAP_HFIFO) &&
>> (serial_in(up, UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)
>> break;
>> + /* The BCM2835 MINI UART THRE bit is really a not-full bit. */
>> + if ((up->capabilities & UART_CAP_MINI) &&
>> + !(serial_in(up, UART_LSR) & UART_LSR_THRE))
>> + break;
>> } while (--count > 0);
>>
>> if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
^ permalink raw reply
* Re: [PATCH] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Stefan Wahren @ 2017-06-28 8:54 UTC (permalink / raw)
To: Phil Elwell, Greg Kroah-Hartman, Andy Shevchenko, Peter Hurley
Cc: Jan Kiszka, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Yegor Yefremov
In-Reply-To: <1498639379-143432-1-git-send-email-phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
Am 28.06.2017 um 10:42 schrieb Phil Elwell:
> The BCM2835 MINI UART has non-standard THRE semantics. Conventionally
> the bit means that the FIFO is empty (although there may still be a
> byte in the transmit register), but on 2835 it indicates that the FIFO
> is not full. This causes interrupts after every byte is transmitted,
> with the FIFO providing some interrupt latency tolerance.
>
> A consequence of this difference is that the usual strategy of writing
> multiple bytes into the TX FIFO after checking THRE once is unsafe.
> In the worst case of 7 bytes in the FIFO, writing 8 bytes loses all
> but the first since by then the FIFO is full.
>
> There is an HFIFO ("Hidden FIFO") capability that causes the transmit
> loop to terminate when both THRE and TEMT are set, i.e. when the TX
> block is completely idle. This is unnecessarily cautious, potentially
> causing gaps in transmission.
>
> Add a new conditional to the transmit loop, predicated on CAP_MINI,
> that exits when THRE is no longer set (the FIFO is full). This allows
> the FIFO to fill quickly but subsequent writes are paced by the
> transmission rate.
>
> Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> drivers/tty/serial/8250/8250_port.c | 4 ++++
> 1 file changed, 4 insertions(+)
please increase the version of the patch and add your changelog below
this line, so Greg has the chance to apply the right fix.
Also i assume that Andy wanted to suggest you to add the fixes tag.
Sorry for the noise before.
Regards
Stefan
>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 4c620be..a5fe0e6 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1764,6 +1764,10 @@ void serial8250_tx_chars(struct uart_8250_port *up)
> if ((up->capabilities & UART_CAP_HFIFO) &&
> (serial_in(up, UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)
> break;
> + /* The BCM2835 MINI UART THRE bit is really a not-full bit. */
> + if ((up->capabilities & UART_CAP_MINI) &&
> + !(serial_in(up, UART_LSR) & UART_LSR_THRE))
> + break;
> } while (--count > 0);
>
> if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
^ permalink raw reply
* [PATCH] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Phil Elwell @ 2017-06-28 8:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andy Shevchenko, Peter Hurley, Yegor Yefremov,
Jan Kiszka, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
The BCM2835 MINI UART has non-standard THRE semantics. Conventionally
the bit means that the FIFO is empty (although there may still be a
byte in the transmit register), but on 2835 it indicates that the FIFO
is not full. This causes interrupts after every byte is transmitted,
with the FIFO providing some interrupt latency tolerance.
A consequence of this difference is that the usual strategy of writing
multiple bytes into the TX FIFO after checking THRE once is unsafe.
In the worst case of 7 bytes in the FIFO, writing 8 bytes loses all
but the first since by then the FIFO is full.
There is an HFIFO ("Hidden FIFO") capability that causes the transmit
loop to terminate when both THRE and TEMT are set, i.e. when the TX
block is completely idle. This is unnecessarily cautious, potentially
causing gaps in transmission.
Add a new conditional to the transmit loop, predicated on CAP_MINI,
that exits when THRE is no longer set (the FIFO is full). This allows
the FIFO to fill quickly but subsequent writes are paced by the
transmission rate.
Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/tty/serial/8250/8250_port.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 4c620be..a5fe0e6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1764,6 +1764,10 @@ void serial8250_tx_chars(struct uart_8250_port *up)
if ((up->capabilities & UART_CAP_HFIFO) &&
(serial_in(up, UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)
break;
+ /* The BCM2835 MINI UART THRE bit is really a not-full bit. */
+ if ((up->capabilities & UART_CAP_MINI) &&
+ !(serial_in(up, UART_LSR) & UART_LSR_THRE))
+ break;
} while (--count > 0);
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Andy Shevchenko @ 2017-06-28 8:18 UTC (permalink / raw)
To: Phil Elwell, Stefan Wahren, Greg Kroah-Hartman, Peter Hurley,
Yegor Yefremov, Jan Kiszka, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <9d796427-3a02-53a6-5426-633bbf6f9a85-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
On Wed, 2017-06-28 at 09:09 +0100, Phil Elwell wrote:
> On 28/06/2017 08:48, Andy Shevchenko wrote:
> > On Tue, 2017-06-27 at 22:00 +0100, Phil Elwell wrote:
> > > On 27/06/2017 18:52, Andy Shevchenko wrote:
> > > > On Tue, 2017-06-27 at 11:30 +0100, Phil Elwell wrote:
> > > > > On 27/06/2017 10:15, Andy Shevchenko wrote:
> > > > Okay, we have several types of flags in the code
> > > > 1. Capabilities: UART_CAP: looks like it defines features of
> > > > hardware
> > > > solely for 8250 compatible devices.
> > > > 2. Flags as quirks UPF_<something, not all of them> (I have a
> > > > patch
> > > > to
> > > > convert them to quirks, need by the way to update and resend):
> > > > they
> > > > are
> > > > for any serial devices.
> > > > 3. Flags as capabilities: UPF_<the rest>, similar function as
> > > > UART_CAP,
> > > > but for any serial device.
> > > >
> > > > >
> > > > > I'm also happy to make this code conditional on
> > > > > CONFIG_SERIAL_8250_BCM2835AUX
> > > > > if that is more acceptable.
> > > >
> > > > No, it is undesired.
> > > >
> > > > Can you describe which one from the above suits the best for
> > > > your
> > > > case?
> > >
> > > This bug I am trying to work around is found in the 8250
> > > implementation of
> > > one family of CPUs, so I would say capabilities are the best fit
> > > because
> > > they are specific to 8250 drivers.
> >
> > Yeah, looks like UART_CAP suits the best for now.
> >
> > One more comment that UART_CAP_* gather capabilities and quirks at
> > the
> > same time.
>
> Is that a request for an additional comment?
No.
> Is there anything else needed
> for an ack?
No, fine by me. Just resend a new version with tags applied. It would be
easier for Greg to pick up it later.
--
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH] serial: 8250: Fix THRE flag usage for CAP_MINI
From: Phil Elwell @ 2017-06-28 8:09 UTC (permalink / raw)
To: Andy Shevchenko, Stefan Wahren, Greg Kroah-Hartman, Peter Hurley,
Yegor Yefremov, Jan Kiszka, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1498636112.22624.212.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On 28/06/2017 08:48, Andy Shevchenko wrote:
> On Tue, 2017-06-27 at 22:00 +0100, Phil Elwell wrote:
>> On 27/06/2017 18:52, Andy Shevchenko wrote:
>>> On Tue, 2017-06-27 at 11:30 +0100, Phil Elwell wrote:
>>>> On 27/06/2017 10:15, Andy Shevchenko wrote:
>>>>> On Mon, 2017-06-26 at 16:15 +0100, Phil Elwell wrote:
>
>>>>>> Besides, the "HFIFO" capability looks a lot like quirk to me.
>>>>>
>>>>> To me either, which raises a question "Should it be fixed
>>>>> accordingly?"
>>>>
>>>> If I was going to make these quirks, are we simply talking about
>>>> renaming the
>>>> capability or is there another mechanism? I've found the 8250_pci
>>>> quirks, and
>>>> they look quite different.
>>>
>>> Okay, we have several types of flags in the code
>>> 1. Capabilities: UART_CAP: looks like it defines features of
>>> hardware
>>> solely for 8250 compatible devices.
>>> 2. Flags as quirks UPF_<something, not all of them> (I have a patch
>>> to
>>> convert them to quirks, need by the way to update and resend): they
>>> are
>>> for any serial devices.
>>> 3. Flags as capabilities: UPF_<the rest>, similar function as
>>> UART_CAP,
>>> but for any serial device.
>>>
>>>>
>>>> I'm also happy to make this code conditional on
>>>> CONFIG_SERIAL_8250_BCM2835AUX
>>>> if that is more acceptable.
>>>
>>> No, it is undesired.
>>>
>>> Can you describe which one from the above suits the best for your
>>> case?
>>
>> This bug I am trying to work around is found in the 8250
>> implementation of
>> one family of CPUs, so I would say capabilities are the best fit
>> because
>> they are specific to 8250 drivers.
>
> Yeah, looks like UART_CAP suits the best for now.
>
> One more comment that UART_CAP_* gather capabilities and quirks at the
> same time.
Is that a request for an additional comment? Is there anything else needed
for an ack?
Thanks,
Phil
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox