Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 27 Nov 2017 22:13:31 +0000 (UTC)
From:      "Landon J. Fuller" <landonf@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r326292 - in head: share/man/man9 sys/dev/bhnd sys/dev/bhnd/bcma sys/dev/bhnd/cores/usb sys/dev/bhnd/siba
Message-ID:  <201711272213.vARMDVit099232@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: landonf
Date: Mon Nov 27 22:13:30 2017
New Revision: 326292
URL: https://svnweb.freebsd.org/changeset/base/326292

Log:
  bhnd(4): Fix bcma/siba core reset behavior
  
  Add missing support for specifying I/O control flags during core reset,
  and resolve a number of siba(4)-specific reset issues:
  
  - Add missing check for target reject flags in siba_is_hw_suspended().
  - Remove incorrect wait on SIBA_TMH_BUSY when modifying any target state
    register; this should only be done when waiting for initiated
    transactions to clear.
  - Add missing wait on SIBA_IM_BY when asserting SIBA_IM_RJ.
  - Overwrite any previously set SIBA_TML_REJ flag when bringing the core
    out of reset. This fixes a lockup that occured when we brought up a core
    (after reboot) that had previously been placed into RESET by siba_bwn(4).
  
  Approved by:	adrian (mentor, implicit)
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D13039

Modified:
  head/share/man/man9/bhnd.9
  head/sys/dev/bhnd/bcma/bcma.c
  head/sys/dev/bhnd/bcma/bcma_subr.c
  head/sys/dev/bhnd/bhnd.h
  head/sys/dev/bhnd/bhnd_bus_if.m
  head/sys/dev/bhnd/cores/usb/bhnd_usb.c
  head/sys/dev/bhnd/siba/siba.c
  head/sys/dev/bhnd/siba/siba_subr.c
  head/sys/dev/bhnd/siba/sibavar.h

Modified: head/share/man/man9/bhnd.9
==============================================================================
--- head/share/man/man9/bhnd.9	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/share/man/man9/bhnd.9	Mon Nov 27 22:13:30 2017	(r326292)
@@ -266,9 +266,9 @@
 .Fa "device_t dev" "bus_size_t offset" "const void *value" "u_int width"
 .Fc
 .Ft int
-.Fn bhnd_reset_hw "device_t dev" "uint16_t ioctl"
+.Fn bhnd_reset_hw "device_t dev" "uint16_t ioctl" "uint16_t reset_ioctl"
 .Ft int
-.Fn bhnd_suspend_hw "device_t dev"
+.Fn bhnd_suspend_hw "device_t dev" "uint16_t ioctl"
 .Ft bool
 .Fn bhnd_is_hw_suspended "device_t dev"
 .\"
@@ -1054,7 +1054,10 @@ function transitions the device
 .Fa dev
 to a low power
 .Dq RESET
-state.
+state, writing
+.Fa ioctl
+to the I/O control flags of
+.Fa dev .
 The hardware may be brought out of this state using
 .Fn bhnd_reset_hw .
 .Pp
@@ -1062,10 +1065,14 @@ The
 .Fn bhnd_reset_hw
 function first transitions the device
 .Fa dev
-to a low power RESET state, and then brings the device out of RESET, writing
+to a low power RESET state, writing
+.Fa ioctl_reset
+to the I/O control flags
+of
+.Fa dev ,
+and then brings the device out of RESET, writing
 .Fa ioctl
-to the I/O control flags of
-.Fa dev .
+to the device's I/O control flags.
 .Pp
 The
 .Fn bhnd_is_hw_suspended

Modified: head/sys/dev/bhnd/bcma/bcma.c
==============================================================================
--- head/sys/dev/bhnd/bcma/bcma.c	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/bcma/bcma.c	Mon Nov 27 22:13:30 2017	(r326292)
@@ -296,22 +296,22 @@ bcma_is_hw_suspended(device_t dev, device_t child)
 }
 
 static int
-bcma_reset_hw(device_t dev, device_t child, uint16_t ioctl)
+bcma_reset_hw(device_t dev, device_t child, uint16_t ioctl,
+    uint16_t reset_ioctl)
 {
-	struct bcma_devinfo		*dinfo;
-	struct bhnd_core_pmu_info	*pm;
-	struct bhnd_resource		*r;
-	int				 error;
+	struct bcma_devinfo	*dinfo;
+	struct bhnd_resource	*r;
+	uint16_t		 clkflags;
+	int			 error;
 
 	if (device_get_parent(child) != dev)
 		return (EINVAL);
 
 	dinfo = device_get_ivars(child);
-	pm = dinfo->pmu_info;
 
-	/* We require exclusive control over BHND_IOCTL_CLK_EN and
-	 * BHND_IOCTL_CLK_FORCE. */
-	if (ioctl & (BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE))
+	/* We require exclusive control over BHND_IOCTL_CLK_(EN|FORCE) */
+	clkflags = BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE;
+	if (ioctl & clkflags)
 		return (EINVAL);
 
 	/* Can't suspend the core without access to the agent registers */
@@ -319,7 +319,7 @@ bcma_reset_hw(device_t dev, device_t child, uint16_t i
 		return (ENODEV);
 
 	/* Place core into known RESET state */
-	if ((error = BHND_BUS_SUSPEND_HW(dev, child)))
+	if ((error = bhnd_suspend_hw(child, reset_ioctl)))
 		return (error);
 
 	/*
@@ -329,9 +329,7 @@ bcma_reset_hw(device_t dev, device_t child, uint16_t i
 	 * - Force clock distribution to ensure propagation throughout the
 	 *   core.
 	 */
-	error = bhnd_write_ioctl(child, 
-	    ioctl | BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE, UINT16_MAX);
-	if (error)
+	if ((error = bhnd_write_ioctl(child, ioctl | clkflags, UINT16_MAX)))
 		return (error);
 
 	/* Bring the core out of reset */
@@ -347,11 +345,11 @@ bcma_reset_hw(device_t dev, device_t child, uint16_t i
 }
 
 static int
-bcma_suspend_hw(device_t dev, device_t child)
+bcma_suspend_hw(device_t dev, device_t child, uint16_t ioctl)
 {
 	struct bcma_devinfo	*dinfo;
 	struct bhnd_resource	*r;
-	uint32_t		 rst;
+	uint16_t		 clkflags;
 	int			 error;
 
 	if (device_get_parent(child) != dev)
@@ -359,6 +357,11 @@ bcma_suspend_hw(device_t dev, device_t child)
 
 	dinfo = device_get_ivars(child);
 
+	/* We require exclusive control over BHND_IOCTL_CLK_(EN|FORCE) */
+	clkflags = BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE;
+	if (ioctl & clkflags)
+		return (EINVAL);
+
 	/* Can't suspend the core without access to the agent registers */
 	if ((r = dinfo->res_agent) == NULL)
 		return (ENODEV);
@@ -367,17 +370,12 @@ bcma_suspend_hw(device_t dev, device_t child)
 	if ((error = bcma_dmp_wait_reset(child, dinfo)))
 		return (error);
 
-	/* Already in reset? */
-	rst = bhnd_bus_read_4(r, BCMA_DMP_RESETCTRL);
-	if (rst & BCMA_DMP_RC_RESET)
-		return (0);
-
-	/* Put core into reset */
+	/* Put core into reset (if not already in reset) */
 	if ((error = bcma_dmp_write_reset(child, dinfo, BCMA_DMP_RC_RESET)))
 		return (error);
 
-	/* Clear core flags */
-	if ((error = bhnd_write_ioctl(child, 0x0, UINT16_MAX)))
+	/* Write core flags (and clear CLK_EN/CLK_FORCE) */
+	if ((error = bhnd_write_ioctl(child, ioctl, ~clkflags)))
 		return (error);
 
 	return (0);

Modified: head/sys/dev/bhnd/bcma/bcma_subr.c
==============================================================================
--- head/sys/dev/bhnd/bcma/bcma_subr.c	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/bcma/bcma_subr.c	Mon Nov 27 22:13:30 2017	(r326292)
@@ -594,8 +594,15 @@ bcma_dmp_wait_reset(device_t child, struct bcma_devinf
 int
 bcma_dmp_write_reset(device_t child, struct bcma_devinfo *dinfo, uint32_t value)
 {
+	uint32_t rst;
+
 	if (dinfo->res_agent == NULL)
 		return (ENODEV);
+
+	/* Already in requested reset state? */
+	rst = bhnd_bus_read_4(dinfo->res_agent, BCMA_DMP_RESETCTRL);
+	if (rst == value)
+		return (0);
 
 	bhnd_bus_write_4(dinfo->res_agent, BCMA_DMP_RESETCTRL, value);
 	bhnd_bus_read_4(dinfo->res_agent, BCMA_DMP_RESETCTRL); /* read-back */

Modified: head/sys/dev/bhnd/bhnd.h
==============================================================================
--- head/sys/dev/bhnd/bhnd.h	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/bhnd.h	Mon Nov 27 22:13:30 2017	(r326292)
@@ -818,23 +818,27 @@ bhnd_is_hw_suspended(device_t dev)
 }
 
 /**
- * Place the bhnd(4) device's hardware into a reset state, and then bring the
- * hardware out of reset with BHND_IOCTL_CLK_EN and @p ioctl flags set.
+ * Place the bhnd(4) device's hardware into a low-power RESET state with
+ * the @p reset_ioctl I/O control flags set, and then bring the hardware out of
+ * RESET with the @p ioctl I/O control flags set.
  * 
- * Any clock or resource PMU requests previously made by @p dev will be
+ * Any clock or resource PMU requests previously made by @p child will be
  * invalidated.
  *
  * @param dev The device to be reset.
- * @param ioctl Device-specific core ioctl flags to be supplied on reset
- * (see BHND_IOCTL_*).
+ * @param ioctl Device-specific I/O control flags to be set when bringing
+ * the core out of its RESET state (see BHND_IOCTL_*).
+ * @param reset_ioctl Device-specific I/O control flags to be set when placing
+ * the core into its RESET state.
  *
  * @retval 0 success
  * @retval non-zero error
  */
 static inline int
-bhnd_reset_hw(device_t dev, uint16_t ioctl)
+bhnd_reset_hw(device_t dev, uint16_t ioctl, uint16_t reset_ioctl)
 {
-	return (BHND_BUS_RESET_HW(device_get_parent(dev), dev, ioctl));
+	return (BHND_BUS_RESET_HW(device_get_parent(dev), dev, ioctl,
+	    reset_ioctl));
 }
 
 /**
@@ -851,9 +855,9 @@ bhnd_reset_hw(device_t dev, uint16_t ioctl)
  * @retval non-zero error
  */
 static inline int
-bhnd_suspend_hw(device_t dev)
+bhnd_suspend_hw(device_t dev, uint16_t ioctl)
 {
-	return (BHND_BUS_SUSPEND_HW(device_get_parent(dev), dev));
+	return (BHND_BUS_SUSPEND_HW(device_get_parent(dev), dev, ioctl));
 }
 
 /**

Modified: head/sys/dev/bhnd/bhnd_bus_if.m
==============================================================================
--- head/sys/dev/bhnd/bhnd_bus_if.m	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/bhnd_bus_if.m	Mon Nov 27 22:13:30 2017	(r326292)
@@ -96,7 +96,8 @@ CODE {
 	}
 
 	static int
-	bhnd_bus_null_reset_hw(device_t dev, device_t child, uint16_t ioctl)
+	bhnd_bus_null_reset_hw(device_t dev, device_t child, uint16_t ioctl,
+	    uint16_t reset_ioctl)
 	{
 		panic("bhnd_bus_reset_hw unimplemented");
 	}
@@ -624,16 +625,19 @@ METHOD bool is_hw_suspended {
 } DEFAULT bhnd_bus_null_is_hw_suspended;
 
 /**
- * Place the bhnd(4) device's hardware into a reset state, and then bring the
- * hardware out of reset with BHND_IOCTL_CLK_EN and @p ioctl flags set.
+ * Place the bhnd(4) device's hardware into a low-power RESET state with
+ * the @p reset_ioctl I/O control flags set, and then bring the hardware out of
+ * RESET with the @p ioctl I/O control flags set.
  * 
  * Any clock or resource PMU requests previously made by @p child will be
  * invalidated.
  *
  * @param dev The bhnd bus parent of @p child.
  * @param child The device to be reset.
- * @param ioctl Device-specific core ioctl flags to be supplied on reset
- * (see BHND_IOCTL_*).
+ * @param ioctl Device-specific I/O control flags to be set when bringing
+ * the core out of its RESET state (see BHND_IOCTL_*).
+ * @param reset_ioctl Device-specific I/O control flags to be set when placing
+ * the core into its RESET state.
  *
  * @retval 0 success
  * @retval non-zero error
@@ -642,18 +646,21 @@ METHOD int reset_hw {
 	device_t dev;
 	device_t child;
 	uint16_t ioctl;
+	uint16_t reset_ioctl;
 } DEFAULT bhnd_bus_null_reset_hw;
 
 /**
- * Suspend @p child's hardware in a low-power reset state.
+ * Suspend @p child's hardware in a low-power RESET state.
  *
  * Any clock or resource PMU requests previously made by @p dev will be
  * invalidated.
  *
- * The hardware may be brought out of reset via bhnd_reset_hw().
+ * The hardware may be brought out of RESET via bhnd_reset_hw().
  *
  * @param dev The bhnd bus parent of @p child.
  * @param dev The device to be suspended.
+ * @param ioctl Device-specific I/O control flags to be set when placing
+ * the core into its RESET state (see BHND_IOCTL_*).
  *
  * @retval 0 success
  * @retval non-zero error
@@ -661,6 +668,7 @@ METHOD int reset_hw {
 METHOD int suspend_hw {
 	device_t dev;
 	device_t child;
+	uint16_t ioctl;
 } DEFAULT bhnd_bus_null_suspend_hw;
 
 /**

Modified: head/sys/dev/bhnd/cores/usb/bhnd_usb.c
==============================================================================
--- head/sys/dev/bhnd/cores/usb/bhnd_usb.c	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/cores/usb/bhnd_usb.c	Mon Nov 27 22:13:30 2017	(r326292)
@@ -98,7 +98,7 @@ bhnd_usb_attach(device_t dev)
 
 	sc = device_get_softc(dev);
 
-	bhnd_reset_hw(dev, 0);
+	bhnd_reset_hw(dev, 0, 0);
 
 	/*
 	 * Allocate the resources which the parent bus has already

Modified: head/sys/dev/bhnd/siba/siba.c
==============================================================================
--- head/sys/dev/bhnd/siba/siba.c	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/siba/siba.c	Mon Nov 27 22:13:30 2017	(r326292)
@@ -603,8 +603,9 @@ siba_write_ioctl(device_t dev, device_t child, uint16_
 	ts_mask = (mask << SIBA_TML_SICF_SHIFT) & SIBA_TML_SICF_MASK;
 	ts_low = (value << SIBA_TML_SICF_SHIFT) & ts_mask;
 
-	return (siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
-	    ts_low, ts_mask));
+	siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
+	    ts_low, ts_mask);
+	return (0);
 }
 
 static bool
@@ -626,6 +627,10 @@ siba_is_hw_suspended(device_t dev, device_t child)
 	if (ts_low & SIBA_TML_RESET)
 		return (true);
 
+	/* Is target reject enabled? */
+	if (ts_low & SIBA_TML_REJ_MASK)
+		return (true);
+
 	/* Is core clocked? */
 	ioctl = SIBA_REG_GET(ts_low, TML_SICF);
 	if (!(ioctl & BHND_IOCTL_CLK_EN))
@@ -635,11 +640,13 @@ siba_is_hw_suspended(device_t dev, device_t child)
 }
 
 static int
-siba_reset_hw(device_t dev, device_t child, uint16_t ioctl)
+siba_reset_hw(device_t dev, device_t child, uint16_t ioctl,
+    uint16_t reset_ioctl)
 {
 	struct siba_devinfo		*dinfo;
 	struct bhnd_resource		*r;
 	uint32_t			 ts_low, imstate;
+	uint16_t			 clkflags;
 	int				 error;
 
 	if (device_get_parent(child) != dev)
@@ -651,66 +658,60 @@ siba_reset_hw(device_t dev, device_t child, uint16_t i
 	if ((r = dinfo->cfg_res[0]) == NULL)
 		return (ENODEV);
 
-	/* We require exclusive control over BHND_IOCTL_CLK_EN and
-	 * BHND_IOCTL_CLK_FORCE. */
-	if (ioctl & (BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE))
+	/* We require exclusive control over BHND_IOCTL_CLK_(EN|FORCE) */
+	clkflags = BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE;
+	if (ioctl & clkflags)
 		return (EINVAL);
 
 	/* Place core into known RESET state */
-	if ((error = BHND_BUS_SUSPEND_HW(dev, child)))
+	if ((error = bhnd_suspend_hw(child, reset_ioctl)))
 		return (error);
 
-	/* Leaving the core in reset, set the caller's IOCTL flags and
-	 * enable the core's clocks. */
-	ts_low = (ioctl | BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE) <<
-	    SIBA_TML_SICF_SHIFT;
-	error = siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
-	    ts_low, SIBA_TML_SICF_MASK);
-	if (error)
-		return (error);
+	/* Set RESET, clear REJ, set the caller's IOCTL flags, and
+	 * force clocks to ensure the signal propagates throughout the
+	 * core. */
+	ts_low = SIBA_TML_RESET |
+		 (ioctl << SIBA_TML_SICF_SHIFT) |
+		 (BHND_IOCTL_CLK_EN << SIBA_TML_SICF_SHIFT) |
+		 (BHND_IOCTL_CLK_FORCE << SIBA_TML_SICF_SHIFT);
 
+	siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
+	    ts_low, UINT32_MAX);
+
 	/* Clear any target errors */
 	if (bhnd_bus_read_4(r, SIBA_CFG0_TMSTATEHIGH) & SIBA_TMH_SERR) {
-		error = siba_write_target_state(child, dinfo,
-		    SIBA_CFG0_TMSTATEHIGH, 0, SIBA_TMH_SERR);
-		if (error)
-			return (error);
+		siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATEHIGH,
+		    0x0, SIBA_TMH_SERR);
 	}
 
 	/* Clear any initiator errors */
 	imstate = bhnd_bus_read_4(r, SIBA_CFG0_IMSTATE);
 	if (imstate & (SIBA_IM_IBE|SIBA_IM_TO)) {
-		error = siba_write_target_state(child, dinfo, SIBA_CFG0_IMSTATE,
-		    0, SIBA_IM_IBE|SIBA_IM_TO);
-		if (error)
-			return (error);
+		siba_write_target_state(child, dinfo, SIBA_CFG0_IMSTATE, 0x0,
+		    SIBA_IM_IBE|SIBA_IM_TO);
 	}
 
 	/* Release from RESET while leaving clocks forced, ensuring the
 	 * signal propagates throughout the core */
-	error = siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
-	    0x0, SIBA_TML_RESET);
-	if (error)
-		return (error);
+	siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW, 0x0,
+	    SIBA_TML_RESET);
 
 	/* The core should now be active; we can clear the BHND_IOCTL_CLK_FORCE
 	 * bit and allow the core to manage clock gating. */
-	error = siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
-	    0x0, (BHND_IOCTL_CLK_FORCE << SIBA_TML_SICF_SHIFT));
-	if (error)
-		return (error);
+	siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW, 0x0,
+	    (BHND_IOCTL_CLK_FORCE << SIBA_TML_SICF_SHIFT));
 
 	return (0);
 }
 
 static int
-siba_suspend_hw(device_t dev, device_t child)
+siba_suspend_hw(device_t dev, device_t child, uint16_t ioctl)
 {
 	struct siba_softc		*sc;
 	struct siba_devinfo		*dinfo;
 	struct bhnd_resource		*r;
-	uint32_t			 idl, ts_low;
-	uint16_t			 ioctl;
+	uint32_t			 idl, ts_low, ts_mask;
+	uint16_t			 cflags, clkflags;
 	int				 error;
 
 	if (device_get_parent(child) != dev)
@@ -723,30 +724,37 @@ siba_suspend_hw(device_t dev, device_t child)
 	if ((r = dinfo->cfg_res[0]) == NULL)
 		return (ENODEV);
 
+	/* We require exclusive control over BHND_IOCTL_CLK_(EN|FORCE) */
+	clkflags = BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE;
+	if (ioctl & clkflags)
+		return (EINVAL);
+
 	/* Already in RESET? */
 	ts_low = bhnd_bus_read_4(r, SIBA_CFG0_TMSTATELOW);
-	if (ts_low & SIBA_TML_RESET) {
-		/* Clear IOCTL flags, ensuring the clock is disabled */
-		return (siba_write_target_state(child, dinfo,
-		    SIBA_CFG0_TMSTATELOW, 0x0, SIBA_TML_SICF_MASK));
+	if (ts_low & SIBA_TML_RESET)
+		return (0);
 
+	/* If clocks are already disabled, we can place the core directly
+	 * into RESET|REJ while setting the caller's IOCTL flags. */
+	cflags = SIBA_REG_GET(ts_low, TML_SICF);
+	if (!(cflags & BHND_IOCTL_CLK_EN)) {
+		ts_low = SIBA_TML_RESET | SIBA_TML_REJ |
+			 (ioctl << SIBA_TML_SICF_SHIFT);
+		ts_mask = SIBA_TML_RESET | SIBA_TML_REJ | SIBA_TML_SICF_MASK;
+
+		siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
+		    ts_low, ts_mask);
 		return (0);
 	}
 
-	/* If clocks are already disabled, we can put the core directly
-	 * into RESET */
-	ioctl = SIBA_REG_GET(ts_low, TML_SICF);
-	if (!(ioctl & BHND_IOCTL_CLK_EN)) {
-		/* Set RESET and clear IOCTL flags */
-		return (siba_write_target_state(child, dinfo, 
-		    SIBA_CFG0_TMSTATELOW,
-		    SIBA_TML_RESET,
-		    SIBA_TML_RESET | SIBA_TML_SICF_MASK));
-	}
-
-	/* Reject any further target backplane transactions */
-	error = siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
+	/* Reject further transactions reaching this core */
+	siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
 	    SIBA_TML_REJ, SIBA_TML_REJ);
+
+	/* Wait for transaction busy flag to clear for all transactions
+	 * initiated by this core */
+	error = siba_wait_target_state(child, dinfo, SIBA_CFG0_TMSTATEHIGH,
+	    0x0, SIBA_TMH_BUSY, 100000);
 	if (error)
 		return (error);
 
@@ -754,44 +762,47 @@ siba_suspend_hw(device_t dev, device_t child)
 	 * transactions too. */
 	idl = bhnd_bus_read_4(r, SIBA_CFG0_IDLOW);
 	if (idl & SIBA_IDL_INIT) {
-		error = siba_write_target_state(child, dinfo, SIBA_CFG0_IMSTATE,
+		/* Reject further initiator transactions */
+		siba_write_target_state(child, dinfo, SIBA_CFG0_IMSTATE,
 		    SIBA_IM_RJ, SIBA_IM_RJ);
+
+		/* Wait for initiator busy flag to clear */
+		error = siba_wait_target_state(child, dinfo, SIBA_CFG0_IMSTATE,
+		    0x0, SIBA_IM_BY, 100000);
 		if (error)
 			return (error);
 	}
 
-	/* Put the core into RESET|REJECT, forcing clocks to ensure the RESET
-	 * signal propagates throughout the core, leaving REJECT asserted. */
-	ts_low = SIBA_TML_RESET;
-	ts_low |= (BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE) <<
-	    SIBA_TML_SICF_SHIFT;
+	/* Put the core into RESET, set the caller's IOCTL flags, and
+	 * force clocks to ensure the RESET signal propagates throughout the
+	 * core. */
+	ts_low = SIBA_TML_RESET |
+		 (ioctl << SIBA_TML_SICF_SHIFT) |
+		 (BHND_IOCTL_CLK_EN << SIBA_TML_SICF_SHIFT) |
+		 (BHND_IOCTL_CLK_FORCE << SIBA_TML_SICF_SHIFT);
+	ts_mask = SIBA_TML_RESET |
+		  SIBA_TML_SICF_MASK;
 
-	error = siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
-		ts_low, ts_low);
+	siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW, ts_low,
+	    ts_mask);
 	if (error)
 		return (error);
 
 	/* Give RESET ample time */
 	DELAY(10);
 
-	/* Leaving core in reset, disable all clocks, clear REJ flags and
-	 * IOCTL state */
-	error = siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW,
-		SIBA_TML_RESET,
-		SIBA_TML_RESET | SIBA_TML_REJ | SIBA_TML_SICF_MASK);
-	if (error)
-		return (error);
-
 	/* Clear previously asserted initiator reject */
 	if (idl & SIBA_IDL_INIT) {
-		error = siba_write_target_state(child, dinfo, SIBA_CFG0_IMSTATE,
-		    0, SIBA_IM_RJ);
-		if (error)
-			return (error);
+		siba_write_target_state(child, dinfo, SIBA_CFG0_IMSTATE, 0x0,
+		    SIBA_IM_RJ);
 	}
 
+	/* Disable all clocks, leaving RESET and REJ asserted */
+	siba_write_target_state(child, dinfo, SIBA_CFG0_TMSTATELOW, 0x0,
+	    (BHND_IOCTL_CLK_EN | BHND_IOCTL_CLK_FORCE) << SIBA_TML_SICF_SHIFT);
+
 	/*
-	 * Core is now in RESET, with clocks disabled and REJ not asserted.
+	 * Core is now in RESET.
 	 *
 	 * If the core holds any PWRCTL clock reservations, we need to release
 	 * those now. This emulates the standard bhnd(4) PMU behavior of RESET

Modified: head/sys/dev/bhnd/siba/siba_subr.c
==============================================================================
--- head/sys/dev/bhnd/siba/siba_subr.c	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/siba/siba_subr.c	Mon Nov 27 22:13:30 2017	(r326292)
@@ -624,83 +624,73 @@ siba_parse_admatch(uint32_t am, uint32_t *addr, uint32
 }
 
 /**
- * Write @p value to @p dev's CFG0 target/initiator state register and
- * wait for completion.
+ * Write @p value to @p dev's CFG0 target/initiator state register, performing
+ * required read-back and waiting for completion.
  * 
  * @param dev The siba(4) child device.
- * @param reg The state register to write (e.g. SIBA_CFG0_TMSTATELOW,
- *    SIBA_CFG0_IMSTATE)
+ * @param reg The CFG0 state register to write (e.g. SIBA_CFG0_TMSTATELOW,
+ * SIBA_CFG0_IMSTATE)
  * @param value The value to write to @p reg.
  * @param mask The mask of bits to be included from @p value.
- * 
- * @retval 0 success.
- * @retval ENODEV if SIBA_CFG0 is not mapped by @p dinfo.
- * @retval ETIMEDOUT if a timeout occurs prior to SIBA_TMH_BUSY clearing.
  */
-int
+void
 siba_write_target_state(device_t dev, struct siba_devinfo *dinfo,
     bus_size_t reg, uint32_t value, uint32_t mask)
 {
 	struct bhnd_resource	*r;
 	uint32_t		 rval;
 
-	/* Must have a CFG0 block */
-	if ((r = dinfo->cfg_res[0]) == NULL)
-		return (ENODEV);
+	r = dinfo->cfg_res[0];
 
-	/* Verify the register offset falls within CFG register block */
-	if (reg > SIBA_CFG_SIZE-4)
-		return (EFAULT);
+	KASSERT(r != NULL, ("%s missing CFG0 mapping",
+	    device_get_nameunit(dev)));
+	KASSERT(reg <= SIBA_CFG_SIZE-4, ("%s invalid CFG0 register offset %#jx",
+	    device_get_nameunit(dev), (uintmax_t)reg));
 
-	for (int i = 0; i < 300; i += 10) {
-		rval = bhnd_bus_read_4(r, reg);
-		rval &= ~mask;
-		rval |= (value & mask);
+	rval = bhnd_bus_read_4(r, reg);
+	rval &= ~mask;
+	rval |= (value & mask);
 
-		bhnd_bus_write_4(r, reg, rval);
-		bhnd_bus_read_4(r, reg); /* read-back */
-		DELAY(1);
-
-		/* If the write has completed, wait for target busy state
-		 * to clear */
-		rval = bhnd_bus_read_4(r, reg);
-		if ((rval & mask) == (value & mask))
-			return (siba_wait_target_busy(dev, dinfo, 100000));
-
-		DELAY(10);
-	}
-
-	return (ETIMEDOUT);
+	bhnd_bus_write_4(r, reg, rval);
+	bhnd_bus_read_4(r, reg); /* read-back */
+	DELAY(1);
 }
 
 /**
- * Spin for up to @p usec waiting for SIBA_TMH_BUSY to clear in
- * @p dev's SIBA_CFG0_TMSTATEHIGH register.
+ * Spin for up to @p usec waiting for @p dev's CFG0 target/initiator state
+ * register value to be equal to @p value after applying @p mask bits to both
+ * values.
  * 
  * @param dev The siba(4) child device to wait on.
  * @param dinfo The @p dev's device info
+ * @param reg The state register to read (e.g. SIBA_CFG0_TMSTATEHIGH,
+ * SIBA_CFG0_IMSTATE)
+ * @param value The value against which @p reg will be compared.
+ * @param mask The mask to be applied when comparing @p value with @p reg.
+ * @param usec The maximum number of microseconds to wait for completion.
  * 
  * @retval 0 if SIBA_TMH_BUSY is cleared prior to the @p usec timeout.
  * @retval ENODEV if SIBA_CFG0 is not mapped by @p dinfo.
- * @retval ETIMEDOUT if a timeout occurs prior to SIBA_TMH_BUSY clearing.
+ * @retval ETIMEDOUT if a timeout occurs.
  */
 int
-siba_wait_target_busy(device_t dev, struct siba_devinfo *dinfo, int usec)
+siba_wait_target_state(device_t dev, struct siba_devinfo *dinfo, bus_size_t reg,
+    uint32_t value, uint32_t mask, u_int usec)
 {
 	struct bhnd_resource	*r;
-	uint32_t		 ts_high;
+	uint32_t		 rval;
 
 	if ((r = dinfo->cfg_res[0]) == NULL)
 		return (ENODEV);
 
+	value &= mask;
 	for (int i = 0; i < usec; i += 10) {
-		ts_high = bhnd_bus_read_4(r, SIBA_CFG0_TMSTATEHIGH);
-		if (!(ts_high & SIBA_TMH_BUSY))
+		rval = bhnd_bus_read_4(r, reg);
+		if ((rval & mask) == value)
 			return (0);
 
 		DELAY(10);
 	}
 
-	device_printf(dev, "SIBA_TMH_BUSY wait timeout\n");
 	return (ETIMEDOUT);
 }

Modified: head/sys/dev/bhnd/siba/sibavar.h
==============================================================================
--- head/sys/dev/bhnd/siba/sibavar.h	Mon Nov 27 21:30:49 2017	(r326291)
+++ head/sys/dev/bhnd/siba/sibavar.h	Mon Nov 27 22:13:30 2017	(r326292)
@@ -117,11 +117,12 @@ u_int			 siba_admatch_offset(uint8_t addrspace);
 int			 siba_parse_admatch(uint32_t am, uint32_t *addr,
 			     uint32_t *size);
 
-int			 siba_write_target_state(device_t dev,
+void			 siba_write_target_state(device_t dev,
 			     struct siba_devinfo *dinfo, bus_size_t reg,
 			     uint32_t value, uint32_t mask);
-int			 siba_wait_target_busy(device_t child,
-			     struct siba_devinfo *dinfo, int usec);
+int			 siba_wait_target_state(device_t dev,
+			     struct siba_devinfo *dinfo, bus_size_t reg,
+			     uint32_t value, uint32_t mask, u_int usec);
 
 							     
 /* Sonics configuration register blocks */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201711272213.vARMDVit099232>