linux-mediatek.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: blogic@openwrt.org
Cc: devel@driverdev.osuosl.org, NeilBrown <neil@brown.name>,
	linux-mediatek@lists.infradead.org
Subject: [bug report] staging: mt7621-mmc: MIPS: ralink: add sdhci for mt7620a SoC
Date: Thu, 5 Apr 2018 15:13:10 +0300	[thread overview]
Message-ID: <20180405121310.GA7782@mwanda> (raw)

[ I just decided to forward you guys all the Smatch warnings.  -dan ]

Hello John Crispin,

The patch 8b634a9c7620: "staging: mt7621-mmc: MIPS: ralink: add sdhci
for mt7620a SoC" from Mar 15, 2018, leads to the following static
checker warning:

	drivers/staging/mt7621-mmc/sd.c:951 msdc_command_start()
	warn: we tested 'opcode == 3' before and it was 'false'

drivers/staging/mt7621-mmc/sd.c
   931  static unsigned int msdc_command_start(struct msdc_host   *host, 
   932                                        struct mmc_command *cmd,
   933                                        int                 tune,   /* not used */
   934                                        unsigned long       timeout)
   935  {
   936      u32 base = host->base;
   937      u32 opcode = cmd->opcode;
   938      u32 rawcmd;
   939      u32 wints = MSDC_INT_CMDRDY  | MSDC_INT_RSPCRCERR  | MSDC_INT_CMDTMO  |  
   940                  MSDC_INT_ACMDRDY | MSDC_INT_ACMDCRCERR | MSDC_INT_ACMDTMO | 
   941                  MSDC_INT_ACMD19_DONE;  
   942                     
   943      u32 resp;  
   944      unsigned long tmo;
   945  
   946      /* Protocol layer does not provide response type, but our hardware needs 
   947       * to know exact type, not just size!
   948       */
   949      if (opcode == MMC_SEND_OP_COND || opcode == SD_APP_OP_COND)
   950          resp = RESP_R3;
   951      else if (opcode == MMC_SET_RELATIVE_ADDR || opcode == SD_SEND_RELATIVE_ADDR)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
MMC_SET_RELATIVE_ADDR and SD_SEND_RELATIVE_ADDR are both 3 so this
is redundant.

   952          resp = (mmc_cmd_type(cmd) == MMC_CMD_BCR) ? RESP_R6 : RESP_R1;
   953      else if (opcode == MMC_FAST_IO)
   954          resp = RESP_R4;
   955      else if (opcode == MMC_GO_IRQ_STATE)
   956          resp = RESP_R5;
   957      else if (opcode == MMC_SELECT_CARD)
   958          resp = (cmd->arg != 0) ? RESP_R1B : RESP_NONE;
   959      else if (opcode == SD_IO_RW_DIRECT || opcode == SD_IO_RW_EXTENDED)
   960          resp = RESP_R1; /* SDIO workaround. */
   961      else if (opcode == SD_SEND_IF_COND && (mmc_cmd_type(cmd) == MMC_CMD_BCR))
   962          resp = RESP_R1;
   963      else {
   964          switch (mmc_resp_type(cmd)) {

        drivers/staging/mt7621-mmc/sd.c:2961 msdc_drv_suspend()
        warn: variable dereferenced before check 'mmc' (see line 2959)

        drivers/staging/mt7621-mmc/sd.c:2976 msdc_drv_resume()
        warn: variable dereferenced before check 'mmc' (see line 2972)

drivers/staging/mt7621-mmc/sd.c
  2953  /* Fix me: Power Flow */
  2954  #ifdef CONFIG_PM
  2955  static int msdc_drv_suspend(struct platform_device *pdev, pm_message_t state)
  2956  {
  2957      int ret = 0;
  2958      struct mmc_host *mmc = platform_get_drvdata(pdev);
  2959      struct msdc_host *host = mmc_priv(mmc);
                                     ^^^^^^^^^^^^^
Dereference

  2960  
  2961      if (mmc && state.event == PM_EVENT_SUSPEND && (host->hw->flags & MSDC_SYS_SUSPEND)) { /* will set for card */
                ^^^
Check

  2962          msdc_pm(state, (void*)host);
  2963      }
  2964      
  2965      return ret;
  2966  }
  2967
  2968  static int msdc_drv_resume(struct platform_device *pdev)
  2969  {
  2970      int ret = 0;
  2971      struct mmc_host *mmc = platform_get_drvdata(pdev);
  2972      struct msdc_host *host = mmc_priv(mmc);
                                     ^^^^^^^^^^^^
Dereference

  2973      struct pm_message state;
  2974  
  2975      state.event = PM_EVENT_RESUME;
  2976      if (mmc && (host->hw->flags & MSDC_SYS_SUSPEND)) {/* will set for card */
                ^^^
Check

  2977          msdc_pm(state, (void*)host);
  2978      }
  2979  
  2980      /* This mean WIFI not controller by PM */
  2981      
  2982      return ret;
  2983  }

        drivers/staging/mt7621-mmc/dbg.c:270 msdc_debug_proc_write()
        warn: copy_to/from_user() returns a positive value

drivers/staging/mt7621-mmc/dbg.c
   257  static ssize_t msdc_debug_proc_write(struct file *file, 
   258                          const char __user *buf, size_t count, loff_t *data)
   259  {
   260          int ret;
   261          
   262          int cmd, p1, p2;   
   263          int id, zone;
   264          int mode, size;  
   265    
   266          if (count == 0)return -1;
   267          if(count > 255)count = 255;
   268  
   269          ret = copy_from_user(cmd_buf, buf, count);
   270          if (ret < 0)return -1;

This should be:

		if (copy_from_user(cmd_buf, buf, count))
			return -EFAULT;

   271          
   272          cmd_buf[count] = '\0';
   273          printk("msdc Write %s\n", cmd_buf);
   274  

        drivers/staging/mt7621-mmc/dbg.c:339 msdc_debug_proc_init()
        warn: proc file '"msdc_debug"' is world writable

	drivers/staging/mt7621-mmc/dbg.c:341 msdc_debug_proc_init()
	warn: 'de' isn't an ERR_PTR


drivers/staging/mt7621-mmc/dbg.c
   337  int msdc_debug_proc_init(void) 
   338  {       
   339      struct proc_dir_entry *de = proc_create("msdc_debug", 0667, NULL, &msdc_debug_fops);
                                                                     ^
This should probably be a zero instead of a seven.

   340  
   341      if (!de || IS_ERR(de))
                       ^^^^^^^^^^
Remove this.

   342          printk("!! Create MSDC debug PROC fail !!\n");
   343      
   344      return 0 ;
   345  }

regards,
dan carpenter

             reply	other threads:[~2018-04-05 12:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-05 12:13 Dan Carpenter [this message]
2018-04-05 12:26 ` [bug report] staging: mt7621-mmc: MIPS: ralink: add sdhci for mt7620a SoC John Crispin
2018-04-05 16:39 ` Christian Lütke-Stetzkamp
2018-04-06  7:09   ` Dan Carpenter
2018-04-06  7:39     ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2018-04-05 11:56 Dan Carpenter
2018-04-05 12:26 ` John Crispin
2018-04-05 16:46 ` Christian Lütke-Stetzkamp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180405121310.GA7782@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=blogic@openwrt.org \
    --cc=devel@driverdev.osuosl.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=neil@brown.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).