From: stsp <stsp@openbsd.org>
Date: Thu, 22 Feb 2024 09:15:34 +0000 (+0000)
Subject: get rid of the qwx(4) mhi_newstate_task
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a5176bb5917058208be9291d3697f84006a69be4;p=openbsd

get rid of the qwx(4) mhi_newstate_task

We can now run MHI state transition code in the interrupt handler.
There is no reason to run a separate thread just to poke at some
registers and send a few of wakeups().
---

diff --git a/sys/dev/pci/if_qwx_pci.c b/sys/dev/pci/if_qwx_pci.c
index ef84e060238..46c20d84063 100644
--- a/sys/dev/pci/if_qwx_pci.c
+++ b/sys/dev/pci/if_qwx_pci.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_qwx_pci.c,v 1.13 2024/02/22 09:12:45 stsp Exp $	*/
+/*	$OpenBSD: if_qwx_pci.c,v 1.14 2024/02/22 09:15:34 stsp Exp $	*/
 
 /*
  * Copyright 2023 Stefan Sperling <stsp@openbsd.org>
@@ -323,16 +323,6 @@ struct qwx_pci_cmd_ring {
 struct qwx_pci_ops;
 struct qwx_msi_config;
 
-struct qwx_mhi_newstate {
-	struct {
-		int mhi_state;
-		int ee;
-	} queue[4];
-	int cur;
-	int tail;
-	int queued;
-};
-
 #define QWX_NUM_MSI_VEC	32
 
 struct qwx_pci_softc {
@@ -366,10 +356,6 @@ struct qwx_pci_softc {
 
 	uint64_t		 wake_db;
 
-	struct qwx_mhi_newstate	 mhi_newstate;
-	struct task		 mhi_newstate_task;
-	struct taskq		*mhi_taskq;
-
 	/*
 	 * DMA memory for AMSS.bin firmware image.
 	 * This memory must remain available to the device until
@@ -471,8 +457,6 @@ int	qwx_mhi_fw_load_bhie(struct qwx_pci_softc *, uint8_t *, size_t);
 void	qwx_rddm_prepare(struct qwx_pci_softc *);
 void	qwx_rddm_task(void *);
 void *	qwx_pci_event_ring_get_elem(struct qwx_pci_event_ring *, uint64_t);
-void	qwx_mhi_queue_state_change(struct qwx_pci_softc *, int, int);
-void	qwx_mhi_state_change(void *);
 void	qwx_pci_intr_ctrl_event_mhi(struct qwx_pci_softc *, uint32_t);
 void	qwx_pci_intr_ctrl_event_ee(struct qwx_pci_softc *, uint32_t);
 void	qwx_pci_intr_ctrl_event_cmd_complete(struct qwx_pci_softc *,
@@ -1035,10 +1019,6 @@ unsupported_wcn6855_soc:
 	if (sc->sc_nswq == NULL)
 		goto err_ce_free;
 
-	psc->mhi_taskq = taskq_create("qwxmhi", 1, IPL_NET, 0);
-	if (psc->mhi_taskq == NULL)
-		goto err_ce_free;
-
 	qwx_pci_init_qmi_ce_config(sc);
 
 	error = qwx_pcic_config_irq(sc, pa);
@@ -1065,7 +1045,6 @@ unsupported_wcn6855_soc:
 		goto err_irq_affinity_cleanup;
 	}
 #endif
-	task_set(&psc->mhi_newstate_task, qwx_mhi_state_change, psc);
 	task_set(&psc->rddm_task, qwx_rddm_task, psc);
 
 	ic->ic_phytype = IEEE80211_T_OFDM;	/* not only, but not used */
@@ -2188,12 +2167,8 @@ int
 qwx_pci_power_up(struct qwx_softc *sc)
 {
 	struct qwx_pci_softc *psc = (struct qwx_pci_softc *)sc;
-	struct qwx_mhi_newstate *ns = &psc->mhi_newstate;
 	int error;
 
-	task_del(psc->mhi_taskq, &psc->mhi_newstate_task);
-	memset(ns, 0, sizeof(*ns));
-
 	psc->register_window = 0;
 	clear_bit(ATH11K_FLAG_DEVICE_INIT_DONE, sc->sc_flags);
 
@@ -2219,12 +2194,6 @@ qwx_pci_power_up(struct qwx_softc *sc)
 void
 qwx_pci_power_down(struct qwx_softc *sc)
 {
-	struct qwx_pci_softc *psc = (struct qwx_pci_softc *)sc;
-	struct qwx_mhi_newstate *ns = &psc->mhi_newstate;
-
-	task_del(psc->mhi_taskq, &psc->mhi_newstate_task);
-	memset(ns, 0, sizeof(*ns));
-
 	/* restore aspm in case firmware bootup fails */
 	qwx_pci_aspm_restore(sc);
 
@@ -3612,119 +3581,87 @@ qwx_pci_event_ring_get_elem(struct qwx_pci_event_ring *ring, uint64_t rp)
 }
 
 void
-qwx_mhi_state_change(void *arg)
+qwx_mhi_state_change(struct qwx_pci_softc *psc, int ee, int mhi_state)
 {
-	struct qwx_pci_softc *psc = arg;
 	struct qwx_softc *sc = &psc->sc_sc;
-	struct qwx_mhi_newstate *ns = &psc->mhi_newstate;
-	int s = splnet();
-
-	while (ns->tail != ns->cur) {
-		int mhi_state = ns->queue[ns->tail].mhi_state;
-		int ee = ns->queue[ns->tail].ee;
-		uint32_t old_ee = psc->bhi_ee;
-		uint32_t old_mhi_state = psc->mhi_state;
-
-		KASSERT(ns->queued > 0);
+	uint32_t old_ee = psc->bhi_ee;
+	uint32_t old_mhi_state = psc->mhi_state;
 
-		if (ee != -1 && psc->bhi_ee != ee) {
-			switch (ee) {
-			case MHI_EE_PBL:
-				DNPRINTF(QWX_D_MHI, "%s: new EE PBL\n",
-				    sc->sc_dev.dv_xname);
-				psc->bhi_ee = ee;
-				break;
-			case MHI_EE_SBL:
-				psc->bhi_ee = ee;
-				DNPRINTF(QWX_D_MHI, "%s: new EE SBL\n",
-				    sc->sc_dev.dv_xname);
-				break;
-			case MHI_EE_AMSS:
-				DNPRINTF(QWX_D_MHI, "%s: new EE AMSS\n",
-				    sc->sc_dev.dv_xname);
-				psc->bhi_ee = ee;
-				/* Wake thread loading the full AMSS image. */
-				wakeup(&psc->bhie_off);
-				break;
-			case MHI_EE_WFW:
-				DNPRINTF(QWX_D_MHI, "%s: new EE WFW\n",
-				    sc->sc_dev.dv_xname);
-				psc->bhi_ee = ee;
-				break;
-			default:
-				printf("%s: unhandled EE change to %x\n",
-				    sc->sc_dev.dv_xname, ee);
-				break;
-			}
-		}
-
-		if (mhi_state != -1 && psc->mhi_state != mhi_state) {
-			switch (mhi_state) {
-			case -1:
-				break;
-			case MHI_STATE_RESET:
-				DNPRINTF(QWX_D_MHI, "%s: new MHI state RESET\n",
-				    sc->sc_dev.dv_xname);
-				psc->mhi_state = mhi_state;
-				break;
-			case MHI_STATE_READY:
-				DNPRINTF(QWX_D_MHI, "%s: new MHI state READY\n",
-				    sc->sc_dev.dv_xname);
-				psc->mhi_state = mhi_state;
-				qwx_mhi_ready_state_transition(psc);
-				break;
-			case MHI_STATE_M0:
-				DNPRINTF(QWX_D_MHI, "%s: new MHI state M0\n",
-				    sc->sc_dev.dv_xname);
-				psc->mhi_state = mhi_state;
-				qwx_mhi_mission_mode_state_transition(psc);
-				break;
-			case MHI_STATE_M1:
-				DNPRINTF(QWX_D_MHI, "%s: new MHI state M1\n",
-				    sc->sc_dev.dv_xname);
-				psc->mhi_state = mhi_state;
-				qwx_mhi_low_power_mode_state_transition(psc);
-				break;
-			case MHI_STATE_SYS_ERR:
-				DNPRINTF(QWX_D_MHI,
-				    "%s: new MHI state SYS ERR\n",
-				    sc->sc_dev.dv_xname);
-				psc->mhi_state = mhi_state;
-				break;
-			default:
-				printf("%s: unhandled MHI state change to %x\n",
-				    sc->sc_dev.dv_xname, mhi_state);
-				break;
-			}
+	if (ee != -1 && psc->bhi_ee != ee) {
+		switch (ee) {
+		case MHI_EE_PBL:
+			DNPRINTF(QWX_D_MHI, "%s: new EE PBL\n",
+			    sc->sc_dev.dv_xname);
+			psc->bhi_ee = ee;
+			break;
+		case MHI_EE_SBL:
+			psc->bhi_ee = ee;
+			DNPRINTF(QWX_D_MHI, "%s: new EE SBL\n",
+			    sc->sc_dev.dv_xname);
+			break;
+		case MHI_EE_AMSS:
+			DNPRINTF(QWX_D_MHI, "%s: new EE AMSS\n",
+			    sc->sc_dev.dv_xname);
+			psc->bhi_ee = ee;
+			/* Wake thread loading the full AMSS image. */
+			wakeup(&psc->bhie_off);
+			break;
+		case MHI_EE_WFW:
+			DNPRINTF(QWX_D_MHI, "%s: new EE WFW\n",
+			    sc->sc_dev.dv_xname);
+			psc->bhi_ee = ee;
+			break;
+		default:
+			printf("%s: unhandled EE change to %x\n",
+			    sc->sc_dev.dv_xname, ee);
+			break;
 		}
-
-		if (old_ee != psc->bhi_ee)
-			wakeup(&psc->bhi_ee);
-		if (old_mhi_state != psc->mhi_state)
-			wakeup(&psc->mhi_state);
-
-		ns->tail = (ns->tail + 1) % nitems(ns->queue);
-		ns->queued--;
 	}
 
-	splx(s);
-}
-
-void
-qwx_mhi_queue_state_change(struct qwx_pci_softc *psc, int ee, int mhi_state)
-{
-	struct qwx_mhi_newstate *ns = &psc->mhi_newstate;
-
-	if (ns->queued >= nitems(ns->queue)) {
-		printf("%s: event queue full, dropping event\n", __func__);
-		return;
+	if (mhi_state != -1 && psc->mhi_state != mhi_state) {
+		switch (mhi_state) {
+		case -1:
+			break;
+		case MHI_STATE_RESET:
+			DNPRINTF(QWX_D_MHI, "%s: new MHI state RESET\n",
+			    sc->sc_dev.dv_xname);
+			psc->mhi_state = mhi_state;
+			break;
+		case MHI_STATE_READY:
+			DNPRINTF(QWX_D_MHI, "%s: new MHI state READY\n",
+			    sc->sc_dev.dv_xname);
+			psc->mhi_state = mhi_state;
+			qwx_mhi_ready_state_transition(psc);
+			break;
+		case MHI_STATE_M0:
+			DNPRINTF(QWX_D_MHI, "%s: new MHI state M0\n",
+			    sc->sc_dev.dv_xname);
+			psc->mhi_state = mhi_state;
+			qwx_mhi_mission_mode_state_transition(psc);
+			break;
+		case MHI_STATE_M1:
+			DNPRINTF(QWX_D_MHI, "%s: new MHI state M1\n",
+			    sc->sc_dev.dv_xname);
+			psc->mhi_state = mhi_state;
+			qwx_mhi_low_power_mode_state_transition(psc);
+			break;
+		case MHI_STATE_SYS_ERR:
+			DNPRINTF(QWX_D_MHI,
+			    "%s: new MHI state SYS ERR\n",
+			    sc->sc_dev.dv_xname);
+			psc->mhi_state = mhi_state;
+			break;
+		default:
+			printf("%s: unhandled MHI state change to %x\n",
+			    sc->sc_dev.dv_xname, mhi_state);
+			break;
+		}
 	}
 
-	ns->queue[ns->cur].ee = ee;
-	ns->queue[ns->cur].mhi_state = mhi_state;
-	ns->queued++;
-	ns->cur = (ns->cur + 1) % nitems(ns->queue);
-	task_add(psc->mhi_taskq, &psc->mhi_newstate_task);
+	if (old_ee != psc->bhi_ee)
+		wakeup(&psc->bhi_ee);
+	if (old_mhi_state != psc->mhi_state)
+		wakeup(&psc->mhi_state);
 }
 
 void
@@ -3734,7 +3671,7 @@ qwx_pci_intr_ctrl_event_mhi(struct qwx_pci_softc *psc, uint32_t mhi_state)
 	    psc->mhi_state, mhi_state);
 
 	if (psc->mhi_state != mhi_state)
-		qwx_mhi_queue_state_change(psc, -1, mhi_state);
+		qwx_mhi_state_change(psc, -1, mhi_state);
 }
 
 void
@@ -3744,7 +3681,7 @@ qwx_pci_intr_ctrl_event_ee(struct qwx_pci_softc *psc, uint32_t ee)
 	    psc->bhi_ee, ee);
 
 	if (psc->bhi_ee != ee)
-		qwx_mhi_queue_state_change(psc, ee, -1);
+		qwx_mhi_state_change(psc, ee, -1);
 }
 
 void
@@ -4153,7 +4090,7 @@ qwx_pci_intr(void *arg)
 			new_mhi_state = state;
 
 		if (new_ee != -1 || new_mhi_state != -1)
-			qwx_mhi_queue_state_change(psc, new_ee, new_mhi_state);
+			qwx_mhi_state_change(psc, new_ee, new_mhi_state);
 
 		ret = 1;
 	}