[PATCH 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability

[PATCH 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability

[Erling Ljunggren]

This series adds support for the standalone cat24c208 EDID EEPROM i2c device.
Usually EDID support is part of an HDMI receiver, but this is a standalone EEPROM.

Note that EEPROMs for EDIDs are not regular EEPROM devices, these are dual port
devices that follow the VESA E-DDC standard.

Since this is a standalone device that does not capture any video a new
V4L2_CAP_EDID_MEMORY capability is introduced to represent such devices.
Note that such a device doesn't have to be an EEPROM, it can also be
implemented using a microcontroller, for example. Hence the use of the generic
word 'MEMORY'.

The new capability uses the free bit 0x00000008. But we are running out of
capability bits: only 0x40000000 and 0x00000008 are free at the moment.

There is one other capability V4L2_CAP_ASYNCIO (0x02000000) that is not used
at all, it was never implemented. Wouldn't it be better to define
V4L2_CAP_ASYNCIO to 0, mark it as obsolete, and instead reuse it for this
V4L2_CAP_EDID_MEMORY capability?

Erling Ljunggren (4):
  media: videodev2.h: add V4L2_CAP_EDID_MEMORY
  media: docs: Add V4L2_CAP_EDID_MEMORY
  dt-bindings: media: add cat24c208 bindings
  media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY

Jonathan Selnes (1):
  media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

 .../bindings/media/i2c/onnn,cat24c208.yaml    |  40 ++
 .../userspace-api/media/v4l/biblio.rst        |  11 +
 .../media/v4l/vidioc-querycap.rst             |   7 +
 .../media/videodev2.h.rst.exceptions          |   1 +
 MAINTAINERS                                   |   7 +
 drivers/media/i2c/Kconfig                     |   9 +
 drivers/media/i2c/Makefile                    |   1 +
 drivers/media/i2c/cat24c208.c                 | 421 ++++++++++++++++++
 drivers/media/v4l2-core/v4l2-dev.c            |   8 +
 include/uapi/linux/videodev2.h                |   1 +
 10 files changed, 506 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
 create mode 100644 drivers/media/i2c/cat24c208.c

-- 
2.37.1

[PATCH 1/5] media: videodev2.h: add V4L2_CAP_EDID_MEMORY

[Erling Ljunggren]

Add capability flag to indicate that the device is an EDID memory device.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 include/uapi/linux/videodev2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3768a0a80830..3f771e883460 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -451,6 +451,7 @@ struct v4l2_capability {
 #define V4L2_CAP_VIDEO_CAPTURE		0x00000001  /* Is a video capture device */
 #define V4L2_CAP_VIDEO_OUTPUT		0x00000002  /* Is a video output device */
 #define V4L2_CAP_VIDEO_OVERLAY		0x00000004  /* Can do video overlay */
+#define V4L2_CAP_EDID_MEMORY		0x00000008  /* Is an EDID memory only device */
 #define V4L2_CAP_VBI_CAPTURE		0x00000010  /* Is a raw VBI capture device */
 #define V4L2_CAP_VBI_OUTPUT		0x00000020  /* Is a raw VBI output device */
 #define V4L2_CAP_SLICED_VBI_CAPTURE	0x00000040  /* Is a sliced VBI capture device */
-- 
2.37.1

[PATCH 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY

[Erling Ljunggren]

Add documentation for the new edid eeprom capability.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 Documentation/userspace-api/media/v4l/biblio.rst      | 11 +++++++++++
 .../userspace-api/media/v4l/vidioc-querycap.rst       |  7 +++++++
 .../userspace-api/media/videodev2.h.rst.exceptions    |  1 +
 3 files changed, 19 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
index 9cd18c153d19..5cbe41877a63 100644
--- a/Documentation/userspace-api/media/v4l/biblio.rst
+++ b/Documentation/userspace-api/media/v4l/biblio.rst
@@ -334,6 +334,17 @@ VESA DMT
 
 :author:    Video Electronics Standards Association (http://www.vesa.org)
 
+.. _vesaeddc:
+
+E-DDC
+====
+
+
+:title:     VESA Enhanced Display Data Channel (E-DDC) Standard
+:subtitle:  Version 1.3
+
+:author:    Video Electronics Standards Association (http://www.vesa.org)
+
 .. _vesaedid:
 
 EDID
diff --git a/Documentation/userspace-api/media/v4l/vidioc-querycap.rst b/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
index 63e23f6f95ee..bdb530bd6816 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
@@ -173,6 +173,13 @@ specification the ioctl returns an ``EINVAL`` error code.
 	interface. A video overlay device typically stores captured images
 	directly in the video memory of a graphics card, with hardware
 	clipping and scaling.
+    * - ``V4L2_CAP_EDID_MEMORY``
+      - 0x00000008
+      - The device is a standalone EDID memory device. This is typically an eeprom
+        that supports the VESA Enhanced Display Data Channel Standard.
+
+        While an eeprom is the most common implementation, it can be something else
+        as well, such as a microcontroller. Hence the generic name 'memory'.
     * - ``V4L2_CAP_VBI_CAPTURE``
       - 0x00000010
       - The device supports the :ref:`Raw VBI Capture <raw-vbi>`
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 9cbb7a0c354a..12fa290828e7 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -185,6 +185,7 @@ replace define V4L2_CAP_META_OUTPUT device-capabilities
 replace define V4L2_CAP_DEVICE_CAPS device-capabilities
 replace define V4L2_CAP_TOUCH device-capabilities
 replace define V4L2_CAP_IO_MC device-capabilities
+replace define V4L2_CAP_EDID_MEMORY device-capabilities
 
 # V4L2 pix flags
 replace define V4L2_PIX_FMT_PRIV_MAGIC :c:type:`v4l2_pix_format`
-- 
2.37.1

[PATCH 3/5] dt-bindings: media: add cat24c208 bindings

[Erling Ljunggren]

Add devicetree bindings for new cat24c208 EDID EEPROM driver.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 .../bindings/media/i2c/onnn,cat24c208.yaml    | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
new file mode 100644
index 000000000000..e1c861335538
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/onnn,cat24c208.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ON Semiconductor CAT24C208 EDID EEPROM driver
+
+maintainers:
+  - Hans Verkuil <hverkuil-cisco@xs4all.nl>
+
+description: |
+  CAT24C208 is a dual port i2c EEPROM designed for EDID storage.
+
+
+properties:
+  compatible:
+    const: onnn,cat24c208
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        lens@e {
+            compatible = "onnn,cat24c208";
+            reg = <0x31>;
+        };
+    };
+...
-- 
2.37.1

[PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Erling Ljunggren]

From: Jonathan Selnes <jonathansb1@gmail.com>

Support reading and writing the EDID EEPROM through the
v4l2 API.

Signed-off-by: Jonathan Selnes <jonathansb1@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 MAINTAINERS                   |   7 +
 drivers/media/i2c/Kconfig     |   9 +
 drivers/media/i2c/Makefile    |   1 +
 drivers/media/i2c/cat24c208.c | 421 ++++++++++++++++++++++++++++++++++
 4 files changed, 438 insertions(+)
 create mode 100644 drivers/media/i2c/cat24c208.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0a1d3d2b42bc..97d7ead555ac 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14632,6 +14632,13 @@ S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/i2c/ov9734.c
 
+ON SEMICONDUCTOR CAT24C208 EDID EEPROM DRIVER
+M:	Hans Verkuil <hverkuil-cisco@xs4all.nl>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
+F:	drivers/media/i2c/cat24c208*
+
 ONENAND FLASH DRIVER
 M:	Kyungmin Park <kyungmin.park@samsung.com>
 L:	linux-mtd@lists.infradead.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index fae2baabb773..10d437f79a35 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1529,6 +1529,15 @@ endmenu
 menu "Miscellaneous helper chips"
 	visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
 
+config VIDEO_CAT24C208
+	tristate "ON Semiconductor cat24c208 EDID EEPROM"
+	depends on VIDEO_DEV && I2C
+	help
+	  Support for the ON Semiconductor CAT24C208 Dual Port EDID EEPROM.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cat24c208.
+
 config VIDEO_I2C
 	tristate "I2C transport video support"
 	depends on VIDEO_DEV && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 3e1696963e7f..ae5a88204892 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
 obj-$(CONFIG_VIDEO_HI556) += hi556.o
 obj-$(CONFIG_VIDEO_HI846) += hi846.o
 obj-$(CONFIG_VIDEO_HI847) += hi847.o
+obj-$(CONFIG_VIDEO_CAT24C208) += cat24c208.o
 obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
 obj-$(CONFIG_VIDEO_IMX208) += imx208.o
 obj-$(CONFIG_VIDEO_IMX214) += imx214.o
diff --git a/drivers/media/i2c/cat24c208.c b/drivers/media/i2c/cat24c208.c
new file mode 100644
index 000000000000..b56e5f829a8d
--- /dev/null
+++ b/drivers/media/i2c/cat24c208.c
@@ -0,0 +1,421 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * cat24c208 - HDMI i2c controlled EEPROM from ON Semiconductor or Catalyst Semiconductor
+ *
+ * cat24c208.c - Support for i2c based DDC EEPROM
+ *
+ * Copyright (C) 2021-2022 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+
+/*
+ * REF_01 - ON Semiconductor, cat24c208, Datasheet, URL : https://www.onsemi.com/pdf/datasheet/cat24c208-d.pdf
+ *	Revision 7, May 2018
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/videodev2.h>
+#include <linux/kernel.h>
+
+#include <media/v4l2-common.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-ioctl.h>
+
+MODULE_DESCRIPTION("cat24c208 EDID EEPROM driver");
+MODULE_AUTHOR("Jonathan Selnes Bognaes <jonathansb1@gmail.com>");
+MODULE_LICENSE("GPL");
+
+/*
+ * CAT24C208 setup
+ */
+#define BYTES_PER_BLOCK		128
+#define EDID_EXT_FLAG_BIT	126
+#define MAX_WRITE_BYTES		16
+#define NUM_BLOCKS		4
+#define NUM_CLIENTS		3
+#define CONFIG_NB_BIT		BIT(0)
+#define CONFIG_AB0_BIT		BIT(1)
+#define CONFIG_AB1_BIT		BIT(2)
+#define CONFIG_WE_BIT		BIT(3)
+
+/*
+ * From the datasheet:
+ *
+ * The write cycle time is the time from a valid stop condition of a write
+ * sequence to the end of the internal program/erase cycle. During the write
+ * cycle, the bus interface circuits are disabled, SDA is allowed to remain
+ * high, and the device does not respond to its slave address.
+ */
+#define WRITE_CYCLE_TIME_US	5000
+
+/*
+ * CAT24C208 addresses
+ */
+#define CONFIG_I2C_ADDR		0x31
+#define EEPROM_I2C_ADDR		0x50
+#define SEGMENT_I2C_ADDR	0x30
+
+struct cat24c208_state {
+	struct i2c_client *i2c_clients[NUM_CLIENTS];
+	// V4L2 ioctl serialization
+	struct mutex lock;
+
+	struct v4l2_device v4l2_dev;
+	struct video_device vdev;
+
+	u8 edid_blocks;		// edid length can vary, one block = 128 bytes
+	u8 edid[BYTES_PER_BLOCK * NUM_BLOCKS]; // actual active edid data
+};
+
+static const struct v4l2_file_operations cat24c208_fops = {
+	.owner		= THIS_MODULE,
+	.open		= v4l2_fh_open,
+	.release	= v4l2_fh_release,
+	.unlocked_ioctl = video_ioctl2,
+};
+
+static int cat24c208_seg_write(struct cat24c208_state *state, u8 *data, u16 len, u8 seg)
+{
+	struct i2c_client *dev_client = state->i2c_clients[0];
+	struct i2c_client *data_client = state->i2c_clients[1];
+	struct i2c_client *seg_client = state->i2c_clients[2];
+
+	struct i2c_msg msg[] = {
+		{
+			.addr = seg_client->addr,	// Segment
+			.buf = &seg,
+			.len = 1,
+			.flags = 0,
+		},
+		{
+			.addr = data_client->addr,	// write data
+			.buf = data,
+			.len = len,
+			.flags = 0,
+		},
+	};
+	int err;
+
+	if (seg)
+		err = i2c_transfer(dev_client->adapter, msg, ARRAY_SIZE(msg));
+	else
+		err = i2c_transfer(dev_client->adapter, &msg[1], 1);
+	if (err < 0)
+		dev_err(&dev_client->dev, "Writing to 0x%x failed (segment %d)\n",
+			data_client->addr, seg);
+
+	usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
+	return err < 0 ? err : 0;
+}
+
+static int cat24c208_edid_read(struct cat24c208_state *state, u8 *data, u8 seg, u8 offset, u16 len)
+{
+	struct i2c_client *dev_client = state->i2c_clients[0];
+	struct i2c_client *data_client = state->i2c_clients[1];
+	struct i2c_client *seg_client = state->i2c_clients[2];
+	int err;
+
+	len *= BYTES_PER_BLOCK;
+	if (seg) {
+		struct i2c_msg msg[] = {
+			{
+				.addr = seg_client->addr,	// Segment
+				.buf = &seg,
+				.len = 1,
+				.flags = 0,
+			},
+			{
+				.addr = data_client->addr,	// read data
+				.buf = data,
+				.len = len,
+				.flags = I2C_M_RD,
+			},
+		};
+		err = i2c_transfer(dev_client->adapter, msg, ARRAY_SIZE(msg));
+	} else {
+		struct i2c_msg msg[] = {
+			{
+				.addr = data_client->addr,	// set offset
+				.buf = &offset,
+				.len = 1,
+				.flags = 0,
+			},
+			{
+				.addr = data_client->addr,	// read data
+				.buf = data,
+				.len = len,
+				.flags = I2C_M_RD,
+			},
+		};
+		err = i2c_transfer(dev_client->adapter, msg, ARRAY_SIZE(msg));
+	}
+
+	if (err < 0)
+		dev_err(&dev_client->dev, "Reading of EDID failed\n");
+	return err < 0 ? err : 0;
+}
+
+static int cat24c208_set_config(struct i2c_client *client)
+{
+	u8 buf[2] = { 0, CONFIG_NB_BIT };
+	struct i2c_msg msg = {
+		.addr = client->addr,
+		.buf = buf,
+		.len = sizeof(buf),
+		.flags = 0,
+	};
+	int err;
+
+	err = i2c_transfer(client->adapter, &msg, 1);
+	if (err < 0)
+		dev_err(&client->dev, "Could not set config register\n");
+	usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
+	return err < 0 ? err : 0;
+}
+
+static bool cat24c208_is_valid_edid(const u8 *block)
+{
+	static const u8 header_pattern[] = {
+		0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
+	};
+
+	return !memcmp(block, header_pattern, sizeof(header_pattern));
+}
+
+static int cat24c208_set_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+	struct cat24c208_state *state = video_drvdata(file);
+	u8 buf[MAX_WRITE_BYTES + 1];
+	int err;
+	int seg;
+	int i;
+
+	memset(edid->reserved, 0, sizeof(edid->reserved));
+
+	if (edid->pad)
+		return -EINVAL;
+
+	if (edid->blocks > NUM_BLOCKS) {
+		edid->blocks = NUM_BLOCKS;
+		return -E2BIG;
+	}
+
+	if (edid->start_block)
+		return -EINVAL;
+
+	state->edid_blocks = edid->blocks;
+	memcpy(state->edid, edid->edid, state->edid_blocks * BYTES_PER_BLOCK);
+
+	/* Write EDID to EEPROM */
+	for (i = 0; i < edid->blocks * BYTES_PER_BLOCK; i = i + MAX_WRITE_BYTES) {
+		if (i >= 2 * BYTES_PER_BLOCK) {
+			seg = 1;
+			buf[0] = i - BYTES_PER_BLOCK * 2;
+		} else {
+			seg = 0;
+			buf[0] = i;
+		}
+
+		memcpy(buf + 1, &edid->edid[i], MAX_WRITE_BYTES);
+		err = cat24c208_seg_write(state, buf, MAX_WRITE_BYTES + 1, seg);
+		if (err) {
+			dev_err(&state->i2c_clients[0]->dev,
+				"Could not write EDID to EEPROM, i: %d\n", i);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int cat24c208_get_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+	struct cat24c208_state *state = video_drvdata(file);
+
+	memset(edid->reserved, 0, sizeof(edid->reserved));
+
+	if (edid->pad != 0)
+		return -EINVAL;
+
+	if (edid->start_block == 0 && edid->blocks == 0) {
+		edid->blocks = state->edid_blocks;
+		return 0;
+	}
+
+	if (state->edid_blocks == 0)
+		return -ENODATA;
+
+	if (edid->start_block >= state->edid_blocks)
+		return -EINVAL;
+
+	if (edid->start_block + edid->blocks > state->edid_blocks)
+		edid->blocks = state->edid_blocks - edid->start_block;
+
+	memcpy(edid->edid, state->edid + edid->start_block * BYTES_PER_BLOCK,
+	       edid->blocks * BYTES_PER_BLOCK);
+
+	return 0;
+}
+
+static int cat24c208_get_input(struct file *file, void *priv, unsigned int *i)
+{
+	*i = 0;
+	return 0;
+}
+
+static int cat24c208_set_input(struct file *file, void *priv, unsigned int i)
+{
+	return i > 0 ? -EINVAL : 0;
+}
+
+static int cat24c208_enum_input(struct file *file, void *priv,
+				struct v4l2_input *inp)
+{
+	if (inp->index)
+		return -EINVAL;
+	strscpy(inp->name, "HDMI", sizeof(inp->name));
+	inp->capabilities = 0;
+	inp->type = V4L2_INPUT_TYPE_CAMERA;
+	return 0;
+}
+
+static int cat24c208_querycap(struct file *file,
+			      void *priv, struct v4l2_capability *cap)
+{
+	strscpy(cap->driver, "cat24c208", sizeof(cap->driver));
+	strscpy(cap->card, "cat24c208 EDID EEPROM", sizeof(cap->card));
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops cat24c208_ioctl_ops = {
+	.vidioc_querycap	= cat24c208_querycap,
+	.vidioc_g_edid		= cat24c208_get_edid,
+	.vidioc_s_edid		= cat24c208_set_edid,
+	.vidioc_g_input		= cat24c208_get_input,
+	.vidioc_s_input		= cat24c208_set_input,
+	.vidioc_enum_input	= cat24c208_enum_input,
+};
+
+static void cat24c208_release(struct video_device *vdev)
+{
+	struct cat24c208_state *state = video_get_drvdata(vdev);
+
+	v4l2_device_unregister(&state->v4l2_dev);
+	mutex_destroy(&state->lock);
+	kfree(state);
+}
+
+static int cat24c208_probe(struct i2c_client *client)
+{
+	struct cat24c208_state *state;
+	struct v4l2_device *v4l2_dev;
+	int blocks;
+	int ret;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	state->i2c_clients[0] = client;
+	state->i2c_clients[1] = i2c_new_ancillary_device(client, "eeprom", EEPROM_I2C_ADDR);
+	if (IS_ERR(state->i2c_clients[1])) {
+		ret = PTR_ERR(state->i2c_clients[1]);
+		goto free_state;
+	}
+	state->i2c_clients[2] = i2c_new_ancillary_device(client, "segment", SEGMENT_I2C_ADDR);
+	if (IS_ERR(state->i2c_clients[2])) {
+		ret = PTR_ERR(state->i2c_clients[2]);
+		goto unreg_i2c_first;
+	}
+
+	ret = cat24c208_set_config(client);
+	if (ret)
+		goto unreg_i2c_all;
+
+	if (cat24c208_edid_read(state, state->edid, 0, 0, 2) >= 0 &&
+	    cat24c208_is_valid_edid(state->edid)) {
+		unsigned int i;
+
+		blocks = 1 + state->edid[126];
+		state->edid_blocks = blocks;
+		for (i = 2; i < blocks; i += 2) {
+			if (cat24c208_edid_read(state, state->edid + i * BYTES_PER_BLOCK,
+						i / 2, 0, (i + 1 >= blocks ? 1 : 2))) {
+				state->edid_blocks = i;
+				break;
+			}
+		}
+	}
+
+	v4l2_dev = &state->v4l2_dev;
+	strscpy(v4l2_dev->name, "cat24c208", sizeof(v4l2_dev->name));
+	ret = v4l2_device_register(&client->dev, v4l2_dev);
+	if (ret) {
+		dev_err(&client->dev, "v4l2_device_register failed: %d\n", ret);
+		goto unreg_i2c_all;
+	}
+
+	mutex_init(&state->lock);
+
+	snprintf(state->vdev.name, sizeof(state->vdev.name),
+		 "cat24c208 %d-%d", client->adapter->nr, client->addr);
+
+	state->vdev.v4l2_dev = v4l2_dev;
+	state->vdev.fops = &cat24c208_fops;
+	state->vdev.ioctl_ops = &cat24c208_ioctl_ops;
+	state->vdev.lock = &state->lock;
+	state->vdev.release = cat24c208_release;
+	state->vdev.device_caps = V4L2_CAP_EDID_MEMORY;
+
+	video_set_drvdata(&state->vdev, state);
+	i2c_set_clientdata(client, state);
+	ret = video_register_device(&state->vdev, VFL_TYPE_VIDEO, -1);
+	if (ret != 0) {
+		dev_err(&client->dev, "Video registering failed: %d\n", ret);
+		goto unreg_v4l2_dev;
+	}
+	return 0;
+
+unreg_v4l2_dev:
+	v4l2_device_unregister(&state->v4l2_dev);
+unreg_i2c_all:
+	i2c_unregister_device(state->i2c_clients[2]);
+unreg_i2c_first:
+	i2c_unregister_device(state->i2c_clients[1]);
+free_state:
+	kfree(state);
+	return ret;
+}
+
+static int cat24c208_remove(struct i2c_client *client)
+{
+	struct cat24c208_state *state = i2c_get_clientdata(client);
+
+	i2c_unregister_device(state->i2c_clients[1]);
+	i2c_unregister_device(state->i2c_clients[2]);
+	video_unregister_device(&state->vdev);
+
+	return 0;
+}
+
+static const struct of_device_id cat24c208_of_match[] = {
+	{ .compatible = "onnn,cat24c208"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, cat24c208_of_match);
+
+static struct i2c_driver cat24c208_driver = {
+	.driver = {
+		.name	= "cat24c208",
+		.of_match_table = of_match_ptr(cat24c208_of_match),
+	},
+	.probe_new	= cat24c208_probe,
+	.remove		= cat24c208_remove,
+};
+module_i2c_driver(cat24c208_driver);
-- 
2.37.1

[PATCH 5/5] media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY

[Erling Ljunggren]

When the V4L2_CAP_EDID_MEMORY capability flag is set,
ioctls for enum inputs and get/set edid are automatically set.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 drivers/media/v4l2-core/v4l2-dev.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index d00237ee4cae..39437911edcc 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -556,6 +556,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
 	bool is_io_mc = vdev->device_caps & V4L2_CAP_IO_MC;
+	bool is_edid_mem =  vdev->device_caps & V4L2_CAP_EDID_MEMORY;
 
 	bitmap_zero(valid_ioctls, BASE_VIDIOC_PRIVATE);
 
@@ -778,6 +779,13 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
 		SET_VALID_IOCTL(ops, VIDIOC_S_HW_FREQ_SEEK, vidioc_s_hw_freq_seek);
 	}
+	if (is_edid_mem) {
+		SET_VALID_IOCTL(ops, VIDIOC_G_EDID, vidioc_g_edid);
+		SET_VALID_IOCTL(ops, VIDIOC_S_EDID, vidioc_s_edid);
+		SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
+		SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
+		SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
+	}
 
 	bitmap_andnot(vdev->valid_ioctls, valid_ioctls, vdev->valid_ioctls,
 			BASE_VIDIOC_PRIVATE);
-- 
2.37.1

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

Hi Andy,

On 28/07/2022 14:02, Andy Shevchenko wrote:
> 
> 
> On Thursday, July 28, 2022, Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>> wrote:
> 
>     From: Jonathan Selnes <jonathansb1@gmail.com <mailto:jonathansb1@gmail.com>>
> 
>     Support reading and writing the EDID EEPROM through the
>     v4l2 API.
> 
> 
> 
> Why the normal way of representing as a memory (we have framework and drivers) can’t work?

Because support for EDID for video sinks is already part of the media subsystem (V4L2).
Normally it is integrated into an HDMI receiver, but in this case it is just the EDID
support without the video receiver. It belongs in drivers/media in any case since EDIDs
are closely tied to media.

> 
> Moreover, this driver seems limited in support of variety of the eeprom chips.

Not quite sure what you mean. The cat24c208 is what this was developed for and
the only one we have.

Note that an EDID EEPROM != a regular EEPROM: it has to support the VESA E-DDC
standard, which a normal EEPROM doesn't. So these devices are specifically made
for this use-case.

Regards,

	Hans

> 
>  
> 
> 
>     Signed-off-by: Jonathan Selnes <jonathansb1@gmail.com <mailto:jonathansb1@gmail.com>>
>     Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl <mailto:hverkuil-cisco@xs4all.nl>>
>     Signed-off-by: Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>>
>     ---
>      MAINTAINERS                   |   7 +
>      drivers/media/i2c/Kconfig     |   9 +
>      drivers/media/i2c/Makefile    |   1 +
>      drivers/media/i2c/cat24c208.c | 421 ++++++++++++++++++++++++++++++++++
>      4 files changed, 438 insertions(+)
>      create mode 100644 drivers/media/i2c/cat24c208.c
> 
>     diff --git a/MAINTAINERS b/MAINTAINERS
>     index 0a1d3d2b42bc..97d7ead555ac 100644
>     --- a/MAINTAINERS
>     +++ b/MAINTAINERS
>     @@ -14632,6 +14632,13 @@ S:     Maintained
>      T:     git git://linuxtv.org/media_tree.git <http://linuxtv.org/media_tree.git>
>      F:     drivers/media/i2c/ov9734.c
> 
>     +ON SEMICONDUCTOR CAT24C208 EDID EEPROM DRIVER
>     +M:     Hans Verkuil <hverkuil-cisco@xs4all.nl <mailto:hverkuil-cisco@xs4all.nl>>
>     +L:     linux-media@vger.kernel.org <mailto:linux-media@vger.kernel.org>
>     +S:     Maintained
>     +F:     Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
>     +F:     drivers/media/i2c/cat24c208*
>     +
>      ONENAND FLASH DRIVER
>      M:     Kyungmin Park <kyungmin.park@samsung.com <mailto:kyungmin.park@samsung.com>>
>      L:     linux-mtd@lists.infradead.org <mailto:linux-mtd@lists.infradead.org>
>     diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>     index fae2baabb773..10d437f79a35 100644
>     --- a/drivers/media/i2c/Kconfig
>     +++ b/drivers/media/i2c/Kconfig
>     @@ -1529,6 +1529,15 @@ endmenu
>      menu "Miscellaneous helper chips"
>             visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
> 
>     +config VIDEO_CAT24C208
>     +       tristate "ON Semiconductor cat24c208 EDID EEPROM"
>     +       depends on VIDEO_DEV && I2C
>     +       help
>     +         Support for the ON Semiconductor CAT24C208 Dual Port EDID EEPROM.
>     +
>     +         To compile this driver as a module, choose M here: the
>     +         module will be called cat24c208.
>     +
>      config VIDEO_I2C
>             tristate "I2C transport video support"
>             depends on VIDEO_DEV && I2C
>     diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>     index 3e1696963e7f..ae5a88204892 100644
>     --- a/drivers/media/i2c/Makefile
>     +++ b/drivers/media/i2c/Makefile
>     @@ -35,6 +35,7 @@ obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
>      obj-$(CONFIG_VIDEO_HI556) += hi556.o
>      obj-$(CONFIG_VIDEO_HI846) += hi846.o
>      obj-$(CONFIG_VIDEO_HI847) += hi847.o
>     +obj-$(CONFIG_VIDEO_CAT24C208) += cat24c208.o
>      obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
>      obj-$(CONFIG_VIDEO_IMX208) += imx208.o
>      obj-$(CONFIG_VIDEO_IMX214) += imx214.o
>     diff --git a/drivers/media/i2c/cat24c208.c b/drivers/media/i2c/cat24c208.c
>     new file mode 100644
>     index 000000000000..b56e5f829a8d
>     --- /dev/null
>     +++ b/drivers/media/i2c/cat24c208.c
>     @@ -0,0 +1,421 @@
>     +// SPDX-License-Identifier: GPL-2.0-only
>     +/*
>     + * cat24c208 - HDMI i2c controlled EEPROM from ON Semiconductor or Catalyst Semiconductor
>     + *
>     + * cat24c208.c - Support for i2c based DDC EEPROM
>     + *
>     + * Copyright (C) 2021-2022 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
>     + */
>     +
>     +/*
>     + * REF_01 - ON Semiconductor, cat24c208, Datasheet, URL : https://www.onsemi.com/pdf/datasheet/cat24c208-d.pdf <https://www.onsemi.com/pdf/datasheet/cat24c208-d.pdf>
>     + *     Revision 7, May 2018
>     + */
>     +
>     +#include <linux/i2c.h>
>     +#include <linux/module.h>
>     +#include <linux/mutex.h>
>     +#include <linux/of_device.h>
>     +#include <linux/regmap.h>
>     +#include <linux/slab.h>
>     +#include <linux/videodev2.h>
>     +#include <linux/kernel.h>
>     +
>     +#include <media/v4l2-common.h>
>     +#include <media/v4l2-device.h>
>     +#include <media/v4l2-event.h>
>     +#include <media/v4l2-fh.h>
>     +#include <media/v4l2-ioctl.h>
>     +
>     +MODULE_DESCRIPTION("cat24c208 EDID EEPROM driver");
>     +MODULE_AUTHOR("Jonathan Selnes Bognaes <jonathansb1@gmail.com <mailto:jonathansb1@gmail.com>>");
>     +MODULE_LICENSE("GPL");
>     +
>     +/*
>     + * CAT24C208 setup
>     + */
>     +#define BYTES_PER_BLOCK                128
>     +#define EDID_EXT_FLAG_BIT      126
>     +#define MAX_WRITE_BYTES                16
>     +#define NUM_BLOCKS             4
>     +#define NUM_CLIENTS            3
>     +#define CONFIG_NB_BIT          BIT(0)
>     +#define CONFIG_AB0_BIT         BIT(1)
>     +#define CONFIG_AB1_BIT         BIT(2)
>     +#define CONFIG_WE_BIT          BIT(3)
>     +
>     +/*
>     + * From the datasheet:
>     + *
>     + * The write cycle time is the time from a valid stop condition of a write
>     + * sequence to the end of the internal program/erase cycle. During the write
>     + * cycle, the bus interface circuits are disabled, SDA is allowed to remain
>     + * high, and the device does not respond to its slave address.
>     + */
>     +#define WRITE_CYCLE_TIME_US    5000
>     +
>     +/*
>     + * CAT24C208 addresses
>     + */
>     +#define CONFIG_I2C_ADDR                0x31
>     +#define EEPROM_I2C_ADDR                0x50
>     +#define SEGMENT_I2C_ADDR       0x30
>     +
>     +struct cat24c208_state {
>     +       struct i2c_client *i2c_clients[NUM_CLIENTS];
>     +       // V4L2 ioctl serialization
>     +       struct mutex lock;
>     +
>     +       struct v4l2_device v4l2_dev;
>     +       struct video_device vdev;
>     +
>     +       u8 edid_blocks;         // edid length can vary, one block = 128 bytes
>     +       u8 edid[BYTES_PER_BLOCK * NUM_BLOCKS]; // actual active edid data
>     +};
>     +
>     +static const struct v4l2_file_operations cat24c208_fops = {
>     +       .owner          = THIS_MODULE,
>     +       .open           = v4l2_fh_open,
>     +       .release        = v4l2_fh_release,
>     +       .unlocked_ioctl = video_ioctl2,
>     +};
>     +
>     +static int cat24c208_seg_write(struct cat24c208_state *state, u8 *data, u16 len, u8 seg)
>     +{
>     +       struct i2c_client *dev_client = state->i2c_clients[0];
>     +       struct i2c_client *data_client = state->i2c_clients[1];
>     +       struct i2c_client *seg_client = state->i2c_clients[2];
>     +
>     +       struct i2c_msg msg[] = {
>     +               {
>     +                       .addr = seg_client->addr,       // Segment
>     +                       .buf = &seg,
>     +                       .len = 1,
>     +                       .flags = 0,
>     +               },
>     +               {
>     +                       .addr = data_client->addr,      // write data
>     +                       .buf = data,
>     +                       .len = len,
>     +                       .flags = 0,
>     +               },
>     +       };
>     +       int err;
>     +
>     +       if (seg)
>     +               err = i2c_transfer(dev_client->adapter, msg, ARRAY_SIZE(msg));
>     +       else
>     +               err = i2c_transfer(dev_client->adapter, &msg[1], 1);
>     +       if (err < 0)
>     +               dev_err(&dev_client->dev, "Writing to 0x%x failed (segment %d)\n",
>     +                       data_client->addr, seg);
>     +
>     +       usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
>     +       return err < 0 ? err : 0;
>     +}
>     +
>     +static int cat24c208_edid_read(struct cat24c208_state *state, u8 *data, u8 seg, u8 offset, u16 len)
>     +{
>     +       struct i2c_client *dev_client = state->i2c_clients[0];
>     +       struct i2c_client *data_client = state->i2c_clients[1];
>     +       struct i2c_client *seg_client = state->i2c_clients[2];
>     +       int err;
>     +
>     +       len *= BYTES_PER_BLOCK;
>     +       if (seg) {
>     +               struct i2c_msg msg[] = {
>     +                       {
>     +                               .addr = seg_client->addr,       // Segment
>     +                               .buf = &seg,
>     +                               .len = 1,
>     +                               .flags = 0,
>     +                       },
>     +                       {
>     +                               .addr = data_client->addr,      // read data
>     +                               .buf = data,
>     +                               .len = len,
>     +                               .flags = I2C_M_RD,
>     +                       },
>     +               };
>     +               err = i2c_transfer(dev_client->adapter, msg, ARRAY_SIZE(msg));
>     +       } else {
>     +               struct i2c_msg msg[] = {
>     +                       {
>     +                               .addr = data_client->addr,      // set offset
>     +                               .buf = &offset,
>     +                               .len = 1,
>     +                               .flags = 0,
>     +                       },
>     +                       {
>     +                               .addr = data_client->addr,      // read data
>     +                               .buf = data,
>     +                               .len = len,
>     +                               .flags = I2C_M_RD,
>     +                       },
>     +               };
>     +               err = i2c_transfer(dev_client->adapter, msg, ARRAY_SIZE(msg));
>     +       }
>     +
>     +       if (err < 0)
>     +               dev_err(&dev_client->dev, "Reading of EDID failed\n");
>     +       return err < 0 ? err : 0;
>     +}
>     +
>     +static int cat24c208_set_config(struct i2c_client *client)
>     +{
>     +       u8 buf[2] = { 0, CONFIG_NB_BIT };
>     +       struct i2c_msg msg = {
>     +               .addr = client->addr,
>     +               .buf = buf,
>     +               .len = sizeof(buf),
>     +               .flags = 0,
>     +       };
>     +       int err;
>     +
>     +       err = i2c_transfer(client->adapter, &msg, 1);
>     +       if (err < 0)
>     +               dev_err(&client->dev, "Could not set config register\n");
>     +       usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
>     +       return err < 0 ? err : 0;
>     +}
>     +
>     +static bool cat24c208_is_valid_edid(const u8 *block)
>     +{
>     +       static const u8 header_pattern[] = {
>     +               0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
>     +       };
>     +
>     +       return !memcmp(block, header_pattern, sizeof(header_pattern));
>     +}
>     +
>     +static int cat24c208_set_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>     +{
>     +       struct cat24c208_state *state = video_drvdata(file);
>     +       u8 buf[MAX_WRITE_BYTES + 1];
>     +       int err;
>     +       int seg;
>     +       int i;
>     +
>     +       memset(edid->reserved, 0, sizeof(edid->reserved));
>     +
>     +       if (edid->pad)
>     +               return -EINVAL;
>     +
>     +       if (edid->blocks > NUM_BLOCKS) {
>     +               edid->blocks = NUM_BLOCKS;
>     +               return -E2BIG;
>     +       }
>     +
>     +       if (edid->start_block)
>     +               return -EINVAL;
>     +
>     +       state->edid_blocks = edid->blocks;
>     +       memcpy(state->edid, edid->edid, state->edid_blocks * BYTES_PER_BLOCK);
>     +
>     +       /* Write EDID to EEPROM */
>     +       for (i = 0; i < edid->blocks * BYTES_PER_BLOCK; i = i + MAX_WRITE_BYTES) {
>     +               if (i >= 2 * BYTES_PER_BLOCK) {
>     +                       seg = 1;
>     +                       buf[0] = i - BYTES_PER_BLOCK * 2;
>     +               } else {
>     +                       seg = 0;
>     +                       buf[0] = i;
>     +               }
>     +
>     +               memcpy(buf + 1, &edid->edid[i], MAX_WRITE_BYTES);
>     +               err = cat24c208_seg_write(state, buf, MAX_WRITE_BYTES + 1, seg);
>     +               if (err) {
>     +                       dev_err(&state->i2c_clients[0]->dev,
>     +                               "Could not write EDID to EEPROM, i: %d\n", i);
>     +                       return err;
>     +               }
>     +       }
>     +
>     +       return 0;
>     +}
>     +
>     +static int cat24c208_get_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>     +{
>     +       struct cat24c208_state *state = video_drvdata(file);
>     +
>     +       memset(edid->reserved, 0, sizeof(edid->reserved));
>     +
>     +       if (edid->pad != 0)
>     +               return -EINVAL;
>     +
>     +       if (edid->start_block == 0 && edid->blocks == 0) {
>     +               edid->blocks = state->edid_blocks;
>     +               return 0;
>     +       }
>     +
>     +       if (state->edid_blocks == 0)
>     +               return -ENODATA;
>     +
>     +       if (edid->start_block >= state->edid_blocks)
>     +               return -EINVAL;
>     +
>     +       if (edid->start_block + edid->blocks > state->edid_blocks)
>     +               edid->blocks = state->edid_blocks - edid->start_block;
>     +
>     +       memcpy(edid->edid, state->edid + edid->start_block * BYTES_PER_BLOCK,
>     +              edid->blocks * BYTES_PER_BLOCK);
>     +
>     +       return 0;
>     +}
>     +
>     +static int cat24c208_get_input(struct file *file, void *priv, unsigned int *i)
>     +{
>     +       *i = 0;
>     +       return 0;
>     +}
>     +
>     +static int cat24c208_set_input(struct file *file, void *priv, unsigned int i)
>     +{
>     +       return i > 0 ? -EINVAL : 0;
>     +}
>     +
>     +static int cat24c208_enum_input(struct file *file, void *priv,
>     +                               struct v4l2_input *inp)
>     +{
>     +       if (inp->index)
>     +               return -EINVAL;
>     +       strscpy(inp->name, "HDMI", sizeof(inp->name));
>     +       inp->capabilities = 0;
>     +       inp->type = V4L2_INPUT_TYPE_CAMERA;
>     +       return 0;
>     +}
>     +
>     +static int cat24c208_querycap(struct file *file,
>     +                             void *priv, struct v4l2_capability *cap)
>     +{
>     +       strscpy(cap->driver, "cat24c208", sizeof(cap->driver));
>     +       strscpy(cap->card, "cat24c208 EDID EEPROM", sizeof(cap->card));
>     +       return 0;
>     +}
>     +
>     +static const struct v4l2_ioctl_ops cat24c208_ioctl_ops = {
>     +       .vidioc_querycap        = cat24c208_querycap,
>     +       .vidioc_g_edid          = cat24c208_get_edid,
>     +       .vidioc_s_edid          = cat24c208_set_edid,
>     +       .vidioc_g_input         = cat24c208_get_input,
>     +       .vidioc_s_input         = cat24c208_set_input,
>     +       .vidioc_enum_input      = cat24c208_enum_input,
>     +};
>     +
>     +static void cat24c208_release(struct video_device *vdev)
>     +{
>     +       struct cat24c208_state *state = video_get_drvdata(vdev);
>     +
>     +       v4l2_device_unregister(&state->v4l2_dev);
>     +       mutex_destroy(&state->lock);
>     +       kfree(state);
>     +}
>     +
>     +static int cat24c208_probe(struct i2c_client *client)
>     +{
>     +       struct cat24c208_state *state;
>     +       struct v4l2_device *v4l2_dev;
>     +       int blocks;
>     +       int ret;
>     +
>     +       state = kzalloc(sizeof(*state), GFP_KERNEL);
>     +       if (!state)
>     +               return -ENOMEM;
>     +
>     +       state->i2c_clients[0] = client;
>     +       state->i2c_clients[1] = i2c_new_ancillary_device(client, "eeprom", EEPROM_I2C_ADDR);
>     +       if (IS_ERR(state->i2c_clients[1])) {
>     +               ret = PTR_ERR(state->i2c_clients[1]);
>     +               goto free_state;
>     +       }
>     +       state->i2c_clients[2] = i2c_new_ancillary_device(client, "segment", SEGMENT_I2C_ADDR);
>     +       if (IS_ERR(state->i2c_clients[2])) {
>     +               ret = PTR_ERR(state->i2c_clients[2]);
>     +               goto unreg_i2c_first;
>     +       }
>     +
>     +       ret = cat24c208_set_config(client);
>     +       if (ret)
>     +               goto unreg_i2c_all;
>     +
>     +       if (cat24c208_edid_read(state, state->edid, 0, 0, 2) >= 0 &&
>     +           cat24c208_is_valid_edid(state->edid)) {
>     +               unsigned int i;
>     +
>     +               blocks = 1 + state->edid[126];
>     +               state->edid_blocks = blocks;
>     +               for (i = 2; i < blocks; i += 2) {
>     +                       if (cat24c208_edid_read(state, state->edid + i * BYTES_PER_BLOCK,
>     +                                               i / 2, 0, (i + 1 >= blocks ? 1 : 2))) {
>     +                               state->edid_blocks = i;
>     +                               break;
>     +                       }
>     +               }
>     +       }
>     +
>     +       v4l2_dev = &state->v4l2_dev;
>     +       strscpy(v4l2_dev->name, "cat24c208", sizeof(v4l2_dev->name));
>     +       ret = v4l2_device_register(&client->dev, v4l2_dev);
>     +       if (ret) {
>     +               dev_err(&client->dev, "v4l2_device_register failed: %d\n", ret);
>     +               goto unreg_i2c_all;
>     +       }
>     +
>     +       mutex_init(&state->lock);
>     +
>     +       snprintf(state->vdev.name <http://vdev.name>, sizeof(state->vdev.name <http://vdev.name>),
>     +                "cat24c208 %d-%d", client->adapter->nr, client->addr);
>     +
>     +       state->vdev.v4l2_dev = v4l2_dev;
>     +       state->vdev.fops = &cat24c208_fops;
>     +       state->vdev.ioctl_ops = &cat24c208_ioctl_ops;
>     +       state->vdev.lock = &state->lock;
>     +       state->vdev.release = cat24c208_release;
>     +       state->vdev.device_caps = V4L2_CAP_EDID_MEMORY;
>     +
>     +       video_set_drvdata(&state->vdev, state);
>     +       i2c_set_clientdata(client, state);
>     +       ret = video_register_device(&state->vdev, VFL_TYPE_VIDEO, -1);
>     +       if (ret != 0) {
>     +               dev_err(&client->dev, "Video registering failed: %d\n", ret);
>     +               goto unreg_v4l2_dev;
>     +       }
>     +       return 0;
>     +
>     +unreg_v4l2_dev:
>     +       v4l2_device_unregister(&state->v4l2_dev);
>     +unreg_i2c_all:
>     +       i2c_unregister_device(state->i2c_clients[2]);
>     +unreg_i2c_first:
>     +       i2c_unregister_device(state->i2c_clients[1]);
>     +free_state:
>     +       kfree(state);
>     +       return ret;
>     +}
>     +
>     +static int cat24c208_remove(struct i2c_client *client)
>     +{
>     +       struct cat24c208_state *state = i2c_get_clientdata(client);
>     +
>     +       i2c_unregister_device(state->i2c_clients[1]);
>     +       i2c_unregister_device(state->i2c_clients[2]);
>     +       video_unregister_device(&state->vdev);
>     +
>     +       return 0;
>     +}
>     +
>     +static const struct of_device_id cat24c208_of_match[] = {
>     +       { .compatible = "onnn,cat24c208"},
>     +       {}
>     +};
>     +MODULE_DEVICE_TABLE(of, cat24c208_of_match);
>     +
>     +static struct i2c_driver cat24c208_driver = {
>     +       .driver = {
>     +               .name   = "cat24c208",
>     +               .of_match_table = of_match_ptr(cat24c208_of_match),
>     +       },
>     +       .probe_new      = cat24c208_probe,
>     +       .remove         = cat24c208_remove,
>     +};
>     +module_i2c_driver(cat24c208_driver);
>     -- 
>     2.37.1
> 
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH 3/5] dt-bindings: media: add cat24c208 bindings

[Rob Herring]

On Thu, 28 Jul 2022 13:40:48 +0200, Erling Ljunggren wrote:
> Add devicetree bindings for new cat24c208 EDID EEPROM driver.
> 
> Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
> ---
>  .../bindings/media/i2c/onnn,cat24c208.yaml    | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.example.dts:22.20-25.15: Warning (i2c_bus_reg): /example-0/i2c/lens@e: I2C bus unit address format error, expected "31"

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 3:23 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 28/07/2022 14:02, Andy Shevchenko wrote:
> > On Thursday, July 28, 2022, Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>> wrote:

> >     Support reading and writing the EDID EEPROM through the
> >     v4l2 API.
> >
> > Why the normal way of representing as a memory (we have framework and drivers) can’t work?
>
> Because support for EDID for video sinks is already part of the media subsystem (V4L2).
> Normally it is integrated into an HDMI receiver, but in this case it is just the EDID
> support without the video receiver. It belongs in drivers/media in any case since EDIDs
> are closely tied to media.

It's fine. From the Linux perspective we do not reduplicate the
drivers that are done by other frameworks, right?

> > Moreover, this driver seems limited in support of variety of the eeprom chips.
>
> Not quite sure what you mean. The cat24c208 is what this was developed for and
> the only one we have.
>
> Note that an EDID EEPROM != a regular EEPROM: it has to support the VESA E-DDC
> standard, which a normal EEPROM doesn't. So these devices are specifically made
> for this use-case.

What is the difference from a programming interface?
Can the nvmem driver(s) be reused (at24?)?

...

> >      drivers/media/i2c/cat24c208.c | 421 ++++++++++++++++++++++++++++++++++

It really seems silly to me to add so many LoCs for the existing
drivers and perhaps we need to extend the nvmem to support EDID rather
than copying everything again?

Note, I can be well mistaken by not understanding some underlying
issues, perhaps there is some documentation to read...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

Hi Andy,

On 28/07/2022 22:56, Andy Shevchenko wrote:
> On Thu, Jul 28, 2022 at 3:23 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>> On 28/07/2022 14:02, Andy Shevchenko wrote:
>>> On Thursday, July 28, 2022, Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>> wrote:
> 
>>>     Support reading and writing the EDID EEPROM through the
>>>     v4l2 API.
>>>
>>> Why the normal way of representing as a memory (we have framework and drivers) can’t work?
>>
>> Because support for EDID for video sinks is already part of the media subsystem (V4L2).
>> Normally it is integrated into an HDMI receiver, but in this case it is just the EDID
>> support without the video receiver. It belongs in drivers/media in any case since EDIDs
>> are closely tied to media.
> 
> It's fine. From the Linux perspective we do not reduplicate the
> drivers that are done by other frameworks, right?
> 
>>> Moreover, this driver seems limited in support of variety of the eeprom chips.
>>
>> Not quite sure what you mean. The cat24c208 is what this was developed for and
>> the only one we have.
>>
>> Note that an EDID EEPROM != a regular EEPROM: it has to support the VESA E-DDC
>> standard, which a normal EEPROM doesn't. So these devices are specifically made
>> for this use-case.
> 
> What is the difference from a programming interface?
> Can the nvmem driver(s) be reused (at24?)?

No. EDID EEPROM devices are specific to storing EDIDs: they have two i2c
ports, one connected to (typically) the HDMI bus (DDC lines) allowing a
video source to read the EDID, the other is connected to the SoC to write to
and configure the device. The HDMI bus side has two i2c addresses (reading the
EEPROM and to write to the segment address for EDIDs > 256 bytes), the SoC
side has three i2c addresses: to configure the behavior, the segment address,
and to write the EDID from the SoC.

So it is a much more complex device than a regular eeprom, and it really
is dedicated to EDIDs only.

Also note that the V4L2 API is already used to get/set EDIDs, everything is
in place for supporting that, including support for parsing EDIDs for the
physical address, which is something that is needed if this is combined with
HDMI CEC hardware. It's not implemented in this driver since it is not
needed in our use-case, but that might change in the future.

And by using the V4L2 API you can use v4l2-ctl --get-edid and --set-edid
out of the box, using the standard API for EDIDs.

Regards,

	Hans

> 
> ...
> 
>>>      drivers/media/i2c/cat24c208.c | 421 ++++++++++++++++++++++++++++++++++
> 
> It really seems silly to me to add so many LoCs for the existing
> drivers and perhaps we need to extend the nvmem to support EDID rather
> than copying everything again?
> 
> Note, I can be well mistaken by not understanding some underlying
> issues, perhaps there is some documentation to read...
> 

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Fri, Jul 29, 2022 at 9:21 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 28/07/2022 22:56, Andy Shevchenko wrote:
> > On Thu, Jul 28, 2022 at 3:23 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >> On 28/07/2022 14:02, Andy Shevchenko wrote:
> >>> On Thursday, July 28, 2022, Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>> wrote:
> >
> >>>     Support reading and writing the EDID EEPROM through the
> >>>     v4l2 API.
> >>>
> >>> Why the normal way of representing as a memory (we have framework and drivers) can’t work?
> >>
> >> Because support for EDID for video sinks is already part of the media subsystem (V4L2).
> >> Normally it is integrated into an HDMI receiver, but in this case it is just the EDID
> >> support without the video receiver. It belongs in drivers/media in any case since EDIDs
> >> are closely tied to media.
> >
> > It's fine. From the Linux perspective we do not reduplicate the
> > drivers that are done by other frameworks, right?
> >
> >>> Moreover, this driver seems limited in support of variety of the eeprom chips.
> >>
> >> Not quite sure what you mean. The cat24c208 is what this was developed for and
> >> the only one we have.
> >>
> >> Note that an EDID EEPROM != a regular EEPROM: it has to support the VESA E-DDC
> >> standard, which a normal EEPROM doesn't. So these devices are specifically made
> >> for this use-case.
> >
> > What is the difference from a programming interface?
> > Can the nvmem driver(s) be reused (at24?)?
>
> No. EDID EEPROM devices are specific to storing EDIDs: they have two i2c
> ports, one connected to (typically) the HDMI bus (DDC lines) allowing a
> video source to read the EDID, the other is connected to the SoC to write to
> and configure the device. The HDMI bus side has two i2c addresses (reading the
> EEPROM and to write to the segment address for EDIDs > 256 bytes), the SoC
> side has three i2c addresses: to configure the behavior, the segment address,
> and to write the EDID from the SoC.
>
> So it is a much more complex device than a regular eeprom, and it really
> is dedicated to EDIDs only.

Thanks for the explanation, but it's still unclear what the
differences are in the programming interface there. Perhaps you may
simply register a platform device in this driver and reuse the rest
from at24?

> Also note that the V4L2 API is already used to get/set EDIDs, everything is
> in place for supporting that, including support for parsing EDIDs for the
> physical address, which is something that is needed if this is combined with
> HDMI CEC hardware. It's not implemented in this driver since it is not
> needed in our use-case, but that might change in the future.
>
> And by using the V4L2 API you can use v4l2-ctl --get-edid and --set-edid
> out of the box, using the standard API for EDIDs.

Bonus question: we have cat24c04/cat24c05 are recognized by at24
already, are they different to cat24c08?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

Hi Andy,

On 29/07/2022 14:00, Andy Shevchenko wrote:
> On Fri, Jul 29, 2022 at 9:21 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>> On 28/07/2022 22:56, Andy Shevchenko wrote:
>>> On Thu, Jul 28, 2022 at 3:23 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>>> On 28/07/2022 14:02, Andy Shevchenko wrote:
>>>>> On Thursday, July 28, 2022, Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>> wrote:
>>>
>>>>>     Support reading and writing the EDID EEPROM through the
>>>>>     v4l2 API.
>>>>>
>>>>> Why the normal way of representing as a memory (we have framework and drivers) can’t work?
>>>>
>>>> Because support for EDID for video sinks is already part of the media subsystem (V4L2).
>>>> Normally it is integrated into an HDMI receiver, but in this case it is just the EDID
>>>> support without the video receiver. It belongs in drivers/media in any case since EDIDs
>>>> are closely tied to media.
>>>
>>> It's fine. From the Linux perspective we do not reduplicate the
>>> drivers that are done by other frameworks, right?
>>>
>>>>> Moreover, this driver seems limited in support of variety of the eeprom chips.
>>>>
>>>> Not quite sure what you mean. The cat24c208 is what this was developed for and
>>>> the only one we have.
>>>>
>>>> Note that an EDID EEPROM != a regular EEPROM: it has to support the VESA E-DDC
>>>> standard, which a normal EEPROM doesn't. So these devices are specifically made
>>>> for this use-case.
>>>
>>> What is the difference from a programming interface?
>>> Can the nvmem driver(s) be reused (at24?)?
>>
>> No. EDID EEPROM devices are specific to storing EDIDs: they have two i2c
>> ports, one connected to (typically) the HDMI bus (DDC lines) allowing a
>> video source to read the EDID, the other is connected to the SoC to write to
>> and configure the device. The HDMI bus side has two i2c addresses (reading the
>> EEPROM and to write to the segment address for EDIDs > 256 bytes), the SoC
>> side has three i2c addresses: to configure the behavior, the segment address,
>> and to write the EDID from the SoC.
>>
>> So it is a much more complex device than a regular eeprom, and it really
>> is dedicated to EDIDs only.
> 
> Thanks for the explanation, but it's still unclear what the
> differences are in the programming interface there. Perhaps you may
> simply register a platform device in this driver and reuse the rest
> from at24?

No, it's really different from a regular eeprom.

> 
>> Also note that the V4L2 API is already used to get/set EDIDs, everything is
>> in place for supporting that, including support for parsing EDIDs for the
>> physical address, which is something that is needed if this is combined with
>> HDMI CEC hardware. It's not implemented in this driver since it is not
>> needed in our use-case, but that might change in the future.
>>
>> And by using the V4L2 API you can use v4l2-ctl --get-edid and --set-edid
>> out of the box, using the standard API for EDIDs.
> 
> Bonus question: we have cat24c04/cat24c05 are recognized by at24
> already, are they different to cat24c08?
> 

Yes, they are different.

Regards,

	Hans

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Fri, Jul 29, 2022 at 2:11 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 29/07/2022 14:00, Andy Shevchenko wrote:
> > On Fri, Jul 29, 2022 at 9:21 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >> On 28/07/2022 22:56, Andy Shevchenko wrote:
> >>> On Thu, Jul 28, 2022 at 3:23 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>>> On 28/07/2022 14:02, Andy Shevchenko wrote:
> >>>>> On Thursday, July 28, 2022, Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>> wrote:
> >>>
> >>>>>     Support reading and writing the EDID EEPROM through the
> >>>>>     v4l2 API.
> >>>>>
> >>>>> Why the normal way of representing as a memory (we have framework and drivers) can’t work?
> >>>>
> >>>> Because support for EDID for video sinks is already part of the media subsystem (V4L2).
> >>>> Normally it is integrated into an HDMI receiver, but in this case it is just the EDID
> >>>> support without the video receiver. It belongs in drivers/media in any case since EDIDs
> >>>> are closely tied to media.
> >>>
> >>> It's fine. From the Linux perspective we do not reduplicate the
> >>> drivers that are done by other frameworks, right?
> >>>
> >>>>> Moreover, this driver seems limited in support of variety of the eeprom chips.
> >>>>
> >>>> Not quite sure what you mean. The cat24c208 is what this was developed for and
> >>>> the only one we have.
> >>>>
> >>>> Note that an EDID EEPROM != a regular EEPROM: it has to support the VESA E-DDC
> >>>> standard, which a normal EEPROM doesn't. So these devices are specifically made
> >>>> for this use-case.
> >>>
> >>> What is the difference from a programming interface?
> >>> Can the nvmem driver(s) be reused (at24?)?
> >>
> >> No. EDID EEPROM devices are specific to storing EDIDs: they have two i2c
> >> ports, one connected to (typically) the HDMI bus (DDC lines) allowing a
> >> video source to read the EDID, the other is connected to the SoC to write to
> >> and configure the device. The HDMI bus side has two i2c addresses (reading the
> >> EEPROM and to write to the segment address for EDIDs > 256 bytes), the SoC
> >> side has three i2c addresses: to configure the behavior, the segment address,
> >> and to write the EDID from the SoC.
> >>
> >> So it is a much more complex device than a regular eeprom, and it really
> >> is dedicated to EDIDs only.
> >
> > Thanks for the explanation, but it's still unclear what the
> > differences are in the programming interface there. Perhaps you may
> > simply register a platform device in this driver and reuse the rest
> > from at24?
>
> No, it's really different from a regular eeprom.
>
> >> Also note that the V4L2 API is already used to get/set EDIDs, everything is
> >> in place for supporting that, including support for parsing EDIDs for the
> >> physical address, which is something that is needed if this is combined with
> >> HDMI CEC hardware. It's not implemented in this driver since it is not
> >> needed in our use-case, but that might change in the future.
> >>
> >> And by using the V4L2 API you can use v4l2-ctl --get-edid and --set-edid
> >> out of the box, using the standard API for EDIDs.
> >
> > Bonus question: we have cat24c04/cat24c05 are recognized by at24
> > already, are they different to cat24c08?
> >
>
> Yes, they are different.

Thanks for your patience and elaboration, I got it.

Would this driver be used only by v4l2? Or potentially some other
hardware may need it (DRM?)?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

On 29/07/2022 16:47, Andy Shevchenko wrote:
> On Fri, Jul 29, 2022 at 2:11 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>> On 29/07/2022 14:00, Andy Shevchenko wrote:
>>> On Fri, Jul 29, 2022 at 9:21 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>>> On 28/07/2022 22:56, Andy Shevchenko wrote:
>>>>> On Thu, Jul 28, 2022 at 3:23 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>>>>> On 28/07/2022 14:02, Andy Shevchenko wrote:
>>>>>>> On Thursday, July 28, 2022, Erling Ljunggren <hljunggr@cisco.com <mailto:hljunggr@cisco.com>> wrote:
>>>>>
>>>>>>>     Support reading and writing the EDID EEPROM through the
>>>>>>>     v4l2 API.
>>>>>>>
>>>>>>> Why the normal way of representing as a memory (we have framework and drivers) can’t work?
>>>>>>
>>>>>> Because support for EDID for video sinks is already part of the media subsystem (V4L2).
>>>>>> Normally it is integrated into an HDMI receiver, but in this case it is just the EDID
>>>>>> support without the video receiver. It belongs in drivers/media in any case since EDIDs
>>>>>> are closely tied to media.
>>>>>
>>>>> It's fine. From the Linux perspective we do not reduplicate the
>>>>> drivers that are done by other frameworks, right?
>>>>>
>>>>>>> Moreover, this driver seems limited in support of variety of the eeprom chips.
>>>>>>
>>>>>> Not quite sure what you mean. The cat24c208 is what this was developed for and
>>>>>> the only one we have.
>>>>>>
>>>>>> Note that an EDID EEPROM != a regular EEPROM: it has to support the VESA E-DDC
>>>>>> standard, which a normal EEPROM doesn't. So these devices are specifically made
>>>>>> for this use-case.
>>>>>
>>>>> What is the difference from a programming interface?
>>>>> Can the nvmem driver(s) be reused (at24?)?
>>>>
>>>> No. EDID EEPROM devices are specific to storing EDIDs: they have two i2c
>>>> ports, one connected to (typically) the HDMI bus (DDC lines) allowing a
>>>> video source to read the EDID, the other is connected to the SoC to write to
>>>> and configure the device. The HDMI bus side has two i2c addresses (reading the
>>>> EEPROM and to write to the segment address for EDIDs > 256 bytes), the SoC
>>>> side has three i2c addresses: to configure the behavior, the segment address,
>>>> and to write the EDID from the SoC.
>>>>
>>>> So it is a much more complex device than a regular eeprom, and it really
>>>> is dedicated to EDIDs only.
>>>
>>> Thanks for the explanation, but it's still unclear what the
>>> differences are in the programming interface there. Perhaps you may
>>> simply register a platform device in this driver and reuse the rest
>>> from at24?
>>
>> No, it's really different from a regular eeprom.
>>
>>>> Also note that the V4L2 API is already used to get/set EDIDs, everything is
>>>> in place for supporting that, including support for parsing EDIDs for the
>>>> physical address, which is something that is needed if this is combined with
>>>> HDMI CEC hardware. It's not implemented in this driver since it is not
>>>> needed in our use-case, but that might change in the future.
>>>>
>>>> And by using the V4L2 API you can use v4l2-ctl --get-edid and --set-edid
>>>> out of the box, using the standard API for EDIDs.
>>>
>>> Bonus question: we have cat24c04/cat24c05 are recognized by at24
>>> already, are they different to cat24c08?
>>>
>>
>> Yes, they are different.
> 
> Thanks for your patience and elaboration, I got it.
> 
> Would this driver be used only by v4l2? Or potentially some other
> hardware may need it (DRM?)?

Only V4L2: an EDID describes the capabilities of a video sink (e.g. a display),
so it is specific to video receivers, and that's the domain of V4L2.

Regards,

	Hans

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com> wrote:
>
> From: Jonathan Selnes <jonathansb1@gmail.com>
>
> Support reading and writing the EDID EEPROM through the
> v4l2 API.
>
> Signed-off-by: Jonathan Selnes <jonathansb1@gmail.com>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>

Wondering if the last two people don't do any development, otherwise
Co-developed-by would be appreciated.

...

>  obj-$(CONFIG_VIDEO_HI556) += hi556.o
>  obj-$(CONFIG_VIDEO_HI846) += hi846.o
>  obj-$(CONFIG_VIDEO_HI847) += hi847.o
> +obj-$(CONFIG_VIDEO_CAT24C208) += cat24c208.o

Perhaps more sorted?

>  obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
>  obj-$(CONFIG_VIDEO_IMX208) += imx208.o
>  obj-$(CONFIG_VIDEO_IMX214) += imx214.o

...

> +/*
> + * cat24c208 - HDMI i2c controlled EEPROM from ON Semiconductor or Catalyst Semiconductor

Here...

> + * cat24c208.c - Support for i2c based DDC EEPROM

...and here and in general putting filename into the file is not a
good idea in the long term. For example, this can be expanded in the
future to support more EDID EEPROMs, and if we want to rename, this
adds an additional burden.

> + * Copyright (C) 2021-2022 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
> + */

...

> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>

> +#include <linux/of_device.h>

Why? Who is the user of this?
Perhaps you meant mod_devicetable.h, which is currently missed?

> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/videodev2.h>

> +#include <linux/kernel.h>

Perhaps keep it ordered?

...

> +/*
> + * From the datasheet:

Maybe  add an URL to the Datasheet?

> + * The write cycle time is the time from a valid stop condition of a write
> + * sequence to the end of the internal program/erase cycle. During the write
> + * cycle, the bus interface circuits are disabled, SDA is allowed to remain
> + * high, and the device does not respond to its slave address.
> + */
> +#define WRITE_CYCLE_TIME_US    5000

...

> +       struct i2c_client *dev_client = state->i2c_clients[0];
> +       struct i2c_client *data_client = state->i2c_clients[1];
> +       struct i2c_client *seg_client = state->i2c_clients[2];

Why not have those clients named accordingly in the data struct,
instead of indexing them?

...

> +       if (seg)
> +               err = i2c_transfer(dev_client->adapter, msg, ARRAY_SIZE(msg));
> +       else
> +               err = i2c_transfer(dev_client->adapter, &msg[1], 1);
> +       if (err < 0)
> +               dev_err(&dev_client->dev, "Writing to 0x%x failed (segment %d)\n",
> +                       data_client->addr, seg);

> +       usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);

Sleep even in case of error? Is it required?
(Same Q per other similar places)

> +       return err < 0 ? err : 0;

Hence here...

...

> +       if (err < 0)
> +               dev_err(&dev_client->dev, "Reading of EDID failed\n");
> +       return err < 0 ? err : 0;

...and here we can avoid a duplication test for error code being set, right?
(Same suggestion per other similar cases)

...

> +       static const u8 header_pattern[] = {
> +               0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00

Keeping a comma at the end is good anyway.

> +       };

...

> +       state = kzalloc(sizeof(*state), GFP_KERNEL);
> +       if (!state)
> +               return -ENOMEM;

devm_kzalloc() ?

...

> +               blocks = 1 + state->edid[126];

Magic index.

...

> +               .of_match_table = of_match_ptr(cat24c208_of_match),

of_match_ptr() brings more harm than help.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

Hi Erling,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linus/master v5.19-rc8 next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Erling-Ljunggren/Add-the-cat24c208-EDID-EEPROM-driver-new-EDID-capability/20220728-194524
base:   git://linuxtv.org/media_tree.git master
reproduce: make htmldocs

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> Documentation/userspace-api/media/v4l/biblio.rst:340: WARNING: Title underline too short.

vim +340 Documentation/userspace-api/media/v4l/biblio.rst

   338	
   339	E-DDC
 > 340	====
   341	
   342	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

[-- Attachment #2: config --]
[-- Type: text/plain, Size: 31459 bytes --]

#
# Automatically generated file; DO NOT EDIT.
# Linux/i386 5.19.0-rc3 Kernel Configuration
#
CONFIG_CC_VERSION_TEXT="gcc-11 (Debian 11.3.0-3) 11.3.0"
CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=110300
CONFIG_CLANG_VERSION=0
CONFIG_AS_IS_GNU=y
CONFIG_AS_VERSION=23800
CONFIG_LD_IS_BFD=y
CONFIG_LD_VERSION=23800
CONFIG_LLD_VERSION=0
CONFIG_CC_CAN_LINK=y
CONFIG_CC_CAN_LINK_STATIC=y
CONFIG_CC_HAS_ASM_GOTO=y
CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
CONFIG_CC_HAS_ASM_INLINE=y
CONFIG_CC_HAS_NO_PROFILE_FN_ATTR=y
CONFIG_PAHOLE_VERSION=123
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_TABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y

#
# General setup
#
CONFIG_BROKEN_ON_SMP=y
CONFIG_INIT_ENV_ARG_LIMIT=32
# CONFIG_COMPILE_TEST is not set
# CONFIG_WERROR is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_BUILD_SALT=""
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_HAVE_KERNEL_ZSTD=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
# CONFIG_KERNEL_ZSTD is not set
CONFIG_DEFAULT_INIT=""
CONFIG_DEFAULT_HOSTNAME="(none)"
# CONFIG_SYSVIPC is not set
# CONFIG_WATCH_QUEUE is not set
# CONFIG_CROSS_MEMORY_ATTACH is not set
# CONFIG_USELIB is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_HARDIRQS_SW_RESEND=y
CONFIG_GENERIC_IRQ_RESERVATION_MODE=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
# end of IRQ subsystem

CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_INIT=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y
CONFIG_HAVE_POSIX_CPU_TIMERS_TASK_WORK=y
CONFIG_POSIX_CPU_TIMERS_TASK_WORK=y

#
# Timers subsystem
#
CONFIG_HZ_PERIODIC=y
# CONFIG_NO_HZ_IDLE is not set
# CONFIG_NO_HZ is not set
# CONFIG_HIGH_RES_TIMERS is not set
CONFIG_CLOCKSOURCE_WATCHDOG_MAX_SKEW_US=100
# end of Timers subsystem

CONFIG_HAVE_EBPF_JIT=y

#
# BPF subsystem
#
# CONFIG_BPF_SYSCALL is not set
# end of BPF subsystem

CONFIG_PREEMPT_NONE_BUILD=y
CONFIG_PREEMPT_NONE=y
# CONFIG_PREEMPT_VOLUNTARY is not set
# CONFIG_PREEMPT is not set
# CONFIG_PREEMPT_DYNAMIC is not set

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_IRQ_TIME_ACCOUNTING is not set
# CONFIG_BSD_PROCESS_ACCT is not set
# CONFIG_PSI is not set
# end of CPU/Task time and stats accounting

#
# RCU Subsystem
#
CONFIG_TINY_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
CONFIG_TINY_SRCU=y
# end of RCU Subsystem

# CONFIG_IKCONFIG is not set
# CONFIG_IKHEADERS is not set
CONFIG_LOG_BUF_SHIFT=17
CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=13
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y

#
# Scheduler features
#
# end of Scheduler features

CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y
CONFIG_CC_IMPLICIT_FALLTHROUGH="-Wimplicit-fallthrough=5"
CONFIG_GCC12_NO_ARRAY_BOUNDS=y
# CONFIG_CGROUPS is not set
CONFIG_NAMESPACES=y
# CONFIG_UTS_NS is not set
# CONFIG_TIME_NS is not set
# CONFIG_USER_NS is not set
# CONFIG_PID_NS is not set
# CONFIG_CHECKPOINT_RESTORE is not set
# CONFIG_SCHED_AUTOGROUP is not set
# CONFIG_SYSFS_DEPRECATED is not set
# CONFIG_RELAY is not set
# CONFIG_BLK_DEV_INITRD is not set
# CONFIG_BOOT_CONFIG is not set
# CONFIG_INITRAMFS_PRESERVE_MTIME is not set
CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y
# CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
CONFIG_LD_ORPHAN_WARN=y
CONFIG_SYSCTL=y
CONFIG_HAVE_UID16=y
CONFIG_SYSCTL_EXCEPTION_TRACE=y
CONFIG_HAVE_PCSPKR_PLATFORM=y
# CONFIG_EXPERT is not set
CONFIG_UID16=y
CONFIG_MULTIUSER=y
CONFIG_SGETMASK_SYSCALL=y
CONFIG_SYSFS_SYSCALL=y
CONFIG_FHANDLE=y
CONFIG_POSIX_TIMERS=y
CONFIG_PRINTK=y
CONFIG_BUG=y
CONFIG_ELF_CORE=y
CONFIG_PCSPKR_PLATFORM=y
CONFIG_BASE_FULL=y
CONFIG_FUTEX=y
CONFIG_FUTEX_PI=y
CONFIG_EPOLL=y
CONFIG_SIGNALFD=y
CONFIG_TIMERFD=y
CONFIG_EVENTFD=y
CONFIG_SHMEM=y
CONFIG_AIO=y
CONFIG_IO_URING=y
CONFIG_ADVISE_SYSCALLS=y
CONFIG_MEMBARRIER=y
CONFIG_KALLSYMS=y
CONFIG_KALLSYMS_BASE_RELATIVE=y
CONFIG_ARCH_HAS_MEMBARRIER_SYNC_CORE=y
CONFIG_RSEQ=y
# CONFIG_EMBEDDED is not set
CONFIG_HAVE_PERF_EVENTS=y

#
# Kernel Performance Events And Counters
#
CONFIG_PERF_EVENTS=y
# end of Kernel Performance Events And Counters

# CONFIG_PROFILING is not set
# end of General setup

CONFIG_X86_32=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf32-i386"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_BITS_MAX=16
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_NR_GPIO=512
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=2
CONFIG_CC_HAS_SANE_STACKPROTECTOR=y

#
# Processor type and features
#
# CONFIG_SMP is not set
CONFIG_X86_FEATURE_NAMES=y
# CONFIG_GOLDFISH is not set
CONFIG_RETPOLINE=y
CONFIG_CC_HAS_SLS=y
# CONFIG_X86_CPU_RESCTRL is not set
# CONFIG_X86_EXTENDED_PLATFORM is not set
# CONFIG_X86_32_IRIS is not set
# CONFIG_SCHED_OMIT_FRAME_POINTER is not set
# CONFIG_HYPERVISOR_GUEST is not set
# CONFIG_M486SX is not set
# CONFIG_M486 is not set
# CONFIG_M586 is not set
# CONFIG_M586TSC is not set
# CONFIG_M586MMX is not set
CONFIG_M686=y
# CONFIG_MPENTIUMII is not set
# CONFIG_MPENTIUMIII is not set
# CONFIG_MPENTIUMM is not set
# CONFIG_MPENTIUM4 is not set
# CONFIG_MK6 is not set
# CONFIG_MK7 is not set
# CONFIG_MK8 is not set
# CONFIG_MCRUSOE is not set
# CONFIG_MEFFICEON is not set
# CONFIG_MWINCHIPC6 is not set
# CONFIG_MWINCHIP3D is not set
# CONFIG_MELAN is not set
# CONFIG_MGEODEGX1 is not set
# CONFIG_MGEODE_LX is not set
# CONFIG_MCYRIXIII is not set
# CONFIG_MVIAC3_2 is not set
# CONFIG_MVIAC7 is not set
# CONFIG_MCORE2 is not set
# CONFIG_MATOM is not set
# CONFIG_X86_GENERIC is not set
CONFIG_X86_INTERNODE_CACHE_SHIFT=5
CONFIG_X86_L1_CACHE_SHIFT=5
CONFIG_X86_USE_PPRO_CHECKSUM=y
CONFIG_X86_TSC=y
CONFIG_X86_CMPXCHG64=y
CONFIG_X86_CMOV=y
CONFIG_X86_MINIMUM_CPU_FAMILY=6
CONFIG_X86_DEBUGCTLMSR=y
CONFIG_IA32_FEAT_CTL=y
CONFIG_X86_VMX_FEATURE_NAMES=y
CONFIG_CPU_SUP_INTEL=y
CONFIG_CPU_SUP_AMD=y
CONFIG_CPU_SUP_HYGON=y
CONFIG_CPU_SUP_CENTAUR=y
CONFIG_CPU_SUP_TRANSMETA_32=y
CONFIG_CPU_SUP_ZHAOXIN=y
CONFIG_CPU_SUP_VORTEX_32=y
# CONFIG_HPET_TIMER is not set
CONFIG_DMI=y
CONFIG_NR_CPUS_RANGE_BEGIN=1
CONFIG_NR_CPUS_RANGE_END=1
CONFIG_NR_CPUS_DEFAULT=1
CONFIG_NR_CPUS=1
# CONFIG_X86_UP_APIC is not set
# CONFIG_X86_MCE is not set

#
# Performance monitoring
#
# CONFIG_PERF_EVENTS_AMD_POWER is not set
# CONFIG_PERF_EVENTS_AMD_UNCORE is not set
# CONFIG_PERF_EVENTS_AMD_BRS is not set
# end of Performance monitoring

# CONFIG_X86_LEGACY_VM86 is not set
CONFIG_X86_16BIT=y
CONFIG_X86_ESPFIX32=y
# CONFIG_X86_IOPL_IOPERM is not set
# CONFIG_TOSHIBA is not set
# CONFIG_X86_REBOOTFIXUPS is not set
# CONFIG_MICROCODE is not set
# CONFIG_X86_MSR is not set
# CONFIG_X86_CPUID is not set
# CONFIG_NOHIGHMEM is not set
CONFIG_HIGHMEM4G=y
# CONFIG_HIGHMEM64G is not set
CONFIG_PAGE_OFFSET=0xC0000000
CONFIG_HIGHMEM=y
CONFIG_ARCH_FLATMEM_ENABLE=y
CONFIG_ARCH_SPARSEMEM_ENABLE=y
CONFIG_ARCH_SELECT_MEMORY_MODEL=y
CONFIG_ILLEGAL_POINTER_VALUE=0
# CONFIG_HIGHPTE is not set
# CONFIG_X86_CHECK_BIOS_CORRUPTION is not set
CONFIG_MTRR=y
# CONFIG_MTRR_SANITIZER is not set
CONFIG_X86_PAT=y
CONFIG_ARCH_USES_PG_UNCACHED=y
CONFIG_ARCH_RANDOM=y
CONFIG_X86_UMIP=y
CONFIG_CC_HAS_IBT=y
CONFIG_X86_INTEL_TSX_MODE_OFF=y
# CONFIG_X86_INTEL_TSX_MODE_ON is not set
# CONFIG_X86_INTEL_TSX_MODE_AUTO is not set
# CONFIG_HZ_100 is not set
CONFIG_HZ_250=y
# CONFIG_HZ_300 is not set
# CONFIG_HZ_1000 is not set
CONFIG_HZ=250
# CONFIG_KEXEC is not set
# CONFIG_CRASH_DUMP is not set
CONFIG_PHYSICAL_START=0x1000000
# CONFIG_RELOCATABLE is not set
CONFIG_PHYSICAL_ALIGN=0x200000
# CONFIG_COMPAT_VDSO is not set
# CONFIG_CMDLINE_BOOL is not set
CONFIG_MODIFY_LDT_SYSCALL=y
# CONFIG_STRICT_SIGALTSTACK_SIZE is not set
# end of Processor type and features

CONFIG_ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE=y

#
# Power management and ACPI options
#
# CONFIG_SUSPEND is not set
# CONFIG_PM is not set
CONFIG_ARCH_SUPPORTS_ACPI=y
# CONFIG_ACPI is not set

#
# CPU Frequency scaling
#
# CONFIG_CPU_FREQ is not set
# end of CPU Frequency scaling

#
# CPU Idle
#
# CONFIG_CPU_IDLE is not set
# end of CPU Idle
# end of Power management and ACPI options

#
# Bus options (PCI etc.)
#
CONFIG_ISA_DMA_API=y
# CONFIG_ISA is not set
# CONFIG_SCx200 is not set
# CONFIG_OLPC is not set
# CONFIG_ALIX is not set
# CONFIG_NET5501 is not set
# CONFIG_GEOS is not set
# end of Bus options (PCI etc.)

#
# Binary Emulations
#
CONFIG_COMPAT_32=y
# end of Binary Emulations

CONFIG_HAVE_ATOMIC_IOMAP=y
CONFIG_HAVE_KVM=y
# CONFIG_VIRTUALIZATION is not set
CONFIG_AS_AVX512=y
CONFIG_AS_SHA1_NI=y
CONFIG_AS_SHA256_NI=y
CONFIG_AS_TPAUSE=y

#
# General architecture-dependent options
#
CONFIG_GENERIC_ENTRY=y
# CONFIG_JUMP_LABEL is not set
# CONFIG_STATIC_CALL_SELFTEST is not set
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y
CONFIG_ARCH_USE_BUILTIN_BSWAP=y
CONFIG_HAVE_IOREMAP_PROT=y
CONFIG_HAVE_KPROBES=y
CONFIG_HAVE_KRETPROBES=y
CONFIG_HAVE_OPTPROBES=y
CONFIG_HAVE_KPROBES_ON_FTRACE=y
CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE=y
CONFIG_HAVE_FUNCTION_ERROR_INJECTION=y
CONFIG_HAVE_NMI=y
CONFIG_TRACE_IRQFLAGS_SUPPORT=y
CONFIG_HAVE_ARCH_TRACEHOOK=y
CONFIG_HAVE_DMA_CONTIGUOUS=y
CONFIG_GENERIC_SMP_IDLE_THREAD=y
CONFIG_ARCH_HAS_FORTIFY_SOURCE=y
CONFIG_ARCH_HAS_SET_MEMORY=y
CONFIG_ARCH_HAS_SET_DIRECT_MAP=y
CONFIG_HAVE_ARCH_THREAD_STRUCT_WHITELIST=y
CONFIG_ARCH_WANTS_DYNAMIC_TASK_STRUCT=y
CONFIG_ARCH_WANTS_NO_INSTR=y
CONFIG_ARCH_32BIT_OFF_T=y
CONFIG_HAVE_ASM_MODVERSIONS=y
CONFIG_HAVE_REGS_AND_STACK_ACCESS_API=y
CONFIG_HAVE_RSEQ=y
CONFIG_HAVE_FUNCTION_ARG_ACCESS_API=y
CONFIG_HAVE_HW_BREAKPOINT=y
CONFIG_HAVE_MIXED_BREAKPOINTS_REGS=y
CONFIG_HAVE_USER_RETURN_NOTIFIER=y
CONFIG_HAVE_PERF_EVENTS_NMI=y
CONFIG_HAVE_HARDLOCKUP_DETECTOR_PERF=y
CONFIG_HAVE_PERF_REGS=y
CONFIG_HAVE_PERF_USER_STACK_DUMP=y
CONFIG_HAVE_ARCH_JUMP_LABEL=y
CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE=y
CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG=y
CONFIG_HAVE_ALIGNED_STRUCT_PAGE=y
CONFIG_HAVE_CMPXCHG_LOCAL=y
CONFIG_HAVE_CMPXCHG_DOUBLE=y
CONFIG_ARCH_WANT_IPC_PARSE_VERSION=y
CONFIG_HAVE_ARCH_SECCOMP=y
CONFIG_HAVE_ARCH_SECCOMP_FILTER=y
# CONFIG_SECCOMP is not set
CONFIG_HAVE_ARCH_STACKLEAK=y
CONFIG_HAVE_STACKPROTECTOR=y
# CONFIG_STACKPROTECTOR is not set
CONFIG_ARCH_SUPPORTS_LTO_CLANG=y
CONFIG_ARCH_SUPPORTS_LTO_CLANG_THIN=y
CONFIG_LTO_NONE=y
CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES=y
CONFIG_HAVE_IRQ_TIME_ACCOUNTING=y
CONFIG_HAVE_MOVE_PUD=y
CONFIG_HAVE_MOVE_PMD=y
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_HAVE_MOD_ARCH_SPECIFIC=y
CONFIG_MODULES_USE_ELF_REL=y
CONFIG_HAVE_SOFTIRQ_ON_OWN_STACK=y
CONFIG_ARCH_HAS_ELF_RANDOMIZE=y
CONFIG_HAVE_ARCH_MMAP_RND_BITS=y
CONFIG_HAVE_EXIT_THREAD=y
CONFIG_ARCH_MMAP_RND_BITS=8
CONFIG_PAGE_SIZE_LESS_THAN_64KB=y
CONFIG_PAGE_SIZE_LESS_THAN_256KB=y
CONFIG_CLONE_BACKWARDS=y
CONFIG_OLD_SIGSUSPEND3=y
CONFIG_OLD_SIGACTION=y
# CONFIG_COMPAT_32BIT_TIME is not set
CONFIG_HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET=y
CONFIG_RANDOMIZE_KSTACK_OFFSET=y
# CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT is not set
CONFIG_ARCH_HAS_STRICT_KERNEL_RWX=y
CONFIG_STRICT_KERNEL_RWX=y
CONFIG_ARCH_HAS_STRICT_MODULE_RWX=y
CONFIG_HAVE_ARCH_PREL32_RELOCATIONS=y
CONFIG_ARCH_HAS_MEM_ENCRYPT=y
CONFIG_HAVE_STATIC_CALL=y
CONFIG_HAVE_PREEMPT_DYNAMIC=y
CONFIG_HAVE_PREEMPT_DYNAMIC_CALL=y
CONFIG_ARCH_WANT_LD_ORPHAN_WARN=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_ARCH_SPLIT_ARG64=y
CONFIG_ARCH_HAS_PARANOID_L1D_FLUSH=y
CONFIG_DYNAMIC_SIGFRAME=y

#
# GCOV-based kernel profiling
#
CONFIG_ARCH_HAS_GCOV_PROFILE_ALL=y
# end of GCOV-based kernel profiling

CONFIG_HAVE_GCC_PLUGINS=y
# CONFIG_GCC_PLUGINS is not set
# end of General architecture-dependent options

CONFIG_RT_MUTEXES=y
CONFIG_BASE_SMALL=0
# CONFIG_MODULES is not set
CONFIG_MODULES_TREE_LOOKUP=y
CONFIG_BLOCK=y
# CONFIG_BLOCK_LEGACY_AUTOLOAD is not set
# CONFIG_BLK_DEV_BSGLIB is not set
# CONFIG_BLK_DEV_INTEGRITY is not set
# CONFIG_BLK_DEV_ZONED is not set
# CONFIG_BLK_WBT is not set
# CONFIG_BLK_SED_OPAL is not set
# CONFIG_BLK_INLINE_ENCRYPTION is not set

#
# Partition Types
#
# CONFIG_PARTITION_ADVANCED is not set
CONFIG_MSDOS_PARTITION=y
CONFIG_EFI_PARTITION=y
# end of Partition Types

#
# IO Schedulers
#
# CONFIG_MQ_IOSCHED_DEADLINE is not set
# CONFIG_MQ_IOSCHED_KYBER is not set
# CONFIG_IOSCHED_BFQ is not set
# end of IO Schedulers

CONFIG_INLINE_SPIN_UNLOCK_IRQ=y
CONFIG_INLINE_READ_UNLOCK=y
CONFIG_INLINE_READ_UNLOCK_IRQ=y
CONFIG_INLINE_WRITE_UNLOCK=y
CONFIG_INLINE_WRITE_UNLOCK_IRQ=y
CONFIG_ARCH_SUPPORTS_ATOMIC_RMW=y
CONFIG_ARCH_USE_QUEUED_SPINLOCKS=y
CONFIG_ARCH_USE_QUEUED_RWLOCKS=y
CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE=y
CONFIG_ARCH_HAS_SYNC_CORE_BEFORE_USERMODE=y
CONFIG_ARCH_HAS_SYSCALL_WRAPPER=y

#
# Executable file formats
#
# CONFIG_BINFMT_ELF is not set
# CONFIG_BINFMT_SCRIPT is not set
# CONFIG_BINFMT_MISC is not set
CONFIG_COREDUMP=y
# end of Executable file formats

#
# Memory Management options
#
# CONFIG_SWAP is not set

#
# SLAB allocator options
#
# CONFIG_SLAB is not set
CONFIG_SLUB=y
# CONFIG_SLAB_MERGE_DEFAULT is not set
# CONFIG_SLAB_FREELIST_RANDOM is not set
# CONFIG_SLAB_FREELIST_HARDENED is not set
# CONFIG_SLUB_STATS is not set
# end of SLAB allocator options

# CONFIG_SHUFFLE_PAGE_ALLOCATOR is not set
# CONFIG_COMPAT_BRK is not set
CONFIG_SELECT_MEMORY_MODEL=y
CONFIG_FLATMEM_MANUAL=y
# CONFIG_SPARSEMEM_MANUAL is not set
CONFIG_FLATMEM=y
CONFIG_SPARSEMEM_STATIC=y
CONFIG_HAVE_FAST_GUP=y
CONFIG_EXCLUSIVE_SYSTEM_RAM=y
CONFIG_SPLIT_PTLOCK_CPUS=4
# CONFIG_COMPACTION is not set
# CONFIG_PAGE_REPORTING is not set
# CONFIG_BOUNCE is not set
CONFIG_VIRT_TO_BUS=y
# CONFIG_KSM is not set
CONFIG_DEFAULT_MMAP_MIN_ADDR=4096
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
# CONFIG_TRANSPARENT_HUGEPAGE is not set
CONFIG_NEED_PER_CPU_KM=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
# CONFIG_CMA is not set
CONFIG_GENERIC_EARLY_IOREMAP=y
# CONFIG_IDLE_PAGE_TRACKING is not set
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_CURRENT_STACK_POINTER=y
CONFIG_ARCH_HAS_VM_GET_PAGE_PROT=y
CONFIG_ZONE_DMA=y
CONFIG_VM_EVENT_COUNTERS=y
# CONFIG_PERCPU_STATS is not set

#
# GUP_TEST needs to have DEBUG_FS enabled
#
CONFIG_ARCH_HAS_PTE_SPECIAL=y
CONFIG_KMAP_LOCAL=y
CONFIG_SECRETMEM=y
# CONFIG_ANON_VMA_NAME is not set
# CONFIG_USERFAULTFD is not set

#
# Data Access Monitoring
#
# CONFIG_DAMON is not set
# end of Data Access Monitoring
# end of Memory Management options

# CONFIG_NET is not set

#
# Device Drivers
#
CONFIG_HAVE_EISA=y
# CONFIG_EISA is not set
CONFIG_HAVE_PCI=y
# CONFIG_PCI is not set
# CONFIG_PCCARD is not set

#
# Generic Driver Options
#
# CONFIG_UEVENT_HELPER is not set
# CONFIG_DEVTMPFS is not set
# CONFIG_STANDALONE is not set
# CONFIG_PREVENT_FIRMWARE_BUILD is not set

#
# Firmware loader
#
CONFIG_FW_LOADER=y
CONFIG_EXTRA_FIRMWARE=""
# CONFIG_FW_LOADER_USER_HELPER is not set
# CONFIG_FW_LOADER_COMPRESS is not set
# CONFIG_FW_UPLOAD is not set
# end of Firmware loader

CONFIG_ALLOW_DEV_COREDUMP=y
CONFIG_GENERIC_CPU_AUTOPROBE=y
CONFIG_GENERIC_CPU_VULNERABILITIES=y
# end of Generic Driver Options

#
# Bus devices
#
# CONFIG_MHI_BUS is not set
# CONFIG_MHI_BUS_EP is not set
# end of Bus devices

#
# Firmware Drivers
#

#
# ARM System Control and Management Interface Protocol
#
# end of ARM System Control and Management Interface Protocol

# CONFIG_EDD is not set
CONFIG_FIRMWARE_MEMMAP=y
# CONFIG_DMIID is not set
# CONFIG_DMI_SYSFS is not set
CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK=y
# CONFIG_FW_CFG_SYSFS is not set
# CONFIG_SYSFB_SIMPLEFB is not set
# CONFIG_GOOGLE_FIRMWARE is not set

#
# Tegra firmware driver
#
# end of Tegra firmware driver
# end of Firmware Drivers

# CONFIG_GNSS is not set
# CONFIG_MTD is not set
# CONFIG_OF is not set
CONFIG_ARCH_MIGHT_HAVE_PC_PARPORT=y
# CONFIG_PARPORT is not set
# CONFIG_BLK_DEV is not set

#
# NVME Support
#
# CONFIG_NVME_FC is not set
# end of NVME Support

#
# Misc devices
#
# CONFIG_DUMMY_IRQ is not set
# CONFIG_ENCLOSURE_SERVICES is not set
# CONFIG_SRAM is not set
# CONFIG_XILINX_SDFEC is not set
# CONFIG_C2PORT is not set

#
# EEPROM support
#
# CONFIG_EEPROM_93CX6 is not set
# end of EEPROM support

#
# Texas Instruments shared transport line discipline
#
# end of Texas Instruments shared transport line discipline

#
# Altera FPGA firmware download module (requires I2C)
#
# CONFIG_ECHO is not set
# CONFIG_PVPANIC is not set
# end of Misc devices

#
# SCSI device support
#
CONFIG_SCSI_MOD=y
# CONFIG_RAID_ATTRS is not set
# CONFIG_SCSI is not set
# end of SCSI device support

# CONFIG_ATA is not set
# CONFIG_MD is not set
# CONFIG_TARGET_CORE is not set
# CONFIG_MACINTOSH_DRIVERS is not set

#
# Input device support
#
CONFIG_INPUT=y
# CONFIG_INPUT_FF_MEMLESS is not set
# CONFIG_INPUT_SPARSEKMAP is not set
# CONFIG_INPUT_MATRIXKMAP is not set

#
# Userland interfaces
#
# CONFIG_INPUT_MOUSEDEV is not set
# CONFIG_INPUT_JOYDEV is not set
# CONFIG_INPUT_EVDEV is not set
# CONFIG_INPUT_EVBUG is not set

#
# Input Device Drivers
#
# CONFIG_INPUT_KEYBOARD is not set
# CONFIG_INPUT_MOUSE is not set
# CONFIG_INPUT_JOYSTICK is not set
# CONFIG_INPUT_TABLET is not set
# CONFIG_INPUT_TOUCHSCREEN is not set
# CONFIG_INPUT_MISC is not set
# CONFIG_RMI4_CORE is not set

#
# Hardware I/O ports
#
# CONFIG_SERIO is not set
CONFIG_ARCH_MIGHT_HAVE_PC_SERIO=y
# CONFIG_GAMEPORT is not set
# end of Hardware I/O ports
# end of Input device support

#
# Character devices
#
CONFIG_TTY=y
CONFIG_VT=y
CONFIG_CONSOLE_TRANSLATIONS=y
CONFIG_VT_CONSOLE=y
CONFIG_HW_CONSOLE=y
# CONFIG_VT_HW_CONSOLE_BINDING is not set
CONFIG_UNIX98_PTYS=y
# CONFIG_LEGACY_PTYS is not set
# CONFIG_LDISC_AUTOLOAD is not set

#
# Serial drivers
#
# CONFIG_SERIAL_8250 is not set

#
# Non-8250 serial port support
#
# CONFIG_SERIAL_UARTLITE is not set
# CONFIG_SERIAL_LANTIQ is not set
# CONFIG_SERIAL_SCCNXP is not set
# CONFIG_SERIAL_TIMBERDALE is not set
# CONFIG_SERIAL_ALTERA_JTAGUART is not set
# CONFIG_SERIAL_ALTERA_UART is not set
# CONFIG_SERIAL_ARC is not set
# CONFIG_SERIAL_FSL_LPUART is not set
# CONFIG_SERIAL_FSL_LINFLEXUART is not set
# end of Serial drivers

# CONFIG_SERIAL_NONSTANDARD is not set
# CONFIG_NULL_TTY is not set
# CONFIG_SERIAL_DEV_BUS is not set
# CONFIG_VIRTIO_CONSOLE is not set
# CONFIG_IPMI_HANDLER is not set
# CONFIG_HW_RANDOM is not set
# CONFIG_MWAVE is not set
# CONFIG_PC8736x_GPIO is not set
# CONFIG_NSC_GPIO is not set
# CONFIG_DEVMEM is not set
# CONFIG_NVRAM is not set
# CONFIG_HANGCHECK_TIMER is not set
# CONFIG_TCG_TPM is not set
# CONFIG_TELCLOCK is not set
# CONFIG_RANDOM_TRUST_CPU is not set
# CONFIG_RANDOM_TRUST_BOOTLOADER is not set
# end of Character devices

#
# I2C support
#
# CONFIG_I2C is not set
# end of I2C support

# CONFIG_I3C is not set
# CONFIG_SPI is not set
# CONFIG_SPMI is not set
# CONFIG_HSI is not set
# CONFIG_PPS is not set

#
# PTP clock support
#
CONFIG_PTP_1588_CLOCK_OPTIONAL=y

#
# Enable PHYLIB and NETWORK_PHY_TIMESTAMPING to see the additional clocks.
#
# end of PTP clock support

# CONFIG_PINCTRL is not set
# CONFIG_GPIOLIB is not set
# CONFIG_W1 is not set
# CONFIG_POWER_RESET is not set
# CONFIG_POWER_SUPPLY is not set
# CONFIG_HWMON is not set
# CONFIG_THERMAL is not set
# CONFIG_WATCHDOG is not set
CONFIG_SSB_POSSIBLE=y
# CONFIG_SSB is not set
CONFIG_BCMA_POSSIBLE=y
# CONFIG_BCMA is not set

#
# Multifunction device drivers
#
# CONFIG_MFD_MADERA is not set
# CONFIG_HTC_PASIC3 is not set
# CONFIG_MFD_KEMPLD is not set
# CONFIG_MFD_MT6397 is not set
# CONFIG_MFD_SM501 is not set
# CONFIG_MFD_SYSCON is not set
# CONFIG_MFD_TI_AM335X_TSCADC is not set
# CONFIG_MFD_TQMX86 is not set
# end of Multifunction device drivers

# CONFIG_REGULATOR is not set
# CONFIG_RC_CORE is not set

#
# CEC support
#
# CONFIG_MEDIA_CEC_SUPPORT is not set
# end of CEC support

# CONFIG_MEDIA_SUPPORT is not set

#
# Graphics support
#
# CONFIG_DRM is not set

#
# ARM devices
#
# end of ARM devices

#
# Frame buffer Devices
#
# CONFIG_FB is not set
# end of Frame buffer Devices

#
# Backlight & LCD device support
#
# CONFIG_LCD_CLASS_DEVICE is not set
# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
# end of Backlight & LCD device support

#
# Console display driver support
#
CONFIG_VGA_CONSOLE=y
CONFIG_DUMMY_CONSOLE=y
CONFIG_DUMMY_CONSOLE_COLUMNS=80
CONFIG_DUMMY_CONSOLE_ROWS=25
# end of Console display driver support
# end of Graphics support

# CONFIG_SOUND is not set

#
# HID support
#
# CONFIG_HID is not set
# end of HID support

CONFIG_USB_OHCI_LITTLE_ENDIAN=y
# CONFIG_USB_SUPPORT is not set
# CONFIG_MMC is not set
# CONFIG_MEMSTICK is not set
# CONFIG_NEW_LEDS is not set
# CONFIG_ACCESSIBILITY is not set
CONFIG_EDAC_ATOMIC_SCRUB=y
CONFIG_EDAC_SUPPORT=y
CONFIG_RTC_LIB=y
CONFIG_RTC_MC146818_LIB=y
# CONFIG_RTC_CLASS is not set
# CONFIG_DMADEVICES is not set

#
# DMABUF options
#
# CONFIG_SYNC_FILE is not set
# CONFIG_DMABUF_HEAPS is not set
# end of DMABUF options

# CONFIG_AUXDISPLAY is not set
# CONFIG_UIO is not set
# CONFIG_VFIO is not set
# CONFIG_VIRT_DRIVERS is not set
# CONFIG_VIRTIO_MENU is not set
# CONFIG_VHOST_MENU is not set

#
# Microsoft Hyper-V guest support
#
# end of Microsoft Hyper-V guest support

# CONFIG_GREYBUS is not set
# CONFIG_COMEDI is not set
# CONFIG_STAGING is not set
# CONFIG_X86_PLATFORM_DEVICES is not set
# CONFIG_CHROME_PLATFORMS is not set
# CONFIG_MELLANOX_PLATFORM is not set
# CONFIG_SURFACE_PLATFORMS is not set
# CONFIG_COMMON_CLK is not set
# CONFIG_HWSPINLOCK is not set

#
# Clock Source drivers
#
CONFIG_CLKSRC_I8253=y
CONFIG_CLKEVT_I8253=y
CONFIG_I8253_LOCK=y
CONFIG_CLKBLD_I8253=y
# end of Clock Source drivers

# CONFIG_MAILBOX is not set
# CONFIG_IOMMU_SUPPORT is not set

#
# Remoteproc drivers
#
# CONFIG_REMOTEPROC is not set
# end of Remoteproc drivers

#
# Rpmsg drivers
#
# CONFIG_RPMSG_VIRTIO is not set
# end of Rpmsg drivers

#
# SOC (System On Chip) specific Drivers
#

#
# Amlogic SoC drivers
#
# end of Amlogic SoC drivers

#
# Broadcom SoC drivers
#
# end of Broadcom SoC drivers

#
# NXP/Freescale QorIQ SoC drivers
#
# end of NXP/Freescale QorIQ SoC drivers

#
# i.MX SoC drivers
#
# end of i.MX SoC drivers

#
# Enable LiteX SoC Builder specific drivers
#
# end of Enable LiteX SoC Builder specific drivers

#
# Qualcomm SoC drivers
#
# end of Qualcomm SoC drivers

# CONFIG_SOC_TI is not set

#
# Xilinx SoC drivers
#
# end of Xilinx SoC drivers
# end of SOC (System On Chip) specific Drivers

# CONFIG_PM_DEVFREQ is not set
# CONFIG_EXTCON is not set
# CONFIG_MEMORY is not set
# CONFIG_IIO is not set
# CONFIG_PWM is not set

#
# IRQ chip support
#
# end of IRQ chip support

# CONFIG_IPACK_BUS is not set
# CONFIG_RESET_CONTROLLER is not set

#
# PHY Subsystem
#
# CONFIG_GENERIC_PHY is not set
# CONFIG_PHY_CAN_TRANSCEIVER is not set

#
# PHY drivers for Broadcom platforms
#
# CONFIG_BCM_KONA_USB2_PHY is not set
# end of PHY drivers for Broadcom platforms

# CONFIG_PHY_PXA_28NM_HSIC is not set
# CONFIG_PHY_PXA_28NM_USB2 is not set
# CONFIG_PHY_INTEL_LGM_EMMC is not set
# end of PHY Subsystem

# CONFIG_POWERCAP is not set
# CONFIG_MCB is not set

#
# Performance monitor support
#
# end of Performance monitor support

# CONFIG_RAS is not set

#
# Android
#
# CONFIG_ANDROID is not set
# end of Android

# CONFIG_DAX is not set
# CONFIG_NVMEM is not set

#
# HW tracing support
#
# CONFIG_STM is not set
# CONFIG_INTEL_TH is not set
# end of HW tracing support

# CONFIG_FPGA is not set
# CONFIG_TEE is not set
# CONFIG_SIOX is not set
# CONFIG_SLIMBUS is not set
# CONFIG_INTERCONNECT is not set
# CONFIG_COUNTER is not set
# CONFIG_PECI is not set
# CONFIG_HTE is not set
# end of Device Drivers

#
# File systems
#
CONFIG_DCACHE_WORD_ACCESS=y
# CONFIG_VALIDATE_FS_PARSER is not set
# CONFIG_EXT2_FS is not set
# CONFIG_EXT3_FS is not set
# CONFIG_EXT4_FS is not set
# CONFIG_REISERFS_FS is not set
# CONFIG_JFS_FS is not set
# CONFIG_XFS_FS is not set
# CONFIG_GFS2_FS is not set
# CONFIG_BTRFS_FS is not set
# CONFIG_NILFS2_FS is not set
# CONFIG_F2FS_FS is not set
CONFIG_EXPORTFS=y
# CONFIG_EXPORTFS_BLOCK_OPS is not set
CONFIG_FILE_LOCKING=y
# CONFIG_FS_ENCRYPTION is not set
# CONFIG_FS_VERITY is not set
# CONFIG_DNOTIFY is not set
# CONFIG_INOTIFY_USER is not set
# CONFIG_FANOTIFY is not set
# CONFIG_QUOTA is not set
# CONFIG_AUTOFS4_FS is not set
# CONFIG_AUTOFS_FS is not set
# CONFIG_FUSE_FS is not set
# CONFIG_OVERLAY_FS is not set

#
# Caches
#
# CONFIG_FSCACHE is not set
# end of Caches

#
# CD-ROM/DVD Filesystems
#
# CONFIG_ISO9660_FS is not set
# CONFIG_UDF_FS is not set
# end of CD-ROM/DVD Filesystems

#
# DOS/FAT/EXFAT/NT Filesystems
#
# CONFIG_MSDOS_FS is not set
# CONFIG_VFAT_FS is not set
# CONFIG_EXFAT_FS is not set
# CONFIG_NTFS_FS is not set
# CONFIG_NTFS3_FS is not set
# end of DOS/FAT/EXFAT/NT Filesystems

#
# Pseudo filesystems
#
CONFIG_PROC_FS=y
# CONFIG_PROC_KCORE is not set
CONFIG_PROC_SYSCTL=y
CONFIG_PROC_PAGE_MONITOR=y
# CONFIG_PROC_CHILDREN is not set
CONFIG_PROC_PID_ARCH_STATUS=y
CONFIG_KERNFS=y
CONFIG_SYSFS=y
# CONFIG_TMPFS is not set
# CONFIG_HUGETLBFS is not set
# CONFIG_CONFIGFS_FS is not set
# end of Pseudo filesystems

# CONFIG_MISC_FILESYSTEMS is not set
# CONFIG_NLS is not set
# CONFIG_UNICODE is not set
CONFIG_IO_WQ=y
# end of File systems

#
# Security options
#
# CONFIG_KEYS is not set
# CONFIG_SECURITY_DMESG_RESTRICT is not set
# CONFIG_SECURITY is not set
# CONFIG_SECURITYFS is not set
CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR=y
# CONFIG_HARDENED_USERCOPY is not set
# CONFIG_FORTIFY_SOURCE is not set
# CONFIG_STATIC_USERMODEHELPER is not set
CONFIG_DEFAULT_SECURITY_DAC=y
CONFIG_LSM="landlock,lockdown,yama,loadpin,safesetid,integrity,bpf"

#
# Kernel hardening options
#

#
# Memory initialization
#
CONFIG_INIT_STACK_NONE=y
# CONFIG_INIT_ON_ALLOC_DEFAULT_ON is not set
# CONFIG_INIT_ON_FREE_DEFAULT_ON is not set
CONFIG_CC_HAS_ZERO_CALL_USED_REGS=y
# CONFIG_ZERO_CALL_USED_REGS is not set
# end of Memory initialization

CONFIG_RANDSTRUCT_NONE=y
# end of Kernel hardening options
# end of Security options

# CONFIG_CRYPTO is not set

#
# Library routines
#
# CONFIG_PACKING is not set
CONFIG_BITREVERSE=y
CONFIG_GENERIC_STRNCPY_FROM_USER=y
CONFIG_GENERIC_STRNLEN_USER=y
# CONFIG_CORDIC is not set
# CONFIG_PRIME_NUMBERS is not set
CONFIG_GENERIC_PCI_IOMAP=y
CONFIG_GENERIC_IOMAP=y
CONFIG_ARCH_HAS_FAST_MULTIPLIER=y
CONFIG_ARCH_USE_SYM_ANNOTATIONS=y

#
# Crypto library routines
#
CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC=y
# CONFIG_CRYPTO_LIB_CURVE25519 is not set
CONFIG_CRYPTO_LIB_POLY1305_RSIZE=1
# CONFIG_CRYPTO_LIB_POLY1305 is not set
# end of Crypto library routines

# CONFIG_CRC_CCITT is not set
# CONFIG_CRC16 is not set
# CONFIG_CRC_T10DIF is not set
# CONFIG_CRC64_ROCKSOFT is not set
# CONFIG_CRC_ITU_T is not set
CONFIG_CRC32=y
# CONFIG_CRC32_SELFTEST is not set
CONFIG_CRC32_SLICEBY8=y
# CONFIG_CRC32_SLICEBY4 is not set
# CONFIG_CRC32_SARWATE is not set
# CONFIG_CRC32_BIT is not set
# CONFIG_CRC64 is not set
# CONFIG_CRC4 is not set
# CONFIG_CRC7 is not set
# CONFIG_LIBCRC32C is not set
# CONFIG_CRC8 is not set
# CONFIG_RANDOM32_SELFTEST is not set
# CONFIG_XZ_DEC is not set
CONFIG_HAS_IOMEM=y
CONFIG_HAS_IOPORT_MAP=y
CONFIG_HAS_DMA=y
CONFIG_NEED_SG_DMA_LENGTH=y
# CONFIG_DMA_API_DEBUG is not set
# CONFIG_IRQ_POLL is not set
CONFIG_HAVE_GENERIC_VDSO=y
CONFIG_GENERIC_GETTIMEOFDAY=y
CONFIG_GENERIC_VDSO_32=y
CONFIG_GENERIC_VDSO_TIME_NS=y
CONFIG_ARCH_STACKWALK=y
CONFIG_STACKDEPOT=y
CONFIG_STACK_HASH_ORDER=20
CONFIG_SBITMAP=y
# end of Library routines

#
# Kernel hacking
#

#
# printk and dmesg options
#
# CONFIG_PRINTK_TIME is not set
# CONFIG_PRINTK_CALLER is not set
# CONFIG_STACKTRACE_BUILD_ID is not set
CONFIG_CONSOLE_LOGLEVEL_DEFAULT=7
CONFIG_CONSOLE_LOGLEVEL_QUIET=4
CONFIG_MESSAGE_LOGLEVEL_DEFAULT=4
# CONFIG_DYNAMIC_DEBUG is not set
# CONFIG_DYNAMIC_DEBUG_CORE is not set
# CONFIG_SYMBOLIC_ERRNAME is not set
CONFIG_DEBUG_BUGVERBOSE=y
# end of printk and dmesg options

# CONFIG_DEBUG_KERNEL is not set

#
# Compile-time checks and compiler options
#
CONFIG_FRAME_WARN=1024
# CONFIG_STRIP_ASM_SYMS is not set
# CONFIG_HEADERS_INSTALL is not set
CONFIG_DEBUG_SECTION_MISMATCH=y
CONFIG_SECTION_MISMATCH_WARN_ONLY=y
CONFIG_FRAME_POINTER=y
# end of Compile-time checks and compiler options

#
# Generic Kernel Debugging Instruments
#
# CONFIG_MAGIC_SYSRQ is not set
# CONFIG_DEBUG_FS is not set
CONFIG_HAVE_ARCH_KGDB=y
CONFIG_ARCH_HAS_UBSAN_SANITIZE_ALL=y
# CONFIG_UBSAN is not set
CONFIG_HAVE_KCSAN_COMPILER=y
# end of Generic Kernel Debugging Instruments

#
# Networking Debugging
#
# end of Networking Debugging

#
# Memory Debugging
#
# CONFIG_PAGE_EXTENSION is not set
CONFIG_SLUB_DEBUG=y
# CONFIG_SLUB_DEBUG_ON is not set
# CONFIG_PAGE_POISONING is not set
# CONFIG_DEBUG_RODATA_TEST is not set
CONFIG_ARCH_HAS_DEBUG_WX=y
# CONFIG_DEBUG_WX is not set
CONFIG_GENERIC_PTDUMP=y
CONFIG_HAVE_DEBUG_KMEMLEAK=y
CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE=y
# CONFIG_DEBUG_VM_PGTABLE is not set
CONFIG_ARCH_HAS_DEBUG_VIRTUAL=y
CONFIG_DEBUG_MEMORY_INIT=y
CONFIG_ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP=y
CONFIG_HAVE_DEBUG_STACKOVERFLOW=y
CONFIG_CC_HAS_KASAN_GENERIC=y
CONFIG_CC_HAS_WORKING_NOSANITIZE_ADDRESS=y
CONFIG_HAVE_ARCH_KFENCE=y
# CONFIG_KFENCE is not set
# end of Memory Debugging

#
# Debug Oops, Lockups and Hangs
#
# CONFIG_PANIC_ON_OOPS is not set
CONFIG_PANIC_ON_OOPS_VALUE=0
CONFIG_PANIC_TIMEOUT=0
# end of Debug Oops, Lockups and Hangs

#
# Scheduler Debugging
#
# end of Scheduler Debugging

# CONFIG_DEBUG_TIMEKEEPING is not set

#
# Lock Debugging (spinlocks, mutexes, etc...)
#
CONFIG_LOCK_DEBUGGING_SUPPORT=y
# CONFIG_WW_MUTEX_SELFTEST is not set
# end of Lock Debugging (spinlocks, mutexes, etc...)

# CONFIG_DEBUG_IRQFLAGS is not set
CONFIG_STACKTRACE=y
# CONFIG_WARN_ALL_UNSEEDED_RANDOM is not set

#
# Debug kernel data structures
#
# CONFIG_BUG_ON_DATA_CORRUPTION is not set
# end of Debug kernel data structures

#
# RCU Debugging
#
# end of RCU Debugging

CONFIG_USER_STACKTRACE_SUPPORT=y
CONFIG_HAVE_RETHOOK=y
CONFIG_HAVE_FUNCTION_TRACER=y
CONFIG_HAVE_FUNCTION_GRAPH_TRACER=y
CONFIG_HAVE_DYNAMIC_FTRACE=y
CONFIG_HAVE_DYNAMIC_FTRACE_WITH_REGS=y
CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS=y
CONFIG_HAVE_FTRACE_MCOUNT_RECORD=y
CONFIG_HAVE_SYSCALL_TRACEPOINTS=y
CONFIG_HAVE_C_RECORDMCOUNT=y
CONFIG_HAVE_BUILDTIME_MCOUNT_SORT=y
CONFIG_TRACING_SUPPORT=y
# CONFIG_FTRACE is not set
# CONFIG_SAMPLES is not set
CONFIG_ARCH_HAS_DEVMEM_IS_ALLOWED=y

#
# x86 Debugging
#
CONFIG_TRACE_IRQFLAGS_NMI_SUPPORT=y
# CONFIG_X86_VERBOSE_BOOTUP is not set
CONFIG_EARLY_PRINTK=y
CONFIG_HAVE_MMIOTRACE_SUPPORT=y
CONFIG_IO_DELAY_0X80=y
# CONFIG_IO_DELAY_0XED is not set
# CONFIG_IO_DELAY_UDELAY is not set
# CONFIG_IO_DELAY_NONE is not set
CONFIG_UNWINDER_FRAME_POINTER=y
# end of x86 Debugging

#
# Kernel Testing and Coverage
#
# CONFIG_KUNIT is not set
CONFIG_CC_HAS_SANCOV_TRACE_PC=y
# CONFIG_RUNTIME_TESTING_MENU is not set
CONFIG_ARCH_USE_MEMTEST=y
# CONFIG_MEMTEST is not set
# end of Kernel Testing and Coverage
# end of Kernel hacking

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Erling Ljunggren (hljunggr)]

On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote:
> On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com>
> wrote:
> > 
> > From: Jonathan Selnes <jonathansb1@gmail.com>
> > 
> > Support reading and writing the EDID EEPROM through the
> > v4l2 API.
> > 
> > Signed-off-by: Jonathan Selnes <jonathansb1@gmail.com>
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
> 
> Wondering if the last two people don't do any development, otherwise
> Co-developed-by would be appreciated.
> 
OK
> ...
> 
> >  obj-$(CONFIG_VIDEO_HI556) += hi556.o
> >  obj-$(CONFIG_VIDEO_HI846) += hi846.o
> >  obj-$(CONFIG_VIDEO_HI847) += hi847.o
> > +obj-$(CONFIG_VIDEO_CAT24C208) += cat24c208.o
> 
> Perhaps more sorted?
OK
> 
> >  obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
> >  obj-$(CONFIG_VIDEO_IMX208) += imx208.o
> >  obj-$(CONFIG_VIDEO_IMX214) += imx214.o
> 
> ...
> 
> > +/*
> > + * cat24c208 - HDMI i2c controlled EEPROM from ON Semiconductor or
> > Catalyst Semiconductor
> 
> Here...
> 
> > + * cat24c208.c - Support for i2c based DDC EEPROM
> 
> ...and here and in general putting filename into the file is not a
> good idea in the long term. For example, this can be expanded in the
> future to support more EDID EEPROMs, and if we want to rename, this
> adds an additional burden.
OK
> 
> > + * Copyright (C) 2021-2022 Cisco Systems, Inc. and/or its
> > affiliates. All rights reserved.
> > + */
> 
> ...
> 
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> 
> > +#include <linux/of_device.h>
> 
> Why? Who is the user of this?
> Perhaps you meant mod_devicetable.h, which is currently missed?
yes
> 
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/videodev2.h>
> 
> > +#include <linux/kernel.h>
> 
> Perhaps keep it ordered?
OK
> 
> ...
> 
> > +/*
> > + * From the datasheet:
> 
> Maybe  add an URL to the Datasheet?

There already is a link to the datasheet in the source file further up,
no need to add another.


> 
> > + * The write cycle time is the time from a valid stop condition of
> > a write
> > + * sequence to the end of the internal program/erase cycle. During
> > the write
> > + * cycle, the bus interface circuits are disabled, SDA is allowed
> > to remain
> > + * high, and the device does not respond to its slave address.
> > + */
> > +#define WRITE_CYCLE_TIME_US    5000
> 
> ...
> 
> > +       struct i2c_client *dev_client = state->i2c_clients[0];
> > +       struct i2c_client *data_client = state->i2c_clients[1];
> > +       struct i2c_client *seg_client = state->i2c_clients[2];
> 
> Why not have those clients named accordingly in the data struct,
> instead of indexing them?
OK
> 
> ...
> 
> > +       if (seg)
> > +               err = i2c_transfer(dev_client->adapter, msg,
> > ARRAY_SIZE(msg));
> > +       else
> > +               err = i2c_transfer(dev_client->adapter, &msg[1],
> > 1);
> > +       if (err < 0)
> > +               dev_err(&dev_client->dev, "Writing to 0x%x failed
> > (segment %d)\n",
> > +                       data_client->addr, seg);
> 
> > +       usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
> 
> Sleep even in case of error? Is it required?
> (Same Q per other similar places)

The i2c transfer may still have written some data, and we need to wait
for the EEPROM to update.

> 
> > +       return err < 0 ? err : 0;
> 
> Hence here...
> 
> ...
> 
> > +       if (err < 0)
> > +               dev_err(&dev_client->dev, "Reading of EDID
> > failed\n");
> > +       return err < 0 ? err : 0;
> 
> ...and here we can avoid a duplication test for error code being set,
> right?
> (Same suggestion per other similar cases)
OK
> 
> ...
> 
> > +       static const u8 header_pattern[] = {
> > +               0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
> 
> Keeping a comma at the end is good anyway.

This header pattern is fixed to 8 bytes, and will never be more than 8
bytes. So I don't think think the added comma is necessary.

> 
> > +       };
> 
> ...
> 
> > +       state = kzalloc(sizeof(*state), GFP_KERNEL);
> > +       if (!state)
> > +               return -ENOMEM;
> 
> devm_kzalloc() ?

This will fail if the device is forcibly unloaded while some
application has the device node open.

> 
> ...
> 
> > +               blocks = 1 + state->edid[126];
> 
> Magic index.
Ack
> 
> ...
> 
> > +               .of_match_table = of_match_ptr(cat24c208_of_match),
> 
> of_match_ptr() brings more harm than help.
OK
> 

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Mon, Aug 1, 2022 at 3:07 PM Erling Ljunggren (hljunggr)
<hljunggr@cisco.com> wrote:
> On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote:
> > On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com>
> > wrote:

...

> > > +       usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
> >
> > Sleep even in case of error? Is it required?
> > (Same Q per other similar places)
>
> The i2c transfer may still have written some data, and we need to wait
> for the EEPROM to update.

But in an error case you will leave EEPROM in an erroneous state?

...

> > > +       static const u8 header_pattern[] = {
> > > +               0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
> >
> > Keeping a comma at the end is good anyway.
>
> This header pattern is fixed to 8 bytes, and will never be more than 8
> bytes. So I don't think think the added comma is necessary.

It's minor, so up to you, folks.

> > > +       };

...

> > > +       state = kzalloc(sizeof(*state), GFP_KERNEL);
> > > +       if (!state)
> > > +               return -ENOMEM;
> >
> > devm_kzalloc() ?
>
> This will fail if the device is forcibly unloaded while some
> application has the device node open.

I'm not sure how it's related. Can you elaborate a bit, please?

If you try to forcibly unload the device (driver) when it's open and
it somehow succeeds, that will be a sign of lifetime issues in the
code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

On 01/08/2022 16:57, Andy Shevchenko wrote:
> On Mon, Aug 1, 2022 at 3:07 PM Erling Ljunggren (hljunggr)
> <hljunggr@cisco.com> wrote:
>> On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote:
>>> On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com>
>>> wrote:
> 
> ...
> 
>>>> +       usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
>>>
>>> Sleep even in case of error? Is it required?
>>> (Same Q per other similar places)
>>
>> The i2c transfer may still have written some data, and we need to wait
>> for the EEPROM to update.
> 
> But in an error case you will leave EEPROM in an erroneous state?

Yes, if this happens you likely have a hardware error anyway. It's not
worth it trying to be smart in that case. For that matter, I don't know
what a smart solution would be.

> 
> ...
> 
>>>> +       static const u8 header_pattern[] = {
>>>> +               0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
>>>
>>> Keeping a comma at the end is good anyway.
>>
>> This header pattern is fixed to 8 bytes, and will never be more than 8
>> bytes. So I don't think think the added comma is necessary.
> 
> It's minor, so up to you, folks.
> 
>>>> +       };
> 
> ...
> 
>>>> +       state = kzalloc(sizeof(*state), GFP_KERNEL);
>>>> +       if (!state)
>>>> +               return -ENOMEM;
>>>
>>> devm_kzalloc() ?
>>
>> This will fail if the device is forcibly unloaded while some
>> application has the device node open.
> 
> I'm not sure how it's related. Can you elaborate a bit, please?
> 
> If you try to forcibly unload the device (driver) when it's open and
> it somehow succeeds, that will be a sign of lifetime issues in the
> code.
> 

Not with rmmod but using the unbind facility. For new media drivers we generally
want to avoid using devm_*alloc, it causes more problems than it solves.

It's unlikely to be an issue here, but I'd rather keep it as-is.

Regards,

	Hans

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Mon, Aug 1, 2022 at 8:35 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 01/08/2022 16:57, Andy Shevchenko wrote:
> > On Mon, Aug 1, 2022 at 3:07 PM Erling Ljunggren (hljunggr)
> > <hljunggr@cisco.com> wrote:
> >> On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote:
> >>> On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com>
> >>> wrote:

...

> >>>> +       state = kzalloc(sizeof(*state), GFP_KERNEL);
> >>>> +       if (!state)
> >>>> +               return -ENOMEM;
> >>>
> >>> devm_kzalloc() ?
> >>
> >> This will fail if the device is forcibly unloaded while some
> >> application has the device node open.
> >
> > I'm not sure how it's related. Can you elaborate a bit, please?
> >
> > If you try to forcibly unload the device (driver) when it's open and
> > it somehow succeeds, that will be a sign of lifetime issues in the
> > code.
>
> Not with rmmod but using the unbind facility.

And what is the difference? The device driver core calls the same, no?

> For new media drivers we generally
> want to avoid using devm_*alloc, it causes more problems than it solves.

I think it's because people don't think much about the lifetime of
objects. I don't think devm is an issue here.

> It's unlikely to be an issue here, but I'd rather keep it as-is.

OK.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

On 8/2/22 10:42, Andy Shevchenko wrote:
> On Mon, Aug 1, 2022 at 8:35 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>> On 01/08/2022 16:57, Andy Shevchenko wrote:
>>> On Mon, Aug 1, 2022 at 3:07 PM Erling Ljunggren (hljunggr)
>>> <hljunggr@cisco.com> wrote:
>>>> On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote:
>>>>> On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com>
>>>>> wrote:
> 
> ...
> 
>>>>>> +       state = kzalloc(sizeof(*state), GFP_KERNEL);
>>>>>> +       if (!state)
>>>>>> +               return -ENOMEM;
>>>>>
>>>>> devm_kzalloc() ?
>>>>
>>>> This will fail if the device is forcibly unloaded while some
>>>> application has the device node open.
>>>
>>> I'm not sure how it's related. Can you elaborate a bit, please?
>>>
>>> If you try to forcibly unload the device (driver) when it's open and
>>> it somehow succeeds, that will be a sign of lifetime issues in the
>>> code.
>>
>> Not with rmmod but using the unbind facility.
> 
> And what is the difference? The device driver core calls the same, no?

rmmod when the /dev/videoX is open won't work (device is busy), whereas
unbind *will* work, and it is really the same as a USB unplug.

> 
>> For new media drivers we generally
>> want to avoid using devm_*alloc, it causes more problems than it solves.
> 
> I think it's because people don't think much about the lifetime of
> objects. I don't think devm is an issue here.

Yes, it is: unbind will call the driver remove() function, and after that
function all memory allocated with devm_*alloc will be freed immediately.

But if an application still has a filehandle open and was possibly even in
the middle of an ioctl call, then having the memory removed instantaneously
is a really bad thing.

Hotpluggable devices in general definitely should not use it. I'm not a fan
of devm_*alloc anymore.

Regards,

	Hans

> 
>> It's unlikely to be an issue here, but I'd rather keep it as-is.
> 
> OK.
> 

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Tue, Aug 2, 2022 at 11:06 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 8/2/22 10:42, Andy Shevchenko wrote:
> > On Mon, Aug 1, 2022 at 8:35 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >> On 01/08/2022 16:57, Andy Shevchenko wrote:
> >>> On Mon, Aug 1, 2022 at 3:07 PM Erling Ljunggren (hljunggr)
> >>> <hljunggr@cisco.com> wrote:
> >>>> On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote:
> >>>>> On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com>
> >>>>> wrote:

...

> >>>>>> +       state = kzalloc(sizeof(*state), GFP_KERNEL);
> >>>>>> +       if (!state)
> >>>>>> +               return -ENOMEM;
> >>>>>
> >>>>> devm_kzalloc() ?
> >>>>
> >>>> This will fail if the device is forcibly unloaded while some
> >>>> application has the device node open.
> >>>
> >>> I'm not sure how it's related. Can you elaborate a bit, please?
> >>>
> >>> If you try to forcibly unload the device (driver) when it's open and
> >>> it somehow succeeds, that will be a sign of lifetime issues in the
> >>> code.
> >>
> >> Not with rmmod but using the unbind facility.
> >
> > And what is the difference? The device driver core calls the same, no?
>
> rmmod when the /dev/videoX is open won't work (device is busy), whereas
> unbind *will* work, and it is really the same as a USB unplug.

Seems there are no guards against that.

> >> For new media drivers we generally
> >> want to avoid using devm_*alloc, it causes more problems than it solves.
> >
> > I think it's because people don't think much about the lifetime of
> > objects. I don't think devm is an issue here.
>
> Yes, it is: unbind will call the driver remove() function, and after that
> function all memory allocated with devm_*alloc will be freed immediately.

Yes.

> But if an application still has a filehandle open and was possibly even in
> the middle of an ioctl call, then having the memory removed instantaneously
> is a really bad thing.

True.

> Hotpluggable devices in general definitely should not use it. I'm not a fan
> of devm_*alloc anymore.

You are blaming the wrong man here, i.e. devm. The problem as I stated
above is developers who do not understand (pay attention to) the
lifetime of the objects.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Aug 2, 2022 at 11:06 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> > On 8/2/22 10:42, Andy Shevchenko wrote:
> > > On Mon, Aug 1, 2022 at 8:35 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> > >> On 01/08/2022 16:57, Andy Shevchenko wrote:
> > >>> On Mon, Aug 1, 2022 at 3:07 PM Erling Ljunggren (hljunggr)
> > >>> <hljunggr@cisco.com> wrote:
> > >>>> On Fri, 2022-07-29 at 17:51 +0200, Andy Shevchenko wrote:
> > >>>>> On Thu, Jul 28, 2022 at 1:53 PM Erling Ljunggren <hljunggr@cisco.com>
> > >>>>> wrote:
>
> ...
>
> > >>>>>> +       state = kzalloc(sizeof(*state), GFP_KERNEL);
> > >>>>>> +       if (!state)
> > >>>>>> +               return -ENOMEM;
> > >>>>>
> > >>>>> devm_kzalloc() ?
> > >>>>
> > >>>> This will fail if the device is forcibly unloaded while some
> > >>>> application has the device node open.
> > >>>
> > >>> I'm not sure how it's related. Can you elaborate a bit, please?
> > >>>
> > >>> If you try to forcibly unload the device (driver) when it's open and
> > >>> it somehow succeeds, that will be a sign of lifetime issues in the
> > >>> code.
> > >>
> > >> Not with rmmod but using the unbind facility.
> > >
> > > And what is the difference? The device driver core calls the same, no?
> >
> > rmmod when the /dev/videoX is open won't work (device is busy), whereas
> > unbind *will* work, and it is really the same as a USB unplug.
>
> Seems there are no guards against that.
>
> > >> For new media drivers we generally
> > >> want to avoid using devm_*alloc, it causes more problems than it solves.
> > >
> > > I think it's because people don't think much about the lifetime of
> > > objects. I don't think devm is an issue here.
> >
> > Yes, it is: unbind will call the driver remove() function, and after that
> > function all memory allocated with devm_*alloc will be freed immediately.
>
> Yes.
>
> > But if an application still has a filehandle open and was possibly even in
> > the middle of an ioctl call, then having the memory removed instantaneously
> > is a really bad thing.
>
> True.
>
> > Hotpluggable devices in general definitely should not use it. I'm not a fan
> > of devm_*alloc anymore.
>
> You are blaming the wrong man here, i.e. devm. The problem as I stated
> above is developers who do not understand (pay attention to) the
> lifetime of the objects.

That said, the devm has nothing to do with the driver still being
problematic for the scenario you described, no?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Tue, Aug 2, 2022 at 2:23 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:

...

> > You are blaming the wrong man here, i.e. devm. The problem as I stated
> > above is developers who do not understand (pay attention to) the
> > lifetime of the objects.
>
> That said, the devm has nothing to do with the driver still being
> problematic for the scenario you described, no?

And the cleanest (at the first glance) solution is to make v4l2 to fix
this bug by suppressing unbind attributes when the device is opened
for all v4l2 subdev drivers, and restore it back when it's closed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

On 8/2/22 14:26, Andy Shevchenko wrote:
> On Tue, Aug 2, 2022 at 2:23 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
> 
> ...
> 
>>> You are blaming the wrong man here, i.e. devm. The problem as I stated
>>> above is developers who do not understand (pay attention to) the
>>> lifetime of the objects.
>>
>> That said, the devm has nothing to do with the driver still being
>> problematic for the scenario you described, no?
> 
> And the cleanest (at the first glance) solution is to make v4l2 to fix
> this bug by suppressing unbind attributes when the device is opened
> for all v4l2 subdev drivers, and restore it back when it's closed.
> 

Why would we do that? The patch works in the scenario that I described:
the memory is freed in the struct video_device release() callback, which
is called when the last reference to the video node goes away. This is
standard V4L2 framework behavior that works great in the case of a unbind.

Without devm_kzalloc it works fine, even when unbind is called. With
devm_kzalloc the unbind attributes would have to be suppressed. I see no
reason for that as media maintainer.

Regards,

	Hans

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Tue, Aug 2, 2022 at 2:45 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 8/2/22 14:26, Andy Shevchenko wrote:
> > On Tue, Aug 2, 2022 at 2:23 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> >> On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko
> >> <andy.shevchenko@gmail.com> wrote:

...

> >>> You are blaming the wrong man here, i.e. devm. The problem as I stated
> >>> above is developers who do not understand (pay attention to) the
> >>> lifetime of the objects.
> >>
> >> That said, the devm has nothing to do with the driver still being
> >> problematic for the scenario you described, no?
> >
> > And the cleanest (at the first glance) solution is to make v4l2 to fix
> > this bug by suppressing unbind attributes when the device is opened
> > for all v4l2 subdev drivers, and restore it back when it's closed.
>
> Why would we do that? The patch works in the scenario that I described:
> the memory is freed in the struct video_device release() callback, which
> is called when the last reference to the video node goes away. This is
> standard V4L2 framework behavior that works great in the case of a unbind.
>
> Without devm_kzalloc it works fine, even when unbind is called. With
> devm_kzalloc the unbind attributes would have to be suppressed. I see no
> reason for that as media maintainer.

I'm not sure anymore that we are talking about the same thing.

Your driver allocates memory with kzalloc() in ->probe() and frees it
in ->remove(). How is this different from the lifetime of a devm:ed
object? If what you said is true, than driver is still problematic,
since ->remove() frees this memory the very same way at unbind call.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Hans Verkuil]

On 8/2/22 14:49, Andy Shevchenko wrote:
> On Tue, Aug 2, 2022 at 2:45 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>> On 8/2/22 14:26, Andy Shevchenko wrote:
>>> On Tue, Aug 2, 2022 at 2:23 PM Andy Shevchenko
>>> <andy.shevchenko@gmail.com> wrote:
>>>> On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko
>>>> <andy.shevchenko@gmail.com> wrote:
> 
> ...
> 
>>>>> You are blaming the wrong man here, i.e. devm. The problem as I stated
>>>>> above is developers who do not understand (pay attention to) the
>>>>> lifetime of the objects.
>>>>
>>>> That said, the devm has nothing to do with the driver still being
>>>> problematic for the scenario you described, no?
>>>
>>> And the cleanest (at the first glance) solution is to make v4l2 to fix
>>> this bug by suppressing unbind attributes when the device is opened
>>> for all v4l2 subdev drivers, and restore it back when it's closed.
>>
>> Why would we do that? The patch works in the scenario that I described:
>> the memory is freed in the struct video_device release() callback, which
>> is called when the last reference to the video node goes away. This is
>> standard V4L2 framework behavior that works great in the case of a unbind.
>>
>> Without devm_kzalloc it works fine, even when unbind is called. With
>> devm_kzalloc the unbind attributes would have to be suppressed. I see no
>> reason for that as media maintainer.
> 
> I'm not sure anymore that we are talking about the same thing.
> 
> Your driver allocates memory with kzalloc() in ->probe() and frees it
> in ->remove(). How is this different from the lifetime of a devm:ed
> object? If what you said is true, than driver is still problematic,
> since ->remove() frees this memory the very same way at unbind call.

No, it is not freed in remove(). remove() calls video_unregister_device(),
and that calls cat24c208_release() when the last user of /dev/videoX closes
its filehandle. And it is cat24c208_release() that finally frees the memory.

Regards,

	Hans

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Tue, Aug 2, 2022 at 2:58 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 8/2/22 14:49, Andy Shevchenko wrote:
> > On Tue, Aug 2, 2022 at 2:45 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >> On 8/2/22 14:26, Andy Shevchenko wrote:
> >>> On Tue, Aug 2, 2022 at 2:23 PM Andy Shevchenko
> >>> <andy.shevchenko@gmail.com> wrote:
> >>>> On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko
> >>>> <andy.shevchenko@gmail.com> wrote:

...

> >>>>> You are blaming the wrong man here, i.e. devm. The problem as I stated
> >>>>> above is developers who do not understand (pay attention to) the
> >>>>> lifetime of the objects.
> >>>>
> >>>> That said, the devm has nothing to do with the driver still being
> >>>> problematic for the scenario you described, no?
> >>>
> >>> And the cleanest (at the first glance) solution is to make v4l2 to fix
> >>> this bug by suppressing unbind attributes when the device is opened
> >>> for all v4l2 subdev drivers, and restore it back when it's closed.
> >>
> >> Why would we do that? The patch works in the scenario that I described:
> >> the memory is freed in the struct video_device release() callback, which
> >> is called when the last reference to the video node goes away. This is
> >> standard V4L2 framework behavior that works great in the case of a unbind.
> >>
> >> Without devm_kzalloc it works fine, even when unbind is called. With
> >> devm_kzalloc the unbind attributes would have to be suppressed. I see no
> >> reason for that as media maintainer.
> >
> > I'm not sure anymore that we are talking about the same thing.
> >
> > Your driver allocates memory with kzalloc() in ->probe() and frees it
> > in ->remove(). How is this different from the lifetime of a devm:ed
> > object? If what you said is true, than driver is still problematic,
> > since ->remove() frees this memory the very same way at unbind call.
>
> No, it is not freed in remove(). remove() calls video_unregister_device(),
> and that calls cat24c208_release() when the last user of /dev/videoX closes
> its filehandle. And it is cat24c208_release() that finally frees the memory.

Okay, I got it.

So, if you unbind the driver the state will be stale and still being
accessible by the user (with outdated info). Now, what happens if you
unbind and try to bind back, while the user is slow enough to close
the device file?

Also it's still racy against any i2c calls done via IOCTLs, because
you have unregistered I2C devices.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[Andy Shevchenko]

On Tue, Aug 2, 2022 at 6:26 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Aug 2, 2022 at 2:58 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> > On 8/2/22 14:49, Andy Shevchenko wrote:
> > > On Tue, Aug 2, 2022 at 2:45 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> > >> On 8/2/22 14:26, Andy Shevchenko wrote:
> > >>> On Tue, Aug 2, 2022 at 2:23 PM Andy Shevchenko
> > >>> <andy.shevchenko@gmail.com> wrote:
> > >>>> On Tue, Aug 2, 2022 at 2:21 PM Andy Shevchenko
> > >>>> <andy.shevchenko@gmail.com> wrote:
>
> ...
>
> > >>>>> You are blaming the wrong man here, i.e. devm. The problem as I stated
> > >>>>> above is developers who do not understand (pay attention to) the
> > >>>>> lifetime of the objects.
> > >>>>
> > >>>> That said, the devm has nothing to do with the driver still being
> > >>>> problematic for the scenario you described, no?
> > >>>
> > >>> And the cleanest (at the first glance) solution is to make v4l2 to fix
> > >>> this bug by suppressing unbind attributes when the device is opened
> > >>> for all v4l2 subdev drivers, and restore it back when it's closed.
> > >>
> > >> Why would we do that? The patch works in the scenario that I described:
> > >> the memory is freed in the struct video_device release() callback, which
> > >> is called when the last reference to the video node goes away. This is
> > >> standard V4L2 framework behavior that works great in the case of a unbind.
> > >>
> > >> Without devm_kzalloc it works fine, even when unbind is called. With
> > >> devm_kzalloc the unbind attributes would have to be suppressed. I see no
> > >> reason for that as media maintainer.
> > >
> > > I'm not sure anymore that we are talking about the same thing.
> > >
> > > Your driver allocates memory with kzalloc() in ->probe() and frees it
> > > in ->remove(). How is this different from the lifetime of a devm:ed
> > > object? If what you said is true, than driver is still problematic,
> > > since ->remove() frees this memory the very same way at unbind call.
> >
> > No, it is not freed in remove(). remove() calls video_unregister_device(),
> > and that calls cat24c208_release() when the last user of /dev/videoX closes
> > its filehandle. And it is cat24c208_release() that finally frees the memory.
>
> Okay, I got it.
>
> So, if you unbind the driver the state will be stale and still being
> accessible by the user (with outdated info). Now, what happens if you
> unbind and try to bind back, while the user is slow enough to close
> the device file?
>
> Also it's still racy against any i2c calls done via IOCTLs, because
> you have unregistered I2C devices.

As far as I understand current design, you may not call anything but
video_device_unregister() in the subdevice's ->remove(), or be very
careful on what resources can be used via IOCTLs.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

[kernel test robot]

Hi Erling,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linus/master v5.19]
[cannot apply to next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Erling-Ljunggren/Add-the-cat24c208-EDID-EEPROM-driver-new-EDID-capability/20220728-194524
base:   git://linuxtv.org/media_tree.git master
config: s390-randconfig-c005-20220802 (https://download.01.org/0day-ci/archive/20220803/202208030925.bp514QvB-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/94ba9bb463937ad05b92c6be56a552894b386774
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Erling-Ljunggren/Add-the-cat24c208-EDID-EEPROM-driver-new-EDID-capability/20220728-194524
        git checkout 94ba9bb463937ad05b92c6be56a552894b386774
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/gpu/drm/ drivers/media/i2c/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/media/i2c/cat24c208.c:19:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from drivers/media/i2c/cat24c208.c:19:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from drivers/media/i2c/cat24c208.c:19:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/media/i2c/cat24c208.c:407:34: warning: unused variable 'cat24c208_of_match' [-Wunused-const-variable]
   static const struct of_device_id cat24c208_of_match[] = {
                                    ^
   13 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for DRM_DP_AUX_BUS
   Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && OF [=n]
   Selected by [y]:
   - DRM_MSM [=y] && HAS_IOMEM [=y] && DRM [=y] && (ARCH_QCOM || SOC_IMX5 || COMPILE_TEST [=y]) && COMMON_CLK [=y] && IOMMU_SUPPORT [=y] && (QCOM_OCMEM [=n] || QCOM_OCMEM [=n]=n) && (QCOM_LLCC [=y] || QCOM_LLCC [=y]=n) && (QCOM_COMMAND_DB [=n] || QCOM_COMMAND_DB [=n]=n)


vim +/cat24c208_of_match +407 drivers/media/i2c/cat24c208.c

   406	
 > 407	static const struct of_device_id cat24c208_of_match[] = {
   408		{ .compatible = "onnn,cat24c208"},
   409		{}
   410	};
   411	MODULE_DEVICE_TABLE(of, cat24c208_of_match);
   412	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp