[PATCH 1/2] tty: serial: 8250_core: Remove trailing whitespaces

[PATCH 1/2] tty: serial: 8250_core: Remove trailing whitespaces

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

No functional changes.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/tty/serial/8250/8250_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 11c66856ba2f..65e52c9109e2 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -884,7 +884,7 @@ static int broken_efr(struct uart_8250_port *up)
 	/*
 	 * Exar ST16C2550 "A2" devices incorrectly detect as
 	 * having an EFR, and report an ID of 0x0201.  See
-	 * http://linux.derkeiler.com/Mailing-Lists/Kernel/2004-11/4812.html
+	 * http://linux.derkeiler.com/Mailing-Lists/Kernel/2004-11/4812.html
 	 */
 	if (autoconfig_read_divisor_id(up) == 0x0201 && size_fifo(up) == 16)
 		return 1;
@@ -1239,7 +1239,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 	serial_out(up, UART_LCR, save_lcr);

 	port->fifosize = uart_config[up->port.type].fifo_size;
-	old_capabilities = up->capabilities;
+	old_capabilities = up->capabilities;
 	up->capabilities = uart_config[port->type].flags;
 	up->tx_loadsz = uart_config[port->type].tx_loadsz;

--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]

This patch should be the part of:
"tty: serial: 8250_core: use the ->line argument as a hint in
serial8250_find_match_or_unused()"
(sha1: 59b3e898ddfc81a65975043b5eb44103cc29ff6e)

port->line can be setup by DT driver to -1 which needs to
be also checked.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

I have similar patch to 59b3e898ddfc81a65975043b5eb44103cc29ff6e
in xilinx tree for a while and port->line can be -1.

The second part of this also should be (what other drivers do)
reading aliases via of_alias_get_id() in of_serial.c:of_platform_serial_probe()
which setup port.line.
---
 drivers/tty/serial/8250/8250_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 65e52c9109e2..aab765177763 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3512,7 +3512,8 @@ static struct uart_8250_port *serial8250_find_match_or_unused(struct uart_port *

 	/* try line number first if still available */
 	i = port->line;
-	if (i < nr_uarts && serial8250_ports[i].port.type == PORT_UNKNOWN &&
+	if (i >= 0 && i < nr_uarts &&
+	    serial8250_ports[i].port.type == PORT_UNKNOWN &&
 			serial8250_ports[i].port.iobase == 0)
 		return &serial8250_ports[i];
 	/*
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Sebastian Andrzej Siewior]

On 01/15/2015 03:55 PM, Michal Simek wrote:
> This patch should be the part of:
> "tty: serial: 8250_core: use the ->line argument as a hint in
> serial8250_find_match_or_unused()"
> (sha1: 59b3e898ddfc81a65975043b5eb44103cc29ff6e)
> 
> port->line can be setup by DT driver to -1 which needs to
> be also checked.

This is what I have in 8250_omap to make sure it does not get < 0:

         if (pdev->dev.of_node) {
                 ret = of_alias_get_id(pdev->dev.of_node, "serial");

                 of_property_read_u32(pdev->dev.of_node, "clock-frequency",
                                      &up.port.uartclk);
                 priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
         } else {
                 ret = pdev->id;
         }
         if (ret < 0) {
                 dev_err(&pdev->dev, "failed to get alias/pdev id\n");
                 return ret;
         }
         up.port.line = ret;

but I didn't think of that others might pass < 0 since it wasn't used
before.
Does the original patch (you noted) break anything as of now? Because
then this patch should go stable.

> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> ---
> 
> I have similar patch to 59b3e898ddfc81a65975043b5eb44103cc29ff6e
> in xilinx tree for a while and port->line can be -1.
> 
> The second part of this also should be (what other drivers do)
> reading aliases via of_alias_get_id() in of_serial.c:of_platform_serial_probe()
> which setup port.line.

Which second part?
Hmmm. You could use of_alias_get_id(port.dev->of_node, "serial") here,
correct. But I think additionally, since it won't work for the
non-DT devices like pure platform devices.

> ---
>  drivers/tty/serial/8250/8250_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 65e52c9109e2..aab765177763 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3512,7 +3512,8 @@ static struct uart_8250_port *serial8250_find_match_or_unused(struct uart_port *
> 
>  	/* try line number first if still available */
>  	i = port->line;
> -	if (i < nr_uarts && serial8250_ports[i].port.type == PORT_UNKNOWN &&
> +	if (i >= 0 && i < nr_uarts &&
> +	    serial8250_ports[i].port.type == PORT_UNKNOWN &&
>  			serial8250_ports[i].port.iobase == 0)
>  		return &serial8250_ports[i];
>  	/*
> --
> 1.8.2.3

Sebastian

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Michal Simek]

Hi,

On 01/16/2015 10:00 AM, Sebastian Andrzej Siewior wrote:
> On 01/15/2015 03:55 PM, Michal Simek wrote:
>> This patch should be the part of:
>> "tty: serial: 8250_core: use the ->line argument as a hint in
>> serial8250_find_match_or_unused()"
>> (sha1: 59b3e898ddfc81a65975043b5eb44103cc29ff6e)
>>
>> port->line can be setup by DT driver to -1 which needs to
>> be also checked.
> 
> This is what I have in 8250_omap to make sure it does not get < 0:
> 
>          if (pdev->dev.of_node) {
>                  ret = of_alias_get_id(pdev->dev.of_node, "serial");
> 
>                  of_property_read_u32(pdev->dev.of_node, "clock-frequency",
>                                       &up.port.uartclk);
>                  priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
>          } else {
>                  ret = pdev->id;
>          }
>          if (ret < 0) {
>                  dev_err(&pdev->dev, "failed to get alias/pdev id\n");
>                  return ret;
>          }
>          up.port.line = ret;
> 
> but I didn't think of that others might pass < 0 since it wasn't used
> before.
> Does the original patch (you noted) break anything as of now? Because
> then this patch should go stable.

Origin patch looks good to me but this checking will be good to add.
Are you using of_serial.c because I didn't find any of_alias_get_id call
for 8250?

Your logic here is a little bit different than I have in my tree.
(Some sort of pl011 solution). You don't need to failed if you port.line is
below 0 but if you are not able to read aliases port enumeration
may be unpredictable.

Thanks,
Michal

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Sebastian Andrzej Siewior]

- Yoshihiro YUNOMAE
(reason: 550 5.1.1 <yoshihiro.yunomae.ez@hitachi.com>: Recipient
address rejected: User Unknown)

On 01/16/2015 11:37 AM, Michal Simek wrote:
> Hi,

Hi,

> Origin patch looks good to me but this checking will be good to add.
> Are you using of_serial.c because I didn't find any of_alias_get_id call
> for 8250?

I'm using of_alias_get_id() in 8250_omap.c which made it into
v3.19-rc1. I think the change you mention made it one release earlier.

> Your logic here is a little bit different than I have in my tree.
> (Some sort of pl011 solution). You don't need to failed if you port.line is
> below 0 but if you are not able to read aliases port enumeration
> may be unpredictable.

For omap we should have valid aliases because we have multiple UARTs.
So the platform id should always be >=0 and the DT includes it already.
And Tony wanted to have predicted numbers so I don't mind to fail here.
Also it is nice if you have ttyS2 which maps UART2 in the manual even
without having ttyS[01].

> Thanks,
> Michal

Sebastian

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Michal Simek]

On 01/16/2015 11:51 AM, Sebastian Andrzej Siewior wrote:
> - Yoshihiro YUNOMAE
> (reason: 550 5.1.1 <yoshihiro.yunomae.ez@hitachi.com>: Recipient
> address rejected: User Unknown)
> 
> On 01/16/2015 11:37 AM, Michal Simek wrote:
>> Hi,
> 
> Hi,
> 
>> Origin patch looks good to me but this checking will be good to add.
>> Are you using of_serial.c because I didn't find any of_alias_get_id call
>> for 8250?
> 
> I'm using of_alias_get_id() in 8250_omap.c which made it into
> v3.19-rc1. I think the change you mention made it one release earlier.

I see
serial: of-serial: fetch line number from DT
commit 1bd8324535ec1ff44aef55c0e40b9e7d56b310fb

but it was reverted

Revert "serial: of-serial: fetch line number from DT"
commit e4c787dac21e50ac9ef53fac91b9182d4fede465

Olof: what was the reason for that breakage?
Is it missing alias list on Tegra?
which caused that port->line number is bogus.

Thanks,
Michal

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Sebastian Andrzej Siewior]

On 01/16/2015 12:02 PM, Michal Simek wrote:
>>> Origin patch looks good to me but this checking will be good to add.
>>> Are you using of_serial.c because I didn't find any of_alias_get_id call
>>> for 8250?
>>
>> I'm using of_alias_get_id() in 8250_omap.c which made it into
>> v3.19-rc1. I think the change you mention made it one release earlier.
> 
> I see
> serial: of-serial: fetch line number from DT
> commit 1bd8324535ec1ff44aef55c0e40b9e7d56b310fb
> 
> but it was reverted
> 
> Revert "serial: of-serial: fetch line number from DT"
> commit e4c787dac21e50ac9ef53fac91b9182d4fede465
> 
> Olof: what was the reason for that breakage?
> Is it missing alias list on Tegra?
> which caused that port->line number is bogus.

I doubt it, because a missing alias would return -1 and would not be
considered for port->line.

I think it is more what happens if you use the second UART and the
primary is unused (as in not probed). With this change the console will
be on ttyS1 - without it it remains on ttyS0. The former is what we
wanted.

And it seems they use for "nvidia,tegra[23]0-hsuart" the serial-tegra.c
driver which makes use of of_alias_get_id() but the of_serial() one
which is used by nvidia,tegra20-uart() does not. And since this change
does not notify anyone during upgrade but removes the console everyone
gets a little grumpy*.

* and I've been looking for a smooth solution for the ttyOx -> ttySx
  change in OMAP just to avoid people complaining and risking a revert
  :)

> Thanks,
> Michal

Sebastian

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Greg Kroah-Hartman]

On Thu, Jan 15, 2015 at 03:55:08PM +0100, Michal Simek wrote:
> This patch should be the part of:
> "tty: serial: 8250_core: use the ->line argument as a hint in
> serial8250_find_match_or_unused()"
> (sha1: 59b3e898ddfc81a65975043b5eb44103cc29ff6e)
> 

How can I go back in time and add a patch to another one?

confused,

greg k-h

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Michal Simek]

On 01/31/2015 12:23 AM, Greg Kroah-Hartman wrote:
> On Thu, Jan 15, 2015 at 03:55:08PM +0100, Michal Simek wrote:
>> This patch should be the part of:
>> "tty: serial: 8250_core: use the ->line argument as a hint in
>> serial8250_find_match_or_unused()"
>> (sha1: 59b3e898ddfc81a65975043b5eb44103cc29ff6e)
>>
> 
> How can I go back in time and add a patch to another one?

ok. Let me rewrite it to make more sense. Definitely not asking
you to squash that to the patch. It is just pointer to origin
patch which added this code and which should be extended with this one.

Thanks,
Michal

Re: [PATCH 2/2] tty: serial: 8250_core: Check that port->line is >=0

[Andy Shevchenko]

On Mon, 2015-02-02 at 09:09 +0100, Michal Simek wrote:
> On 01/31/2015 12:23 AM, Greg Kroah-Hartman wrote:
> > On Thu, Jan 15, 2015 at 03:55:08PM +0100, Michal Simek wrote:
> >> This patch should be the part of:
> >> "tty: serial: 8250_core: use the ->line argument as a hint in
> >> serial8250_find_match_or_unused()"
> >> (sha1: 59b3e898ddfc81a65975043b5eb44103cc29ff6e)
> >>
> > 
> > How can I go back in time and add a patch to another one?
> 
> ok. Let me rewrite it to make more sense. Definitely not asking
> you to squash that to the patch. It is just pointer to origin
> patch which added this code and which should be extended with this one.

I guess you can use Fixes tag for that, though it doesn't sound like a
fix.

> 
> Thanks,
> Michal
> 


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

Re: [PATCH 1/2] tty: serial: 8250_core: Remove trailing whitespaces

[Greg Kroah-Hartman]

On Thu, Jan 15, 2015 at 03:55:07PM +0100, Michal Simek wrote:
> No functional changes.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---

I can't take gpg-signed patches, git chokes on them :(

Re: [PATCH 1/2] tty: serial: 8250_core: Remove trailing whitespaces

[Michal Simek]

Hi Greg,

On 01/31/2015 12:22 AM, Greg Kroah-Hartman wrote:
> On Thu, Jan 15, 2015 at 03:55:07PM +0100, Michal Simek wrote:
>> No functional changes.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
> 
> I can't take gpg-signed patches, git chokes on them :(
> 

Interesting. No problem to remove this signature if needed
but I got today email from your system that you have applied
this patch https://lkml.org/lkml/2015/1/16/245
which also has signature too.

What's the difference between them?

Thanks,
Michal

Re: [PATCH 1/2] tty: serial: 8250_core: Remove trailing whitespaces

[Greg Kroah-Hartman]

On Mon, Feb 02, 2015 at 09:08:30AM +0100, Michal Simek wrote:
> Hi Greg,
> 
> On 01/31/2015 12:22 AM, Greg Kroah-Hartman wrote:
> > On Thu, Jan 15, 2015 at 03:55:07PM +0100, Michal Simek wrote:
> >> No functional changes.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> > 
> > I can't take gpg-signed patches, git chokes on them :(
> > 
> 
> Interesting. No problem to remove this signature if needed
> but I got today email from your system that you have applied
> this patch https://lkml.org/lkml/2015/1/16/245
> which also has signature too.
> 
> What's the difference between them?

No idea, sorry.

greg k-h