Hello Greg,
I'm a bit disappointed that you didn't drop this patch. At the time I
sent my review it was still in your tty-testing branch and now it's part
of your pull request for 4.13-rc1 (as commit
a3015affdf76ef279fbbb3710a220bab7e9ea04b). :-|
I'm still convinced that this patch is wrong in its current form.
Best regards
Uwe
On Thu, Jun 29, 2017 at 08:26:18PM +0200, Uwe Kleine-König wrote:
> Cc: += devicetree@xxxxxxxxxxxxxxx
>
> On Wed, Jun 28, 2017 at 12:15:14PM +0200, Romain Perier wrote:
> > From: Nandor Han <nandor.han@xxxxxx>
> >
> > The size of the DMA buffer can affect the delta time between data being
> > produced and data being consumed. Basically the DMA system will move
> > data to tty buffer when a) DMA buffer is full b) serial line is idle.
> > The situation is visible when producer generates data continuously and
> > there is no possibility for idle line. At this point the DMA buffer is
> > directly affecting the delta time.
>
> This doesn't look like a hw property but a configuration item. Also I
> don't understand the problematic case. The i.MX is sending continuously
> and then doesn't receive bytes until the DMA buffer is full? What is the
> DMA buffer? You don't mean the FIFO here, do you?
>
> That doesn't sound like a good fix but more like a work around. Which
> other options did you test to fix your problem?
>
> > The patch will add the possibility to configure the DMA buffers in DT,
> > which case by case can be configured separately for every driver
> > instance. The DT configuration is optional and in case missing the
> > driver will use the 4096 buffer with 4 periods (as before), therefore no
> > clients are impacted by this change.
> >
> > Signed-off-by: Nandor Han <nandor.han@xxxxxx>
> > Signed-off-by: Romain Perier <romain.perier@xxxxxxxxxxxxx>
> > ---
> > .../devicetree/bindings/serial/fsl-imx-uart.txt | 2 ++
> > drivers/tty/serial/imx.c | 25 +++++++++++++++-------
> > 2 files changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> > index 574c3a2..e6b5724 100644
> > --- a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> > +++ b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> > @@ -9,6 +9,7 @@ Optional properties:
> > - fsl,irda-mode : Indicate the uart supports irda mode
> > - fsl,dte-mode : Indicate the uart works in DTE mode. The uart works
> > in DCE mode by default.
> > +- fsl,dma-size : Indicate the size of the DMA buffer and its periods
>
> This is a sparse description, just from reading that I don't understand
> what it does.
>
> > Please check Documentation/devicetree/bindings/serial/serial.txt
> > for the complete list of generic properties.
> > @@ -28,4 +29,5 @@ uart1: serial@73fbc000 {
> > interrupts = <31>;
> > uart-has-rtscts;
> > fsl,dte-mode;
> > + fsl,dma-size = <1024 4>;
> > };
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index bbefddd..7327477 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -186,6 +186,11 @@
> >
> > #define UART_NR 8
> >
> > +/* RX DMA buffer periods */
> > +#define RX_DMA_PERIODS 4
> > +#define RX_BUF_SIZE (PAGE_SIZE)
>
> These names should include "DEFAULT" in their name now to fit their new
> semantic.
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
