| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140416110603.GA19671@lee--X1>
Date: Wed, 16 Apr 2014 12:06:03 +0100
From: Lee Jones <lee.jones@...aro.org>
To: Matt Porter <mporter@...aro.org>
Cc: Devicetree List <devicetree@...r.kernel.org>,
Samuel Ortiz <sameo@...ux.intel.com>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
Tim Kryger <tim.kryger@...aro.org>,
Markus Mayer <markus.mayer@...aro.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux ARM Kernel List <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave
address space
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 <mporter@...aro.org>
> ---
> 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 <linux/regmap.h>
>
> /* 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists