From: Stefan Sperling <stsp@stsp.name>
Subject: Re: qwx: remove MHI LOOPBACK channels and move PCI ops registration ahead
To: Kevin Lo <kevlo@kevlo.org>
Cc: tech@openbsd.org
Date: Fri, 4 Oct 2024 09:04:28 +0200

On Fri, Oct 04, 2024 at 02:38:53PM +0800, Kevin Lo wrote:
> Hi,
> 
> This diff removes two MHI LOOPBACK channels, there's no need to match them [1].
> Also move PCI ops registration ahead [2].  That way the ops will be registered
> if any subsequent operations within the case processing require the ops to be
> present.
> 
> Tested:
> qwx0 at pci2 dev 0 function 0 "Qualcomm QCNFA765" rev 0x01: msi
> qwx0: wcn6855 hw2.1 fw 0x11088c35 address 38:d5:7a:xx:xx:xx
> 
> [1] Linux commit fbb2a14afe00a5691b43cd19c946472e59e16fc6
> [2] Linux commit 515bcdf587f9911f2d5de51524cb7e048d295052
> 
> ok?

ok stsp@

> Index: sys/dev/pci/if_qwx_pci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_qwx_pci.c,v
> diff -u -p -u -p -r1.22 if_qwx_pci.c
> --- sys/dev/pci/if_qwx_pci.c	6 Jul 2024 05:34:35 -0000	1.22
> +++ sys/dev/pci/if_qwx_pci.c	4 Oct 2024 05:58:54 -0000
> @@ -374,11 +374,9 @@ struct qwx_pci_softc {
>  	struct qwx_dmamem	*cmd_ctxt;
> 
> 
> -	struct qwx_pci_xfer_ring xfer_rings[4];
> -#define QWX_PCI_XFER_RING_LOOPBACK_OUTBOUND	0
> -#define QWX_PCI_XFER_RING_LOOPBACK_INBOUND	1
> -#define QWX_PCI_XFER_RING_IPCR_OUTBOUND		2
> -#define QWX_PCI_XFER_RING_IPCR_INBOUND		3
> +	struct qwx_pci_xfer_ring xfer_rings[2];
> +#define QWX_PCI_XFER_RING_IPCR_OUTBOUND		0
> +#define QWX_PCI_XFER_RING_IPCR_INBOUND		1
>  	struct qwx_pci_event_ring event_rings[QWX_NUM_EVENT_CTX];
>  	struct qwx_pci_cmd_ring cmd_ring;
>  };
> @@ -745,7 +743,6 @@ qwx_pci_attach(struct device *parent, st
>  	struct ieee80211com *ic = &sc->sc_ic;
>  	struct ifnet *ifp = &ic->ic_if;
>  	uint32_t soc_hw_version_major, soc_hw_version_minor;
> -	const struct qwx_pci_ops *pci_ops;
>  	struct pci_attach_args *pa = aux;
>  	pci_intr_handle_t ih;
>  	pcireg_t memtype, reg;
> @@ -884,6 +881,9 @@ qwx_pci_attach(struct device *parent, st
> 
>  	pci_set_powerstate(pa->pa_pc, pa->pa_tag, PCI_PMCSR_STATE_D0);
> 
> +	/* register PCI ops */
> +	psc->sc_pci_ops = &qwx_pci_ops_qca6390;
> +
>  	switch (PCI_PRODUCT(pa->pa_id)) {
>  	case PCI_PRODUCT_QUALCOMM_QCA6390:
>  		qwx_pci_read_hw_version(sc, &soc_hw_version_major,
> @@ -898,11 +898,10 @@ qwx_pci_attach(struct device *parent, st
>  			return;
>  		}
> 
> -		pci_ops = &qwx_pci_ops_qca6390;
>  		psc->max_chan = QWX_MHI_CONFIG_QCA6390_MAX_CHANNELS;
>  		break;
>  	case PCI_PRODUCT_QUALCOMM_QCN9074:
> -		pci_ops = &qwx_pci_ops_qcn9074;
> +		psc->sc_pci_ops = &qwx_pci_ops_qcn9074;
>  		sc->sc_hw_rev = ATH11K_HW_QCN9074_HW10;
>  		psc->max_chan = QWX_MHI_CONFIG_QCA9074_MAX_CHANNELS;
>  		break;
> @@ -932,7 +931,6 @@ unsupported_wcn6855_soc:
>  			return;
>  		}
> 
> -		pci_ops = &qwx_pci_ops_qca6390;
>  		psc->max_chan = QWX_MHI_CONFIG_QCA6390_MAX_CHANNELS;
>  		break;
>  	default:
> @@ -940,9 +938,6 @@ unsupported_wcn6855_soc:
>  		return;
>  	}
> 
> -	/* register PCI ops */
> -	psc->sc_pci_ops = pci_ops;
> -
>  	error = qwx_pcic_init_msi_config(sc);
>  	if (error)
>  		goto err_pci_free_region;
> @@ -1297,18 +1292,6 @@ qwx_pci_alloc_xfer_rings_qca6390(struct
>  	int ret;
> 
>  	ret = qwx_pci_alloc_xfer_ring(sc,
> -	    &psc->xfer_rings[QWX_PCI_XFER_RING_LOOPBACK_OUTBOUND],
> -	    0, MHI_CHAN_TYPE_OUTBOUND, 0, 32);
> -	if (ret)
> -		goto fail;
> -
> -	ret = qwx_pci_alloc_xfer_ring(sc,
> -	    &psc->xfer_rings[QWX_PCI_XFER_RING_LOOPBACK_INBOUND],
> -	    1, MHI_CHAN_TYPE_INBOUND, 0, 32);
> -	if (ret)
> -		goto fail;
> -
> -	ret = qwx_pci_alloc_xfer_ring(sc,
>  	    &psc->xfer_rings[QWX_PCI_XFER_RING_IPCR_OUTBOUND],
>  	    20, MHI_CHAN_TYPE_OUTBOUND, 1, 64);
>  	if (ret)
> @@ -1331,18 +1314,6 @@ qwx_pci_alloc_xfer_rings_qcn9074(struct
>  {
>  	struct qwx_softc *sc = &psc->sc_sc;
>  	int ret;
> -
> -	ret = qwx_pci_alloc_xfer_ring(sc,
> -	    &psc->xfer_rings[QWX_PCI_XFER_RING_LOOPBACK_OUTBOUND],
> -	    0, MHI_CHAN_TYPE_OUTBOUND, 1, 32);
> -	if (ret)
> -		goto fail;
> -
> -	ret = qwx_pci_alloc_xfer_ring(sc,
> -	    &psc->xfer_rings[QWX_PCI_XFER_RING_LOOPBACK_INBOUND],
> -	    1, MHI_CHAN_TYPE_INBOUND, 1, 32);
> -	if (ret)
> -		goto fail;
> 
>  	ret = qwx_pci_alloc_xfer_ring(sc,
>  	    &psc->xfer_rings[QWX_PCI_XFER_RING_IPCR_OUTBOUND],
> 
>