Re: [PATCH -next v4 1/3] regmap: add SCCB support

From: Sebastian Reichel
Date: Thu Jul 19 2018 - 08:33:16 EST


Hi,

On Tue, Jul 17, 2018 at 12:47:48AM +0900, Akinobu Mita wrote:
> This adds Serial Camera Control Bus (SCCB) support for regmap API that
> is intended to be used by some of Omnivision sensor drivers.
>
> The ov772x and ov9650 drivers are going to use this SCCB regmap API.
>
> The ov772x driver was previously only worked with the i2c controller
> drivers that support I2C_FUNC_PROTOCOL_MANGLING, because the ov772x
> device doesn't support repeated starts. After commit 0b964d183cbf
> ("media: ov772x: allow i2c controllers without
> I2C_FUNC_PROTOCOL_MANGLING"), reading ov772x register is replaced with
> issuing two separated i2c messages in order to avoid repeated start.
> Using this SCCB regmap hides the implementation detail.
>
> The ov9650 driver also issues two separated i2c messages to read the
> registers as the device doesn't support repeated start. So it can
> make use of this SCCB regmap.
>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Peter Rosin <peda@xxxxxxxxxx>
> Cc: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx>
> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> Cc: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> Cc: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
> Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx>

-- Sebastian

> drivers/base/regmap/Kconfig | 4 ++
> drivers/base/regmap/Makefile | 1 +
> drivers/base/regmap/regmap-sccb.c | 128 ++++++++++++++++++++++++++++++++++++++
> include/linux/regmap.h | 35 +++++++++++
> 4 files changed, 168 insertions(+)
> create mode 100644 drivers/base/regmap/regmap-sccb.c
>
> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> index aff34c0..6ad5ef4 100644
> --- a/drivers/base/regmap/Kconfig
> +++ b/drivers/base/regmap/Kconfig
> @@ -45,3 +45,7 @@ config REGMAP_IRQ
> config REGMAP_SOUNDWIRE
> tristate
> depends on SOUNDWIRE_BUS
> +
> +config REGMAP_SCCB
> + tristate
> + depends on I2C
> diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
> index 5ed0023..f5b4e88 100644
> --- a/drivers/base/regmap/Makefile
> +++ b/drivers/base/regmap/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_REGMAP_MMIO) += regmap-mmio.o
> obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
> obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
> obj-$(CONFIG_REGMAP_SOUNDWIRE) += regmap-sdw.o
> +obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
> diff --git a/drivers/base/regmap/regmap-sccb.c b/drivers/base/regmap/regmap-sccb.c
> new file mode 100644
> index 0000000..b6eb876
> --- /dev/null
> +++ b/drivers/base/regmap/regmap-sccb.c
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Register map access API - SCCB support
> +
> +#include <linux/regmap.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +
> +#include "internal.h"
> +
> +/**
> + * sccb_is_available - Check if the adapter supports SCCB protocol
> + * @adap: I2C adapter
> + *
> + * Return true if the I2C adapter is capable of using SCCB helper functions,
> + * false otherwise.
> + */
> +static bool sccb_is_available(struct i2c_adapter *adap)
> +{
> + u32 needed_funcs = I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA;
> +
> + /*
> + * If we ever want support for hardware doing SCCB natively, we will
> + * introduce a sccb_xfer() callback to struct i2c_algorithm and check
> + * for it here.
> + */
> +
> + return (i2c_get_functionality(adap) & needed_funcs) == needed_funcs;
> +}
> +
> +/**
> + * regmap_sccb_read - Read data from SCCB slave device
> + * @context: Device that will be interacted wit
> + * @reg: Register to be read from
> + * @val: Pointer to store read value
> + *
> + * This executes the 2-phase write transmission cycle that is followed by a
> + * 2-phase read transmission cycle, returning negative errno else zero on
> + * success.
> + */
> +static int regmap_sccb_read(void *context, unsigned int reg, unsigned int *val)
> +{
> + struct device *dev = context;
> + struct i2c_client *i2c = to_i2c_client(dev);
> + int ret;
> + union i2c_smbus_data data;
> +
> + i2c_lock_bus(i2c->adapter, I2C_LOCK_SEGMENT);
> +
> + ret = __i2c_smbus_xfer(i2c->adapter, i2c->addr, i2c->flags,
> + I2C_SMBUS_WRITE, reg, I2C_SMBUS_BYTE, NULL);
> + if (ret < 0)
> + goto out;
> +
> + ret = __i2c_smbus_xfer(i2c->adapter, i2c->addr, i2c->flags,
> + I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data);
> + if (ret < 0)
> + goto out;
> +
> + *val = data.byte;
> +out:
> + i2c_unlock_bus(i2c->adapter, I2C_LOCK_SEGMENT);
> +
> + return ret;
> +}
> +
> +/**
> + * regmap_sccb_write - Write data to SCCB slave device
> + * @context: Device that will be interacted wit
> + * @reg: Register to write to
> + * @val: Value to be written
> + *
> + * This executes the SCCB 3-phase write transmission cycle, returning negative
> + * errno else zero on success.
> + */
> +static int regmap_sccb_write(void *context, unsigned int reg, unsigned int val)
> +{
> + struct device *dev = context;
> + struct i2c_client *i2c = to_i2c_client(dev);
> +
> + return i2c_smbus_write_byte_data(i2c, reg, val);
> +}
> +
> +static struct regmap_bus regmap_sccb_bus = {
> + .reg_write = regmap_sccb_write,
> + .reg_read = regmap_sccb_read,
> +};
> +
> +static const struct regmap_bus *regmap_get_sccb_bus(struct i2c_client *i2c,
> + const struct regmap_config *config)
> +{
> + if (config->val_bits == 8 && config->reg_bits == 8 &&
> + sccb_is_available(i2c->adapter))
> + return &regmap_sccb_bus;
> +
> + return ERR_PTR(-ENOTSUPP);
> +}
> +
> +struct regmap *__regmap_init_sccb(struct i2c_client *i2c,
> + const struct regmap_config *config,
> + struct lock_class_key *lock_key,
> + const char *lock_name)
> +{
> + const struct regmap_bus *bus = regmap_get_sccb_bus(i2c, config);
> +
> + if (IS_ERR(bus))
> + return ERR_CAST(bus);
> +
> + return __regmap_init(&i2c->dev, bus, &i2c->dev, config,
> + lock_key, lock_name);
> +}
> +EXPORT_SYMBOL_GPL(__regmap_init_sccb);
> +
> +struct regmap *__devm_regmap_init_sccb(struct i2c_client *i2c,
> + const struct regmap_config *config,
> + struct lock_class_key *lock_key,
> + const char *lock_name)
> +{
> + const struct regmap_bus *bus = regmap_get_sccb_bus(i2c, config);
> +
> + if (IS_ERR(bus))
> + return ERR_CAST(bus);
> +
> + return __devm_regmap_init(&i2c->dev, bus, &i2c->dev, config,
> + lock_key, lock_name);
> +}
> +EXPORT_SYMBOL_GPL(__devm_regmap_init_sccb);
> +
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index 4f38068..52bf358 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -514,6 +514,10 @@ struct regmap *__regmap_init_i2c(struct i2c_client *i2c,
> const struct regmap_config *config,
> struct lock_class_key *lock_key,
> const char *lock_name);
> +struct regmap *__regmap_init_sccb(struct i2c_client *i2c,
> + const struct regmap_config *config,
> + struct lock_class_key *lock_key,
> + const char *lock_name);
> struct regmap *__regmap_init_slimbus(struct slim_device *slimbus,
> const struct regmap_config *config,
> struct lock_class_key *lock_key,
> @@ -558,6 +562,10 @@ struct regmap *__devm_regmap_init_i2c(struct i2c_client *i2c,
> const struct regmap_config *config,
> struct lock_class_key *lock_key,
> const char *lock_name);
> +struct regmap *__devm_regmap_init_sccb(struct i2c_client *i2c,
> + const struct regmap_config *config,
> + struct lock_class_key *lock_key,
> + const char *lock_name);
> struct regmap *__devm_regmap_init_spi(struct spi_device *dev,
> const struct regmap_config *config,
> struct lock_class_key *lock_key,
> @@ -646,6 +654,19 @@ int regmap_attach_dev(struct device *dev, struct regmap *map,
> i2c, config)
>
> /**
> + * regmap_init_sccb() - Initialise register map
> + *
> + * @i2c: Device that will be interacted with
> + * @config: Configuration for register map
> + *
> + * The return value will be an ERR_PTR() on error or a valid pointer to
> + * a struct regmap.
> + */
> +#define regmap_init_sccb(i2c, config) \
> + __regmap_lockdep_wrapper(__regmap_init_sccb, #config, \
> + i2c, config)
> +
> +/**
> * regmap_init_slimbus() - Initialise register map
> *
> * @slimbus: Device that will be interacted with
> @@ -798,6 +819,20 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
> i2c, config)
>
> /**
> + * devm_regmap_init_sccb() - Initialise managed register map
> + *
> + * @i2c: Device that will be interacted with
> + * @config: Configuration for register map
> + *
> + * The return value will be an ERR_PTR() on error or a valid pointer
> + * to a struct regmap. The regmap will be automatically freed by the
> + * device management code.
> + */
> +#define devm_regmap_init_sccb(i2c, config) \
> + __regmap_lockdep_wrapper(__devm_regmap_init_sccb, #config, \
> + i2c, config)
> +
> +/**
> * devm_regmap_init_spi() - Initialise register map
> *
> * @dev: Device that will be interacted with
> --
> 2.7.4
>

Attachment: signature.asc
Description: PGP signature