2015-08-12 13:16:55

by Peter Hurley

[permalink] [raw]
Subject: Re: [v3 3/3] serial: 8250_dw: Add UPF_SKIP_TEST to flags depend on device tree

Hi Noam,

On 08/12/2015 07:18 AM, Noam Camus wrote:
> From: Noam Camus <[email protected]>
>
> Add support for OF option "no-loopback-test"
>
> use case: simulator which does not implements loopback test mode.

I think Greg's question about the simulator still applies: why upstream this?
The simulator is not even identified so how is someone supposed to know
this workaround applies?

The fact there are no in-tree DT users of this workaround argues against
its acceptance.

Regards,
Peter Hurley


> Signed-off-by: Noam Camus <[email protected]>
> ---
> .../bindings/serial/snps-dw-apb-uart.txt | 2 ++
> drivers/tty/serial/8250/8250_dw.c | 3 +++
> 2 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.txt b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.txt
> index 289c40e..5d16047 100644
> --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.txt
> @@ -33,6 +33,8 @@ Optional properties:
> - ri-override : Override the RI modem status signal. This signal will always be
> reported as inactive instead of being obtained from the modem status register.
> Define this if your serial port does not use this pin.
> +- no-loopback-test: set to indicate that the port does not implements loopback
> + test mode
>
> Example:
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 62f766a..0f397ae 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -394,6 +394,9 @@ static int dw8250_probe_of(struct uart_port *p,
> up->dma->txconf.dst_maxburst = p->fifosize / 4;
> }
>
> + if (of_find_property(np, "no-loopback-test", NULL))
> + p->flags |= UPF_SKIP_TEST;
> +
> if (!of_property_read_u32(np, "reg-shift", &val))
> p->regshift = val;
>
>


2015-08-12 15:51:14

by Noam Camus

[permalink] [raw]
Subject: RE: [v3 3/3] serial: 8250_dw: Add UPF_SKIP_TEST to flags depend on device tree

> From: Peter Hurley [mailto:[email protected]]
> Sent: Wednesday, August 12, 2015 4:17 PM

> I think Greg's question about the simulator still applies: why upstream this?
> The simulator is not even identified so how is someone supposed to know this workaround applies?

> The fact there are no in-tree DT users of this workaround argues against its acceptance.

I am using UART peripheral for Synopsys simulator same as one used by arch/arc/plat-sim
I know this platform do not use CONFIG_SERIAL_8250_DW due to some problem I suspect it is relate to the loop test.

Maybe Vineet Gupta or Alexey Brodkin from Synopsys which are CC here can comment.

So It just happen for me to be a pioneer with this.

More than that "no-loopback-test" is an option already exist for core 8250, and since DW is only an extension for this driver it should also benefit from this option.

If all this is yet not enough, should I re-send this "patch set" again without this specific patch?

Regards,
Noam Camus
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?