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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Wed,  1 Jul 2020 23:21:00 +0200
From:   Michael Walle <michael@...le.cc>
To:     linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Cc:     Lee Jones <lee.jones@...aro.org>, robh+dt@...nel.org,
        broonie@...nel.org, gregkh@...uxfoundation.org,
        andriy.shevchenko@...ux.intel.com, linus.walleij@...aro.org,
        arnd@...db.de, bgolaszewski@...libre.com,
        Michael Walle <michael@...le.cc>
Subject: [RFC PATCH] mfd: add simple regmap based I2C driver

There are I2C devices which contain several different functions but
doesn't require any special access functions. For these kind of drivers
an I2C regmap should be enough.

Create an I2C driver which creates an I2C regmap and enumerates its
children. If a device wants to use this as its MFD core driver, it has
to add an individual compatible string. It may provide its own regmap
configuration and a .pre_probe hook. The latter allows simple checks
and abort probing, for example a version check.

Subdevices can use dev_get_regmap() on the parent to get their regmap
instance.

Signed-off-by: Michael Walle <michael@...le.cc>
---

I don't think the syscon-i2c is the way to go:

   "TBC, while fine for a driver to bind on 'simple-mfd', a
    DT compatible with that alone is not fine." [1]

   "Yes, this is why specific compatible strings are
    required." [1]

   "No. I'd like to just kill off syscon and simple-mfd really. Those are
	just hints meaning a specific compatible is still needed, but I see
	them all the time alone (or combined like above). 'syscon' just serves
	to create a regmap. This could be accomplished just with a list of
	compatibles to register a regmap for. That might be a longish list, but
	wanting a regmap is really a kernel implementation detail and
	decision." [2]

So I don't get it why we would now add another syscon type. Instead, here
is a new try to generalize the idea to just have a simple driver which just
have a bunch of compatible strings for supported devices (and thus can be
added quirks to it without changing the DT) and just creates a regmap. A
simple device can just add itself to the list of compatible strings. If
quirks are needed later, they can either be added right to simple-mfd-i2c.c
or split off to an own file, but time will tell.

Right now, there is just a .pre_probe() hook which can be used to cancel
the probing of the device or print some useful info to the kernel log. Yes,
this is for "my" version check. And TBH I don't see a problem with adding
that to this generic driver.

[1] https://lore.kernel.org/linux-devicetree/CAL_JsqKr1aDVzgAMjwwK8E8O_f29vSrx1HXk81FF+rd3sEe==w@mail.gmail.com/
[2] https://lore.kernel.org/linux-devicetree/20200609165401.GB1019634@bogus/

 drivers/mfd/Kconfig          |  9 ++++++
 drivers/mfd/Makefile         |  1 +
 drivers/mfd/simple-mfd-i2c.c | 57 ++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 drivers/mfd/simple-mfd-i2c.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 7e72ed3441f1..96055c7e5c55 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1162,6 +1162,15 @@ config MFD_SI476X_CORE
 	  To compile this driver as a module, choose M here: the
 	  module will be called si476x-core.
 
+config MFD_SIMPLE_MFD_I2C
+	tristate "Simple regmap based I2C devices"
+	depends on I2C
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  This is a consolidated driver for all MFD devices which are
+	  basically just a regmap bus driver.
+
 config MFD_SM501
 	tristate "Silicon Motion SM501"
 	depends on HAS_DMA
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 7e26e7f98ac2..fa3a621a5a21 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -265,4 +265,5 @@ obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
 
 obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
 
+obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
 obj-$(CONFIG_MFD_SL28CPLD)	+= sl28cpld.o
diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
new file mode 100644
index 000000000000..60708e95f1a0
--- /dev/null
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+
+struct simple_mfd_i2c_config {
+	int (*pre_probe)(struct i2c_client *i2c, struct regmap *regmap);
+	const struct regmap_config *regmap_config;
+};
+
+static const struct regmap_config simple_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int simple_mfd_i2c_probe(struct i2c_client *i2c)
+{
+	const struct regmap_config *regmap_config = &simple_regmap_config;
+	const struct simple_mfd_i2c_config *config;
+	struct regmap *regmap;
+	int ret;
+
+	config = device_get_match_data(&i2c->dev);
+
+	if (config && config->regmap_config)
+		regmap_config = config->regmap_config;
+
+	regmap = devm_regmap_init_i2c(i2c, regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	if (config && config->pre_probe) {
+		ret = config->pre_probe(i2c, regmap);
+		if (ret)
+			return ret;
+	}
+
+	return devm_of_platform_populate(&i2c->dev);
+}
+
+static const struct of_device_id simple_mfd_i2c_of_match[] = {
+	{}
+};
+
+static struct i2c_driver simple_mfd_i2c_driver = {
+	.probe_new = simple_mfd_i2c_probe,
+	.driver = {
+		.name = "simple-mfd-i2c",
+		.of_match_table = simple_mfd_i2c_of_match,
+	},
+};
+builtin_i2c_driver(simple_mfd_i2c_driver);
-- 
2.20.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ