[rft, PATCH v1 00/14] serial: Add a helper to parse device properties and more

[rft, PATCH v1 00/14] serial: Add a helper to parse device properties and more

[Andy Shevchenko]

I have noticed that many drivers are using the subset of the common
properties and IRQ retrieval code. With the moving it to one place
we have got a common parser one for many.

Tested on Intel Apollo Lake with DesingWare 8250 UARTs.
The rest has been compile tested on x86_64 with clang.

Andy Shevchenko (14):
  serial: core: Move struct uart_port::quirks closer to possible values
  serial: core: Add UPIO_UNSET constant for unset port type
  serial: port: Introduce a common helper to read properties
  serial: 8250_aspeed_vuart: Switch to use uart_read_port_properties()
  serial: 8250_bcm2835aux: Switch to use uart_read_port_properties()
  serial: 8250_bcm7271: Switch to use uart_read_port_properties()
  serial: 8250_dw: Switch to use uart_read_port_properties()
  serial: 8250_ingenic: Switch to use uart_read_port_properties()
  serial: 8250_lpc18xx: Switch to use uart_read_port_properties()
  serial: 8250_of: Switch to use uart_read_port_properties()
  serial: 8250_omap: Switch to use uart_read_port_properties()
  serial: 8250_pxa: Switch to use uart_read_port_properties()
  serial: 8250_tegra: Switch to use uart_read_port_properties()
  serial: 8250_uniphier: Switch to use uart_read_port_properties()

 drivers/tty/serial/8250/8250_aspeed_vuart.c |  50 +++-----
 drivers/tty/serial/8250/8250_bcm2835aux.c   |  92 ++++++-------
 drivers/tty/serial/8250/8250_bcm7271.c      |  53 +++-----
 drivers/tty/serial/8250/8250_dw.c           |  67 ++++------
 drivers/tty/serial/8250/8250_ingenic.c      |  20 +--
 drivers/tty/serial/8250/8250_lpc18xx.c      |  20 ++-
 drivers/tty/serial/8250/8250_of.c           | 105 ++++-----------
 drivers/tty/serial/8250/8250_omap.c         |  29 ++---
 drivers/tty/serial/8250/8250_pxa.c          |  22 ++--
 drivers/tty/serial/8250/8250_tegra.c        |  26 ++--
 drivers/tty/serial/8250/8250_uniphier.c     |  17 +--
 drivers/tty/serial/serial_port.c            | 135 ++++++++++++++++++++
 include/linux/serial_core.h                 |  10 +-
 13 files changed, 313 insertions(+), 333 deletions(-)

-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 01/14] serial: core: Move struct uart_port::quirks closer to possible values

[Andy Shevchenko]

Currently it's not crystal clear what UPIO_* and UPQ_* definitions
belong two. Reindent the code, so it will be easy to read and understand.
No functional changes intended.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/serial_core.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 55b1f3ba48ac..2d2ec99eca93 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -467,8 +467,8 @@ struct uart_port {
 	unsigned int		fifosize;		/* tx fifo size */
 	unsigned char		x_char;			/* xon/xoff char */
 	unsigned char		regshift;		/* reg offset shift */
+
 	unsigned char		iotype;			/* io access style */
-	unsigned char		quirks;			/* internal quirks */
 
 #define UPIO_PORT		(SERIAL_IO_PORT)	/* 8b I/O port access */
 #define UPIO_HUB6		(SERIAL_IO_HUB6)	/* Hub6 ISA card */
@@ -479,7 +479,9 @@ struct uart_port {
 #define UPIO_MEM32BE		(SERIAL_IO_MEM32BE)	/* 32b big endian */
 #define UPIO_MEM16		(SERIAL_IO_MEM16)	/* 16b little endian */
 
-	/* quirks must be updated while holding port mutex */
+	unsigned char		quirks;			/* internal quirks */
+
+	/* internal quirks must be updated while holding port mutex */
 #define UPQ_NO_TXEN_TEST	BIT(0)
 
 	unsigned int		read_status_mask;	/* driver specific */
-- 
2.43.0.rc1.1.gbec44491f096

Re: [PATCH v1 01/14] serial: core: Move struct uart_port::quirks closer to possible values

[Hugo Villeneuve]

On Wed, 21 Feb 2024 20:31:17 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Currently it's not crystal clear what UPIO_* and UPQ_* definitions
> belong two. Reindent the code, so it will be easy to read and understand.

two -> to.

Hugo V.


> No functional changes intended.
> 
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/serial_core.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 55b1f3ba48ac..2d2ec99eca93 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -467,8 +467,8 @@ struct uart_port {
>  	unsigned int		fifosize;		/* tx fifo size */
>  	unsigned char		x_char;			/* xon/xoff char */
>  	unsigned char		regshift;		/* reg offset shift */
> +
>  	unsigned char		iotype;			/* io access style */
> -	unsigned char		quirks;			/* internal quirks */
>  
>  #define UPIO_PORT		(SERIAL_IO_PORT)	/* 8b I/O port access */
>  #define UPIO_HUB6		(SERIAL_IO_HUB6)	/* Hub6 ISA card */
> @@ -479,7 +479,9 @@ struct uart_port {
>  #define UPIO_MEM32BE		(SERIAL_IO_MEM32BE)	/* 32b big endian */
>  #define UPIO_MEM16		(SERIAL_IO_MEM16)	/* 16b little endian */
>  
> -	/* quirks must be updated while holding port mutex */
> +	unsigned char		quirks;			/* internal quirks */
> +
> +	/* internal quirks must be updated while holding port mutex */
>  #define UPQ_NO_TXEN_TEST	BIT(0)
>  
>  	unsigned int		read_status_mask;	/* driver specific */
> -- 
> 2.43.0.rc1.1.gbec44491f096
> 
> 


-- 
Hugo Villeneuve

Re: [PATCH v1 01/14] serial: core: Move struct uart_port::quirks closer to possible values

[Andy Shevchenko]

On Wed, Feb 21, 2024 at 01:54:52PM -0500, Hugo Villeneuve wrote:
> On Wed, 21 Feb 2024 20:31:17 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Currently it's not crystal clear what UPIO_* and UPQ_* definitions
> > belong two. Reindent the code, so it will be easy to read and understand.
> 
> two -> to.

Ah, thanks, will fix!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Andy Shevchenko]

In some APIs we would like to assign the special value to iotype
and compare against it in another places. Introduce UPIO_UNSET
for this purpose.

Note, we can't use 0, because it's a valid value for IO port access.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/serial_core.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2d2ec99eca93..2b0526ae1fac 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -470,6 +470,7 @@ struct uart_port {
 
 	unsigned char		iotype;			/* io access style */
 
+#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */
 #define UPIO_PORT		(SERIAL_IO_PORT)	/* 8b I/O port access */
 #define UPIO_HUB6		(SERIAL_IO_HUB6)	/* Hub6 ISA card */
 #define UPIO_MEM		(SERIAL_IO_MEM)		/* driver-specific */
-- 
2.43.0.rc1.1.gbec44491f096

Re: [PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Florian Fainelli]

On 2/21/24 10:31, Andy Shevchenko wrote:
> In some APIs we would like to assign the special value to iotype
> and compare against it in another places. Introduce UPIO_UNSET
> for this purpose.
> 
> Note, we can't use 0, because it's a valid value for IO port access.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   include/linux/serial_core.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 2d2ec99eca93..2b0526ae1fac 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -470,6 +470,7 @@ struct uart_port {
>   
>   	unsigned char		iotype;			/* io access style */
>   
> +#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */

Nit: I would name this UPIO_UNKNOWN, or UPIO_NOTSET, unset means to me 
that it was previously set and we undid that action, whereas unknown or 
not set means we never did.
-- 
Florian

Re: [PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Andy Shevchenko]

On Wed, Feb 21, 2024 at 10:47:13AM -0800, Florian Fainelli wrote:
> On 2/21/24 10:31, Andy Shevchenko wrote:

...

> >   	unsigned char		iotype;			/* io access style */
> > +#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */
> 
> Nit: I would name this UPIO_UNKNOWN, or UPIO_NOTSET, unset means to me that
> it was previously set and we undid that action, whereas unknown or not set
> means we never did.

Works for me. I will wait for a few days / week to have more reviews and
likely testings to be collected. Would be nice if you be able to test on
(some of) the hardware in the list of modified drivers.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Jiri Slaby]

On 21. 02. 24, 19:31, Andy Shevchenko wrote:
> In some APIs we would like to assign the special value to iotype
> and compare against it in another places. Introduce UPIO_UNSET
> for this purpose.
> 
> Note, we can't use 0, because it's a valid value for IO port access.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   include/linux/serial_core.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 2d2ec99eca93..2b0526ae1fac 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -470,6 +470,7 @@ struct uart_port {
>   
>   	unsigned char		iotype;			/* io access style */
>   
> +#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */

Perhaps making the var u8 and this U8_MAX then? It would make more sense 
to me.

>   #define UPIO_PORT		(SERIAL_IO_PORT)	/* 8b I/O port access */
>   #define UPIO_HUB6		(SERIAL_IO_HUB6)	/* Hub6 ISA card */
>   #define UPIO_MEM		(SERIAL_IO_MEM)		/* driver-specific */

thanks,
-- 
js
suse labs

Re: [PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Andy Shevchenko]

On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
> On 21. 02. 24, 19:31, Andy Shevchenko wrote:

...

> >   	unsigned char		iotype;			/* io access style */
> > +#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */
> 
> Perhaps making the var u8 and this U8_MAX then? It would make more sense to
> me.

WFM, should it be a separate change? Btw, how can I justify it?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Jiri Slaby]

On 22. 02. 24, 14:21, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
>> On 21. 02. 24, 19:31, Andy Shevchenko wrote:
> 
> ...
> 
>>>    	unsigned char		iotype;			/* io access style */
>>> +#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */
>>
>> Perhaps making the var u8 and this U8_MAX then? It would make more sense to
>> me.
> 
> WFM, should it be a separate change?

Likely.

> Btw, how can I justify it?

Hmm, thinking about it, why is it not an enum?

But it could be also an u8 because you want it be exactly 8 bits as you 
want to be sure values up to 255 fit.

thanks,
-- 
js
suse labs

Re: [PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Andy Shevchenko]

On Fri, Feb 23, 2024 at 06:42:15AM +0100, Jiri Slaby wrote:
> On 22. 02. 24, 14:21, Andy Shevchenko wrote:
> > On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
> > > On 21. 02. 24, 19:31, Andy Shevchenko wrote:

...

> > > >    	unsigned char		iotype;			/* io access style */
> > > > +#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */
> > > 
> > > Perhaps making the var u8 and this U8_MAX then? It would make more sense to
> > > me.
> > 
> > WFM, should it be a separate change?
> 
> Likely.

Then I need a commit message, because I'm unable to justify this change myself.

> > Btw, how can I justify it?
> 
> Hmm, thinking about it, why is it not an enum?

Maybe, but it is a replica of UAPI definitions, do we want to see it as a enum?
To me it will be a bit ugly looking.

> But it could be also an u8 because you want it be exactly 8 bits as you want
> to be sure values up to 255 fit.

Depends on what we assume UAPI does with those flags. It maybe even less than
8 bits, or great than, currently 8 bits is enough...

TL;DR: I would rather take a patch from you and incorporate into the series
than trying hard to invent a justification and proper type.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 02/14] serial: core: Add UPIO_UNSET constant for unset port type

[Andy Shevchenko]

On Fri, Feb 23, 2024 at 04:59:25PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 23, 2024 at 06:42:15AM +0100, Jiri Slaby wrote:
> > On 22. 02. 24, 14:21, Andy Shevchenko wrote:
> > > On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
> > > > On 21. 02. 24, 19:31, Andy Shevchenko wrote:

...

> > > > >    	unsigned char		iotype;			/* io access style */
> > > > > +#define UPIO_UNSET		((unsigned char)~0U)	/* UCHAR_MAX */
> > > > 
> > > > Perhaps making the var u8 and this U8_MAX then? It would make more sense to
> > > > me.
> > > 
> > > WFM, should it be a separate change?
> > 
> > Likely.
> 
> Then I need a commit message, because I'm unable to justify this change myself.
> 
> > > Btw, how can I justify it?
> > 
> > Hmm, thinking about it, why is it not an enum?
> 
> Maybe, but it is a replica of UAPI definitions, do we want to see it as a enum?
> To me it will be a bit ugly looking.
> 
> > But it could be also an u8 because you want it be exactly 8 bits as you want
> > to be sure values up to 255 fit.
> 
> Depends on what we assume UAPI does with those flags. It maybe even less than
> 8 bits, or great than, currently 8 bits is enough...
> 
> TL;DR: I would rather take a patch from you and incorporate into the series
> than trying hard to invent a justification and proper type.

Okay, I want to send a new version, for now I leave the type change for
the next time. It looks that quirks as well will benefit from type clarifying.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 03/14] serial: port: Introduce a common helper to read properties

[Andy Shevchenko]

Several serial drivers want to read the same or similar set of
the port properties. Make a common helper for them.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/serial_port.c | 135 +++++++++++++++++++++++++++++++
 include/linux/serial_core.h      |   1 +
 2 files changed, 136 insertions(+)

diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c
index 88975a4df306..111e1f25fbc6 100644
--- a/drivers/tty/serial/serial_port.c
+++ b/drivers/tty/serial/serial_port.c
@@ -8,7 +8,10 @@
 
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/serial_core.h>
 #include <linux/spinlock.h>
 
@@ -82,6 +85,138 @@ void uart_remove_one_port(struct uart_driver *drv, struct uart_port *port)
 }
 EXPORT_SYMBOL(uart_remove_one_port);
 
+/**
+ * uart_read_port_properties - read firmware properties of the given UART port
+ * @port: corresponding port
+ * @use_defaults: apply defaults or validate the values
+ *
+ * The following device properties are supported:
+ *   - clock-frequency (optional)
+ *   - fifo-size (optional)
+ *   - no-loopback-test (optional)
+ *   - reg-shift (defaults may apply)
+ *   - reg-offset (value may be validated)
+ *   - reg-io-width (defaults may apply or value may be validated)
+ *   - interrupts (OF only)
+ *   - serial [alias ID] (OF only)
+ *
+ * If the port->dev is of struct platform_device type the interrupt line
+ * will be retrieved via platform_get_irq() call against that device.
+ * Otherwise it will be assigned by fwnode_irq_get() call. In both cases
+ * the index 0 of the resource is used.
+ *
+ * The caller is responsible to initialize the following fields of the @port
+ *   ->dev (must be valid)
+ *   ->flags
+ *   ->iotype (if @use_defaults is false)
+ *   ->mapbase
+ *   ->mapsize
+ *   ->regshift (if @use_defaults is false)
+ * before calling this function. Alternatively the above mentioned fields
+ * may be zeroed, in such case the only ones, that have associated properties
+ * found, will be set to the respective values.
+ *
+ * When @use_defaults is true and the respective property is not found
+ * the following values will be applied:
+ *   ->iotype = UPIO_MEM
+ *   ->regshift = 0
+ * In this case IRQ must be provided, otherwise an error will be returned.
+ *
+ * The ->irq, ->mapbase, ->mapsize are always altered if no error happened.
+ *
+ * When @use_defaults is false and the respective property is found
+ * the following values will be validated:
+ *   - reg-io-width (->iotype)
+ *   - reg-offset (->mapsize against ->mapbase)
+ *
+ * Returns: 0 on success or negative errno on failure
+ */
+int uart_read_port_properties(struct uart_port *port, bool use_defaults)
+{
+	struct device *dev = port->dev;
+	u32 value;
+	int ret;
+
+	/* Read optional UART functional clock frequency */
+	device_property_read_u32(dev, "clock-frequency", &port->uartclk);
+
+	/* Read the registers alignment (default: 8-bit) */
+	ret = device_property_read_u32(dev, "reg-shift", &value);
+	if (ret)
+		port->regshift = use_defaults ? 0 : port->regshift;
+	else
+		port->regshift = value;
+
+	/* Read the registers I/O access type (default: MMIO 8-bit) */
+	ret = device_property_read_u32(dev, "reg-io-width", &value);
+	if (ret) {
+		port->iotype = use_defaults ? UPIO_MEM : port->iotype;
+	} else {
+		switch (value) {
+		case 1:
+			port->iotype = UPIO_MEM;
+			break;
+		case 2:
+			port->iotype = UPIO_MEM16;
+			break;
+		case 4:
+			port->iotype = device_is_big_endian(dev) ? UPIO_MEM32BE : UPIO_MEM32;
+			break;
+		default:
+			if (!use_defaults) {
+				dev_err(dev, "Unsupported reg-io-width (%u)\n", value);
+				return -EINVAL;
+			}
+			port->iotype = UPIO_UNSET;
+			break;
+		}
+	}
+
+	/* Read the address mapping base offset (default: no offset) */
+	ret = device_property_read_u32(dev, "reg-offset", &value);
+	if (ret)
+		value = 0;
+
+	/* Check for shifted address mapping overflow */
+	if (!use_defaults && port->mapsize < value) {
+		dev_err(dev, "reg-offset %u exceeds region size %pa\n", value, &port->mapsize);
+		return -EINVAL;
+	}
+
+	port->mapbase += value;
+	port->mapsize -= value;
+
+	/* Read optional FIFO size */
+	device_property_read_u32(dev, "fifo-size", &port->fifosize);
+
+	if (device_property_read_bool(dev, "no-loopback-test"))
+		port->flags |= UPF_SKIP_TEST;
+
+	/* Get index of serial line, if found in DT aliases */
+	ret = of_alias_get_id(dev_of_node(dev), "serial");
+	if (ret >= 0)
+		port->line = ret;
+
+	if (dev_is_platform(dev))
+		ret = platform_get_irq(to_platform_device(dev), 0);
+	else
+		ret = fwnode_irq_get(dev_fwnode(dev), 0);
+	if (ret == -EPROBE_DEFER)
+		return ret;
+	if (ret > 0)
+		port->irq = ret;
+	else if (use_defaults)
+		/* By default IRQ support is mandatory */
+		return ret;
+	else
+		port->irq = 0;
+
+	port->flags |= UPF_SHARE_IRQ;
+
+	return 0;
+}
+EXPORT_SYMBOL(uart_read_port_properties);
+
 static struct device_driver serial_port_driver = {
 	.name = "port",
 	.suppress_bind_attrs = true,
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2b0526ae1fac..c93d775fc8a8 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -962,6 +962,7 @@ int uart_register_driver(struct uart_driver *uart);
 void uart_unregister_driver(struct uart_driver *uart);
 int uart_add_one_port(struct uart_driver *reg, struct uart_port *port);
 void uart_remove_one_port(struct uart_driver *reg, struct uart_port *port);
+int uart_read_port_properties(struct uart_port *port, bool use_defaults);
 bool uart_match_port(const struct uart_port *port1,
 		const struct uart_port *port2);
 
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 04/14] serial: 8250_aspeed_vuart: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_aspeed_vuart.c | 50 +++++++--------------
 1 file changed, 15 insertions(+), 35 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 8c2aaf7af7b7..2a4bc39b11af 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -419,8 +419,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
 	struct aspeed_vuart *vuart;
 	struct device_node *np;
 	struct resource *res;
-	u32 clk, prop, sirq[2];
 	int rc, sirq_polarity;
+	u32 prop, sirq[2];
 	struct clk *vclk;
 
 	np = pdev->dev.of_node;
@@ -447,53 +447,35 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
 	port.port.status = UPSTAT_SYNC_FIFO;
 	port.port.dev = &pdev->dev;
 	port.port.has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
+	port.port.flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_FIXED_PORT | UPF_FIXED_TYPE |
+			  UPF_NO_THRE_TEST;
 	port.bugs |= UART_BUG_TXRACE;
 
 	rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
 	if (rc < 0)
 		return rc;
 
-	if (of_property_read_u32(np, "clock-frequency", &clk)) {
+	rc = uart_read_port_properties(&port.port, true);
+	if (rc)
+		goto err_sysfs_remove;
+
+	/* Get clk rate through clk driver if present */
+	if (!port.port.uartclk) {
 		vclk = devm_clk_get_enabled(dev, NULL);
 		if (IS_ERR(vclk)) {
 			rc = dev_err_probe(dev, PTR_ERR(vclk), "clk or clock-frequency not defined\n");
 			goto err_sysfs_remove;
 		}
 
-		clk = clk_get_rate(vclk);
+		port.port.uartclk = clk_get_rate(vclk);
 	}
 
 	/* If current-speed was set, then try not to change it. */
 	if (of_property_read_u32(np, "current-speed", &prop) == 0)
-		port.port.custom_divisor = clk / (16 * prop);
+		port.port.custom_divisor = port.port.uartclk / (16 * prop);
 
-	/* Check for shifted address mapping */
-	if (of_property_read_u32(np, "reg-offset", &prop) == 0)
-		port.port.mapbase += prop;
-
-	/* Check for registers offset within the devices address range */
-	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
-		port.port.regshift = prop;
-
-	/* Check for fifo size */
-	if (of_property_read_u32(np, "fifo-size", &prop) == 0)
-		port.port.fifosize = prop;
-
-	/* Check for a fixed line number */
-	rc = of_alias_get_id(np, "serial");
-	if (rc >= 0)
-		port.port.line = rc;
-
-	port.port.irq = irq_of_parse_and_map(np, 0);
 	port.port.handle_irq = aspeed_vuart_handle_irq;
-	port.port.iotype = UPIO_MEM;
 	port.port.type = PORT_ASPEED_VUART;
-	port.port.uartclk = clk;
-	port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
-		| UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
-
-	if (of_property_read_bool(np, "no-loopback-test"))
-		port.port.flags |= UPF_SKIP_TEST;
 
 	if (port.port.fifosize)
 		port.capabilities = UART_CAP_FIFO;
@@ -503,7 +485,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
 
 	rc = serial8250_register_8250_port(&port);
 	if (rc < 0)
-		goto err_clk_disable;
+		goto err_sysfs_remove;
 
 	vuart->line = rc;
 	vuart->port = serial8250_get_port(vuart->line);
@@ -529,7 +511,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
 	rc = aspeed_vuart_set_lpc_address(vuart, prop);
 	if (rc < 0) {
 		dev_err_probe(dev, rc, "invalid value in aspeed,lpc-io-reg property\n");
-		goto err_clk_disable;
+		goto err_sysfs_remove;
 	}
 
 	rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
@@ -541,14 +523,14 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
 	rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
 	if (rc < 0) {
 		dev_err_probe(dev, rc, "invalid sirq number in aspeed,lpc-interrupts property\n");
-		goto err_clk_disable;
+		goto err_sysfs_remove;
 	}
 
 	sirq_polarity = aspeed_vuart_map_irq_polarity(sirq[1]);
 	if (sirq_polarity < 0) {
 		rc = dev_err_probe(dev, sirq_polarity,
 				   "invalid sirq polarity in aspeed,lpc-interrupts property\n");
-		goto err_clk_disable;
+		goto err_sysfs_remove;
 	}
 
 	aspeed_vuart_set_sirq_polarity(vuart, sirq_polarity);
@@ -559,8 +541,6 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_clk_disable:
-	irq_dispose_mapping(port.port.irq);
 err_sysfs_remove:
 	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
 	return rc;
-- 
2.43.0.rc1.1.gbec44491f096

Re: [PATCH v1 04/14] serial: 8250_aspeed_vuart: Switch to use uart_read_port_properties()

[Andrew Jeffery]

On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> Since we have now a common helper to read port properties
> use it instead of sparse home grown solution.
> 
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>

[PATCH v1 05/14] serial: 8250_bcm2835aux: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_bcm2835aux.c | 92 +++++++++++------------
 1 file changed, 42 insertions(+), 50 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index beac6b340ace..69c3c5ca77f7 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -45,10 +45,6 @@ struct bcm2835aux_data {
 	u32 cntl;
 };
 
-struct bcm2835_aux_serial_driver_data {
-	resource_size_t offset;
-};
-
 static void bcm2835aux_rs485_start_tx(struct uart_8250_port *up)
 {
 	if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX)) {
@@ -85,10 +81,9 @@ static void bcm2835aux_rs485_stop_tx(struct uart_8250_port *up)
 
 static int bcm2835aux_serial_probe(struct platform_device *pdev)
 {
-	const struct bcm2835_aux_serial_driver_data *bcm_data;
+	const struct software_node *bcm2835_swnode;
 	struct uart_8250_port up = { };
 	struct bcm2835aux_data *data;
-	resource_size_t offset = 0;
 	struct resource *res;
 	unsigned int uartclk;
 	int ret;
@@ -101,12 +96,8 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 	/* initialize data */
 	up.capabilities = UART_CAP_FIFO | UART_CAP_MINI;
 	up.port.dev = &pdev->dev;
-	up.port.regshift = 2;
 	up.port.type = PORT_16550;
-	up.port.iotype = UPIO_MEM;
-	up.port.fifosize = 8;
-	up.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE |
-			UPF_SKIP_TEST | UPF_IOREMAP;
+	up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SKIP_TEST | UPF_IOREMAP;
 	up.port.rs485_config = serial8250_em485_config;
 	up.port.rs485_supported = serial8250_em485_supported;
 	up.rs485_start_tx = bcm2835aux_rs485_start_tx;
@@ -122,12 +113,6 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 	if (IS_ERR(data->clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(data->clk), "could not get clk\n");
 
-	/* get the interrupt */
-	ret = platform_get_irq(pdev, 0);
-	if (ret < 0)
-		return ret;
-	up.port.irq = ret;
-
 	/* map the main registers */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -135,52 +120,40 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	bcm_data = device_get_match_data(&pdev->dev);
+	up.port.mapbase = res->start;
+	up.port.mapsize = resource_size(res);
 
-	/* Some UEFI implementations (e.g. tianocore/edk2 for the Raspberry Pi)
-	 * describe the miniuart with a base address that encompasses the auxiliary
-	 * registers shared between the miniuart and spi.
-	 *
-	 * This is due to historical reasons, see discussion here :
-	 * https://edk2.groups.io/g/devel/topic/87501357#84349
-	 *
-	 * We need to add the offset between the miniuart and auxiliary
-	 * registers to get the real miniuart base address.
-	 */
-	if (bcm_data)
-		offset = bcm_data->offset;
+	bcm2835_swnode = device_get_match_data(&pdev->dev);
+	if (bcm2835_swnode) {
+		ret = device_add_software_node(&pdev->dev, bcm2835_swnode);
+		if (ret)
+			return ret;
+	}
 
-	up.port.mapbase = res->start + offset;
-	up.port.mapsize = resource_size(res) - offset;
+	ret = uart_read_port_properties(&up.port, true);
+	if (ret)
+		goto rm_swnode;
 
-	/* Check for a fixed line number */
-	ret = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (ret >= 0)
-		up.port.line = ret;
+	up.port.regshift = 2;
+	up.port.fifosize = 8;
 
 	/* enable the clock as a last step */
 	ret = clk_prepare_enable(data->clk);
 	if (ret) {
-		dev_err(&pdev->dev, "unable to enable uart clock - %d\n",
-			ret);
-		return ret;
+		dev_err_probe(&pdev->dev, ret, "unable to enable uart clock\n");
+		goto rm_swnode;
 	}
 
 	uartclk = clk_get_rate(data->clk);
-	if (!uartclk) {
-		ret = device_property_read_u32(&pdev->dev, "clock-frequency", &uartclk);
-		if (ret) {
-			dev_err_probe(&pdev->dev, ret, "could not get clk rate\n");
-			goto dis_clk;
-		}
-	}
+	if (uartclk)
+		up.port.uartclk = uartclk;
 
 	/* the HW-clock divider for bcm2835aux is 8,
 	 * but 8250 expects a divider of 16,
 	 * so we have to multiply the actual clock by 2
 	 * to get identical baudrates.
 	 */
-	up.port.uartclk = uartclk * 2;
+	up.port.uartclk *= 2;
 
 	/* register the port */
 	ret = serial8250_register_8250_port(&up);
@@ -194,6 +167,8 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 
 dis_clk:
 	clk_disable_unprepare(data->clk);
+rm_swnode:
+	device_remove_software_node(&pdev->dev);
 	return ret;
 }
 
@@ -203,10 +178,27 @@ static void bcm2835aux_serial_remove(struct platform_device *pdev)
 
 	serial8250_unregister_port(data->line);
 	clk_disable_unprepare(data->clk);
+	device_remove_software_node(&pdev->dev);
 }
 
-static const struct bcm2835_aux_serial_driver_data bcm2835_acpi_data = {
-	.offset = 0x40,
+/*
+ * Some UEFI implementations (e.g. tianocore/edk2 for the Raspberry Pi)
+ * describe the miniuart with a base address that encompasses the auxiliary
+ * registers shared between the miniuart and spi.
+ *
+ * This is due to historical reasons, see discussion here:
+ * https://edk2.groups.io/g/devel/topic/87501357#84349
+ *
+ * We need to add the offset between the miniuart and auxiliary registers
+ * to get the real miniuart base address.
+ */
+static const struct property_entry bcm2835_acpi_properties[] = {
+	PROPERTY_ENTRY_U32("reg-offset", 0x40),
+	{ }
+};
+
+static const struct software_node bcm2835_acpi_node = {
+	.properties = bcm2835_acpi_properties,
 };
 
 static const struct of_device_id bcm2835aux_serial_match[] = {
@@ -216,7 +208,7 @@ static const struct of_device_id bcm2835aux_serial_match[] = {
 MODULE_DEVICE_TABLE(of, bcm2835aux_serial_match);
 
 static const struct acpi_device_id bcm2835aux_serial_acpi_match[] = {
-	{ "BCM2836", (kernel_ulong_t)&bcm2835_acpi_data },
+	{ "BCM2836", (kernel_ulong_t)&bcm2835_acpi_node },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, bcm2835aux_serial_acpi_match);
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 06/14] serial: 8250_bcm7271: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_bcm7271.c | 53 +++++++++-----------------
 1 file changed, 18 insertions(+), 35 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
index 1532fa2e8ec4..5a25e78857f7 100644
--- a/drivers/tty/serial/8250/8250_bcm7271.c
+++ b/drivers/tty/serial/8250/8250_bcm7271.c
@@ -942,10 +942,8 @@ static int brcmuart_probe(struct platform_device *pdev)
 	struct brcmuart_priv *priv;
 	struct clk *baud_mux_clk;
 	struct uart_8250_port up;
-	int irq;
 	void __iomem *membase = NULL;
 	resource_size_t mapbase = 0;
-	u32 clk_rate = 0;
 	int ret;
 	int x;
 	int dma_irq;
@@ -953,9 +951,6 @@ static int brcmuart_probe(struct platform_device *pdev)
 		"uart", "dma_rx", "dma_tx", "dma_intr2", "dma_arb"
 	};
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
 	priv = devm_kzalloc(dev, sizeof(struct brcmuart_priv),
 			GFP_KERNEL);
 	if (!priv)
@@ -1011,7 +1006,23 @@ static int brcmuart_probe(struct platform_device *pdev)
 		}
 	}
 
-	of_property_read_u32(np, "clock-frequency", &clk_rate);
+	dev_dbg(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
+
+	memset(&up, 0, sizeof(up));
+	up.port.type = PORT_BCM7271;
+	up.port.dev = dev;
+	up.port.mapbase = mapbase;
+	up.port.membase = membase;
+	up.port.handle_irq = brcmuart_handle_irq;
+	up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
+	up.port.private_data = priv;
+
+	ret = uart_read_port_properties(&up.port, true);
+	if (ret)
+		goto release_dma;
+
+	up.port.regshift = 2;
+	up.port.iotype = device_is_big_endian(dev) ? UPIO_MEM32BE : UPIO_MEM32;
 
 	/* See if a Baud clock has been specified */
 	baud_mux_clk = devm_clk_get_optional_enabled(dev, "sw_baud");
@@ -1023,39 +1034,11 @@ static int brcmuart_probe(struct platform_device *pdev)
 
 		priv->baud_mux_clk = baud_mux_clk;
 		init_real_clk_rates(dev, priv);
-		clk_rate = priv->default_mux_rate;
+		up.port.uartclk = priv->default_mux_rate;
 	} else {
 		dev_dbg(dev, "BAUD MUX clock not specified\n");
 	}
 
-	if (clk_rate == 0) {
-		ret = dev_err_probe(dev, -EINVAL, "clock-frequency or clk not defined\n");
-		goto release_dma;
-	}
-
-	dev_dbg(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
-
-	memset(&up, 0, sizeof(up));
-	up.port.type = PORT_BCM7271;
-	up.port.uartclk = clk_rate;
-	up.port.dev = dev;
-	up.port.mapbase = mapbase;
-	up.port.membase = membase;
-	up.port.irq = irq;
-	up.port.handle_irq = brcmuart_handle_irq;
-	up.port.regshift = 2;
-	up.port.iotype = of_device_is_big_endian(np) ?
-		UPIO_MEM32BE : UPIO_MEM32;
-	up.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
-		| UPF_FIXED_PORT | UPF_FIXED_TYPE;
-	up.port.dev = dev;
-	up.port.private_data = priv;
-
-	/* Check for a fixed line number */
-	ret = of_alias_get_id(np, "serial");
-	if (ret >= 0)
-		up.port.line = ret;
-
 	/* setup HR timer */
 	hrtimer_init(&priv->hrt, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
 	priv->hrt.function = brcmuart_hrtimer_func;
-- 
2.43.0.rc1.1.gbec44491f096

Re: [PATCH v1 06/14] serial: 8250_bcm7271: Switch to use uart_read_port_properties()

[kernel test robot]

Hi Andy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.8-rc5 next-20240222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/serial-core-Move-struct-uart_port-quirks-closer-to-possible-values/20240222-023850
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20240221183442.4124354-7-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 06/14] serial: 8250_bcm7271: Switch to use uart_read_port_properties()
config: hexagon-randconfig-r123-20240222 (https://download.01.org/0day-ci/archive/20240223/202402231238.AWqLyIoM-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 36adfec155de366d722f2bac8ff9162289dcf06c)
reproduce: (https://download.01.org/0day-ci/archive/20240223/202402231238.AWqLyIoM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402231238.AWqLyIoM-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/tty/serial/8250/8250_bcm7271.c:15:
   In file included from include/linux/tty.h:11:
   In file included from include/linux/tty_port.h:5:
   In file included from include/linux/kfifo.h:42:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/tty/serial/8250/8250_bcm7271.c:15:
   In file included from include/linux/tty.h:11:
   In file included from include/linux/tty_port.h:5:
   In file included from include/linux/kfifo.h:42:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/tty/serial/8250/8250_bcm7271.c:15:
   In file included from include/linux/tty.h:11:
   In file included from include/linux/tty_port.h:5:
   In file included from include/linux/kfifo.h:42:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/tty/serial/8250/8250_bcm7271.c:938:22: warning: unused variable 'np' [-Wunused-variable]
     938 |         struct device_node *np = pdev->dev.of_node;
         |                             ^~
   7 warnings generated.


vim +/np +938 drivers/tty/serial/8250/8250_bcm7271.c

41a469482de257e Al Cooper       2021-03-25   933  
41a469482de257e Al Cooper       2021-03-25   934  
41a469482de257e Al Cooper       2021-03-25   935  static int brcmuart_probe(struct platform_device *pdev)
41a469482de257e Al Cooper       2021-03-25   936  {
41a469482de257e Al Cooper       2021-03-25   937  	struct resource *regs;
41a469482de257e Al Cooper       2021-03-25  @938  	struct device_node *np = pdev->dev.of_node;
41a469482de257e Al Cooper       2021-03-25   939  	const struct of_device_id *of_id = NULL;
41a469482de257e Al Cooper       2021-03-25   940  	struct uart_8250_port *new_port;
41a469482de257e Al Cooper       2021-03-25   941  	struct device *dev = &pdev->dev;
41a469482de257e Al Cooper       2021-03-25   942  	struct brcmuart_priv *priv;
41a469482de257e Al Cooper       2021-03-25   943  	struct clk *baud_mux_clk;
41a469482de257e Al Cooper       2021-03-25   944  	struct uart_8250_port up;
8a66b31a15966ea Colin Ian King  2021-07-19   945  	void __iomem *membase = NULL;
41a469482de257e Al Cooper       2021-03-25   946  	resource_size_t mapbase = 0;
41a469482de257e Al Cooper       2021-03-25   947  	int ret;
41a469482de257e Al Cooper       2021-03-25   948  	int x;
41a469482de257e Al Cooper       2021-03-25   949  	int dma_irq;
41a469482de257e Al Cooper       2021-03-25   950  	static const char * const reg_names[REGS_MAX] = {
41a469482de257e Al Cooper       2021-03-25   951  		"uart", "dma_rx", "dma_tx", "dma_intr2", "dma_arb"
41a469482de257e Al Cooper       2021-03-25   952  	};
41a469482de257e Al Cooper       2021-03-25   953  
41a469482de257e Al Cooper       2021-03-25   954  	priv = devm_kzalloc(dev, sizeof(struct brcmuart_priv),
41a469482de257e Al Cooper       2021-03-25   955  			GFP_KERNEL);
41a469482de257e Al Cooper       2021-03-25   956  	if (!priv)
41a469482de257e Al Cooper       2021-03-25   957  		return -ENOMEM;
41a469482de257e Al Cooper       2021-03-25   958  
41a469482de257e Al Cooper       2021-03-25   959  	of_id = of_match_node(brcmuart_dt_ids, np);
41a469482de257e Al Cooper       2021-03-25   960  	if (!of_id || !of_id->data)
41a469482de257e Al Cooper       2021-03-25   961  		priv->rate_table = brcmstb_rate_table;
41a469482de257e Al Cooper       2021-03-25   962  	else
41a469482de257e Al Cooper       2021-03-25   963  		priv->rate_table = of_id->data;
41a469482de257e Al Cooper       2021-03-25   964  
41a469482de257e Al Cooper       2021-03-25   965  	for (x = 0; x < REGS_MAX; x++) {
41a469482de257e Al Cooper       2021-03-25   966  		regs = platform_get_resource_byname(pdev, IORESOURCE_MEM,
41a469482de257e Al Cooper       2021-03-25   967  						reg_names[x]);
41a469482de257e Al Cooper       2021-03-25   968  		if (!regs)
41a469482de257e Al Cooper       2021-03-25   969  			break;
41a469482de257e Al Cooper       2021-03-25   970  		priv->regs[x] =	devm_ioremap(dev, regs->start,
41a469482de257e Al Cooper       2021-03-25   971  					     resource_size(regs));
64b1510642f845d Wei Yongjun     2021-03-29   972  		if (!priv->regs[x])
64b1510642f845d Wei Yongjun     2021-03-29   973  			return -ENOMEM;
41a469482de257e Al Cooper       2021-03-25   974  		if (x == REGS_8250) {
41a469482de257e Al Cooper       2021-03-25   975  			mapbase = regs->start;
41a469482de257e Al Cooper       2021-03-25   976  			membase = priv->regs[x];
41a469482de257e Al Cooper       2021-03-25   977  		}
41a469482de257e Al Cooper       2021-03-25   978  	}
41a469482de257e Al Cooper       2021-03-25   979  
41a469482de257e Al Cooper       2021-03-25   980  	/* We should have just the uart base registers or all the registers */
c77247a52be2359 Andy Shevchenko 2023-09-18   981  	if (x != 1 && x != REGS_MAX)
c77247a52be2359 Andy Shevchenko 2023-09-18   982  		return dev_err_probe(dev, -EINVAL, "%s registers not specified\n",
c77247a52be2359 Andy Shevchenko 2023-09-18   983  				     reg_names[x]);
41a469482de257e Al Cooper       2021-03-25   984  
41a469482de257e Al Cooper       2021-03-25   985  	/* if the DMA registers were specified, try to enable DMA */
41a469482de257e Al Cooper       2021-03-25   986  	if (x > REGS_DMA_RX) {
41a469482de257e Al Cooper       2021-03-25   987  		if (brcmuart_arbitration(priv, 1) == 0) {
41a469482de257e Al Cooper       2021-03-25   988  			u32 txrev = 0;
41a469482de257e Al Cooper       2021-03-25   989  			u32 rxrev = 0;
41a469482de257e Al Cooper       2021-03-25   990  
41a469482de257e Al Cooper       2021-03-25   991  			txrev = udma_readl(priv, REGS_DMA_RX, UDMA_RX_REVISION);
41a469482de257e Al Cooper       2021-03-25   992  			rxrev = udma_readl(priv, REGS_DMA_TX, UDMA_TX_REVISION);
41a469482de257e Al Cooper       2021-03-25   993  			if ((txrev >= UDMA_TX_REVISION_REQUIRED) &&
41a469482de257e Al Cooper       2021-03-25   994  				(rxrev >= UDMA_RX_REVISION_REQUIRED)) {
41a469482de257e Al Cooper       2021-03-25   995  
41a469482de257e Al Cooper       2021-03-25   996  				/* Enable the use of the DMA hardware */
41a469482de257e Al Cooper       2021-03-25   997  				priv->dma_enabled = true;
41a469482de257e Al Cooper       2021-03-25   998  			} else {
41a469482de257e Al Cooper       2021-03-25   999  				brcmuart_arbitration(priv, 0);
41a469482de257e Al Cooper       2021-03-25  1000  				dev_err(dev,
41a469482de257e Al Cooper       2021-03-25  1001  					"Unsupported DMA Hardware Revision\n");
41a469482de257e Al Cooper       2021-03-25  1002  			}
41a469482de257e Al Cooper       2021-03-25  1003  		} else {
41a469482de257e Al Cooper       2021-03-25  1004  			dev_err(dev,
41a469482de257e Al Cooper       2021-03-25  1005  				"Timeout arbitrating for UART DMA hardware\n");
41a469482de257e Al Cooper       2021-03-25  1006  		}
41a469482de257e Al Cooper       2021-03-25  1007  	}
41a469482de257e Al Cooper       2021-03-25  1008  
19010c5b7125670 Andy Shevchenko 2024-02-21  1009  	dev_dbg(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
19010c5b7125670 Andy Shevchenko 2024-02-21  1010  
19010c5b7125670 Andy Shevchenko 2024-02-21  1011  	memset(&up, 0, sizeof(up));
19010c5b7125670 Andy Shevchenko 2024-02-21  1012  	up.port.type = PORT_BCM7271;
19010c5b7125670 Andy Shevchenko 2024-02-21  1013  	up.port.dev = dev;
19010c5b7125670 Andy Shevchenko 2024-02-21  1014  	up.port.mapbase = mapbase;
19010c5b7125670 Andy Shevchenko 2024-02-21  1015  	up.port.membase = membase;
19010c5b7125670 Andy Shevchenko 2024-02-21  1016  	up.port.handle_irq = brcmuart_handle_irq;
19010c5b7125670 Andy Shevchenko 2024-02-21  1017  	up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
19010c5b7125670 Andy Shevchenko 2024-02-21  1018  	up.port.private_data = priv;
19010c5b7125670 Andy Shevchenko 2024-02-21  1019  
19010c5b7125670 Andy Shevchenko 2024-02-21  1020  	ret = uart_read_port_properties(&up.port, true);
19010c5b7125670 Andy Shevchenko 2024-02-21  1021  	if (ret)
19010c5b7125670 Andy Shevchenko 2024-02-21  1022  		goto release_dma;
19010c5b7125670 Andy Shevchenko 2024-02-21  1023  
19010c5b7125670 Andy Shevchenko 2024-02-21  1024  	up.port.regshift = 2;
19010c5b7125670 Andy Shevchenko 2024-02-21  1025  	up.port.iotype = device_is_big_endian(dev) ? UPIO_MEM32BE : UPIO_MEM32;
41a469482de257e Al Cooper       2021-03-25  1026  
41a469482de257e Al Cooper       2021-03-25  1027  	/* See if a Baud clock has been specified */
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05  1028  	baud_mux_clk = devm_clk_get_optional_enabled(dev, "sw_baud");
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05  1029  	ret = PTR_ERR_OR_ZERO(baud_mux_clk);
41a469482de257e Al Cooper       2021-03-25  1030  	if (ret)
15ac1122fd6d4bf Doug Berger     2023-03-09  1031  		goto release_dma;
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05  1032  	if (baud_mux_clk) {
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05  1033  		dev_dbg(dev, "BAUD MUX clock found\n");
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05  1034  
41a469482de257e Al Cooper       2021-03-25  1035  		priv->baud_mux_clk = baud_mux_clk;
41a469482de257e Al Cooper       2021-03-25  1036  		init_real_clk_rates(dev, priv);
19010c5b7125670 Andy Shevchenko 2024-02-21  1037  		up.port.uartclk = priv->default_mux_rate;
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05  1038  	} else {
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05  1039  		dev_dbg(dev, "BAUD MUX clock not specified\n");
41a469482de257e Al Cooper       2021-03-25  1040  	}
41a469482de257e Al Cooper       2021-03-25  1041  
41a469482de257e Al Cooper       2021-03-25  1042  	/* setup HR timer */
41a469482de257e Al Cooper       2021-03-25  1043  	hrtimer_init(&priv->hrt, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
41a469482de257e Al Cooper       2021-03-25  1044  	priv->hrt.function = brcmuart_hrtimer_func;
41a469482de257e Al Cooper       2021-03-25  1045  
41a469482de257e Al Cooper       2021-03-25  1046  	up.port.shutdown = brcmuart_shutdown;
41a469482de257e Al Cooper       2021-03-25  1047  	up.port.startup = brcmuart_startup;
41a469482de257e Al Cooper       2021-03-25  1048  	up.port.throttle = brcmuart_throttle;
41a469482de257e Al Cooper       2021-03-25  1049  	up.port.unthrottle = brcmuart_unthrottle;
41a469482de257e Al Cooper       2021-03-25  1050  	up.port.set_termios = brcmstb_set_termios;
41a469482de257e Al Cooper       2021-03-25  1051  
41a469482de257e Al Cooper       2021-03-25  1052  	if (priv->dma_enabled) {
41a469482de257e Al Cooper       2021-03-25  1053  		priv->rx_size = RX_BUF_SIZE * RX_BUFS_COUNT;
41a469482de257e Al Cooper       2021-03-25  1054  		priv->rx_bufs = dma_alloc_coherent(dev,
41a469482de257e Al Cooper       2021-03-25  1055  						   priv->rx_size,
41a469482de257e Al Cooper       2021-03-25  1056  						   &priv->rx_addr, GFP_KERNEL);
c195438f1e84de8 Lad Prabhakar   2021-12-24  1057  		if (!priv->rx_bufs) {
0e479b460e342c5 Lad Prabhakar   2022-01-05  1058  			ret = -ENOMEM;
41a469482de257e Al Cooper       2021-03-25  1059  			goto err;
c195438f1e84de8 Lad Prabhakar   2021-12-24  1060  		}
41a469482de257e Al Cooper       2021-03-25  1061  		priv->tx_size = UART_XMIT_SIZE;
41a469482de257e Al Cooper       2021-03-25  1062  		priv->tx_buf = dma_alloc_coherent(dev,
41a469482de257e Al Cooper       2021-03-25  1063  						  priv->tx_size,
41a469482de257e Al Cooper       2021-03-25  1064  						  &priv->tx_addr, GFP_KERNEL);
c195438f1e84de8 Lad Prabhakar   2021-12-24  1065  		if (!priv->tx_buf) {
0e479b460e342c5 Lad Prabhakar   2022-01-05  1066  			ret = -ENOMEM;
41a469482de257e Al Cooper       2021-03-25  1067  			goto err;
41a469482de257e Al Cooper       2021-03-25  1068  		}
c195438f1e84de8 Lad Prabhakar   2021-12-24  1069  	}
41a469482de257e Al Cooper       2021-03-25  1070  
41a469482de257e Al Cooper       2021-03-25  1071  	ret = serial8250_register_8250_port(&up);
41a469482de257e Al Cooper       2021-03-25  1072  	if (ret < 0) {
c77247a52be2359 Andy Shevchenko 2023-09-18  1073  		dev_err_probe(dev, ret, "unable to register 8250 port\n");
41a469482de257e Al Cooper       2021-03-25  1074  		goto err;
41a469482de257e Al Cooper       2021-03-25  1075  	}
41a469482de257e Al Cooper       2021-03-25  1076  	priv->line = ret;
41a469482de257e Al Cooper       2021-03-25  1077  	new_port = serial8250_get_port(ret);
41a469482de257e Al Cooper       2021-03-25  1078  	priv->up = &new_port->port;
41a469482de257e Al Cooper       2021-03-25  1079  	if (priv->dma_enabled) {
41a469482de257e Al Cooper       2021-03-25  1080  		dma_irq = platform_get_irq_byname(pdev,  "dma");
41a469482de257e Al Cooper       2021-03-25  1081  		if (dma_irq < 0) {
c77247a52be2359 Andy Shevchenko 2023-09-18  1082  			ret = dev_err_probe(dev, dma_irq, "no IRQ resource info\n");
41a469482de257e Al Cooper       2021-03-25  1083  			goto err1;
41a469482de257e Al Cooper       2021-03-25  1084  		}
41a469482de257e Al Cooper       2021-03-25  1085  		ret = devm_request_irq(dev, dma_irq, brcmuart_isr,
41a469482de257e Al Cooper       2021-03-25  1086  				IRQF_SHARED, "uart DMA irq", &new_port->port);
41a469482de257e Al Cooper       2021-03-25  1087  		if (ret) {
c77247a52be2359 Andy Shevchenko 2023-09-18  1088  			dev_err_probe(dev, ret, "unable to register IRQ handler\n");
41a469482de257e Al Cooper       2021-03-25  1089  			goto err1;
41a469482de257e Al Cooper       2021-03-25  1090  		}
41a469482de257e Al Cooper       2021-03-25  1091  	}
41a469482de257e Al Cooper       2021-03-25  1092  	platform_set_drvdata(pdev, priv);
41a469482de257e Al Cooper       2021-03-25  1093  	brcmuart_init_debugfs(priv, dev_name(&pdev->dev));
41a469482de257e Al Cooper       2021-03-25  1094  	return 0;
41a469482de257e Al Cooper       2021-03-25  1095  
41a469482de257e Al Cooper       2021-03-25  1096  err1:
41a469482de257e Al Cooper       2021-03-25  1097  	serial8250_unregister_port(priv->line);
41a469482de257e Al Cooper       2021-03-25  1098  err:
41a469482de257e Al Cooper       2021-03-25  1099  	brcmuart_free_bufs(dev, priv);
15ac1122fd6d4bf Doug Berger     2023-03-09  1100  release_dma:
15ac1122fd6d4bf Doug Berger     2023-03-09  1101  	if (priv->dma_enabled)
41a469482de257e Al Cooper       2021-03-25  1102  		brcmuart_arbitration(priv, 0);
c195438f1e84de8 Lad Prabhakar   2021-12-24  1103  	return ret;
41a469482de257e Al Cooper       2021-03-25  1104  }
41a469482de257e Al Cooper       2021-03-25  1105  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 06/14] serial: 8250_bcm7271: Switch to use uart_read_port_properties()

[Andy Shevchenko]

On Fri, Feb 23, 2024 at 12:14:38PM +0800, kernel test robot wrote:
> Hi Andy,
> 
> kernel test robot noticed the following build warnings:

> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202402231238.AWqLyIoM-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 

>    In file included from arch/hexagon/include/asm/io.h:328:
>    include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>      547 |         val = __raw_readb(PCI_IOBASE + addr);
>          |                           ~~~~~~~~~~ ^

Okay, the above is well known issue with hexagon arch, not related to the series.

> >> drivers/tty/serial/8250/8250_bcm7271.c:938:22: warning: unused variable 'np' [-Wunused-variable]
>      938 |         struct device_node *np = pdev->dev.of_node;
>          |                             ^~

This one is a good catch! Will be fixed in a new version.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 07/14] serial: 8250_dw: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 67 +++++++++++++------------------
 1 file changed, 27 insertions(+), 40 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 2d1f350a4bea..a1825e83231f 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -17,7 +17,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/notifier.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
@@ -449,12 +448,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 
 	if (np) {
 		unsigned int quirks = data->pdata->quirks;
-		int id;
 
-		/* get index of serial line, if found in DT aliases */
-		id = of_alias_get_id(np, "serial");
-		if (id >= 0)
-			p->line = id;
 #ifdef CONFIG_64BIT
 		if (quirks & DW_UART_QUIRK_OCTEON) {
 			p->serial_in = dw8250_serial_inq;
@@ -465,12 +459,6 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 		}
 #endif
 
-		if (of_device_is_big_endian(np)) {
-			p->iotype = UPIO_MEM32BE;
-			p->serial_in = dw8250_serial_in32be;
-			p->serial_out = dw8250_serial_out32be;
-		}
-
 		if (quirks & DW_UART_QUIRK_ARMADA_38X)
 			p->serial_out = dw8250_serial_out38x;
 		if (quirks & DW_UART_QUIRK_SKIP_SET_RATE)
@@ -510,39 +498,21 @@ static int dw8250_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct dw8250_data *data;
 	struct resource *regs;
-	int irq;
 	int err;
-	u32 val;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!regs)
 		return dev_err_probe(dev, -EINVAL, "no registers defined\n");
 
-	irq = platform_get_irq_optional(pdev, 0);
-	/* no interrupt -> fall back to polling */
-	if (irq == -ENXIO)
-		irq = 0;
-	if (irq < 0)
-		return irq;
-
 	spin_lock_init(&p->lock);
-	p->mapbase	= regs->start;
-	p->irq		= irq;
 	p->handle_irq	= dw8250_handle_irq;
 	p->pm		= dw8250_do_pm;
 	p->type		= PORT_8250;
-	p->flags	= UPF_SHARE_IRQ | UPF_FIXED_PORT;
+	p->flags	= UPF_FIXED_PORT;
 	p->dev		= dev;
-	p->iotype	= UPIO_MEM;
-	p->serial_in	= dw8250_serial_in;
-	p->serial_out	= dw8250_serial_out;
 	p->set_ldisc	= dw8250_set_ldisc;
 	p->set_termios	= dw8250_set_termios;
 
-	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
-	if (!p->membase)
-		return -ENOMEM;
-
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
@@ -554,15 +524,35 @@ static int dw8250_probe(struct platform_device *pdev)
 	data->uart_16550_compatible = device_property_read_bool(dev,
 						"snps,uart-16550-compatible");
 
-	err = device_property_read_u32(dev, "reg-shift", &val);
-	if (!err)
-		p->regshift = val;
+	p->mapbase = regs->start;
+	p->mapsize = resource_size(regs);
 
-	err = device_property_read_u32(dev, "reg-io-width", &val);
-	if (!err && val == 4) {
-		p->iotype = UPIO_MEM32;
+	p->membase = devm_ioremap(dev, p->mapbase, p->mapsize);
+	if (!p->membase)
+		return -ENOMEM;
+
+	err = uart_read_port_properties(p, true);
+	/* no interrupt -> fall back to polling */
+	if (err == -ENXIO)
+		err = 0;
+	if (err)
+		return err;
+
+	switch (p->iotype) {
+	case UPIO_MEM:
+		p->serial_in = dw8250_serial_in;
+		p->serial_out = dw8250_serial_out;
+		break;
+	case UPIO_MEM32:
 		p->serial_in = dw8250_serial_in32;
 		p->serial_out = dw8250_serial_out32;
+		break;
+	case UPIO_MEM32BE:
+		p->serial_in = dw8250_serial_in32be;
+		p->serial_out = dw8250_serial_out32be;
+		break;
+	default:
+		return -ENODEV;
 	}
 
 	if (device_property_read_bool(dev, "dcd-override")) {
@@ -589,9 +579,6 @@ static int dw8250_probe(struct platform_device *pdev)
 		data->msr_mask_off |= UART_MSR_TERI;
 	}
 
-	/* Always ask for fixed clock rate from a property. */
-	device_property_read_u32(dev, "clock-frequency", &p->uartclk);
-
 	/* If there is separate baudclk, get the rate from it. */
 	data->clk = devm_clk_get_optional_enabled(dev, "baudclk");
 	if (data->clk == NULL)
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 08/14] serial: 8250_ingenic: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_ingenic.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index a12f737924c0..7603129f1c07 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -234,7 +234,7 @@ static int ingenic_uart_probe(struct platform_device *pdev)
 	struct ingenic_uart_data *data;
 	const struct ingenic_uart_config *cdata;
 	struct resource *regs;
-	int irq, err, line;
+	int err;
 
 	cdata = of_device_get_match_data(&pdev->dev);
 	if (!cdata) {
@@ -242,10 +242,6 @@ static int ingenic_uart_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!regs) {
 		dev_err(&pdev->dev, "no registers defined\n");
@@ -259,21 +255,19 @@ static int ingenic_uart_probe(struct platform_device *pdev)
 	spin_lock_init(&uart.port.lock);
 	uart.port.type = PORT_16550A;
 	uart.port.flags = UPF_SKIP_TEST | UPF_IOREMAP | UPF_FIXED_TYPE;
-	uart.port.iotype = UPIO_MEM;
 	uart.port.mapbase = regs->start;
-	uart.port.regshift = 2;
 	uart.port.serial_out = ingenic_uart_serial_out;
 	uart.port.serial_in = ingenic_uart_serial_in;
-	uart.port.irq = irq;
 	uart.port.dev = &pdev->dev;
-	uart.port.fifosize = cdata->fifosize;
 	uart.tx_loadsz = cdata->tx_loadsz;
 	uart.capabilities = UART_CAP_FIFO | UART_CAP_RTOIE;
 
-	/* Check for a fixed line number */
-	line = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (line >= 0)
-		uart.port.line = line;
+	err = uart_read_port_properties(&uart.port, true);
+	if (err)
+		return err;
+
+	uart.port.regshift = 2;
+	uart.port.fifosize = cdata->fifosize;
 
 	uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
 					 resource_size(regs));
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 09/14] serial: 8250_lpc18xx: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_lpc18xx.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index 8d728a6a5991..e4a6b7b4caf2 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -92,11 +92,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
 	struct lpc18xx_uart_data *data;
 	struct uart_8250_port uart;
 	struct resource *res;
-	int irq, ret;
-
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -139,19 +135,12 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
 		goto dis_clk_reg;
 	}
 
-	ret = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (ret >= 0)
-		uart.port.line = ret;
-
 	data->dma.rx_param = data;
 	data->dma.tx_param = data;
 
 	spin_lock_init(&uart.port.lock);
 	uart.port.dev = &pdev->dev;
-	uart.port.irq = irq;
-	uart.port.iotype = UPIO_MEM32;
 	uart.port.mapbase = res->start;
-	uart.port.regshift = 2;
 	uart.port.type = PORT_16550A;
 	uart.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SKIP_TEST;
 	uart.port.uartclk = clk_get_rate(data->clk_uart);
@@ -160,6 +149,13 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
 	uart.port.rs485_supported = lpc18xx_rs485_supported;
 	uart.port.serial_out = lpc18xx_uart_serial_out;
 
+	ret = uart_read_port_properties(&uart.port, true);
+	if (ret)
+		return ret;
+
+	uart.port.iotype = UPIO_MEM32;
+	uart.port.regshift = 2;
+
 	uart.dma = &data->dma;
 	uart.dma->rxconf.src_maxburst = 1;
 	uart.dma->txconf.dst_maxburst = 1;
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_of.c | 105 +++++++-----------------------
 1 file changed, 22 insertions(+), 83 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 9dcc17e33269..1a699ce2e812 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -69,37 +69,22 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 	struct device *dev = &ofdev->dev;
 	struct device_node *np = dev->of_node;
 	struct uart_port *port = &up->port;
-	u32 clk, spd, prop;
-	int ret, irq;
+	u32 spd;
+	int ret;
 
 	memset(port, 0, sizeof *port);
 
 	pm_runtime_enable(&ofdev->dev);
 	pm_runtime_get_sync(&ofdev->dev);
 
-	if (of_property_read_u32(np, "clock-frequency", &clk)) {
-
-		/* Get clk rate through clk driver if present */
-		info->clk = devm_clk_get_enabled(dev, NULL);
-		if (IS_ERR(info->clk)) {
-			ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n");
-			goto err_pmruntime;
-		}
-
-		clk = clk_get_rate(info->clk);
-	}
-	/* If current-speed was set, then try not to change it. */
-	if (of_property_read_u32(np, "current-speed", &spd) == 0)
-		port->custom_divisor = clk / (16 * spd);
-
 	ret = of_address_to_resource(np, 0, &resource);
 	if (ret) {
 		dev_err_probe(dev, ret, "invalid address\n");
 		goto err_pmruntime;
 	}
 
-	port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT |
-				  UPF_FIXED_TYPE;
+	port->dev = &ofdev->dev;
+	port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
 	spin_lock_init(&port->lock);
 
 	if (resource_type(&resource) == IORESOURCE_IO) {
@@ -108,70 +93,31 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 	} else {
 		port->mapbase = resource.start;
 		port->mapsize = resource_size(&resource);
-
-		/* Check for shifted address mapping */
-		if (of_property_read_u32(np, "reg-offset", &prop) == 0) {
-			if (prop >= port->mapsize) {
-				ret = dev_err_probe(dev, -EINVAL, "reg-offset %u exceeds region size %pa\n",
-						    prop, &port->mapsize);
-				goto err_pmruntime;
-			}
-
-			port->mapbase += prop;
-			port->mapsize -= prop;
-		}
-
-		port->iotype = UPIO_MEM;
-		if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
-			switch (prop) {
-			case 1:
-				port->iotype = UPIO_MEM;
-				break;
-			case 2:
-				port->iotype = UPIO_MEM16;
-				break;
-			case 4:
-				port->iotype = of_device_is_big_endian(np) ?
-					       UPIO_MEM32BE : UPIO_MEM32;
-				break;
-			default:
-				ret = dev_err_probe(dev, -EINVAL, "unsupported reg-io-width (%u)\n",
-						    prop);
-				goto err_pmruntime;
-			}
-		}
 		port->flags |= UPF_IOREMAP;
 	}
 
+	ret = uart_read_port_properties(port, false);
+	if (ret)
+		goto err_pmruntime;
+
+	/* Get clk rate through clk driver if present */
+	if (!port->uartclk) {
+		info->clk = devm_clk_get_enabled(dev, NULL);
+		if (IS_ERR(info->clk)) {
+			ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n");
+			goto err_pmruntime;
+		}
+
+		port->uartclk = clk_get_rate(info->clk);
+	}
+	/* If current-speed was set, then try not to change it. */
+	if (of_property_read_u32(np, "current-speed", &spd) == 0)
+		port->custom_divisor = port->uartclk / (16 * spd);
+
 	/* Compatibility with the deprecated pxa driver and 8250_pxa drivers. */
 	if (of_device_is_compatible(np, "mrvl,mmp-uart"))
 		port->regshift = 2;
 
-	/* Check for registers offset within the devices address range */
-	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
-		port->regshift = prop;
-
-	/* Check for fifo size */
-	if (of_property_read_u32(np, "fifo-size", &prop) == 0)
-		port->fifosize = prop;
-
-	/* Check for a fixed line number */
-	ret = of_alias_get_id(np, "serial");
-	if (ret >= 0)
-		port->line = ret;
-
-	irq = of_irq_get(np, 0);
-	if (irq < 0) {
-		if (irq == -EPROBE_DEFER) {
-			ret = -EPROBE_DEFER;
-			goto err_pmruntime;
-		}
-		/* IRQ support not mandatory */
-		irq = 0;
-	}
-
-	port->irq = irq;
-
 	info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
 	if (IS_ERR(info->rst)) {
 		ret = PTR_ERR(info->rst);
@@ -183,12 +129,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 		goto err_pmruntime;
 
 	port->type = type;
-	port->uartclk = clk;
-
-	if (of_property_read_bool(np, "no-loopback-test"))
-		port->flags |= UPF_SKIP_TEST;
-
-	port->dev = &ofdev->dev;
 	port->rs485_config = serial8250_em485_config;
 	port->rs485_supported = serial8250_em485_supported;
 	up->rs485_start_tx = serial8250_em485_start_tx;
@@ -280,7 +220,6 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
 	platform_set_drvdata(ofdev, info);
 	return 0;
 err_dispose:
-	irq_dispose_mapping(port8250.port.irq);
 	pm_runtime_put_sync(&ofdev->dev);
 	pm_runtime_disable(&ofdev->dev);
 err_free:
-- 
2.43.0.rc1.1.gbec44491f096

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Andrew Jeffery]

On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> Since we have now a common helper to read port properties
> use it instead of sparse home grown solution.

I did some brief testing of the series for the Aspeed machines under
qemu, building them on top of v6.8-rc5:

export ARCH=arm
export CROSS_COMPILE=arm-linux-gnueabihf-
make aspeed_g5_defconfig
make -j$(nproc)
qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...

I got an oops during boot, which bisected to this change:

[    0.314946] 8<--- cut here ---
[    0.315051] Unable to handle kernel paging request at virtual address fee00000 when write
[    0.315201] [fee00000] *pgd=00000000
[    0.315593] Internal error: Oops: 805 [#1] SMP ARM
[    0.315847] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc5-00010-g8a2c8fe174cf #13
[    0.316071] Hardware name: Generic DT based system
[    0.316216] PC is at io_serial_out+0x18/0x20
[    0.316677] LR is at serial8250_do_set_mctrl+0x54/0x90
[    0.316781] pc : [<8060eea8>]    lr : [<806108b0>]    psr: 40000093
[    0.316891] sp : bf815b08  ip : 00000000  fp : 00000026
[    0.316987] r10: 81698240  r9 : 40000013  r8 : 81cae600
[    0.317087] r7 : 81d7d1a8  r6 : 81d7d110  r5 : 81008158  r4 : 00000000
[    0.317197] r3 : fee00000  r2 : 00000000  r1 : 00000004  r0 : 81008158
[    0.317350] Flags: nZcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    0.317471] Control: 10c5387d  Table: 8000406a  DAC: 00000051
[    0.317593] Register r0 information: non-slab/vmalloc memory
[    0.317892] Register r1 information: non-paged memory
[    0.317996] Register r2 information: NULL pointer
[    0.318080] Register r3 information: vmalloc memory
[    0.318176] Register r4 information: NULL pointer
[    0.318264] Register r5 information: non-slab/vmalloc memory
[    0.318362] Register r6 information: slab kmalloc-2k start 81d7d000 pointer offset 272 size 2048
[    0.318701] Register r7 information: slab kmalloc-2k start 81d7d000 pointer offset 424 size 2048
[    0.318860] Register r8 information: slab kmalloc-512 start 81cae600 pointer offset 0 size 512
[    0.319095] Register r9 information: non-paged memory
[    0.319194] Register r10 information: slab kmalloc-64 start 81698240 pointer offset 0 size 64
[    0.319266] Freeing initrd memory: 13684K
[    0.319384] Register r11 information: non-paged memory
[    0.319593] Register r12 information: NULL pointer
[    0.319703] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[    0.320006] Stack: (0xbf815b08 to 0xbf816000)
[    0.320157] 5b00:                   81008158 80f85a88 81d7d110 8060cb78 bf815b34 00000026
[    0.320313] 5b20: 0016e360 80cba110 81e65e80 80cfcdf4 00000003 204f2f49 00307830 00000000
[    0.320457] 5b40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.320600] 5b60: 00000000 00000000 00000000 00000000 00000000 ed1677db 81008158 81008158
[    0.320744] 5b80: bf815c00 81e5c5c0 81008304 81007f58 bf815d2c bf815dac 00000000 8060e1f4
[    0.320890] 5ba0: 80cba4ec 8081e2c4 bf815dfc 00000001 00000000 81cf5400 81cf5410 81e65e00
[    0.321030] 5bc0: 00000004 00000000 00000001 80616538 00000000 00000000 00000000 00000000
[    0.321176] 5be0: 1e784000 1e784fff bd7c1a94 00000200 00000000 00000000 00000000 00000000
[    0.321325] 5c00: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.321469] 5c20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.321624] 5c40: 00000000 00000000 8060fb34 00000000 00000000 00000000 00000026 00000000
[    0.321777] 5c60: 016e3600 00000000 00000200 00000000 00000000 00000000 00000000 00000000
[    0.321920] 5c80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.322063] 5ca0: 00000000 00000000 b9000040 00000000 00000000 00000000 00000000 00000000
[    0.322204] 5cc0: 00000004 00000000 00000000 00000004 00000000 1e784000 00001000 81cf5410
[    0.322347] 5ce0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.322492] 5d00: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000037
[    0.322640] 5d20: 00000001 00000001 00000000 00000000 00000000 00000000 00000000 00000000
[    0.322800] 5d40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.322957] 5d60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.323114] 5d80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.323271] 5da0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.323422] 5dc0: 00000000 00000000 806111c8 80610eb4 00000000 00000000 00000000 00000000
[    0.323573] 5de0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.323723] 5e00: 00000000 ed1677db 00000000 81cf5410 80f85cf8 00000000 00000000 81e5c638
[    0.323878] 5e20: 80e66f48 8067f888 81cf5410 00000000 80f85cf8 00000000 00000000 8067ca08
[    0.324029] 5e40: 81cf5410 00000000 81cf5410 81cf5410 80f85cf8 81cf5454 81cf5410 8067cda8
[    0.324181] 5e60: 60000013 81e5c638 81008d4c 81008d54 81cf5454 81cf5410 00000000 8067cf3c
[    0.324337] 5e80: 81cf5410 80f85cf8 81cf5454 814cec00 00000000 8067d21c 00000000 80f85cf8
[    0.324494] 5ea0: 8067d11c 8067aa04 814cec00 814cec58 816a4bb4 ed1677db 814cec00 81e5c600
[    0.324646] 5ec0: 00000000 80f85cf8 814cec00 8067bc6c 80cba524 00000000 00000006 80f85cf8
[    0.324795] 5ee0: 8158b480 00000006 bf815f14 00000000 80d19438 8067e284 80e221c4 8158b480
[    0.324945] 5f00: 00000006 80e01414 80d2d3b0 000000db 8173ad17 00000000 00000000 00000000
[    0.325096] 5f20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.325247] 5f40: 00000000 00000000 00000000 00000000 00000000 ed1677db 8173ad00 00000020
[    0.325403] 5f60: 00000006 80e3b83c 80e3b860 80e01750 00000006 00000006 00000000 80e004f8
[    0.325553] 5f80: 80f05cc0 80a50e18 00000000 00000000 00000000 00000000 00000000 80a50e34
[    0.325699] 5fa0: 00000000 80a50e18 00000000 8010016c 00000000 00000000 00000000 00000000
[    0.325848] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.325995] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[    0.326531]  io_serial_out from serial8250_do_set_mctrl+0x54/0x90
[    0.326761]  serial8250_do_set_mctrl from serial_core_register_port+0x4c4/0x694
[    0.326917]  serial_core_register_port from serial8250_register_8250_port+0x310/0x4bc
[    0.327063]  serial8250_register_8250_port from of_platform_serial_probe+0x300/0x45c
[    0.327242]  of_platform_serial_probe from platform_probe+0x60/0xb8
[    0.327367]  platform_probe from really_probe+0xd4/0x3e4
[    0.327471]  really_probe from __driver_probe_device+0x90/0x1ec
[    0.327568]  __driver_probe_device from driver_probe_device+0x38/0xd0
[    0.327674]  driver_probe_device from __driver_attach+0x100/0x1dc
[    0.327793]  __driver_attach from bus_for_each_dev+0x84/0xd4
[    0.327906]  bus_for_each_dev from bus_add_driver+0xec/0x1f0
[    0.328015]  bus_add_driver from driver_register+0x84/0x11c
[    0.328126]  driver_register from do_one_initcall+0x84/0x1c8
[    0.328297]  do_one_initcall from kernel_init_freeable+0x19c/0x22c
[    0.328419]  kernel_init_freeable from kernel_init+0x1c/0x138
[    0.328534]  kernel_init from ret_from_fork+0x14/0x28
[    0.328656] Exception stack(0xbf815fb0 to 0xbf815ff8)
[    0.328755] 5fa0:                                     00000000 00000000 00000000 00000000
[    0.328901] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.329112] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.329413] Code: e3a03000 ee073f9a e2433612 e6ef2072 (e5c32000)
[    0.329824] ---[ end trace 0000000000000000 ]---
[    0.336692] Kernel panic - not syncing: Fatal exception

Andrew

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Andy Shevchenko]

On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > Since we have now a common helper to read port properties
> > use it instead of sparse home grown solution.
> 
> I did some brief testing of the series for the Aspeed machines under
> qemu, building them on top of v6.8-rc5:
> 
> export ARCH=arm
> export CROSS_COMPILE=arm-linux-gnueabihf-
> make aspeed_g5_defconfig
> make -j$(nproc)
> qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
> 
> I got an oops during boot, which bisected to this change:

Thank you for prompt testing! I will look at it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Andy Shevchenko]

On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> > On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > > Since we have now a common helper to read port properties
> > > use it instead of sparse home grown solution.
> > 
> > I did some brief testing of the series for the Aspeed machines under
> > qemu, building them on top of v6.8-rc5:
> > 
> > export ARCH=arm
> > export CROSS_COMPILE=arm-linux-gnueabihf-
> > make aspeed_g5_defconfig
> > make -j$(nproc)
> > qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
> > 
> > I got an oops during boot, which bisected to this change:
> 
> Thank you for prompt testing! I will look at it.

I found the issue, will be fixed in next version.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Andy Shevchenko]

On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> > > On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > > > Since we have now a common helper to read port properties
> > > > use it instead of sparse home grown solution.
> > > 
> > > I did some brief testing of the series for the Aspeed machines under
> > > qemu, building them on top of v6.8-rc5:
> > > 
> > > export ARCH=arm
> > > export CROSS_COMPILE=arm-linux-gnueabihf-
> > > make aspeed_g5_defconfig
> > > make -j$(nproc)
> > > qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
> > > 
> > > I got an oops during boot, which bisected to this change:
> > 
> > Thank you for prompt testing! I will look at it.
> 
> I found the issue, will be fixed in next version.

Whoever is going to test this series, the

-		port->iotype = use_defaults ? UPIO_MEM : port->iotype;
+		port->iotype = UPIO_MEM;

should be applied to uart_read_port_properties() implementation.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Florian Fainelli]

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

On 2/22/24 08:47, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
>> On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
>>> On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
>>>> On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
>>>>> Since we have now a common helper to read port properties
>>>>> use it instead of sparse home grown solution.
>>>>
>>>> I did some brief testing of the series for the Aspeed machines under
>>>> qemu, building them on top of v6.8-rc5:
>>>>
>>>> export ARCH=arm
>>>> export CROSS_COMPILE=arm-linux-gnueabihf-
>>>> make aspeed_g5_defconfig
>>>> make -j$(nproc)
>>>> qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
>>>>
>>>> I got an oops during boot, which bisected to this change:
>>>
>>> Thank you for prompt testing! I will look at it.
>>
>> I found the issue, will be fixed in next version.
> 
> Whoever is going to test this series, the
> 
> -		port->iotype = use_defaults ? UPIO_MEM : port->iotype;
> +		port->iotype = UPIO_MEM;
> 
> should be applied to uart_read_port_properties() implementation.
> 

Thanks, on 8250_bcm7271.c with the above hunk applied, I did not spot 
any differences between the values returned by stty or a cat 
/sys/class/tty/ttyS0/* before or after, the console remained fully 
functional. I will see if I can run an additional test where I removed 
the DT's "clocks" property and confirm that the fall back to 
"clock-frequency" works.

Thanks Andy!
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Florian Fainelli]

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

On 2/22/24 09:39, Florian Fainelli wrote:
> On 2/22/24 08:47, Andy Shevchenko wrote:
>> On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
>>> On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
>>>> On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
>>>>> On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
>>>>>> Since we have now a common helper to read port properties
>>>>>> use it instead of sparse home grown solution.
>>>>>
>>>>> I did some brief testing of the series for the Aspeed machines under
>>>>> qemu, building them on top of v6.8-rc5:
>>>>>
>>>>> export ARCH=arm
>>>>> export CROSS_COMPILE=arm-linux-gnueabihf-
>>>>> make aspeed_g5_defconfig
>>>>> make -j$(nproc)
>>>>> qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel 
>>>>> arch/arm/boot/zImage -dtb 
>>>>> arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
>>>>>
>>>>> I got an oops during boot, which bisected to this change:
>>>>
>>>> Thank you for prompt testing! I will look at it.
>>>
>>> I found the issue, will be fixed in next version.
>>
>> Whoever is going to test this series, the
>>
>> -        port->iotype = use_defaults ? UPIO_MEM : port->iotype;
>> +        port->iotype = UPIO_MEM;
>>
>> should be applied to uart_read_port_properties() implementation.
>>
> 
> Thanks, on 8250_bcm7271.c with the above hunk applied, I did not spot 
> any differences between the values returned by stty or a cat 
> /sys/class/tty/ttyS0/* before or after, the console remained fully 
> functional. I will see if I can run an additional test where I removed 
> the DT's "clocks" property and confirm that the fall back to 
> "clock-frequency" works.
> 
> Thanks Andy!

Also appears to work properly on a Raspberry Pi 4 with the console using 
the bcm2835-aux driver, will provide Tested-by tags on the next 
submission, thanks!
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Andy Shevchenko]

On Thu, Feb 22, 2024 at 11:54:46AM -0800, Florian Fainelli wrote:
> On 2/22/24 09:39, Florian Fainelli wrote:
> > On 2/22/24 08:47, Andy Shevchenko wrote:

...

> > Thanks, on 8250_bcm7271.c with the above hunk applied, I did not spot
> > any differences between the values returned by stty or a cat
> > /sys/class/tty/ttyS0/* before or after, the console remained fully
> > functional. I will see if I can run an additional test where I removed
> > the DT's "clocks" property and confirm that the fall back to
> > "clock-frequency" works.
> 
> Also appears to work properly on a Raspberry Pi 4 with the console using the
> bcm2835-aux driver, will provide Tested-by tags on the next submission,
> thanks!

Thank you for prompt testing on real HW!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

[Andrew Jeffery]

On Thu, 2024-02-22 at 18:47 +0200, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
> > > On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> > > > On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > > > > Since we have now a common helper to read port properties
> > > > > use it instead of sparse home grown solution.
> > > > 
> > > > I did some brief testing of the series for the Aspeed machines under
> > > > qemu, building them on top of v6.8-rc5:
> > > > 
> > > > export ARCH=arm
> > > > export CROSS_COMPILE=arm-linux-gnueabihf-
> > > > make aspeed_g5_defconfig
> > > > make -j$(nproc)
> > > > qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
> > > > 
> > > > I got an oops during boot, which bisected to this change:
> > > 
> > > Thank you for prompt testing! I will look at it.
> > 
> > I found the issue, will be fixed in next version.
> 
> Whoever is going to test this series, the
> 
> -		port->iotype = use_defaults ? UPIO_MEM : port->iotype;
> +		port->iotype = UPIO_MEM;
> 
> should be applied to uart_read_port_properties() implementation.
> 

Thanks, with that fix applied it works fine for me also.

Andrew

[PATCH v1 11/14] serial: 8250_omap: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_omap.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 6942990a333c..173af575a43e 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1394,11 +1394,7 @@ static int omap8250_probe(struct platform_device *pdev)
 	struct uart_8250_port up;
 	struct resource *regs;
 	void __iomem *membase;
-	int irq, ret;
-
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	int ret;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!regs) {
@@ -1419,7 +1415,6 @@ static int omap8250_probe(struct platform_device *pdev)
 	up.port.dev = &pdev->dev;
 	up.port.mapbase = regs->start;
 	up.port.membase = membase;
-	up.port.irq = irq;
 	/*
 	 * It claims to be 16C750 compatible however it is a little different.
 	 * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
@@ -1429,13 +1424,9 @@ static int omap8250_probe(struct platform_device *pdev)
 	 * or pm callback.
 	 */
 	up.port.type = PORT_8250;
-	up.port.iotype = UPIO_MEM;
-	up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
-		UPF_HARD_FLOW;
+	up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW | UPF_HARD_FLOW;
 	up.port.private_data = priv;
 
-	up.port.regshift = OMAP_UART_REGSHIFT;
-	up.port.fifosize = 64;
 	up.tx_loadsz = 64;
 	up.capabilities = UART_CAP_FIFO;
 #ifdef CONFIG_PM
@@ -1461,14 +1452,14 @@ static int omap8250_probe(struct platform_device *pdev)
 	up.rs485_stop_tx = serial8250_em485_stop_tx;
 	up.port.has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
 
-	ret = of_alias_get_id(np, "serial");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias\n");
+	ret = uart_read_port_properties(&up.port, true);
+	if (ret)
 		return ret;
-	}
-	up.port.line = ret;
 
-	if (of_property_read_u32(np, "clock-frequency", &up.port.uartclk)) {
+	up.port.regshift = OMAP_UART_REGSHIFT;
+	up.port.fifosize = 64;
+
+	if (!up.port.uartclk) {
 		struct clk *clk;
 
 		clk = devm_clk_get(&pdev->dev, NULL);
@@ -1560,8 +1551,8 @@ static int omap8250_probe(struct platform_device *pdev)
 	}
 #endif
 
-	irq_set_status_flags(irq, IRQ_NOAUTOEN);
-	ret = devm_request_irq(&pdev->dev, irq, omap8250_irq, 0,
+	irq_set_status_flags(up.port.irq, IRQ_NOAUTOEN);
+	ret = devm_request_irq(&pdev->dev, up.port.irq, omap8250_irq, 0,
 			       dev_name(&pdev->dev), priv);
 	if (ret < 0)
 		return ret;
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 12/14] serial: 8250_pxa: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_pxa.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pxa.c b/drivers/tty/serial/8250/8250_pxa.c
index 77686da42ce8..0c9140b93414 100644
--- a/drivers/tty/serial/8250/8250_pxa.c
+++ b/drivers/tty/serial/8250/8250_pxa.c
@@ -92,11 +92,7 @@ static int serial_pxa_probe(struct platform_device *pdev)
 	struct uart_8250_port uart = {};
 	struct pxa8250_data *data;
 	struct resource *mmres;
-	int irq, ret;
-
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	int ret;
 
 	mmres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mmres)
@@ -114,21 +110,21 @@ static int serial_pxa_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (ret >= 0)
-		uart.port.line = ret;
-
 	uart.port.type = PORT_XSCALE;
-	uart.port.iotype = UPIO_MEM32;
 	uart.port.mapbase = mmres->start;
-	uart.port.regshift = 2;
-	uart.port.irq = irq;
-	uart.port.fifosize = 64;
 	uart.port.flags = UPF_IOREMAP | UPF_SKIP_TEST | UPF_FIXED_TYPE;
 	uart.port.dev = &pdev->dev;
 	uart.port.uartclk = clk_get_rate(data->clk);
 	uart.port.pm = serial_pxa_pm;
 	uart.port.private_data = data;
+
+	ret = uart_read_port_properties(&uart.port, true);
+	if (ret)
+		return ret;
+
+	uart.port.iotype = UPIO_MEM32;
+	uart.port.regshift = 2;
+	uart.port.fifosize = 64;
 	uart.dl_write = serial_pxa_dl_write;
 
 	ret = serial8250_register_8250_port(&uart);
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 13/14] serial: 8250_tegra: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_tegra.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_tegra.c b/drivers/tty/serial/8250/8250_tegra.c
index ba352262df75..ce48d02dfa0d 100644
--- a/drivers/tty/serial/8250/8250_tegra.c
+++ b/drivers/tty/serial/8250/8250_tegra.c
@@ -57,25 +57,11 @@ static int tegra_uart_probe(struct platform_device *pdev)
 	port = &port8250.port;
 	spin_lock_init(&port->lock);
 
-	port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT |
-		      UPF_FIXED_TYPE;
-	port->iotype = UPIO_MEM32;
-	port->regshift = 2;
+	port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
 	port->type = PORT_TEGRA;
-	port->irqflags |= IRQF_SHARED;
 	port->dev = &pdev->dev;
 	port->handle_break = tegra_uart_handle_break;
 
-	ret = of_alias_get_id(pdev->dev.of_node, "serial");
-	if (ret >= 0)
-		port->line = ret;
-
-	ret = platform_get_irq(pdev, 0);
-	if (ret < 0)
-		return ret;
-
-	port->irq = ret;
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
 		return -ENODEV;
@@ -88,12 +74,18 @@ static int tegra_uart_probe(struct platform_device *pdev)
 	port->mapbase = res->start;
 	port->mapsize = resource_size(res);
 
+	ret = uart_read_port_properties(port, true);
+	if (ret)
+		return ret;
+
+	port->iotype = UPIO_MEM32;
+	port->regshift = 2;
+
 	uart->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
 	if (IS_ERR(uart->rst))
 		return PTR_ERR(uart->rst);
 
-	if (device_property_read_u32(&pdev->dev, "clock-frequency",
-				     &port->uartclk)) {
+	if (!port->uartclk) {
 		uart->clk = devm_clk_get(&pdev->dev, NULL);
 		if (IS_ERR(uart->clk)) {
 			dev_err(&pdev->dev, "failed to get clock!\n");
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v1 14/14] serial: 8250_uniphier: Switch to use uart_read_port_properties()

[Andy Shevchenko]

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_uniphier.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
index 6399a38ecce2..d3f270a191ee 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -162,7 +162,6 @@ static int uniphier_uart_probe(struct platform_device *pdev)
 	struct uniphier8250_priv *priv;
 	struct resource *regs;
 	void __iomem *membase;
-	int irq;
 	int ret;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -175,23 +174,12 @@ static int uniphier_uart_probe(struct platform_device *pdev)
 	if (!membase)
 		return -ENOMEM;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	memset(&up, 0, sizeof(up));
 
-	ret = of_alias_get_id(dev->of_node, "serial");
-	if (ret < 0) {
-		dev_err(dev, "failed to get alias id\n");
-		return ret;
-	}
-	up.port.line = ret;
-
 	priv->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		dev_err(dev, "failed to get clock\n");
@@ -211,7 +199,10 @@ static int uniphier_uart_probe(struct platform_device *pdev)
 	up.port.mapbase = regs->start;
 	up.port.mapsize = resource_size(regs);
 	up.port.membase = membase;
-	up.port.irq = irq;
+
+	ret = uart_read_port_properties(&up.port, true);
+	if (ret)
+		return ret;
 
 	up.port.type = PORT_16550A;
 	up.port.iotype = UPIO_MEM32;
-- 
2.43.0.rc1.1.gbec44491f096