All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: vz@mleia.com
Cc: linux-clk@vger.kernel.org
Subject: re: clk: lpc32xx: add common clock framework driver
Date: Tue, 5 Jan 2016 14:44:52 +0300	[thread overview]
Message-ID: <20160105114452.GA31494@mwanda> (raw)

Hello Vladimir Zapolskiy (and other clk devs as well),

The patch f7c82a60ba26: "clk: lpc32xx: add common clock framework
driver" from Dec 6, 2015, leads to the following static checker
warning:

	drivers/clk/nxp/clk-lpc32xx.c:1019 clk_mux_get_parent()
	warn: signedness bug returning '(-22)'

drivers/clk/nxp/clk-lpc32xx.c
  1003  static u8 clk_mux_get_parent(struct clk_hw *hw)
               ^^
  1004  {
  1005          struct lpc32xx_clk_mux *mux = to_lpc32xx_mux(hw);
  1006          u32 num_parents = clk_hw_get_num_parents(hw);
  1007          u32 val;
  1008  
  1009          regmap_read(clk_regmap, mux->reg, &val);
  1010          val >>= mux->shift;
  1011          val &= mux->mask;
  1012  
  1013          if (mux->table) {
  1014                  u32 i;
  1015  
  1016                  for (i = 0; i < num_parents; i++)
  1017                          if (mux->table[i] == val)
  1018                                  return i;
  1019                  return -EINVAL;
                        ^^^^^^^^^^^^^^
  1020          }
  1021  
  1022          if (val >= num_parents)
  1023                  return -EINVAL;
                        ^^^^^^^^^^^^^^
This obviously doesn't work since this is a u8 function.

I looked at how this was called to see what we should return on error.
This is called from __clk_init().  It tests for negative error codes.
Ooops.  Also there is no static checker warning for that but I will
create one.  I'm not sure what to do here.  I bet the correct thing is
to convert all the get_parent() function to return int.  Another option
might be to add documentation, but I feel like in the kernel int returns
are self documenting and that's the best option.

  1024  
  1025          return val;
  1026  }

regards,
dan carpenter

             reply	other threads:[~2016-01-05 11:45 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-05 11:44 Dan Carpenter [this message]
2016-01-08  4:02 ` clk: lpc32xx: add common clock framework driver Vladimir Zapolskiy

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=20160105114452.GA31494@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=vz@mleia.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.