Devicetree
 help / color / mirror / Atom feed
* [PATCH v4 3/4] Input: mtk-pmic-keys - add support for MT6358
From: Mattijs Korpershoek @ 2022-01-21 14:03 UTC (permalink / raw)
  To: Dmitry Torokhov, Matthias Brugger, Rob Herring
  Cc: Fabien Parent, Kevin Hilman, linux-input, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel,
	Mattijs Korpershoek
In-Reply-To: <20220121140323.4080640-1-mkorpershoek@baylibre.com>

MT6358 pmic keys behave differently than mt6397 and mt6323: there are
two interrupts per key: one for press, the other one for release (_r)

Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 drivers/input/keyboard/mtk-pmic-keys.c | 48 ++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index d1abf95d5701..c31ab4368388 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -9,6 +9,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/mfd/mt6323/registers.h>
+#include <linux/mfd/mt6358/registers.h>
 #include <linux/mfd/mt6397/core.h>
 #include <linux/mfd/mt6397/registers.h>
 #include <linux/module.h>
@@ -74,11 +75,22 @@ static const struct mtk_pmic_regs mt6323_regs = {
 	.pmic_rst_reg = MT6323_TOP_RST_MISC,
 };
 
+static const struct mtk_pmic_regs mt6358_regs = {
+	.keys_regs[MTK_PMIC_PWRKEY_INDEX] =
+		MTK_PMIC_KEYS_REGS(MT6358_TOPSTATUS,
+				   0x2, MT6358_PSC_TOP_INT_CON0, 0x5),
+	.keys_regs[MTK_PMIC_HOMEKEY_INDEX] =
+		MTK_PMIC_KEYS_REGS(MT6358_TOPSTATUS,
+				   0x8, MT6358_PSC_TOP_INT_CON0, 0xa),
+	.pmic_rst_reg = MT6358_TOP_RST_MISC,
+};
+
 struct mtk_pmic_keys_info {
 	struct mtk_pmic_keys *keys;
 	const struct mtk_pmic_keys_regs *regs;
 	unsigned int keycode;
 	int irq;
+	int irq_r; /* optional: release irq if different */
 	bool wakeup:1;
 };
 
@@ -188,6 +200,18 @@ static int mtk_pmic_key_setup(struct mtk_pmic_keys *keys,
 		return ret;
 	}
 
+	if (info->irq_r > 0) {
+		ret = devm_request_threaded_irq(keys->dev, info->irq_r, NULL,
+						mtk_pmic_keys_irq_handler_thread,
+						IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
+						"mtk-pmic-keys", info);
+		if (ret) {
+			dev_err(keys->dev, "Failed to request IRQ_r: %d: %d\n",
+				info->irq, ret);
+			return ret;
+		}
+	}
+
 	input_set_capability(keys->input_dev, EV_KEY, info->keycode);
 
 	return 0;
@@ -199,8 +223,11 @@ static int __maybe_unused mtk_pmic_keys_suspend(struct device *dev)
 	int index;
 
 	for (index = 0; index < MTK_PMIC_MAX_KEY_COUNT; index++) {
-		if (keys->keys[index].wakeup)
+		if (keys->keys[index].wakeup) {
 			enable_irq_wake(keys->keys[index].irq);
+			if (keys->keys[index].irq_r > 0)
+				enable_irq_wake(keys->keys[index].irq_r);
+		}
 	}
 
 	return 0;
@@ -212,8 +239,11 @@ static int __maybe_unused mtk_pmic_keys_resume(struct device *dev)
 	int index;
 
 	for (index = 0; index < MTK_PMIC_MAX_KEY_COUNT; index++) {
-		if (keys->keys[index].wakeup)
+		if (keys->keys[index].wakeup) {
 			disable_irq_wake(keys->keys[index].irq);
+			if (keys->keys[index].irq_r > 0)
+				disable_irq_wake(keys->keys[index].irq_r);
+		}
 	}
 
 	return 0;
@@ -229,6 +259,9 @@ static const struct of_device_id of_mtk_pmic_keys_match_tbl[] = {
 	}, {
 		.compatible = "mediatek,mt6323-keys",
 		.data = &mt6323_regs,
+	}, {
+		.compatible = "mediatek,mt6358-keys",
+		.data = &mt6358_regs,
 	}, {
 		/* sentinel */
 	}
@@ -242,6 +275,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 	struct mt6397_chip *pmic_chip = dev_get_drvdata(pdev->dev.parent);
 	struct device_node *node = pdev->dev.of_node, *child;
 	static const char *const irqnames[] = { "powerkey", "homekey" };
+	static const char *const irqnames_r[] = { "powerkey_r", "homekey_r" };
 	struct mtk_pmic_keys *keys;
 	const struct mtk_pmic_regs *mtk_pmic_regs;
 	struct input_dev *input_dev;
@@ -285,6 +319,16 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 			return keys->keys[index].irq;
 		}
 
+		if (of_device_is_compatible(node, "mediatek,mt6358-keys")) {
+			keys->keys[index].irq_r = platform_get_irq_byname(pdev,
+									  irqnames_r[index]);
+
+			if (keys->keys[index].irq_r < 0) {
+				of_node_put(child);
+				return keys->keys[index].irq_r;
+			}
+		}
+
 		error = of_property_read_u32(child,
 			"linux,keycodes", &keys->keys[index].keycode);
 		if (error) {
-- 
2.32.0


^ permalink raw reply related

* [PATCH v4 4/4] arm64: dts: mt6358: add mt6358-keys node
From: Mattijs Korpershoek @ 2022-01-21 14:03 UTC (permalink / raw)
  To: Dmitry Torokhov, Matthias Brugger, Rob Herring
  Cc: Fabien Parent, Kevin Hilman, linux-input, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel,
	Mattijs Korpershoek
In-Reply-To: <20220121140323.4080640-1-mkorpershoek@baylibre.com>

This enables the power,home keys on MediaTek boards with a mt6358 pmic.

Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 arch/arm64/boot/dts/mediatek/mt6358.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt6358.dtsi b/arch/arm64/boot/dts/mediatek/mt6358.dtsi
index 95145076b7e6..98f3b0e0c9f6 100644
--- a/arch/arm64/boot/dts/mediatek/mt6358.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt6358.dtsi
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) 2020 MediaTek Inc.
  */
+#include <dt-bindings/input/input.h>
 
 &pwrap {
 	pmic: mt6358 {
@@ -357,5 +358,16 @@ mt6358_vsim2_reg: ldo_vsim2 {
 		mt6358rtc: mt6358rtc {
 			compatible = "mediatek,mt6358-rtc";
 		};
+
+		mt6358keys: mt6358keys {
+			compatible = "mediatek,mt6358-keys";
+			power {
+				linux,keycodes = <KEY_POWER>;
+				wakeup-source;
+			};
+			home {
+				linux,keycodes = <KEY_HOME>;
+			};
+		};
 	};
 };
-- 
2.32.0


^ permalink raw reply related

* [PATCH v4 2/4] dt-bindings: input: mtk-pmic-keys: add MT6358 binding definition
From: Mattijs Korpershoek @ 2022-01-21 14:03 UTC (permalink / raw)
  To: Dmitry Torokhov, Matthias Brugger, Rob Herring
  Cc: Fabien Parent, Kevin Hilman, linux-input, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel,
	Mattijs Korpershoek, Rob Herring
In-Reply-To: <20220121140323.4080640-1-mkorpershoek@baylibre.com>

Add the binding documentation of the mtk-pmic-keys for the MT6358 PMICs.

MT6358 is a little different since it used separate IRQs for the
release key (_r) event

Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/input/mtk-pmic-keys.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt b/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
index 535d92885372..9d00f2a8e13a 100644
--- a/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
+++ b/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
@@ -9,7 +9,10 @@ For MT6397/MT6323 MFD bindings see:
 Documentation/devicetree/bindings/mfd/mt6397.txt
 
 Required properties:
-- compatible: "mediatek,mt6397-keys" or "mediatek,mt6323-keys"
+- compatible: Should be one of:
+	- "mediatek,mt6397-keys"
+	- "mediatek,mt6323-keys"
+	- "mediatek,mt6358-keys"
 - linux,keycodes: See Documentation/devicetree/bindings/input/input.yaml
 
 Optional Properties:
-- 
2.32.0


^ permalink raw reply related

* [PATCH v4 1/2] leds: ktd20xx: Extension of the KTD20xx family of LED drivers from Kinetic
From: Florian Eckert @ 2022-01-21 14:01 UTC (permalink / raw)
  To: pavel, robh+dt, andy.shevchenko
  Cc: Eckert.Florian, linux-kernel, linux-leds, devicetree
In-Reply-To: <20220121140150.1729-1-fe@dev.tdt.de>

Introducing the KTD2061/58/59/60 RGB LED drivers. The difference in
these are the address numbers on the I2C bus that the device listens to.

All KT20xx units can drive up to 12 LEDs.

Due to the hardware limitation, we can only set 7 colors and the color
black (LED off) for each LED independently and not the full RGB range.
This is because the chip only has two color registers.

To control the LEDs independently, the chip has to be configured in a
special way.

Color register 0 must be loaded with the current value 0mA, and color
register 1 must be loaded with the value 'kinetic,led-current' from the
device tree node. If the property is omitted, the register is loaded
with the default value (0x28 = 5mA).

To select a color for an LED, a combination must be written to the color
selection register of that LED. This range for selecting the value is 3
bits wide (RGB). A '0' in any of the bits uses color register '0' and a
'1' uses color register '1'.

So we could choose the following combination for each LED:
R G B
0 0 0 = Black (off)
0 0 1 = Blue
0 1 0 = green
0 1 1 = Cyan
1 0 0 = Red
1 0 1 = Magenta
1 1 0 = Yellow
1 1 1 = White

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 MAINTAINERS                 |   6 +
 drivers/leds/Kconfig        |  12 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-ktd20xx.c | 582 ++++++++++++++++++++++++++++++++++++
 4 files changed, 601 insertions(+)
 create mode 100644 drivers/leds/leds-ktd20xx.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a58544f7b699..04d68985d348 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10739,6 +10739,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/leds/backlight/kinetic,ktd253.yaml
 F:	drivers/video/backlight/ktd253-backlight.c
 
+KTD20XX LED CONTROLLER DRIVER
+M:	Florian Eckert <fe@dev.tdt.de>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+F:	drivers/leds/leds-ktd20xx.c
+
 KTEST
 M:	Steven Rostedt <rostedt@goodmis.org>
 M:	John Hawley <warthog9@eaglescrag.net>
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6090e647daee..a96e6bf7918b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -157,6 +157,18 @@ config LEDS_EL15203000
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-el15203000.
 
+config LEDS_KTD20XX
+	tristate "LED Support for KTD2061/58/59/60 LED driver chip"
+	depends on LEDS_CLASS && I2C
+	depends on LEDS_CLASS_MULTICOLOR
+	select REGMAP_I2C
+	help
+	  If you say yes here you get support for the Kinetic
+	  KTD2061, KTD2058, KTD2059 and KTD2060 LED driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called leds-ktd20xx.
+
 config LEDS_TURRIS_OMNIA
 	tristate "LED support for CZ.NIC's Turris Omnia"
 	depends on LEDS_CLASS_MULTICOLOR
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index e58ecb36360f..5a86e72ea722 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_LEDS_IP30)			+= leds-ip30.o
 obj-$(CONFIG_LEDS_IPAQ_MICRO)		+= leds-ipaq-micro.o
 obj-$(CONFIG_LEDS_IS31FL319X)		+= leds-is31fl319x.o
 obj-$(CONFIG_LEDS_IS31FL32XX)		+= leds-is31fl32xx.o
+obj-${CONFIG_LEDS_KTD20XX}		+= leds-ktd20xx.o
 obj-$(CONFIG_LEDS_LM3530)		+= leds-lm3530.o
 obj-$(CONFIG_LEDS_LM3532)		+= leds-lm3532.o
 obj-$(CONFIG_LEDS_LM3533)		+= leds-lm3533.o
diff --git a/drivers/leds/leds-ktd20xx.c b/drivers/leds/leds-ktd20xx.c
new file mode 100644
index 000000000000..7e2a1a603b50
--- /dev/null
+++ b/drivers/leds/leds-ktd20xx.c
@@ -0,0 +1,582 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  LEDs driver for the Kinetic KDT20xx device
+ *
+ *  Copyright (C) 2021 TDT AG Florian Eckert <fe@dev.tdt.de>
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+/* I2C Register Map */
+#define KTD20XX_ID		0x00
+#define KTD20XX_MONITOR		0x01
+#define KTD20XX_CONTROL		0x02
+
+/* Color0 Configuration Registers */
+#define KTD20XX_IRED0		0x03
+#define KTD20XX_IGRN0		0x04
+#define KTD20XX_IBLU0		0x05
+
+/* Color1 Configuration Registers */
+#define KTD20XX_IRED1		0x06
+#define KTD20XX_IGRN1		0x07
+#define KTD20XX_IBLU1		0x08
+
+/* Selection Configuration Register */
+#define KTD20XX_ISELA12		0x09
+#define KTD20XX_ISELA34		0x0A
+#define KTD20XX_ISELB12		0x0B
+#define KTD20XX_ISELB34		0x0C
+#define KTD20XX_ISELC12		0x0D
+#define KTD20XX_ISELC34		0x0E
+
+#define KTD20XX_MAX_LEDS	12
+#define KTD20XX_LED_CHANNELS	3
+
+enum ktd20xx_led_number {
+	/* ISELA12 */
+	RGB_A1,
+	RGB_A2,
+	/* ISELA34 */
+	RGB_A3,
+	RGB_A4,
+	/* ISELB12 */
+	RGB_B1,
+	RGB_B2,
+	/* ISELB34 */
+	RGB_B3,
+	RGB_B4,
+	/* ISELC12 */
+	RGB_C1,
+	RGB_C2,
+	/* ISELC34 */
+	RGB_C3,
+	RGB_C4,
+};
+
+enum ktd20xx_control_mode {
+	CONTROL_MODE_OFF,
+	CONTROL_MODE_NIGHT,
+	CONTROL_MODE_NORMAL,
+	CONTROL_MODE_RESET,
+};
+
+static const struct reg_default ktd20xx_reg_defs[] = {
+	/* Color0 Configuration Registers */
+	{KTD20XX_IRED0, 0x28},
+	{KTD20XX_IGRN0, 0x28},
+	{KTD20XX_IBLU0, 0x28},
+	/* Color1 Configuration Registers */
+	{KTD20XX_IRED1, 0x60},
+	{KTD20XX_IGRN1, 0x60},
+	{KTD20XX_IBLU1, 0x60},
+	/* Selection Configuration Register */
+	{KTD20XX_ISELA12, 0x00},
+	{KTD20XX_ISELA34, 0x00},
+	{KTD20XX_ISELB12, 0x00},
+	{KTD20XX_ISELB34, 0x00},
+	{KTD20XX_ISELC12, 0x00},
+	{KTD20XX_ISELC34, 0x00}
+};
+
+/* Chip values */
+static const struct reg_field kt20xx_control_mode = REG_FIELD(KTD20XX_CONTROL, 6, 7);
+static const struct reg_field kt20xx_faderate = REG_FIELD(KTD20XX_CONTROL, 0, 2);
+static const struct reg_field kt20xx_vendor = REG_FIELD(KTD20XX_ID, 5, 7);
+static const struct reg_field kt20xx_chip_id = REG_FIELD(KTD20XX_ID, 0, 4);
+static const struct reg_field kt20xx_chip_rev = REG_FIELD(KTD20XX_MONITOR, 4, 7);
+
+/* ISELA1 and ISELA2 */
+static const struct reg_field kt20xx_a1_select = REG_FIELD(KTD20XX_ISELA12, 4, 6);
+static const struct reg_field kt20xx_a1_enable = REG_FIELD(KTD20XX_ISELA12, 7, 7);
+static const struct reg_field kt20xx_a2_select = REG_FIELD(KTD20XX_ISELA12, 0, 2);
+static const struct reg_field kt20xx_a2_enable = REG_FIELD(KTD20XX_ISELA12, 3, 3);
+
+/* ISELA3 and ISELA4 */
+static const struct reg_field kt20xx_a3_select = REG_FIELD(KTD20XX_ISELA34, 4, 6);
+static const struct reg_field kt20xx_a3_enable = REG_FIELD(KTD20XX_ISELA34, 7, 7);
+static const struct reg_field kt20xx_a4_select = REG_FIELD(KTD20XX_ISELA34, 0, 2);
+static const struct reg_field kt20xx_a4_enable = REG_FIELD(KTD20XX_ISELA34, 3, 3);
+
+/* ISELB1 and ISELB2 */
+static const struct reg_field kt20xx_b1_select = REG_FIELD(KTD20XX_ISELB12, 4, 6);
+static const struct reg_field kt20xx_b1_enable = REG_FIELD(KTD20XX_ISELB12, 7, 7);
+static const struct reg_field kt20xx_b2_select = REG_FIELD(KTD20XX_ISELB12, 0, 2);
+static const struct reg_field kt20xx_b2_enable = REG_FIELD(KTD20XX_ISELB12, 3, 3);
+
+/* ISELB3 and ISELB4 */
+static const struct reg_field kt20xx_b3_select = REG_FIELD(KTD20XX_ISELB34, 4, 6);
+static const struct reg_field kt20xx_b3_enable = REG_FIELD(KTD20XX_ISELB34, 7, 7);
+static const struct reg_field kt20xx_b4_select = REG_FIELD(KTD20XX_ISELB34, 0, 2);
+static const struct reg_field kt20xx_b4_enable = REG_FIELD(KTD20XX_ISELB34, 3, 3);
+
+/* ISELC1 and ISELC2 */
+static const struct reg_field kt20xx_c1_select = REG_FIELD(KTD20XX_ISELC12, 4, 6);
+static const struct reg_field kt20xx_c1_enable = REG_FIELD(KTD20XX_ISELC12, 7, 7);
+static const struct reg_field kt20xx_c2_select = REG_FIELD(KTD20XX_ISELC12, 0, 2);
+static const struct reg_field kt20xx_c2_enable = REG_FIELD(KTD20XX_ISELC12, 3, 3);
+
+/* ISELC3 and ISELC4 */
+static const struct reg_field kt20xx_c3_select = REG_FIELD(KTD20XX_ISELC34, 4, 6);
+static const struct reg_field kt20xx_c3_enable = REG_FIELD(KTD20XX_ISELC34, 7, 7);
+static const struct reg_field kt20xx_c4_select = REG_FIELD(KTD20XX_ISELC34, 0, 2);
+static const struct reg_field kt20xx_c4_enable = REG_FIELD(KTD20XX_ISELC34, 3, 3);
+
+static const struct regmap_range ktd20xx_volatile_ranges = {
+	.range_min = KTD20XX_ID,
+	.range_max = KTD20XX_CONTROL,
+};
+
+static const struct regmap_access_table ktd20xx_volatile_table = {
+	.yes_ranges = &ktd20xx_volatile_ranges,
+	.n_yes_ranges = 1,
+};
+
+static const struct regmap_range ktd20xx_readable_ranges = {
+	.range_min = KTD20XX_ID,
+	.range_max = KTD20XX_MONITOR,
+};
+
+static const struct regmap_access_table ktd20xx_readable_table = {
+	.yes_ranges = &ktd20xx_readable_ranges,
+	.n_yes_ranges = 1,
+};
+
+static const struct regmap_config ktd20xx_regmap_config = {
+	.name = "ktd20xx_regmap",
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = KTD20XX_ISELC34,
+
+	.volatile_table = &ktd20xx_volatile_table,
+	.rd_table = &ktd20xx_readable_table,
+
+	.reg_defaults = ktd20xx_reg_defs,
+	.num_reg_defaults = ARRAY_SIZE(ktd20xx_reg_defs),
+	.cache_type = REGCACHE_FLAT,
+};
+
+struct ktd20xx_led {
+	struct led_classdev_mc mc_cdev;
+	struct mc_subled subled_info[KTD20XX_LED_CHANNELS];
+	int index;
+	struct regmap_field *enable;
+	struct regmap_field *select;
+	struct ktd20xx *chip;
+};
+
+struct ktd20xx {
+	struct mutex lock;
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct regmap_field *control_mode;
+	struct regmap_field *faderate;
+	struct regmap_field *vendor;
+	struct regmap_field *chip_id;
+	struct regmap_field *chip_rev;
+	struct ktd20xx_led leds[KTD20XX_MAX_LEDS];
+};
+
+static int ktd20xx_hwinit(struct ktd20xx *chip)
+{
+	struct device *dev = &chip->client->dev;
+	int ret;
+	unsigned int value;
+
+	/*
+	 * If the device tree property 'kinetc,led-current' is found
+	 * then set this value into the color0 register as the max current
+	 * for all color channel LEDs. If this poperty is not set then
+	 * use the default value 0x28 set by the chip after a hardware reset.
+	 * The hardware default value 0x28 corresponds to 5mA.
+	 */
+	/* Set color1 register current value to 0x00 and therefor 0mA */
+	regmap_write(chip->regmap, KTD20XX_IRED1, 0);
+	regmap_write(chip->regmap, KTD20XX_IGRN1, 0);
+	regmap_write(chip->regmap, KTD20XX_IBLU1, 0);
+
+	ret = device_property_read_u32(dev, "kinetic,led-current", &value);
+	if (ret) {
+		dev_warn(dev, "property 'kinetic,led-current' not found. Using default hardware value 0x28 (5mA).\n");
+	} else {
+		dev_dbg(dev, "property 'kinetic,led-current' found. Using value 0x%02x.\n",
+				value);
+		regmap_write(chip->regmap, KTD20XX_IRED0, value);
+		regmap_write(chip->regmap, KTD20XX_IGRN0, value);
+		regmap_write(chip->regmap, KTD20XX_IBLU0, value);
+	}
+
+	/* Enable chip to run in 'normal mode' */
+	regmap_field_write(chip->control_mode, CONTROL_MODE_NORMAL);
+
+	return 0;
+}
+
+static struct ktd20xx_led *mcled_cdev_to_led(struct led_classdev_mc *mc_cdev)
+{
+	return container_of(mc_cdev, struct ktd20xx_led, mc_cdev);
+}
+
+static int ktd20xx_brightness_set(struct led_classdev *cdev,
+		enum led_brightness brightness)
+{
+	struct led_classdev_mc *mc_dev = lcdev_to_mccdev(cdev);
+	struct ktd20xx_led *led = mcled_cdev_to_led(mc_dev);
+	struct device *dev = &led->chip->client->dev;
+	int ret;
+	int i;
+	unsigned long rgb = 0;
+
+	mutex_lock(&led->chip->lock);
+	ret = regmap_field_write(led->enable, brightness ? 1 : 0);
+	if (ret) {
+		dev_err(dev, "Cannot set enable flag of LED %d error: %d\n",
+				led->index, ret);
+		goto unlock_out;
+	}
+
+	for (i = 0; i < led->mc_cdev.num_colors; i++) {
+		unsigned int intensity = mc_dev->subled_info[i].intensity;
+		unsigned int channel = mc_dev->subled_info[i].channel;
+
+		if (intensity > 0)
+			set_bit(channel, &rgb);
+	}
+
+	/*
+	 * To use the color0 registers default value after an hadware reset,
+	 * if the device tree property 'kinetc,led-current' is not set,
+	 * we have to 'invert' the rgb channel!
+	 */
+	ret = regmap_field_write(led->select, ~rgb);
+	if (ret) {
+		dev_err(dev, "Can not set RGB for LED %d error: %d\n",
+				led->index, ret);
+		goto unlock_out;
+	}
+
+unlock_out:
+	mutex_unlock(&led->chip->lock);
+	return ret;
+}
+
+static int ktd20xx_probe_dt(struct ktd20xx *chip)
+{
+	struct fwnode_handle *child = NULL;
+	struct led_init_data init_data = {};
+	struct led_classdev *led_cdev;
+	struct ktd20xx_led *led;
+	struct device *dev = &chip->client->dev;
+	int color;
+	int ret;
+	int i = 0;
+
+	device_for_each_child_node(dev, child) {
+		led = &chip->leds[i];
+
+		ret = fwnode_property_read_u32(child, "reg", &led->index);
+		if (ret) {
+			dev_err(dev, "missing property 'reg'\n");
+			goto child_out;
+		}
+		if (led->index >= KTD20XX_MAX_LEDS) {
+			dev_warn(dev, "property 'reg' is greater then '%i'\n",
+					KTD20XX_MAX_LEDS);
+			ret = -EINVAL;
+			goto child_out;
+		}
+
+		ret = fwnode_property_read_u32(child, "color", &color);
+		if (ret) {
+			dev_err(dev, "missing property 'color'\n");
+			goto child_out;
+		}
+		if (color != LED_COLOR_ID_MULTI) {
+			dev_warn(dev, "property 'color' is not equal to the value 'LED_COLOR_ID_MULTI'\n");
+			ret = -EINVAL;
+			goto child_out;
+		}
+
+		led->subled_info[0].color_index = LED_COLOR_ID_RED;
+		led->subled_info[0].channel = 2;
+		led->subled_info[0].intensity = 1;
+		led->subled_info[1].color_index = LED_COLOR_ID_GREEN;
+		led->subled_info[1].channel = 1;
+		led->subled_info[1].intensity = 1;
+		led->subled_info[2].color_index = LED_COLOR_ID_BLUE;
+		led->subled_info[2].channel = 0;
+		led->subled_info[2].intensity = 1;
+
+		led->mc_cdev.subled_info = led->subled_info;
+		led->mc_cdev.num_colors = KTD20XX_LED_CHANNELS;
+
+		init_data.fwnode = child;
+
+		led->chip = chip;
+		led_cdev = &led->mc_cdev.led_cdev;
+		led_cdev->brightness_set_blocking = ktd20xx_brightness_set;
+
+		switch (led->index) {
+		case RGB_A1:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a1_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a1_enable);
+			break;
+		case RGB_A2:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a2_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a2_enable);
+			break;
+		case RGB_A3:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a3_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a3_enable);
+			break;
+		case RGB_A4:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a4_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_a4_enable);
+			break;
+		case RGB_B1:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b1_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b1_enable);
+			break;
+		case RGB_B2:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b2_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b2_enable);
+			break;
+		case RGB_B3:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b3_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b3_enable);
+			break;
+		case RGB_B4:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b4_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_b4_enable);
+			break;
+		case RGB_C1:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c1_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c1_enable);
+			break;
+		case RGB_C2:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c2_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c2_enable);
+			break;
+		case RGB_C3:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c3_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c3_enable);
+			break;
+		case RGB_C4:
+			led->select = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c4_select);
+			led->enable = devm_regmap_field_alloc(dev,
+					chip->regmap, kt20xx_c4_enable);
+			break;
+		}
+
+		ret = devm_led_classdev_multicolor_register_ext(dev,
+			&led->mc_cdev,
+			&init_data);
+
+		if (ret) {
+			dev_err(dev, "led register err: %d\n", ret);
+			goto child_out;
+		}
+
+		i++;
+	}
+
+	return 0;
+
+child_out:
+	fwnode_handle_put(child);
+	return ret;
+}
+
+/*
+ * The chip also offers the option "Night Mode".
+ * All LED current settings are divided by 16 for a 0 to 1.5mA current
+ * setting range.
+ */
+static ssize_t nightmode_show(struct device *dev, struct device_attribute *a,
+		char *buf)
+{
+	struct ktd20xx *chip = dev_get_drvdata(dev);
+	unsigned int value;
+
+	mutex_lock(&chip->lock);
+	regmap_field_read(chip->control_mode, &value);
+	mutex_unlock(&chip->lock);
+
+	return sysfs_emit(buf, "%d\n", value == CONTROL_MODE_NIGHT ? 1 : 0);
+}
+
+static ssize_t nightmode_store(struct device *dev, struct device_attribute *a,
+		const char *buf, size_t count)
+{
+	struct ktd20xx *chip = dev_get_drvdata(dev);
+	bool value;
+	int ret;
+
+	ret = kstrtobool(buf, &value);
+	if (ret)
+		return ret;
+
+	mutex_lock(&chip->lock);
+	ret = regmap_field_write(chip->control_mode,
+			value == 1 ? CONTROL_MODE_NIGHT : CONTROL_MODE_NORMAL);
+	mutex_unlock(&chip->lock);
+
+	if (ret)
+		return ret;
+
+	return count;
+}
+static DEVICE_ATTR_RW(nightmode);
+
+static struct attribute *ktd20xx_led_controller_attrs[] = {
+	&dev_attr_nightmode.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(ktd20xx_led_controller);
+
+static int ktd20xx_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct ktd20xx *chip;
+	unsigned int vendor;
+	unsigned int chip_id;
+	unsigned int chip_rev;
+	struct device *dev;
+	int ret;
+
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	mutex_init(&chip->lock);
+	chip->client = client;
+	dev = &client->dev;
+	i2c_set_clientdata(client, chip);
+
+	chip->regmap = devm_regmap_init_i2c(client, &ktd20xx_regmap_config);
+	if (IS_ERR(chip->regmap)) {
+		dev_err_probe(dev, PTR_ERR(chip->regmap),
+			"Failed to allocate register map\n");
+		goto error;
+	}
+
+	chip->control_mode = devm_regmap_field_alloc(dev, chip->regmap,
+			kt20xx_control_mode);
+	chip->faderate = devm_regmap_field_alloc(dev, chip->regmap,
+			kt20xx_faderate);
+	chip->vendor = devm_regmap_field_alloc(dev, chip->regmap,
+			kt20xx_vendor);
+	chip->chip_id = devm_regmap_field_alloc(dev, chip->regmap,
+			kt20xx_chip_id);
+	chip->chip_rev = devm_regmap_field_alloc(dev, chip->regmap,
+			kt20xx_chip_rev);
+
+	/* Reset all registers to hardware device default settings */
+	regmap_field_write(chip->control_mode, CONTROL_MODE_RESET);
+
+	ret = regmap_field_read(chip->vendor, &vendor);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to read vendor\n");
+		goto error;
+	}
+
+	ret = regmap_field_read(chip->chip_id, &chip_id);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to read chip id\n");
+		goto error;
+	}
+
+	ret = regmap_field_read(chip->chip_rev, &chip_rev);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to read chip rev\n");
+		goto error;
+	}
+
+	dev_info(dev, "vendor: 0x%02x chip-id: 0x%02x chip-rev: 0x%02x\n",
+			vendor, chip_id, chip_rev);
+
+	ret = ktd20xx_probe_dt(chip);
+	if (ret)
+		goto error;
+
+	ret = ktd20xx_hwinit(chip);
+	if (ret)
+		goto error;
+
+	return 0;
+
+error:
+	return ret;
+}
+
+static int ktd20xx_remove(struct i2c_client *client)
+{
+	struct ktd20xx *chip = i2c_get_clientdata(client);
+
+	mutex_lock(&chip->lock);
+	regmap_field_write(chip->control_mode, CONTROL_MODE_OFF);
+	mutex_unlock(&chip->lock);
+
+	return 0;
+}
+
+static const struct i2c_device_id ktd20xx_id[] = {
+	{ "ktd20xx", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ktd20xx_id);
+
+static const struct of_device_id of_ktd20xx_leds_match[] = {
+	{ .compatible = "kinetic,ktd20xx", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_ktd20xx_leds_match);
+
+static struct i2c_driver ktd20xx_driver = {
+	.driver = {
+		.name = "ktd20xx",
+		.dev_groups = ktd20xx_led_controller_groups,
+		.of_match_table = of_ktd20xx_leds_match,
+	},
+	.probe = ktd20xx_probe,
+	.remove = ktd20xx_remove,
+	.id_table = ktd20xx_id
+};
+module_i2c_driver(ktd20xx_driver);
+
+MODULE_DESCRIPTION("Kinetic KTD20xx LED driver");
+MODULE_AUTHOR("Florian Eckert <fe@dev.tdt.de>");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 2/2] dt: bindings: KTD20xx: Introduce the ktd20xx family of RGB drivers
From: Florian Eckert @ 2022-01-21 14:01 UTC (permalink / raw)
  To: pavel, robh+dt, andy.shevchenko
  Cc: Eckert.Florian, linux-kernel, linux-leds, devicetree, Rob Herring
In-Reply-To: <20220121140150.1729-1-fe@dev.tdt.de>

Introduce the bindings for the Kinetic KTD2061/58/59/60RGB LED device
driver. The KTD20xx can control RGB LEDs individually. Because of the
hardware limitations, only 7 colors and the color black (off) can be set.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/leds/leds-ktd20xx.yaml           | 130 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd20xx.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-ktd20xx.yaml b/Documentation/devicetree/bindings/leds/leds-ktd20xx.yaml
new file mode 100644
index 000000000000..c4e440cc6945
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-ktd20xx.yaml
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-ktd20xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LED driver for KTD20xx RGB LED from Kinetic.
+
+maintainers:
+  - Florian Eckert <fe@dev.tdt.de>
+
+description: |
+  The KTD20XX is multi-channel, I2C LED driver. Into can control up to 12
+  LEDs per device. The RGB value can be set for each LED. Due to hardware
+  limitations, the full RGB range cannot be used. Only 7 colors and the
+  color black can be set (black means off).
+  R G B
+  0 0 0 = Black (off)
+  0 0 1 = Blue
+  0 1 0 = Green
+  0 1 1 = Cyan
+  1 0 0 = Red
+  1 0 1 = Magenta
+  1 1 0 = Yellow
+  1 1 1 = White
+
+properties:
+  compatible:
+    enum:
+      - kinetic,ktd20xx
+
+  reg:
+    maxItems: 1
+    description:
+      I2C slave address
+      ktd2061/58/59/60 0x68 0x69 0x6A 0x6B
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  'kinetic,led-current':
+    $ref: /schemas/types.yaml#/definitions/uint8
+    description:
+      This value is a current setting for all LEDs connected to this device.
+      If this value is not set then the default value off 0x28 (5mA) is set.
+      This means all LEDs get 5mA. The max value is 24mA. We have the
+      following mapping in 125uA steps. We can set a maximum of 24mA.
+      0000 0000 (0x00) = 0uA
+      0000 0001 (0x01) = 125uA
+      .... ....
+      0010 1000 (0x28) = 5mA
+      .... ....
+      1100 0000 (0xC0) = 24mA
+      1100 0001 (0xC1) = 24mA
+      .... ....
+      1111 1111 (0xFF) = 24mA
+    minimum: 0
+    maximum: 255
+
+patternProperties:
+  '^multi-led@[0-9a-f]$':
+    type: object
+    allOf:
+      - $ref: leds-class-multicolor.yaml#
+    description:
+      This node represents one of the Multicolor LED. No subnodes need to
+      be added for subchannels since this controller only supports 1bit
+      RGB values. We could display seven different colors and the color
+      black which means off.
+
+    properties:
+      reg:
+        minimum: 0
+        maximum: 11
+        description:
+          This property identifies wired connection of the LED to this device.
+          0x00  LEDA1
+          0x01  LEDA2
+          0x02  LEDA3
+          0x03  LEDA4
+          0x04  LEDB1
+          0x05  LEDB2
+          0x06  LEDB3
+          0x07  LEDB4
+          0x08  LEDC1
+          0x09  LEDC2
+          0x0A  LEDC3
+          0x0B  LEDC4
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+   #include <dt-bindings/leds/common.h>
+
+   i2c {
+       #address-cells = <1>;
+       #size-cells = <0>;
+
+       led-controller@14 {
+           compatible = "kinetic,ktd20xx";
+           reg = <0x68>;
+           #address-cells = <1>;
+           #size-cells = <0>;
+           kinetic,led-current = <0x28>; // Current for all LEDs is 5mA
+
+           multi-led@0 {
+               reg = <0x0>;
+               color = <LED_COLOR_ID_MULTI>;
+               function = LED_FUNCTION_CHARGING;
+               linux,default-trigger = "default-on";
+          };
+
+          multi-led@2 {
+            reg = <0x2>;
+            color = <LED_COLOR_ID_MULTI>;
+            function = LED_FUNCTION_STANDBY;
+            linux,default-trigger = "default-on";
+         };
+       };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 04d68985d348..b56d8392119c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10743,6 +10743,7 @@ KTD20XX LED CONTROLLER DRIVER
 M:	Florian Eckert <fe@dev.tdt.de>
 L:	linux-leds@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/leds/leds-ktd20xx.yaml
 F:	drivers/leds/leds-ktd20xx.c
 
 KTEST
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 0/2] leds: add ktd20xx LED driver support
From: Florian Eckert @ 2022-01-21 14:01 UTC (permalink / raw)
  To: pavel, robh+dt, andy.shevchenko
  Cc: Eckert.Florian, linux-kernel, linux-leds, devicetree

v1: Initial send
v2: Remove variant 1 from source
v3: Changes requested by Andy Shevchenko added. Thanks for reviewing
  - Removing OF dependency
  - Add missing includes
  - Use device_property_read_u32() instead of fwnode_property_read_u32()
  - Use one liner function pattern <test> ? <value-true> : <value-false>
  - Remove switch case call for intensity color selection use BIT()
    instead
  - Remove not needed fwnode_handle_put() in ktd200xx_probe_dt()
    function
  - Use dev_get_drvdata() instead of i2c_get_clientdata() function call
  - Use sysfs_emit() function call
  - Use kstrtobool() function call
  - Remove not needed comma after last array element
  - Use dev_err_probe() instead of dev_error() in driver probe function
  - Do not use dev_group registration function set .dev_groups directly
    into ktd20xx_driver struct.
v4: Changes requested by Andy Shevchenko. Thanks again for your review
  - Fix Author indentation
  - Reduce logging noise
  - Use 'if' standard pattern
  - Use set_bit function to make code cleaner
  - Use meaningful jump labels
  - Updating the logging output to better match the source code
  - Remove duplicate dev pointer usage. This is not necessary as the
    information can be used directly from the client structure
  - Do not hide return value from kstrbool function
  - Do not use mutex_destroy function in devm mananged structs

Florian Eckert (2):
  leds: ktd20xx: Extension of the KTD20xx family of LED drivers from
    Kinetic
  dt: bindings: KTD20xx: Introduce the ktd20xx family of RGB drivers

 .../bindings/leds/leds-ktd20xx.yaml           | 130 ++++
 MAINTAINERS                                   |   7 +
 drivers/leds/Kconfig                          |  12 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-ktd20xx.c                   | 582 ++++++++++++++++++
 5 files changed, 732 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-ktd20xx.yaml
 create mode 100644 drivers/leds/leds-ktd20xx.c

-- 
2.20.1


^ permalink raw reply

* [PATCH V4 6/6] MAINTAINERS: Add maintainer entry for EUD
From: Souradeep Chowdhury @ 2022-01-21 13:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson,
	greg, robh
  Cc: linux-kernel, quic_tsoni, quic_psodagud, quic_satyap,
	quic_pheragu, quic_rjendra, quic_sibis, quic_saipraka,
	quic_schowdhu
In-Reply-To: <cover.1642768837.git.quic_schowdhu@quicinc.com>

Add the entry for maintainer for EUD driver
and other associated files.

Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b84e2d5..0fa9d54 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7227,6 +7227,14 @@ F:	include/uapi/linux/mdio.h
 F:	include/uapi/linux/mii.h
 F:	net/core/of_net.c
 
+QCOM EMBEDDED USB DEBUGGER(EUD)
+M:	Souradeep Chowdhury <quic_schowdhu@quicinc.com>
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/ABI/testing/sysfs-driver-eud
+F:	Documentation/devicetree/bindings/arm/msm/qcom,eud.yaml
+F:	drivers/usb/common/qcom_eud.c
+
 EXEC & BINFMT API
 R:	Eric Biederman <ebiederm@xmission.com>
 R:	Kees Cook <keescook@chromium.org>
-- 
2.7.4


^ permalink raw reply related

* [PATCH V4 5/6] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2
From: Souradeep Chowdhury @ 2022-01-21 13:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson,
	greg, robh
  Cc: linux-kernel, quic_tsoni, quic_psodagud, quic_satyap,
	quic_pheragu, quic_rjendra, quic_sibis, quic_saipraka,
	quic_schowdhu
In-Reply-To: <cover.1642768837.git.quic_schowdhu@quicinc.com>

Set the default dr_mode for usb2 node to "otg" to enable
role-switch for EUD(Embedded USB Debugger) connector node.

Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280-idp.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 9b991ba..f40eaa5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -61,6 +61,10 @@
 	modem-init;
 };
 
+&usb_2_dwc3 {
+	dr_mode = "otg";
+};
+
 &pmk8350_rtc {
 	status = "okay";
 };
-- 
2.7.4


^ permalink raw reply related

* [PATCH V4 4/6] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector
From: Souradeep Chowdhury @ 2022-01-21 13:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson,
	greg, robh
  Cc: linux-kernel, quic_tsoni, quic_psodagud, quic_satyap,
	quic_pheragu, quic_rjendra, quic_sibis, quic_saipraka,
	quic_schowdhu
In-Reply-To: <cover.1642768837.git.quic_schowdhu@quicinc.com>

Add the Embedded USB Debugger(EUD) device tree node. The
node contains EUD base register region and EUD mode
manager register regions along with the interrupt entry.
Also add the typec connector node for EUD which is attached to
EUD node via port. EUD is also attached to DWC3 node via port.
Also add the role-switch property to dwc3 node.

Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 39 ++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 937c2e0..daac831 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2583,6 +2583,14 @@
 				phys = <&usb_2_hsphy>;
 				phy-names = "usb2-phy";
 				maximum-speed = "high-speed";
+				usb-role-switch;
+				ports {
+					port@0 {
+						usb2_role_switch: endpoint {
+							remote-endpoint = <&eud_ep>;
+						};
+					};
+				};
 			};
 		};
 
@@ -2624,6 +2632,37 @@
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		eud: eud@88e0000 {
+			compatible = "qcom,sc7280-eud","qcom,eud";
+			reg = <0 0x88e0000 0 0x2000>,
+			      <0 0x88e2000 0 0x1000>;
+			interrupt-parent = <&pdc>;
+			interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
+			ports {
+				port@0 {
+					eud_ep: endpoint {
+						remote-endpoint = <&usb2_role_switch>;
+					};
+				};
+				port@1 {
+					eud_con: endpoint {
+						remote-endpoint = <&con_eud>;
+					};
+				};
+			};
+		};
+
+		eud_typec: connector {
+			compatible = "usb-c-connector";
+			ports {
+				port@0 {
+					con_eud: endpoint {
+						remote-endpoint = <&eud_con>;
+					};
+				};
+			};
+		};
+
 		nsp_noc: interconnect@a0c0000 {
 			reg = <0 0x0a0c0000 0 0x10000>;
 			compatible = "qcom,sc7280-nsp-noc";
-- 
2.7.4


^ permalink raw reply related

* [PATCH V4 3/6] soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD)
From: Souradeep Chowdhury @ 2022-01-21 13:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson,
	greg, robh
  Cc: linux-kernel, quic_tsoni, quic_psodagud, quic_satyap,
	quic_pheragu, quic_rjendra, quic_sibis, quic_saipraka,
	quic_schowdhu
In-Reply-To: <cover.1642768837.git.quic_schowdhu@quicinc.com>

Add support for control peripheral of EUD (Embedded USB Debugger) to
listen to events such as USB attach/detach, pet EUD to indicate software
is functional.Reusing the platform device kobj, sysfs entry 'enable' is
created to enable or disable EUD.

To enable the eud the following needs to be done
echo 1 > /sys/bus/platform/.../enable

To disable eud, following is the command
echo 0 > /sys/bus/platform/.../enable

Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
 Documentation/ABI/testing/sysfs-driver-eud |   9 ++
 drivers/soc/qcom/Kconfig                   |  10 ++
 drivers/soc/qcom/Makefile                  |   1 +
 drivers/soc/qcom/qcom_eud.c                | 250 +++++++++++++++++++++++++++++
 4 files changed, 270 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
 create mode 100644 drivers/soc/qcom/qcom_eud.c

diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
new file mode 100644
index 0000000..2381552
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-eud
@@ -0,0 +1,9 @@
+What:		/sys/bus/platform/drivers/eud/.../enable
+Date:           January 2022
+Contact:        Souradeep Chowdhury <quic_schowdhu@quicinc.com>
+Description:
+		The Enable/Disable sysfs interface for Embedded
+		USB Debugger(EUD). This enables and disables the
+		EUD based on a 1 or a 0 value. By enabling EUD,
+		the user is able to activate the mini-usb hub of
+		EUD for debug and trace capabilities.
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e718b87..abc6be0 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -42,6 +42,16 @@ config QCOM_CPR
 	  To compile this driver as a module, choose M here: the module will
 	  be called qcom-cpr

+config QCOM_EUD
+        tristate "QCOM Embedded USB Debugger(EUD) Driver"
+	select USB_ROLE_SWITCH
+	help
+	  This module enables support for Qualcomm Technologies, Inc.
+	  Embedded USB Debugger (EUD). The EUD is a control peripheral
+	  which reports VBUS attach/detach events and has USB-based
+	  debug and trace capabilities. On selecting m, the module name
+	  that is built is qcom_eud.ko
+
 config QCOM_GENI_SE
 	tristate "QCOM GENI Serial Engine Driver"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 70d5de6..e0c7d2d 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_QCOM_AOSS_QMP) +=	qcom_aoss.o
 obj-$(CONFIG_QCOM_GENI_SE) +=	qcom-geni-se.o
 obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_CPR)		+= cpr.o
+obj-$(CONFIG_QCOM_EUD)          += qcom_eud.o
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
 obj-$(CONFIG_QCOM_OCMEM)	+= ocmem.o
diff --git a/drivers/soc/qcom/qcom_eud.c b/drivers/soc/qcom/qcom_eud.c
new file mode 100644
index 0000000..a538645
--- /dev/null
+++ b/drivers/soc/qcom/qcom_eud.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/usb/role.h>
+
+#define EUD_REG_INT1_EN_MASK	0x0024
+#define EUD_REG_INT_STATUS_1	0x0044
+#define EUD_REG_CTL_OUT_1	0x0074
+#define EUD_REG_VBUS_INT_CLR	0x0080
+#define EUD_REG_CSR_EUD_EN	0x1014
+#define EUD_REG_SW_ATTACH_DET	0x1018
+#define EUD_REG_EUD_EN2         0x0000
+
+#define EUD_ENABLE		BIT(0)
+#define EUD_INT_PET_EUD		BIT(0)
+#define EUD_INT_VBUS		BIT(2)
+#define EUD_INT_SAFE_MODE	BIT(4)
+#define EUD_INT_ALL		(EUD_INT_VBUS|EUD_INT_SAFE_MODE)
+
+struct eud_chip {
+	struct device			*dev;
+	struct usb_role_switch		*role_sw;
+	void __iomem			*base;
+	void __iomem			*mode_mgr;
+	unsigned int			int_status;
+	int				irq;
+	bool				enabled;
+	bool				usb_attached;
+};
+
+static int enable_eud(struct eud_chip *priv)
+{
+	writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
+	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
+			priv->base + EUD_REG_INT1_EN_MASK);
+	writel(1, priv->mode_mgr + EUD_REG_EUD_EN2);
+
+	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
+}
+
+static void disable_eud(struct eud_chip *priv)
+{
+	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
+	writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
+}
+
+static ssize_t enable_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct eud_chip *chip = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", chip->enabled);
+}
+
+static ssize_t enable_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct eud_chip *chip = dev_get_drvdata(dev);
+	bool enable;
+	int ret;
+
+	if (kstrtobool(buf, &enable))
+		return -EINVAL;
+
+	if (enable) {
+		ret = enable_eud(chip);
+		if (!ret)
+			chip->enabled = enable;
+		else
+			disable_eud(chip);
+	} else {
+		disable_eud(chip);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(enable);
+
+static struct attribute *eud_attrs[] = {
+	&dev_attr_enable.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(eud);
+
+static void usb_attach_detach(struct eud_chip *chip)
+{
+	u32 reg;
+
+	/* read ctl_out_1[4] to find USB attach or detach event */
+	reg = readl(chip->base + EUD_REG_CTL_OUT_1);
+	chip->usb_attached = reg & EUD_INT_SAFE_MODE;
+}
+
+static void pet_eud(struct eud_chip *chip)
+{
+	u32 reg;
+	int ret;
+
+	/* When the EUD_INT_PET_EUD in SW_ATTACH_DET is set, the cable has been
+	 * disconnected and we need to detach the pet to check if EUD is in safe
+	 * mode before attaching again.
+	 */
+	reg = readl(chip->base + EUD_REG_SW_ATTACH_DET);
+	if (reg & EUD_INT_PET_EUD) {
+		/* Detach & Attach pet for EUD */
+		writel(0, chip->base + EUD_REG_SW_ATTACH_DET);
+		/* Delay to make sure detach pet is done before attach pet */
+		ret = readl_poll_timeout(chip->base + EUD_REG_SW_ATTACH_DET,
+					reg, (reg == 0), 1, 100);
+		if (ret) {
+			dev_err(chip->dev, "Detach pet failed\n");
+			return;
+		}
+	}
+	/* Attach pet for EUD */
+	writel(EUD_INT_PET_EUD, chip->base +EUD_REG_SW_ATTACH_DET);
+}
+
+static irqreturn_t handle_eud_irq(int irq, void *data)
+{
+	struct eud_chip *chip = data;
+	u32 reg;
+
+	reg = readl(chip->base + EUD_REG_INT_STATUS_1);
+	switch (reg & EUD_INT_ALL) {
+	case EUD_INT_VBUS:
+		chip->int_status = EUD_INT_VBUS;
+		usb_attach_detach(chip);
+		return IRQ_WAKE_THREAD;
+	case EUD_INT_SAFE_MODE:
+		pet_eud(chip);
+		return IRQ_HANDLED;
+	default:
+		return IRQ_NONE;
+	}
+}
+
+static irqreturn_t handle_eud_irq_thread(int irq, void *data)
+{
+	struct eud_chip *chip = data;
+	int ret;
+
+	if (chip->int_status == EUD_INT_VBUS) {
+		if (chip->usb_attached)
+			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
+		else
+			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
+		if (ret)
+			dev_err(chip->dev, "failed to set role switch\n");
+	}
+
+	/* set and clear vbus_int_clr[0] to clear interrupt */
+	writel(BIT(0), chip->base + EUD_REG_VBUS_INT_CLR);
+	writel(0, chip->base + EUD_REG_VBUS_INT_CLR);
+
+	return IRQ_HANDLED;
+}
+
+static int eud_probe(struct platform_device *pdev)
+{
+	struct eud_chip *chip;
+	struct fwnode_handle *fwnode = pdev->dev.fwnode, *dwc3;
+	int ret;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = &pdev->dev;
+
+	dwc3 = fwnode_graph_get_next_endpoint(fwnode, NULL);
+	if (!dwc3)
+		return -ENODEV;
+
+	chip->role_sw = fwnode_usb_role_switch_get(dwc3);
+	if (IS_ERR(chip->role_sw)) {
+		ret = PTR_ERR(chip->role_sw);
+		usb_role_switch_put(chip->role_sw);
+		return dev_err_probe(chip->dev, ret,
+					"failed to get role switch\n");
+	}
+
+	chip->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(chip->base))
+		return PTR_ERR(chip->base);
+
+	chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(chip->mode_mgr))
+		return PTR_ERR(chip->mode_mgr);
+
+	chip->irq = platform_get_irq(pdev, 0);
+	ret = devm_request_threaded_irq(&pdev->dev, chip->irq, handle_eud_irq,
+			handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
+	if (ret)
+		return dev_err_probe(chip->dev, ret, "failed to allocate irq\n");
+
+	enable_irq_wake(chip->irq);
+
+	platform_set_drvdata(pdev, chip);
+
+	return 0;
+}
+
+static int eud_remove(struct platform_device *pdev)
+{
+	struct eud_chip *chip = platform_get_drvdata(pdev);
+
+	if (chip->enabled)
+		disable_eud(chip);
+
+	device_init_wakeup(&pdev->dev, false);
+	disable_irq_wake(chip->irq);
+
+	return 0;
+}
+
+static const struct of_device_id eud_dt_match[] = {
+	{ .compatible = "qcom,sc7280-eud" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, eud_dt_match);
+
+static struct platform_driver eud_driver = {
+	.probe	= eud_probe,
+	.remove	= eud_remove,
+	.driver	= {
+		.name = "qcom_eud",
+		.dev_groups = eud_groups,
+		.of_match_table = eud_dt_match,
+	},
+};
+module_platform_driver(eud_driver);
+
+MODULE_DESCRIPTION("QTI EUD driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4


^ permalink raw reply related

* [PATCH V4 2/6] bindings: usb: dwc3: Update dwc3 properties for EUD connector
From: Souradeep Chowdhury @ 2022-01-21 13:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson,
	greg, robh
  Cc: linux-kernel, quic_tsoni, quic_psodagud, quic_satyap,
	quic_pheragu, quic_rjendra, quic_sibis, quic_saipraka,
	quic_schowdhu
In-Reply-To: <cover.1642768837.git.quic_schowdhu@quicinc.com>

Add the ports property for dwc3 node. This port can be used
by the Embedded USB Debugger for role switching the controller
from device to host mode and vice versa.

Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index d29ffcd..ccb1236 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -332,6 +332,16 @@ properties:
     items:
       enum: [1, 4, 8, 16, 32, 64, 128, 256]
 
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    description:
+      This port is to be attached to the endpoint of the Embedded USB Debugger.
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Connector endpoint of Embedded USB debugger.
+
 unevaluatedProperties: false
 
 required:
-- 
2.7.4


^ permalink raw reply related

* [PATCH V4 1/6] dt-bindings: Add the yaml bindings for EUD
From: Souradeep Chowdhury @ 2022-01-21 13:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson,
	greg, robh
  Cc: linux-kernel, quic_tsoni, quic_psodagud, quic_satyap,
	quic_pheragu, quic_rjendra, quic_sibis, quic_saipraka,
	quic_schowdhu
In-Reply-To: <cover.1642768837.git.quic_schowdhu@quicinc.com>

Documentation for Embedded USB Debugger(EUD) device tree
bindings in yaml format.

Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
 .../devicetree/bindings/soc/qcom/qcom,eud.yaml     | 77 ++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
new file mode 100644
index 0000000..c98aab2
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/qcom/qcom,eud.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm Embedded USB Debugger
+
+maintainers:
+  - Souradeep Chowdhury <quic_schowdhu@quicinc.com>
+
+description:
+  This binding is used to describe the Qualcomm Embedded USB Debugger, which is
+  mini USB-hub implemented on chip to support USB-based debug capabilities.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,sc7280-eud
+      - const: qcom,eud
+
+  reg:
+    items:
+      - description: EUD Base Register Region
+      - description: EUD Mode Manager Register
+
+  interrupts:
+    description: EUD interrupt
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    description:
+      These ports are to be attached to the endpoint of the DWC3 controller node
+      and type C connector node. The controller has the "usb-role-switch"
+      property.
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: This port is to be attached to the DWC3 controller.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: This port is to be attached to the type C connector.
+
+required:
+  - compatible
+  - reg
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    eud@88e0000 {
+           compatible = "qcom,sc7280-eud","qcom,eud";
+           reg = <0x88e0000 0x2000>,
+                 <0x88e2000 0x1000>;
+           ports {
+                   #address-cells = <1>;
+                   #size-cells = <0>;
+                   port@0 {
+                           reg = <0>;
+                           eud_ep: endpoint {
+                                   remote-endpoint = <&usb2_role_switch>;
+                           };
+                   };
+                   port@1 {
+                           reg = <1>;
+                           eud_con: endpoint {
+                                   remote-endpoint = <&con_eud>;
+                           };
+                   };
+           };
+    };
--
2.7.4


^ permalink raw reply related

* [PATCH V4 0/6] Add Embedded USB Debugger (EUD) driver
From: Souradeep Chowdhury @ 2022-01-21 13:53 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson,
	greg, robh
  Cc: linux-kernel, quic_tsoni, quic_psodagud, quic_satyap,
	quic_pheragu, quic_rjendra, quic_sibis, quic_saipraka,
	quic_schowdhu

This is a series of patches that implements a driver for the control
peripheral, EUD (Embedded USB Debugger). The EUD is a mini-USB hub
implemented on chip to support the USB-based debug and trace capabilities.
Apart from debug capabilities, EUD has a control peripheral. Control
Peripheral is on when EUD is on and gets signals like USB attach, pet
EUD etc. EUD driver listens to events like USB attach or detach and then
informs the USB about these events via ROLE-SWITCH. At regular intervals,
the EUD driver receives an interrupt to pet the driver indicating that
the software is functional.

Changes in V4

* Aligned the device tree node structure of EUD as per discussion.

* Changes to usb-connector.yaml is no longer required and is not 
  included in the patch series.
  
* Implemented the rest of the comments on Version 3 of the patch.  

Changes in V3

* Removed the patch for registration of EUD connector as it is no longer
  required.
  
* Added the description to include EUD in usb-connector.yaml  

* Implemented comments on V2 of the patch.

Changes in V2

* Fixed the yaml issue and also implemented comments on yaml in V1.

Changes in V1

* EUD has now been mapped as a separate DT node as it is an independent QCOM IP.

* EUD is attached to the connector child of dwc3 via port end point since EUD
  driver needs the connector for role-switching.

* EUD driver has been moved now to drivers/soc/qcom/qcom_eud.c.

* All the comments from version 0 of the patch has been implemented.

Souradeep Chowdhury (6):
  dt-bindings: Add the yaml bindings for EUD
  bindings: usb: dwc3: Update dwc3 properties for EUD connector
  soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD)
  arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector
  arm64: dts: qcom: sc7280: Set the default dr_mode for usb2
  MAINTAINERS: Add maintainer entry for EUD

 Documentation/ABI/testing/sysfs-driver-eud         |   9 +
 .../devicetree/bindings/soc/qcom/qcom,eud.yaml     |  77 +++++++
 .../devicetree/bindings/usb/snps,dwc3.yaml         |  10 +
 MAINTAINERS                                        |   8 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dts            |   4 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |  39 ++++
 drivers/soc/qcom/Kconfig                           |  10 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/qcom_eud.c                        | 250 +++++++++++++++++++++
 9 files changed, 408 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
 create mode 100644 drivers/soc/qcom/qcom_eud.c

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH v3 3/7] iio: adc: qcom-spmi-rradc: introduce round robin adc
From: Jonathan Cameron @ 2022-01-21 13:45 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Andy Gross,
	Bjorn Andersson, Lee Jones, linux-iio, devicetree, linux-kernel,
	linux-arm-msm, sumit.semwal, amit.pundir, john.stultz
In-Reply-To: <d0d42804-f437-e964-1c0d-4eb65e76db6c@linaro.org>

On Wed, 19 Jan 2022 17:42:51 +0000
Caleb Connolly <caleb.connolly@linaro.org> wrote:

> On 09/01/2022 17:29, Jonathan Cameron wrote:
> > On Thu,  6 Jan 2022 17:31:27 +0000
> > Caleb Connolly <caleb.connolly@linaro.org> wrote:
> >   
> >> The Round Robin ADC is responsible for reading data about the rate of
> >> charge from the USB or DC in jacks, it can also read the battery
> >> ID (resistence) and some temperatures. It is found on the PMI8998 and
> >> PM660 Qualcomm PMICs.
> >>
> >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>  
> > Hi Calib,  
> Hi Jonathan,
> 
> I've spent some time on this and mostly reworked things, thanks a lot for
> your feedback, it's been quite interesting to learn about IIO. :)
> 
> Quite a few of the channels fit well into the (adc_code + offset) * scale format,
> however the one you commented on "rradc_post_process_chg_temp()" doesn't seem to
> fit, it requires multiple steps of applying offsets and scale and I haven't been
> able to re-arrange it to work sensibly.

I had a go below. Not trivial but may still be worth doing.  It seems to be linear
though the maths is a bit nasty!  This depends on that fab_coeff() returning
a constant value for a particular part which I haven't checked.

> 
> I noticed the calibbias properties which seems like something I should expose
> for "rradc_get_fab_coeff()"?
calibbias is usually an internal tweak factor that in hardware corrects for
inaccuracies - something like a DAC tweaking something in the analog front end.

Here it doesn't feel like it's useful to expose a software tweak to userspace
as it would have no idea what to do with it.

> 
> Could you point me in the right direction here? For reference my WIP tree can be
> found here: https://github.com/aospm/linux/commits/upstreaming/spmi-rradc
> 
> I also tried switching to labels, but I found that when I drop the extend_name
> property the driver fails to probe because multiple channels end up with the same
> name in sysfs (e.g. "in_temp_raw"). I've read through the docs and looked at a few
> other drivers but I wasn't able to find out what I'm missing for this to work.

Set channel and indexed for the channel. You should have in_temp0_raw, in_temp1_raw etc
Extend name was never meant to replace the index as it isn't visible in things
like event codes if you ever need to implement that stuff.  So they should all
have and unique index values anyway.

> 
> I've snipped to the relevant bits below.
> 
> Kind regards,
> Caleb
> > 
> > Various things inline but biggest is probably that in IIO we prefer
> > if possible to make application of offsets and scales a job for the caller,
> > either userspace or in kernel callers. This allows them to maintain precision
> > better if they need to further transform the data.
> > 
> > Jonathan
> >   
> >> ---
> >>   drivers/iio/adc/Kconfig           |   13 +
> >>   drivers/iio/adc/Makefile          |    1 +
> >>   drivers/iio/adc/qcom-spmi-rradc.c | 1070 +++++++++++++++++++++++++++++
> >>   3 files changed, 1084 insertions(+)
> >>   create mode 100644 drivers/iio/adc/qcom-spmi-rradc.c
> >>  
> 
> [snip]
> 
> >> +static int rradc_post_process_chg_temp(struct rradc_chip *chip, u16 adc_code,
> >> +				       int *result_millidegc)
> >> +{
> >> +	int64_t uv, offset, slope;
> >> +	int ret;
> >> +
> >> +	ret = rradc_get_fab_coeff(chip, &offset, &slope);
> >> +	if (ret < 0) {
> >> +		dev_err(chip->dev, "Unable to get fab id coefficients\n");
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	uv = ((int64_t)adc_code * RR_ADC_TEMP_FS_VOLTAGE_NUM);
> >> +	uv = div64_s64(uv,
> >> +		       (RR_ADC_TEMP_FS_VOLTAGE_DEN * RR_ADC_CHAN_MAX_VALUE));
> >> +	uv = offset - uv;
> >> +	uv = div64_s64((uv * MILLI), slope);
> >> +	uv += RR_ADC_CHG_TEMP_OFFSET_MILLI_DEGC;
> >> +	*result_millidegc = (int)uv;  
> > 
> > Marginally harder than the one below, but this is still looking like it can
> > be well expressed as an offset + scale.  Thus making the tedious maths
> > userspaces or callers problem.  I'm working backwards hence won't comment on
> > similar before this point. Key is to transform whatever maths you have into
> > 
> > (adc_code + offset) * scale then expose offset and scale as well as the
> > raw value.  The right maths will get done for in kernel users and
> > userspace can do it nicely with floating point.

So I think this is: 

([fab_offset - (adc_code * FS_N )/ (FS_DEN * MAX_VAL)] * MILI / fab_slope + offset

Let A= FS_N/(FS_N * MAX_VAL)
    B = MILLI/fab_slope

= (fab_ofset - adc_code * A)*B + offset
= [((fab_offset / A - adc_code)*A + (offset / B)] * B
= [fab_offset/A - adc_code + offset / AB] * AB
= [adc_code - fab_offset/A - offset/AB]* -AB
= (a + offset) * scale)

where a is the adc_code,
offset = - fab_offset * FS_N * MAX_VAL/ FS_N - offset * FS_N * MAX_VAL * fab_slow / (MILLI  * FS_N)
scale = (FS_N * MILLI) / (FS_N * MAX_VAL * FAB_SLOPE)

So I think it can can done.  Quest then becomes whether sufficient precision can be
maintained for it to make sense to do the fixed point maths in the kernel to establish
those two coefficients and push it out to userspace as a constant that can then be
applied to the channel.

Note this assume that rradc_get_fab_coeff() returns a fixed value for a given device.

> >   
> >> +
> >> +	return 0;
> >> +}  
> 
> [snip]
> 
> >> +static const struct iio_chan_spec rradc_iio_chans[RR_ADC_CHAN_MAX] = {
> >> +	{
> >> +		.extend_name = "batt_id",  
> > 
> > We recently introduced channel labels to try and avoid the need for
> > extend_name.  The problem with extend_name is that generic software then
> > has trouble parsing the resulting sysfs files as they can have very
> > freeform naming.  Moving it to label makes that much easier.  Note that
> > there is code to give a default label of extend_name to work around
> > this problem for older drivers.
> >   
> >> +		.type = IIO_RESISTANCE,
> >> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> >> +		.address = RR_ADC_BATT_ID,
> >> +	},  
> 
> > 
> > Thanks,
> > 
> > Jonathan  
> 


^ permalink raw reply

* Re: [PATCH v14 0/3] mmc: Add LiteSDCard mmc driver
From: Gabriel L. Somlo @ 2022-01-21 13:43 UTC (permalink / raw)
  To: Gabriel Somlo
  Cc: linux-kernel, robh+dt, devicetree, ulf.hansson, linux-mmc,
	kgugala, mholenko, krakoczy, mdudek, paulus, joel, shorne, geert,
	david.abdurachmanov, florent, rdunlap, andy.shevchenko, hdanton
In-Reply-To: <20220113170300.3555651-1-gsomlo@gmail.com>

On Thu, Jan 13, 2022 at 12:02:57PM -0500, Gabriel Somlo wrote:
> Add support for the LiteX SD-Card device, LiteSDCard.
> 
> LiteSDCard is a simple SD-Card interface available as part of the LiteX
> environment, used with various RISC-V and other FPGA based SoCs.
> 
> New in v14:
> Documentation/devicetree/bindings/mmc/litex,mmc.yaml:
>   - add missing `vmmc-supply = ...` property to example section
> 
> >New in v13:
> >Documentation/devicetree/bindings/mmc/litex,mmc.yaml:
> >  - add `vmmc-supply` requirement
> >drivers/mmc/host/Kconfig:
> >  - add dependency on REGULATOR, REGULATOR_FIXED_VOLTAGE
> >drivers/mmc/host/litex_mmc.c:
> >  - use `mmc_regulator_get_supply()`, with fallback to hardcoded 3.3V


Ping?


> >>New in v12:
> >>drivers/mmc/host/Kconfig:
> >>  - add dependency on HAVE_CLK for litex_mmc driver
> >>  - (re)add "If unsure, say N" to the litex_mmc driver help message
> >>drivers/mmc/host/litex_mmc.c:
> >>  - prints message using dev_info() before returning success from probe()
> >>
> >>>New in v11:
> >>>  - picked up r/b from Andy
> >>>drivers/mmc/host/litex_mmc.c:
> >>>  - defensive coding of litex_mmc_interrupt() return logic
> >>>  - remove `dev` member of `struct litex_mmc_host`, only used during probe
> >>>
> >>>>New in v10:
> >>>>drivers/mmc/host/litex_mmc.c:
> >>>>  - group `linux/mmc/*` includes by themselves
> >>>>  - clean-up of `return` style (multiple locations throughout source)
> >>>>  - create `mmc_free_host()` wrapper for use with
> >>>>    `devm_add_action_or_reset()`
> >>>>  - use GFP_KERNEL with `dmam_alloc_coherent()`
> >>>>
> >>>>>New in v9:
> >>>>>drivers/mmc/host/Kconfig:
> >>>>>  - fix OF dependency
> >>>>>drivers/mmc/host/litex_mmc.c:
> >>>>>  - remove `linux/of.h` include, no longer needed since dropping
> >>>>>    `of_match_ptr()`
> >>>>>  - add `linux/mod_devicetable.h` include
> >>>>>  - use devm_action_or_reset() to devm-ify mmc_alloc_host(), and obviate
> >>>>>    the need to call mmc_free_host() explicitly during either probe()
> >>>>>    error path or during remove()
> >>>>>
> >>>>>>New in v8:
> >>>>>>commit blurbs:
> >>>>>>  - cosmetic editing of descriptions
> >>>>>>  - removed `Cc:` lines
> >>>>>>drivers/mmc/host/litex_mmc.c:
> >>>>>>  - fix file header comment (for real, this time)
> >>>>>>  - add explicit `bits.h` include
> >>>>>>  - remove `of_match_ptr()` wrapper from around .of_match_table argument
> >>>>>>  - fix devm ordering issues: use `devm_request_irq()`, which precludes
> >>>>>>    the need to call `free_irq()` on `probe()` error path or from `remove()`
> >>>>>>
> >>>>>>>New in v7:
> >>>>>>>
> >>>>>>>drivers/mmc/host/Kconfig:
> >>>>>>>  - added module name in LiteSDCard Kconfig entry
> >>>>>>>
> >>>>>>>drivers/mmc/host/litex_mmc.c:
> >>>>>>>  - fixed comment formatting, ordering, and capitalization throughout
> >>>>>>>    the entire file
> >>>>>>>  - sorted header #include statements
> >>>>>>>  - removed redundant parantheses in readx_poll_timeout() condition
> >>>>>>>  - explicit handling of readx_poll_timeout() timeout scenarios
> >>>>>>>  - dev_err() used in litex_mmc_sdcard_wait_done()
> >>>>>>>  - use memcpy_fromio() to grab command response
> >>>>>>>  - no need to apply 0xffff mask to a 32-bit value right-shifted by 16
> >>>>>>>    (host->resp[3])
> >>>>>>>  - use clamp() instead of min(max(...)...)
> >>>>>>>  - reworked platform_get_irq_optional() error handling logic
> >>>>>>>  - no need to explicitly zero host->irq, kzalloc() does that already
> >>>>>>>  - added missing free_irq() in litex_mmc_probe() error path
> >>>>>>>  - reordered calls inside litex_mmc_remove() (calling mmc_free_host()
> >>>>>>>    before free_irq()
> >>>>>>>
> >>>>>>>>New in v6:
> >>>>>>>>
> >>>>>>>>drivers/mmc/host/litex_mmc.c:
> >>>>>>>>  - fix handling of deferred probe vs. platform_get_irq_optional()
> >>>>>>>>  - don't #ifdef dma_set_mask_and_coherent(), since it automatically
> >>>>>>>>    does the right thing on both 32- and 64-bit DMA capable arches
> >>>>>>>>  - remove MMC_CAP2_FULL_PWR_CYCLE, add MMC_CAP2_NO_MMC to list of
> >>>>>>>>    hardcoded capabilities during litex_mmc_probe()
> >>>>>>>>  - hardcode mmc->ocr_avail to the full 2.7-3.6V range allowed by the
> >>>>>>>>    SDCard spec (the LiteSDCard device doesn't accept software
> >>>>>>>>    configuration)
> >>>>>>>>
> >>>>>>>>>New in v5:
> >>>>>>>>>
> >>>>>>>>>MAINTAINERS:
> >>>>>>>>>
> >>>>>>>>>  - picked up a/b Mateusz
> >>>>>>>>>
> >>>>>>>>>Doc/dt/bindings/mmc/litex,mmc.yaml:
> >>>>>>>>>
> >>>>>>>>>  - picked up r/b Rob, Joel
> >>>>>>>>>
> >>>>>>>>>drivers/mmc/host/litex_mmc.c:
> >>>>>>>>>
> >>>>>>>>>  - shorten #define constant names (cosmetic, make them less unwieldy)
> >>>>>>>>>  - picked up r/b Joel
> >>>>>>>>>
> >>>>>>>>>>New in v4:
> >>>>>>>>>>
> >>>>>>>>>>Doc/dt/bindings/mmc/litex,mmc.yaml:
> >>>>>>>>>>
> >>>>>>>>>>  - fixed `dt_binding_check` errors uncovered by Rob's script
> >>>>>>>>>>
> >>>>>>>>>>drivers/mmc/host/litex_mmc.c:
> >>>>>>>>>>
> >>>>>>>>>>  - struct litex_mmc_host fields re-ordered so that `pahole` reports
> >>>>>>>>>>    no holes in either 32- or 64-bit builds
> >>>>>>>>>>  - litex_mmc_set_bus_width() now encapsulates check for
> >>>>>>>>>>    host->is_bus_width_set
> >>>>>>>>>>  - litex_mmc_request() - factor out dma data setup into separate
> >>>>>>>>>>    helper function: litex_mmc_do_dma()
> >>>>>>>>>>
> >>>>>>>>>>>New in v3:
> >>>>>>>>>>>
> >>>>>>>>>>>  MAINTAINERS:
> >>>>>>>>>>>
> >>>>>>>>>>>  - picked up acked-by Joel
> >>>>>>>>>>>  - added listing for liteeth driver
> >>>>>>>>>>>  - added Joel as additional co-maintainer (thanks!)
> >>>>>>>>>>>
> >>>>>>>>>>>  Doc/dt/bindings/mmc/litex,mmc.yaml:
> >>>>>>>>>>>
> >>>>>>>>>>>  - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
> >>>>>>>>>>>    bindings document (please let me know if that was premature, and
> >>>>>>>>>>>    happy to take further review if needed :)
> >>>>>>>>>>>  - add dedicated DT property for source clock frequency
> >>>>>>>>>>>
> >>>>>>>>>>>  drivers/mmc/host/litex_mmc.c:
> >>>>>>>>>>>
> >>>>>>>>>>>  - fixed function signature (no line split), and naming (litex_mmc_*)
> >>>>>>>>>>>  - more informative MODULE_AUTHOR() entries
> >>>>>>>>>>>    - also added matching "Copyright" entries in file header
> >>>>>>>>>>>  - fixed description in Kconfig
> >>>>>>>>>>>  - fixed DT documentation
> >>>>>>>>>>>  - removed magic constants
> >>>>>>>>>>>  - removed litex_map_status(), have sdcard_wait_done() return *real*
> >>>>>>>>>>>    error codes directly instead.
> >>>>>>>>>>>  - streamlined litex_mmc_reponse_len()
> >>>>>>>>>>>  - call litex_mmc_set_bus_width() only once, and ensure it returns
> >>>>>>>>>>>    correct error code(s)
> >>>>>>>>>>>  - use readx_poll_timeout() -- more concise -- instead of
> >>>>>>>>>>>    read_poll_timeout()
> >>>>>>>>>>>  - use dev_err() in litex_mmc_send_cmd() (instead of pr_err())
> >>>>>>>>>>>  - litex_mmc_setclk() will update host->clock before returning
> >>>>>>>>>>>  - separate irq initialization into its own function,
> >>>>>>>>>>>    litex_mmc_irq_init()
> >>>>>>>>>>>  - document rationale for f_min, f_max
> >>>>>>>>>>>  - use dmam_alloc_coherent(), which simplifies cleanup significantly
> >>>>>>>>>>>  - large `if (data) { ... }` block in litex_mmc_request() left as-is,
> >>>>>>>>>>>    there are too many variables shared with the rest of the parent
> >>>>>>>>>>>    function body to easily separate (e.g., `len`, `transfer`, `direct`).
> >>>>>>>>>>>    If this is indeed a blocker, I can take another shot at refactoring
> >>>>>>>>>>>    it in a future revision!
> >>>>>>>>>>>  - bump dma_set_mask_and_coherent() to 64-bits on suitable
> >>>>>>>>>>>    architectures
> >>>>>>>>>>>  - clock source picked up from dedicated DT clock reference property
> >>>>>>>>>>>  - remove gpio card-detect logic (needs testing and a dt binding
> >>>>>>>>>>>    example before being eligible for upstream inclusion)
> >>>>>>>>>>>
> >>>>>>>>>>>> New in v2:
> >>>>>>>>>>>>   - reword info message in litex_set_clk()
> >>>>>>>>>>>>   - streamline code in litex_map_status()
> >>>>>>>>>>>>   - fix typos in Kconfig (thanks Randy Dunlap <rdunlap@infradead.org>)
> >>>>>>>>>>>>   - improvements suggested by Stafford Horne <shorne@gmail.com>
> >>>>>>>>>>>>     - allow COMPILE_TEST in Kconfig
> >>>>>>>>>>>>     - use read_poll_timeout() when waiting for cmd/data/DMA
> >>>>>>>>>>>>       xfer completion
> >>>>>>>>>>>>   - include interrupt.h (thanks kernel test robot <lkp@intel.com>)
> 
> Gabriel Somlo (3):
>   MAINTAINERS: co-maintain LiteX platform
>   dt-bindings: mmc: Add bindings for LiteSDCard
>   mmc: Add driver for LiteX's LiteSDCard interface
> 
>  .../devicetree/bindings/mmc/litex,mmc.yaml    |  78 +++
>  MAINTAINERS                                   |   9 +-
>  drivers/mmc/host/Kconfig                      |  13 +
>  drivers/mmc/host/Makefile                     |   1 +
>  drivers/mmc/host/litex_mmc.c                  | 661 ++++++++++++++++++
>  5 files changed, 760 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml
>  create mode 100644 drivers/mmc/host/litex_mmc.c
> 
> -- 
> 2.31.1
> 

^ permalink raw reply

* [PATCH v4 11/11] ARM: multi_v7_defconfig: add atmel video pipeline modules
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

Add drivers for the atmel video capture pipeline: atmel isc, xisc and
microchip csi2dc.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 arch/arm/configs/multi_v7_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index c951aeed2138..92b7749a6df8 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -638,7 +638,10 @@ CONFIG_VIDEO_S5P_MIPI_CSIS=m
 CONFIG_VIDEO_EXYNOS_FIMC_LITE=m
 CONFIG_VIDEO_EXYNOS4_FIMC_IS=m
 CONFIG_VIDEO_RCAR_VIN=m
+CONFIG_VIDEO_ATMEL_ISC=m
+CONFIG_VIDEO_ATMEL_XISC=m
 CONFIG_VIDEO_ATMEL_ISI=m
+CONFIG_VIDEO_MICROCHIP_CSI2DC=m
 CONFIG_V4L_MEM2MEM_DRIVERS=y
 CONFIG_VIDEO_SAMSUNG_S5P_JPEG=m
 CONFIG_VIDEO_SAMSUNG_S5P_MFC=m
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 10/11] ARM: configs: at91: sama7: add xisc and csi2dc
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

Enable XISC and CSI2DC drivers.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 arch/arm/configs/sama7_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/sama7_defconfig b/arch/arm/configs/sama7_defconfig
index 938aae4bd80b..15978f2ab4ea 100644
--- a/arch/arm/configs/sama7_defconfig
+++ b/arch/arm/configs/sama7_defconfig
@@ -126,6 +126,8 @@ CONFIG_MEDIA_SUPPORT_FILTER=y
 CONFIG_MEDIA_CAMERA_SUPPORT=y
 CONFIG_MEDIA_PLATFORM_SUPPORT=y
 CONFIG_V4L_PLATFORM_DRIVERS=y
+CONFIG_VIDEO_ATMEL_XISC=y
+CONFIG_VIDEO_MICROCHIP_CSI2DC=y
 CONFIG_VIDEO_IMX219=m
 CONFIG_VIDEO_IMX274=m
 CONFIG_VIDEO_OV5647=m
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 09/11] ARM: dts: at91: sama7g5: add nodes for video capture
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

Add node for the XISC (eXtended Image Sensor Controller) and CSI2DC
(csi2 demux controller).
These nodes represent the top level of the video capture hardware pipeline
and are directly connected in hardware.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
Changes in v4:
- add mandatory property bus type

Changes in v3:
- change bus width for endpoints to the default 14

 arch/arm/boot/dts/sama7g5.dtsi | 49 ++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm/boot/dts/sama7g5.dtsi b/arch/arm/boot/dts/sama7g5.dtsi
index 7039311bf678..d47d62f01895 100644
--- a/arch/arm/boot/dts/sama7g5.dtsi
+++ b/arch/arm/boot/dts/sama7g5.dtsi
@@ -236,6 +236,55 @@ sdmmc2: mmc@e120c000 {
 			status = "disabled";
 		};
 
+		csi2dc: csi2dc@e1404000 {
+			compatible = "microchip,sama7g5-csi2dc";
+			reg = <0xe1404000 0x500>;
+			clocks = <&pmc PMC_TYPE_PERIPHERAL 34>, <&xisc>;
+			clock-names = "pclk", "scck";
+			assigned-clocks = <&xisc>;
+			assigned-clock-rates = <266000000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					csi2dc_in: endpoint {
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					csi2dc_out: endpoint {
+						bus-width = <14>;
+						hsync-active = <1>;
+						vsync-active = <1>;
+						remote-endpoint = <&xisc_in>;
+					};
+				};
+			};
+		};
+
+		xisc: xisc@e1408000 {
+			compatible = "microchip,sama7g5-isc";
+			reg = <0xe1408000 0x2000>;
+			interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&pmc PMC_TYPE_PERIPHERAL 56>;
+			clock-names = "hclock";
+			#clock-cells = <0>;
+			clock-output-names = "isc-mck";
+
+			port {
+				xisc_in: endpoint {
+					bus-type = <5>; /* Parallel */
+					bus-width = <14>;
+					hsync-active = <1>;
+					vsync-active = <1>;
+					remote-endpoint = <&csi2dc_out>;
+				};
+			};
+		};
+
 		pwm: pwm@e1604000 {
 			compatible = "microchip,sama7g5-pwm", "atmel,sama5d2-pwm";
 			reg = <0xe1604000 0x4000>;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 08/11] dt-bindings: media: microchip,xisc: add bus-width of 14
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

The Microchip XISC supports a bus width of 14 bits.
Add it to the supported bus widths.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 Documentation/devicetree/bindings/media/microchip,xisc.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/microchip,xisc.yaml b/Documentation/devicetree/bindings/media/microchip,xisc.yaml
index 086e1430af4f..3be8f64c3e21 100644
--- a/Documentation/devicetree/bindings/media/microchip,xisc.yaml
+++ b/Documentation/devicetree/bindings/media/microchip,xisc.yaml
@@ -67,7 +67,7 @@ properties:
           remote-endpoint: true
 
           bus-width:
-            enum: [8, 9, 10, 11, 12]
+            enum: [8, 9, 10, 11, 12, 14]
             default: 12
 
           hsync-active:
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 07/11] media: atmel: atmel-isc: change format propagation to subdev into only verification
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

As a top MC video driver, the atmel-isc should not propagate the format to the
subdevice.
It should rather check at start_streaming() time if the subdev is properly
configured with a compatible format.
Removed the whole format finding logic, and reworked the format verification
at start_streaming time, such that the ISC will return an error if the subdevice
is not properly configured. To achieve this, media_pipeline_start
is called and a link_validate callback is created to check the formats.
With this being done, the module parameter 'sensor_preferred' makes no sense
anymore. The ISC should not decide which format the sensor is using. The
ISC should only cope with the situation and inform userspace if the streaming
is possible in the current configuration.
The redesign of the format propagation has also risen the question of the
enumfmt callback. If enumfmt is called with an mbus_code, the enumfmt handler
should only return the formats that are supported for this mbus_code.
To make it more easy to understand the formats, changed the report order
to report first the native formats, and after that the formats that the ISC
can convert to.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
Changes in v4:
- moved validation code into link_validate and used media_pipeline_start
- merged this patch with the enum_fmt patch which was previously in v3 of
the series

Changes in v3:
- clamp to maximum resolution once the frame size from the subdev is found

 drivers/media/platform/atmel/atmel-isc-base.c | 335 ++++++++++--------
 .../media/platform/atmel/atmel-isc-scaler.c   |   5 +
 drivers/media/platform/atmel/atmel-isc.h      |   3 +
 3 files changed, 191 insertions(+), 152 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index f7a88399bd54..31c79313aadc 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -36,11 +36,6 @@ static unsigned int debug;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "debug level (0-2)");
 
-static unsigned int sensor_preferred = 1;
-module_param(sensor_preferred, uint, 0644);
-MODULE_PARM_DESC(sensor_preferred,
-		 "Sensor is preferred to output the specified format (1-on 0-off), default 1");
-
 #define ISC_IS_FORMAT_RAW(mbus_code) \
 	(((mbus_code) & 0xf000) == 0x3000)
 
@@ -337,6 +332,10 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
 	unsigned long flags;
 	int ret;
 
+	ret = media_pipeline_start(&isc->video_dev.entity, &isc->mpipe);
+	if (ret)
+		goto err_pipeline_start;
+
 	/* Enable stream on the sub device */
 	ret = v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 1);
 	if (ret && ret != -ENOIOCTLCMD) {
@@ -384,6 +383,9 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
 	v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 0);
 
 err_start_stream:
+	media_pipeline_stop(&isc->video_dev.entity);
+
+err_pipeline_start:
 	spin_lock_irqsave(&isc->dma_queue_lock, flags);
 	list_for_each_entry(buf, &isc->dma_queue, list)
 		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
@@ -420,6 +422,9 @@ static void isc_stop_streaming(struct vb2_queue *vq)
 	if (ret && ret != -ENOIOCTLCMD)
 		v4l2_err(&isc->v4l2_dev, "stream off failed in subdev\n");
 
+	/* Stop media pipeline */
+	media_pipeline_stop(&isc->video_dev.entity);
+
 	/* Release all active buffers */
 	spin_lock_irqsave(&isc->dma_queue_lock, flags);
 	if (unlikely(isc->cur_frm)) {
@@ -496,21 +501,56 @@ static int isc_enum_fmt_vid_cap(struct file *file, void *priv,
 	u32 index = f->index;
 	u32 i, supported_index;
 
-	if (index < isc->controller_formats_size) {
-		f->pixelformat = isc->controller_formats[index].fourcc;
-		return 0;
+	supported_index = 0;
+
+	for (i = 0; i < isc->formats_list_size; i++) {
+		if (!isc->formats_list[i].sd_support)
+			continue;
+		/*
+		 * If specific mbus_code is requested, provide only
+		 * supported formats with this mbus code
+		 */
+		if (f->mbus_code && f->mbus_code !=
+		    isc->formats_list[i].mbus_code)
+			continue;
+		if (supported_index == index) {
+			f->pixelformat = isc->formats_list[i].fourcc;
+			return 0;
+		}
+		supported_index++;
 	}
 
-	index -= isc->controller_formats_size;
+	/*
+	 * If the sensor does not support this mbus_code whatsoever,
+	 * there is no reason to advertise any of our output formats
+	 */
+	if (supported_index == 0)
+		return -EINVAL;
 
+	/*
+	 * If the sensor uses a format that is not raw, then we cannot
+	 * convert it to any of the formats that we usually can with a
+	 * RAW sensor. Thus, do not advertise them.
+	 */
+	if (isc->config.sd_format &&
+	    !ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code))
+		return -EINVAL;
+
+	/*
+	 * Iterate again through the formats that we can convert to.
+	 * However, to avoid duplicates, skip the formats that
+	 * the sensor already supports directly
+	 */
+	index -= supported_index;
 	supported_index = 0;
 
-	for (i = 0; i < isc->formats_list_size; i++) {
-		if (!ISC_IS_FORMAT_RAW(isc->formats_list[i].mbus_code) ||
-		    !isc->formats_list[i].sd_support)
+	for (i = 0; i < isc->controller_formats_size; i++) {
+		/* if this format is already supported by sensor, skip it */
+		if (find_format_by_fourcc(isc, isc->controller_formats[i].fourcc))
 			continue;
 		if (supported_index == index) {
-			f->pixelformat = isc->formats_list[i].fourcc;
+			f->pixelformat =
+				isc->controller_formats[i].fourcc;
 			return 0;
 		}
 		supported_index++;
@@ -581,20 +621,30 @@ static int isc_try_validate_formats(struct isc_device *isc)
 		break;
 	default:
 	/* any other different formats are not supported */
+		v4l2_err(&isc->v4l2_dev, "Requested unsupported format.\n");
 		ret = -EINVAL;
 	}
 	v4l2_dbg(1, debug, &isc->v4l2_dev,
 		 "Format validation, requested rgb=%u, yuv=%u, grey=%u, bayer=%u\n",
 		 rgb, yuv, grey, bayer);
 
-	/* we cannot output RAW if we do not receive RAW */
-	if ((bayer) && !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code))
+	if ((bayer) &&
+	    !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) {
+		v4l2_err(&isc->v4l2_dev, "Cannot output RAW if we do not receive RAW.\n");
 		return -EINVAL;
+	}
 
-	/* we cannot output GREY if we do not receive RAW/GREY */
 	if (grey && !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code) &&
-	    !ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code))
+	    !ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code)) {
+		v4l2_err(&isc->v4l2_dev, "Cannot output GREY if we do not receive RAW/GREY.\n");
+		return -EINVAL;
+	}
+
+	if ((rgb || bayer || yuv) &&
+	    ISC_IS_FORMAT_GREY(isc->try_config.sd_format->mbus_code)) {
+		v4l2_err(&isc->v4l2_dev, "Cannot convert GREY to another format.\n");
 		return -EINVAL;
+	}
 
 	return ret;
 }
@@ -822,7 +872,7 @@ static void isc_try_fse(struct isc_device *isc,
 	 * If we do not know yet which format the subdev is using, we cannot
 	 * do anything.
 	 */
-	if (!isc->try_config.sd_format)
+	if (!isc->config.sd_format)
 		return;
 
 	fse.code = isc->try_config.sd_format->mbus_code;
@@ -843,180 +893,141 @@ static void isc_try_fse(struct isc_device *isc,
 	}
 }
 
-static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
-			u32 *code)
+static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f)
 {
-	int i;
-	struct isc_format *sd_fmt = NULL, *direct_fmt = NULL;
 	struct v4l2_pix_format *pixfmt = &f->fmt.pix;
-	struct v4l2_subdev_pad_config pad_cfg = {};
-	struct v4l2_subdev_state pad_state = {
-		.pads = &pad_cfg
-		};
-	struct v4l2_subdev_format format = {
-		.which = V4L2_SUBDEV_FORMAT_TRY,
-	};
-	u32 mbus_code;
-	int ret;
-	bool rlp_dma_direct_dump = false;
+	unsigned int i;
 
 	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	/* Step 1: find a RAW format that is supported */
-	for (i = 0; i < isc->num_user_formats; i++) {
-		if (ISC_IS_FORMAT_RAW(isc->user_formats[i]->mbus_code)) {
-			sd_fmt = isc->user_formats[i];
+	isc->try_config.fourcc = isc->user_formats[0]->fourcc;
+
+	/* find if the format requested is supported */
+	for (i = 0; i < isc->controller_formats_size; i++)
+		if (isc->controller_formats[i].fourcc == pixfmt->pixelformat) {
+			isc->try_config.fourcc = pixfmt->pixelformat;
 			break;
 		}
-	}
-	/* Step 2: We can continue with this RAW format, or we can look
-	 * for better: maybe sensor supports directly what we need.
-	 */
-	direct_fmt = find_format_by_fourcc(isc, pixfmt->pixelformat);
-
-	/* Step 3: We have both. We decide given the module parameter which
-	 * one to use.
-	 */
-	if (direct_fmt && sd_fmt && sensor_preferred)
-		sd_fmt = direct_fmt;
-
-	/* Step 4: we do not have RAW but we have a direct format. Use it. */
-	if (direct_fmt && !sd_fmt)
-		sd_fmt = direct_fmt;
-
-	/* Step 5: if we are using a direct format, we need to package
-	 * everything as 8 bit data and just dump it
-	 */
-	if (sd_fmt == direct_fmt)
-		rlp_dma_direct_dump = true;
-
-	/* Step 6: We have no format. This can happen if the userspace
-	 * requests some weird/invalid format.
-	 * In this case, default to whatever we have
-	 */
-	if (!sd_fmt && !direct_fmt) {
-		sd_fmt = isc->user_formats[isc->num_user_formats - 1];
-		v4l2_dbg(1, debug, &isc->v4l2_dev,
-			 "Sensor not supporting %.4s, using %.4s\n",
-			 (char *)&pixfmt->pixelformat, (char *)&sd_fmt->fourcc);
-	}
-
-	if (!sd_fmt) {
-		ret = -EINVAL;
-		goto isc_try_fmt_err;
-	}
-
-	/* Step 7: Print out what we decided for debugging */
-	v4l2_dbg(1, debug, &isc->v4l2_dev,
-		 "Preferring to have sensor using format %.4s\n",
-		 (char *)&sd_fmt->fourcc);
-
-	/* Step 8: at this moment we decided which format the subdev will use */
-	isc->try_config.sd_format = sd_fmt;
-
-	/* Limit to Atmel ISC hardware capabilities */
-	if (pixfmt->width > isc->max_width)
-		pixfmt->width = isc->max_width;
-	if (pixfmt->height > isc->max_height)
-		pixfmt->height = isc->max_height;
-
-	/*
-	 * The mbus format is the one the subdev outputs.
-	 * The pixels will be transferred in this format Sensor -> ISC
-	 */
-	mbus_code = sd_fmt->mbus_code;
-
-	/*
-	 * Validate formats. If the required format is not OK, default to raw.
-	 */
-
-	isc->try_config.fourcc = pixfmt->pixelformat;
-
-	if (isc_try_validate_formats(isc)) {
-		pixfmt->pixelformat = isc->try_config.fourcc = sd_fmt->fourcc;
-		/* Re-try to validate the new format */
-		ret = isc_try_validate_formats(isc);
-		if (ret)
-			goto isc_try_fmt_err;
-	}
-
-	ret = isc_try_configure_rlp_dma(isc, rlp_dma_direct_dump);
-	if (ret)
-		goto isc_try_fmt_err;
-
-	ret = isc_try_configure_pipeline(isc);
-	if (ret)
-		goto isc_try_fmt_err;
 
-	/* Obtain frame sizes if possible to have crop requirements ready */
-	isc_try_fse(isc, &pad_state);
-
-	v4l2_fill_mbus_format(&format.format, pixfmt, mbus_code);
-	ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt,
-			       &pad_state, &format);
-	if (ret < 0)
-		goto isc_try_fmt_subdev_err;
+	/* If we did not find the requested format, we will fallback here */
+	pixfmt->pixelformat = isc->try_config.fourcc;
+	pixfmt->colorspace = V4L2_COLORSPACE_SRGB;
+	pixfmt->field = V4L2_FIELD_NONE;
 
-	v4l2_fill_pix_format(pixfmt, &format.format);
+	isc_try_configure_rlp_dma(isc, false);
 
 	/* Limit to Atmel ISC hardware capabilities */
-	if (pixfmt->width > isc->max_width)
-		pixfmt->width = isc->max_width;
-	if (pixfmt->height > isc->max_height)
-		pixfmt->height = isc->max_height;
+	v4l_bound_align_image(&pixfmt->width, 16, isc->max_width, 0,
+			      &pixfmt->height, 16, isc->max_height, 0, 0);
 
 	pixfmt->field = V4L2_FIELD_NONE;
 	pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp_v4l2) >> 3;
 	pixfmt->sizeimage = ((pixfmt->width * isc->try_config.bpp) >> 3) *
 			     pixfmt->height;
 
-	if (code)
-		*code = mbus_code;
+	isc->try_fmt = *f;
 
 	return 0;
+}
 
-isc_try_fmt_err:
-	v4l2_err(&isc->v4l2_dev, "Could not find any possible format for a working pipeline\n");
-isc_try_fmt_subdev_err:
-	memset(&isc->try_config, 0, sizeof(isc->try_config));
+static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
+{
+	isc_try_fmt(isc, f);
 
-	return ret;
+	/* make the try configuration active */
+	isc->config = isc->try_config;
+	isc->fmt = isc->try_fmt;
+
+	v4l2_dbg(1, debug, &isc->v4l2_dev, "ISC set_fmt to %.4s @%dx%d\n",
+		 (char *)&f->fmt.pix.pixelformat,
+		 f->fmt.pix.width, f->fmt.pix.height);
+
+	return 0;
 }
 
-static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
+static int isc_validate(struct isc_device *isc)
 {
+	int ret;
+	int i;
+	struct isc_format *sd_fmt = NULL;
+	struct v4l2_pix_format *pixfmt = &isc->fmt.fmt.pix;
 	struct v4l2_subdev_format format = {
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+		.pad = isc->remote_pad,
+	};
+	struct v4l2_subdev_pad_config pad_cfg = {};
+	struct v4l2_subdev_state pad_state = {
+		.pads = &pad_cfg,
 	};
-	u32 mbus_code = 0;
-	int ret;
 
-	ret = isc_try_fmt(isc, f, &mbus_code);
+	/* Get current format from subdev */
+	ret = v4l2_subdev_call(isc->current_subdev->sd, pad, get_fmt, NULL,
+			       &format);
 	if (ret)
 		return ret;
 
-	v4l2_fill_mbus_format(&format.format, &f->fmt.pix, mbus_code);
-	ret = v4l2_subdev_call(isc->current_subdev->sd, pad,
-			       set_fmt, NULL, &format);
-	if (ret < 0)
-		return ret;
+	/* Identify the subdev's format configuration */
+	for (i = 0; i < isc->num_user_formats; i++)
+		if (isc->user_formats[i]->mbus_code == format.format.code) {
+			sd_fmt = isc->user_formats[i];
+			break;
+		}
+
+	/* Check if the format is not supported */
+	if (!sd_fmt) {
+		v4l2_err(&isc->v4l2_dev,
+			 "Current subdevice is streaming a media bus code that is not supported 0x%x\n",
+			 format.format.code);
+		return -EPIPE;
+	}
+
+	/* At this moment we know which format the subdev will use */
+	isc->try_config.sd_format = sd_fmt;
+
+	/* If the sensor is not RAW, we can only do a direct dump */
+	if (!ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code))
+		isc_try_configure_rlp_dma(isc, true);
 
 	/* Limit to Atmel ISC hardware capabilities */
-	if (f->fmt.pix.width > isc->max_width)
-		f->fmt.pix.width = isc->max_width;
-	if (f->fmt.pix.height > isc->max_height)
-		f->fmt.pix.height = isc->max_height;
+	v4l_bound_align_image(&format.format.width, 16, isc->max_width, 0,
+			      &format.format.height, 16, isc->max_height, 0, 0);
+
+	/* Check if the frame size is the same. Otherwise we may overflow */
+	if (pixfmt->height != format.format.height ||
+	    pixfmt->width != format.format.width) {
+		v4l2_err(&isc->v4l2_dev,
+			 "ISC not configured with the proper frame size: %dx%d\n",
+			 format.format.width, format.format.height);
+		return -EPIPE;
+	}
 
-	isc->fmt = *f;
+	v4l2_dbg(1, debug, &isc->v4l2_dev,
+		 "Identified subdev using format %.4s with %dx%d %d bpp\n",
+		 (char *)&sd_fmt->fourcc, pixfmt->width, pixfmt->height,
+		 isc->try_config.bpp);
 
+	/* Reset and restart AWB if the subdevice changed the format */
 	if (isc->try_config.sd_format && isc->config.sd_format &&
 	    isc->try_config.sd_format != isc->config.sd_format) {
 		isc->ctrls.hist_stat = HIST_INIT;
 		isc_reset_awb_ctrls(isc);
 		isc_update_v4l2_ctrls(isc);
 	}
-	/* make the try configuration active */
+
+	/* Validate formats */
+	ret = isc_try_validate_formats(isc);
+	if (ret)
+		return ret;
+
+	/* Obtain frame sizes if possible to have crop requirements ready */
+	isc_try_fse(isc, &pad_state);
+
+	/* Configure ISC pipeline for the config */
+	ret = isc_try_configure_pipeline(isc);
+	if (ret)
+		return ret;
+
 	isc->config = isc->try_config;
 
 	v4l2_dbg(1, debug, &isc->v4l2_dev, "New ISC configuration in place\n");
@@ -1040,7 +1051,7 @@ static int isc_try_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct isc_device *isc = video_drvdata(file);
 
-	return isc_try_fmt(isc, f, NULL);
+	return isc_try_fmt(isc, f);
 }
 
 static int isc_enum_input(struct file *file, void *priv,
@@ -1841,7 +1852,7 @@ static int isc_set_default_fmt(struct isc_device *isc)
 	};
 	int ret;
 
-	ret = isc_try_fmt(isc, &f, NULL);
+	ret = isc_try_fmt(isc, &f);
 	if (ret)
 		return ret;
 
@@ -2015,6 +2026,24 @@ int isc_pipeline_init(struct isc_device *isc)
 }
 EXPORT_SYMBOL_GPL(isc_pipeline_init);
 
+int isc_link_validate(struct media_link *link)
+{
+	struct video_device *vdev =
+		media_entity_to_video_device(link->sink->entity);
+	struct isc_device *isc = video_get_drvdata(vdev);
+	int ret;
+
+	ret = v4l2_subdev_link_validate(link);
+	if (ret)
+		return ret;
+
+	return isc_validate(isc);
+}
+
+static const struct media_entity_operations isc_entity_operations = {
+	.link_validate = isc_link_validate,
+};
+
 int isc_mc_init(struct isc_device *isc, u32 ver)
 {
 	const struct of_device_id *match;
@@ -2022,6 +2051,8 @@ int isc_mc_init(struct isc_device *isc, u32 ver)
 
 	isc->video_dev.entity.function = MEDIA_ENT_F_IO_V4L;
 	isc->video_dev.entity.flags = MEDIA_ENT_FL_DEFAULT;
+	isc->video_dev.entity.ops = &isc_entity_operations;
+
 	isc->pads[ISC_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
 
 	ret = media_entity_pads_init(&isc->video_dev.entity, ISC_PADS_NUM,
diff --git a/drivers/media/platform/atmel/atmel-isc-scaler.c b/drivers/media/platform/atmel/atmel-isc-scaler.c
index ec95c9665883..93375a61aea6 100644
--- a/drivers/media/platform/atmel/atmel-isc-scaler.c
+++ b/drivers/media/platform/atmel/atmel-isc-scaler.c
@@ -155,6 +155,10 @@ static const struct v4l2_subdev_pad_ops isc_scaler_pad_ops = {
 	.init_cfg = isc_scaler_init_cfg,
 };
 
+static const struct media_entity_operations isc_scaler_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
 static const struct v4l2_subdev_ops xisc_scaler_subdev_ops = {
 	.pad = &isc_scaler_pad_ops,
 };
@@ -172,6 +176,7 @@ int isc_scaler_init(struct isc_device *isc)
 
 	isc->scaler_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	isc->scaler_sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
+	isc->scaler_sd.entity.ops = &isc_scaler_entity_ops;
 	isc->scaler_pads[ISC_SCALER_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
 	isc->scaler_pads[ISC_SCALER_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
 
diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
index 04ea5e340808..e36cf9d3942a 100644
--- a/drivers/media/platform/atmel/atmel-isc.h
+++ b/drivers/media/platform/atmel/atmel-isc.h
@@ -270,6 +270,7 @@ enum isc_scaler_pads {
  * @formats_list_size:	size of formats_list array
  * @pads:		media controller pads for isc video entity
  * @mdev:		media device that is registered by the isc
+ * @mpipe:		media device pipeline used by the isc
  * @remote_pad:		remote pad on the connected subdevice
  * @scaler_sd:		subdevice for the scaler that isc registers
  * @scaler_pads:	media controller pads for the scaler subdevice
@@ -295,6 +296,7 @@ struct isc_device {
 	struct completion	comp;
 
 	struct v4l2_format	fmt;
+	struct v4l2_format	try_fmt;
 	struct isc_format	**user_formats;
 	unsigned int		num_user_formats;
 
@@ -366,6 +368,7 @@ struct isc_device {
 	struct {
 		struct media_pad		pads[ISC_PADS_NUM];
 		struct media_device		mdev;
+		struct media_pipeline		mpipe;
 
 		u32				remote_pad;
 	};
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 06/11] media: atmel: atmel-isc: compact the controller formats list
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

Compact the list array to be more readable.
No other changes, only cosmetic.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../media/platform/atmel/atmel-sama5d2-isc.c  | 51 ++++++----------
 .../media/platform/atmel/atmel-sama7g5-isc.c  | 60 +++++++------------
 2 files changed, 37 insertions(+), 74 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
index 025c3e8a7e95..d96ee3373889 100644
--- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
@@ -60,56 +60,39 @@
 static const struct isc_format sama5d2_controller_formats[] = {
 	{
 		.fourcc		= V4L2_PIX_FMT_ARGB444,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_ARGB555,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_RGB565,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_ABGR32,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_XBGR32,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_YUV420,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_YUYV,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_YUV422P,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_GREY,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_Y10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SBGGR8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGBRG8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGRBG8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SRGGB8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SBGGR10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGBRG10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGRBG10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SRGGB10,
 	},
 };
diff --git a/drivers/media/platform/atmel/atmel-sama7g5-isc.c b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
index 9dc75eed0098..e07ae188c15f 100644
--- a/drivers/media/platform/atmel/atmel-sama7g5-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
@@ -63,65 +63,45 @@
 static const struct isc_format sama7g5_controller_formats[] = {
 	{
 		.fourcc		= V4L2_PIX_FMT_ARGB444,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_ARGB555,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_RGB565,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_ABGR32,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_XBGR32,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_YUV420,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_UYVY,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_VYUY,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_YUYV,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_YUV422P,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_GREY,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_Y10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_Y16,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SBGGR8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGBRG8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGRBG8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SRGGB8,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SBGGR10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGBRG10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SGRBG10,
-	},
-	{
+	}, {
 		.fourcc		= V4L2_PIX_FMT_SRGGB10,
 	},
 };
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 05/11] media: atmel: atmel-isc-base: use mutex to lock awb workqueue from streaming
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

The AWB workqueue runs in a kernel thread and needs to be synchronized
w.r.t. the streaming status.
It is possible that streaming is stopped while the AWB workq is running.
In this case it is likely that the check for vb2_start_streaming_called is done
at one point in time, but the AWB computations are done later, including a call
to isc_update_profile, which requires streaming to be started.
Thus , isc_update_profile will fail if during this operation sequence the
streaming was stopped.
To solve this issue, a mutex is added, that will serialize the awb work and
streaming stopping, with the mention that either streaming is stopped
completely including termination of the last frame is done, and after that
the AWB work can check stream status and stop; either first AWB work is
completed and after that the streaming can stop correctly.
The awb spin lock cannot be used since this spinlock is taken in the same
context and using it in the stop streaming will result in a recursion BUG.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/media/platform/atmel/atmel-isc-base.c | 31 ++++++++++++++++---
 drivers/media/platform/atmel/atmel-isc.h      |  1 +
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index 6b482270eb93..f7a88399bd54 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -400,6 +400,7 @@ static void isc_stop_streaming(struct vb2_queue *vq)
 	struct isc_buffer *buf;
 	int ret;
 
+	mutex_lock(&isc->awb_mutex);
 	v4l2_ctrl_activate(isc->do_wb_ctrl, false);
 
 	/* Wait until the end of the current frame */
@@ -407,6 +408,8 @@ static void isc_stop_streaming(struct vb2_queue *vq)
 		v4l2_err(&isc->v4l2_dev,
 			 "Timeout waiting for end of the capture\n");
 
+	mutex_unlock(&isc->awb_mutex);
+
 	/* Disable DMA interrupt */
 	regmap_write(isc->regmap, ISC_INTDIS, ISC_INT_DDONE);
 
@@ -1395,10 +1398,6 @@ static void isc_awb_work(struct work_struct *w)
 	u32 min, max;
 	int ret;
 
-	/* streaming is not active anymore */
-	if (!vb2_start_streaming_called(&isc->vb2_vidq))
-		return;
-
 	if (ctrls->hist_stat != HIST_ENABLED)
 		return;
 
@@ -1453,7 +1452,24 @@ static void isc_awb_work(struct work_struct *w)
 	}
 	regmap_write(regmap, ISC_HIS_CFG + isc->offsets.his,
 		     hist_id | baysel | ISC_HIS_CFG_RAR);
+
+	/*
+	 * We have to make sure the streaming has not stopped meanwhile.
+	 * ISC requires a frame to clock the internal profile update.
+	 * To avoid issues, lock the sequence with a mutex
+	 */
+	mutex_lock(&isc->awb_mutex);
+
+	/* streaming is not active anymore */
+	if (!vb2_start_streaming_called(&isc->vb2_vidq)) {
+		mutex_unlock(&isc->awb_mutex);
+		return;
+	};
+
 	isc_update_profile(isc);
+
+	mutex_unlock(&isc->awb_mutex);
+
 	/* if awb has been disabled, we don't need to start another histogram */
 	if (ctrls->awb)
 		regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
@@ -1532,6 +1548,8 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
 
 		isc_update_awb_ctrls(isc);
 
+		mutex_lock(&isc->awb_mutex);
+
 		if (vb2_start_streaming_called(&isc->vb2_vidq)) {
 			/*
 			 * If we are streaming, we can update profile to
@@ -1546,6 +1564,7 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
 			 */
 			v4l2_ctrl_activate(isc->do_wb_ctrl, false);
 		}
+		mutex_unlock(&isc->awb_mutex);
 
 		/* if we have autowhitebalance on, start histogram procedure */
 		if (ctrls->awb == ISC_WB_AUTO &&
@@ -1738,6 +1757,7 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier,
 {
 	struct isc_device *isc = container_of(notifier->v4l2_dev,
 					      struct isc_device, v4l2_dev);
+	mutex_destroy(&isc->awb_mutex);
 	cancel_work_sync(&isc->awb_work);
 	video_unregister_device(&isc->video_dev);
 	v4l2_ctrl_handler_free(&isc->ctrls.handler);
@@ -1848,6 +1868,8 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
 	isc->current_subdev = container_of(notifier,
 					   struct isc_subdev_entity, notifier);
 	mutex_init(&isc->lock);
+	mutex_init(&isc->awb_mutex);
+
 	init_completion(&isc->comp);
 
 	/* Initialize videobuf2 queue */
@@ -1927,6 +1949,7 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
 	video_unregister_device(vdev);
 
 isc_async_complete_err:
+	mutex_destroy(&isc->awb_mutex);
 	mutex_destroy(&isc->lock);
 	return ret;
 }
diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
index c9234c90ae58..04ea5e340808 100644
--- a/drivers/media/platform/atmel/atmel-isc.h
+++ b/drivers/media/platform/atmel/atmel-isc.h
@@ -305,6 +305,7 @@ struct isc_device {
 	struct work_struct	awb_work;
 
 	struct mutex		lock; /* serialize access to file operations */
+	struct mutex		awb_mutex; /* serialize access to streaming status from awb work queue */
 	spinlock_t		awb_lock; /* serialize access to DMA buffers from awb work queue */
 
 	struct regmap_field	*pipeline[ISC_PIPE_LINE_NODE_NUM];
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 04/11] media: atmel: atmel-sama5d2-isc: fix wrong mask in YUYV format check
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

While this does not happen in production, this check should be done
versus the mask, as checking with the YCYC value may not include
some bits that may be set.
Is it correct and safe to check the whole mask.

Fixes: 123aaf816b95 ("media: atmel: atmel-sama5d2-isc: fix YUYV format")
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/media/platform/atmel/atmel-sama5d2-isc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
index c244682ea22f..025c3e8a7e95 100644
--- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
@@ -291,7 +291,7 @@ static void isc_sama5d2_config_rlp(struct isc_device *isc)
 	 * Thus, if the YCYC mode is selected, replace it with the
 	 * sama5d2-compliant mode which is YYCC .
 	 */
-	if ((rlp_mode & ISC_RLP_CFG_MODE_YCYC) == ISC_RLP_CFG_MODE_YCYC) {
+	if ((rlp_mode & ISC_RLP_CFG_MODE_MASK) == ISC_RLP_CFG_MODE_YCYC) {
 		rlp_mode &= ~ISC_RLP_CFG_MODE_MASK;
 		rlp_mode |= ISC_RLP_CFG_MODE_YYCC;
 	}
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 03/11] media: atmel: atmel-isc: implement media controller
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

Implement the support for media-controller.
This means that the capabilities of the driver have changed and now
it also advertises the IO_MC .
The driver will register it's media device, and add the video entity to this
media device. The subdevices are registered to the same media device.
The ISC will have a base entity which is auto-detected as atmel_isc_base.
It will also register a subdevice that allows cropping of the incoming frame
to the maximum frame size supported by the ISC.
The ISC will create a link between the subdevice that is asynchronously
registered and the atmel_isc_scaler entity.
Then, the atmel_isc_scaler and atmel_isc_base are connected through another
link.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
Changes in v4:
As suggested by Jacopo:
- renamed atmel_isc_mc to atmel_isc_scaler.c
- moved init_mc/clean_mc to isc_base file

Changes in v2:
- implement try formats

 drivers/media/platform/atmel/Makefile         |   2 +-
 drivers/media/platform/atmel/atmel-isc-base.c |  73 +++++-
 .../media/platform/atmel/atmel-isc-scaler.c   | 245 ++++++++++++++++++
 drivers/media/platform/atmel/atmel-isc.h      |  37 +++
 .../media/platform/atmel/atmel-sama5d2-isc.c  |  14 +-
 .../media/platform/atmel/atmel-sama7g5-isc.c  |  12 +-
 6 files changed, 375 insertions(+), 8 deletions(-)
 create mode 100644 drivers/media/platform/atmel/atmel-isc-scaler.c

diff --git a/drivers/media/platform/atmel/Makefile b/drivers/media/platform/atmel/Makefile
index 794e8f739287..f02d03df89d6 100644
--- a/drivers/media/platform/atmel/Makefile
+++ b/drivers/media/platform/atmel/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 atmel-isc-objs = atmel-sama5d2-isc.o
 atmel-xisc-objs = atmel-sama7g5-isc.o
-atmel-isc-common-objs = atmel-isc-base.o atmel-isc-clk.o
+atmel-isc-common-objs = atmel-isc-base.o atmel-isc-clk.o atmel-isc-scaler.o
 
 obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
 obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-common.o
diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index 6b0005987a17..6b482270eb93 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -1710,6 +1710,7 @@ static int isc_async_bound(struct v4l2_async_notifier *notifier,
 					      struct isc_device, v4l2_dev);
 	struct isc_subdev_entity *subdev_entity =
 		container_of(notifier, struct isc_subdev_entity, notifier);
+	int pad;
 
 	if (video_is_registered(&isc->video_dev)) {
 		v4l2_err(&isc->v4l2_dev, "only supports one sub-device.\n");
@@ -1718,6 +1719,16 @@ static int isc_async_bound(struct v4l2_async_notifier *notifier,
 
 	subdev_entity->sd = subdev;
 
+	pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
+					  MEDIA_PAD_FL_SOURCE);
+	if (pad < 0) {
+		v4l2_err(&isc->v4l2_dev, "failed to find pad for %s\n",
+			 subdev->name);
+		return pad;
+	}
+
+	isc->remote_pad = pad;
+
 	return 0;
 }
 
@@ -1732,8 +1743,8 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier,
 	v4l2_ctrl_handler_free(&isc->ctrls.handler);
 }
 
-static struct isc_format *find_format_by_code(struct isc_device *isc,
-					      unsigned int code, int *index)
+struct isc_format *isc_find_format_by_code(struct isc_device *isc,
+					   unsigned int code, int *index)
 {
 	struct isc_format *fmt = &isc->formats_list[0];
 	unsigned int i;
@@ -1749,6 +1760,7 @@ static struct isc_format *find_format_by_code(struct isc_device *isc,
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(isc_find_format_by_code);
 
 static int isc_formats_init(struct isc_device *isc)
 {
@@ -1765,7 +1777,7 @@ static int isc_formats_init(struct isc_device *isc)
 	       NULL, &mbus_code)) {
 		mbus_code.index++;
 
-		fmt = find_format_by_code(isc, mbus_code.code, &i);
+		fmt = isc_find_format_by_code(isc, mbus_code.code, &i);
 		if (!fmt) {
 			v4l2_warn(&isc->v4l2_dev, "Mbus code %x not supported\n",
 				  mbus_code.code);
@@ -1891,7 +1903,8 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
 	vdev->queue		= q;
 	vdev->lock		= &isc->lock;
 	vdev->ctrl_handler	= &isc->ctrls.handler;
-	vdev->device_caps	= V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE;
+	vdev->device_caps	= V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE |
+				  V4L2_CAP_IO_MC;
 	video_set_drvdata(vdev, isc);
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
@@ -1901,8 +1914,18 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
 		goto isc_async_complete_err;
 	}
 
+	ret = isc_scaler_link(isc);
+	if (ret < 0)
+		goto isc_async_complete_unregister_device;
+
+	ret = media_device_register(&isc->mdev);
+	if (ret < 0)
+		goto isc_async_complete_unregister_device;
 	return 0;
 
+isc_async_complete_unregister_device:
+	video_unregister_device(vdev);
+
 isc_async_complete_err:
 	mutex_destroy(&isc->lock);
 	return ret;
@@ -1969,6 +1992,48 @@ int isc_pipeline_init(struct isc_device *isc)
 }
 EXPORT_SYMBOL_GPL(isc_pipeline_init);
 
+int isc_mc_init(struct isc_device *isc, u32 ver)
+{
+	const struct of_device_id *match;
+	int ret;
+
+	isc->video_dev.entity.function = MEDIA_ENT_F_IO_V4L;
+	isc->video_dev.entity.flags = MEDIA_ENT_FL_DEFAULT;
+	isc->pads[ISC_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+
+	ret = media_entity_pads_init(&isc->video_dev.entity, ISC_PADS_NUM,
+				     isc->pads);
+	if (ret < 0) {
+		dev_err(isc->dev, "media entity init failed\n");
+		return ret;
+	}
+
+	isc->mdev.dev = isc->dev;
+
+	match = of_match_node(isc->dev->driver->of_match_table,
+			      isc->dev->of_node);
+
+	strscpy(isc->mdev.driver_name, KBUILD_MODNAME,
+		sizeof(isc->mdev.driver_name));
+	strscpy(isc->mdev.model, match->compatible, sizeof(isc->mdev.model));
+	snprintf(isc->mdev.bus_info, sizeof(isc->mdev.bus_info), "platform:%s",
+		 isc->v4l2_dev.name);
+	isc->mdev.hw_revision = ver;
+
+	media_device_init(&isc->mdev);
+
+	isc->v4l2_dev.mdev = &isc->mdev;
+
+	return isc_scaler_init(isc);
+}
+EXPORT_SYMBOL_GPL(isc_mc_init);
+
+void isc_mc_cleanup(struct isc_device *isc)
+{
+	media_entity_cleanup(&isc->video_dev.entity);
+}
+EXPORT_SYMBOL_GPL(isc_mc_cleanup);
+
 /* regmap configuration */
 #define ATMEL_ISC_REG_MAX    0xd5c
 const struct regmap_config isc_regmap_config = {
diff --git a/drivers/media/platform/atmel/atmel-isc-scaler.c b/drivers/media/platform/atmel/atmel-isc-scaler.c
new file mode 100644
index 000000000000..ec95c9665883
--- /dev/null
+++ b/drivers/media/platform/atmel/atmel-isc-scaler.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Microchip Image Sensor Controller (ISC) Scaler entity support
+ *
+ * Copyright (C) 2021 Microchip Technology, Inc.
+ *
+ * Author: Eugen Hristev <eugen.hristev@microchip.com>
+ *
+ */
+
+#include <media/media-device.h>
+#include <media/media-entity.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+#include "atmel-isc-regs.h"
+#include "atmel-isc.h"
+
+static int isc_scaler_get_fmt(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *sd_state,
+			      struct v4l2_subdev_format *format)
+{
+	struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+	struct v4l2_mbus_framefmt *v4l2_try_fmt;
+
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+		v4l2_try_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+							  format->pad);
+		format->format = *v4l2_try_fmt;
+
+		return 0;
+	}
+
+	format->format = isc->scaler_format;
+
+	return 0;
+}
+
+static int isc_scaler_set_fmt(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *sd_state,
+			      struct v4l2_subdev_format *req_fmt)
+{
+	struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+	struct v4l2_mbus_framefmt *v4l2_try_fmt;
+	struct isc_format *fmt;
+	unsigned int i;
+
+	if (req_fmt->pad == ISC_SCALER_PAD_SOURCE)
+		v4l_bound_align_image
+			(&req_fmt->format.width, 16, isc->max_width, 0,
+			 &req_fmt->format.height, 16, isc->max_height, 0, 0);
+	else
+		v4l_bound_align_image
+			(&req_fmt->format.width, 16, 10000, 0,
+			 &req_fmt->format.height, 16, 10000, 0, 0);
+
+	req_fmt->format.colorspace = V4L2_COLORSPACE_SRGB;
+	req_fmt->format.field = V4L2_FIELD_NONE;
+	req_fmt->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	req_fmt->format.quantization = V4L2_QUANTIZATION_DEFAULT;
+	req_fmt->format.xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+	fmt = isc_find_format_by_code(isc, req_fmt->format.code, &i);
+
+	if (!fmt)
+		fmt = &isc->formats_list[0];
+
+	req_fmt->format.code = fmt->mbus_code;
+
+	if (req_fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+		v4l2_try_fmt = v4l2_subdev_get_try_format(sd, sd_state,
+							  req_fmt->pad);
+		*v4l2_try_fmt = req_fmt->format;
+		/* Trying on the pad sink makes the source sink change too */
+		if (req_fmt->pad == ISC_SCALER_PAD_SINK) {
+			v4l2_try_fmt =
+				v4l2_subdev_get_try_format(sd, sd_state,
+							   ISC_SCALER_PAD_SOURCE);
+			*v4l2_try_fmt = req_fmt->format;
+
+			v4l_bound_align_image(&v4l2_try_fmt->width,
+					      16, isc->max_width, 0,
+					      &v4l2_try_fmt->height,
+					      16, isc->max_height, 0, 0);
+		}
+		/* if we are just trying, we are done */
+		return 0;
+	}
+
+	isc->scaler_format = req_fmt->format;
+
+	return 0;
+}
+
+static int isc_scaler_enum_mbus_code(struct v4l2_subdev *sd,
+				     struct v4l2_subdev_state *sd_state,
+				     struct v4l2_subdev_mbus_code_enum *code)
+{
+	struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+	int supported_index = 0;
+	int i;
+
+	for (i = 0; i < isc->formats_list_size; i++) {
+		if (!isc->formats_list[i].sd_support)
+			continue;
+		if (supported_index == code->index) {
+			code->code = isc->formats_list[i].mbus_code;
+			return 0;
+		}
+		supported_index++;
+	}
+
+	return -EINVAL;
+}
+
+static int isc_scaler_g_sel(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *sd_state,
+			    struct v4l2_subdev_selection *sel)
+{
+	struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+
+	if (sel->pad == ISC_SCALER_PAD_SOURCE)
+		return -EINVAL;
+
+	if (sel->target != V4L2_SEL_TGT_CROP_BOUNDS &&
+	    sel->target != V4L2_SEL_TGT_CROP)
+		return -EINVAL;
+
+	sel->r.height = isc->max_height;
+	sel->r.width = isc->max_width;
+
+	sel->r.left = 0;
+	sel->r.top = 0;
+
+	return 0;
+}
+
+static int isc_scaler_init_cfg(struct v4l2_subdev *sd,
+			       struct v4l2_subdev_state *sd_state)
+{
+	struct v4l2_mbus_framefmt *v4l2_try_fmt =
+		v4l2_subdev_get_try_format(sd, sd_state, 0);
+	struct isc_device *isc = container_of(sd, struct isc_device, scaler_sd);
+
+	*v4l2_try_fmt = isc->scaler_format;
+
+	return 0;
+}
+
+static const struct v4l2_subdev_pad_ops isc_scaler_pad_ops = {
+	.enum_mbus_code = isc_scaler_enum_mbus_code,
+	.set_fmt = isc_scaler_set_fmt,
+	.get_fmt = isc_scaler_get_fmt,
+	.get_selection = isc_scaler_g_sel,
+	.init_cfg = isc_scaler_init_cfg,
+};
+
+static const struct v4l2_subdev_ops xisc_scaler_subdev_ops = {
+	.pad = &isc_scaler_pad_ops,
+};
+
+int isc_scaler_init(struct isc_device *isc)
+{
+	int ret;
+
+	v4l2_subdev_init(&isc->scaler_sd, &xisc_scaler_subdev_ops);
+
+	isc->scaler_sd.owner = THIS_MODULE;
+	isc->scaler_sd.dev = isc->dev;
+	snprintf(isc->scaler_sd.name, sizeof(isc->scaler_sd.name),
+		 "atmel_isc_scaler");
+
+	isc->scaler_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	isc->scaler_sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
+	isc->scaler_pads[ISC_SCALER_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+	isc->scaler_pads[ISC_SCALER_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
+
+	isc->scaler_format.height = isc->max_height;
+	isc->scaler_format.width = isc->max_width;
+	isc->scaler_format.code = isc->formats_list[0].mbus_code;
+	isc->scaler_format.colorspace = V4L2_COLORSPACE_SRGB;
+	isc->scaler_format.field = V4L2_FIELD_NONE;
+	isc->scaler_format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	isc->scaler_format.quantization = V4L2_QUANTIZATION_DEFAULT;
+	isc->scaler_format.xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+	ret = media_entity_pads_init(&isc->scaler_sd.entity,
+				     ISC_SCALER_PADS_NUM,
+				     isc->scaler_pads);
+	if (ret < 0) {
+		dev_err(isc->dev, "scaler sd media entity init failed\n");
+		return ret;
+	}
+	ret = v4l2_device_register_subdev(&isc->v4l2_dev, &isc->scaler_sd);
+	if (ret < 0) {
+		dev_err(isc->dev, "scaler sd failed to register subdev\n");
+		return ret;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(isc_scaler_init);
+
+int isc_scaler_link(struct isc_device *isc)
+{
+	int ret;
+
+	ret = media_create_pad_link(&isc->current_subdev->sd->entity,
+				    isc->remote_pad, &isc->scaler_sd.entity,
+				    ISC_SCALER_PAD_SINK,
+				    MEDIA_LNK_FL_ENABLED |
+				    MEDIA_LNK_FL_IMMUTABLE);
+
+	if (ret < 0) {
+		v4l2_err(&isc->v4l2_dev,
+			 "Failed to create pad link: %s to %s\n",
+			 isc->current_subdev->sd->entity.name,
+			 isc->scaler_sd.entity.name);
+		return ret;
+	}
+
+	dev_dbg(isc->dev, "link with %s pad: %d\n",
+		isc->current_subdev->sd->name, isc->remote_pad);
+
+	ret = media_create_pad_link(&isc->scaler_sd.entity,
+				    ISC_SCALER_PAD_SOURCE,
+				    &isc->video_dev.entity, ISC_PAD_SINK,
+				    MEDIA_LNK_FL_ENABLED |
+				    MEDIA_LNK_FL_IMMUTABLE);
+
+	if (ret < 0) {
+		v4l2_err(&isc->v4l2_dev,
+			 "Failed to create pad link: %s to %s\n",
+			 isc->scaler_sd.entity.name,
+			 isc->video_dev.entity.name);
+		return ret;
+	}
+
+	dev_dbg(isc->dev, "link with %s pad: %d\n", isc->scaler_sd.name,
+		ISC_SCALER_PAD_SOURCE);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(isc_scaler_link);
+
diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
index 5fbf52a9080b..c9234c90ae58 100644
--- a/drivers/media/platform/atmel/atmel-isc.h
+++ b/drivers/media/platform/atmel/atmel-isc.h
@@ -183,6 +183,17 @@ struct isc_reg_offsets {
 	u32 his_entry;
 };
 
+enum isc_mc_pads {
+	ISC_PAD_SINK	= 0,
+	ISC_PADS_NUM	= 1,
+};
+
+enum isc_scaler_pads {
+	ISC_SCALER_PAD_SINK	= 0,
+	ISC_SCALER_PAD_SOURCE	= 1,
+	ISC_SCALER_PADS_NUM	= 2,
+};
+
 /*
  * struct isc_device - ISC device driver data/config struct
  * @regmap:		Register map
@@ -257,6 +268,12 @@ struct isc_reg_offsets {
  *			be used as an input to the controller
  * @controller_formats_size:	size of controller_formats array
  * @formats_list_size:	size of formats_list array
+ * @pads:		media controller pads for isc video entity
+ * @mdev:		media device that is registered by the isc
+ * @remote_pad:		remote pad on the connected subdevice
+ * @scaler_sd:		subdevice for the scaler that isc registers
+ * @scaler_pads:	media controller pads for the scaler subdevice
+ * @scaler_format:	current format for the scaler subdevice
  */
 struct isc_device {
 	struct regmap		*regmap;
@@ -344,6 +361,19 @@ struct isc_device {
 	struct isc_format		*formats_list;
 	u32				controller_formats_size;
 	u32				formats_list_size;
+
+	struct {
+		struct media_pad		pads[ISC_PADS_NUM];
+		struct media_device		mdev;
+
+		u32				remote_pad;
+	};
+
+	struct {
+		struct v4l2_subdev		scaler_sd;
+		struct media_pad		scaler_pads[ISC_SCALER_PADS_NUM];
+		struct v4l2_mbus_framefmt	scaler_format;
+	};
 };
 
 extern const struct regmap_config isc_regmap_config;
@@ -355,4 +385,11 @@ int isc_clk_init(struct isc_device *isc);
 void isc_subdev_cleanup(struct isc_device *isc);
 void isc_clk_cleanup(struct isc_device *isc);
 
+int isc_scaler_link(struct isc_device *isc);
+int isc_scaler_init(struct isc_device *isc);
+int isc_mc_init(struct isc_device *isc, u32 ver);
+void isc_mc_cleanup(struct isc_device *isc);
+
+struct isc_format *isc_find_format_by_code(struct isc_device *isc,
+					   unsigned int code, int *index);
 #endif
diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
index c5b9563e36cb..c244682ea22f 100644
--- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
@@ -553,6 +553,12 @@ static int atmel_isc_probe(struct platform_device *pdev)
 			break;
 	}
 
+	regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
+
+	ret = isc_mc_init(isc, ver);
+	if (ret < 0)
+		goto isc_probe_mc_init_err;
+
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 	pm_request_idle(dev);
@@ -562,7 +568,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
 	ret = clk_prepare_enable(isc->ispck);
 	if (ret) {
 		dev_err(dev, "failed to enable ispck: %d\n", ret);
-		goto cleanup_subdev;
+		goto isc_probe_mc_init_err;
 	}
 
 	/* ispck should be greater or equal to hclock */
@@ -572,7 +578,6 @@ static int atmel_isc_probe(struct platform_device *pdev)
 		goto unprepare_clk;
 	}
 
-	regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
 	dev_info(dev, "Microchip ISC version %x\n", ver);
 
 	return 0;
@@ -580,6 +585,9 @@ static int atmel_isc_probe(struct platform_device *pdev)
 unprepare_clk:
 	clk_disable_unprepare(isc->ispck);
 
+isc_probe_mc_init_err:
+	isc_mc_cleanup(isc);
+
 cleanup_subdev:
 	isc_subdev_cleanup(isc);
 
@@ -600,6 +608,8 @@ static int atmel_isc_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
+	isc_mc_cleanup(isc);
+
 	isc_subdev_cleanup(isc);
 
 	v4l2_device_unregister(&isc->v4l2_dev);
diff --git a/drivers/media/platform/atmel/atmel-sama7g5-isc.c b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
index 07a80b08bc54..9dc75eed0098 100644
--- a/drivers/media/platform/atmel/atmel-sama7g5-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
@@ -547,15 +547,23 @@ static int microchip_xisc_probe(struct platform_device *pdev)
 			break;
 	}
 
+	regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
+
+	ret = isc_mc_init(isc, ver);
+	if (ret < 0)
+		goto isc_probe_mc_init_err;
+
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 	pm_request_idle(dev);
 
-	regmap_read(isc->regmap, ISC_VERSION + isc->offsets.version, &ver);
 	dev_info(dev, "Microchip XISC version %x\n", ver);
 
 	return 0;
 
+isc_probe_mc_init_err:
+	isc_mc_cleanup(isc);
+
 cleanup_subdev:
 	isc_subdev_cleanup(isc);
 
@@ -576,6 +584,8 @@ static int microchip_xisc_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
+	isc_mc_cleanup(isc);
+
 	isc_subdev_cleanup(isc);
 
 	v4l2_device_unregister(&isc->v4l2_dev);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 02/11] media: atmel: atmel-isc-base: use streaming status when queueing buffers
From: Eugen Hristev @ 2022-01-21 13:14 UTC (permalink / raw)
  To: robh+dt, linux-media, devicetree, linux-kernel, linux-arm-kernel,
	jacopo+renesas, hverkuil-cisco
  Cc: nicolas.ferre, sakari.ailus, laurent.pinchart, Eugen Hristev
In-Reply-To: <20220121131416.603972-1-eugen.hristev@microchip.com>

During experiments with libcamera, it looks like vb2_is_streaming returns
true before our start streaming is called.
Order of operations is streamon -> queue -> start_streaming
ISC would have started the DMA immediately when a buffer is being added
to the vbqueue if the queue is streaming.
It is more safe to start the DMA after the start streaming of the driver is
called.
Thus, even if vb2queue is streaming, add the buffer to the dma queue of the
driver instead of actually starting the DMA process, if the start streaming
has not been called yet.
Tho achieve this, we have to use vb2_start_streaming_called instead of
vb2_is_streaming.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
Changes in v4:
- changed to using vb2_start_streaming_called instead of stop variable

 drivers/media/platform/atmel/atmel-isc-base.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index 9c62d0ae7887..6b0005987a17 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -439,7 +439,7 @@ static void isc_buffer_queue(struct vb2_buffer *vb)
 
 	spin_lock_irqsave(&isc->dma_queue_lock, flags);
 	if (!isc->cur_frm && list_empty(&isc->dma_queue) &&
-		vb2_is_streaming(vb->vb2_queue)) {
+		vb2_start_streaming_called(vb->vb2_queue)) {
 		isc->cur_frm = buf;
 		isc_start_dma(isc);
 	} else
@@ -1532,7 +1532,7 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
 
 		isc_update_awb_ctrls(isc);
 
-		if (vb2_is_streaming(&isc->vb2_vidq)) {
+		if (vb2_start_streaming_called(&isc->vb2_vidq)) {
 			/*
 			 * If we are streaming, we can update profile to
 			 * have the new settings in place.
@@ -1549,7 +1549,7 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
 
 		/* if we have autowhitebalance on, start histogram procedure */
 		if (ctrls->awb == ISC_WB_AUTO &&
-		    vb2_is_streaming(&isc->vb2_vidq) &&
+		    vb2_start_streaming_called(&isc->vb2_vidq) &&
 		    ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code))
 			isc_set_histogram(isc, true);
 
-- 
2.25.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox