From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752464AbbJEJmz (ORCPT ); Mon, 5 Oct 2015 05:42:55 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:33306 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752007AbbJEJmy (ORCPT ); Mon, 5 Oct 2015 05:42:54 -0400 Date: Mon, 5 Oct 2015 10:42:50 +0100 From: Lee Jones To: Grigoryev Denis Cc: linux-kernel , "broonie@kernel.org" , "lgirdwood@gmail.com" Subject: Re: [PATCH] mfd: tps6105x: Use i2c regmap to access registers Message-ID: <20151005094250.GH3243@x1> References: <1E0B3F54A97F2C4591427D8CCF3028AD691D8F2E@SPB-PRIMARY-1.spb.prosoft.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1E0B3F54A97F2C4591427D8CCF3028AD691D8F2E@SPB-PRIMARY-1.spb.prosoft.ru> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 02 Oct 2015, Grigoryev Denis wrote: > This patch modifies tps6105x and associated function driver to use regmap > instead of operating directly on i2c. > > Signed-off-by: Denis Grigoryev > --- > drivers/mfd/tps6105x.c | 78 ++++++---------------------------- > drivers/regulator/tps6105x-regulator.c | 16 +++---- > include/linux/mfd/tps6105x.h | 10 ++--- > 3 files changed, 24 insertions(+), 80 deletions(-) Applied, thanks. > diff --git a/drivers/mfd/tps6105x.c b/drivers/mfd/tps6105x.c > index 182ebe0..51c5495 100644 > --- a/drivers/mfd/tps6105x.c > +++ b/drivers/mfd/tps6105x.c > @@ -16,7 +16,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -25,73 +25,18 @@ > #include > #include > > -int tps6105x_set(struct tps6105x *tps6105x, u8 reg, u8 value) > -{ > - int ret; > - > - ret = mutex_lock_interruptible(&tps6105x->lock); > - if (ret) > - return ret; > - ret = i2c_smbus_write_byte_data(tps6105x->client, reg, value); > - mutex_unlock(&tps6105x->lock); > - if (ret < 0) > - return ret; > - > - return 0; > -} > -EXPORT_SYMBOL(tps6105x_set); > - > -int tps6105x_get(struct tps6105x *tps6105x, u8 reg, u8 *buf) > -{ > - int ret; > - > - ret = mutex_lock_interruptible(&tps6105x->lock); > - if (ret) > - return ret; > - ret = i2c_smbus_read_byte_data(tps6105x->client, reg); > - mutex_unlock(&tps6105x->lock); > - if (ret < 0) > - return ret; > - > - *buf = ret; > - return 0; > -} > -EXPORT_SYMBOL(tps6105x_get); > - > -/* > - * Masks off the bits in the mask and sets the bits in the bitvalues > - * parameter in one atomic operation > - */ > -int tps6105x_mask_and_set(struct tps6105x *tps6105x, u8 reg, > - u8 bitmask, u8 bitvalues) > -{ > - int ret; > - u8 regval; > - > - ret = mutex_lock_interruptible(&tps6105x->lock); > - if (ret) > - return ret; > - ret = i2c_smbus_read_byte_data(tps6105x->client, reg); > - if (ret < 0) > - goto fail; > - regval = ret; > - regval = (~bitmask & regval) | (bitmask & bitvalues); > - ret = i2c_smbus_write_byte_data(tps6105x->client, reg, regval); > -fail: > - mutex_unlock(&tps6105x->lock); > - if (ret < 0) > - return ret; > - > - return 0; > -} > -EXPORT_SYMBOL(tps6105x_mask_and_set); > +static struct regmap_config tps6105x_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = TPS6105X_REG_3, > +}; > > static int tps6105x_startup(struct tps6105x *tps6105x) > { > int ret; > - u8 regval; > + unsigned int regval; > > - ret = tps6105x_get(tps6105x, TPS6105X_REG_0, ®val); > + ret = regmap_read(tps6105x->regmap, TPS6105X_REG_0, ®val); > if (ret) > return ret; > switch (regval >> TPS6105X_REG0_MODE_SHIFT) { > @@ -165,10 +110,13 @@ static int tps6105x_probe(struct i2c_client *client, > if (!tps6105x) > return -ENOMEM; > > + tps6105x->regmap = devm_regmap_init_i2c(client, &tps6105x_regmap_config); > + if (IS_ERR(tps6105x->regmap)) > + return PTR_ERR(tps6105x->regmap); > + > i2c_set_clientdata(client, tps6105x); > tps6105x->client = client; > tps6105x->pdata = pdata; > - mutex_init(&tps6105x->lock); > > ret = tps6105x_startup(tps6105x); > if (ret) { > @@ -212,7 +160,7 @@ static int tps6105x_remove(struct i2c_client *client) > mfd_remove_devices(&client->dev); > > /* Put chip in shutdown mode */ > - tps6105x_mask_and_set(tps6105x, TPS6105X_REG_0, > + regmap_update_bits(tps6105x->regmap, TPS6105X_REG_0, > TPS6105X_REG0_MODE_MASK, > TPS6105X_MODE_SHUTDOWN << TPS6105X_REG0_MODE_SHIFT); > > diff --git a/drivers/regulator/tps6105x-regulator.c b/drivers/regulator/tps6105x-regulator.c > index 3510b3e..ddc4f10 100644 > --- a/drivers/regulator/tps6105x-regulator.c > +++ b/drivers/regulator/tps6105x-regulator.c > @@ -14,7 +14,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -33,7 +33,7 @@ static int tps6105x_regulator_enable(struct regulator_dev *rdev) > int ret; > > /* Activate voltage mode */ > - ret = tps6105x_mask_and_set(tps6105x, TPS6105X_REG_0, > + ret = regmap_update_bits(tps6105x->regmap, TPS6105X_REG_0, > TPS6105X_REG0_MODE_MASK, > TPS6105X_REG0_MODE_VOLTAGE << TPS6105X_REG0_MODE_SHIFT); > if (ret) > @@ -48,7 +48,7 @@ static int tps6105x_regulator_disable(struct regulator_dev *rdev) > int ret; > > /* Set into shutdown mode */ > - ret = tps6105x_mask_and_set(tps6105x, TPS6105X_REG_0, > + ret = regmap_update_bits(tps6105x->regmap, TPS6105X_REG_0, > TPS6105X_REG0_MODE_MASK, > TPS6105X_REG0_MODE_SHUTDOWN << TPS6105X_REG0_MODE_SHIFT); > if (ret) > @@ -60,10 +60,10 @@ static int tps6105x_regulator_disable(struct regulator_dev *rdev) > static int tps6105x_regulator_is_enabled(struct regulator_dev *rdev) > { > struct tps6105x *tps6105x = rdev_get_drvdata(rdev); > - u8 regval; > + unsigned int regval; > int ret; > > - ret = tps6105x_get(tps6105x, TPS6105X_REG_0, ®val); > + ret = regmap_read(tps6105x->regmap, TPS6105X_REG_0, ®val); > if (ret) > return ret; > regval &= TPS6105X_REG0_MODE_MASK; > @@ -78,10 +78,10 @@ static int tps6105x_regulator_is_enabled(struct regulator_dev *rdev) > static int tps6105x_regulator_get_voltage_sel(struct regulator_dev *rdev) > { > struct tps6105x *tps6105x = rdev_get_drvdata(rdev); > - u8 regval; > + unsigned int regval; > int ret; > > - ret = tps6105x_get(tps6105x, TPS6105X_REG_0, ®val); > + ret = regmap_read(tps6105x->regmap, TPS6105X_REG_0, ®val); > if (ret) > return ret; > > @@ -96,7 +96,7 @@ static int tps6105x_regulator_set_voltage_sel(struct regulator_dev *rdev, > struct tps6105x *tps6105x = rdev_get_drvdata(rdev); > int ret; > > - ret = tps6105x_mask_and_set(tps6105x, TPS6105X_REG_0, > + ret = regmap_update_bits(tps6105x->regmap, TPS6105X_REG_0, > TPS6105X_REG0_VOLTAGE_MASK, > selector << TPS6105X_REG0_VOLTAGE_SHIFT); > if (ret) > diff --git a/include/linux/mfd/tps6105x.h b/include/linux/mfd/tps6105x.h > index 386743d..8bc5118 100644 > --- a/include/linux/mfd/tps6105x.h > +++ b/include/linux/mfd/tps6105x.h > @@ -10,6 +10,7 @@ > #define MFD_TPS6105X_H > > #include > +#include > #include > > /* > @@ -82,20 +83,15 @@ struct tps6105x_platform_data { > > /** > * struct tps6105x - state holder for the TPS6105x drivers > - * @mutex: mutex to serialize I2C accesses > * @i2c_client: corresponding I2C client > * @regulator: regulator device if used in voltage mode > + * @regmap: used for i2c communcation on accessing registers > */ > struct tps6105x { > struct tps6105x_platform_data *pdata; > - struct mutex lock; > struct i2c_client *client; > struct regulator_dev *regulator; > + struct regmap *regmap; > }; > > -extern int tps6105x_set(struct tps6105x *tps6105x, u8 reg, u8 value); > -extern int tps6105x_get(struct tps6105x *tps6105x, u8 reg, u8 *buf); > -extern int tps6105x_mask_and_set(struct tps6105x *tps6105x, u8 reg, > - u8 bitmask, u8 bitvalues); > - > #endif -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog