Am 2020-07-02 09:14, schrieb Lee Jones:
On Tue, 30 Jun 2020, Michael Walle wrote:

Hi Lee,

I'm just trying to use this for my sl28 driver. Some remarks, see below.

Am 2020-06-22 09:51, schrieb Lee Jones:
> The existing SYSCON implementation only supports MMIO (memory mapped)
> accesses, facilitated by Regmap.  This extends support for registers
> held behind I2C busses.
>
> Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> ---
> Changelog:
>
> v3 => v4
>   - Add ability to provide a non-default Regmap configuration
>
> v2 => v3
>   - Change 'is CONFIG' present check to include loadable modules
>     - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if
> IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/
>
> v1 => v2
>   - Remove legacy references to OF
>   - Allow building as a module (fixes h8300 0-day issue)
>
> drivers/mfd/Kconfig            |   7 +++
>  drivers/mfd/Makefile           |   1 +
>  drivers/mfd/syscon-i2c.c       | 104 +++++++++++++++++++++++++++++++++
>  include/linux/mfd/syscon-i2c.h |  36 ++++++++++++
>  4 files changed, 148 insertions(+)
>  create mode 100644 drivers/mfd/syscon-i2c.c
>  create mode 100644 include/linux/mfd/syscon-i2c.h
>

[..]

> +static struct regmap *syscon_i2c_get_regmap(struct i2c_client *client,
> +                                      struct regmap_config *regmap_config)
> +{
> +  struct device *dev = &client->dev;
> +  struct syscon *entry, *syscon = NULL;
> +
> +  spin_lock(&syscon_i2c_list_slock);
> +
> +  list_for_each_entry(entry, &syscon_i2c_list, list)
> +          if (entry->dev == dev) {
> +                  syscon = entry;
> +                  break;
> +          }
> +
> +  spin_unlock(&syscon_i2c_list_slock);
> +
> +  if (!syscon)
> +          syscon = syscon_i2c_register(client, regmap_config);
> +
> +  if (IS_ERR(syscon))
> +          return ERR_CAST(syscon);
> +
> +  return syscon->regmap;
> +}
> +
> +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client,
> +                                     struct regmap_config *regmap_config)
> +{
> +  return syscon_i2c_get_regmap(client, regmap_config);
> +}
> +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config);
> +
> +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client)
> +{
> +  return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config);
> +}
> +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap);

What do you think about

struct regmap *syscon_i2c_to_regmap(struct device *dev)
{
        struct i2c_client *client = i2c_verify_client(dev);

        if (!client)
                return ERR_PTR(-EINVAL);

        return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config);
}

Or even move it to syscon_i2c_get_regmap().

This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it
all over again in all sub drivers.

What is your use-case?

Still my sl28 mfd driver. There the sub devices just need a regmap.

 This is set-up for based I2C drivers to call
into.  'client' is given to them as their .probe() arg.

Ok, I see. Then this doesn't fit.

-michael

So you could just do a
  regmap = syscon_i2c_to_regmap(pdev->dev.parent);

I've also noticed that the mmio syscon uses device_node as parameter. What
was the reason to divert from that? Just curious.

This is a helper for I2C clients.  There aren't any OF helpers in here
(yet).  If you think they would be helpful we can add them.  How do
you see them being used?

Reply via email to