Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755579AbaDPLGN (ORCPT ); Wed, 16 Apr 2014 07:06:13 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:53017 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755376AbaDPLGJ (ORCPT ); Wed, 16 Apr 2014 07:06:09 -0400 Date: Wed, 16 Apr 2014 12:06:03 +0100 From: Lee Jones To: Matt Porter Cc: Devicetree List , Samuel Ortiz , Liam Girdwood , Mark Brown , Tim Kryger , Markus Mayer , Linux Kernel Mailing List , Linux ARM Kernel List Subject: Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space Message-ID: <20140416110603.GA19671@lee--X1> References: <1397501428-8857-1-git-send-email-mporter@linaro.org> <1397501428-8857-3-git-send-email-mporter@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1397501428-8857-3-git-send-email-mporter@linaro.org> 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 Mon, 14 Apr 2014, Matt Porter wrote: > BCM590xx utilizes a second i2c slave address to access additional s/i2c/I2C > register space. Add support for the second address space by > instantiated a dummy i2c device with the appropriate secondary s/instantiated/instantiating > i2c slave address. Expose a second regmap register space so that s/i2c/I2C Exposing? s/regmap/Regmap > mfd drivers can access this secondary i2c slave address space. s/mfd/MFD s/i2c/I2C > Signed-off-by: Matt Porter > --- > drivers/mfd/bcm590xx.c | 60 +++++++++++++++++++++++++++++++++----------- > include/linux/mfd/bcm590xx.h | 9 ++++--- > 2 files changed, 52 insertions(+), 17 deletions(-) > > diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c > index e9a33c7..b710ffa 100644 > --- a/drivers/mfd/bcm590xx.c > +++ b/drivers/mfd/bcm590xx.c > @@ -28,39 +28,71 @@ static const struct mfd_cell bcm590xx_devs[] = { > }, > }; > > -static const struct regmap_config bcm590xx_regmap_config = { > +static const struct regmap_config bcm590xx_regmap_config_0 = { Not loving _0 and _1 appendages. Is one of them {primary|master} and the other {secondary|slave}? > .reg_bits = 8, > .val_bits = 8, > - .max_register = BCM590XX_MAX_REGISTER, > + .max_register = BCM590XX_MAX_REGISTER_0, > .cache_type = REGCACHE_RBTREE, > }; > > -static int bcm590xx_i2c_probe(struct i2c_client *i2c, > +static const struct regmap_config bcm590xx_regmap_config_1 = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = BCM590XX_MAX_REGISTER_1, > + .cache_type = REGCACHE_RBTREE, > +}; > + > +static int bcm590xx_i2c_probe(struct i2c_client *addmap0, Would this be best left as i2c, then naming the other one i2c_secondary for instance? addmap{0,1} doesn't quite sit right with me. REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad as I first thought, but still, is there a better naming convention you could use? > const struct i2c_device_id *id) > { > struct bcm590xx *bcm590xx; > int ret; > > - bcm590xx = devm_kzalloc(&i2c->dev, sizeof(*bcm590xx), GFP_KERNEL); > + bcm590xx = devm_kzalloc(&addmap0->dev, sizeof(*bcm590xx), GFP_KERNEL); > if (!bcm590xx) > return -ENOMEM; > > - i2c_set_clientdata(i2c, bcm590xx); > - bcm590xx->dev = &i2c->dev; > - bcm590xx->i2c_client = i2c; > + i2c_set_clientdata(addmap0, bcm590xx); > + bcm590xx->dev = &addmap0->dev; > + bcm590xx->addmap0 = addmap0; > > - bcm590xx->regmap = devm_regmap_init_i2c(i2c, &bcm590xx_regmap_config); > - if (IS_ERR(bcm590xx->regmap)) { > - ret = PTR_ERR(bcm590xx->regmap); > - dev_err(&i2c->dev, "regmap initialization failed: %d\n", ret); > + bcm590xx->regmap0 = devm_regmap_init_i2c(addmap0, > + &bcm590xx_regmap_config_0); > + if (IS_ERR(bcm590xx->regmap0)) { > + ret = PTR_ERR(bcm590xx->regmap0); > + dev_err(&addmap0->dev, "regmap 0 init failed: %d\n", ret); > return ret; > } > > - ret = mfd_add_devices(&i2c->dev, -1, bcm590xx_devs, > + /* Second I2C slave address is the base address with A(2) asserted */ > + bcm590xx->addmap1 = i2c_new_dummy(addmap0->adapter, > + addmap0->addr | BIT(2)); > + if (IS_ERR_OR_NULL(bcm590xx->addmap1)) { > + dev_err(&addmap0->dev, "failed to add address map 1 device\n"); > + return -ENODEV; > + } > + i2c_set_clientdata(bcm590xx->addmap1, bcm590xx); > + > + bcm590xx->regmap1 = devm_regmap_init_i2c(bcm590xx->addmap1, > + &bcm590xx_regmap_config_1); > + if (IS_ERR(bcm590xx->regmap1)) { > + ret = PTR_ERR(bcm590xx->regmap1); > + dev_err(&bcm590xx->addmap1->dev, > + "regmap 1 init failed: %d\n", ret); > + goto err; > + } > + > + ret = mfd_add_devices(&addmap0->dev, -1, bcm590xx_devs, > ARRAY_SIZE(bcm590xx_devs), NULL, 0, NULL); > - if (ret < 0) > - dev_err(&i2c->dev, "failed to add sub-devices: %d\n", ret); > + if (ret < 0) { > + dev_err(&addmap0->dev, "failed to add sub-devices: %d\n", ret); > + goto err; > + } > + > + return 0; > > +err: > + i2c_unregister_device(bcm590xx->addmap1); > return ret; > } > > diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h > index 434df2d..a2723f2 100644 > --- a/include/linux/mfd/bcm590xx.h > +++ b/include/linux/mfd/bcm590xx.h > @@ -19,12 +19,15 @@ > #include > > /* max register address */ > -#define BCM590XX_MAX_REGISTER 0xe7 > +#define BCM590XX_MAX_REGISTER_0 0xe7 > +#define BCM590XX_MAX_REGISTER_1 0xf0 > > struct bcm590xx { > struct device *dev; > - struct i2c_client *i2c_client; > - struct regmap *regmap; > + struct i2c_client *addmap0; > + struct i2c_client *addmap1; > + struct regmap *regmap0; > + struct regmap *regmap1; > unsigned int id; > }; > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/