On Mon, Apr 23, 2018 at 03:23:10PM +0200, Uwe Kleine-König wrote:
> Hello Einar,
Hi,
>
> On Mon, Apr 23, 2018 at 02:14:36PM +0200, Einar Vading wrote:
> > When using RS485 in half duplex mode the RTS line is typically used to
> > toggle receive/transmit on the transceiver IC. As some protocols (OSDP
> > f.ex.) specify that the slave should respond within some short time period,
> > it is important that the RTS line toggles the transceiver from transmit
> > mode to receive mode within that time.
> >
> > This change puts a pm_qos requirement on CPU_DMA_LATENCY when in RS485 tx
> > mode so that CPU wake times won't cause to long delays.
> >
> > Signed-off-by: Einar Vading <einarv@xxxxxxxx>
>
> I don't know much about that qos stuff to provide an informed comment,
> just two minor style issues below.
>
> Do I understand right that this change prevents the CPU from sleeping
> while RS485 is in use? If so, maybe this should be configurable? Is
> this something that should better be solved in serial_core instead of
> the imx driver?
Thanks for the feedback!
Yes, prevent sleeping so that waking up and handeling the RTS line won't
take to long for some protocols.
Configuration might be a good idea, maybe just put it in the DT?
I don't know about serial_core since some SOCs have atuo-RTS HW feature.
I think that makes this solution more of a per-SOC thing, at least I
think so.
>
> > @@ -1868,6 +1886,7 @@ static const struct uart_ops imx_uart_pops = {
> > .type = imx_uart_type,
> > .config_port = imx_uart_config_port,
> > .verify_port = imx_uart_verify_port,
> > +
>
> Unrelated change, please drop this.
>
> > #if defined(CONFIG_CONSOLE_POLL)
> > .poll_init = imx_uart_poll_init,
> > .poll_get_char = imx_uart_poll_get_char,
> > @@ -2335,6 +2354,10 @@ static int imx_uart_probe(struct platform_device *pdev)
> > }
> > }
> >
> > + pm_qos_add_request(&sport->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> > + PM_QOS_DEFAULT_VALUE);
>
> Please align the 2nd line to the opening parenthesis.
>
> > + INIT_WORK(&sport->qos_work, imx_qos_work);
> > +
> > imx_uart_ports[sport->port.line] = sport;
> >
> > platform_set_drvdata(pdev, sport);
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards
Einar
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
