1. Patchew
  2. linux
  3. [PATCH v2 0/7] serial: exar: add Connect Tech serial cards to Exar driver
  4. View patch

[PATCH v2 4/7] serial: exar: add optional board_setup function

parker@finest.io posted 7 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH v2 4/7] serial: exar: add optional board_setup function
Posted by parker@finest.io 1 year, 10 months ago 
From: Parker Newman <pnewman@connecttech.com>

Adds an optional "board_setup" function pointer to struct
exar8250_board. This gets called once during probe prior to setting up
the ports.

Signed-off-by: Parker Newman <pnewman@connecttech.com>
---
 drivers/tty/serial/8250/8250_exar.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 9915a99cb7c6..b30f3855652a 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -143,7 +143,7 @@
  *
  * MPIO		Port	Function
  * ----		----	--------
- * 0		2 	Mode bit 0
+ * 0		2	Mode bit 0
  * 1		2	Mode bit 1
  * 2		2	Terminate bus
  * 3		-	<reserved>
@@ -179,22 +179,24 @@ struct exar8250_platform {
 	int (*rs485_config)(struct uart_port *port, struct ktermios *termios,
 			    struct serial_rs485 *rs485);
 	const struct serial_rs485 *rs485_supported;
-	int (*register_gpio)(struct pci_dev *, struct uart_8250_port *);
-	void (*unregister_gpio)(struct uart_8250_port *);
+	int (*register_gpio)(struct pci_dev *pcidev, struct uart_8250_port *port);
+	void (*unregister_gpio)(struct uart_8250_port *port);
 };

 /**
  * struct exar8250_board - board information
  * @num_ports: number of serial ports
  * @reg_shift: describes UART register mapping in PCI memory
- * @setup: quirk run at ->probe() stage
+ * @board_setup: quirk run once at ->probe() stage before setting up ports
+ * @setup: quirk run at ->probe() stage for each port
  * @exit: quirk run at ->remove() stage
  */
 struct exar8250_board {
 	unsigned int num_ports;
 	unsigned int reg_shift;
-	int	(*setup)(struct exar8250 *, struct pci_dev *,
-			 struct uart_8250_port *, int);
+	int     (*board_setup)(struct exar8250 *priv);
+	int	(*setup)(struct exar8250 *priv, struct pci_dev *pcidev,
+			 struct uart_8250_port *port, int idx);
 	void	(*exit)(struct pci_dev *pcidev);
 };

@@ -966,6 +968,15 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
 	if (rc)
 		return rc;

+	if (board->board_setup) {
+		rc = board->board_setup(priv);
+		if (rc) {
+			pci_err(pcidev,
+				"failed to setup serial board: %d\n", rc);
+			return rc;
+		}
+	}
+
 	for (i = 0; i < nr_ports && i < maxnr; i++) {
 		rc = board->setup(priv, pcidev, &uart, i);
 		if (rc) {
--
2.43.2
Re: [PATCH v2 4/7] serial: exar: add optional board_setup function
Posted by Ilpo Järvinen 1 year, 10 months ago 
On Thu, 11 Apr 2024, parker@finest.io wrote:

> From: Parker Newman <pnewman@connecttech.com>
> 
> Adds an optional "board_setup" function pointer to struct
> exar8250_board. This gets called once during probe prior to setting up
> the ports.
> 
> Signed-off-by: Parker Newman <pnewman@connecttech.com>
> ---
>  drivers/tty/serial/8250/8250_exar.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index 9915a99cb7c6..b30f3855652a 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -143,7 +143,7 @@
>   *
>   * MPIO		Port	Function
>   * ----		----	--------
> - * 0		2 	Mode bit 0
> + * 0		2	Mode bit 0

Unrelated change.

>   * 1		2	Mode bit 1
>   * 2		2	Terminate bus
>   * 3		-	<reserved>
> @@ -179,22 +179,24 @@ struct exar8250_platform {
>  	int (*rs485_config)(struct uart_port *port, struct ktermios *termios,
>  			    struct serial_rs485 *rs485);
>  	const struct serial_rs485 *rs485_supported;
> -	int (*register_gpio)(struct pci_dev *, struct uart_8250_port *);
> -	void (*unregister_gpio)(struct uart_8250_port *);
> +	int (*register_gpio)(struct pci_dev *pcidev, struct uart_8250_port *port);
> +	void (*unregister_gpio)(struct uart_8250_port *port);
>  };
> 
>  /**
>   * struct exar8250_board - board information
>   * @num_ports: number of serial ports
>   * @reg_shift: describes UART register mapping in PCI memory
> - * @setup: quirk run at ->probe() stage
> + * @board_setup: quirk run once at ->probe() stage before setting up ports
> + * @setup: quirk run at ->probe() stage for each port
>   * @exit: quirk run at ->remove() stage
>   */
>  struct exar8250_board {
>  	unsigned int num_ports;
>  	unsigned int reg_shift;
> -	int	(*setup)(struct exar8250 *, struct pci_dev *,
> -			 struct uart_8250_port *, int);
> +	int     (*board_setup)(struct exar8250 *priv);
> +	int	(*setup)(struct exar8250 *priv, struct pci_dev *pcidev,
> +			 struct uart_8250_port *port, int idx);
>  	void	(*exit)(struct pci_dev *pcidev);
>  };
> 
> @@ -966,6 +968,15 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
>  	if (rc)
>  		return rc;
> 
> +	if (board->board_setup) {
> +		rc = board->board_setup(priv);

Could this be called board_init() as having both ->board_setup() and 
->setup() is bit confusing.

> +		if (rc) {
> +			pci_err(pcidev,
> +				"failed to setup serial board: %d\n", rc);

pci_err() belongs to pci subsystem. Please use dev_err() or return 
dev_err_probe().

> +			return rc;
> +		}
> +	}
> +
>  	for (i = 0; i < nr_ports && i < maxnr; i++) {
>  		rc = board->setup(priv, pcidev, &uart, i);
>  		if (rc) {


-- 
 i.
Re: [PATCH v2 4/7] serial: exar: add optional board_setup function
Posted by Parker Newman 1 year, 10 months ago 
On Fri, 12 Apr 2024 13:08:58 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Thu, 11 Apr 2024, parker@finest.io wrote:
> 
> > From: Parker Newman <pnewman@connecttech.com>
> > 
> > Adds an optional "board_setup" function pointer to struct
> > exar8250_board. This gets called once during probe prior to setting up
> > the ports.
> > 
> > Signed-off-by: Parker Newman <pnewman@connecttech.com>
> > ---
> >  drivers/tty/serial/8250/8250_exar.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> > index 9915a99cb7c6..b30f3855652a 100644
> > --- a/drivers/tty/serial/8250/8250_exar.c
> > +++ b/drivers/tty/serial/8250/8250_exar.c
> > @@ -143,7 +143,7 @@
> >   *
> >   * MPIO		Port	Function
> >   * ----		----	--------
> > - * 0		2 	Mode bit 0
> > + * 0		2	Mode bit 0  
> 
> Unrelated change.
>

Sorry, should have added a note about fixing this and the missing argument names
below due to checkpatch.pl. I will add a note in v3. 
 
> >   * 1		2	Mode bit 1
> >   * 2		2	Terminate bus
> >   * 3		-	<reserved>
> > @@ -179,22 +179,24 @@ struct exar8250_platform {
> >  	int (*rs485_config)(struct uart_port *port, struct ktermios *termios,
> >  			    struct serial_rs485 *rs485);
> >  	const struct serial_rs485 *rs485_supported;
> > -	int (*register_gpio)(struct pci_dev *, struct uart_8250_port *);
> > -	void (*unregister_gpio)(struct uart_8250_port *);
> > +	int (*register_gpio)(struct pci_dev *pcidev, struct uart_8250_port *port);
> > +	void (*unregister_gpio)(struct uart_8250_port *port);
> >  };
> > 
> >  /**
> >   * struct exar8250_board - board information
> >   * @num_ports: number of serial ports
> >   * @reg_shift: describes UART register mapping in PCI memory
> > - * @setup: quirk run at ->probe() stage
> > + * @board_setup: quirk run once at ->probe() stage before setting up ports
> > + * @setup: quirk run at ->probe() stage for each port
> >   * @exit: quirk run at ->remove() stage
> >   */
> >  struct exar8250_board {
> >  	unsigned int num_ports;
> >  	unsigned int reg_shift;
> > -	int	(*setup)(struct exar8250 *, struct pci_dev *,
> > -			 struct uart_8250_port *, int);
> > +	int     (*board_setup)(struct exar8250 *priv);
> > +	int	(*setup)(struct exar8250 *priv, struct pci_dev *pcidev,
> > +			 struct uart_8250_port *port, int idx);
> >  	void	(*exit)(struct pci_dev *pcidev);
> >  };
> > 
> > @@ -966,6 +968,15 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
> >  	if (rc)
> >  		return rc;
> > 
> > +	if (board->board_setup) {
> > +		rc = board->board_setup(priv);  
> 
> Could this be called board_init() as having both ->board_setup() and 
> ->setup() is bit confusing.  

sure, good idea.

> > +		if (rc) {
> > +			pci_err(pcidev,
> > +				"failed to setup serial board: %d\n", rc);  
> 
> pci_err() belongs to pci subsystem. Please use dev_err() or return 
> dev_err_probe().

Did not know it was not allowed outside the PCI subsystem. I will switch 
to dev_err() etc. 

> > +			return rc;
> > +		}
> > +	}
> > +
> >  	for (i = 0; i < nr_ports && i < maxnr; i++) {
> >  		rc = board->setup(priv, pcidev, &uart, i);
> >  		if (rc) {  
> 
> 

Thanks,
-Parker

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.