All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] mmc: bcm2835: Add new driver for the sdhost controller.
@ 2017-03-21 20:14 ` Dan Carpenter
  2017-03-21 22:25   ` Jaehoon Chung
  2017-03-24  7:24   ` Ulf Hansson
  0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-03-21 20:14 UTC (permalink / raw)
  To: eric; +Cc: linux-mmc

Hello Eric Anholt,

This is a semi-automatic email about new static checker warnings.

The patch 5aa25d971ab4: "mmc: bcm2835: Add new driver for the sdhost 
controller." from Mar 8, 2017, leads to the following Smatch 
complaint:

drivers/mmc/host/bcm2835.c:1203 bcm2835_request()
	 error: we previously assumed 'host->mrq->data' could be null (see line 1186)

drivers/mmc/host/bcm2835.c
  1156  static void bcm2835_request(struct mmc_host *mmc, struct mmc_request *mrq)
  1157  {
  1158          struct bcm2835_host *host = mmc_priv(mmc);
  1159          struct device *dev = &host->pdev->dev;
  1160          u32 edm, fsm;
  1161  
  1162          /* Reset the error statuses in case this is a retry */
  1163          if (mrq->sbc)
  1164                  mrq->sbc->error = 0;
  1165          if (mrq->cmd)
  1166                  mrq->cmd->error = 0;
  1167          if (mrq->data)
  1168                  mrq->data->error = 0;
  1169          if (mrq->stop)
  1170                  mrq->stop->error = 0;
  1171  
  1172          if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
                    ^^^^^^^^^
Tested here.

  1173                  dev_err(dev, "unsupported block size (%d bytes)\n",
  1174                          mrq->data->blksz);
  1175                  mrq->cmd->error = -EINVAL;
  1176                  mmc_request_done(mmc, mrq);
  1177                  return;
  1178          }
  1179  
  1180          if (host->use_dma && mrq->data && (mrq->data->blocks > PIO_THRESHOLD))
                                     ^^^^^^^^^
And here.

  1181                  bcm2835_prepare_dma(host, mrq->data);
  1182  
  1183          mutex_lock(&host->mutex);
  1184  
  1185		WARN_ON(host->mrq);
  1186		host->mrq = mrq;

Now host->mrq->data is possibly NULL.

  1187	
  1188		edm = readl(host->ioaddr + SDEDM);
  1189		fsm = edm & SDEDM_FSM_MASK;
  1190	
  1191		if ((fsm != SDEDM_FSM_IDENTMODE) &&
  1192		    (fsm != SDEDM_FSM_DATAMODE)) {
  1193			dev_err(dev, "previous command (%d) not complete (EDM %08x)\n",
  1194				readl(host->ioaddr + SDCMD) & SDCMD_CMD_MASK,
  1195				edm);
  1196			bcm2835_dumpregs(host);
  1197			mrq->cmd->error = -EILSEQ;
  1198			bcm2835_finish_request(host);
  1199			mutex_unlock(&host->mutex);
  1200			return;
  1201		}
  1202	
  1203		host->use_sbc = !!mrq->sbc && (host->mrq->data->flags & MMC_DATA_READ);
                                               ^^^^^^^^^^^^^^^^^
Dereferenced without a test.

  1204		if (host->use_sbc) {
  1205			if (bcm2835_send_command(host, mrq->sbc)) {

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] mmc: bcm2835: Add new driver for the sdhost controller.
  2017-03-21 20:14 ` [bug report] mmc: bcm2835: Add new driver for the sdhost controller Dan Carpenter
@ 2017-03-21 22:25   ` Jaehoon Chung
  2017-03-24  7:24   ` Ulf Hansson
  1 sibling, 0 replies; 3+ messages in thread
From: Jaehoon Chung @ 2017-03-21 22:25 UTC (permalink / raw)
  To: Dan Carpenter, eric; +Cc: linux-mmc

On 03/22/2017 05:14 AM, Dan Carpenter wrote:
> Hello Eric Anholt,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 5aa25d971ab4: "mmc: bcm2835: Add new driver for the sdhost 
> controller." from Mar 8, 2017, leads to the following Smatch 
> complaint:
> 
> drivers/mmc/host/bcm2835.c:1203 bcm2835_request()
> 	 error: we previously assumed 'host->mrq->data' could be null (see line 1186)
> 
> drivers/mmc/host/bcm2835.c
>   1156  static void bcm2835_request(struct mmc_host *mmc, struct mmc_request *mrq)
>   1157  {
>   1158          struct bcm2835_host *host = mmc_priv(mmc);
>   1159          struct device *dev = &host->pdev->dev;
>   1160          u32 edm, fsm;
>   1161  
>   1162          /* Reset the error statuses in case this is a retry */
>   1163          if (mrq->sbc)
>   1164                  mrq->sbc->error = 0;
>   1165          if (mrq->cmd)
>   1166                  mrq->cmd->error = 0;
>   1167          if (mrq->data)
>   1168                  mrq->data->error = 0;
>   1169          if (mrq->stop)
>   1170                  mrq->stop->error = 0;
>   1171  
>   1172          if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
>                     ^^^^^^^^^
> Tested here.
> 
>   1173                  dev_err(dev, "unsupported block size (%d bytes)\n",
>   1174                          mrq->data->blksz);
>   1175                  mrq->cmd->error = -EINVAL;
>   1176                  mmc_request_done(mmc, mrq);
>   1177                  return;
>   1178          }
>   1179  
>   1180          if (host->use_dma && mrq->data && (mrq->data->blocks > PIO_THRESHOLD))
>                                      ^^^^^^^^^
> And here.
> 
>   1181                  bcm2835_prepare_dma(host, mrq->data);
>   1182  
>   1183          mutex_lock(&host->mutex);
>   1184  
>   1185		WARN_ON(host->mrq);
>   1186		host->mrq = mrq;
> 
> Now host->mrq->data is possibly NULL.
> 
>   1187	
>   1188		edm = readl(host->ioaddr + SDEDM);
>   1189		fsm = edm & SDEDM_FSM_MASK;
>   1190	
>   1191		if ((fsm != SDEDM_FSM_IDENTMODE) &&
>   1192		    (fsm != SDEDM_FSM_DATAMODE)) {
>   1193			dev_err(dev, "previous command (%d) not complete (EDM %08x)\n",
>   1194				readl(host->ioaddr + SDCMD) & SDCMD_CMD_MASK,
>   1195				edm);
>   1196			bcm2835_dumpregs(host);
>   1197			mrq->cmd->error = -EILSEQ;
>   1198			bcm2835_finish_request(host);
>   1199			mutex_unlock(&host->mutex);
>   1200			return;
>   1201		}
>   1202	
>   1203		host->use_sbc = !!mrq->sbc && (host->mrq->data->flags & MMC_DATA_READ);
>                                                ^^^^^^^^^^^^^^^^^
> Dereferenced without a test.

I think that it can be just added the condition checking for host->mrq->data.

Best Regards,
Jaehoon Chung

> 
>   1204		if (host->use_sbc) {
>   1205			if (bcm2835_send_command(host, mrq->sbc)) {
> 
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] mmc: bcm2835: Add new driver for the sdhost controller.
  2017-03-21 20:14 ` [bug report] mmc: bcm2835: Add new driver for the sdhost controller Dan Carpenter
  2017-03-21 22:25   ` Jaehoon Chung
@ 2017-03-24  7:24   ` Ulf Hansson
  1 sibling, 0 replies; 3+ messages in thread
From: Ulf Hansson @ 2017-03-24  7:24 UTC (permalink / raw)
  To: Eric Anholt, Gerd Hoffmann; +Cc: linux-mmc, Dan Carpenter

+Gerd

On 21 March 2017 at 21:14, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Eric Anholt,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 5aa25d971ab4: "mmc: bcm2835: Add new driver for the sdhost
> controller." from Mar 8, 2017, leads to the following Smatch
> complaint:
>
> drivers/mmc/host/bcm2835.c:1203 bcm2835_request()
>          error: we previously assumed 'host->mrq->data' could be null (see line 1186)
>
> drivers/mmc/host/bcm2835.c
>   1156  static void bcm2835_request(struct mmc_host *mmc, struct mmc_request *mrq)
>   1157  {
>   1158          struct bcm2835_host *host = mmc_priv(mmc);
>   1159          struct device *dev = &host->pdev->dev;
>   1160          u32 edm, fsm;
>   1161
>   1162          /* Reset the error statuses in case this is a retry */
>   1163          if (mrq->sbc)
>   1164                  mrq->sbc->error = 0;
>   1165          if (mrq->cmd)
>   1166                  mrq->cmd->error = 0;
>   1167          if (mrq->data)
>   1168                  mrq->data->error = 0;
>   1169          if (mrq->stop)
>   1170                  mrq->stop->error = 0;
>   1171
>   1172          if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
>                     ^^^^^^^^^
> Tested here.
>
>   1173                  dev_err(dev, "unsupported block size (%d bytes)\n",
>   1174                          mrq->data->blksz);
>   1175                  mrq->cmd->error = -EINVAL;
>   1176                  mmc_request_done(mmc, mrq);
>   1177                  return;
>   1178          }
>   1179
>   1180          if (host->use_dma && mrq->data && (mrq->data->blocks > PIO_THRESHOLD))
>                                      ^^^^^^^^^
> And here.
>
>   1181                  bcm2835_prepare_dma(host, mrq->data);
>   1182
>   1183          mutex_lock(&host->mutex);
>   1184
>   1185          WARN_ON(host->mrq);
>   1186          host->mrq = mrq;
>
> Now host->mrq->data is possibly NULL.
>
>   1187
>   1188          edm = readl(host->ioaddr + SDEDM);
>   1189          fsm = edm & SDEDM_FSM_MASK;
>   1190
>   1191          if ((fsm != SDEDM_FSM_IDENTMODE) &&
>   1192              (fsm != SDEDM_FSM_DATAMODE)) {
>   1193                  dev_err(dev, "previous command (%d) not complete (EDM %08x)\n",
>   1194                          readl(host->ioaddr + SDCMD) & SDCMD_CMD_MASK,
>   1195                          edm);
>   1196                  bcm2835_dumpregs(host);
>   1197                  mrq->cmd->error = -EILSEQ;
>   1198                  bcm2835_finish_request(host);
>   1199                  mutex_unlock(&host->mutex);
>   1200                  return;
>   1201          }
>   1202
>   1203          host->use_sbc = !!mrq->sbc && (host->mrq->data->flags & MMC_DATA_READ);
>                                                ^^^^^^^^^^^^^^^^^
> Dereferenced without a test.
>
>   1204          if (host->use_sbc) {
>   1205                  if (bcm2835_send_command(host, mrq->sbc)) {
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Gerd, Eric - can you please look into posting a fix for this!?

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-24  7:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20170321201612epcas3p3b8f6908c50556c7d91c3bce97837a92c@epcas3p3.samsung.com>
2017-03-21 20:14 ` [bug report] mmc: bcm2835: Add new driver for the sdhost controller Dan Carpenter
2017-03-21 22:25   ` Jaehoon Chung
2017-03-24  7:24   ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.