Re: [PATCH v3 1/2] mmc: Add SD/SDIO driver for Sunplus SP7021

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 12049 bytes --]

Hi Li-hao,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pza/reset/next]
[also build test WARNING on robh/for-next ulf-hansson-mmc-mirror/next linus/master v5.16-rc7 next-20211224]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Li-hao-Kuo/Add-SD-SDIO-control-driver-for-Sunplus-SP7021-SoC/20211224-140704
base:   https://git.pengutronix.de/git/pza/linux reset/next
config: sparc64-randconfig-c003-20211228 (https://download.01.org/0day-ci/archive/20211230/202112300248.VVmtGRgf-lkp(a)intel.com/config)
compiler: sparc64-linux-gcc (GCC) 11.2.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
        # https://github.com/0day-ci/linux/commit/16ea62b9e9561c6054da687edb9f8603d1e7a158
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Li-hao-Kuo/Add-SD-SDIO-control-driver-for-Sunplus-SP7021-SoC/20211224-140704
        git checkout 16ea62b9e9561c6054da687edb9f8603d1e7a158
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc64 SHELL=/bin/bash drivers/mmc/host/

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

All warnings (new ones prefixed by >>):

   drivers/mmc/host/sunplus_sd2.c: In function 'spsdc_set_bus_timing':
>> drivers/mmc/host/sunplus_sd2.c:280:15: warning: variable 'timing_name' set but not used [-Wunused-but-set-variable]
     280 |         char *timing_name;
         |               ^~~~~~~~~~~
   drivers/mmc/host/sunplus_sd2.c: In function 'spsdc_set_bus_width':
>> drivers/mmc/host/sunplus_sd2.c:300:13: warning: variable 'bus_width' set but not used [-Wunused-but-set-variable]
     300 |         int bus_width;
         |             ^~~~~~~~~
   drivers/mmc/host/sunplus_sd2.c: At top level:
>> drivers/mmc/host/sunplus_sd2.c:743:13: warning: no previous prototype for 'spsdc_irq' [-Wmissing-prototypes]
     743 | irqreturn_t spsdc_irq(int irq, void *dev_id)
         |             ^~~~~~~~~
>> drivers/mmc/host/sunplus_sd2.c:849:5: warning: no previous prototype for 'spsdc_get_cd' [-Wmissing-prototypes]
     849 | int spsdc_get_cd(struct mmc_host *mmc)
         |     ^~~~~~~~~~~~
   drivers/mmc/host/sunplus_sd2.c: In function 'spsdc_drv_probe':
>> drivers/mmc/host/sunplus_sd2.c:980:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     980 |         mode = (int)of_device_get_match_data(&pdev->dev);
         |                ^
   In file included from include/linux/notifier.h:14,
                    from include/linux/clk.h:14,
                    from drivers/mmc/host/sunplus_sd2.c:8:
   drivers/mmc/host/sunplus_sd2.c: In function 'spsdc_request':
>> include/linux/mutex.h:188:40: warning: ignoring return value of 'mutex_lock_interruptible_nested' declared with attribute 'warn_unused_result' [-Wunused-result]
     188 | #define mutex_lock_interruptible(lock) mutex_lock_interruptible_nested(lock, 0)
         |                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/mmc/host/sunplus_sd2.c:775:9: note: in expansion of macro 'mutex_lock_interruptible'
     775 |         mutex_lock_interruptible(&host->mrq_lock);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
--
   drivers/mmc/host/sunplus_sd2.c:175: warning: Function parameter or member 'host' not described in 'spsdc_wait_finish'
>> drivers/mmc/host/sunplus_sd2.c:175: warning: expecting prototype for wait for transaction done, return(). Prototype was for spsdc_wait_finish() instead
   drivers/mmc/host/sunplus_sd2.c:326: warning: Function parameter or member 'host' not described in 'spsdc_select_mode'
   drivers/mmc/host/sunplus_sd2.c:326: warning: Function parameter or member 'mode' not described in 'spsdc_select_mode'
>> drivers/mmc/host/sunplus_sd2.c:326: warning: expecting prototype for select the working mode of controller(). Prototype was for spsdc_select_mode() instead
>> drivers/mmc/host/sunplus_sd2.c:563: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * check if error during transaction.
   drivers/mmc/host/sunplus_sd2.c:705: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * 1. unmap scatterlist if needed;
   drivers/mmc/host/sunplus_sd2.c:850: warning: Function parameter or member 'mmc' not described in 'spsdc_get_cd'
>> drivers/mmc/host/sunplus_sd2.c:850: warning: expecting prototype for Return values for the get_cd callback should be(). Prototype was for spsdc_get_cd() instead


vim +/timing_name +280 drivers/mmc/host/sunplus_sd2.c

   170	
   171	/**
   172	 * wait for transaction done, return -1 if error.
   173	 */
   174	static inline int spsdc_wait_finish(struct spsdc_host *host)
 > 175	{
   176		/* Wait for transaction finish */
   177		unsigned long timeout = jiffies + msecs_to_jiffies(5000);
   178	
   179		while (!time_after(jiffies, timeout)) {
   180			if (readl(host->base + SPSD2_SD_STATE_REG) & SPSDC_STATE_FINISH)
   181				return 0;
   182			if (readl(host->base + SPSD2_SD_STATE_REG) & SPSDC_STATE_ERROR)
   183				return -1;
   184		}
   185		return -1;
   186	}
   187	
   188	static inline int spsdc_wait_sdstatus(struct spsdc_host *host, unsigned int status_bit)
   189	{
   190		unsigned long timeout = jiffies + msecs_to_jiffies(5000);
   191	
   192		while (!time_after(jiffies, timeout)) {
   193			if (readl(host->base + SPSD2_SD_STATUS_REG) & status_bit)
   194				return 0;
   195			if (readl(host->base + SPSD2_SD_STATE_REG) & SPSDC_STATE_ERROR)
   196				return -1;
   197		}
   198		return -1;
   199	}
   200	
   201	#define spsdc_wait_rspbuf_full(host) spsdc_wait_sdstatus(host, SPSDC_STS_RSP_BUF_FULL)
   202	#define spsdc_wait_rxbuf_full(host) spsdc_wait_sdstatus(host, SPSDC_STS_RX_BUF_FULL)
   203	#define spsdc_wait_txbuf_empty(host) spsdc_wait_sdstatus(host, SPSDC_STS_TX_BUF_EMP)
   204	
   205	static void spsdc_get_rsp(struct spsdc_host *host, struct mmc_command *cmd)
   206	{
   207		u32 value0_3, value4_5;
   208	
   209		if (unlikely(!(cmd->flags & MMC_RSP_PRESENT)))
   210			return;
   211		if (unlikely(cmd->flags & MMC_RSP_136)) {
   212			if (spsdc_wait_rspbuf_full(host))
   213				return;
   214			value0_3 = readl(host->base + SPSD2_SD_RSP_BUF0_3_REG);
   215			value4_5 = readl(host->base + SPSD2_SD_RSP_BUF4_5_REG) & 0xffff;
   216			cmd->resp[0] = (value0_3 << 8) | (value4_5 >> 8);
   217			cmd->resp[1] = value4_5 << 24;
   218			if (spsdc_wait_rspbuf_full(host))
   219				return;
   220			value0_3 = readl(host->base + SPSD2_SD_RSP_BUF0_3_REG);
   221			value4_5 = readl(host->base + SPSD2_SD_RSP_BUF4_5_REG) & 0xffff;
   222			cmd->resp[1] |= value0_3 >> 8;
   223			cmd->resp[2] = value0_3 << 24;
   224			cmd->resp[2] |= value4_5 << 8;
   225			if (spsdc_wait_rspbuf_full(host))
   226				return;
   227			value0_3 = readl(host->base + SPSD2_SD_RSP_BUF0_3_REG);
   228			value4_5 = readl(host->base + SPSD2_SD_RSP_BUF4_5_REG) & 0xffff;
   229			cmd->resp[2] |= value0_3 >> 24;
   230			cmd->resp[3] = value0_3 << 8;
   231			cmd->resp[3] |= value4_5 >> 8;
   232		} else {
   233			if (spsdc_wait_rspbuf_full(host))
   234				return;
   235			value0_3 = readl(host->base + SPSD2_SD_RSP_BUF0_3_REG);
   236			value4_5 = readl(host->base + SPSD2_SD_RSP_BUF4_5_REG) & 0xffff;
   237			cmd->resp[0] = (value0_3 << 8) | (value4_5 >> 8);
   238			cmd->resp[1] = value4_5 << 24;
   239		}
   240	}
   241	
   242	static void spsdc_set_bus_clk(struct spsdc_host *host, int clk)
   243	{
   244		unsigned int clkdiv;
   245		int f_min = host->mmc->f_min;
   246		int f_max = host->mmc->f_max;
   247		u32 value = readl(host->base + SPSD2_SD_CONF_REG);
   248	
   249		if (clk < f_min)
   250			clk = f_min;
   251		if (clk > f_max)
   252			clk = f_max;
   253	
   254		// SD 2.0 only max set to 50Mhz CLK
   255		if (clk >= SPSDC_50M_CLK)
   256			clk = f_max;
   257	
   258		clkdiv = (clk_get_rate(host->clk) + clk) / clk - 1;
   259		if (clkdiv > 0xfff)
   260			clkdiv = 0xfff;
   261	
   262		value &= ~SPSDC_CONF_CLK_DIV;
   263		value |= FIELD_PREP(SPSDC_CONF_CLK_DIV, clkdiv);
   264		writel(value, host->base + SPSD2_SD_CONF_REG);
   265		/* In order to reduce the frequency of context switch,
   266		 * if it is high speed or upper, we do not use interrupt
   267		 * when send a command that without data.
   268		 */
   269		if (clk > 25000000)
   270			host->use_int = 0;
   271		else
   272			host->use_int = 1;
   273	}
   274	
   275	static void spsdc_set_bus_timing(struct spsdc_host *host, unsigned int timing)
   276	{
   277		u32 value = readl(host->base + SPSD2_SD_TIMING_CONF0_REG);
   278		int clkdiv = FIELD_GET(SPSDC_CONF_CLK_DIV, readl(host->base + SPSD2_SD_CONF_REG));
   279		int delay = (clkdiv / 2 < 7) ? clkdiv / 2 : 7;
 > 280		char *timing_name;
   281	
   282		switch (timing) {
   283		case MMC_TIMING_LEGACY:
   284			value &= ~SPSDC_TIMING_CONF0_HS_EN;
   285			timing_name = "legacy";
   286			break;
   287		case MMC_TIMING_SD_HS:
   288		case MMC_TIMING_MMC_HS:
   289			value |= SPSDC_TIMING_CONF0_HS_EN |
   290				FIELD_PREP(SPSDC_TIMING_CONF0_WRTD, delay);
   291			timing_name = "hs";
   292			break;
   293		}
   294		writel(value, host->base + SPSD2_SD_TIMING_CONF0_REG);
   295	}
   296	
   297	static void spsdc_set_bus_width(struct spsdc_host *host, int width)
   298	{
   299		u32 value = readl(host->base + SPSD2_SD_CONF_REG);
 > 300		int bus_width;
   301	
   302		switch (width) {
   303		case MMC_BUS_WIDTH_8:
   304			value &= ~SPSDC_CONF_4BIT_MODE;
   305			value |= SPSDC_CONF_MMC8BIT;
   306			bus_width = 8;
   307			break;
   308		case MMC_BUS_WIDTH_4:
   309			value |= SPSDC_CONF_4BIT_MODE;
   310			value &= ~SPSDC_CONF_MMC8BIT;
   311			bus_width = 4;
   312			break;
   313		default:
   314			value &= ~SPSDC_CONF_4BIT_MODE;
   315			value &= ~SPSDC_CONF_MMC8BIT;
   316			bus_width = 1;
   317			break;
   318		};
   319		writel(value, host->base + SPSD2_SD_CONF_REG);
   320	}
   321	
   322	/**
   323	 * select the working mode of controller: sd/sdio/emmc
   324	 */
   325	static void spsdc_select_mode(struct spsdc_host *host, int mode)
 > 326	{
   327		u32 value = readl(host->base + SPSD2_SD_CONF_REG);
   328	
   329		host->mode = mode;
   330		/* set `sdmmcmode', as it will sample data at fall edge
   331		 * of SD bus clock if `sdmmcmode' is not set when
   332		 * `sd_high_speed_en' is not set, which is not compliant
   333		 * with SD specification
   334		 */
   335		value |= SPSDC_CONF_SD_MODE;
   336		switch (mode) {
   337		case SPSDC_MODE_EMMC:
   338			value &= ~SPSDC_CONF_SDIO_MODE;
   339			writel(value, host->base + SPSD2_SD_CONF_REG);
   340			break;
   341		case SPSDC_MODE_SDIO:
   342			value |= SPSDC_CONF_SDIO_MODE;
   343			writel(value, host->base + SPSD2_SD_CONF_REG);
   344			value = readl(host->base + SPSD2_SDIO_CTRL_REG);
   345			value |= SPSDC_SDIO_CTRL_MULTI_TRIG;
   346			writel(value, host->base + SPSD2_SDIO_CTRL_REG);
   347			break;
   348		case SPSDC_MODE_SD:
   349		default:
   350			value &= ~SPSDC_CONF_SDIO_MODE;
   351			host->mode = SPSDC_MODE_SD;
   352			writel(value, host->base + SPSD2_SD_CONF_REG);
   353			break;
   354		}
   355	}
   356	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org