From: Noam Camus <[email protected]>
Coming to use 8250_dw driver with my serial device I had few problems
I solved with following patch set.
First and fourth patches are aimed to support BIG endian device.
Second patch is is aimed to support device without test loop support.
Third patch is aimed to minimize chance for getting interrupt before
we called request_irq() during initialize probing.
Noam Camus (4):
serial: 8250_dw: Add support for big-endian MMIO accesses
serial: 8250_dw: Add UPF_SKIP_TEST to flags depend on device tree
serial: 8250_dw: Add UPF_FIXED_TYPE to flags
serial: 8250_dw: use serial_in() and not readl()
.../bindings/serial/snps-dw-apb-uart.txt | 2 +
drivers/tty/serial/8250/8250_dw.c | 61 +++++++++++++++++---
2 files changed, 54 insertions(+), 9 deletions(-)
From: Noam Camus <[email protected]>
Add support for UPIO_MEM32BE in addition to UPIO_MEM32.
Signed-off-by: Noam Camus <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 42 ++++++++++++++++++++++++++++++------
1 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d48b506..fe0b487 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -173,15 +173,13 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
}
#endif /* CONFIG_64BIT */
-static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
+static void dw8250_check_control(struct uart_port *p, int offset, int value)
{
struct dw8250_data *d = p->private_data;
if (offset == UART_MCR)
d->last_mcr = value;
- writel(value, p->membase + (offset << p->regshift));
-
/* Make sure LCR write wasn't ignored */
if (offset == UART_LCR) {
int tries = 1000;
@@ -190,7 +188,12 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
return;
dw8250_force_idle(p);
- writel(value, p->membase + (UART_LCR << p->regshift));
+ if (p->iotype == UPIO_MEM32BE)
+ iowrite32be(value,
+ p->membase + (UART_LCR << p->regshift));
+ else
+ writel(value,
+ p->membase + (UART_LCR << p->regshift));
}
/*
* FIXME: this deadlocks if port->lock is already held
@@ -199,6 +202,12 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
}
}
+static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
+{
+ writel(value, p->membase + (offset << p->regshift));
+ dw8250_check_control(p, offset, value);
+}
+
static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
{
unsigned int value = readl(p->membase + (offset << p->regshift));
@@ -206,6 +215,19 @@ static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
return dw8250_modify_msr(p, offset, value);
}
+static void dw8250_serial_out32be(struct uart_port *p, int offset, int value)
+{
+ iowrite32be(value, p->membase + (offset << p->regshift));
+ dw8250_check_control(p, offset, value);
+}
+
+static unsigned int dw8250_serial_in32be(struct uart_port *p, int offset)
+{
+ unsigned int value = ioread32be(p->membase + (offset << p->regshift));
+
+ return dw8250_modify_msr(p, offset, value);
+}
+
static int dw8250_handle_irq(struct uart_port *p)
{
struct dw8250_data *d = p->private_data;
@@ -322,9 +344,15 @@ static int dw8250_probe_of(struct uart_port *p,
case 1:
break;
case 4:
- p->iotype = UPIO_MEM32;
- p->serial_in = dw8250_serial_in32;
- p->serial_out = dw8250_serial_out32;
+ p->iotype = of_device_is_big_endian(np) ?
+ UPIO_MEM32BE : UPIO_MEM32;
+ if (p->iotype == UPIO_MEM32) {
+ p->serial_in = dw8250_serial_in32;
+ p->serial_out = dw8250_serial_out32;
+ } else {
+ p->serial_in = dw8250_serial_in32be;
+ p->serial_out = dw8250_serial_out32be;
+ }
break;
default:
dev_err(p->dev, "unsupported reg-io-width (%u)\n", val);
--
1.7.1
From: Noam Camus <[email protected]>
Add support for OF option "no-loopback-test"
use case: simulator which does not implements loopback test mode.
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 fe0b487..1a57105 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -370,6 +370,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;
--
1.7.1
From: Noam Camus <[email protected]>
Always add UPF_FIXED_TYPE to flags so autoconf() will be skipped.
We do that since autoconf() performs many writes to LCR that cause
BUSY interrupt.
The problem with such interrupt is that driver is not yet called to
request_irq() and generic IRQ subsystem will mask the UART line.
Signed-off-by: Noam Camus <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 1a57105..620f983 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -362,6 +362,14 @@ static int dw8250_probe_of(struct uart_port *p,
if (has_ucv)
dw8250_setup_port(up);
+ /* Writing to LCR may cause BUSY interrupt before we
+ * register the IRQ line.
+ * Currently autoconf() uses several writes to LCR.
+ * In order to avoid calling to autoconf() always add
+ * following flag.
+ */
+ p->flags |= UPF_FIXED_TYPE;
+
/* if we have a valid fifosize, try hooking up DMA here */
if (p->fifosize) {
up->dma = &data->dma;
--
1.7.1
From: Noam Camus <[email protected]>
This is now matches device endianness.
Signed-off-by: Noam Camus <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 620f983..a64197c 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -291,7 +291,9 @@ static bool dw8250_dma_filter(struct dma_chan *chan, void *param)
static void dw8250_setup_port(struct uart_8250_port *up)
{
struct uart_port *p = &up->port;
- u32 reg = readl(p->membase + DW_UART_UCV);
+ u32 reg = (p->iotype == UPIO_MEM32BE) ?
+ ioread32be(p->membase + DW_UART_UCV) :
+ readl(p->membase + DW_UART_UCV);
/*
* If the Component Version Register returns zero, we know that
@@ -303,7 +305,9 @@ static void dw8250_setup_port(struct uart_8250_port *up)
dev_dbg_ratelimited(p->dev, "Designware UART version %c.%c%c\n",
(reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
- reg = readl(p->membase + DW_UART_CPR);
+ reg = (p->iotype == UPIO_MEM32BE) ?
+ ioread32be(p->membase + DW_UART_CPR) :
+ readl(p->membase + DW_UART_CPR);
if (!reg)
return;
--
1.7.1
Hi Noam,
On 07/22/2015 05:34 AM, Noam Camus wrote:
> From: Noam Camus <[email protected]>
>
> Add support for UPIO_MEM32BE in addition to UPIO_MEM32.
This is not an adequate changelog.
Please describe the refactoring.
Regards,
Peter Hurley
> Signed-off-by: Noam Camus <[email protected]>
> ---
> drivers/tty/serial/8250/8250_dw.c | 42 ++++++++++++++++++++++++++++++------
> 1 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index d48b506..fe0b487 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -173,15 +173,13 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
> }
> #endif /* CONFIG_64BIT */
>
> -static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
> +static void dw8250_check_control(struct uart_port *p, int offset, int value)
> {
> struct dw8250_data *d = p->private_data;
>
> if (offset == UART_MCR)
> d->last_mcr = value;
>
> - writel(value, p->membase + (offset << p->regshift));
> -
> /* Make sure LCR write wasn't ignored */
> if (offset == UART_LCR) {
> int tries = 1000;
> @@ -190,7 +188,12 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
> if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> return;
> dw8250_force_idle(p);
> - writel(value, p->membase + (UART_LCR << p->regshift));
> + if (p->iotype == UPIO_MEM32BE)
> + iowrite32be(value,
> + p->membase + (UART_LCR << p->regshift));
> + else
> + writel(value,
> + p->membase + (UART_LCR << p->regshift));
> }
> /*
> * FIXME: this deadlocks if port->lock is already held
> @@ -199,6 +202,12 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
> }
> }
>
> +static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
> +{
> + writel(value, p->membase + (offset << p->regshift));
> + dw8250_check_control(p, offset, value);
> +}
> +
> static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
> {
> unsigned int value = readl(p->membase + (offset << p->regshift));
> @@ -206,6 +215,19 @@ static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
> return dw8250_modify_msr(p, offset, value);
> }
>
> +static void dw8250_serial_out32be(struct uart_port *p, int offset, int value)
> +{
> + iowrite32be(value, p->membase + (offset << p->regshift));
> + dw8250_check_control(p, offset, value);
> +}
> +
> +static unsigned int dw8250_serial_in32be(struct uart_port *p, int offset)
> +{
> + unsigned int value = ioread32be(p->membase + (offset << p->regshift));
> +
> + return dw8250_modify_msr(p, offset, value);
> +}
> +
> static int dw8250_handle_irq(struct uart_port *p)
> {
> struct dw8250_data *d = p->private_data;
> @@ -322,9 +344,15 @@ static int dw8250_probe_of(struct uart_port *p,
> case 1:
> break;
> case 4:
> - p->iotype = UPIO_MEM32;
> - p->serial_in = dw8250_serial_in32;
> - p->serial_out = dw8250_serial_out32;
> + p->iotype = of_device_is_big_endian(np) ?
> + UPIO_MEM32BE : UPIO_MEM32;
> + if (p->iotype == UPIO_MEM32) {
> + p->serial_in = dw8250_serial_in32;
> + p->serial_out = dw8250_serial_out32;
> + } else {
> + p->serial_in = dw8250_serial_in32be;
> + p->serial_out = dw8250_serial_out32be;
> + }
> break;
> default:
> dev_err(p->dev, "unsupported reg-io-width (%u)\n", val);
>
Hi Noam,
On 07/22/2015 05:34 AM, Noam Camus wrote:
> From: Noam Camus <[email protected]>
>
> Add support for OF option "no-loopback-test"
Changes to devicetree need to at least get acks from DT maintainers.
Regards,
Peter Hurley
> use case: simulator which does not implements loopback test mode.
>
> 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 fe0b487..1a57105 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -370,6 +370,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;
>
>
Hi Noam,
On 07/22/2015 05:34 AM, Noam Camus wrote:
> From: Noam Camus <[email protected]>
>
> Always add UPF_FIXED_TYPE to flags so autoconf() will be skipped.
> We do that since autoconf() performs many writes to LCR that cause
> BUSY interrupt.
> The problem with such interrupt is that driver is not yet called to
> request_irq() and generic IRQ subsystem will mask the UART line.
>
> Signed-off-by: Noam Camus <[email protected]>
> ---
> drivers/tty/serial/8250/8250_dw.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 1a57105..620f983 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -362,6 +362,14 @@ static int dw8250_probe_of(struct uart_port *p,
> if (has_ucv)
> dw8250_setup_port(up);
>
> + /* Writing to LCR may cause BUSY interrupt before we
> + * register the IRQ line.
> + * Currently autoconf() uses several writes to LCR.
> + * In order to avoid calling to autoconf() always add
> + * following flag.
> + */
> + p->flags |= UPF_FIXED_TYPE;
Why only for devicetree DW8250's? Don't all DW8250's have this LCR "feature"?
And what port type does this id DW8250's as, PORT_8250? Except with fifos,
autoflow control, dma, etc.?
If the port type is being fixed, then please fix it to a new port type.
> +
> /* if we have a valid fifosize, try hooking up DMA here */
> if (p->fifosize) {
> up->dma = &data->dma;
>
On 07/22/2015 08:19 AM, Peter Hurley wrote:
> Hi Noam,
>
> On 07/22/2015 05:34 AM, Noam Camus wrote:
>> From: Noam Camus <[email protected]>
>>
>> Add support for UPIO_MEM32BE in addition to UPIO_MEM32.
>
> This is not an adequate changelog.
> Please describe the refactoring.
>
> Regards,
> Peter Hurley
>
>> Signed-off-by: Noam Camus <[email protected]>
>> ---
>> drivers/tty/serial/8250/8250_dw.c | 42 ++++++++++++++++++++++++++++++------
>> 1 files changed, 35 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
>> index d48b506..fe0b487 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -173,15 +173,13 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
>> }
>> #endif /* CONFIG_64BIT */
>>
>> -static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
>> +static void dw8250_check_control(struct uart_port *p, int offset, int value)
Also, I think this fn name should be dw8250_check_LCR() to distinguish it
from modem control.
Regards,
Peter Hurley
>> {
>> struct dw8250_data *d = p->private_data;
>>
>> if (offset == UART_MCR)
>> d->last_mcr = value;
>>
>> - writel(value, p->membase + (offset << p->regshift));
>> -
>> /* Make sure LCR write wasn't ignored */
>> if (offset == UART_LCR) {
>> int tries = 1000;
>> @@ -190,7 +188,12 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
>> if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
>> return;
>> dw8250_force_idle(p);
>> - writel(value, p->membase + (UART_LCR << p->regshift));
>> + if (p->iotype == UPIO_MEM32BE)
>> + iowrite32be(value,
>> + p->membase + (UART_LCR << p->regshift));
>> + else
>> + writel(value,
>> + p->membase + (UART_LCR << p->regshift));
>> }
>> /*
>> * FIXME: this deadlocks if port->lock is already held
>> @@ -199,6 +202,12 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
>> }
>> }
>>
>> +static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
>> +{
>> + writel(value, p->membase + (offset << p->regshift));
>> + dw8250_check_control(p, offset, value);
>> +}
>> +
>> static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
>> {
>> unsigned int value = readl(p->membase + (offset << p->regshift));
>> @@ -206,6 +215,19 @@ static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
>> return dw8250_modify_msr(p, offset, value);
>> }
>>
>> +static void dw8250_serial_out32be(struct uart_port *p, int offset, int value)
>> +{
>> + iowrite32be(value, p->membase + (offset << p->regshift));
>> + dw8250_check_control(p, offset, value);
>> +}
>> +
>> +static unsigned int dw8250_serial_in32be(struct uart_port *p, int offset)
>> +{
>> + unsigned int value = ioread32be(p->membase + (offset << p->regshift));
>> +
>> + return dw8250_modify_msr(p, offset, value);
>> +}
>> +
>> static int dw8250_handle_irq(struct uart_port *p)
>> {
>> struct dw8250_data *d = p->private_data;
>> @@ -322,9 +344,15 @@ static int dw8250_probe_of(struct uart_port *p,
>> case 1:
>> break;
>> case 4:
>> - p->iotype = UPIO_MEM32;
>> - p->serial_in = dw8250_serial_in32;
>> - p->serial_out = dw8250_serial_out32;
>> + p->iotype = of_device_is_big_endian(np) ?
>> + UPIO_MEM32BE : UPIO_MEM32;
>> + if (p->iotype == UPIO_MEM32) {
>> + p->serial_in = dw8250_serial_in32;
>> + p->serial_out = dw8250_serial_out32;
>> + } else {
>> + p->serial_in = dw8250_serial_in32be;
>> + p->serial_out = dw8250_serial_out32be;
>> + }
>> break;
>> default:
>> dev_err(p->dev, "unsupported reg-io-width (%u)\n", val);
>>
>
Hi Noam,
On 07/22/2015 05:34 AM, Noam Camus wrote:
> From: Noam Camus <[email protected]>
>
> This is now matches device endianness.
I'm not seeing where serial_in() is used here, as claimed by the $subject.
Regards,
Peter Hurley
> Signed-off-by: Noam Camus <[email protected]>
> ---
> drivers/tty/serial/8250/8250_dw.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 620f983..a64197c 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -291,7 +291,9 @@ static bool dw8250_dma_filter(struct dma_chan *chan, void *param)
> static void dw8250_setup_port(struct uart_8250_port *up)
> {
> struct uart_port *p = &up->port;
> - u32 reg = readl(p->membase + DW_UART_UCV);
> + u32 reg = (p->iotype == UPIO_MEM32BE) ?
> + ioread32be(p->membase + DW_UART_UCV) :
> + readl(p->membase + DW_UART_UCV);
>
> /*
> * If the Component Version Register returns zero, we know that
> @@ -303,7 +305,9 @@ static void dw8250_setup_port(struct uart_8250_port *up)
> dev_dbg_ratelimited(p->dev, "Designware UART version %c.%c%c\n",
> (reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
>
> - reg = readl(p->membase + DW_UART_CPR);
> + reg = (p->iotype == UPIO_MEM32BE) ?
> + ioread32be(p->membase + DW_UART_CPR) :
> + readl(p->membase + DW_UART_CPR);
> if (!reg)
> return;
>
>
From: Peter Hurley [mailto:[email protected]]
Sent: Wednesday, July 22, 2015 3:19 PM
> This is not an adequate changelog.
> Please describe the refactoring.
I will in my v2 patch set
Noam
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
From: Peter Hurley [mailto:[email protected]]
Sent: Wednesday, July 22, 2015 3:41 PM
>> diff --git a/drivers/tty/serial/8250/8250_dw.c
>> b/drivers/tty/serial/8250/8250_dw.c
>> index d48b506..fe0b487 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -173,15 +173,13 @@ static void dw8250_serial_outq(struct uart_port
>> *p, int offset, int value) } #endif /* CONFIG_64BIT */
>>
>> -static void dw8250_serial_out32(struct uart_port *p, int offset, int
>> value)
>> +static void dw8250_check_control(struct uart_port *p, int offset,
>> +int value)
> Also, I think this fn name should be dw8250_check_LCR() to distinguish it from modem control.
No problem will rename in my v2
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
From: Peter Hurley [mailto:[email protected]]
Sent: Wednesday, July 22, 2015 3:21 PM
>>On 07/22/2015 05:34 AM, Noam Camus wrote:
>> From: Noam Camus <[email protected]>
>>
>> Add support for OF option "no-loopback-test"
> Changes to devicetree need to at least get acks from DT maintainers.
I will add them in my v2
Should I take this patch apart from this set or should I add DT maintainers to whole set?
Noam
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Thu, Jul 23, 2015 at 8:21 AM, Noam Camus <[email protected]> wrote:
> From: Peter Hurley [mailto:[email protected]]
> Sent: Wednesday, July 22, 2015 3:21 PM
>
>>>On 07/22/2015 05:34 AM, Noam Camus wrote:
>>> From: Noam Camus <[email protected]>
>>>
>>> Add support for OF option "no-loopback-test"
>
>> Changes to devicetree need to at least get acks from DT maintainers.
> I will add them in my v2
> Should I take this patch apart from this set or should I add DT maintainers to whole set?
Add them at least to your cover letter and this one specifically. It's
a small set, so it's fine to add them to all patches, so they can see
the bigger picture. No need to take this patch out of the set (and
context).
Frans
From: Peter Hurley [mailto:[email protected]]
Sent: Wednesday, July 22, 2015 3:39 PM
>> + /* Writing to LCR may cause BUSY interrupt before we
>> + * register the IRQ line.
>> + * Currently autoconf() uses several writes to LCR.
>> + * In order to avoid calling to autoconf() always add
>> + * following flag.
>> + */
>> + p->flags |= UPF_FIXED_TYPE;
>
> Why only for devicetree DW8250's? Don't all DW8250's have this LCR "feature"?
>
Yes, all DW8250 got this "feature" i.e. busy functionality.
> And what port type does this id DW8250's as, PORT_8250? Except with fifos, autoflow control, dma, etc.?
The only thing that DW8250 is not fully 16550 compatibility is this busy functionality feature.
This feature means that a serial transfer is in progress.
It is indicated by special identification in IIR, and raised when LCR is written while device is busy.
> If the port type is being fixed, then please fix it to a new port type.
As an extension to 16550 type is not fixed.
Seem that I need to rethink on this.
I will remove this commit from v2 patch set till I will be sure what is the right thing to do.
Noam
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
From: Peter Hurley [mailto:[email protected]]
Sent: Wednesday, July 22, 2015 3:52 PM
> Subject: Re: [PATCH 4/4] serial: 8250_dw: use serial_in() and not readl()
> From: Noam Camus <[email protected]>
>
> This is now matches device endianness.
> I'm not seeing where serial_in() is used here, as claimed by the $subject.
Thanks, I will "reword" my commit in v2
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?