| Message ID | 20171107124322.mm3fqkrggw3zidrx@mwanda (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 7/11/2017 20:43, Dan Carpenter wrote: > The original code does this: "1 << (1 << 11)" which is undefined in C. > > Fixes: dbc4deda03fe ("power: Adds support for Smart Battery System Manager") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- >>From static analysis. Not tested. > > diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c > index ccb4217b9638..cb6e8f66c7a2 100644 > --- a/drivers/power/supply/sbs-manager.c > +++ b/drivers/power/supply/sbs-manager.c > @@ -183,7 +183,7 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) > return ret; > > /* chan goes from 1 ... 4 */ > - reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); > + reg = BIT(SBSM_SMB_BAT_OFFSET + chan); > ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); > if (ret) > dev_err(dev, "Failed to select channel %i\n", chan); > > Oops. That's my fault. Reviewed-by: Phil Reid <preid@electromag.com.au>
Hi, On Tue, Nov 07, 2017 at 03:43:22PM +0300, Dan Carpenter wrote: > The original code does this: "1 << (1 << 11)" which is undefined in C. > > Fixes: dbc4deda03fe ("power: Adds support for Smart Battery System Manager") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > From static analysis. Not tested. > > diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c > index ccb4217b9638..cb6e8f66c7a2 100644 > --- a/drivers/power/supply/sbs-manager.c > +++ b/drivers/power/supply/sbs-manager.c > @@ -183,7 +183,7 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) > return ret; > > /* chan goes from 1 ... 4 */ > - reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); > + reg = BIT(SBSM_SMB_BAT_OFFSET + chan); > ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); > if (ret) > dev_err(dev, "Failed to select channel %i\n", chan); Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> sbs-manager has been added to linux-next through the i2c tree due to dependencies, so this one also needs to go through i2c. -- Sebastian
On Mon, Nov 13, 2017 at 11:55:24AM +0100, Sebastian Reichel wrote: > Hi, > > On Tue, Nov 07, 2017 at 03:43:22PM +0300, Dan Carpenter wrote: > > The original code does this: "1 << (1 << 11)" which is undefined in C. > > > > Fixes: dbc4deda03fe ("power: Adds support for Smart Battery System Manager") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > From static analysis. Not tested. > > > > diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c > > index ccb4217b9638..cb6e8f66c7a2 100644 > > --- a/drivers/power/supply/sbs-manager.c > > +++ b/drivers/power/supply/sbs-manager.c > > @@ -183,7 +183,7 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) > > return ret; > > > > /* chan goes from 1 ... 4 */ > > - reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); > > + reg = BIT(SBSM_SMB_BAT_OFFSET + chan); > > ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); > > if (ret) > > dev_err(dev, "Failed to select channel %i\n", chan); > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> > > sbs-manager has been added to linux-next through the i2c tree due to > dependencies, so this one also needs to go through i2c. I want to send Linus my pull request early this time (like today or tomorrow). So, you either apply this patch then yourself afterwards on top of linus' tree. Or I send it to him, but then I'd need the original patch bounced or resent. Both fine with me.
> > > /* chan goes from 1 ... 4 */ > > > - reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); > > > + reg = BIT(SBSM_SMB_BAT_OFFSET + chan); > > > ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); > > > if (ret) > > > dev_err(dev, "Failed to select channel %i\n", chan); > > > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> > > > > sbs-manager has been added to linux-next through the i2c tree due to > > dependencies, so this one also needs to go through i2c. > > I want to send Linus my pull request early this time (like today or > tomorrow). So, you either apply this patch then yourself afterwards on > top of linus' tree. Or I send it to him, but then I'd need the original > patch bounced or resent. Both fine with me. The sbs-manager code is in linus' tree now. How would you like to proceed?
Hi, On Wed, Nov 15, 2017 at 08:45:44AM +0100, Wolfram Sang wrote: > > > > /* chan goes from 1 ... 4 */ > > > > - reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); > > > > + reg = BIT(SBSM_SMB_BAT_OFFSET + chan); > > > > ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); > > > > if (ret) > > > > dev_err(dev, "Failed to select channel %i\n", chan); > > > > > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> > > > > > > sbs-manager has been added to linux-next through the i2c tree due to > > > dependencies, so this one also needs to go through i2c. > > > > I want to send Linus my pull request early this time (like today or > > tomorrow). So, you either apply this patch then yourself afterwards on > > top of linus' tree. Or I send it to him, but then I'd need the original > > patch bounced or resent. Both fine with me. > > The sbs-manager code is in linus' tree now. How would you like to > proceed? I plan to send my pull request today and will take it during the -rc phase through the power-supply tree. Thanks, -- Sebastian
Hi, On Tue, Nov 07, 2017 at 03:43:22PM +0300, Dan Carpenter wrote: > The original code does this: "1 << (1 << 11)" which is undefined in C. > > Fixes: dbc4deda03fe ("power: Adds support for Smart Battery System Manager") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- Thanks, queued. -- Sebastian > From static analysis. Not tested. > > diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c > index ccb4217b9638..cb6e8f66c7a2 100644 > --- a/drivers/power/supply/sbs-manager.c > +++ b/drivers/power/supply/sbs-manager.c > @@ -183,7 +183,7 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) > return ret; > > /* chan goes from 1 ... 4 */ > - reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); > + reg = BIT(SBSM_SMB_BAT_OFFSET + chan); > ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); > if (ret) > dev_err(dev, "Failed to select channel %i\n", chan);
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c index ccb4217b9638..cb6e8f66c7a2 100644 --- a/drivers/power/supply/sbs-manager.c +++ b/drivers/power/supply/sbs-manager.c @@ -183,7 +183,7 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) return ret; /* chan goes from 1 ... 4 */ - reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); + reg = BIT(SBSM_SMB_BAT_OFFSET + chan); ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); if (ret) dev_err(dev, "Failed to select channel %i\n", chan);
The original code does this: "1 << (1 << 11)" which is undefined in C. Fixes: dbc4deda03fe ("power: Adds support for Smart Battery System Manager") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- From static analysis. Not tested.