diff mbox series

[2/5] bus: ti-sysc: Use optional clocks on for enable and wait for softreset bit

Message ID 20200531193941.13179-3-tony@atomide.com (mailing list archive)
State New, archived
Headers show
Series Suspend and resume fixes for omapdrm pdata removal | expand

Commit Message

Tony Lindgren May 31, 2020, 7:39 p.m. UTC 
Some modules reset automatically when idled, and when re-enabled, we must
wait for the automatic OCP softreset to complete. And if optional clocks
are configured, we need to keep the clocks on while waiting for the reset
to complete.

Let's fix the issue by moving the OCP softreset code to a separate
function sysc_wait_softreset(), and call it also from sysc_enable_module()
with the optional clocks enabled.

This is based on what we're already doing for legacy platform data booting
in _enable_sysc().

Fixes: 7324a7a0d5e2 ("bus: ti-sysc: Implement display subsystem reset quirk")
Reported-by: Faiz Abbas <faiz_abbas@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/bus/ti-sysc.c | 81 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 61 insertions(+), 20 deletions(-)

Comments

kernel test robot June 1, 2020, 2:19 a.m. UTC | #1 
Hi Tony,

I love your patch! Perhaps something to improve:

[auto build test WARNING on omap/for-next]
[also build test WARNING on balbi-usb/testing/next char-misc/char-misc-testing staging/staging-testing linus/master v5.7 next-20200529]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/Suspend-and-resume-fixes-for-omapdrm-pdata-removal/20200601-050321
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git for-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

drivers/bus/ti-sysc.c: In function 'sysc_wait_softreset':
>> drivers/bus/ti-sysc.c:228:6: warning: variable 'sysc_offset' set but not used [-Wunused-but-set-variable]
228 |  int sysc_offset, syss_offset, error = 0;
|      ^~~~~~~~~~~

vim +/sysc_offset +228 drivers/bus/ti-sysc.c

   223	
   224	/* Poll on reset status */
   225	static int sysc_wait_softreset(struct sysc *ddata)
   226	{
   227		u32 sysc_mask, syss_done, rstval;
 > 228		int sysc_offset, syss_offset, error = 0;
   229	
   230		sysc_offset = ddata->offsets[SYSC_SYSCONFIG];
   231		syss_offset = ddata->offsets[SYSC_SYSSTATUS];
   232		sysc_mask = BIT(ddata->cap->regbits->srst_shift);
   233	
   234		if (ddata->cfg.quirks & SYSS_QUIRK_RESETDONE_INVERTED)
   235			syss_done = 0;
   236		else
   237			syss_done = ddata->cfg.syss_mask;
   238	
   239		if (syss_offset >= 0) {
   240			error = readx_poll_timeout(sysc_read_sysstatus, ddata, rstval,
   241						   (rstval & ddata->cfg.syss_mask) ==
   242						   syss_done,
   243						   100, MAX_MODULE_SOFTRESET_WAIT);
   244	
   245		} else if (ddata->cfg.quirks & SYSC_QUIRK_RESET_STATUS) {
   246			error = readx_poll_timeout(sysc_read_sysconfig, ddata, rstval,
   247						   !(rstval & sysc_mask),
   248						   100, MAX_MODULE_SOFTRESET_WAIT);
   249		}
   250	
   251		return error;
   252	}
   253	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Tony Lindgren June 1, 2020, 3:01 p.m. UTC | #2 
* kbuild test robot <lkp@intel.com> [200601 02:21]:
> drivers/bus/ti-sysc.c: In function 'sysc_wait_softreset':
> >> drivers/bus/ti-sysc.c:228:6: warning: variable 'sysc_offset' set but not used [-Wunused-but-set-variable]
> 228 |  int sysc_offset, syss_offset, error = 0;
> |      ^~~~~~~~~~~

Oh thanks, I'll just drop sysc_offset from the patch then. Sounds like
I need to also update my cross compiler too to see these warnings.

Regards,

Tony
diff mbox series

Patch

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -221,6 +221,36 @@  static u32 sysc_read_sysstatus(struct sysc *ddata)
 	return sysc_read(ddata, offset);
 }
 
+/* Poll on reset status */
+static int sysc_wait_softreset(struct sysc *ddata)
+{
+	u32 sysc_mask, syss_done, rstval;
+	int sysc_offset, syss_offset, error = 0;
+
+	sysc_offset = ddata->offsets[SYSC_SYSCONFIG];
+	syss_offset = ddata->offsets[SYSC_SYSSTATUS];
+	sysc_mask = BIT(ddata->cap->regbits->srst_shift);
+
+	if (ddata->cfg.quirks & SYSS_QUIRK_RESETDONE_INVERTED)
+		syss_done = 0;
+	else
+		syss_done = ddata->cfg.syss_mask;
+
+	if (syss_offset >= 0) {
+		error = readx_poll_timeout(sysc_read_sysstatus, ddata, rstval,
+					   (rstval & ddata->cfg.syss_mask) ==
+					   syss_done,
+					   100, MAX_MODULE_SOFTRESET_WAIT);
+
+	} else if (ddata->cfg.quirks & SYSC_QUIRK_RESET_STATUS) {
+		error = readx_poll_timeout(sysc_read_sysconfig, ddata, rstval,
+					   !(rstval & sysc_mask),
+					   100, MAX_MODULE_SOFTRESET_WAIT);
+	}
+
+	return error;
+}
+
 static int sysc_add_named_clock_from_child(struct sysc *ddata,
 					   const char *name,
 					   const char *optfck_name)
@@ -925,8 +955,34 @@  static int sysc_enable_module(struct device *dev)
 	struct sysc *ddata;
 	const struct sysc_regbits *regbits;
 	u32 reg, idlemodes, best_mode;
+	int error;
 
 	ddata = dev_get_drvdata(dev);
+
+	/*
+	 * Some modules like DSS reset automatically on idle. Enable optional
+	 * reset clocks and wait for OCP softreset to complete.
+	 */
+	if (ddata->cfg.quirks & SYSC_QUIRK_OPT_CLKS_IN_RESET) {
+		error = sysc_enable_opt_clocks(ddata);
+		if (error) {
+			dev_err(ddata->dev,
+				"Optional clocks failed for enable: %i\n",
+				error);
+			return error;
+		}
+	}
+	error = sysc_wait_softreset(ddata);
+	if (error)
+		dev_warn(ddata->dev, "OCP softreset timed out\n");
+	if (ddata->cfg.quirks & SYSC_QUIRK_OPT_CLKS_IN_RESET)
+		sysc_disable_opt_clocks(ddata);
+
+	/*
+	 * Some subsystem private interconnects, like DSS top level module,
+	 * need only the automatic OCP softreset handling with no sysconfig
+	 * register bits to configure.
+	 */
 	if (ddata->offsets[SYSC_SYSCONFIG] == -ENODEV)
 		return 0;
 
@@ -1828,11 +1884,10 @@  static int sysc_legacy_init(struct sysc *ddata)
  */
 static int sysc_reset(struct sysc *ddata)
 {
-	int sysc_offset, syss_offset, sysc_val, rstval, error = 0;
-	u32 sysc_mask, syss_done;
+	int sysc_offset, sysc_val, error;
+	u32 sysc_mask;
 
 	sysc_offset = ddata->offsets[SYSC_SYSCONFIG];
-	syss_offset = ddata->offsets[SYSC_SYSSTATUS];
 
 	if (ddata->legacy_mode ||
 	    ddata->cap->regbits->srst_shift < 0 ||
@@ -1841,11 +1896,6 @@  static int sysc_reset(struct sysc *ddata)
 
 	sysc_mask = BIT(ddata->cap->regbits->srst_shift);
 
-	if (ddata->cfg.quirks & SYSS_QUIRK_RESETDONE_INVERTED)
-		syss_done = 0;
-	else
-		syss_done = ddata->cfg.syss_mask;
-
 	if (ddata->pre_reset_quirk)
 		ddata->pre_reset_quirk(ddata);
 
@@ -1862,18 +1912,9 @@  static int sysc_reset(struct sysc *ddata)
 	if (ddata->post_reset_quirk)
 		ddata->post_reset_quirk(ddata);
 
-	/* Poll on reset status */
-	if (syss_offset >= 0) {
-		error = readx_poll_timeout(sysc_read_sysstatus, ddata, rstval,
-					   (rstval & ddata->cfg.syss_mask) ==
-					   syss_done,
-					   100, MAX_MODULE_SOFTRESET_WAIT);
-
-	} else if (ddata->cfg.quirks & SYSC_QUIRK_RESET_STATUS) {
-		error = readx_poll_timeout(sysc_read_sysconfig, ddata, rstval,
-					   !(rstval & sysc_mask),
-					   100, MAX_MODULE_SOFTRESET_WAIT);
-	}
+	error = sysc_wait_softreset(ddata);
+	if (error)
+		dev_warn(ddata->dev, "OCP softreset timed out\n");
 
 	if (ddata->reset_done_quirk)
 		ddata->reset_done_quirk(ddata);