All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: mchehab+huawei@kernel.org
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org
Subject: [bug report] Revert "media: staging: atomisp: Remove driver"
Date: Fri, 26 Jun 2020 13:42:56 +0300	[thread overview]
Message-ID: <20200626104256.GD314359@mwanda> (raw)

Hello Mauro Carvalho Chehab,

The patch ad85094b293e: "Revert "media: staging: atomisp: Remove
driver"" from Apr 19, 2020, leads to the following static checker
warning:

	drivers/staging/media/atomisp/pci/atomisp_acc.c:506 atomisp_acc_load_extensions()
	warn: iterator used outside loop: 'acc_fw'

drivers/staging/media/atomisp/pci/atomisp_acc.c
   446  int atomisp_acc_load_extensions(struct atomisp_sub_device *asd)
   447  {
   448          struct atomisp_acc_fw *acc_fw;
   449          bool ext_loaded = false;
   450          bool continuous = asd->continuous_mode->val &&
   451                            asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW;
   452          int ret = 0, i = -1;
   453          struct atomisp_device *isp = asd->isp;
   454  
   455          if (asd->acc.pipeline || asd->acc.extension_mode)
   456                  return -EBUSY;
   457  
   458          /* Invalidate caches. FIXME: should flush only necessary buffers */
   459          wbinvd();
   460  
   461          list_for_each_entry(acc_fw, &asd->acc.fw, list) {
   462                  if (acc_fw->type != ATOMISP_ACC_FW_LOAD_TYPE_OUTPUT &&
   463                      acc_fw->type != ATOMISP_ACC_FW_LOAD_TYPE_VIEWFINDER)
   464                          continue;
   465  
   466                  for (i = 0; i < ARRAY_SIZE(acc_flag_to_pipe); i++) {
   467                          /* QoS (ACC pipe) acceleration stages are currently
   468                           * allowed only in continuous mode. Skip them for
   469                           * all other modes. */
   470                          if (!continuous &&
   471                              acc_flag_to_pipe[i].flag ==
   472                              ATOMISP_ACC_FW_LOAD_FL_ACC)
   473                                  continue;
   474  
   475                          if (acc_fw->flags & acc_flag_to_pipe[i].flag) {
   476                                  ret = atomisp_css_load_acc_extension(asd,
   477                                                                       acc_fw->fw,
   478                                                                       acc_flag_to_pipe[i].pipe_id,
   479                                                                       acc_fw->type);
   480                                  if (ret)
   481                                          goto error;

The little loop is intended to clean up a partial iteration from this
goto.

   482  
   483                                  ext_loaded = true;
   484                          }
   485                  }
   486  
   487                  ret = atomisp_css_set_acc_parameters(acc_fw);
   488                  if (ret < 0)
   489                          goto error;

And this one.

   490          }
   491  
   492          if (!ext_loaded)
   493                  return ret;
   494  
   495          ret = atomisp_css_update_stream(asd);
   496          if (ret) {
   497                  dev_err(isp->dev, "%s: update stream failed.\n", __func__);
   498                  goto error;

But if we hit this goto then "i" is non-zero and "acc_fw" is a bogus
pointer.

   499          }
   500  
   501          asd->acc.extension_mode = true;
   502          return 0;
   503  
   504  error:
   505          while (--i >= 0) {
   506                  if (acc_fw->flags & acc_flag_to_pipe[i].flag) {
                            ^^^^^^^^^^^^^

Don't we need to check:

	if (!continuous &&
	    acc_flag_to_pipe[i].flag == ATOMISP_ACC_FW_LOAD_FL_ACC)

?

   507                          atomisp_css_unload_acc_extension(asd, acc_fw->fw,
   508                                                           acc_flag_to_pipe[i].pipe_id);
   509                  }
   510          }
   511  
   512          list_for_each_entry_continue_reverse(acc_fw, &asd->acc.fw, list) {
   513                  if (acc_fw->type != ATOMISP_ACC_FW_LOAD_TYPE_OUTPUT &&
   514                      acc_fw->type != ATOMISP_ACC_FW_LOAD_TYPE_VIEWFINDER)
   515                          continue;
   516  
   517                  for (i = ARRAY_SIZE(acc_flag_to_pipe) - 1; i >= 0; i--) {
   518                          if (!continuous &&
   519                              acc_flag_to_pipe[i].flag ==
   520                              ATOMISP_ACC_FW_LOAD_FL_ACC)
   521                                  continue;
   522                          if (acc_fw->flags & acc_flag_to_pipe[i].flag) {
   523                                  atomisp_css_unload_acc_extension(asd,
   524                                                                   acc_fw->fw,
   525                                                                   acc_flag_to_pipe[i].pipe_id);
   526                          }
   527                  }
   528          }
   529          return ret;
   530  }

regards,
dan carpenter

             reply	other threads:[~2020-06-26 10:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-26 10:42 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-03-12  6:43 [bug report] Revert "media: staging: atomisp: Remove driver" Dan Carpenter
2021-03-12  7:24 ` Mauro Carvalho Chehab
2021-03-12 10:08   ` Dan Carpenter
2020-05-29 10:41 Dan Carpenter
2020-05-29 15:36 ` Mauro Carvalho Chehab

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=20200626104256.GD314359@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab+huawei@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /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 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.