Re: modular i2c device driver: direct match

[yarl-baudig%mailoo.org@localhost]

On Thu, May 02, 2019 at 09:46:08AM +0200, yarl-baudig%mailoo.org@localhost wrote:
> Hello,
> 
> What do you think of the following, at least the idea, the patch is probably not very clean?
> With this, i2c device driver module can use information gathered at i2cbus attachment.
> direct match is then possible for modules, not only for builtins.

Could you explain what prevents that now?

Martin

Sure, I can try. I see 2 reasons.

If I understood well, iic parent (i2cbus) gather i2c devices in a prop_array and pass it to
i2c using either:
- an entry name i2c-child-devices in it's dv_properties (thus i2c calls device_properties(parent))
- the struct i2cbus_attach_args
That said, I don't know why not only one of those two method for passing i2c children is used.
Anyway, if the i2cbus_attach_args method is used, the array is lost because not retained.

The second reason is that iic_rescan simply does not try to use that information.

Is that what you asked for?

Below is an example module that does match with the pach and does not without, on rpi with the following overlay:
/dts-v1/;
/plugin/;

/ {
  compatible = "brcm,bcm2708";
  fragment@0 {
    target-path = "/soc/i2c@7e804000";
    __overlay__ {
      i2c_foo@69 {
        reg = <0x69>;
        compatible = "i2c_foo";
      };
    };
  };
};

diff --git a/sys/dev/i2c/files.i2c b/sys/dev/i2c/files.i2c
index 25b338a58739..7e8cd9ee7847 100644
--- a/sys/dev/i2c/files.i2c
+++ b/sys/dev/i2c/files.i2c
@@ -366,3 +366,7 @@ file	dev/i2c/ssdfb_i2c.c			ssdfb_iic
 device	anxedp: edid, videomode, drmkms, drmkms_i2c
 attach	anxedp at iic
 file	dev/i2c/anxedp.c			anxedp
+
+device	i2c_foo
+attach	i2c_foo at iic
+file	dev/i2c/i2c_foo.c			i2c_foo
diff --git a/sys/dev/i2c/i2c_foo.c b/sys/dev/i2c/i2c_foo.c
new file mode 100644
index 000000000000..322f935081bb
--- /dev/null
+++ b/sys/dev/i2c/i2c_foo.c
@@ -0,0 +1,91 @@
+#include <sys/cdefs.h>
+#include <sys/param.h>
+#include <sys/conf.h>
+#include <sys/device.h>
+#include <sys/once.h>
+#include <sys/module.h>
+#include <sys/proc.h>
+
+#include <dev/i2c/i2cvar.h>
+
+#include "ioconf.h"
+
+static int	i2c_foo_match(device_t, cfdata_t, void*);
+static void	i2c_foo_attach(device_t, device_t, void*);
+static int  	i2c_foo_detach(device_t, int);
+
+struct i2c_foo_softc {
+	device_t sc_dev;
+};
+
+CFATTACH_DECL_NEW(i2c_foo, sizeof(struct i2c_foo_softc),
+	i2c_foo_match, i2c_foo_attach, i2c_foo_detach, NULL);
+
+static const struct device_compatible_entry compat_data[] = {
+	{ "i2c_foo",		0 },
+	{ NULL,			0 }
+};
+
+static int
+i2c_foo_match(device_t parent, cfdata_t match, void *aux)
+{
+	struct i2c_attach_args* ia = aux;
+	int match_result;
+
+	if (iic_use_direct_match(ia, match, compat_data, &match_result))
+	{
+		aprint_normal("iic_use_direct_match result: %d\n", match_result);
+		return match_result;
+	}
+
+	return 0;
+}
+
+static void
+i2c_foo_attach(device_t parent, device_t self, void *aux)
+{
+}
+
+static int
+i2c_foo_detach(device_t self, int flags)
+{
+	return 0;
+}
+
+MODULE(MODULE_CLASS_DRIVER, i2c_foo, NULL);
+
+#ifdef _MODULE
+#include "ioconf.c"
+#endif
+
+static int
+i2c_foo_modcmd(modcmd_t cmd, void *aux)
+{
+#ifdef _MODULE
+#endif
+	int ret = 0;
+
+	switch (cmd) {
+	case MODULE_CMD_INIT:
+#ifdef _MODULE
+		ret = config_init_component(cfdriver_ioconf_i2c_foo,
+			cfattach_ioconf_i2c_foo, cfdata_ioconf_i2c_foo);
+		if (ret) {
+			aprint_error("%s: unable to init component\n",
+			    i2c_foo_cd.cd_name);
+		}
+#endif
+		break;
+	case MODULE_CMD_FINI:
+#ifdef _MODULE
+		ret = config_fini_component(cfdriver_ioconf_i2c_foo,
+			cfattach_ioconf_i2c_foo, cfdata_ioconf_i2c_foo);
+		if (ret != 0)
+			break;
+#endif
+		break;
+	default:
+		ret = ENOTTY;
+	}
+	return ret;
+}
diff --git a/sys/modules/Makefile b/sys/modules/Makefile
index 93c30472ac18..074a9f2dc3f1 100644
--- a/sys/modules/Makefile
+++ b/sys/modules/Makefile
@@ -69,6 +69,7 @@ SUBDIR+=	si70xxtemp
 SUBDIR+=	am2315temp
 SUBDIR+=	i2cexec
 SUBDIR+=	i2c_bitbang
+SUBDIR+=	i2c_foo
 SUBDIR+=	if_agr
 SUBDIR+=	if_axe
 SUBDIR+=	if_axen
diff --git a/sys/modules/i2c_foo/Makefile b/sys/modules/i2c_foo/Makefile
new file mode 100644
index 000000000000..852cf1dbcba0
--- /dev/null
+++ b/sys/modules/i2c_foo/Makefile
@@ -0,0 +1,10 @@
+
+.include "../Makefile.inc"
+
+.PATH:	${S}/dev/i2c
+
+KMOD=	i2c_foo
+IOCONF=	i2c_foo.ioconf
+SRCS=	i2c_foo.c
+
+.include <bsd.kmodule.mk>
diff --git a/sys/modules/i2c_foo/i2c_foo.ioconf b/sys/modules/i2c_foo/i2c_foo.ioconf
new file mode 100644
index 000000000000..afa5dfb9c648
--- /dev/null
+++ b/sys/modules/i2c_foo/i2c_foo.ioconf
@@ -0,0 +1,9 @@
+# $NetBSD: sdtemp.ioconf,v 1.2 2011/08/02 18:52:35 pgoyette Exp $
+
+ioconf i2c_foo
+
+include "conf/files"
+
+pseudo-root iic*
+
+i2c_foo* at iic? addr ?