[PATCH 0/2] serial: 8250_men_mcb: Parameter for base clock rate

[PATCH 0/2] serial: 8250_men_mcb: Parameter for base clock rate

[Filip Jensen]

Until now, the base clock rate was fixed for all the UARTS depending on the
card they were attached to. However this base clock rate of men devices is
variable and can be set by user. For this end, a parameter is here added to
allow changing this rate for up to 64 UARTs. Also cleaning unused defines in
another commit.

Filip Jensen (2):
  serial: 8250_men_mcb: Clean defines
  serial: 8250_men_mcb: Parameter for base clockrate

 drivers/tty/serial/8250/8250_men_mcb.c | 28 ++++++++++++++++----------
 1 file changed, 17 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH 1/2] serial: 8250_men_mcb: Clean defines

[Filip Jensen]

The Z125 define is not used anywhere, thus removed. Also used a
more specific name for the maximum number of ports per unit and
removed the duplicated define MAX_PORTS

Reviewed-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
Signed-off-by: Filip Jensen <dev-Felipe.Jensen@duagon.com>
---
 drivers/tty/serial/8250/8250_men_mcb.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_men_mcb.c b/drivers/tty/serial/8250/8250_men_mcb.c
index a78ef35c8187..6ba7464bb2dd 100644
--- a/drivers/tty/serial/8250/8250_men_mcb.c
+++ b/drivers/tty/serial/8250/8250_men_mcb.c
@@ -28,8 +28,6 @@
 #define MEN_UART3_MASK	0x04
 #define MEN_UART4_MASK	0x08
 
-#define MEN_Z125_UARTS_AVAILABLE	0x01
-
 #define MEN_Z025_MAX_UARTS		4
 #define MEN_UART_MEM_SIZE		0x10
 #define MEM_UART_REGISTER_SIZE		0x01
@@ -42,12 +40,10 @@
 
 #define MEN_READ_REGISTER(addr)	readb(addr)
 
-#define MAX_PORTS	4
-
 struct serial_8250_men_mcb_data {
 	int num_ports;
-	int line[MAX_PORTS];
-	unsigned int offset[MAX_PORTS];
+	int line[MEN_Z025_MAX_UARTS];
+	unsigned int offset[MEN_Z025_MAX_UARTS];
 };
 
 /*
@@ -126,7 +122,7 @@ static int read_serial_data(struct mcb_device *mdev,
 	if (res < 0)
 		return res;
 
-	for (i = 0; i < MAX_PORTS; i++) {
+	for (i = 0; i < MEN_Z025_MAX_UARTS; i++) {
 		mask = 0x1 << i;
 		switch (uarts_available & mask) {
 		case MEN_UART1_MASK:
@@ -150,7 +146,7 @@ static int read_serial_data(struct mcb_device *mdev,
 		}
 	}
 
-	if (count <= 0 || count > MAX_PORTS) {
+	if (count <= 0 || count > MEN_Z025_MAX_UARTS) {
 		dev_err(&mdev->dev, "unexpected number of ports: %u\n",
 			count);
 		return -ENODEV;
-- 
2.34.1

[PATCH 2/2] serial: 8250_men_mcb: Parameter for base clockrate

[Filip Jensen]

A parameter is needed for allowing to change the base clockrate for
each uart port, as this fpga units have no fixed base clock. If the
parameter is not set or is set to 0, it will use the default value.
Allows setting the clockrate up to 64 UARTs.

Reviewed-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
Signed-off-by: Filip Jensen <dev-Felipe.Jensen@duagon.com>
---
 drivers/tty/serial/8250/8250_men_mcb.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_men_mcb.c b/drivers/tty/serial/8250/8250_men_mcb.c
index 6ba7464bb2dd..01dcb9c17b0c 100644
--- a/drivers/tty/serial/8250/8250_men_mcb.c
+++ b/drivers/tty/serial/8250/8250_men_mcb.c
@@ -28,6 +28,7 @@
 #define MEN_UART3_MASK	0x04
 #define MEN_UART4_MASK	0x08
 
+#define MEN_8250_MAX_SETUP		64
 #define MEN_Z025_MAX_UARTS		4
 #define MEN_UART_MEM_SIZE		0x10
 #define MEM_UART_REGISTER_SIZE		0x01
@@ -45,12 +46,18 @@ struct serial_8250_men_mcb_data {
 	int line[MEN_Z025_MAX_UARTS];
 	unsigned int offset[MEN_Z025_MAX_UARTS];
 };
+static int portindex;
+static ulong uartclk[MEN_8250_MAX_SETUP];
+module_param_array(uartclk, ulong, NULL, 0664);
+MODULE_PARM_DESC(
+	uartclk,
+	"Base for baudrate generation for each port. E.g.: uartclk=1843200,1843200,1041666,1041666. If set to 0 it will use board's default");
 
 /*
  * The Z125 16550-compatible UART has no fixed base clock assigned
  * So, depending on the board we're on, we need to adjust the
- * parameter in order to really set the correct baudrate, and
- * do so if possible without user interaction
+ * parameter in order to really set the correct baudrate. Can be
+ * overridden with uartclk parameter.
  */
 static u32 men_lookup_uartclk(struct mcb_device *mdev)
 {
@@ -60,7 +67,9 @@ static u32 men_lookup_uartclk(struct mcb_device *mdev)
 	dev_info(&mdev->dev, "%s on board %s\n",
 		dev_name(&mdev->dev),
 		mdev->bus->name);
-	if  (strncmp(mdev->bus->name, "F075", 4) == 0)
+	if (portindex < MEN_8250_MAX_SETUP && uartclk[portindex])
+		clkval = uartclk[portindex];
+	else if (strncmp(mdev->bus->name, "F075", 4) == 0)
 		clkval = 1041666;
 	else if (strncmp(mdev->bus->name, "F216", 4) == 0)
 		clkval = 1843200;
@@ -72,6 +81,7 @@ static u32 men_lookup_uartclk(struct mcb_device *mdev)
 		dev_info(&mdev->dev,
 			 "board not detected, using default uartclk\n");
 
+	++portindex;
 	clkval = clkval  << 4;
 
 	return clkval;
-- 
2.34.1

Re: [PATCH 2/2] serial: 8250_men_mcb: Parameter for base clockrate

[Greg KH]

On Wed, Dec 10, 2025 at 05:48:04PM +0100, Filip Jensen wrote:
> A parameter is needed for allowing to change the base clockrate for
> each uart port, as this fpga units have no fixed base clock. If the
> parameter is not set or is set to 0, it will use the default value.
> Allows setting the clockrate up to 64 UARTs.
> 
> Reviewed-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
> Signed-off-by: Filip Jensen <dev-Felipe.Jensen@duagon.com>
> ---
>  drivers/tty/serial/8250/8250_men_mcb.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_men_mcb.c b/drivers/tty/serial/8250/8250_men_mcb.c
> index 6ba7464bb2dd..01dcb9c17b0c 100644
> --- a/drivers/tty/serial/8250/8250_men_mcb.c
> +++ b/drivers/tty/serial/8250/8250_men_mcb.c
> @@ -28,6 +28,7 @@
>  #define MEN_UART3_MASK	0x04
>  #define MEN_UART4_MASK	0x08
>  
> +#define MEN_8250_MAX_SETUP		64
>  #define MEN_Z025_MAX_UARTS		4
>  #define MEN_UART_MEM_SIZE		0x10
>  #define MEM_UART_REGISTER_SIZE		0x01
> @@ -45,12 +46,18 @@ struct serial_8250_men_mcb_data {
>  	int line[MEN_Z025_MAX_UARTS];
>  	unsigned int offset[MEN_Z025_MAX_UARTS];
>  };
> +static int portindex;
> +static ulong uartclk[MEN_8250_MAX_SETUP];
> +module_param_array(uartclk, ulong, NULL, 0664);
> +MODULE_PARM_DESC(
> +	uartclk,
> +	"Base for baudrate generation for each port. E.g.: uartclk=1843200,1843200,1041666,1041666. If set to 0 it will use board's default");

This is not the 1990's, please do nto add new module parameters.  That
way lies madness and an inability to handle dynamic devices in any way.

Why can't this be found out some other way?  Device discovery?  Device
tree?  quirk for something else?  This should not be so broken that you
need to manually come up with kernel boot lines like this.  What about
fixing the hardware itself to not need this at all?

thanks,

greg k-h