Devicetree
 help / color / mirror / Atom feed
* [PATCH v6 05/11] mfd: tps6594-i2c: Add TI TPS65224 PMIC I2C
From: Bhargav Raviprakash @ 2024-04-08 12:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: m.nirmaladevi, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jpanis, devicetree, arnd, gregkh, lgirdwood, broonie,
	linus.walleij, linux-gpio, linux-arm-kernel, nm, vigneshr, kristo,
	eblanc, Bhargav Raviprakash
In-Reply-To: <20240408124047.191895-1-bhargav.r@ltts.com>

Add support for TPS65224 PMIC in TPS6594's I2C driver which has
significant functional overlap.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Acked-by: Julien Panis <jpanis@baylibre.com>
---
 drivers/mfd/tps6594-i2c.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/tps6594-i2c.c b/drivers/mfd/tps6594-i2c.c
index 9e2ed48b7..0d741da74 100644
--- a/drivers/mfd/tps6594-i2c.c
+++ b/drivers/mfd/tps6594-i2c.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * I2C access driver for TI TPS6594/TPS6593/LP8764 PMICs
+ * I2C access driver for TI TPS65224/TPS6594/TPS6593/LP8764 PMICs
  *
  * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
  */
@@ -192,16 +192,27 @@ static const struct regmap_config tps6594_i2c_regmap_config = {
 	.write = tps6594_i2c_write,
 };
 
+static const struct regmap_config tps65224_i2c_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = TPS6594_REG_DWD_FAIL_CNT_REG,
+	.volatile_table = &tps65224_volatile_table,
+	.read = tps6594_i2c_read,
+	.write = tps6594_i2c_write,
+};
+
 static const struct tps6594_match_data match_data[] = {
 	[TPS6594] = {TPS6594, &tps6594_i2c_regmap_config},
 	[TPS6593] = {TPS6593, &tps6594_i2c_regmap_config},
 	[LP8764] = {LP8764, &tps6594_i2c_regmap_config},
+	[TPS65224] = {TPS65224, &tps65224_i2c_regmap_config},
 };
 
 static const struct of_device_id tps6594_i2c_of_match_table[] = {
 	{ .compatible = "ti,tps6594-q1", .data = &match_data[TPS6594], },
 	{ .compatible = "ti,tps6593-q1", .data = &match_data[TPS6593], },
 	{ .compatible = "ti,lp8764-q1",  .data = &match_data[LP8764], },
+	{ .compatible = "ti,tps65224-q1", .data = &match_data[TPS65224], },
 	{}
 };
 MODULE_DEVICE_TABLE(of, tps6594_i2c_of_match_table);
@@ -248,5 +259,5 @@ static struct i2c_driver tps6594_i2c_driver = {
 module_i2c_driver(tps6594_i2c_driver);
 
 MODULE_AUTHOR("Julien Panis <jpanis@baylibre.com>");
-MODULE_DESCRIPTION("TPS6594 I2C Interface Driver");
+MODULE_DESCRIPTION("I2C Interface Driver for TPS65224, TPS6594/3, and LP8764");
 MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 06/11] mfd: tps6594-spi: Add TI TPS65224 PMIC SPI
From: Bhargav Raviprakash @ 2024-04-08 12:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: m.nirmaladevi, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jpanis, devicetree, arnd, gregkh, lgirdwood, broonie,
	linus.walleij, linux-gpio, linux-arm-kernel, nm, vigneshr, kristo,
	eblanc, Bhargav Raviprakash
In-Reply-To: <20240408124047.191895-1-bhargav.r@ltts.com>

Add support for TPS65224 PMIC in TPS6594's SPI driver which has
significant functional overlap.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Acked-by: Julien Panis <jpanis@baylibre.com>
---
 drivers/mfd/tps6594-spi.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/tps6594-spi.c b/drivers/mfd/tps6594-spi.c
index 82a1c02e3..a9cdc524f 100644
--- a/drivers/mfd/tps6594-spi.c
+++ b/drivers/mfd/tps6594-spi.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * SPI access driver for TI TPS6594/TPS6593/LP8764 PMICs
+ * SPI access driver for TI TPS65224/TPS6594/TPS6593/LP8764 PMICs
  *
  * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
  */
@@ -77,16 +77,29 @@ static const struct regmap_config tps6594_spi_regmap_config = {
 	.use_single_write = true,
 };
 
+static const struct regmap_config tps65224_spi_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = TPS6594_REG_DWD_FAIL_CNT_REG,
+	.volatile_table = &tps65224_volatile_table,
+	.reg_read = tps6594_spi_reg_read,
+	.reg_write = tps6594_spi_reg_write,
+	.use_single_read = true,
+	.use_single_write = true,
+};
+
 static const struct tps6594_match_data match_data[] = {
 	[TPS6594] = {TPS6594, &tps6594_spi_regmap_config},
 	[TPS6593] = {TPS6593, &tps6594_spi_regmap_config},
 	[LP8764] = {LP8764, &tps6594_spi_regmap_config},
+	[TPS65224] = {TPS65224, &tps65224_spi_regmap_config},
 };
 
 static const struct of_device_id tps6594_spi_of_match_table[] = {
 	{ .compatible = "ti,tps6594-q1", .data = &match_data[TPS6594], },
 	{ .compatible = "ti,tps6593-q1", .data = &match_data[TPS6593], },
 	{ .compatible = "ti,lp8764-q1",  .data = &match_data[LP8764], },
+	{ .compatible = "ti,tps65224-q1", .data = &match_data[TPS65224],},
 	{}
 };
 MODULE_DEVICE_TABLE(of, tps6594_spi_of_match_table);
@@ -133,5 +146,5 @@ static struct spi_driver tps6594_spi_driver = {
 module_spi_driver(tps6594_spi_driver);
 
 MODULE_AUTHOR("Julien Panis <jpanis@baylibre.com>");
-MODULE_DESCRIPTION("TPS6594 SPI Interface Driver");
+MODULE_DESCRIPTION("SPI Interface Driver for TPS65224, TPS6594/3, and LP8764");
 MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 07/11] mfd: tps6594-core: Add TI TPS65224 PMIC core
From: Bhargav Raviprakash @ 2024-04-08 12:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: m.nirmaladevi, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jpanis, devicetree, arnd, gregkh, lgirdwood, broonie,
	linus.walleij, linux-gpio, linux-arm-kernel, nm, vigneshr, kristo,
	eblanc, Bhargav Raviprakash
In-Reply-To: <20240408124047.191895-1-bhargav.r@ltts.com>

Add functionality of the TPS65224 PMIC to the TPS6594 core driver. This
includes adding IRQ resource, MFD cells, and device initialization for
TPS65224.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Acked-by: Julien Panis <jpanis@baylibre.com>
---
 drivers/mfd/tps6594-core.c | 237 ++++++++++++++++++++++++++++++++++---
 1 file changed, 219 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
index 089ab8cc8..c59f3d7e3 100644
--- a/drivers/mfd/tps6594-core.c
+++ b/drivers/mfd/tps6594-core.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Core functions for TI TPS6594/TPS6593/LP8764 PMICs
+ * Core functions for TI TPS65224/TPS6594/TPS6593/LP8764 PMICs
  *
  * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
  */
@@ -278,16 +278,159 @@ static const unsigned int tps6594_irq_reg[] = {
 	TPS6594_REG_RTC_STATUS,
 };
 
+/* TPS65224 Resources */
+
+static const struct resource tps65224_regulator_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK1_UVOV, TPS65224_IRQ_NAME_BUCK1_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK2_UVOV, TPS65224_IRQ_NAME_BUCK2_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK3_UVOV, TPS65224_IRQ_NAME_BUCK3_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK4_UVOV, TPS65224_IRQ_NAME_BUCK4_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_LDO1_UVOV, TPS65224_IRQ_NAME_LDO1_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_LDO2_UVOV, TPS65224_IRQ_NAME_LDO2_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_LDO3_UVOV, TPS65224_IRQ_NAME_LDO3_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VCCA_UVOV, TPS65224_IRQ_NAME_VCCA_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VMON1_UVOV, TPS65224_IRQ_NAME_VMON1_UVOV),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VMON2_UVOV, TPS65224_IRQ_NAME_VMON2_UVOV),
+};
+
+static const struct resource tps65224_pinctrl_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO1, TPS65224_IRQ_NAME_GPIO1),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO2, TPS65224_IRQ_NAME_GPIO2),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO3, TPS65224_IRQ_NAME_GPIO3),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO4, TPS65224_IRQ_NAME_GPIO4),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO5, TPS65224_IRQ_NAME_GPIO5),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO6, TPS65224_IRQ_NAME_GPIO6),
+};
+
+static const struct resource tps65224_pfsm_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VSENSE, TPS65224_IRQ_NAME_VSENSE),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ENABLE, TPS65224_IRQ_NAME_ENABLE),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_SHORT, TPS65224_IRQ_NAME_PB_SHORT),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_FSD, TPS65224_IRQ_NAME_FSD),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_SOFT_REBOOT, TPS65224_IRQ_NAME_SOFT_REBOOT),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BIST_PASS, TPS65224_IRQ_NAME_BIST_PASS),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_EXT_CLK, TPS65224_IRQ_NAME_EXT_CLK),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_UNLOCK, TPS65224_IRQ_NAME_REG_UNLOCK),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TWARN, TPS65224_IRQ_NAME_TWARN),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_LONG, TPS65224_IRQ_NAME_PB_LONG),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_FALL, TPS65224_IRQ_NAME_PB_FALL),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_RISE, TPS65224_IRQ_NAME_PB_RISE),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TSD_ORD, TPS65224_IRQ_NAME_TSD_ORD),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BIST_FAIL, TPS65224_IRQ_NAME_BIST_FAIL),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_CRC_ERR, TPS65224_IRQ_NAME_REG_CRC_ERR),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_RECOV_CNT, TPS65224_IRQ_NAME_RECOV_CNT),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TSD_IMM, TPS65224_IRQ_NAME_TSD_IMM),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VCCA_OVP, TPS65224_IRQ_NAME_VCCA_OVP),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PFSM_ERR, TPS65224_IRQ_NAME_PFSM_ERR),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BG_XMON, TPS65224_IRQ_NAME_BG_XMON),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_IMM_SHUTDOWN, TPS65224_IRQ_NAME_IMM_SHUTDOWN),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ORD_SHUTDOWN, TPS65224_IRQ_NAME_ORD_SHUTDOWN),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_MCU_PWR_ERR, TPS65224_IRQ_NAME_MCU_PWR_ERR),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_SOC_PWR_ERR, TPS65224_IRQ_NAME_SOC_PWR_ERR),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_COMM_ERR, TPS65224_IRQ_NAME_COMM_ERR),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_I2C2_ERR, TPS65224_IRQ_NAME_I2C2_ERR),
+};
+
+static const struct resource tps65224_adc_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ADC_CONV_READY, TPS65224_IRQ_NAME_ADC_CONV_READY),
+};
+
+static const struct mfd_cell tps65224_common_cells[] = {
+	MFD_CELL_RES("tps65224-adc", tps65224_adc_resources),
+	MFD_CELL_RES("tps6594-pfsm", tps65224_pfsm_resources),
+	MFD_CELL_RES("tps6594-pinctrl", tps65224_pinctrl_resources),
+	MFD_CELL_RES("tps6594-regulator", tps65224_regulator_resources),
+};
+
+static const struct regmap_irq tps65224_irqs[] = {
+	/* INT_BUCK register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_BUCK1_UVOV, 0, TPS65224_BIT_BUCK1_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_BUCK2_UVOV, 0, TPS65224_BIT_BUCK2_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_BUCK3_UVOV, 0, TPS65224_BIT_BUCK3_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_BUCK4_UVOV, 0, TPS65224_BIT_BUCK4_UVOV_INT),
+
+	/* INT_VMON_LDO register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_LDO1_UVOV, 1, TPS65224_BIT_LDO1_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_LDO2_UVOV, 1, TPS65224_BIT_LDO2_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_LDO3_UVOV, 1, TPS65224_BIT_LDO3_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_VCCA_UVOV, 1, TPS65224_BIT_VCCA_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_VMON1_UVOV, 1, TPS65224_BIT_VMON1_UVOV_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_VMON2_UVOV, 1, TPS65224_BIT_VMON2_UVOV_INT),
+
+	/* INT_GPIO register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_GPIO1, 2, TPS65224_BIT_GPIO1_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_GPIO2, 2, TPS65224_BIT_GPIO2_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_GPIO3, 2, TPS65224_BIT_GPIO3_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_GPIO4, 2, TPS65224_BIT_GPIO4_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_GPIO5, 2, TPS65224_BIT_GPIO5_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_GPIO6, 2, TPS65224_BIT_GPIO6_INT),
+
+	/* INT_STARTUP register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_VSENSE, 3, TPS65224_BIT_VSENSE_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_ENABLE, 3, TPS6594_BIT_ENABLE_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_PB_SHORT, 3, TPS65224_BIT_PB_SHORT_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_FSD, 3, TPS6594_BIT_FSD_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_SOFT_REBOOT, 3, TPS6594_BIT_SOFT_REBOOT_INT),
+
+	/* INT_MISC register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_BIST_PASS, 4, TPS6594_BIT_BIST_PASS_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_EXT_CLK, 4, TPS6594_BIT_EXT_CLK_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_REG_UNLOCK, 4, TPS65224_BIT_REG_UNLOCK_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_TWARN, 4, TPS6594_BIT_TWARN_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_PB_LONG, 4, TPS65224_BIT_PB_LONG_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_PB_FALL, 4, TPS65224_BIT_PB_FALL_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_PB_RISE, 4, TPS65224_BIT_PB_RISE_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_ADC_CONV_READY, 4, TPS65224_BIT_ADC_CONV_READY_INT),
+
+	/* INT_MODERATE_ERR register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_TSD_ORD, 5, TPS6594_BIT_TSD_ORD_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_BIST_FAIL, 5, TPS6594_BIT_BIST_FAIL_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_REG_CRC_ERR, 5, TPS6594_BIT_REG_CRC_ERR_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_RECOV_CNT, 5, TPS6594_BIT_RECOV_CNT_INT),
+
+	/* INT_SEVERE_ERR register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_TSD_IMM, 6, TPS6594_BIT_TSD_IMM_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_VCCA_OVP, 6, TPS6594_BIT_VCCA_OVP_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_PFSM_ERR, 6, TPS6594_BIT_PFSM_ERR_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_BG_XMON, 6, TPS65224_BIT_BG_XMON_INT),
+
+	/* INT_FSM_ERR register */
+	REGMAP_IRQ_REG(TPS65224_IRQ_IMM_SHUTDOWN, 7, TPS6594_BIT_IMM_SHUTDOWN_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_ORD_SHUTDOWN, 7, TPS6594_BIT_ORD_SHUTDOWN_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_MCU_PWR_ERR, 7, TPS6594_BIT_MCU_PWR_ERR_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_SOC_PWR_ERR, 7, TPS6594_BIT_SOC_PWR_ERR_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_COMM_ERR, 7, TPS6594_BIT_COMM_ERR_INT),
+	REGMAP_IRQ_REG(TPS65224_IRQ_I2C2_ERR, 7, TPS65224_BIT_I2C2_ERR_INT),
+};
+
+static const unsigned int tps65224_irq_reg[] = {
+	TPS6594_REG_INT_BUCK,
+	TPS6594_REG_INT_LDO_VMON,
+	TPS6594_REG_INT_GPIO,
+	TPS6594_REG_INT_STARTUP,
+	TPS6594_REG_INT_MISC,
+	TPS6594_REG_INT_MODERATE_ERR,
+	TPS6594_REG_INT_SEVERE_ERR,
+	TPS6594_REG_INT_FSM_ERR,
+};
+
 static inline unsigned int tps6594_get_irq_reg(struct regmap_irq_chip_data *data,
 					       unsigned int base, int index)
 {
 	return tps6594_irq_reg[index];
 };
 
+static inline unsigned int tps65224_get_irq_reg(struct regmap_irq_chip_data *data,
+						unsigned int base, int index)
+{
+	return tps65224_irq_reg[index];
+};
+
 static int tps6594_handle_post_irq(void *irq_drv_data)
 {
 	struct tps6594 *tps = irq_drv_data;
 	int ret = 0;
+	unsigned int regmap_reg, mask_val;
 
 	/*
 	 * When CRC is enabled, writing to a read-only bit triggers an error,
@@ -299,10 +442,17 @@ static int tps6594_handle_post_irq(void *irq_drv_data)
 	 * COMM_ADR_ERR_INT bit set. Clear immediately this bit to avoid raising
 	 * a new interrupt.
 	 */
-	if (tps->use_crc)
-		ret = regmap_write_bits(tps->regmap, TPS6594_REG_INT_COMM_ERR,
-					TPS6594_BIT_COMM_ADR_ERR_INT,
-					TPS6594_BIT_COMM_ADR_ERR_INT);
+	if (tps->use_crc) {
+		if (tps->chip_id == TPS65224) {
+			regmap_reg = TPS6594_REG_INT_FSM_ERR;
+			mask_val = TPS6594_BIT_COMM_ERR_INT;
+		} else {
+			regmap_reg = TPS6594_REG_INT_COMM_ERR;
+			mask_val = TPS6594_BIT_COMM_ADR_ERR_INT;
+		}
+
+		ret = regmap_write_bits(tps->regmap, regmap_reg, mask_val, mask_val);
+	}
 
 	return ret;
 };
@@ -319,6 +469,18 @@ static struct regmap_irq_chip tps6594_irq_chip = {
 	.handle_post_irq = tps6594_handle_post_irq,
 };
 
+static struct regmap_irq_chip tps65224_irq_chip = {
+	.ack_base = TPS6594_REG_INT_BUCK,
+	.ack_invert = 1,
+	.clear_ack = 1,
+	.init_ack_masked = 1,
+	.num_regs = ARRAY_SIZE(tps65224_irq_reg),
+	.irqs = tps65224_irqs,
+	.num_irqs = ARRAY_SIZE(tps65224_irqs),
+	.get_irq_reg = tps65224_get_irq_reg,
+	.handle_post_irq = tps6594_handle_post_irq,
+};
+
 static const struct regmap_range tps6594_volatile_ranges[] = {
 	regmap_reg_range(TPS6594_REG_INT_TOP, TPS6594_REG_STAT_READBACK_ERR),
 	regmap_reg_range(TPS6594_REG_RTC_STATUS, TPS6594_REG_RTC_STATUS),
@@ -330,17 +492,35 @@ const struct regmap_access_table tps6594_volatile_table = {
 };
 EXPORT_SYMBOL_GPL(tps6594_volatile_table);
 
+static const struct regmap_range tps65224_volatile_ranges[] = {
+	regmap_reg_range(TPS6594_REG_INT_TOP, TPS6594_REG_STAT_SEVERE_ERR),
+};
+
+const struct regmap_access_table tps65224_volatile_table = {
+	.yes_ranges = tps65224_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(tps65224_volatile_ranges),
+};
+EXPORT_SYMBOL_GPL(tps65224_volatile_table);
+
 static int tps6594_check_crc_mode(struct tps6594 *tps, bool primary_pmic)
 {
 	int ret;
+	unsigned int regmap_reg, mask_val;
+
+	if (tps->chip_id == TPS65224) {
+		regmap_reg = TPS6594_REG_CONFIG_2;
+		mask_val = TPS65224_BIT_I2C1_SPI_CRC_EN;
+	} else {
+		regmap_reg = TPS6594_REG_SERIAL_IF_CONFIG;
+		mask_val = TPS6594_BIT_I2C1_SPI_CRC_EN;
+	};
 
 	/*
 	 * Check if CRC is enabled.
 	 * Once CRC is enabled, it can't be disabled until next power cycle.
 	 */
 	tps->use_crc = true;
-	ret = regmap_test_bits(tps->regmap, TPS6594_REG_SERIAL_IF_CONFIG,
-			       TPS6594_BIT_I2C1_SPI_CRC_EN);
+	ret = regmap_test_bits(tps->regmap, regmap_reg, mask_val);
 	if (ret == 0) {
 		ret = -EIO;
 	} else if (ret > 0) {
@@ -355,6 +535,15 @@ static int tps6594_check_crc_mode(struct tps6594 *tps, bool primary_pmic)
 static int tps6594_set_crc_feature(struct tps6594 *tps)
 {
 	int ret;
+	unsigned int regmap_reg, mask_val;
+
+	if (tps->chip_id == TPS65224) {
+		regmap_reg = TPS6594_REG_CONFIG_2;
+		mask_val = TPS65224_BIT_I2C1_SPI_CRC_EN;
+	} else {
+		regmap_reg = TPS6594_REG_FSM_I2C_TRIGGERS;
+		mask_val = TPS6594_BIT_TRIGGER_I2C(2);
+	}
 
 	ret = tps6594_check_crc_mode(tps, true);
 	if (ret) {
@@ -363,8 +552,7 @@ static int tps6594_set_crc_feature(struct tps6594 *tps)
 		 * on primary PMIC.
 		 */
 		tps->use_crc = false;
-		ret = regmap_write_bits(tps->regmap, TPS6594_REG_FSM_I2C_TRIGGERS,
-					TPS6594_BIT_TRIGGER_I2C(2), TPS6594_BIT_TRIGGER_I2C(2));
+		ret = regmap_write_bits(tps->regmap, regmap_reg, mask_val, mask_val);
 		if (ret)
 			return ret;
 
@@ -420,6 +608,9 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 {
 	struct device *dev = tps->dev;
 	int ret;
+	struct regmap_irq_chip *irq_chip;
+	const struct mfd_cell *cells;
+	int n_cells;
 
 	if (enable_crc) {
 		ret = tps6594_enable_crc(tps);
@@ -433,26 +624,35 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to set PMIC state\n");
 
-	tps6594_irq_chip.irq_drv_data = tps;
-	tps6594_irq_chip.name = devm_kasprintf(dev, GFP_KERNEL, "%s-%ld-0x%02x",
-					       dev->driver->name, tps->chip_id, tps->reg);
+	if (tps->chip_id == TPS65224) {
+		irq_chip = &tps65224_irq_chip;
+		n_cells = ARRAY_SIZE(tps65224_common_cells);
+		cells = tps65224_common_cells;
+	} else {
+		irq_chip = &tps6594_irq_chip;
+		n_cells = ARRAY_SIZE(tps6594_common_cells);
+		cells = tps6594_common_cells;
+	}
+
+	irq_chip->irq_drv_data = tps;
+	irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-%ld-0x%02x",
+					dev->driver->name, tps->chip_id, tps->reg);
 
-	if (!tps6594_irq_chip.name)
+	if (!irq_chip->name)
 		return -ENOMEM;
 
 	ret = devm_regmap_add_irq_chip(dev, tps->regmap, tps->irq, IRQF_SHARED | IRQF_ONESHOT,
-				       0, &tps6594_irq_chip, &tps->irq_data);
+				       0, irq_chip, &tps->irq_data);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add regmap IRQ\n");
 
-	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, tps6594_common_cells,
-				   ARRAY_SIZE(tps6594_common_cells), NULL, 0,
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, n_cells, NULL, 0,
 				   regmap_irq_get_domain(tps->irq_data));
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add common child devices\n");
 
-	/* No RTC for LP8764 */
-	if (tps->chip_id != LP8764) {
+	/* No RTC for LP8764 and TPS65224 */
+	if (tps->chip_id != LP8764 && tps->chip_id != TPS65224) {
 		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, tps6594_rtc_cells,
 					   ARRAY_SIZE(tps6594_rtc_cells), NULL, 0,
 					   regmap_irq_get_domain(tps->irq_data));
@@ -465,5 +665,6 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 EXPORT_SYMBOL_GPL(tps6594_device_init);
 
 MODULE_AUTHOR("Julien Panis <jpanis@baylibre.com>");
+MODULE_AUTHOR("Bhargav Raviprakash <bhargav.r@ltts.com");
 MODULE_DESCRIPTION("TPS6594 Driver");
 MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 08/11] misc: tps6594-pfsm: Add TI TPS65224 PMIC PFSM
From: Bhargav Raviprakash @ 2024-04-08 12:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: m.nirmaladevi, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jpanis, devicetree, arnd, gregkh, lgirdwood, broonie,
	linus.walleij, linux-gpio, linux-arm-kernel, nm, vigneshr, kristo,
	eblanc, Bhargav Raviprakash
In-Reply-To: <20240408124047.191895-1-bhargav.r@ltts.com>

Add support for TPS65224 PFSM in the TPS6594 PFSM driver as they share
significant functionality.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Acked-by: Julien Panis <jpanis@baylibre.com>
---
 drivers/misc/tps6594-pfsm.c | 48 +++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/tps6594-pfsm.c b/drivers/misc/tps6594-pfsm.c
index 88dcac814..9bcca1856 100644
--- a/drivers/misc/tps6594-pfsm.c
+++ b/drivers/misc/tps6594-pfsm.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * PFSM (Pre-configurable Finite State Machine) driver for TI TPS6594/TPS6593/LP8764 PMICs
+ * PFSM (Pre-configurable Finite State Machine) driver for TI TPS65224/TPS6594/TPS6593/LP8764 PMICs
  *
  * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
  */
@@ -39,10 +39,12 @@
  *
  * @miscdev: misc device infos
  * @regmap:  regmap for accessing the device registers
+ * @chip_id: chip identifier of the device
  */
 struct tps6594_pfsm {
 	struct miscdevice miscdev;
 	struct regmap *regmap;
+	unsigned long chip_id;
 };
 
 static ssize_t tps6594_pfsm_read(struct file *f, char __user *buf,
@@ -133,21 +135,29 @@ static long tps6594_pfsm_ioctl(struct file *f, unsigned int cmd, unsigned long a
 	struct tps6594_pfsm *pfsm = TPS6594_FILE_TO_PFSM(f);
 	struct pmic_state_opt state_opt;
 	void __user *argp = (void __user *)arg;
+	unsigned int regmap_reg, mask;
 	int ret = -ENOIOCTLCMD;
 
 	switch (cmd) {
 	case PMIC_GOTO_STANDBY:
-		/* Disable LP mode */
-		ret = regmap_clear_bits(pfsm->regmap, TPS6594_REG_RTC_CTRL_2,
-					TPS6594_BIT_LP_STANDBY_SEL);
-		if (ret)
-			return ret;
+		/* Disable LP mode on TPS6594 Family PMIC */
+		if (pfsm->chip_id != TPS65224) {
+			ret = regmap_clear_bits(pfsm->regmap, TPS6594_REG_RTC_CTRL_2,
+						TPS6594_BIT_LP_STANDBY_SEL);
+
+			if (ret)
+				return ret;
+		}
 
 		/* Force trigger */
 		ret = regmap_write_bits(pfsm->regmap, TPS6594_REG_FSM_I2C_TRIGGERS,
 					TPS6594_BIT_TRIGGER_I2C(0), TPS6594_BIT_TRIGGER_I2C(0));
 		break;
 	case PMIC_GOTO_LP_STANDBY:
+		/* TPS65224 does not support LP STANDBY */
+		if (pfsm->chip_id == TPS65224)
+			return ret;
+
 		/* Enable LP mode */
 		ret = regmap_set_bits(pfsm->regmap, TPS6594_REG_RTC_CTRL_2,
 				      TPS6594_BIT_LP_STANDBY_SEL);
@@ -169,6 +179,10 @@ static long tps6594_pfsm_ioctl(struct file *f, unsigned int cmd, unsigned long a
 				      TPS6594_BIT_NSLEEP1B | TPS6594_BIT_NSLEEP2B);
 		break;
 	case PMIC_SET_MCU_ONLY_STATE:
+		/* TPS65224 does not support MCU_ONLY_STATE */
+		if (pfsm->chip_id == TPS65224)
+			return ret;
+
 		if (copy_from_user(&state_opt, argp, sizeof(state_opt)))
 			return -EFAULT;
 
@@ -192,14 +206,20 @@ static long tps6594_pfsm_ioctl(struct file *f, unsigned int cmd, unsigned long a
 			return -EFAULT;
 
 		/* Configure wake-up destination */
+		if (pfsm->chip_id == TPS65224) {
+			regmap_reg = TPS65224_REG_STARTUP_CTRL;
+			mask = TPS65224_MASK_STARTUP_DEST;
+		} else {
+			regmap_reg = TPS6594_REG_RTC_CTRL_2;
+			mask = TPS6594_MASK_STARTUP_DEST;
+		}
+
 		if (state_opt.mcu_only_startup_dest)
-			ret = regmap_write_bits(pfsm->regmap, TPS6594_REG_RTC_CTRL_2,
-						TPS6594_MASK_STARTUP_DEST,
-						TPS6594_STARTUP_DEST_MCU_ONLY);
+			ret = regmap_write_bits(pfsm->regmap, regmap_reg,
+						mask, TPS6594_STARTUP_DEST_MCU_ONLY);
 		else
-			ret = regmap_write_bits(pfsm->regmap, TPS6594_REG_RTC_CTRL_2,
-						TPS6594_MASK_STARTUP_DEST,
-						TPS6594_STARTUP_DEST_ACTIVE);
+			ret = regmap_write_bits(pfsm->regmap, regmap_reg,
+						mask, TPS6594_STARTUP_DEST_ACTIVE);
 		if (ret)
 			return ret;
 
@@ -211,7 +231,8 @@ static long tps6594_pfsm_ioctl(struct file *f, unsigned int cmd, unsigned long a
 
 		/* Modify NSLEEP1-2 bits */
 		ret = regmap_clear_bits(pfsm->regmap, TPS6594_REG_FSM_NSLEEP_TRIGGERS,
-					TPS6594_BIT_NSLEEP2B);
+					pfsm->chip_id == TPS65224 ?
+					TPS6594_BIT_NSLEEP1B : TPS6594_BIT_NSLEEP2B);
 		break;
 	}
 
@@ -262,6 +283,7 @@ static int tps6594_pfsm_probe(struct platform_device *pdev)
 					    tps->chip_id, tps->reg);
 	pfsm->miscdev.fops = &tps6594_pfsm_fops;
 	pfsm->miscdev.parent = dev->parent;
+	pfsm->chip_id = tps->chip_id;
 
 	for (i = 0 ; i < pdev->num_resources ; i++) {
 		irq = platform_get_irq_byname(pdev, pdev->resource[i].name);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 09/11] regulator: tps6594-regulator: Add TI TPS65224 PMIC regulators
From: Bhargav Raviprakash @ 2024-04-08 12:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: m.nirmaladevi, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jpanis, devicetree, arnd, gregkh, lgirdwood, broonie,
	linus.walleij, linux-gpio, linux-arm-kernel, nm, vigneshr, kristo,
	eblanc, Bhargav Raviprakash
In-Reply-To: <20240408124047.191895-1-bhargav.r@ltts.com>

From: Nirmala Devi Mal Nadar <m.nirmaladevi@ltts.com>

Add support for TPS65224 regulators (bucks and LDOs) to TPS6594 driver as
they have significant functional overlap. TPS65224 PMIC has 4 buck
regulators and 3 LDOs. BUCK12 can operate in dual phase.
The output voltages are configurable and are meant to supply power to the
main processor and other components.

Signed-off-by: Nirmala Devi Mal Nadar <m.nirmaladevi@ltts.com>
Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/Kconfig             |   4 +-
 drivers/regulator/tps6594-regulator.c | 243 +++++++++++++++++++++++---
 2 files changed, 222 insertions(+), 25 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 7db0a29b5..1e4119f00 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1563,13 +1563,15 @@ config REGULATOR_TPS6594
 	depends on MFD_TPS6594 && OF
 	default MFD_TPS6594
 	help
-	  This driver supports TPS6594 voltage regulator chips.
+	  This driver supports TPS6594 series and TPS65224 voltage regulator chips.
 	  TPS6594 series of PMICs have 5 BUCKs and 4 LDOs
 	  voltage regulators.
 	  BUCKs 1,2,3,4 can be used in single phase or multiphase mode.
 	  Part number defines which single or multiphase mode is i used.
 	  It supports software based voltage control
 	  for different voltage domains.
+	  TPS65224 PMIC has 4 BUCKs and 3 LDOs. BUCK12 can be used in dual phase.
+	  All BUCKs and LDOs volatge can be controlled through software.
 
 config REGULATOR_TPS6524X
 	tristate "TI TPS6524X Power regulators"
diff --git a/drivers/regulator/tps6594-regulator.c b/drivers/regulator/tps6594-regulator.c
index b7f0c8779..3c8e0b1cd 100644
--- a/drivers/regulator/tps6594-regulator.c
+++ b/drivers/regulator/tps6594-regulator.c
@@ -66,6 +66,15 @@ static struct tps6594_regulator_irq_type tps6594_ext_regulator_irq_types[] = {
 	  REGULATOR_EVENT_OVER_VOLTAGE_WARN },
 };
 
+static struct tps6594_regulator_irq_type tps65224_ext_regulator_irq_types[] = {
+	{ TPS65224_IRQ_NAME_VCCA_UVOV, "VCCA", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+	{ TPS65224_IRQ_NAME_VMON1_UVOV, "VMON1", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+	{ TPS65224_IRQ_NAME_VMON2_UVOV, "VMON2", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
 struct tps6594_regulator_irq_data {
 	struct device *dev;
 	struct tps6594_regulator_irq_type *type;
@@ -122,6 +131,27 @@ static const struct linear_range ldos_4_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1200000, 0x20, 0x74, 25000),
 };
 
+/* Voltage range for TPS65224 Bucks and LDOs */
+static const struct linear_range tps65224_bucks_1_ranges[] = {
+	REGULATOR_LINEAR_RANGE(500000, 0x0a, 0x0e, 20000),
+	REGULATOR_LINEAR_RANGE(600000, 0x0f, 0x72, 5000),
+	REGULATOR_LINEAR_RANGE(1100000, 0x73, 0xaa, 10000),
+	REGULATOR_LINEAR_RANGE(1660000, 0xab, 0xfd, 20000),
+};
+
+static const struct linear_range tps65224_bucks_2_3_4_ranges[] = {
+	REGULATOR_LINEAR_RANGE(500000, 0x0, 0x1a, 25000),
+	REGULATOR_LINEAR_RANGE(1200000, 0x1b, 0x45, 50000),
+};
+
+static const struct linear_range tps65224_ldos_1_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1200000, 0xC, 0x36, 50000),
+};
+
+static const struct linear_range tps65224_ldos_2_3_ranges[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x38, 50000),
+};
+
 /* Operations permitted on BUCK1/2/3/4/5 */
 static const struct regulator_ops tps6594_bucks_ops = {
 	.is_enabled		= regulator_is_enabled_regmap,
@@ -197,6 +227,38 @@ static const struct regulator_desc buck_regs[] = {
 			  4, 0, 0, NULL, 0, 0),
 };
 
+/* Buck configuration for TPS65224 */
+static const struct regulator_desc tps65224_buck_regs[] = {
+	TPS6594_REGULATOR("BUCK1", "buck1", TPS6594_BUCK_1,
+			  REGULATOR_VOLTAGE, tps6594_bucks_ops, TPS65224_MASK_BUCK1_VSET,
+			  TPS6594_REG_BUCKX_VOUT_1(0),
+			  TPS65224_MASK_BUCK1_VSET,
+			  TPS6594_REG_BUCKX_CTRL(0),
+			  TPS6594_BIT_BUCK_EN, 0, 0, tps65224_bucks_1_ranges,
+			  4, 0, 0, NULL, 0, 0),
+	TPS6594_REGULATOR("BUCK2", "buck2", TPS6594_BUCK_2,
+			  REGULATOR_VOLTAGE, tps6594_bucks_ops, TPS65224_MASK_BUCKS_VSET,
+			  TPS6594_REG_BUCKX_VOUT_1(1),
+			  TPS65224_MASK_BUCKS_VSET,
+			  TPS6594_REG_BUCKX_CTRL(1),
+			  TPS6594_BIT_BUCK_EN, 0, 0, tps65224_bucks_2_3_4_ranges,
+			  4, 0, 0, NULL, 0, 0),
+	TPS6594_REGULATOR("BUCK3", "buck3", TPS6594_BUCK_3,
+			  REGULATOR_VOLTAGE, tps6594_bucks_ops, TPS65224_MASK_BUCKS_VSET,
+			  TPS6594_REG_BUCKX_VOUT_1(2),
+			  TPS65224_MASK_BUCKS_VSET,
+			  TPS6594_REG_BUCKX_CTRL(2),
+			  TPS6594_BIT_BUCK_EN, 0, 0, tps65224_bucks_2_3_4_ranges,
+			  4, 0, 0, NULL, 0, 0),
+	TPS6594_REGULATOR("BUCK4", "buck4", TPS6594_BUCK_4,
+			  REGULATOR_VOLTAGE, tps6594_bucks_ops, TPS65224_MASK_BUCKS_VSET,
+			  TPS6594_REG_BUCKX_VOUT_1(3),
+			  TPS65224_MASK_BUCKS_VSET,
+			  TPS6594_REG_BUCKX_CTRL(3),
+			  TPS6594_BIT_BUCK_EN, 0, 0, tps65224_bucks_2_3_4_ranges,
+			  4, 0, 0, NULL, 0, 0),
+};
+
 static struct tps6594_regulator_irq_type tps6594_buck1_irq_types[] = {
 	{ TPS6594_IRQ_NAME_BUCK1_OV, "BUCK1", "overvoltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
 	{ TPS6594_IRQ_NAME_BUCK1_UV, "BUCK1", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
@@ -269,6 +331,41 @@ static struct tps6594_regulator_irq_type tps6594_ldo4_irq_types[] = {
 	  REGULATOR_EVENT_OVER_CURRENT },
 };
 
+static struct tps6594_regulator_irq_type tps65224_buck1_irq_types[] = {
+	{ TPS65224_IRQ_NAME_BUCK1_UVOV, "BUCK1", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
+static struct tps6594_regulator_irq_type tps65224_buck2_irq_types[] = {
+	{ TPS65224_IRQ_NAME_BUCK2_UVOV, "BUCK2", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
+static struct tps6594_regulator_irq_type tps65224_buck3_irq_types[] = {
+	{ TPS65224_IRQ_NAME_BUCK3_UVOV, "BUCK3", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
+static struct tps6594_regulator_irq_type tps65224_buck4_irq_types[] = {
+	{ TPS65224_IRQ_NAME_BUCK4_UVOV, "BUCK4", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
+static struct tps6594_regulator_irq_type tps65224_ldo1_irq_types[] = {
+	{ TPS65224_IRQ_NAME_LDO1_UVOV, "LDO1", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
+static struct tps6594_regulator_irq_type tps65224_ldo2_irq_types[] = {
+	{ TPS65224_IRQ_NAME_LDO2_UVOV, "LDO2", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
+static struct tps6594_regulator_irq_type tps65224_ldo3_irq_types[] = {
+	{ TPS65224_IRQ_NAME_LDO3_UVOV, "LDO3", "voltage out of range",
+	  REGULATOR_EVENT_REGULATION_OUT },
+};
+
 static struct tps6594_regulator_irq_type *tps6594_bucks_irq_types[] = {
 	tps6594_buck1_irq_types,
 	tps6594_buck2_irq_types,
@@ -284,7 +381,20 @@ static struct tps6594_regulator_irq_type *tps6594_ldos_irq_types[] = {
 	tps6594_ldo4_irq_types,
 };
 
-static const struct regulator_desc multi_regs[] = {
+static struct tps6594_regulator_irq_type *tps65224_bucks_irq_types[] = {
+	tps65224_buck1_irq_types,
+	tps65224_buck2_irq_types,
+	tps65224_buck3_irq_types,
+	tps65224_buck4_irq_types,
+};
+
+static struct tps6594_regulator_irq_type *tps65224_ldos_irq_types[] = {
+	tps65224_ldo1_irq_types,
+	tps65224_ldo2_irq_types,
+	tps65224_ldo3_irq_types,
+};
+
+static const struct regulator_desc tps6594_multi_regs[] = {
 	TPS6594_REGULATOR("BUCK12", "buck12", TPS6594_BUCK_1,
 			  REGULATOR_VOLTAGE, tps6594_bucks_ops, TPS6594_MASK_BUCKS_VSET,
 			  TPS6594_REG_BUCKX_VOUT_1(1),
@@ -315,7 +425,17 @@ static const struct regulator_desc multi_regs[] = {
 			  4, 4000, 0, NULL, 0, 0),
 };
 
-static const struct regulator_desc ldo_regs[] = {
+static const struct regulator_desc tps65224_multi_regs[] = {
+	TPS6594_REGULATOR("BUCK12", "buck12", TPS6594_BUCK_1,
+			  REGULATOR_VOLTAGE, tps6594_bucks_ops, TPS65224_MASK_BUCK1_VSET,
+			  TPS6594_REG_BUCKX_VOUT_1(0),
+			  TPS65224_MASK_BUCK1_VSET,
+			  TPS6594_REG_BUCKX_CTRL(0),
+			  TPS6594_BIT_BUCK_EN, 0, 0, tps65224_bucks_1_ranges,
+			  4, 4000, 0, NULL, 0, 0),
+};
+
+static const struct regulator_desc tps6594_ldo_regs[] = {
 	TPS6594_REGULATOR("LDO1", "ldo1", TPS6594_LDO_1,
 			  REGULATOR_VOLTAGE, tps6594_ldos_1_2_3_ops, TPS6594_MASK_LDO123_VSET,
 			  TPS6594_REG_LDOX_VOUT(0),
@@ -346,6 +466,30 @@ static const struct regulator_desc ldo_regs[] = {
 			  1, 0, 0, NULL, 0, 0),
 };
 
+static const struct regulator_desc tps65224_ldo_regs[] = {
+	TPS6594_REGULATOR("LDO1", "ldo1", TPS6594_LDO_1,
+			  REGULATOR_VOLTAGE, tps6594_ldos_1_2_3_ops, TPS6594_MASK_LDO123_VSET,
+			  TPS6594_REG_LDOX_VOUT(0),
+			  TPS6594_MASK_LDO123_VSET,
+			  TPS6594_REG_LDOX_CTRL(0),
+			  TPS6594_BIT_LDO_EN, 0, 0, tps65224_ldos_1_ranges,
+			  1, 0, 0, NULL, 0, TPS6594_BIT_LDO_BYPASS),
+	TPS6594_REGULATOR("LDO2", "ldo2", TPS6594_LDO_2,
+			  REGULATOR_VOLTAGE, tps6594_ldos_1_2_3_ops, TPS6594_MASK_LDO123_VSET,
+			  TPS6594_REG_LDOX_VOUT(1),
+			  TPS6594_MASK_LDO123_VSET,
+			  TPS6594_REG_LDOX_CTRL(1),
+			  TPS6594_BIT_LDO_EN, 0, 0, tps65224_ldos_2_3_ranges,
+			  1, 0, 0, NULL, 0, TPS6594_BIT_LDO_BYPASS),
+	TPS6594_REGULATOR("LDO3", "ldo3", TPS6594_LDO_3,
+			  REGULATOR_VOLTAGE, tps6594_ldos_1_2_3_ops, TPS6594_MASK_LDO123_VSET,
+			  TPS6594_REG_LDOX_VOUT(2),
+			  TPS6594_MASK_LDO123_VSET,
+			  TPS6594_REG_LDOX_CTRL(2),
+			  TPS6594_BIT_LDO_EN, 0, 0, tps65224_ldos_2_3_ranges,
+			  1, 0, 0, NULL, 0, TPS6594_BIT_LDO_BYPASS),
+};
+
 static irqreturn_t tps6594_regulator_irq_handler(int irq, void *data)
 {
 	struct tps6594_regulator_irq_data *irq_data = data;
@@ -369,17 +513,18 @@ static irqreturn_t tps6594_regulator_irq_handler(int irq, void *data)
 static int tps6594_request_reg_irqs(struct platform_device *pdev,
 				    struct regulator_dev *rdev,
 				    struct tps6594_regulator_irq_data *irq_data,
-				    struct tps6594_regulator_irq_type *tps6594_regs_irq_types,
+				    struct tps6594_regulator_irq_type *regs_irq_types,
+				    size_t interrupt_cnt,
 				    int *irq_idx)
 {
 	struct tps6594_regulator_irq_type *irq_type;
 	struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent);
-	int j;
+	size_t j;
 	int irq;
 	int error;
 
-	for (j = 0; j < REGS_INT_NB; j++) {
-		irq_type = &tps6594_regs_irq_types[j];
+	for (j = 0; j < interrupt_cnt; j++) {
+		irq_type = &regs_irq_types[j];
 		irq = platform_get_irq_byname(pdev, irq_type->irq_name);
 		if (irq < 0)
 			return -EINVAL;
@@ -412,14 +557,38 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 	struct tps6594_ext_regulator_irq_data *irq_ext_reg_data;
 	struct tps6594_regulator_irq_type *irq_type;
 	u8 buck_configured[BUCK_NB] = { 0 };
+	u8 ldo_configured[LDO_NB] = { 0 };
 	u8 buck_multi[MULTI_PHASE_NB] = { 0 };
-	static const char * const multiphases[] = {"buck12", "buck123", "buck1234", "buck34"};
+	static const char * const tps6594_multiphases[] = {"buck12", "buck123",
+							   "buck1234", "buck34"};
+	static const char * const tps65224_multiphases[] = {"buck12"};
 	static const char *npname;
 	int error, i, irq, multi, delta;
 	int irq_idx = 0;
 	int buck_idx = 0;
+	unsigned int multi_phase_cnt = 0;
 	size_t ext_reg_irq_nb = 2;
 	size_t reg_irq_nb;
+	struct tps6594_regulator_irq_type **bucks_irq_types;
+	const struct regulator_desc *multi_regs;
+	struct tps6594_regulator_irq_type **ldos_irq_types;
+	const struct regulator_desc *ldo_regs;
+	size_t interrupt_count;
+
+	if (tps->chip_id == TPS65224) {
+		bucks_irq_types = tps65224_bucks_irq_types;
+		interrupt_count = ARRAY_SIZE(tps65224_buck1_irq_types);
+		multi_regs = tps65224_multi_regs;
+		ldos_irq_types = tps65224_ldos_irq_types;
+		ldo_regs = tps65224_ldo_regs;
+	} else {
+		bucks_irq_types = tps6594_bucks_irq_types;
+		interrupt_count = ARRAY_SIZE(tps6594_buck1_irq_types);
+		multi_regs = tps6594_multi_regs;
+		ldos_irq_types = tps6594_ldos_irq_types;
+		ldo_regs = tps6594_ldo_regs;
+	}
+
 	enum {
 		MULTI_BUCK12,
 		MULTI_BUCK123,
@@ -434,6 +603,10 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 	config.driver_data = tps;
 	config.regmap = tps->regmap;
 
+	multi_phase_cnt = (tps->chip_id == TPS65224) ?
+			   ARRAY_SIZE(tps65224_multiphases) :
+			   ARRAY_SIZE(tps6594_multiphases);
+
 	/*
 	 * Switch case defines different possible multi phase config
 	 * This is based on dts buck node name.
@@ -442,13 +615,13 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 	 * In case of Multiphase configuration, value should be defined for
 	 * buck_configured to avoid creating bucks for every buck in multiphase
 	 */
-	for (multi = MULTI_FIRST; multi < MULTI_NUM; multi++) {
-		np = of_find_node_by_name(tps->dev->of_node, multiphases[multi]);
+	for (multi = MULTI_FIRST; multi < multi_phase_cnt; multi++) {
+		np = of_find_node_by_name(tps->dev->of_node, tps6594_multiphases[multi]);
 		npname = of_node_full_name(np);
 		np_pmic_parent = of_get_parent(of_get_parent(np));
 		if (of_node_cmp(of_node_full_name(np_pmic_parent), tps->dev->of_node->full_name))
 			continue;
-		delta = strcmp(npname, multiphases[multi]);
+		delta = strcmp(npname, tps6594_multiphases[multi]);
 		if (!delta) {
 			switch (multi) {
 			case MULTI_BUCK12:
@@ -486,6 +659,11 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 		/* There is only 4 buck on LP8764 */
 		buck_configured[4] = 1;
 		reg_irq_nb = size_mul(REGS_INT_NB, (BUCK_NB - 1));
+	} else if (tps->chip_id == TPS65224) {
+		/* TPS65224 has 4 bucks and 3 LDOs. 1 Interrupt for each buck and ldo */
+		buck_configured[4] = 1;
+		ldo_configured[3] = 1;
+		reg_irq_nb = size_mul(1, (size_add((BUCK_NB - 1), (LDO_NB - 1))));
 	} else {
 		reg_irq_nb = size_mul(REGS_INT_NB, (size_add(BUCK_NB, LDO_NB)));
 	}
@@ -495,7 +673,7 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 	if (!irq_data)
 		return -ENOMEM;
 
-	for (i = 0; i < MULTI_PHASE_NB; i++) {
+	for (i = 0; i < multi_phase_cnt; i++) {
 		if (buck_multi[i] == 0)
 			continue;
 
@@ -508,18 +686,23 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 		/* config multiphase buck12+buck34 */
 		if (i == 1)
 			buck_idx = 2;
+
 		error = tps6594_request_reg_irqs(pdev, rdev, irq_data,
-						 tps6594_bucks_irq_types[buck_idx], &irq_idx);
+						 bucks_irq_types[buck_idx],
+						 interrupt_count, &irq_idx);
 		if (error)
 			return error;
+
 		error = tps6594_request_reg_irqs(pdev, rdev, irq_data,
-						 tps6594_bucks_irq_types[buck_idx + 1], &irq_idx);
+						 bucks_irq_types[buck_idx + 1],
+						 interrupt_count, &irq_idx);
 		if (error)
 			return error;
 
 		if (i == 2 || i == 3) {
 			error = tps6594_request_reg_irqs(pdev, rdev, irq_data,
 							 tps6594_bucks_irq_types[buck_idx + 2],
+							 interrupt_count,
 							 &irq_idx);
 			if (error)
 				return error;
@@ -527,6 +710,7 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 		if (i == 3) {
 			error = tps6594_request_reg_irqs(pdev, rdev, irq_data,
 							 tps6594_bucks_irq_types[buck_idx + 3],
+							 interrupt_count,
 							 &irq_idx);
 			if (error)
 				return error;
@@ -537,21 +721,26 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 		if (buck_configured[i] == 1)
 			continue;
 
-		rdev = devm_regulator_register(&pdev->dev, &buck_regs[i], &config);
+		const struct regulator_desc *buck_cfg = (tps->chip_id == TPS65224) ?
+							 tps65224_buck_regs : buck_regs;
+
+		rdev = devm_regulator_register(&pdev->dev, &buck_cfg[i], &config);
 		if (IS_ERR(rdev))
 			return dev_err_probe(tps->dev, PTR_ERR(rdev),
-					     "failed to register %s regulator\n",
-					     pdev->name);
+					     "failed to register %s regulator\n", pdev->name);
 
 		error = tps6594_request_reg_irqs(pdev, rdev, irq_data,
-						 tps6594_bucks_irq_types[i], &irq_idx);
+						 bucks_irq_types[i], interrupt_count, &irq_idx);
 		if (error)
 			return error;
 	}
 
-	/* LP8764 dosen't have LDO */
+	/* LP8764 doesn't have LDO */
 	if (tps->chip_id != LP8764) {
-		for (i = 0; i < ARRAY_SIZE(ldo_regs); i++) {
+		for (i = 0; i < LDO_NB; i++) {
+			if (ldo_configured[i] == 1)
+				continue;
+
 			rdev = devm_regulator_register(&pdev->dev, &ldo_regs[i], &config);
 			if (IS_ERR(rdev))
 				return dev_err_probe(tps->dev, PTR_ERR(rdev),
@@ -559,7 +748,7 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 						     pdev->name);
 
 			error = tps6594_request_reg_irqs(pdev, rdev, irq_data,
-							 tps6594_ldos_irq_types[i],
+							 ldos_irq_types[i], interrupt_count,
 							 &irq_idx);
 			if (error)
 				return error;
@@ -568,16 +757,21 @@ static int tps6594_regulator_probe(struct platform_device *pdev)
 
 	if (tps->chip_id == LP8764)
 		ext_reg_irq_nb = ARRAY_SIZE(tps6594_ext_regulator_irq_types);
+	else if (tps->chip_id == TPS65224)
+		ext_reg_irq_nb = ARRAY_SIZE(tps65224_ext_regulator_irq_types);
 
 	irq_ext_reg_data = devm_kmalloc_array(tps->dev,
-					ext_reg_irq_nb,
-					sizeof(struct tps6594_ext_regulator_irq_data),
-					GFP_KERNEL);
+					      ext_reg_irq_nb,
+					      sizeof(struct tps6594_ext_regulator_irq_data),
+					      GFP_KERNEL);
 	if (!irq_ext_reg_data)
 		return -ENOMEM;
 
 	for (i = 0; i < ext_reg_irq_nb; ++i) {
-		irq_type = &tps6594_ext_regulator_irq_types[i];
+		if (tps->chip_id == TPS65224)
+			irq_type = &tps65224_ext_regulator_irq_types[i];
+		else
+			irq_type = &tps6594_ext_regulator_irq_types[i];
 
 		irq = platform_get_irq_byname(pdev, irq_type->irq_name);
 		if (irq < 0)
@@ -610,5 +804,6 @@ module_platform_driver(tps6594_regulator_driver);
 
 MODULE_ALIAS("platform:tps6594-regulator");
 MODULE_AUTHOR("Jerome Neanne <jneanne@baylibre.com>");
+MODULE_AUTHOR("Nirmala Devi Mal Nadar <m.nirmaladevi@ltts.com>");
 MODULE_DESCRIPTION("TPS6594 voltage regulator driver");
 MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* Re: [RESEND v3 2/2] ASoC: qcom: sc8280xp: Add support for QCM6490 and QCS6490
From: Bjorn Andersson @ 2024-04-08 12:45 UTC (permalink / raw)
  To: Mohammad Rafi Shaik
  Cc: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
	Takashi Iwai, linux-arm-msm, alsa-devel, linux-sound, devicetree,
	linux-kernel, quic_rohkumar
In-Reply-To: <20240408042331.403103-3-quic_mohs@quicinc.com>

On Mon, Apr 08, 2024 at 09:53:31AM +0530, Mohammad Rafi Shaik wrote:
> Add compatibles for sound card on Qualcomm QCM6490 IDP and
> QCS6490 RB3Gen2 boards.
> 
> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
> ---
>  sound/soc/qcom/sc8280xp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
> index b7fd503a1666..878bd50ad4a7 100644
> --- a/sound/soc/qcom/sc8280xp.c
> +++ b/sound/soc/qcom/sc8280xp.c
> @@ -169,6 +169,8 @@ static int sc8280xp_platform_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id snd_sc8280xp_dt_match[] = {
> +	{.compatible = "qcom,qcm6490-idp-sndcard", "qcm6490"},
> +	{.compatible = "qcom,qcs6490-rb3gen2-sndcard", "qcs6490"},

We now have 4 <platform>-sndcard and two <board>-sndcard compatibles
here.

Not saying that your patch is wrong, but is this driver board-specific
or soc-specific? Srinivas, Krzysztof?

Regards,
Bjorn

>  	{.compatible = "qcom,sc8280xp-sndcard", "sc8280xp"},
>  	{.compatible = "qcom,sm8450-sndcard", "sm8450"},
>  	{.compatible = "qcom,sm8550-sndcard", "sm8550"},
> -- 
> 2.25.1
> 
> 

^ permalink raw reply

* [PATCH 1/1] arm64: dts: rockchip: enable onboard spi flash for rock-3a
From: Chukun Pan @ 2024-04-08 12:40 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Conor Dooley, Krzysztof Kozlowski, Rob Herring, linux-arm-kernel,
	linux-rockchip, linux-kernel, devicetree, Chukun Pan

There is a mx25u12835f spi flash on this board, enable it.

[    2.525805] spi-nor spi4.0: mx25u12835f (16384 Kbytes)

Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index a5e974ea659e..d8738cc47c73 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -757,6 +757,18 @@ &sdmmc2 {
 	status = "okay";
 };
 
+&sfc {
+	status = "okay";
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0x0>;
+		spi-max-frequency = <50000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <1>;
+	};
+};
+
 &tsadc {
 	rockchip,hw-tshut-mode = <1>;
 	rockchip,hw-tshut-polarity = <0>;
-- 
2.25.1


^ permalink raw reply related

* Re: [RESEND v3 2/2] ASoC: qcom: sc8280xp: Add support for QCM6490 and QCS6490
From: Krzysztof Kozlowski @ 2024-04-08 12:48 UTC (permalink / raw)
  To: Bjorn Andersson, Mohammad Rafi Shaik
  Cc: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
	Takashi Iwai, linux-arm-msm, alsa-devel, linux-sound, devicetree,
	linux-kernel, quic_rohkumar
In-Reply-To: <45yy3cvepk4uwi2jdmh4w6l5ac3dffqhbot6xzv2bwjvo637ss@pryzth2hghyu>

On 08/04/2024 14:45, Bjorn Andersson wrote:
> On Mon, Apr 08, 2024 at 09:53:31AM +0530, Mohammad Rafi Shaik wrote:
>> Add compatibles for sound card on Qualcomm QCM6490 IDP and
>> QCS6490 RB3Gen2 boards.
>>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> ---
>>  sound/soc/qcom/sc8280xp.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
>> index b7fd503a1666..878bd50ad4a7 100644
>> --- a/sound/soc/qcom/sc8280xp.c
>> +++ b/sound/soc/qcom/sc8280xp.c
>> @@ -169,6 +169,8 @@ static int sc8280xp_platform_probe(struct platform_device *pdev)
>>  }
>>  
>>  static const struct of_device_id snd_sc8280xp_dt_match[] = {
>> +	{.compatible = "qcom,qcm6490-idp-sndcard", "qcm6490"},
>> +	{.compatible = "qcom,qcs6490-rb3gen2-sndcard", "qcs6490"},
> 
> We now have 4 <platform>-sndcard and two <board>-sndcard compatibles
> here.
> 
> Not saying that your patch is wrong, but is this driver board-specific
> or soc-specific? Srinivas, Krzysztof?

To my understanding this is mostly board specific, but it is also
partially a software construct thus we simplify it by using SoC
compatible for all boards with given SoC.

I have no clue whether this applies here. I think we asked about
clarifying the differences, but as you can see nothing improved in the
commit msg.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Ondřej Jirman @ 2024-04-08 12:48 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Pavel Machek, phone-devel, kernel list, fiona.klute, martijn,
	samuel, heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree
In-Reply-To: <7976e254-ed1e-406d-870b-1ecdc4b1e23c@linaro.org>

On Mon, Apr 08, 2024 at 01:59:12PM GMT, Krzysztof Kozlowski wrote:
> On 08/04/2024 13:52, Ondřej Jirman wrote:
> > On Mon, Apr 08, 2024 at 01:24:03PM GMT, Krzysztof Kozlowski wrote:
> >> On 08/04/2024 13:21, Pavel Machek wrote:
> >>> Hi!
> >>>
> >>>>> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> >>>>> but I did best I could.
> >>>>>
> >>>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> >>>>
> >>>> ...
> >>>>
> >>>>> +  cabledet-gpios:
> >>>>> +    maxItems: 1
> >>>>> +    description: GPIO controlling CABLE_DET (C3) pin.
> >>>>> +
> >>>>> +  avdd10-supply:
> >>>>> +    description: 1.0V power supply going to AVDD10 (A4, ...) pins
> >>>>> +
> >>>>> +  dvdd10-supply:
> >>>>> +    description: 1.0V power supply going to DVDD10 (D6, ...) pins
> >>>>> +
> >>>>> +  avdd18-supply:
> >>>>> +    description: 1.8V power supply going to AVDD18 (E3, ...) pins
> >>>>> +
> >>>>> +  dvdd18-supply:
> >>>>> +    description: 1.8V power supply going to DVDD18 (G4, ...) pins
> >>>>> +
> >>>>> +  avdd33-supply:
> >>>>> +    description: 3.3V power supply going to AVDD33 (C4, ...) pins
> >>>>> +
> >>>>> +  i2c-supply: true
> >>>>> +  vconn-supply: true
> >>>>
> >>>> There are no such supplies like i2c and vconn on the schematics.
> >>>>
> >>>> I think this represents some other part of component which was added
> >>>> here only for convenience.
> >>>
> >>> Can you give me pointer to documentation you are looking at?
> >>
> >> The schematics you linked in the document at the beginning. Page 13. Do
> >> you see these pins there? I saw only VCONN1_EN, but that's not a supply.
> > 
> > The supply is U1308.
> 
> That's not a supply to anx7688.

Yeah, I understand where the confusion is. The driver is not for anx7688 chip
really. The driver is named anx7688, but that's mostly a historical accident at
this point.

I guess there can be a driver for anx7688 chip that can directly use the chip's
resources from the host by directly manipulating its registers and implementing
type-c functionality via eg. Linux's TCPM or TCPCI stack, etc. (eg. like
fusb302 driver, or various tcpci subdrivers).

But in this case the chip is driven by an optional on-chip microcontroller's
firmware and *this driver* is specifically for *the Type-C port on Pinephone*
and serves as an integration driver for quite a bunch of things that need to
work together on Pinephone for all of the Type-C port's features to operate
reasonably well (and one of those is some communication with anx7688 firmware
that we use, and enabling power to this chip and other things as appropriate,
based on the communication from the firmware).

It handles the specific needs of the Pinephone's Type-C implementation, all of
its quirks (of which there are many over several HW revisions) that can't be
handled by the particular implementation of on-chip microcontroller firmware
directly and need host side interaction.

In an ideal world, many of the things this driver handles would be handled by
embedded microcontroller on the board (like it is with some RK3399 based Google
devices), but Pinephone has no such thing and this glue needs to be implemented
somewhere in the kernel.

Kind regards,
	o.

> Best regards,
> Krzysztof
> 

^ permalink raw reply

* Re: [RESEND v3 2/2] ASoC: qcom: sc8280xp: Add support for QCM6490 and QCS6490
From: Srinivas Kandagatla @ 2024-04-08 12:53 UTC (permalink / raw)
  To: Bjorn Andersson, Mohammad Rafi Shaik
  Cc: Banajit Goswami, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai,
	linux-arm-msm, alsa-devel, linux-sound, devicetree, linux-kernel,
	quic_rohkumar
In-Reply-To: <45yy3cvepk4uwi2jdmh4w6l5ac3dffqhbot6xzv2bwjvo637ss@pryzth2hghyu>



On 08/04/2024 13:45, Bjorn Andersson wrote:
> On Mon, Apr 08, 2024 at 09:53:31AM +0530, Mohammad Rafi Shaik wrote:
>> Add compatibles for sound card on Qualcomm QCM6490 IDP and
>> QCS6490 RB3Gen2 boards.
>>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> ---
>>   sound/soc/qcom/sc8280xp.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
>> index b7fd503a1666..878bd50ad4a7 100644
>> --- a/sound/soc/qcom/sc8280xp.c
>> +++ b/sound/soc/qcom/sc8280xp.c
>> @@ -169,6 +169,8 @@ static int sc8280xp_platform_probe(struct platform_device *pdev)
>>   }
>>   
>>   static const struct of_device_id snd_sc8280xp_dt_match[] = {
>> +	{.compatible = "qcom,qcm6490-idp-sndcard", "qcm6490"},
>> +	{.compatible = "qcom,qcs6490-rb3gen2-sndcard", "qcs6490"},
> 
> We now have 4 <platform>-sndcard and two <board>-sndcard compatibles
> here.
> 
> Not saying that your patch is wrong, but is this driver board-specific
> or soc-specific? Srinivas, Krzysztof?

Normally this should be board specific.

In the past we made them SoC specific and provided a way to do board 
specific changes based on compatible. Recently we stopped adding new 
drivers as most of these drivers turned out to be identical and lots of 
code duplication.

Having these compatibles will help both the userspace UCM and provide 
hooks to do any board/soc specific configurations.

thanks,
Srini

> 
> Regards,
> Bjorn
> 
>>   	{.compatible = "qcom,sc8280xp-sndcard", "sc8280xp"},
>>   	{.compatible = "qcom,sm8450-sndcard", "sm8450"},
>>   	{.compatible = "qcom,sm8550-sndcard", "sm8550"},
>> -- 
>> 2.25.1
>>
>>

^ permalink raw reply

* Re: [PATCH v10 1/2] dt-bindings: mtd: fixed-partitions: Add alignment properties
From: Miquel Raynal @ 2024-04-08 13:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: devicetree, Michael Walle, U-Boot Mailing List, Tom Rini,
	Rob Herring, linux-mtd, Conor Dooley, Krzysztof Kozlowski,
	Rafał Miłecki, Richard Weinberger, Vignesh Raghavendra,
	linux-kernel
In-Reply-To: <20240326200645.1182803-1-sjg@chromium.org>

Hi Simon,

sjg@chromium.org wrote on Tue, 26 Mar 2024 14:06:44 -0600:

> Add three properties for controlling alignment of partitions, aka
> 'entries' in fixed-partition.
> 
> For now there is no explicit mention of hierarchy, so a 'section' is
> just the 'fixed-partitions' node.
> 
> These new properties are inputs to the Binman packaging process, but are
> also needed if the firmware is repacked, to ensure that alignment
> constraints are not violated. Therefore they are provided as part of
> the schema.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> 
> Changes in v10:
> - Update the minimum to 2
> 
> Changes in v9:
> - Move binding example to next batch to avoid build error
> 
> Changes in v7:
> - Drop patch 'Add binman compatible'
> - Put the alignment properties into the fixed-partition binding
> 
> Changes in v6:
> - Correct schema-validation errors missed due to older dt-schema
>   (enum fix and reg addition)
> 
> Changes in v5:
> - Add value ranges
> - Consistently mention alignment must be power-of-2
> - Mention that alignment refers to bytes
> 
> Changes in v2:
> - Fix 'a' typo in commit message
> 
>  .../bindings/mtd/partitions/partition.yaml    | 51 +++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/partition.yaml b/Documentation/devicetree/bindings/mtd/partitions/partition.yaml
> index 1ebe9e2347ea..656ca3db1762 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/partition.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/partition.yaml
> @@ -57,6 +57,57 @@ properties:
>        user space from
>      type: boolean
>  
> +  align:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 2
> +    maximum: 0x80000000
> +    multipleOf: 2
> +    description:
> +      This sets the alignment of the entry in bytes.
> +
> +      The entry offset is adjusted so that the entry starts on an aligned
> +      boundary within the containing section or image. For example ‘align =
> +      <16>’ means that the entry will start on a 16-byte boundary. This may
> +      mean that padding is added before the entry. The padding is part of
> +      the containing section but is not included in the entry, meaning that
> +      an empty space may be created before the entry starts. Alignment
> +      must be a power of 2. If ‘align’ is not provided, no alignment is
> +      performed.
> +
> +  align-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 2
> +    maximum: 0x80000000
> +    multipleOf: 2
> +    description:
> +      This sets the alignment of the entry size in bytes. It must be a power
> +      of 2.
> +
> +      For example, to ensure that the size of an entry is a multiple of 64
> +      bytes, set this to 64. While this does not affect the contents of the
> +      entry within binman itself (the padding is performed only when its
> +      parent section is assembled), the end result is that the entry ends
> +      with the padding bytes, so may grow. If ‘align-size’ is not provided,
> +      no alignment is performed.

I don't think we should mention binman here. Can we have a software
agnostic description? This should be understandable from anyone playing
with mtd partitions I guess.

> +
> +  align-end:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 2
> +    maximum: 0x80000000
> +    multipleOf: 2

seems not to perfectly match the constraint, but I don't know if there
is a powerOf keyword? (same above)

> +    description:
> +      This sets the alignment (in bytes) of the end of an entry with respect
> +      to the containing section. It must be a power of 2.
> +
> +      Some entries require that they end on an alignment boundary,
> +      regardless of where they start. This does not move the start of the
> +      entry, so the contents of the entry will still start at the beginning.
> +      But there may be padding at the end. While this does not affect the
> +      contents of the entry within binman itself (the padding is performed

content?				same comment about binman?

> +      only when its parent section is assembled), the end result is that the
> +      entry ends with the padding bytes, so may grow. If ‘align-end’ is not
> +      provided, no alignment is performed.
> +
>  if:
>    not:
>      required: [ reg ]


Thanks,
Miquèl

^ permalink raw reply

* [PATCH v2 0/2] Bluetooth: btnxpuart: Update firmware names
From: Neeraj Sanjay Kale @ 2024-04-08 13:22 UTC (permalink / raw)
  To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
	amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
	ziniu.wang_1, haibo.chen, LnxRevLi

This patch series updates the BT firmware file names in BTNXPUART
driver, and adds a new optional firmware-name device tree property to
override the firmware file names hardcoded in the driver. This will
allow user to continue using the older firmware files.

This change is necessary as newer firmware releases will have
standardized naming convention aligned across all newer and legacy
chipsets.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>

Neeraj Sanjay Kale (2):
  dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
  Bluetooth: btnxpuart: Update firmware names

 .../net/bluetooth/nxp,88w8987-bt.yaml         |  6 ++++
 drivers/bluetooth/btnxpuart.c                 | 28 +++++++++++--------
 2 files changed, 23 insertions(+), 11 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH v2 1/2] dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
From: Neeraj Sanjay Kale @ 2024-04-08 13:22 UTC (permalink / raw)
  To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
	amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
	ziniu.wang_1, haibo.chen, LnxRevLi
In-Reply-To: <20240408132241.750792-1-neeraj.sanjaykale@nxp.com>

This adds a new optional device tree property called firware-name.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
v2: Add maxItems, simplify description, remove "nxp/". (Krzysztof)
---
 .../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml   | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
index f01a3988538c..6774cc4d6a9e 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
@@ -31,6 +31,11 @@ properties:
       This property depends on the module vendor's
       configuration.
 
+  firmware-name:
+    maxItems: 1
+    description:
+      Specify firmware file name.
+
 required:
   - compatible
 
@@ -42,5 +47,6 @@ examples:
         bluetooth {
             compatible = "nxp,88w8987-bt";
             fw-init-baudrate = <3000000>;
+            firmware-name = "uartuart8987_bt_v0.bin";
         };
     };
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 2/2] Bluetooth: btnxpuart: Update firmware names
From: Neeraj Sanjay Kale @ 2024-04-08 13:22 UTC (permalink / raw)
  To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
	amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
	ziniu.wang_1, haibo.chen, LnxRevLi
In-Reply-To: <20240408132241.750792-1-neeraj.sanjaykale@nxp.com>

This updates the firmware names of 3 chipsets: w8987, w8997, w9098.
These changes are been done to standardize chip specific firmware
file names.
To allow user to use older firmware file names, a new device tree
property has been introduced called firmware-name, which will override
the hardcoded firmware names in the driver.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
v2: Remove "nxp/" from all firmware name definitions to be inline with
firware file name read from device tree file. (Krzysztof)
---
 drivers/bluetooth/btnxpuart.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 0b93c2ff29e4..4442d911eba8 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -33,16 +33,16 @@
 /* NXP HW err codes */
 #define BTNXPUART_IR_HW_ERR		0xb0
 
-#define FIRMWARE_W8987		"nxp/uartuart8987_bt.bin"
-#define FIRMWARE_W8997		"nxp/uartuart8997_bt_v4.bin"
-#define FIRMWARE_W9098		"nxp/uartuart9098_bt_v1.bin"
-#define FIRMWARE_IW416		"nxp/uartiw416_bt_v0.bin"
-#define FIRMWARE_IW612		"nxp/uartspi_n61x_v1.bin.se"
-#define FIRMWARE_IW624		"nxp/uartiw624_bt.bin"
-#define FIRMWARE_SECURE_IW624	"nxp/uartiw624_bt.bin.se"
-#define FIRMWARE_AW693		"nxp/uartaw693_bt.bin"
-#define FIRMWARE_SECURE_AW693	"nxp/uartaw693_bt.bin.se"
-#define FIRMWARE_HELPER		"nxp/helper_uart_3000000.bin"
+#define FIRMWARE_W8987		"uart8987_bt_v0.bin"
+#define FIRMWARE_W8997		"uart8997_bt_v4.bin"
+#define FIRMWARE_W9098		"uart9098_bt_v1.bin"
+#define FIRMWARE_IW416		"uartiw416_bt_v0.bin"
+#define FIRMWARE_IW612		"uartspi_n61x_v1.bin.se"
+#define FIRMWARE_IW624		"uartiw624_bt.bin"
+#define FIRMWARE_SECURE_IW624	"uartiw624_bt.bin.se"
+#define FIRMWARE_AW693		"uartaw693_bt.bin"
+#define FIRMWARE_SECURE_AW693	"uartaw693_bt.bin.se"
+#define FIRMWARE_HELPER		"helper_uart_3000000.bin"
 
 #define CHIP_ID_W9098		0x5c03
 #define CHIP_ID_IW416		0x7201
@@ -685,13 +685,19 @@ static bool process_boot_signature(struct btnxpuart_dev *nxpdev)
 static int nxp_request_firmware(struct hci_dev *hdev, const char *fw_name)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
+	const char *fw_name_dt;
 	int err = 0;
 
 	if (!fw_name)
 		return -ENOENT;
 
 	if (!strlen(nxpdev->fw_name)) {
-		snprintf(nxpdev->fw_name, MAX_FW_FILE_NAME_LEN, "%s", fw_name);
+		if (strcmp(fw_name, FIRMWARE_HELPER) &&
+		    !device_property_read_string(&nxpdev->serdev->dev,
+						 "firmware-name",
+						 &fw_name_dt))
+			fw_name = fw_name_dt;
+		snprintf(nxpdev->fw_name, MAX_FW_FILE_NAME_LEN, "nxp/%s", fw_name);
 
 		bt_dev_dbg(hdev, "Request Firmware: %s", nxpdev->fw_name);
 		err = request_firmware(&nxpdev->fw, nxpdev->fw_name, &hdev->dev);
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Krzysztof Kozlowski @ 2024-04-08 13:27 UTC (permalink / raw)
  To: Ondřej Jirman, Pavel Machek, phone-devel, kernel list,
	fiona.klute, martijn, samuel, heikki.krogerus, gregkh, linux-usb,
	robh+dt, krzysztof.kozlowski+dt, devicetree
In-Reply-To: <uoo7xltbfx7u6iai7urj3wez7cwotokxt6lwjhff57xbljusqn@fr2xejnrlak7>

On 08/04/2024 14:48, Ondřej Jirman wrote:
> On Mon, Apr 08, 2024 at 01:59:12PM GMT, Krzysztof Kozlowski wrote:
>> On 08/04/2024 13:52, Ondřej Jirman wrote:
>>> On Mon, Apr 08, 2024 at 01:24:03PM GMT, Krzysztof Kozlowski wrote:
>>>> On 08/04/2024 13:21, Pavel Machek wrote:
>>>>> Hi!
>>>>>
>>>>>>> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
>>>>>>> but I did best I could.
>>>>>>>
>>>>>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>> +  cabledet-gpios:
>>>>>>> +    maxItems: 1
>>>>>>> +    description: GPIO controlling CABLE_DET (C3) pin.
>>>>>>> +
>>>>>>> +  avdd10-supply:
>>>>>>> +    description: 1.0V power supply going to AVDD10 (A4, ...) pins
>>>>>>> +
>>>>>>> +  dvdd10-supply:
>>>>>>> +    description: 1.0V power supply going to DVDD10 (D6, ...) pins
>>>>>>> +
>>>>>>> +  avdd18-supply:
>>>>>>> +    description: 1.8V power supply going to AVDD18 (E3, ...) pins
>>>>>>> +
>>>>>>> +  dvdd18-supply:
>>>>>>> +    description: 1.8V power supply going to DVDD18 (G4, ...) pins
>>>>>>> +
>>>>>>> +  avdd33-supply:
>>>>>>> +    description: 3.3V power supply going to AVDD33 (C4, ...) pins
>>>>>>> +
>>>>>>> +  i2c-supply: true
>>>>>>> +  vconn-supply: true
>>>>>>
>>>>>> There are no such supplies like i2c and vconn on the schematics.
>>>>>>
>>>>>> I think this represents some other part of component which was added
>>>>>> here only for convenience.
>>>>>
>>>>> Can you give me pointer to documentation you are looking at?
>>>>
>>>> The schematics you linked in the document at the beginning. Page 13. Do
>>>> you see these pins there? I saw only VCONN1_EN, but that's not a supply.
>>>
>>> The supply is U1308.
>>
>> That's not a supply to anx7688.
> 
> Yeah, I understand where the confusion is. The driver is not for anx7688 chip
> really. The driver is named anx7688, but that's mostly a historical accident at
> this point.
> 
> I guess there can be a driver for anx7688 chip that can directly use the chip's
> resources from the host by directly manipulating its registers and implementing
> type-c functionality via eg. Linux's TCPM or TCPCI stack, etc. (eg. like
> fusb302 driver, or various tcpci subdrivers).
> 
> But in this case the chip is driven by an optional on-chip microcontroller's
> firmware and *this driver* is specifically for *the Type-C port on Pinephone*

We do not talk here about the driver, but bindings, so hardware.

> and serves as an integration driver for quite a bunch of things that need to
> work together on Pinephone for all of the Type-C port's features to operate
> reasonably well (and one of those is some communication with anx7688 firmware
> that we use, and enabling power to this chip and other things as appropriate,
> based on the communication from the firmware).

That's still looking like putting board design into particular device
binding.

> 
> It handles the specific needs of the Pinephone's Type-C implementation, all of
> its quirks (of which there are many over several HW revisions) that can't be
> handled by the particular implementation of on-chip microcontroller firmware
> directly and need host side interaction.
> 
> In an ideal world, many of the things this driver handles would be handled by
> embedded microcontroller on the board (like it is with some RK3399 based Google
> devices), but Pinephone has no such thing and this glue needs to be implemented
> somewhere in the kernel.

You might need multiple schemas, because this is for anx7688, not for
Pinephone type-c implementation.

However I still do not see yet a limitation of DTS requiring stuffing
some other properties into anx7688 or creating some other, virtual entity.


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 2/4] coresight: Add support for multiple output ports on the funnel
From: Mike Leach @ 2024-04-08 13:28 UTC (permalink / raw)
  To: Tao Zhang
  Cc: Suzuki K Poulose, Mathieu Poirier, Alexander Shishkin,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Jinlong Mao,
	Leo Yan, Greg Kroah-Hartman, coresight, linux-arm-kernel,
	linux-kernel, devicetree, Tingwei Zhang, Yuanfang Zhang,
	Trilok Soni, Song Chai, linux-arm-msm, andersson
In-Reply-To: <ffce4577-b0f9-4af3-a379-0385a02ddae8@quicinc.com>

Hi Tao,

Using a DT label in this way is not how connections should be
described in device tree, The association needs to be between the
input and output ports of the component not and extra link back to the
source.

If your "funnel"  has multiple input and output ports, then it is no
longer a funnel.  As you describe it - the input ports are statically
associated with an output port, so the device is effectively a set of
individual funnels.
Given that the association between the inputs and outputs is known at
time of compilation - then these can be inferred from the components
compatible string and handled internally during driver probe.

This component should therefore really have its own driver, and not be
described as a standard  funnel. In this way you can handle the input
and output associations within the driver and create a path in the
normal way, avoiding the need for large changes to the coresight API.
Having a specific driver will allow creation of sets of associated
input and output connections.

Regards

Mike







On Fri, 29 Mar 2024 at 09:27, Tao Zhang <quic_taozha@quicinc.com> wrote:
>
>
> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
> > On 21/03/2024 08:32, Tao Zhang wrote:
> >> Funnel devices are now capable of supporting multiple-inputs and
> >> multiple-outputs configuration with in built hardware filtering
> >> for TPDM devices. Add software support to this function. Output
> >> port is selected according to the source in the trace path.
> >>
> >> The source of the input port on funnels will be marked in the
> >> device tree.
> >> e.g.
> >> tpdm@xxxxxxx {
> >>      ... ... ... ...
> >> };
> >>
> >> funnel_XXX: funnel@xxxxxxx {
> >>      ... ... ... ...
> >>      out-ports {
> >>          ... ... ... ...
> >>          port@x {
> >>              ... ... ... ...
> >>              label = "xxxxxxx.tpdm"; <-- To label the source
> >>          };                           corresponding to the output
> >>      ... ... ... ...                  connection "port@x". And this
> >>      };                               is a hardware static connections.
> >>      ... ... ... ...                  Here needs to refer to hardware
> >> };                                   design.
> >>
> >> Then driver will parse the source label marked in the device tree, and
> >> save it to the coresight path. When the function needs to know the
> >> source label, it could obtain it from coresight path parameter. Finally,
> >> the output port knows which source it corresponds to, and it also knows
> >> which input port it corresponds to.
> >
> > Why do we need labels ? We have connection information for all devices
> > (both in and out), so, why do we need this label to find a device ?
>
> Because our funnel's design has multi-output ports, the data stream will not
>
> know which output port should pass in building the data trace path. This
> source
>
> label can make the data stream find the right output port to go.
>
> >
> > And also, I thought TPDM is a source device, why does a funnel output
> > port link to a source ?
>
> No, this label doesn't mean this funnel output port link to a source, it
> just let
>
> the output port know its data source.
>
> >
> > Are these funnels programmable ? Or, are they static ? If they are
> > static, do these need to be described in the DT ? If they are simply
> > acting as a "LINK" (or HWFIFO ?)
>
> These funnels are static, and we will add the "label" to the DT to
> describe the
>
> multi-output ports for these funnels.
>
> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
> what's the meaning
>
> of this. Could you describe it in detail?
>
>
> Best,
>
> Tao
>
> >
> > Suzuki
> >
> >>
> >> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> >> ---
> >>   drivers/hwtracing/coresight/coresight-core.c  | 81 ++++++++++++++++---
> >>   .../hwtracing/coresight/coresight-platform.c  |  5 ++
> >>   include/linux/coresight.h                     |  2 +
> >>   3 files changed, 75 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-core.c
> >> b/drivers/hwtracing/coresight/coresight-core.c
> >> index 5dde597403b3..b1b5e6d9ec7a 100644
> >> --- a/drivers/hwtracing/coresight/coresight-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-core.c
> >> @@ -113,15 +113,63 @@ struct coresight_device
> >> *coresight_get_percpu_sink(int cpu)
> >>   }
> >>   EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
> >>   +static struct coresight_device *coresight_get_source(struct
> >> list_head *path)
> >> +{
> >> +    struct coresight_device *csdev;
> >> +
> >> +    if (!path)
> >> +        return NULL;
> >> +
> >> +    csdev = list_first_entry(path, struct coresight_node, link)->csdev;
> >> +    if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
> >> +        return NULL;
> >> +
> >> +    return csdev;
> >> +}
> >> +
> >> +/**
> >> + * coresight_source_filter - checks whether the connection matches
> >> the source
> >> + * of path if connection is binded to specific source.
> >> + * @path:    The list of devices
> >> + * @conn:    The connection of one outport
> >> + *
> >> + * Return zero if the connection doesn't have a source binded or
> >> source of the
> >> + * path matches the source binds to connection.
> >> + */
> >> +static int coresight_source_filter(struct list_head *path,
> >> +            struct coresight_connection *conn)
> >> +{
> >> +    int ret = 0;
> >> +    struct coresight_device *source = NULL;
> >> +
> >> +    if (conn->source_label == NULL)
> >> +        return ret;
> >> +
> >> +    source = coresight_get_source(path);
> >> +    if (source == NULL)
> >> +        return ret;
> >> +
> >> +    if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >> +            conn->source_label))
> >> +        ret = 0;
> >> +    else
> >> +        ret = -1;
> >> +
> >> +    return ret;
> >> +}
> >> +
> >>   static struct coresight_connection *
> >>   coresight_find_out_connection(struct coresight_device *src_dev,
> >> -                  struct coresight_device *dest_dev)
> >> +                  struct coresight_device *dest_dev,
> >> +                  struct list_head *path)
> >>   {
> >>       int i;
> >>       struct coresight_connection *conn;
> >>         for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> >>           conn = src_dev->pdata->out_conns[i];
> >> +        if (coresight_source_filter(path, conn))
> >> +            continue;
> >>           if (conn->dest_dev == dest_dev)
> >>               return conn;
> >>       }
> >> @@ -312,7 +360,8 @@ static void coresight_disable_sink(struct
> >> coresight_device *csdev)
> >>     static int coresight_enable_link(struct coresight_device *csdev,
> >>                    struct coresight_device *parent,
> >> -                 struct coresight_device *child)
> >> +                 struct coresight_device *child,
> >> +                 struct list_head *path)
> >>   {
> >>       int ret = 0;
> >>       int link_subtype;
> >> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
> >> coresight_device *csdev,
> >>       if (!parent || !child)
> >>           return -EINVAL;
> >>   -    inconn = coresight_find_out_connection(parent, csdev);
> >> -    outconn = coresight_find_out_connection(csdev, child);
> >> +    inconn = coresight_find_out_connection(parent, csdev, path);
> >> +    outconn = coresight_find_out_connection(csdev, child, path);
> >>       link_subtype = csdev->subtype.link_subtype;
> >>         if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
> >> IS_ERR(inconn))
> >> @@ -341,7 +390,8 @@ static int coresight_enable_link(struct
> >> coresight_device *csdev,
> >>     static void coresight_disable_link(struct coresight_device *csdev,
> >>                      struct coresight_device *parent,
> >> -                   struct coresight_device *child)
> >> +                   struct coresight_device *child,
> >> +                   struct list_head *path)
> >>   {
> >>       int i;
> >>       int link_subtype;
> >> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
> >> coresight_device *csdev,
> >>       if (!parent || !child)
> >>           return;
> >>   -    inconn = coresight_find_out_connection(parent, csdev);
> >> -    outconn = coresight_find_out_connection(csdev, child);
> >> +    inconn = coresight_find_out_connection(parent, csdev, path);
> >> +    outconn = coresight_find_out_connection(csdev, child, path);
> >>       link_subtype = csdev->subtype.link_subtype;
> >>         if (link_ops(csdev)->disable) {
> >> @@ -507,7 +557,7 @@ static void coresight_disable_path_from(struct
> >> list_head *path,
> >>           case CORESIGHT_DEV_TYPE_LINK:
> >>               parent = list_prev_entry(nd, link)->csdev;
> >>               child = list_next_entry(nd, link)->csdev;
> >> -            coresight_disable_link(csdev, parent, child);
> >> +            coresight_disable_link(csdev, parent, child, path);
> >>               break;
> >>           default:
> >>               break;
> >> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path,
> >> enum cs_mode mode,
> >>           case CORESIGHT_DEV_TYPE_LINK:
> >>               parent = list_prev_entry(nd, link)->csdev;
> >>               child = list_next_entry(nd, link)->csdev;
> >> -            ret = coresight_enable_link(csdev, parent, child);
> >> +            ret = coresight_enable_link(csdev, parent, child, path);
> >>               if (ret)
> >>                   goto err;
> >>               break;
> >> @@ -802,7 +852,8 @@ static void coresight_drop_device(struct
> >> coresight_device *csdev)
> >>    */
> >>   static int _coresight_build_path(struct coresight_device *csdev,
> >>                    struct coresight_device *sink,
> >> -                 struct list_head *path)
> >> +                 struct list_head *path,
> >> +                 struct coresight_device *source)
> >>   {
> >>       int i, ret;
> >>       bool found = false;
> >> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
> >> coresight_device *csdev,
> >>         if (coresight_is_percpu_source(csdev) &&
> >> coresight_is_percpu_sink(sink) &&
> >>           sink == per_cpu(csdev_sink,
> >> source_ops(csdev)->cpu_id(csdev))) {
> >> -        if (_coresight_build_path(sink, sink, path) == 0) {
> >> +        if (_coresight_build_path(sink, sink, path, source) == 0) {
> >>               found = true;
> >>               goto out;
> >>           }
> >> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
> >> coresight_device *csdev,
> >>           struct coresight_device *child_dev;
> >>             child_dev = csdev->pdata->out_conns[i]->dest_dev;
> >> +        if (csdev->pdata->out_conns[i]->source_label &&
> >> +            !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >> + csdev->pdata->out_conns[i]->source_label))
> >> +            continue;
> >>           if (child_dev &&
> >> -            _coresight_build_path(child_dev, sink, path) == 0) {
> >> +            _coresight_build_path(child_dev, sink, path, source) ==
> >> 0) {
> >>               found = true;
> >>               break;
> >>           }
> >> @@ -871,7 +926,7 @@ struct list_head *coresight_build_path(struct
> >> coresight_device *source,
> >>         INIT_LIST_HEAD(path);
> >>   -    rc = _coresight_build_path(source, sink, path);
> >> +    rc = _coresight_build_path(source, sink, path, source);
> >>       if (rc) {
> >>           kfree(path);
> >>           return ERR_PTR(rc);
> >> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
> >> b/drivers/hwtracing/coresight/coresight-platform.c
> >> index 9d550f5697fa..f553fb20966d 100644
> >> --- a/drivers/hwtracing/coresight/coresight-platform.c
> >> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> >> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
> >> device *dev,
> >>       struct fwnode_handle *rdev_fwnode;
> >>       struct coresight_connection conn = {};
> >>       struct coresight_connection *new_conn;
> >> +    const char *label;
> >>         do {
> >>           /* Parse the local port details */
> >> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
> >> device *dev,
> >>           conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> >>           conn.dest_port = rendpoint.port;
> >>   +        conn.source_label = NULL;
> >> +        if (!of_property_read_string(ep, "label", &label))
> >> +            conn.source_label = label;
> >> +
> >>           new_conn = coresight_add_out_conn(dev, pdata, &conn);
> >>           if (IS_ERR_VALUE(new_conn)) {
> >>               fwnode_handle_put(conn.dest_fwnode);
> >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> >> index e8b6e388218c..a9c06ef9bbb2 100644
> >> --- a/include/linux/coresight.h
> >> +++ b/include/linux/coresight.h
> >> @@ -167,6 +167,7 @@ struct coresight_desc {
> >>    * struct coresight_connection - representation of a single connection
> >>    * @src_port:    a connection's output port number.
> >>    * @dest_port:    destination's input port number @src_port is
> >> connected to.
> >> + * @source_label: source component's label.
> >>    * @dest_fwnode: destination component's fwnode handle.
> >>    * @dest_dev:    a @coresight_device representation of the component
> >>           connected to @src_port. NULL until the device is created
> >> @@ -195,6 +196,7 @@ struct coresight_desc {
> >>   struct coresight_connection {
> >>       int src_port;
> >>       int dest_port;
> >> +    const char *source_label;
> >>       struct fwnode_handle *dest_fwnode;
> >>       struct coresight_device *dest_dev;
> >>       struct coresight_sysfs_link *link;
> >



--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
From: Krzysztof Kozlowski @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Neeraj Sanjay Kale, marcel, luiz.dentz, davem, edumazet, kuba,
	pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
	amitkumar.karwar, rohit.fule, sherry.sun, ziniu.wang_1,
	haibo.chen, LnxRevLi
In-Reply-To: <20240408132241.750792-2-neeraj.sanjaykale@nxp.com>

On 08/04/2024 15:22, Neeraj Sanjay Kale wrote:
> This adds a new optional device tree property called firware-name.
> 
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>

BTW, there is no such device as btnxpuart. Bindings are for hardware.
With corrected subject:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

> ---
> v2: Add maxItems, simplify description, remove "nxp/". (Krzysztof)
> ---
>  .../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml   | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> index f01a3988538c..6774cc4d6a9e 100644
> --- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> @@ -31,6 +31,11 @@ properties:
>        This property depends on the module vendor's
>        configuration.
>  
> +  firmware-name:
> +    maxItems: 1
> +    description:
> +      Specify firmware file name.

Drop description, redundant. You did not say anything different than
property is saying already.


Best regards,
Krzysztof


^ permalink raw reply

* [PATCH v20 0/9] Add multiport support for DWC3 controllers
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
	Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, Johan Hovold
  Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
	quic_jackp, Krishna Kurapati

Currently the DWC3 driver supports only single port controller which
requires at most two PHYs ie HS and SS PHYs. There are SoCs that has
DWC3 controller with multiple ports that can operate in host mode.
Some of the port supports both SS+HS and other port supports only HS
mode.

This change primarily refactors the Phy logic in core driver to allow
multiport support with Generic Phy's.

Changes have been tested on  QCOM SoC SA8295P which has 4 ports (2
are HS+SS capable and 2 are HS only capable).

Changes in v20:
Modified return check in get_num_ports call.
Code re-verified internally and added Bjorn.A RB Tag in patch (2/9)
from internal review.

Changes in v19:
Replaced IS_ERR(ptr) with a NULL check.
Modified name of function reading the port num in core file.

Changes in v18:
Updated variable names in patch-7 for setup_port_irq and
find_num_ports calls.

Changes in v17:
Modified DT handling patch by checking if dp_hs_phy_1 is present
or not and then going for DT parsing.

Changes in v16:
Removing ACPI has simplified the interrupt reading in wrapper. Also
the logic to find number of ports is based on dp_hs_phy interrupt check
in DT. Enabling and disabling interrupts is now done per port. Added
info on power event irq in commit message.

Changes in v15:
Added minItems property in qcom,dwc3 bindings as suggested by Rob.
Retained all RB's/ACK's got in v14.

Changes in v14:
Moved wrapper binding update to 5th patch in the series as it deals
with only wakeup and not enumeration. The first part of the series
deals with enumeration and the next part deals with wakeup.
Updated commit text for wrapper driver patches.
Added error checks in get_port_index and setup_irq call which were
missing in v13.
Added SOB and CDB tags appropriately for the patches.
Rebased code on top of latest usb next.
DT changes have been removed and will be sent as a separate series.

Changes in v13:
This series is a subset of patches in v11 as the first 3 patches in v11
have been mereged into usb-next.
Moved dr_mode property from platform specific files to common sc8280xp DT.
Fixed function call wrapping, added comments and replaced #defines with
enum in dwc3-qcom for identifying IRQ index appropriately.
Fixed nitpicks pointed out in v11 for suspend-resume handling.
Added reported-by tag for phy refactoring patch as a compile error was
found by kernel test bot [1].
Removed reviewed-by tag of maintainer for phy refactoring patch as a minor
change of increasing phy-names array size by 2-bytes was done to fix
compilation issue mentioned in [1].

Changes in v12:
Pushed as a subset of acked but no-yet-merged patches of v11 with intent
of making rebase of other patches easy. Active reviewers from community
suggested that it would be better to push the whole series in one go as it
would give good clarity and context for all the patches in the series.
So pushed v13 for the same addressing comments received in v11.

Changes in v11:
Implemented port_count calculation by reading interrupt-names from DT.
Refactored IRQ handling in dwc3-qcom.
Moving of macros to xhci-ext-caps.h made as a separate patch.
Names of interrupts to be displayed on /proc/interrupts set to the ones
present in DT.

Changes in v10:
Refactored phy init/exit/power-on/off functions in dwc3 core
Refactored dwc3-qcom irq registration and handling
Implemented wakeup for multiport irq's
Moved few macros from xhci.h to xhci-ext-caps.h
Fixed nits pointed out in v9
Fixed Co-developed by and SOB tags in patches 5 and 11

Changes in v9:
Added IRQ support for DP/DM/SS MP Irq's of SC8280
Refactored code to read port count by accessing xhci registers

Changes in v8:
Reorganised code in patch-5
Fixed nitpicks in code according to comments received on v7
Fixed indentation in DT patches
Added drive strength for pinctrl nodes in SA8295 DT

Changes in v7:
Added power event irq's for Multiport controller.
Udpated commit text for patch-9 (adding DT changes for enabling first
port of multiport controller on sa8540-ride).
Fixed check-patch warnings for driver code.
Fixed DT binding errors for changes in snps,dwc3.yaml
Reabsed code on top of usb-next

Changes in v6:
Updated comments in code after.
Updated variables names appropriately as per review comments.
Updated commit text in patch-2 and added additional info as per review
comments.
The patch header in v5 doesn't have "PATHCH v5" notation present. Corrected
it in this version.

Changes in v5:
Added DT support for first port of Teritiary USB controller on SA8540-Ride
Added support for reading port info from XHCI Extended Params registers.

Changes in RFC v4:
Added DT support for SA8295p.

Changes in RFC v3:
Incase any PHY init fails, then clear/exit the PHYs that
are already initialized.

Changes in RFC v2:
Changed dwc3_count_phys to return the number of PHY Phandles in the node.
This will be used now in dwc3_extract_num_phys to increment num_usb2_phy 
and num_usb3_phy.
Added new parameter "ss_idx" in dwc3_core_get_phy_ny_node and changed its
structure such that the first half is for HS-PHY and second half is for
SS-PHY.
In dwc3_core_get_phy, for multiport controller, only if SS-PHY phandle is
present, pass proper SS_IDX else pass -1.

Tested enumeration interrupt registration on Tertiary controller of
SA8295 ADP:

/ # lsusb
Bus 001 Device 001: ID 1d6b:0002
Bus 002 Device 001: ID 1d6b:0003
Bus 001 Device 002: ID 046d:c06a
/ #
/ # dmesg  | grep ports
[    0.066250] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.154668] dwc3 a400000.usb: K: hs-ports: 4 ss-ports: 2
[    0.223137] xhci-hcd xhci-hcd.0.auto: Host supports USB 3.1 Enhanced SuperSpeed
[    0.227795] hub 1-0:1.0: 4 ports detected
[    0.233724] hub 2-0:1.0: 2 ports detected

Tested interrupt registration for all 4 ports of SA8295 ADP:

/ # cat /proc/interrupts  |grep phy
162: 0 0 0 0 0 0 0 0       PDC 127 Edge      dp_hs_phy_1
163: 0 0 0 0 0 0 0 0       PDC 129 Edge      dp_hs_phy_2
164: 0 0 0 0 0 0 0 0       PDC 131 Edge      dp_hs_phy_3
165: 0 0 0 0 0 0 0 0       PDC 133 Edge      dp_hs_phy_4
166: 0 0 0 0 0 0 0 0       PDC 126 Edge      dm_hs_phy_1
167: 0 0 0 0 0 0 0 0       PDC  16 Level     ss_phy_1
168: 0 0 0 0 0 0 0 0       PDC 128 Edge      dm_hs_phy_2
169: 0 0 0 0 0 0 0 0       PDC  17 Level     ss_phy_2
170: 0 0 0 0 0 0 0 0       PDC 130 Edge      dm_hs_phy_3
171: 0 0 0 0 0 0 0 0       PDC 132 Edge      dm_hs_phy_4
173: 0 0 0 0 0 0 0 0       PDC  14 Edge      dp_hs_phy_irq
174: 0 0 0 0 0 0 0 0       PDC  15 Edge      dm_hs_phy_irq
175: 0 0 0 0 0 0 0 0       PDC 138 Level     ss_phy_irq

Tested working of ADB on SM8550 MTP.

Links to previous versions:
Link to v19: https://lore.kernel.org/all/20240404051229.3082902-1-quic_kriskura@quicinc.com/
Link to v18: https://lore.kernel.org/all/20240326113253.3010447-1-quic_kriskura@quicinc.com/
Link to v17: https://lore.kernel.org/all/20240326102809.2940123-1-quic_kriskura@quicinc.com/
Link to v16: https://lore.kernel.org/all/20240307062052.2319851-1-quic_kriskura@quicinc.com/
Link to v15: https://lore.kernel.org/all/20240216005756.762712-1-quic_kriskura@quicinc.com/
Link to v14: https://lore.kernel.org/all/20240206051825.1038685-1-quic_kriskura@quicinc.com/
Link to v13: https://lore.kernel.org/all/20231007154806.605-1-quic_kriskura@quicinc.com/
Link to v12: https://lore.kernel.org/all/20231004165922.25642-1-quic_kriskura@quicinc.com/
Link to v11: https://lore.kernel.org/all/20230828133033.11988-1-quic_kriskura@quicinc.com/
Link to v10: https://lore.kernel.org/all/20230727223307.8096-1-quic_kriskura@quicinc.com/
Link to v9: https://lore.kernel.org/all/20230621043628.21485-1-quic_kriskura@quicinc.com/
Link to v8: https://lore.kernel.org/all/20230514054917.21318-1-quic_kriskura@quicinc.com/
Link to v7: https://lore.kernel.org/all/20230501143445.3851-1-quic_kriskura@quicinc.com/
Link to v6: https://lore.kernel.org/all/20230405125759.4201-1-quic_kriskura@quicinc.com/
Link to v5: https://lore.kernel.org/all/20230310163420.7582-1-quic_kriskura@quicinc.com/
Link to RFC v4: https://lore.kernel.org/all/20230115114146.12628-1-quic_kriskura@quicinc.com/
Link to RFC v3: https://lore.kernel.org/all/1654709787-23686-1-git-send-email-quic_harshq@quicinc.com/#r
Link to RFC v2: https://lore.kernel.org/all/1653560029-6937-1-git-send-email-quic_harshq@quicinc.com/#r

Krishna Kurapati (9):
  dt-bindings: usb: Add bindings for multiport properties on DWC3
    controller
  usb: dwc3: core: Access XHCI address space temporarily to read port
    info
  usb: dwc3: core: Skip setting event buffers for host only controllers
  usb: dwc3: core: Refactor PHY logic to support Multiport Controller
  dt-bindings: usb: qcom,dwc3: Add bindings for SC8280 Multiport
  usb: dwc3: qcom: Add helper function to request wakeup interrupts
  usb: dwc3: qcom: Refactor IRQ handling in glue driver
  usb: dwc3: qcom: Enable wakeup for applicable ports of multiport
  usb: dwc3: qcom: Add multiport suspend/resume support for wrapper

 .../devicetree/bindings/usb/qcom,dwc3.yaml    |  34 ++
 .../devicetree/bindings/usb/snps,dwc3.yaml    |  13 +-
 drivers/usb/dwc3/core.c                       | 325 +++++++++++++-----
 drivers/usb/dwc3/core.h                       |  19 +-
 drivers/usb/dwc3/drd.c                        |  15 +-
 drivers/usb/dwc3/dwc3-qcom.c                  | 251 +++++++++-----
 6 files changed, 482 insertions(+), 175 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH v20 1/9] dt-bindings: usb: Add bindings for multiport properties on DWC3 controller
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
	Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, Johan Hovold
  Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
	quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>

Add bindings to indicate properties required to support multiport
on Synopsys DWC3 controller.

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../devicetree/bindings/usb/snps,dwc3.yaml          | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 203a1eb66691..bfac0a37d0e4 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -85,15 +85,16 @@ properties:
 
   phys:
     minItems: 1
-    maxItems: 2
+    maxItems: 8
 
   phy-names:
     minItems: 1
-    maxItems: 2
-    items:
-      enum:
-        - usb2-phy
-        - usb3-phy
+    maxItems: 8
+    oneOf:
+      - items:
+          enum: [ usb2-phy, usb3-phy ]
+      - items:
+          pattern: "^usb[23]-[0-3]$"
 
   power-domains:
     description:
-- 
2.34.1


^ permalink raw reply related

* [PATCH v20 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
	Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, Johan Hovold
  Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
	quic_jackp, Krishna Kurapati, Bjorn Andersson
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>

All DWC3 Multi Port controllers that exist today only support host mode.
Temporarily map XHCI address space for host-only controllers and parse
XHCI Extended Capabilities registers to read number of usb2 ports and
usb3 ports present on multiport controller. Each USB Port is at least HS
capable.

The port info for usb2 and usb3 phy are identified as num_usb2_ports
and num_usb3_ports. The intention is as follows:

Wherever we need to perform phy operations like:

LOOP_OVER_NUMBER_OF_AVAILABLE_PORTS()
{
	phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
	phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
}

If number of usb2 ports is 3, loop can go from index 0-2 for
usb2_generic_phy. If number of usb3-ports is 2, we don't know for sure,
if the first 2 ports are SS capable or some other ports like (2 and 3)
are SS capable. So instead, num_usb2_ports is used to loop around all
phy's (both hs and ss) for performing phy operations. If any
usb3_generic_phy turns out to be NULL, phy operation just bails out.
num_usb3_ports is used to modify GUSB3PIPECTL registers while setting up
phy's as we need to know how many SS capable ports are there for this.

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/usb/dwc3/core.c | 61 +++++++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h |  5 ++++
 2 files changed, 66 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 31684cdaaae3..ddab30531f8a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -39,6 +39,7 @@
 #include "io.h"
 
 #include "debug.h"
+#include "../host/xhci-ext-caps.h"
 
 #define DWC3_DEFAULT_AUTOSUSPEND_DELAY	5000 /* ms */
 
@@ -1881,10 +1882,56 @@ static int dwc3_get_clocks(struct dwc3 *dwc)
 	return 0;
 }
 
+static int dwc3_get_num_ports(struct dwc3 *dwc)
+{
+	void __iomem *base;
+	u8 major_revision;
+	u32 offset;
+	u32 val;
+
+	/*
+	 * Remap xHCI address space to access XHCI ext cap regs since it is
+	 * needed to get information on number of ports present.
+	 */
+	base = ioremap(dwc->xhci_resources[0].start,
+		       resource_size(&dwc->xhci_resources[0]));
+	if (!base)
+		return -ENOMEM;
+
+	offset = 0;
+	do {
+		offset = xhci_find_next_ext_cap(base, offset,
+						XHCI_EXT_CAPS_PROTOCOL);
+		if (!offset)
+			break;
+
+		val = readl(base + offset);
+		major_revision = XHCI_EXT_PORT_MAJOR(val);
+
+		val = readl(base + offset + 0x08);
+		if (major_revision == 0x03) {
+			dwc->num_usb3_ports += XHCI_EXT_PORT_COUNT(val);
+		} else if (major_revision <= 0x02) {
+			dwc->num_usb2_ports += XHCI_EXT_PORT_COUNT(val);
+		} else {
+			dev_warn(dwc->dev, "unrecognized port major revision %d\n",
+				 major_revision);
+		}
+	} while (1);
+
+	dev_dbg(dwc->dev, "hs-ports: %u ss-ports: %u\n",
+		dwc->num_usb2_ports, dwc->num_usb3_ports);
+
+	iounmap(base);
+
+	return 0;
+}
+
 static int dwc3_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
 	struct resource		*res, dwc_res;
+	unsigned int		hw_mode;
 	void __iomem		*regs;
 	struct dwc3		*dwc;
 	int			ret;
@@ -1968,6 +2015,20 @@ static int dwc3_probe(struct platform_device *pdev)
 			goto err_disable_clks;
 	}
 
+	/*
+	 * Currently only DWC3 controllers that are host-only capable
+	 * can have more than one port.
+	 */
+	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+	if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
+		ret = dwc3_get_num_ports(dwc);
+		if (ret)
+			goto err_disable_clks;
+	} else {
+		dwc->num_usb2_ports = 1;
+		dwc->num_usb3_ports = 1;
+	}
+
 	spin_lock_init(&dwc->lock);
 	mutex_init(&dwc->mutex);
 
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 7e80dd3d466b..341e4c73cb2e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1039,6 +1039,8 @@ struct dwc3_scratchpad_array {
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
  * @usb3_generic_phy: pointer to USB3 PHY
+ * @num_usb2_ports: number of USB2 ports
+ * @num_usb3_ports: number of USB3 ports
  * @phys_ready: flag to indicate that PHYs are ready
  * @ulpi: pointer to ulpi interface
  * @ulpi_ready: flag to indicate that ULPI is initialized
@@ -1187,6 +1189,9 @@ struct dwc3 {
 	struct phy		*usb2_generic_phy;
 	struct phy		*usb3_generic_phy;
 
+	u8			num_usb2_ports;
+	u8			num_usb3_ports;
+
 	bool			phys_ready;
 
 	struct ulpi		*ulpi;
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v3 2/2] media: i2c: Add GC05A2 image sensor driver
From: Kieran Bingham @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Zhi Mao 毛智, krzysztof.kozlowski+dt, mchehab,
	robh+dt, sakari.ailus
  Cc: heiko
In-Reply-To: <5cdf1a2530ffd927fe2e4130ab6666724cf3354d.camel@mediatek.com>

Quoting Zhi Mao (毛智) (2024-04-08 12:50:21)
> Hi Kieran,
> 
> Thanks for your review this patch.
> 
> It seems that there are some difficult for us(Mediatek) to explain
> these register setting comments.
> As these settings are released by GC sensor vendor, and we have not
> detailed datasheet described them.
> And even if send the letter to ask sensor vendor, I am afraid there may
> be not a clear response.
> 
> Can we just focus on the driver code function and control flow part?
> 

As I said - You can take my comments with a pinch of salt ... but I
wanted to know your position on it ;-)

--
Kieran

> On Sun, 2024-04-07 at 10:08 +0100, Kieran Bingham wrote:
> >        
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >  Hello,
> > 
> > Thanks for helping extending the kernels sensor driver support.
> > 
> > My comments below can likely be taken with a pinch of salt, as they
> > are
> > mostly around the tabled register values ... but we have many drivers
> > which are binary blobs of sensor register values and I think it would
> > be
> > far more beneficial to clean these up where possible...
> > 
> > So the first question is ... Can we ?
> > 
> > 
> > 
> > Quoting Zhi Mao (2024-04-03 04:38:25)
> > > Add a V4L2 sub-device driver for Galaxycore GC05A2 image sensor.
> > > 
> > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com>
> > > ---
> > >  drivers/media/i2c/Kconfig  |   10 +
> > >  drivers/media/i2c/Makefile |    1 +
> > >  drivers/media/i2c/gc05a2.c | 1383
> > ++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 1394 insertions(+)
> > >  create mode 100644 drivers/media/i2c/gc05a2.c
> > > 
> > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > index 56f276b920ab..97993bf160f9 100644
> > > --- a/drivers/media/i2c/Kconfig
> > > +++ b/drivers/media/i2c/Kconfig
> > > @@ -70,6 +70,16 @@ config VIDEO_GC0308
> > >           To compile this driver as a module, choose M here: the
> > >           module will be called gc0308.
> > >  
> > > +config VIDEO_GC05A2
> > > +       tristate "GalaxyCore gc05a2 sensor support"
> > > +       select V4L2_CCI_I2C
> > > +       help
> > > +         This is a Video4Linux2 sensor driver for the GalaxyCore
> > gc05a2
> > > +         camera.
> > > +
> > > +         To compile this driver as a module, choose M here: the
> > > +         module will be called gc05a2.
> > > +
> > >  config VIDEO_GC2145
> > >         select V4L2_CCI_I2C
> > >         tristate "GalaxyCore GC2145 sensor support"
> > > diff --git a/drivers/media/i2c/Makefile
> > b/drivers/media/i2c/Makefile
> > > index dfbe6448b549..8ed6faf0f854 100644
> > > --- a/drivers/media/i2c/Makefile
> > > +++ b/drivers/media/i2c/Makefile
> > > @@ -38,6 +38,7 @@ obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> > >  obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> > >  obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
> > >  obj-$(CONFIG_VIDEO_GC0308) += gc0308.o
> > > +obj-$(CONFIG_VIDEO_GC05A2) += gc05a2.o
> > >  obj-$(CONFIG_VIDEO_GC2145) += gc2145.o
> > >  obj-$(CONFIG_VIDEO_HI556) += hi556.o
> > >  obj-$(CONFIG_VIDEO_HI846) += hi846.o
> > > diff --git a/drivers/media/i2c/gc05a2.c
> > b/drivers/media/i2c/gc05a2.c
> > > new file mode 100644
> > > index 000000000000..461d33055a3b
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/gc05a2.c
> > > @@ -0,0 +1,1383 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Driver for GalaxyCore gc05a2 image sensor
> > > + *
> > > + * Copyright 2024 MediaTek
> > > + *
> > > + * Zhi Mao <zhi.mao@mediatek.com>
> > > + */
> > > +#include <linux/array_size.h>
> > > +#include <linux/bits.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/container_of.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/math64.h>
> > > +#include <linux/mod_devicetable.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/property.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/types.h>
> > > +#include <linux/units.h>
> > > +
> > > +#include <media/v4l2-cci.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-event.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-subdev.h>
> > > +
> > > +#define GC05A2_REG_TEST_PATTERN_EN CCI_REG8(0x008c)
> > > +#define GC05A2_REG_TEST_PATTERN_IDX CCI_REG8(0x008d)
> > > +#define GC05A2_TEST_PATTERN_EN 0x01
> > > +
> > > +#define GC05A2_STREAMING_REG CCI_REG8(0x0100)
> > > +
> > > +#define GC05A2_FLIP_REG CCI_REG8(0x0101)
> > > +#define GC05A2_FLIP_H_MASK BIT(0)
> > > +#define GC05A2_FLIP_V_MASK BIT(1)
> > > +
> > > +#define GC05A2_EXP_REG CCI_REG16(0x0202)
> > > +#define GC05A2_EXP_MARGIN 16
> > > +#define GC05A2_EXP_MIN 4
> > > +#define GC05A2_EXP_STEP 1
> > > +
> > > +#define GC05A2_AGAIN_REG CCI_REG16(0x0204)
> > > +#define GC05A2_AGAIN_MIN 1024
> > > +#define GC05A2_AGAIN_MAX (1024 * 16)
> > > +#define GC05A2_AGAIN_STEP 1
> > > +
> > > +#define GC05A2_FRAME_LENGTH_REG CCI_REG16(0x0340)
> > > +#define GC05A2_VTS_MAX 0xffff
> > > +
> > > +#define GC05A2_REG_CHIP_ID CCI_REG16(0x03f0)
> > > +#define GC05A2_CHIP_ID 0x05a2
> > > +
> > > +#define GC05A2_NATIVE_WIDTH 2592
> > > +#define GC05A2_NATIVE_HEIGHT 1944
> > > +
> > > +#define GC05A2_DEFAULT_CLK_FREQ (24 * HZ_PER_MHZ)
> > > +#define GC05A2_MBUS_CODE MEDIA_BUS_FMT_SGRBG10_1X10
> > > +#define GC05A2_DATA_LANES 2
> > > +#define GC05A2_RGB_DEPTH 10
> > > +#define GC05A2_SLEEP_US  (2 * USEC_PER_MSEC)
> > > +
> > > +static const char *const gc05a2_test_pattern_menu[] = {
> > > +       "No Pattern",  "Fade_to_gray_Color Bar", "Color Bar",
> > > +       "PN9",         "Horizental_gradient",    "Checkboard
> > Pattern",
> > > +       "Slant",       "Resolution",             "Solid Black",
> > > +       "Solid White",
> > > +};
> > > +
> > > +static const s64 gc05a2_link_freq_menu_items[] = {
> > > +       (448 * HZ_PER_MHZ),
> > > +       (224 * HZ_PER_MHZ),
> > > +};
> > > +
> > > +static const char *const gc05a2_supply_name[] = {
> > > +       "avdd",
> > > +       "dvdd",
> > > +       "dovdd",
> > > +};
> > > +
> > > +struct gc05a2 {
> > > +       struct device *dev;
> > > +       struct v4l2_subdev sd;
> > > +       struct media_pad pad;
> > > +
> > > +       struct clk *xclk;
> > > +       struct regulator_bulk_data
> > supplies[ARRAY_SIZE(gc05a2_supply_name)];
> > > +       struct gpio_desc *reset_gpio;
> > > +
> > > +       struct v4l2_ctrl_handler ctrls;
> > > +       struct v4l2_ctrl *pixel_rate;
> > > +       struct v4l2_ctrl *link_freq;
> > > +       struct v4l2_ctrl *exposure;
> > > +       struct v4l2_ctrl *vblank;
> > > +       struct v4l2_ctrl *hblank;
> > > +       struct v4l2_ctrl *hflip;
> > > +       struct v4l2_ctrl *vflip;
> > > +
> > > +       struct regmap *regmap;
> > > +       unsigned long link_freq_bitmap;
> > > +
> > > +       /* True if the device has been identified */
> > > +       bool identified;
> > > +       const struct gc05a2_mode *cur_mode;
> > > +};
> > > +
> > > +struct gc05a2_reg_list {
> > > +       u32 num_of_regs;
> > > +       const struct cci_reg_sequence *regs;
> > > +};
> > > +
> > > +static const struct cci_reg_sequence mode_2592x1944[] = {
> > > +       /* system */
> > > +       { CCI_REG8(0x0135), 0x01 },
> > > +
> > > +       /* pre_setting */
> > > +       { CCI_REG8(0x0084), 0x21 },
> > > +       { CCI_REG8(0x0d05), 0xcc },
> > > +       { CCI_REG8(0x0218), 0x00 },
> > > +       { CCI_REG8(0x005e), 0x48 },
> > > +       { CCI_REG8(0x0d06), 0x01 },
> > > +       { CCI_REG8(0x0007), 0x16 },
> > > +       { CCI_REG8(0x0101), 0x00 },
> > > +
> > > +       /* analog */
> > > +       { CCI_REG8(0x0342), 0x07 },
> > > +       { CCI_REG8(0x0343), 0x28 },
> > > +       { CCI_REG8(0x0220), 0x07 },
> > > +       { CCI_REG8(0x0221), 0xd0 },
> > > +       { CCI_REG8(0x0202), 0x07 },
> > > +       { CCI_REG8(0x0203), 0x32 },
> > > +       { CCI_REG8(0x0340), 0x07 },
> > > +       { CCI_REG8(0x0341), 0xf0 },
> > > +       { CCI_REG8(0x0219), 0x00 },
> > > +       { CCI_REG8(0x0346), 0x00 },
> > > +       { CCI_REG8(0x0347), 0x04 },
> > > +       { CCI_REG8(0x0d14), 0x00 },
> > > +       { CCI_REG8(0x0d13), 0x05 },
> > > +       { CCI_REG8(0x0d16), 0x05 },
> > > +       { CCI_REG8(0x0d15), 0x1d },
> > > +       { CCI_REG8(0x00c0), 0x0a },
> > > +       { CCI_REG8(0x00c1), 0x30 },
> > > +       { CCI_REG8(0x034a), 0x07 },
> > > +       { CCI_REG8(0x034b), 0xa8 },
> > > +       { CCI_REG8(0x0e0a), 0x00 },
> > > +       { CCI_REG8(0x0e0b), 0x00 },
> > > +       { CCI_REG8(0x0e0e), 0x03 },
> > > +       { CCI_REG8(0x0e0f), 0x00 },
> > > +       { CCI_REG8(0x0e06), 0x0a },
> > > +       { CCI_REG8(0x0e23), 0x15 },
> > > +       { CCI_REG8(0x0e24), 0x15 },
> > > +       { CCI_REG8(0x0e2a), 0x10 },
> > > +       { CCI_REG8(0x0e2b), 0x10 },
> > > +       { CCI_REG8(0x0e17), 0x49 },
> > > +       { CCI_REG8(0x0e1b), 0x1c },
> > > +       { CCI_REG8(0x0e3a), 0x36 },
> > > +       { CCI_REG8(0x0d11), 0x84 },
> > > +       { CCI_REG8(0x0e52), 0x14 },
> > > +       { CCI_REG8(0x000b), 0x10 },
> > > +       { CCI_REG8(0x0008), 0x08 },
> > > +       { CCI_REG8(0x0223), 0x17 },
> > > +       { CCI_REG8(0x0d27), 0x39 },
> > > +       { CCI_REG8(0x0d22), 0x00 },
> > > +       { CCI_REG8(0x03f6), 0x0d },
> > > +       { CCI_REG8(0x0d04), 0x07 },
> > > +       { CCI_REG8(0x03f3), 0x72 },
> > > +       { CCI_REG8(0x03f4), 0xb8 },
> > > +       { CCI_REG8(0x03f5), 0xbc },
> > > +       { CCI_REG8(0x0d02), 0x73 },
> > > +
> > > +       /* auto load start */
> > > +       { CCI_REG8(0x00cb), 0x00 },
> > > +
> > > +       /* OUT 2592*1944 */
> > > +       { CCI_REG8(0x0350), 0x01 },
> > > +       { CCI_REG8(0x0353), 0x00 },
> > > +       { CCI_REG8(0x0354), 0x08 },
> > 
> > > +       { CCI_REG8(0x034c), 0x0a },
> > > +       { CCI_REG8(0x034d), 0x20 },
> > 
> > Should/Could this be
> >         { CCI_REG16(0x034c), 2592 }, /* Width */
> > 
> > 
> > > +       { CCI_REG8(0x021f), 0x14 },
> > > +
> > > +       /* MIPI */
> > > +       { CCI_REG8(0x0107), 0x05 },
> > > +       { CCI_REG8(0x0117), 0x01 },
> > > +       { CCI_REG8(0x0d81), 0x00 },
> > > +       { CCI_REG8(0x0d84), 0x0c },
> > > +       { CCI_REG8(0x0d85), 0xa8 },
> > > +       { CCI_REG8(0x0d86), 0x06 },
> > > +       { CCI_REG8(0x0d87), 0x55 },
> > > +       { CCI_REG8(0x0db3), 0x06 },
> > > +       { CCI_REG8(0x0db4), 0x08 },
> > > +       { CCI_REG8(0x0db5), 0x1e },
> > > +       { CCI_REG8(0x0db6), 0x02 },
> > > +       { CCI_REG8(0x0db8), 0x12 },
> > > +       { CCI_REG8(0x0db9), 0x0a },
> > > +       { CCI_REG8(0x0d93), 0x06 },
> > > +       { CCI_REG8(0x0d94), 0x09 },
> > > +       { CCI_REG8(0x0d95), 0x0d },
> > > +       { CCI_REG8(0x0d99), 0x0b },
> > > +       { CCI_REG8(0x0084), 0x01 },
> > > +
> > > +       /* OUT */
> > > +       { CCI_REG8(0x0110), 0x01 },
> > > +};
> > > +
> > > +static const struct cci_reg_sequence mode_1280x720[] = {
> > > +       /* system */
> > > +       { CCI_REG8(0x0135), 0x05 },
> > 
> > In 2592x1944 this is 0x01. Do you have a datasheet? Can you explain
> > why
> > they are different? Can you add register definitions that have names
> > to
> > make this more maintainable or extendable in the future?
> > 
> > There's discussion in the recent series improving the IMX258 which
> > makes
> > me wonder if we should try harder to have sensor drivers with clearer
> > definitions.
> > 
> > 
> > > +
> > > +       /*pre_setting*/
> > 
> > /* pre_setting */ ?
> > 
> > > +       { CCI_REG8(0x0084), 0x21 },
> > > +       { CCI_REG8(0x0d05), 0xcc },
> > > +       { CCI_REG8(0x0218), 0x80 },
> > > +       { CCI_REG8(0x005e), 0x49 },
> > > +       { CCI_REG8(0x0d06), 0x81 },
> > > +       { CCI_REG8(0x0007), 0x16 },
> > > +       { CCI_REG8(0x0101), 0x00 },
> > 
> > In 2592x1944, only register 0x0218 differs. Why? What is that? Can it
> > be
> > broken out to a function that applies the correct configuration at
> > startuup based on a parameter instead of duplicating this table set?
> > 
> > > +
> > > +       /* analog */
> > > +       { CCI_REG8(0x0342), 0x07 },
> > > +       { CCI_REG8(0x0343), 0x10 },
> > > +       { CCI_REG8(0x0220), 0x07 },
> > > +       { CCI_REG8(0x0221), 0xd0 },
> > > +       { CCI_REG8(0x0202), 0x03 },
> > > +       { CCI_REG8(0x0203), 0x32 },
> > > +       { CCI_REG8(0x0340), 0x04 },
> > > +       { CCI_REG8(0x0341), 0x08 },
> > > +       { CCI_REG8(0x0219), 0x00 },
> > > +       { CCI_REG8(0x0346), 0x01 },
> > > +       { CCI_REG8(0x0347), 0x00 },
> > > +       { CCI_REG8(0x0d14), 0x00 },
> > > +       { CCI_REG8(0x0d13), 0x05 },
> > > +       { CCI_REG8(0x0d16), 0x05 },
> > > +       { CCI_REG8(0x0d15), 0x1d },
> > > +       { CCI_REG8(0x00c0), 0x0a },
> > > +       { CCI_REG8(0x00c1), 0x30 },
> > > +       { CCI_REG8(0x034a), 0x05 },
> > > +       { CCI_REG8(0x034b), 0xb0 },
> > > +       { CCI_REG8(0x0e0a), 0x00 },
> > > +       { CCI_REG8(0x0e0b), 0x00 },
> > > +       { CCI_REG8(0x0e0e), 0x03 },
> > > +       { CCI_REG8(0x0e0f), 0x00 },
> > > +       { CCI_REG8(0x0e06), 0x0a },
> > > +       { CCI_REG8(0x0e23), 0x15 },
> > > +       { CCI_REG8(0x0e24), 0x15 },
> > > +       { CCI_REG8(0x0e2a), 0x10 },
> > > +       { CCI_REG8(0x0e2b), 0x10 },
> > > +       { CCI_REG8(0x0e17), 0x49 },
> > > +       { CCI_REG8(0x0e1b), 0x1c },
> > > +       { CCI_REG8(0x0e3a), 0x36 },
> > > +       { CCI_REG8(0x0d11), 0x84 },
> > > +       { CCI_REG8(0x0e52), 0x14 },
> > > +       { CCI_REG8(0x000b), 0x0e },
> > > +       { CCI_REG8(0x0008), 0x03 },
> > > +       { CCI_REG8(0x0223), 0x16 },
> > > +       { CCI_REG8(0x0d27), 0x39 },
> > > +       { CCI_REG8(0x0d22), 0x00 },
> > > +       { CCI_REG8(0x03f6), 0x0d },
> > > +       { CCI_REG8(0x0d04), 0x07 },
> > > +       { CCI_REG8(0x03f3), 0x72 },
> > > +       { CCI_REG8(0x03f4), 0xb8 },
> > > +       { CCI_REG8(0x03f5), 0xbc },
> > > +       { CCI_REG8(0x0d02), 0x73 },
> > > +
> > 
> > Are any of those able to be broken out to named register to be more
> > clear in their intent?
> > 
> > > +       /* auto load start */
> > > +       { CCI_REG8(0x00cb), 0xfc },
> > > +
> > 
> > Why is this auto load start so different to the other modes 'auto
> > load
> > start'? What do the bits refer to ?
> > 
> > > +       /* OUT 1280x720 */
> > > +       { CCI_REG8(0x0350), 0x01 },
> > > +       { CCI_REG8(0x0353), 0x00 },
> > > +       { CCI_REG8(0x0354), 0x0c },
> > 
> > > +       { CCI_REG8(0x034c), 0x05 },
> > > +       { CCI_REG8(0x034d), 0x00 },
> > 
> > Should/Could this be 
> >          { CCI_REG16(0x034c), 1280 },
> > 
> > Are there any other register settings that would make more sense to
> > be
> > in decimal units that match their actual context?
> > 
> > 
> > > +       { CCI_REG8(0x021f), 0x14 },
> > 
> > I don't see a setting for 720/0x2d0. Do these registers only set the
> > width?
> > 
> > > +
> > > +       /* MIPI */
> > > +       { CCI_REG8(0x0107), 0x05 },
> > > +       { CCI_REG8(0x0117), 0x01 },
> > > +       { CCI_REG8(0x0d81), 0x00 },
> > > +       { CCI_REG8(0x0d84), 0x06 },
> > > +       { CCI_REG8(0x0d85), 0x40 },
> > > +       { CCI_REG8(0x0d86), 0x03 },
> > > +       { CCI_REG8(0x0d87), 0x21 },
> > > +       { CCI_REG8(0x0db3), 0x03 },
> > > +       { CCI_REG8(0x0db4), 0x04 },
> > > +       { CCI_REG8(0x0db5), 0x0d },
> > > +       { CCI_REG8(0x0db6), 0x01 },
> > > +       { CCI_REG8(0x0db8), 0x04 },
> > > +       { CCI_REG8(0x0db9), 0x06 },
> > > +       { CCI_REG8(0x0d93), 0x03 },
> > > +       { CCI_REG8(0x0d94), 0x04 },
> > > +       { CCI_REG8(0x0d95), 0x05 },
> > > +       { CCI_REG8(0x0d99), 0x06 },
> > > +       { CCI_REG8(0x0084), 0x01 },
> > > +
> > > +       /* OUT */
> > 
> > Out where? What is out?
> > 
> > > +       { CCI_REG8(0x0110), 0x01 },
> > > +};
> > > +
> > > +static const struct cci_reg_sequence mode_table_common[] = {
> > > +       { GC05A2_STREAMING_REG, 0x00 },
> > > +       /* system */
> > > +       { CCI_REG8(0x0315), 0xd4 },
> > > +       { CCI_REG8(0x0d06), 0x01 },
> > > +       { CCI_REG8(0x0a70), 0x80 },
> > > +       { CCI_REG8(0x031a), 0x00 },
> > > +       { CCI_REG8(0x0314), 0x00 },
> > > +       { CCI_REG8(0x0130), 0x08 },
> > > +       { CCI_REG8(0x0132), 0x01 },
> > > +       { CCI_REG8(0x0136), 0x38 },
> > > +       { CCI_REG8(0x0137), 0x03 },
> > > +       { CCI_REG8(0x0134), 0x5b },
> > > +       { CCI_REG8(0x031c), 0xe0 },
> > > +       { CCI_REG8(0x0d82), 0x14 },
> > > +       { CCI_REG8(0x0dd1), 0x56 },
> > > +
> > > +       /* gate_mode */
> > > +       { CCI_REG8(0x0af4), 0x01 },
> > > +       { CCI_REG8(0x0002), 0x10 },
> > > +       { CCI_REG8(0x00c3), 0x34 },
> > > +
> > > +       /* auto load start */
> > 
> > The previous 'auto load start' referenced 0x00cb ?
> > 
> > > +       { CCI_REG8(0x00c4), 0x00 },
> > > +       { CCI_REG8(0x00c5), 0x01 },
> > > +       { CCI_REG8(0x0af6), 0x00 },
> > > +       { CCI_REG8(0x0ba0), 0x17 },
> > > +       { CCI_REG8(0x0ba1), 0x00 },
> > > +       { CCI_REG8(0x0ba2), 0x00 },
> > > +       { CCI_REG8(0x0ba3), 0x00 },
> > > +       { CCI_REG8(0x0ba4), 0x03 },
> > > +       { CCI_REG8(0x0ba5), 0x00 },
> > > +       { CCI_REG8(0x0ba6), 0x00 },
> > > +       { CCI_REG8(0x0ba7), 0x00 },
> > > +       { CCI_REG8(0x0ba8), 0x40 },
> > > +       { CCI_REG8(0x0ba9), 0x00 },
> > > +       { CCI_REG8(0x0baa), 0x00 },
> > > +       { CCI_REG8(0x0bab), 0x00 },
> > > +       { CCI_REG8(0x0bac), 0x40 },
> > > +       { CCI_REG8(0x0bad), 0x00 },
> > > +       { CCI_REG8(0x0bae), 0x00 },
> > > +       { CCI_REG8(0x0baf), 0x00 },
> > > +       { CCI_REG8(0x0bb0), 0x02 },
> > > +       { CCI_REG8(0x0bb1), 0x00 },
> > > +       { CCI_REG8(0x0bb2), 0x00 },
> > > +       { CCI_REG8(0x0bb3), 0x00 },
> > > +       { CCI_REG8(0x0bb8), 0x02 },
> > > +       { CCI_REG8(0x0bb9), 0x00 },
> > > +       { CCI_REG8(0x0bba), 0x00 },
> > > +       { CCI_REG8(0x0bbb), 0x00 },
> > > +       { CCI_REG8(0x0a70), 0x80 },
> > > +       { CCI_REG8(0x0a71), 0x00 },
> > > +       { CCI_REG8(0x0a72), 0x00 },
> > > +       { CCI_REG8(0x0a66), 0x00 },
> > > +       { CCI_REG8(0x0a67), 0x80 },
> > > +       { CCI_REG8(0x0a4d), 0x4e },
> > > +       { CCI_REG8(0x0a50), 0x00 },
> > > +       { CCI_REG8(0x0a4f), 0x0c },
> > > +       { CCI_REG8(0x0a66), 0x00 },
> > > +       { CCI_REG8(0x00ca), 0x00 },
> > > +       { CCI_REG8(0x00cc), 0x00 },
> > > +       { CCI_REG8(0x00cd), 0x00 },
> > > +       { CCI_REG8(0x0aa1), 0x00 },
> > > +       { CCI_REG8(0x0aa2), 0xe0 },
> > > +       { CCI_REG8(0x0aa3), 0x00 },
> > > +       { CCI_REG8(0x0aa4), 0x40 },
> > > +       { CCI_REG8(0x0a90), 0x03 },
> > > +       { CCI_REG8(0x0a91), 0x0e },
> > > +       { CCI_REG8(0x0a94), 0x80 },
> > > +
> > > +       /* standby */
> > > +       { CCI_REG8(0x0af6), 0x20 },
> > > +       { CCI_REG8(0x0b00), 0x91 },
> > > +       { CCI_REG8(0x0b01), 0x17 },
> > > +       { CCI_REG8(0x0b02), 0x01 },
> > > +       { CCI_REG8(0x0b03), 0x00 },
> > > +       { CCI_REG8(0x0b04), 0x01 },
> > > +       { CCI_REG8(0x0b05), 0x17 },
> > > +       { CCI_REG8(0x0b06), 0x01 },
> > > +       { CCI_REG8(0x0b07), 0x00 },
> > > +       { CCI_REG8(0x0ae9), 0x01 },
> > > +       { CCI_REG8(0x0aea), 0x02 },
> > > +       { CCI_REG8(0x0ae8), 0x53 },
> > > +       { CCI_REG8(0x0ae8), 0x43 },
> > > +
> > > +       /* gain_partition */
> > > +       { CCI_REG8(0x0af6), 0x30 },
> > > +       { CCI_REG8(0x0b00), 0x08 },
> > > +       { CCI_REG8(0x0b01), 0x0f },
> > > +       { CCI_REG8(0x0b02), 0x00 },
> > > +       { CCI_REG8(0x0b04), 0x1c },
> > > +       { CCI_REG8(0x0b05), 0x24 },
> > > +       { CCI_REG8(0x0b06), 0x00 },
> > > +       { CCI_REG8(0x0b08), 0x30 },
> > > +       { CCI_REG8(0x0b09), 0x40 },
> > > +       { CCI_REG8(0x0b0a), 0x00 },
> > > +       { CCI_REG8(0x0b0c), 0x0e },
> > > +       { CCI_REG8(0x0b0d), 0x2a },
> > > +       { CCI_REG8(0x0b0e), 0x00 },
> > > +       { CCI_REG8(0x0b10), 0x0e },
> > > +       { CCI_REG8(0x0b11), 0x2b },
> > > +       { CCI_REG8(0x0b12), 0x00 },
> > > +       { CCI_REG8(0x0b14), 0x0e },
> > > +       { CCI_REG8(0x0b15), 0x23 },
> > > +       { CCI_REG8(0x0b16), 0x00 },
> > > +       { CCI_REG8(0x0b18), 0x0e },
> > > +       { CCI_REG8(0x0b19), 0x24 },
> > > +       { CCI_REG8(0x0b1a), 0x00 },
> > > +       { CCI_REG8(0x0b1c), 0x0c },
> > > +       { CCI_REG8(0x0b1d), 0x0c },
> > > +       { CCI_REG8(0x0b1e), 0x00 },
> > > +       { CCI_REG8(0x0b20), 0x03 },
> > > +       { CCI_REG8(0x0b21), 0x03 },
> > > +       { CCI_REG8(0x0b22), 0x00 },
> > > +       { CCI_REG8(0x0b24), 0x0e },
> > > +       { CCI_REG8(0x0b25), 0x0e },
> > > +       { CCI_REG8(0x0b26), 0x00 },
> > > +       { CCI_REG8(0x0b28), 0x03 },
> > > +       { CCI_REG8(0x0b29), 0x03 },
> > > +       { CCI_REG8(0x0b2a), 0x00 },
> > > +       { CCI_REG8(0x0b2c), 0x12 },
> > > +       { CCI_REG8(0x0b2d), 0x12 },
> > > +       { CCI_REG8(0x0b2e), 0x00 },
> > > +       { CCI_REG8(0x0b30), 0x08 },
> > > +       { CCI_REG8(0x0b31), 0x08 },
> > > +       { CCI_REG8(0x0b32), 0x00 },
> > > +       { CCI_REG8(0x0b34), 0x14 },
> > > +       { CCI_REG8(0x0b35), 0x14 },
> > > +       { CCI_REG8(0x0b36), 0x00 },
> > > +       { CCI_REG8(0x0b38), 0x10 },
> > > +       { CCI_REG8(0x0b39), 0x10 },
> > > +       { CCI_REG8(0x0b3a), 0x00 },
> > > +       { CCI_REG8(0x0b3c), 0x16 },
> > > +       { CCI_REG8(0x0b3d), 0x16 },
> > > +       { CCI_REG8(0x0b3e), 0x00 },
> > > +       { CCI_REG8(0x0b40), 0x10 },
> > > +       { CCI_REG8(0x0b41), 0x10 },
> > > +       { CCI_REG8(0x0b42), 0x00 },
> > > +       { CCI_REG8(0x0b44), 0x19 },
> > > +       { CCI_REG8(0x0b45), 0x19 },
> > > +       { CCI_REG8(0x0b46), 0x00 },
> > > +       { CCI_REG8(0x0b48), 0x16 },
> > > +       { CCI_REG8(0x0b49), 0x16 },
> > > +       { CCI_REG8(0x0b4a), 0x00 },
> > > +       { CCI_REG8(0x0b4c), 0x19 },
> > > +       { CCI_REG8(0x0b4d), 0x19 },
> > > +       { CCI_REG8(0x0b4e), 0x00 },
> > > +       { CCI_REG8(0x0b50), 0x16 },
> > > +       { CCI_REG8(0x0b51), 0x16 },
> > > +       { CCI_REG8(0x0b52), 0x00 },
> > > +       { CCI_REG8(0x0b80), 0x01 },
> > > +       { CCI_REG8(0x0b81), 0x00 },
> > > +       { CCI_REG8(0x0b82), 0x00 },
> > > +       { CCI_REG8(0x0b84), 0x00 },
> > > +       { CCI_REG8(0x0b85), 0x00 },
> > > +       { CCI_REG8(0x0b86), 0x00 },
> > > +       { CCI_REG8(0x0b88), 0x01 },
> > > +       { CCI_REG8(0x0b89), 0x6a },
> > > +       { CCI_REG8(0x0b8a), 0x00 },
> > > +       { CCI_REG8(0x0b8c), 0x00 },
> > > +       { CCI_REG8(0x0b8d), 0x01 },
> > > +       { CCI_REG8(0x0b8e), 0x00 },
> > > +       { CCI_REG8(0x0b90), 0x01 },
> > > +       { CCI_REG8(0x0b91), 0xf6 },
> > > +       { CCI_REG8(0x0b92), 0x00 },
> > > +       { CCI_REG8(0x0b94), 0x00 },
> > > +       { CCI_REG8(0x0b95), 0x02 },
> > > +       { CCI_REG8(0x0b96), 0x00 },
> > > +       { CCI_REG8(0x0b98), 0x02 },
> > > +       { CCI_REG8(0x0b99), 0xc4 },
> > > +       { CCI_REG8(0x0b9a), 0x00 },
> > > +       { CCI_REG8(0x0b9c), 0x00 },
> > > +       { CCI_REG8(0x0b9d), 0x03 },
> > > +       { CCI_REG8(0x0b9e), 0x00 },
> > > +       { CCI_REG8(0x0ba0), 0x03 },
> > > +       { CCI_REG8(0x0ba1), 0xd8 },
> > > +       { CCI_REG8(0x0ba2), 0x00 },
> > > +       { CCI_REG8(0x0ba4), 0x00 },
> > > +       { CCI_REG8(0x0ba5), 0x04 },
> > > +       { CCI_REG8(0x0ba6), 0x00 },
> > > +       { CCI_REG8(0x0ba8), 0x05 },
> > > +       { CCI_REG8(0x0ba9), 0x4d },
> > > +       { CCI_REG8(0x0baa), 0x00 },
> > > +       { CCI_REG8(0x0bac), 0x00 },
> > > +       { CCI_REG8(0x0bad), 0x05 },
> > > +       { CCI_REG8(0x0bae), 0x00 },
> > > +       { CCI_REG8(0x0bb0), 0x07 },
> > > +       { CCI_REG8(0x0bb1), 0x3e },
> > > +       { CCI_REG8(0x0bb2), 0x00 },
> > > +       { CCI_REG8(0x0bb4), 0x00 },
> > > +       { CCI_REG8(0x0bb5), 0x06 },
> > > +       { CCI_REG8(0x0bb6), 0x00 },
> > > +       { CCI_REG8(0x0bb8), 0x0a },
> > > +       { CCI_REG8(0x0bb9), 0x1a },
> > > +       { CCI_REG8(0x0bba), 0x00 },
> > > +       { CCI_REG8(0x0bbc), 0x09 },
> > > +       { CCI_REG8(0x0bbd), 0x36 },
> > > +       { CCI_REG8(0x0bbe), 0x00 },
> > > +       { CCI_REG8(0x0bc0), 0x0e },
> > > +       { CCI_REG8(0x0bc1), 0x66 },
> > > +       { CCI_REG8(0x0bc2), 0x00 },
> > > +       { CCI_REG8(0x0bc4), 0x10 },
> > > +       { CCI_REG8(0x0bc5), 0x06 },
> > > +       { CCI_REG8(0x0bc6), 0x00 },
> > > +       { CCI_REG8(0x02c1), 0xe0 },
> > > +       { CCI_REG8(0x0207), 0x04 },
> > > +       { CCI_REG8(0x02c2), 0x10 },
> > > +       { CCI_REG8(0x02c3), 0x74 },
> > > +       { CCI_REG8(0x02c5), 0x09 },
> > > +       { CCI_REG8(0x02c1), 0xe0 },
> > > +       { CCI_REG8(0x0207), 0x04 },
> > > +       { CCI_REG8(0x02c2), 0x10 },
> > > +       { CCI_REG8(0x02c5), 0x09 },
> > > +       { CCI_REG8(0x02c1), 0xe0 },
> > > +       { CCI_REG8(0x0207), 0x04 },
> > > +       { CCI_REG8(0x02c2), 0x10 },
> > > +       { CCI_REG8(0x02c5), 0x09 },
> > > +
> > > +       /* auto load CH_GAIN */
> > > +       { CCI_REG8(0x0aa1), 0x15 },
> > > +       { CCI_REG8(0x0aa2), 0x50 },
> > > +       { CCI_REG8(0x0aa3), 0x00 },
> > > +       { CCI_REG8(0x0aa4), 0x09 },
> > > +       { CCI_REG8(0x0a90), 0x25 },
> > > +       { CCI_REG8(0x0a91), 0x0e },
> > > +       { CCI_REG8(0x0a94), 0x80 },
> > > +
> > > +       /* ISP */
> > > +       { CCI_REG8(0x0050), 0x00 },
> > > +       { CCI_REG8(0x0089), 0x83 },
> > > +       { CCI_REG8(0x005a), 0x40 },
> > > +       { CCI_REG8(0x00c3), 0x35 },
> > > +       { CCI_REG8(0x00c4), 0x80 },
> > > +       { CCI_REG8(0x0080), 0x10 },
> > > +       { CCI_REG8(0x0040), 0x12 },
> > > +       { CCI_REG8(0x0053), 0x0a },
> > > +       { CCI_REG8(0x0054), 0x44 },
> > > +       { CCI_REG8(0x0055), 0x32 },
> > > +       { CCI_REG8(0x0058), 0x89 },
> > > +       { CCI_REG8(0x004a), 0x03 },
> > > +       { CCI_REG8(0x0048), 0xf0 },
> > > +       { CCI_REG8(0x0049), 0x0f },
> > > +       { CCI_REG8(0x0041), 0x20 },
> > > +       { CCI_REG8(0x0043), 0x0a },
> > > +       { CCI_REG8(0x009d), 0x08 },
> > > +       { CCI_REG8(0x0236), 0x40 },
> > > +
> > > +       /* gain */
> > 
> > Is the gain configurable? Is this analogue gain? digital gain? or
> > colour
> > balanace gains ?
> > 
> > 
> > > +       { CCI_REG8(0x0204), 0x04 },
> > > +       { CCI_REG8(0x0205), 0x00 },
> > > +       { CCI_REG8(0x02b3), 0x00 },
> > > +       { CCI_REG8(0x02b4), 0x00 },
> > > +       { CCI_REG8(0x009e), 0x01 },
> > > +       { CCI_REG8(0x009f), 0x94 },
> > > +
> > > +       /* auto load REG */
> > > +       { CCI_REG8(0x0aa1), 0x10 },
> > > +       { CCI_REG8(0x0aa2), 0xf8 },
> > > +       { CCI_REG8(0x0aa3), 0x00 },
> > > +       { CCI_REG8(0x0aa4), 0x1f },
> > > +       { CCI_REG8(0x0a90), 0x11 },
> > > +       { CCI_REG8(0x0a91), 0x0e },
> > > +       { CCI_REG8(0x0a94), 0x80 },
> > > +       { CCI_REG8(0x03fe), 0x00 },
> > > +       { CCI_REG8(0x0a90), 0x00 },
> > > +       { CCI_REG8(0x0a70), 0x00 },
> > > +       { CCI_REG8(0x0a67), 0x00 },
> > > +       { CCI_REG8(0x0af4), 0x29 },
> > > +
> > > +       /* DPHY */
> > > +       { CCI_REG8(0x0d80), 0x07 },
> > > +       { CCI_REG8(0x0dd3), 0x18 },
> > > +
> > > +       /* CISCTL_Reset */
> > > +       { CCI_REG8(0x031c), 0x80 },
> > > +       { CCI_REG8(0x03fe), 0x30 },
> > > +       { CCI_REG8(0x0d17), 0x06 },
> > > +       { CCI_REG8(0x03fe), 0x00 },
> > > +       { CCI_REG8(0x0d17), 0x00 },
> > > +       { CCI_REG8(0x031c), 0x93 },
> > > +       { CCI_REG8(0x03fe), 0x00 },
> > > +       { CCI_REG8(0x031c), 0x80 },
> > > +       { CCI_REG8(0x03fe), 0x30 },
> > > +       { CCI_REG8(0x0d17), 0x06 },
> > > +       { CCI_REG8(0x03fe), 0x00 },
> > > +       { CCI_REG8(0x0d17), 0x00 },
> > > +       { CCI_REG8(0x031c), 0x93 },
> > > +};
> > > +
> > > +struct gc05a2_mode {
> > > +       u32 width;
> > > +       u32 height;
> > > +       const struct gc05a2_reg_list reg_list;
> > > +
> > > +       u32 hts; /* Horizontal timining size */
> > > +       u32 vts_def; /* Default vertical timining size */
> > > +       u32 vts_min; /* Min vertical timining size */
> > > +};
> > > +
> > > +/* Declare modes in order, from biggest to smallest height. */
> > > +static const struct gc05a2_mode gc05a2_modes[] = {
> > > +       {
> > > +               /* 2592*1944@30fps */
> > > +               .width = GC05A2_NATIVE_WIDTH,
> > > +               .height = GC05A2_NATIVE_HEIGHT,
> > > +               .reg_list = {
> > > +                       .num_of_regs = ARRAY_SIZE(mode_2592x1944),
> > > +                       .regs = mode_2592x1944,
> > > +               },
> > > +               .hts = 3664,
> > > +               .vts_def = 2032,
> > > +               .vts_min = 2032,
> > > +       },
> > > +       {
> > > +               /* 1280*720@60fps */
> > > +               .width = 1280,
> > > +               .height = 720,
> > > +               .reg_list = {
> > > +                       .num_of_regs = ARRAY_SIZE(mode_1280x720),
> > > +                       .regs = mode_1280x720,
> > > +               },
> > > +               .hts = 3616,
> > > +               .vts_def = 1032,
> > > +               .vts_min = 1032,
> > > +       },
> > > +};
> > > +
> > > +static inline struct gc05a2 *to_gc05a2(struct v4l2_subdev *sd)
> > > +{
> > > +       return container_of(sd, struct gc05a2, sd);
> > > +}
> > > +
> > > +static int gc05a2_power_on(struct device *dev)
> > > +{
> > > +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > +       int ret;
> > > +
> > > +       ret = regulator_bulk_enable(ARRAY_SIZE(gc05a2_supply_name),
> > > +                                   gc05a2->supplies);
> > > +       if (ret < 0) {
> > > +               dev_err(gc05a2->dev, "failed to enable regulators:
> > %d\n", ret);
> > > +               return ret;
> > > +       }
> > > +
> > > +       ret = clk_prepare_enable(gc05a2->xclk);
> > > +       if (ret < 0) {
> > >
> > +               regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name)
> > ,
> > > +                                      gc05a2->supplies);
> > > +               dev_err(gc05a2->dev, "clk prepare enable
> > failed\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       fsleep(GC05A2_SLEEP_US);
> > > +
> > > +       gpiod_set_value_cansleep(gc05a2->reset_gpio, 0);
> > > +       fsleep(GC05A2_SLEEP_US);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_power_off(struct device *dev)
> > > +{
> > > +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > +
> > > +       clk_disable_unprepare(gc05a2->xclk);
> > > +       gpiod_set_value_cansleep(gc05a2->reset_gpio, 1);
> > > +       regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name),
> > > +                              gc05a2->supplies);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_enum_mbus_code(struct v4l2_subdev *sd,
> > > +                                struct v4l2_subdev_state
> > *sd_state,
> > > +                                struct v4l2_subdev_mbus_code_enum
> > *code)
> > > +{
> > > +       if (code->index > 0)
> > > +               return -EINVAL;
> > > +
> > > +       code->code = GC05A2_MBUS_CODE;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_enum_frame_size(struct v4l2_subdev *subdev,
> > > +                                 struct v4l2_subdev_state
> > *sd_state,
> > > +                                 struct
> > v4l2_subdev_frame_size_enum *fse)
> > > +{
> > > +       if (fse->code != GC05A2_MBUS_CODE)
> > > +               return -EINVAL;
> > > +
> > > +       if (fse->index >= ARRAY_SIZE(gc05a2_modes))
> > > +               return -EINVAL;
> > > +
> > > +       fse->min_width = gc05a2_modes[fse->index].width;
> > > +       fse->max_width = gc05a2_modes[fse->index].width;
> > > +       fse->min_height = gc05a2_modes[fse->index].height;
> > > +       fse->max_height = gc05a2_modes[fse->index].height;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_update_cur_mode_controls(struct gc05a2 *gc05a2,
> > > +                                          const struct gc05a2_mode
> > *mode)
> > > +{
> > > +       s64 exposure_max, h_blank;
> > > +       int ret;
> > > +
> > > +       ret = __v4l2_ctrl_modify_range(gc05a2->vblank,
> > > +                                      mode->vts_min - mode-
> > >height,
> > > +                                      GC05A2_VTS_MAX - mode-
> > >height, 1,
> > > +                                      mode->vts_def - mode-
> > >height);
> > > +       if (ret) {
> > > +               dev_err(gc05a2->dev, "VB ctrl range update
> > failed\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       h_blank = mode->hts - mode->width;
> > > +       ret = __v4l2_ctrl_modify_range(gc05a2->hblank, h_blank,
> > h_blank, 1,
> > > +                                      h_blank);
> > > +       if (ret) {
> > > +               dev_err(gc05a2->dev, "HB ctrl range update
> > failed\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > > +       ret = __v4l2_ctrl_modify_range(gc05a2->exposure,
> > GC05A2_EXP_MIN,
> > > +                                      exposure_max,
> > GC05A2_EXP_STEP,
> > > +                                      exposure_max);
> > > +       if (ret) {
> > > +               dev_err(gc05a2->dev, "exposure ctrl range update
> > failed\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static void gc05a2_update_pad_format(struct gc05a2 *gc08a3,
> > > +                                    const struct gc05a2_mode
> > *mode,
> > > +                                    struct v4l2_mbus_framefmt
> > *fmt)
> > > +{
> > > +       fmt->width = mode->width;
> > > +       fmt->height = mode->height;
> > > +       fmt->code = GC05A2_MBUS_CODE;
> > > +       fmt->field = V4L2_FIELD_NONE;
> > > +       fmt->colorspace = V4L2_COLORSPACE_RAW;
> > > +       fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt-
> > >colorspace);
> > > +       fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> > > +       fmt->xfer_func = V4L2_XFER_FUNC_NONE;
> > > +}
> > > +
> > > +static int gc05a2_set_format(struct v4l2_subdev *sd,
> > > +                            struct v4l2_subdev_state *state,
> > > +                            struct v4l2_subdev_format *fmt)
> > > +{
> > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > +       struct v4l2_mbus_framefmt *mbus_fmt;
> > > +       struct v4l2_rect *crop;
> > > +       const struct gc05a2_mode *mode;
> > > +
> > > +       mode = v4l2_find_nearest_size(gc05a2_modes,
> > ARRAY_SIZE(gc05a2_modes),
> > > +                                     width, height, fmt-
> > >format.width,
> > > +                                     fmt->format.height);
> > > +
> > > +       /* update crop info to subdev state */
> > > +       crop = v4l2_subdev_state_get_crop(state, 0);
> > > +       crop->width = mode->width;
> > > +       crop->height = mode->height;
> > > +
> > > +       /* update fmt info to subdev state */
> > > +       gc05a2_update_pad_format(gc05a2, mode, &fmt->format);
> > > +       mbus_fmt = v4l2_subdev_state_get_format(state, 0);
> > > +       *mbus_fmt = fmt->format;
> > > +
> > > +       if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > > +               return 0;
> > > +       gc05a2->cur_mode = mode;
> > > +       gc05a2_update_cur_mode_controls(gc05a2, mode);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_get_selection(struct v4l2_subdev *sd,
> > > +                               struct v4l2_subdev_state *state,
> > > +                               struct v4l2_subdev_selection *sel)
> > > +{
> > > +       switch (sel->target) {
> > > +       case V4L2_SEL_TGT_CROP_DEFAULT:
> > > +       case V4L2_SEL_TGT_CROP:
> > > +               sel->r = *v4l2_subdev_state_get_crop(state, 0);
> > > +               break;
> > > +       case V4L2_SEL_TGT_CROP_BOUNDS:
> > > +               sel->r.top = 0;
> > > +               sel->r.left = 0;
> > > +               sel->r.width = GC05A2_NATIVE_WIDTH;
> > > +               sel->r.height = GC05A2_NATIVE_HEIGHT;
> > > +               break;
> > > +       default:
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_init_state(struct v4l2_subdev *sd,
> > > +                            struct v4l2_subdev_state *state)
> > > +{
> > > +       struct v4l2_subdev_format fmt = {
> > > +               .which = V4L2_SUBDEV_FORMAT_TRY,
> > > +               .pad = 0,
> > > +               .format = {
> > > +                       .code = GC05A2_MBUS_CODE,
> > > +                       .width = gc05a2_modes[0].width,
> > > +                       .height = gc05a2_modes[0].height,
> > > +               },
> > > +       };
> > > +
> > > +       gc05a2_set_format(sd, state, &fmt);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_set_ctrl_hflip(struct gc05a2 *gc05a2, u32
> > ctrl_val)
> > > +{
> > > +       int ret;
> > > +       u64 val;
> > > +
> > > +       ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> > NULL);
> > > +       if (ret) {
> > > +               dev_err(gc05a2->dev, "read hflip register failed:
> > %d\n", ret);
> > > +               return ret;
> > > +       }
> > > +
> > > +       return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > > +                              GC05A2_FLIP_H_MASK,
> > > +                              ctrl_val ? GC05A2_FLIP_H_MASK : 0,
> > NULL);
> > > +}
> > > +
> > > +static int gc05a2_set_ctrl_vflip(struct gc05a2 *gc05a2, u32
> > ctrl_val)
> > > +{
> > > +       int ret;
> > > +       u64 val;
> > > +
> > > +       ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> > NULL);
> > > +       if (ret) {
> > > +               dev_err(gc05a2->dev, "read vflip register failed:
> > %d\n", ret);
> > > +               return ret;
> > > +       }
> > > +
> > > +       return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > > +                              GC05A2_FLIP_V_MASK,
> > > +                              ctrl_val ? GC05A2_FLIP_V_MASK : 0,
> > NULL);
> > > +}
> > > +
> > > +static int gc05a2_test_pattern(struct gc05a2 *gc05a2, u32
> > pattern_menu)
> > > +{
> > > +       u32 pattern;
> > > +       int ret;
> > > +
> > > +       if (pattern_menu) {
> > > +               switch (pattern_menu) {
> > > +               case 1:
> > > +               case 2:
> > > +               case 3:
> > > +               case 4:
> > > +               case 5:
> > > +               case 6:
> > > +               case 7:
> > > +                       pattern = pattern_menu << 4;
> > > +                       break;
> > > +
> > > +               case 8:
> > > +                       pattern = 0;
> > > +                       break;
> > > +
> > > +               case 9:
> > > +                       pattern = 4;
> > > +                       break;
> > > +
> > > +               default:
> > > +                       pattern = 0x00;
> > > +                       break;
> > > +               }
> > 
> > This is fairly terse. Can we add comments, or definitions for the
> > types
> > or such so that the above is easier to interpret?
> > 
> > > +
> > > +               ret = cci_write(gc05a2->regmap,
> > GC05A2_REG_TEST_PATTERN_IDX,
> > > +                               pattern, NULL);
> > > +               if (ret)
> > > +                       return ret;
> > > +
> > > +               return cci_write(gc05a2->regmap,
> > GC05A2_REG_TEST_PATTERN_EN,
> > > +                                GC05A2_TEST_PATTERN_EN, NULL);
> > > +       } else {
> > > +               return cci_write(gc05a2->regmap,
> > GC05A2_REG_TEST_PATTERN_EN,
> > > +                                0x00, NULL);
> > > +       }
> > > +}
> > > +
> > > +static int gc05a2_set_ctrl(struct v4l2_ctrl *ctrl)
> > > +{
> > > +       struct gc05a2 *gc05a2 =
> > > +               container_of(ctrl->handler, struct gc05a2, ctrls);
> > > +       int ret = 0;
> > > +       s64 exposure_max;
> > > +       struct v4l2_subdev_state *state;
> > > +       const struct v4l2_mbus_framefmt *format;
> > > +
> > > +       state = v4l2_subdev_get_locked_active_state(&gc05a2->sd);
> > > +       format = v4l2_subdev_state_get_format(state, 0);
> > > +
> > > +       if (ctrl->id == V4L2_CID_VBLANK) {
> > > +               /* Update max exposure while meeting expected
> > vblanking */
> > > +               exposure_max = format->height + ctrl->val -
> > GC05A2_EXP_MARGIN;
> > > +               __v4l2_ctrl_modify_range(gc05a2->exposure,
> > > +                                        gc05a2->exposure->minimum,
> > > +                                        exposure_max, gc05a2-
> > >exposure->step,
> > > +                                        exposure_max);
> > > +       }
> > > +
> > > +       /*
> > > +        * Applying V4L2 control value only happens
> > > +        * when power is on for streaming.
> > > +        */
> > > +       if (!pm_runtime_get_if_active(gc05a2->dev))
> > > +               return 0;
> > > +
> > > +       switch (ctrl->id) {
> > > +       case V4L2_CID_EXPOSURE:
> > > +               ret = cci_write(gc05a2->regmap, GC05A2_EXP_REG,
> > > +                               ctrl->val, NULL);
> > > +               break;
> > > +
> > > +       case V4L2_CID_ANALOGUE_GAIN:
> > > +               ret = cci_write(gc05a2->regmap, GC05A2_AGAIN_REG,
> > > +                               ctrl->val, NULL);
> > > +               break;
> > > +
> > > +       case V4L2_CID_VBLANK:
> > > +               ret = cci_write(gc05a2->regmap,
> > GC05A2_FRAME_LENGTH_REG,
> > > +                               gc05a2->cur_mode->height + ctrl-
> > >val, NULL);
> > > +               break;
> > > +
> > > +       case V4L2_CID_HFLIP:
> > > +               ret = gc05a2_set_ctrl_hflip(gc05a2, ctrl->val);
> > > +               break;
> > > +
> > > +       case V4L2_CID_VFLIP:
> > > +               ret = gc05a2_set_ctrl_vflip(gc05a2, ctrl->val);
> > > +               break;
> > > +
> > > +       case V4L2_CID_TEST_PATTERN:
> > > +               ret = gc05a2_test_pattern(gc05a2, ctrl->val);
> > > +               break;
> > > +
> > > +       default:
> > > +               break;
> > > +       }
> > > +
> > > +       pm_runtime_put(gc05a2->dev);
> > > +
> > > +       return ret;
> > > +}
> > > +
> > > +static const struct v4l2_ctrl_ops gc05a2_ctrl_ops = {
> > > +       .s_ctrl = gc05a2_set_ctrl,
> > > +};
> > > +
> > > +static int gc05a2_identify_module(struct gc05a2 *gc05a2)
> > > +{
> > > +       u64 val;
> > > +       int ret;
> > > +
> > > +       if (gc05a2->identified)
> > > +               return 0;
> > > +
> > > +       ret = cci_read(gc05a2->regmap, GC05A2_REG_CHIP_ID, &val,
> > NULL);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       if (val != GC05A2_CHIP_ID) {
> > > +               dev_err(gc05a2->dev, "chip id mismatch:
> > 0x%x!=0x%llx",
> > > +                       GC05A2_CHIP_ID, val);
> > > +               return -ENXIO;
> > > +       }
> > > +
> > > +       gc05a2->identified = true;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int gc05a2_start_streaming(struct gc05a2 *gc05a2)
> > > +{
> > > +       const struct gc05a2_mode *mode;
> > > +       const struct gc05a2_reg_list *reg_list;
> > > +       int ret;
> > > +
> > > +       ret = pm_runtime_resume_and_get(gc05a2->dev);
> > > +       if (ret < 0)
> > > +               return ret;
> > > +
> > > +       ret = gc05a2_identify_module(gc05a2);
> > > +       if (ret)
> > > +               goto err_rpm_put;
> > > +
> > > +       ret = cci_multi_reg_write(gc05a2->regmap,
> > > +                                 mode_table_common,
> > > +                                 ARRAY_SIZE(mode_table_common),
> > NULL);
> > > +       if (ret)
> > > +               goto err_rpm_put;
> > > +
> > > +       mode = gc05a2->cur_mode;
> > > +       reg_list = &mode->reg_list;
> > > +
> > > +       ret = cci_multi_reg_write(gc05a2->regmap,
> > > +                                 reg_list->regs, reg_list-
> > >num_of_regs, NULL);
> > > +       if (ret < 0)
> > > +               goto err_rpm_put;
> > > +
> > > +       ret = __v4l2_ctrl_handler_setup(&gc05a2->ctrls);
> > > +       if (ret < 0) {
> > > +               dev_err(gc05a2->dev, "could not sync v4l2
> > controls\n");
> > > +               goto err_rpm_put;
> > > +       }
> > > +
> > > +       ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 1,
> > NULL);
> > > +       if (ret < 0) {
> > > +               dev_err(gc05a2->dev, "write STREAMING_REG failed:
> > %d\n", ret);
> > > +               goto err_rpm_put;
> > > +       }
> > > +
> > > +       return 0;
> > > +
> > > +err_rpm_put:
> > > +       pm_runtime_put(gc05a2->dev);
> > > +       return ret;
> > > +}
> > > +
> > > +static int gc05a2_stop_streaming(struct gc05a2 *gc05a2)
> > > +{
> > > +       int ret;
> > > +
> > > +       ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 0,
> > NULL);
> > > +       if (ret < 0)
> > > +               dev_err(gc05a2->dev, "could not sent stop streaming
> > %d\n", ret);
> > > +
> > > +       pm_runtime_put(gc05a2->dev);
> > > +       return ret;
> > > +}
> > > +
> > > +static int gc05a2_s_stream(struct v4l2_subdev *subdev, int enable)
> > > +{
> > > +       struct gc05a2 *gc05a2 = to_gc05a2(subdev);
> > > +       struct v4l2_subdev_state *state;
> > > +       int ret;
> > > +
> > > +       state = v4l2_subdev_lock_and_get_active_state(subdev);
> > > +
> > > +       if (enable)
> > > +               ret = gc05a2_start_streaming(gc05a2);
> > > +       else
> > > +               ret = gc05a2_stop_streaming(gc05a2);
> > > +
> > > +       v4l2_subdev_unlock_state(state);
> > > +
> > > +       return ret;
> > > +}
> > > +
> > > +static const struct v4l2_subdev_video_ops gc05a2_video_ops = {
> > > +       .s_stream = gc05a2_s_stream,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_pad_ops gc05a2_subdev_pad_ops = {
> > > +       .enum_mbus_code = gc05a2_enum_mbus_code,
> > > +       .enum_frame_size = gc05a2_enum_frame_size,
> > > +       .get_fmt = v4l2_subdev_get_fmt,
> > > +       .set_fmt = gc05a2_set_format,
> > > +       .get_selection = gc05a2_get_selection,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_core_ops gc05a2_core_ops = {
> > > +       .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> > > +       .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_ops gc05a2_subdev_ops = {
> > > +       .core = &gc05a2_core_ops,
> > > +       .video = &gc05a2_video_ops,
> > > +       .pad = &gc05a2_subdev_pad_ops,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_internal_ops gc05a2_internal_ops =
> > {
> > > +       .init_state = gc05a2_init_state,
> > > +};
> > > +
> > > +static int gc05a2_get_regulators(struct device *dev, struct gc05a2
> > *gc05a2)
> > > +{
> > > +       unsigned int i;
> > > +
> > > +       for (i = 0; i < ARRAY_SIZE(gc05a2_supply_name); i++)
> > > +               gc05a2->supplies[i].supply = gc05a2_supply_name[i];
> > > +
> > > +       return devm_regulator_bulk_get(dev,
> > ARRAY_SIZE(gc05a2_supply_name),
> > > +                                      gc05a2->supplies);
> > > +}
> > > +
> > > +static int gc05a2_parse_fwnode(struct gc05a2 *gc05a2)
> > > +{
> > > +       struct fwnode_handle *endpoint;
> > > +       struct v4l2_fwnode_endpoint bus_cfg = {
> > > +               .bus_type = V4L2_MBUS_CSI2_DPHY,
> > > +       };
> > > +       int ret;
> > > +       struct device *dev = gc05a2->dev;
> > > +
> > > +       endpoint =
> > > +               fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0,
> > 0,
> > >
> > +                                               FWNODE_GRAPH_ENDPOINT
> > _NEXT);
> > > +       if (!endpoint) {
> > > +               dev_err(dev, "endpoint node not found\n");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg);
> > > +       if (ret) {
> > > +               dev_err(dev, "parsing endpoint node failed\n");
> > > +               goto done;
> > > +       }
> > > +
> > > +       ret = v4l2_link_freq_to_bitmap(dev,
> > bus_cfg.link_frequencies,
> > >
> > +                                      bus_cfg.nr_of_link_frequencies
> > ,
> > > +                                      gc05a2_link_freq_menu_items,
> > >
> > +                                      ARRAY_SIZE(gc05a2_link_freq_me
> > nu_items),
> > > +                                      &gc05a2->link_freq_bitmap);
> > > +       if (ret)
> > > +               goto done;
> > > +
> > > +done:
> > > +       v4l2_fwnode_endpoint_free(&bus_cfg);
> > > +       fwnode_handle_put(endpoint);
> > > +       return ret;
> > > +}
> > > +
> > > +static u64 gc05a2_to_pixel_rate(u32 f_index)
> > > +{
> > > +       u64 pixel_rate =
> > > +               gc05a2_link_freq_menu_items[f_index] * 2 *
> > GC05A2_DATA_LANES;
> > > +
> > > +       return div_u64(pixel_rate, GC05A2_RGB_DEPTH);
> > > +}
> > > +
> > > +static int gc05a2_init_controls(struct gc05a2 *gc05a2)
> > > +{
> > > +       struct i2c_client *client = v4l2_get_subdevdata(&gc05a2-
> > >sd);
> > > +       const struct gc05a2_mode *mode = &gc05a2_modes[0];
> > > +       const struct v4l2_ctrl_ops *ops = &gc05a2_ctrl_ops;
> > > +       struct v4l2_fwnode_device_properties props;
> > > +       struct v4l2_ctrl_handler *ctrl_hdlr;
> > > +       s64 exposure_max, h_blank;
> > > +       int ret;
> > > +
> > > +       ctrl_hdlr = &gc05a2->ctrls;
> > > +       ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       gc05a2->hflip = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > +                                         V4L2_CID_HFLIP, 0, 1, 1,
> > 0);
> > > +       gc05a2->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > +                                         V4L2_CID_VFLIP, 0, 1, 1,
> > 0);
> > > +       v4l2_ctrl_cluster(2, &gc05a2->hflip);
> > > +
> > > +       gc05a2->link_freq =
> > > +       v4l2_ctrl_new_int_menu(ctrl_hdlr,
> > > +                              &gc05a2_ctrl_ops,
> > > +                              V4L2_CID_LINK_FREQ,
> > >
> > +                              ARRAY_SIZE(gc05a2_link_freq_menu_items
> > ) - 1,
> > > +                              0,
> > > +                              gc05a2_link_freq_menu_items);
> > > +       if (gc05a2->link_freq)
> > > +               gc05a2->link_freq->flags |=
> > V4L2_CTRL_FLAG_READ_ONLY;
> > > +
> > > +       gc05a2->pixel_rate =
> > > +               v4l2_ctrl_new_std(ctrl_hdlr,
> > > +                                 &gc05a2_ctrl_ops,
> > > +                                 V4L2_CID_PIXEL_RATE, 0,
> > > +                                 gc05a2_to_pixel_rate(0),
> > > +                                 1,
> > > +                                 gc05a2_to_pixel_rate(0));
> > > +
> > > +       gc05a2->vblank =
> > > +               v4l2_ctrl_new_std(ctrl_hdlr,
> > > +                                 &gc05a2_ctrl_ops,
> > V4L2_CID_VBLANK,
> > > +                                 mode->vts_min - mode->height,
> > > +                                 GC05A2_VTS_MAX - mode->height, 1,
> > > +                                 mode->vts_def - mode->height);
> > > +
> > > +       h_blank = mode->hts - mode->width;
> > > +       gc05a2->hblank = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > +                                          V4L2_CID_HBLANK,
> > h_blank, h_blank, 1,
> > > +                                          h_blank);
> > > +       if (gc05a2->hblank)
> > > +               gc05a2->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > > +
> > > +       v4l2_ctrl_new_std(ctrl_hdlr, &gc05a2_ctrl_ops,
> > > +                         V4L2_CID_ANALOGUE_GAIN, GC05A2_AGAIN_MIN,
> > > +                         GC05A2_AGAIN_MAX, GC05A2_AGAIN_STEP,
> > > +                         GC05A2_AGAIN_MIN);
> > > +
> > > +       exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > > +       gc05a2->exposure = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > +                                            V4L2_CID_EXPOSURE,
> > GC05A2_EXP_MIN,
> > > +                                            exposure_max,
> > GC05A2_EXP_STEP,
> > > +                                            exposure_max);
> > > +
> > > +       v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &gc05a2_ctrl_ops,
> > > +                                    V4L2_CID_TEST_PATTERN,
> > >
> > +                                    ARRAY_SIZE(gc05a2_test_pattern_m
> > enu) - 1,
> > > +                                    0, 0,
> > gc05a2_test_pattern_menu);
> > > +
> > > +       /* register properties to fwnode (e.g. rotation,
> > orientation) */
> > > +       ret = v4l2_fwnode_device_parse(&client->dev, &props);
> > > +       if (ret)
> > > +               goto error_ctrls;
> > > +
> > > +       ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, ops,
> > &props);
> > > +       if (ret)
> > > +               goto error_ctrls;
> > > +
> > > +       if (ctrl_hdlr->error) {
> > > +               ret = ctrl_hdlr->error;
> > > +               goto error_ctrls;
> > > +       }
> > > +
> > > +       gc05a2->sd.ctrl_handler = ctrl_hdlr;
> > > +
> > > +       return 0;
> > > +
> > > +error_ctrls:
> > > +       v4l2_ctrl_handler_free(ctrl_hdlr);
> > > +
> > > +       return ret;
> > > +}
> > > +
> > > +static int gc05a2_probe(struct i2c_client *client)
> > > +{
> > > +       struct device *dev = &client->dev;
> > > +       struct gc05a2 *gc05a2;
> > > +       int ret;
> > > +
> > > +       gc05a2 = devm_kzalloc(dev, sizeof(*gc05a2), GFP_KERNEL);
> > > +       if (!gc05a2)
> > > +               return -ENOMEM;
> > > +
> > > +       gc05a2->dev = dev;
> > > +
> > > +       ret = gc05a2_parse_fwnode(gc05a2);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       gc05a2->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > +       if (IS_ERR(gc05a2->regmap))
> > > +               return dev_err_probe(dev, PTR_ERR(gc05a2->regmap),
> > > +                                    "failed to init CCI\n");
> > > +
> > > +       gc05a2->xclk = devm_clk_get(dev, NULL);
> > > +       if (IS_ERR(gc05a2->xclk))
> > > +               return dev_err_probe(dev, PTR_ERR(gc05a2->xclk),
> > > +                                    "failed to get xclk\n");
> > > +
> > > +       ret = clk_set_rate(gc05a2->xclk, GC05A2_DEFAULT_CLK_FREQ);
> > > +       if (ret)
> > > +               return dev_err_probe(dev, ret,
> > > +                                    "failed to set xclk
> > frequency\n");
> > > +
> > > +       ret = gc05a2_get_regulators(dev, gc05a2);
> > > +       if (ret < 0)
> > > +               return dev_err_probe(dev, ret,
> > > +                                    "failed to get regulators\n");
> > > +
> > > +       gc05a2->reset_gpio = devm_gpiod_get(dev, "reset",
> > GPIOD_OUT_LOW);
> > > +       if (IS_ERR(gc05a2->reset_gpio))
> > > +               return dev_err_probe(dev, PTR_ERR(gc05a2-
> > >reset_gpio),
> > > +                                    "failed to get gpio\n");
> > > +
> > > +       v4l2_i2c_subdev_init(&gc05a2->sd, client,
> > &gc05a2_subdev_ops);
> > > +       gc05a2->sd.internal_ops = &gc05a2_internal_ops;
> > > +       gc05a2->cur_mode = &gc05a2_modes[0];
> > > +
> > > +       ret = gc05a2_init_controls(gc05a2);
> > > +       if (ret)
> > > +               return dev_err_probe(dev, ret,
> > > +                                    "failed to init controls\n");
> > > +
> > > +       gc05a2->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > > +                           V4L2_SUBDEV_FL_HAS_EVENTS;
> > > +       gc05a2->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > +       gc05a2->sd.dev = &client->dev;
> > > +       gc05a2->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > +
> > > +       ret = media_entity_pads_init(&gc05a2->sd.entity, 1,
> > &gc05a2->pad);
> > > +       if (ret < 0) {
> > > +               dev_err(dev, "could not register media entity\n");
> > > +               goto err_v4l2_ctrl_handler_free;
> > > +       }
> > > +
> > > +       gc05a2->sd.state_lock = gc05a2->ctrls.lock;
> > > +       ret = v4l2_subdev_init_finalize(&gc05a2->sd);
> > > +       if (ret < 0) {
> > > +               dev_err(dev, "v4l2 subdev init error: %d\n", ret);
> > > +               goto err_media_entity_cleanup;
> > > +       }
> > > +
> > > +       pm_runtime_set_active(gc05a2->dev);
> > > +       pm_runtime_enable(gc05a2->dev);
> > > +       pm_runtime_set_autosuspend_delay(gc05a2->dev, 1000);
> > > +       pm_runtime_use_autosuspend(gc05a2->dev);
> > > +       pm_runtime_idle(gc05a2->dev);
> > > +
> > > +       ret = v4l2_async_register_subdev_sensor(&gc05a2->sd);
> > > +       if (ret < 0) {
> > > +               dev_err(dev, "could not register v4l2 device\n");
> > > +               goto err_rpm;
> > > +       }
> > > +
> > > +       return 0;
> > > +
> > > +err_rpm:
> > > +       pm_runtime_disable(gc05a2->dev);
> > > +       v4l2_subdev_cleanup(&gc05a2->sd);
> > > +
> > > +err_media_entity_cleanup:
> > > +       media_entity_cleanup(&gc05a2->sd.entity);
> > > +
> > > +err_v4l2_ctrl_handler_free:
> > > +       v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > > +
> > > +       return ret;
> > > +}
> > > +
> > > +static void gc05a2_remove(struct i2c_client *client)
> > > +{
> > > +       struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > +
> > > +       v4l2_async_unregister_subdev(&gc05a2->sd);
> > > +       v4l2_subdev_cleanup(sd);
> > > +       media_entity_cleanup(&gc05a2->sd.entity);
> > > +       v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > > +
> > > +       pm_runtime_disable(&client->dev);
> > > +       if (!pm_runtime_status_suspended(&client->dev))
> > > +               gc05a2_power_off(gc05a2->dev);
> > > +       pm_runtime_set_suspended(&client->dev);
> > > +}
> > > +
> > > +static const struct of_device_id gc05a2_of_match[] = {
> > > +       { .compatible = "galaxycore,gc05a2" },
> > > +       {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, gc05a2_of_match);
> > > +
> > > +static DEFINE_RUNTIME_DEV_PM_OPS(gc05a2_pm_ops,
> > > +                                gc05a2_power_off,
> > > +                                gc05a2_power_on,
> > > +                                NULL);
> > > +
> > > +static struct i2c_driver gc05a2_i2c_driver = {
> > > +       .driver = {
> > > +               .of_match_table = gc05a2_of_match,
> > > +               .pm = pm_ptr(&gc05a2_pm_ops),
> > > +               .name  = "gc05a2",
> > > +       },
> > > +       .probe = gc05a2_probe,
> > > +       .remove = gc05a2_remove,
> > > +};
> > > +module_i2c_driver(gc05a2_i2c_driver);
> > > +
> > > +MODULE_DESCRIPTION("GalaxyCore gc05a2 Camera driver");
> > > +MODULE_AUTHOR("Zhi Mao <zhi.mao@mediatek.com>");
> > > +MODULE_LICENSE("GPL");
> > > -- 
> > > 2.25.1
> > >

^ permalink raw reply

* [PATCH v20 4/9] usb: dwc3: core: Refactor PHY logic to support Multiport Controller
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
	Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, Johan Hovold
  Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
	quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>

Currently the DWC3 driver supports only single port controller
which requires at least one HS PHY and at most one SS PHY.

But the DWC3 USB controller can be connected to multiple ports and
each port can have their own PHYs. Each port of the multiport
controller can either be HS+SS capable or HS only capable
Proper quantification of them is required to modify GUSB2PHYCFG
and GUSB3PIPECTL registers appropriately.

Add support for detecting, obtaining and configuring PHYs supported
by a multiport controller. Limit support to multiport controllers
with up to four ports for now (e.g. as needed for SC8280XP).

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/core.c | 251 ++++++++++++++++++++++++++++------------
 drivers/usb/dwc3/core.h |  14 ++-
 drivers/usb/dwc3/drd.c  |  15 ++-
 3 files changed, 193 insertions(+), 87 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1a3d8a9beea8..1f4f228c970b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -124,6 +124,7 @@ static void __dwc3_set_mode(struct work_struct *work)
 	int ret;
 	u32 reg;
 	u32 desired_dr_role;
+	int i;
 
 	mutex_lock(&dwc->mutex);
 	spin_lock_irqsave(&dwc->lock, flags);
@@ -201,8 +202,10 @@ static void __dwc3_set_mode(struct work_struct *work)
 		} else {
 			if (dwc->usb2_phy)
 				otg_set_vbus(dwc->usb2_phy->otg, true);
-			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
-			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+			for (i = 0; i < dwc->num_usb2_ports; i++) {
+				phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+				phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
+			}
 			if (dwc->dis_split_quirk) {
 				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
 				reg |= DWC3_GUCTL3_SPLITDISABLE;
@@ -217,8 +220,8 @@ static void __dwc3_set_mode(struct work_struct *work)
 
 		if (dwc->usb2_phy)
 			otg_set_vbus(dwc->usb2_phy->otg, false);
-		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
-		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
+		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
+		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
 
 		ret = dwc3_gadget_init(dwc);
 		if (ret)
@@ -589,22 +592,14 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
 	return ret;
 }
 
-/**
- * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
- * @dwc: Pointer to our controller context structure
- *
- * Returns 0 on success. The USB PHY interfaces are configured but not
- * initialized. The PHY interfaces and the PHYs get initialized together with
- * the core in dwc3_core_init.
- */
-static int dwc3_phy_setup(struct dwc3 *dwc)
+static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
 {
 	unsigned int hw_mode;
 	u32 reg;
 
 	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
 
-	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
+	reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(index));
 
 	/*
 	 * Make sure UX_EXIT_PX is cleared as that causes issues with some
@@ -659,9 +654,19 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	if (dwc->dis_del_phy_power_chg_quirk)
 		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
 
-	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
 
-	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+	return 0;
+}
+
+static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
+{
+	unsigned int hw_mode;
+	u32 reg;
+
+	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+
+	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
 
 	/* Select the HS PHY interface */
 	switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
@@ -673,7 +678,7 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 		} else if (dwc->hsphy_interface &&
 				!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
 			reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
-			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
 		} else {
 			/* Relying on default value. */
 			if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
@@ -740,7 +745,35 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	if (dwc->ulpi_ext_vbus_drv)
 		reg |= DWC3_GUSB2PHYCFG_ULPIEXTVBUSDRV;
 
-	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
+
+	return 0;
+}
+
+/**
+ * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
+ * @dwc: Pointer to our controller context structure
+ *
+ * Returns 0 on success. The USB PHY interfaces are configured but not
+ * initialized. The PHY interfaces and the PHYs get initialized together with
+ * the core in dwc3_core_init.
+ */
+static int dwc3_phy_setup(struct dwc3 *dwc)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < dwc->num_usb3_ports; i++) {
+		ret = dwc3_ss_phy_setup(dwc, i);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < dwc->num_usb2_ports; i++) {
+		ret = dwc3_hs_phy_setup(dwc, i);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
@@ -748,23 +781,32 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 static int dwc3_phy_init(struct dwc3 *dwc)
 {
 	int ret;
+	int i;
+	int j;
 
 	usb_phy_init(dwc->usb2_phy);
 	usb_phy_init(dwc->usb3_phy);
 
-	ret = phy_init(dwc->usb2_generic_phy);
-	if (ret < 0)
-		goto err_shutdown_usb3_phy;
+	for (i = 0; i < dwc->num_usb2_ports; i++) {
+		ret = phy_init(dwc->usb2_generic_phy[i]);
+		if (ret < 0)
+			goto err_exit_phy;
 
-	ret = phy_init(dwc->usb3_generic_phy);
-	if (ret < 0)
-		goto err_exit_usb2_phy;
+		ret = phy_init(dwc->usb3_generic_phy[i]);
+		if (ret < 0) {
+			phy_exit(dwc->usb2_generic_phy[i]);
+			goto err_exit_phy;
+		}
+	}
 
 	return 0;
 
-err_exit_usb2_phy:
-	phy_exit(dwc->usb2_generic_phy);
-err_shutdown_usb3_phy:
+err_exit_phy:
+	for (j = i - 1; j >= 0; j--) {
+		phy_exit(dwc->usb3_generic_phy[j]);
+		phy_exit(dwc->usb2_generic_phy[j]);
+	}
+
 	usb_phy_shutdown(dwc->usb3_phy);
 	usb_phy_shutdown(dwc->usb2_phy);
 
@@ -773,8 +815,12 @@ static int dwc3_phy_init(struct dwc3 *dwc)
 
 static void dwc3_phy_exit(struct dwc3 *dwc)
 {
-	phy_exit(dwc->usb3_generic_phy);
-	phy_exit(dwc->usb2_generic_phy);
+	int i;
+
+	for (i = 0; i < dwc->num_usb2_ports; i++) {
+		phy_exit(dwc->usb3_generic_phy[i]);
+		phy_exit(dwc->usb2_generic_phy[i]);
+	}
 
 	usb_phy_shutdown(dwc->usb3_phy);
 	usb_phy_shutdown(dwc->usb2_phy);
@@ -783,23 +829,32 @@ static void dwc3_phy_exit(struct dwc3 *dwc)
 static int dwc3_phy_power_on(struct dwc3 *dwc)
 {
 	int ret;
+	int i;
+	int j;
 
 	usb_phy_set_suspend(dwc->usb2_phy, 0);
 	usb_phy_set_suspend(dwc->usb3_phy, 0);
 
-	ret = phy_power_on(dwc->usb2_generic_phy);
-	if (ret < 0)
-		goto err_suspend_usb3_phy;
+	for (i = 0; i < dwc->num_usb2_ports; i++) {
+		ret = phy_power_on(dwc->usb2_generic_phy[i]);
+		if (ret < 0)
+			goto err_power_off_phy;
 
-	ret = phy_power_on(dwc->usb3_generic_phy);
-	if (ret < 0)
-		goto err_power_off_usb2_phy;
+		ret = phy_power_on(dwc->usb3_generic_phy[i]);
+		if (ret < 0) {
+			phy_power_off(dwc->usb2_generic_phy[i]);
+			goto err_power_off_phy;
+		}
+	}
 
 	return 0;
 
-err_power_off_usb2_phy:
-	phy_power_off(dwc->usb2_generic_phy);
-err_suspend_usb3_phy:
+err_power_off_phy:
+	for (j = i - 1; j >= 0; j--) {
+		phy_power_off(dwc->usb3_generic_phy[j]);
+		phy_power_off(dwc->usb2_generic_phy[j]);
+	}
+
 	usb_phy_set_suspend(dwc->usb3_phy, 1);
 	usb_phy_set_suspend(dwc->usb2_phy, 1);
 
@@ -808,8 +863,12 @@ static int dwc3_phy_power_on(struct dwc3 *dwc)
 
 static void dwc3_phy_power_off(struct dwc3 *dwc)
 {
-	phy_power_off(dwc->usb3_generic_phy);
-	phy_power_off(dwc->usb2_generic_phy);
+	int i;
+
+	for (i = 0; i < dwc->num_usb2_ports; i++) {
+		phy_power_off(dwc->usb3_generic_phy[i]);
+		phy_power_off(dwc->usb2_generic_phy[i]);
+	}
 
 	usb_phy_set_suspend(dwc->usb3_phy, 1);
 	usb_phy_set_suspend(dwc->usb2_phy, 1);
@@ -1201,6 +1260,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	unsigned int		hw_mode;
 	u32			reg;
 	int			ret;
+	int			i;
 
 	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
 
@@ -1244,15 +1304,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
 	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
 		if (!dwc->dis_u3_susphy_quirk) {
-			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
-			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
-			dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+			for (i = 0; i < dwc->num_usb3_ports; i++) {
+				reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
+				reg |= DWC3_GUSB3PIPECTL_SUSPHY;
+				dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
+			}
 		}
 
 		if (!dwc->dis_u2_susphy_quirk) {
-			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
-			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
-			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+			for (i = 0; i < dwc->num_usb2_ports; i++) {
+				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+				reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+			}
 		}
 	}
 
@@ -1372,7 +1436,9 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
 {
 	struct device		*dev = dwc->dev;
 	struct device_node	*node = dev->of_node;
+	char phy_name[9];
 	int ret;
+	int i;
 
 	if (node) {
 		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
@@ -1398,22 +1464,36 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
 			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
 	}
 
-	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
-	if (IS_ERR(dwc->usb2_generic_phy)) {
-		ret = PTR_ERR(dwc->usb2_generic_phy);
-		if (ret == -ENOSYS || ret == -ENODEV)
-			dwc->usb2_generic_phy = NULL;
+	for (i = 0; i < dwc->num_usb2_ports; i++) {
+		if (dwc->num_usb2_ports == 1)
+			sprintf(phy_name, "usb2-phy");
 		else
-			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
-	}
+			sprintf(phy_name, "usb2-%d", i);
+
+		dwc->usb2_generic_phy[i] = devm_phy_get(dev, phy_name);
+		if (IS_ERR(dwc->usb2_generic_phy[i])) {
+			ret = PTR_ERR(dwc->usb2_generic_phy[i]);
+			if (ret == -ENOSYS || ret == -ENODEV)
+				dwc->usb2_generic_phy[i] = NULL;
+			else
+				return dev_err_probe(dev, ret, "failed to lookup phy %s\n",
+							phy_name);
+		}
 
-	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
-	if (IS_ERR(dwc->usb3_generic_phy)) {
-		ret = PTR_ERR(dwc->usb3_generic_phy);
-		if (ret == -ENOSYS || ret == -ENODEV)
-			dwc->usb3_generic_phy = NULL;
+		if (dwc->num_usb2_ports == 1)
+			sprintf(phy_name, "usb3-phy");
 		else
-			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
+			sprintf(phy_name, "usb3-%d", i);
+
+		dwc->usb3_generic_phy[i] = devm_phy_get(dev, phy_name);
+		if (IS_ERR(dwc->usb3_generic_phy[i])) {
+			ret = PTR_ERR(dwc->usb3_generic_phy[i]);
+			if (ret == -ENOSYS || ret == -ENODEV)
+				dwc->usb3_generic_phy[i] = NULL;
+			else
+				return dev_err_probe(dev, ret, "failed to lookup phy %s\n",
+							phy_name);
+		}
 	}
 
 	return 0;
@@ -1423,6 +1503,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 {
 	struct device *dev = dwc->dev;
 	int ret;
+	int i;
 
 	switch (dwc->dr_mode) {
 	case USB_DR_MODE_PERIPHERAL:
@@ -1430,8 +1511,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 
 		if (dwc->usb2_phy)
 			otg_set_vbus(dwc->usb2_phy->otg, false);
-		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
-		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
+		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
+		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
 
 		ret = dwc3_gadget_init(dwc);
 		if (ret)
@@ -1442,8 +1523,10 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 
 		if (dwc->usb2_phy)
 			otg_set_vbus(dwc->usb2_phy->otg, true);
-		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
-		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+		for (i = 0; i < dwc->num_usb2_ports; i++) {
+			phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+			phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
+		}
 
 		ret = dwc3_host_init(dwc);
 		if (ret)
@@ -1937,6 +2020,10 @@ static int dwc3_get_num_ports(struct dwc3 *dwc)
 
 	iounmap(base);
 
+	if (dwc->num_usb2_ports > DWC3_MAX_PORTS ||
+	    dwc->num_usb3_ports > DWC3_MAX_PORTS)
+		return -ENOMEM;
+
 	return 0;
 }
 
@@ -2174,6 +2261,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 {
 	unsigned long	flags;
 	u32 reg;
+	int i;
 
 	switch (dwc->current_dr_role) {
 	case DWC3_GCTL_PRTCAP_DEVICE:
@@ -2192,17 +2280,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 		/* Let controller to suspend HSPHY before PHY driver suspends */
 		if (dwc->dis_u2_susphy_quirk ||
 		    dwc->dis_enblslpm_quirk) {
-			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
-			reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
-				DWC3_GUSB2PHYCFG_SUSPHY;
-			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+			for (i = 0; i < dwc->num_usb2_ports; i++) {
+				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+				reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
+					DWC3_GUSB2PHYCFG_SUSPHY;
+				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+			}
 
 			/* Give some time for USB2 PHY to suspend */
 			usleep_range(5000, 6000);
 		}
 
-		phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
-		phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
+		for (i = 0; i < dwc->num_usb2_ports; i++) {
+			phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
+			phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
+		}
 		break;
 	case DWC3_GCTL_PRTCAP_OTG:
 		/* do nothing during runtime_suspend */
@@ -2232,6 +2324,7 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 	unsigned long	flags;
 	int		ret;
 	u32		reg;
+	int		i;
 
 	switch (dwc->current_dr_role) {
 	case DWC3_GCTL_PRTCAP_DEVICE:
@@ -2251,17 +2344,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 			break;
 		}
 		/* Restore GUSB2PHYCFG bits that were modified in suspend */
-		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
-		if (dwc->dis_u2_susphy_quirk)
-			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+		for (i = 0; i < dwc->num_usb2_ports; i++) {
+			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+			if (dwc->dis_u2_susphy_quirk)
+				reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
 
-		if (dwc->dis_enblslpm_quirk)
-			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
+			if (dwc->dis_enblslpm_quirk)
+				reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
 
-		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+		}
 
-		phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
-		phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
+		for (i = 0; i < dwc->num_usb2_ports; i++) {
+			phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
+			phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
+		}
 		break;
 	case DWC3_GCTL_PRTCAP_OTG:
 		/* nothing to do on runtime_resume */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 341e4c73cb2e..df2e111aa848 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -33,6 +33,12 @@
 
 #include <linux/power_supply.h>
 
+/*
+ * Maximum number of ports currently supported for multiport
+ * controllers.
+ */
+#define DWC3_MAX_PORTS 4
+
 #define DWC3_MSG_MAX	500
 
 /* Global constants */
@@ -1037,8 +1043,8 @@ struct dwc3_scratchpad_array {
  * @usb_psy: pointer to power supply interface.
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
- * @usb2_generic_phy: pointer to USB2 PHY
- * @usb3_generic_phy: pointer to USB3 PHY
+ * @usb2_generic_phy: pointer to array of USB2 PHYs
+ * @usb3_generic_phy: pointer to array of USB3 PHYs
  * @num_usb2_ports: number of USB2 ports
  * @num_usb3_ports: number of USB3 ports
  * @phys_ready: flag to indicate that PHYs are ready
@@ -1186,8 +1192,8 @@ struct dwc3 {
 	struct usb_phy		*usb2_phy;
 	struct usb_phy		*usb3_phy;
 
-	struct phy		*usb2_generic_phy;
-	struct phy		*usb3_generic_phy;
+	struct phy		*usb2_generic_phy[DWC3_MAX_PORTS];
+	struct phy		*usb3_generic_phy[DWC3_MAX_PORTS];
 
 	u8			num_usb2_ports;
 	u8			num_usb3_ports;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 57ddd2e43022..d76ae676783c 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -331,6 +331,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
 	u32 reg;
 	int id;
 	unsigned long flags;
+	int i;
 
 	if (dwc->dr_mode != USB_DR_MODE_OTG)
 		return;
@@ -386,9 +387,12 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
 		} else {
 			if (dwc->usb2_phy)
 				otg_set_vbus(dwc->usb2_phy->otg, true);
-			if (dwc->usb2_generic_phy)
-				phy_set_mode(dwc->usb2_generic_phy,
-					     PHY_MODE_USB_HOST);
+			for (i = 0; i < dwc->num_usb2_ports; i++) {
+				if (dwc->usb2_generic_phy[i]) {
+					phy_set_mode(dwc->usb2_generic_phy[i],
+						     PHY_MODE_USB_HOST);
+				}
+			}
 		}
 		break;
 	case DWC3_OTG_ROLE_DEVICE:
@@ -400,9 +404,8 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
 
 		if (dwc->usb2_phy)
 			otg_set_vbus(dwc->usb2_phy->otg, false);
-		if (dwc->usb2_generic_phy)
-			phy_set_mode(dwc->usb2_generic_phy,
-				     PHY_MODE_USB_DEVICE);
+		if (dwc->usb2_generic_phy[0])
+			phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
 		ret = dwc3_gadget_init(dwc);
 		if (ret)
 			dev_err(dwc->dev, "failed to initialize peripheral\n");
-- 
2.34.1


^ permalink raw reply related

* [PATCH v20 6/9] usb: dwc3: qcom: Add helper function to request wakeup interrupts
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
	Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, Johan Hovold
  Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
	quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>

The logic for requesting interrupts is duplicated for each interrupt. In
the upcoming patches that introduces support for multiport, it would be
better to clean up the duplication before reading mulitport related
interrupts.

Refactor interrupt setup call by adding a new helper function for
requesting the wakeup interrupts. To simplify implementation, make
the display name same as the interrupt name expected in DT.

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/usb/dwc3/dwc3-qcom.c | 53 ++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index f6b2fab49d5e..cae5dab8fcfc 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -501,6 +501,22 @@ static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom)
 			  PIPE_UTMI_CLK_DIS);
 }
 
+static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
+				 const char *name)
+{
+	int ret;
+
+	/* Keep wakeup interrupts disabled until suspend */
+	ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
+					qcom_dwc3_resume_irq,
+					IRQF_ONESHOT | IRQF_NO_AUTOEN,
+					name, qcom);
+	if (ret)
+		dev_err(qcom->dev, "failed to request irq %s: %d\n", name, ret);
+
+	return ret;
+}
+
 static int dwc3_qcom_setup_irq(struct platform_device *pdev)
 {
 	struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
@@ -509,54 +525,33 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
 
 	irq = platform_get_irq_byname_optional(pdev, "qusb2_phy");
 	if (irq > 0) {
-		/* Keep wakeup interrupts disabled until suspend */
-		ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
-					qcom_dwc3_resume_irq,
-					IRQF_ONESHOT | IRQF_NO_AUTOEN,
-					"qcom_dwc3 QUSB2", qcom);
-		if (ret) {
-			dev_err(qcom->dev, "qusb2_phy_irq failed: %d\n", ret);
+		ret = dwc3_qcom_request_irq(qcom, irq, "qusb2_phy");
+		if (ret)
 			return ret;
-		}
 		qcom->qusb2_phy_irq = irq;
 	}
 
 	irq = platform_get_irq_byname_optional(pdev, "dp_hs_phy_irq");
 	if (irq > 0) {
-		ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
-					qcom_dwc3_resume_irq,
-					IRQF_ONESHOT | IRQF_NO_AUTOEN,
-					"qcom_dwc3 DP_HS", qcom);
-		if (ret) {
-			dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret);
+		ret = dwc3_qcom_request_irq(qcom, irq, "dp_hs_phy_irq");
+		if (ret)
 			return ret;
-		}
 		qcom->dp_hs_phy_irq = irq;
 	}
 
 	irq = platform_get_irq_byname_optional(pdev, "dm_hs_phy_irq");
 	if (irq > 0) {
-		ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
-					qcom_dwc3_resume_irq,
-					IRQF_ONESHOT | IRQF_NO_AUTOEN,
-					"qcom_dwc3 DM_HS", qcom);
-		if (ret) {
-			dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret);
+		ret = dwc3_qcom_request_irq(qcom, irq, "dm_hs_phy_irq");
+		if (ret)
 			return ret;
-		}
 		qcom->dm_hs_phy_irq = irq;
 	}
 
 	irq = platform_get_irq_byname_optional(pdev, "ss_phy_irq");
 	if (irq > 0) {
-		ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
-					qcom_dwc3_resume_irq,
-					IRQF_ONESHOT | IRQF_NO_AUTOEN,
-					"qcom_dwc3 SS", qcom);
-		if (ret) {
-			dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret);
+		ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq");
+		if (ret)
 			return ret;
-		}
 		qcom->ss_phy_irq = irq;
 	}
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v20 3/9] usb: dwc3: core: Skip setting event buffers for host only controllers
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
	Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, Johan Hovold
  Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
	quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>

On some SoC's like SA8295P where the tertiary controller is host-only
capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
Trying to access them leads to a crash.

For DRD/Peripheral supported controllers, event buffer setup is done
again in gadget_pullup. Skip setup or cleanup of event buffers if
controller is host-only capable.

Suggested-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
---
 drivers/usb/dwc3/core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index ddab30531f8a..1a3d8a9beea8 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -486,6 +486,13 @@ static void dwc3_free_event_buffers(struct dwc3 *dwc)
 static int dwc3_alloc_event_buffers(struct dwc3 *dwc, unsigned int length)
 {
 	struct dwc3_event_buffer *evt;
+	unsigned int hw_mode;
+
+	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+	if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
+		dwc->ev_buf = NULL;
+		return 0;
+	}
 
 	evt = dwc3_alloc_one_event_buffer(dwc, length);
 	if (IS_ERR(evt)) {
@@ -507,6 +514,9 @@ int dwc3_event_buffers_setup(struct dwc3 *dwc)
 {
 	struct dwc3_event_buffer	*evt;
 
+	if (!dwc->ev_buf)
+		return 0;
+
 	evt = dwc->ev_buf;
 	evt->lpos = 0;
 	dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(0),
@@ -524,6 +534,9 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
 {
 	struct dwc3_event_buffer	*evt;
 
+	if (!dwc->ev_buf)
+		return;
+
 	evt = dwc->ev_buf;
 
 	evt->lpos = 0;
-- 
2.34.1


^ permalink raw reply related

* [PATCH v20 5/9] dt-bindings: usb: qcom,dwc3: Add bindings for SC8280 Multiport
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
	Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
	Felipe Balbi, Johan Hovold
  Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
	quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>

Add the compatible string for SC8280 Multiport USB controller from
Qualcomm.

There are 4 power event irq interrupts supported by this controller
(one for each port of multiport). Added all the 4 as non-optional
interrupts for SC8280XP-MP

Also each port of multiport has one DP and oen DM IRQ. Add all DP/DM
IRQ's related to 4 ports of SC8280XP Teritiary controller.

Also added ss phy irq for both SS Ports.

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../devicetree/bindings/usb/qcom,dwc3.yaml    | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 38a3404ec71b..f55f601c0329 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -30,6 +30,7 @@ properties:
           - qcom,sc7180-dwc3
           - qcom,sc7280-dwc3
           - qcom,sc8280xp-dwc3
+          - qcom,sc8280xp-dwc3-mp
           - qcom,sdm660-dwc3
           - qcom,sdm670-dwc3
           - qcom,sdm845-dwc3
@@ -282,6 +283,7 @@ allOf:
           contains:
             enum:
               - qcom,sc8280xp-dwc3
+              - qcom,sc8280xp-dwc3-mp
               - qcom,x1e80100-dwc3
     then:
       properties:
@@ -470,6 +472,38 @@ allOf:
             - const: dm_hs_phy_irq
             - const: ss_phy_irq
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc8280xp-dwc3-mp
+    then:
+      properties:
+        interrupts:
+          minItems: 18
+          maxItems: 18
+        interrupt-names:
+          items:
+            - const: pwr_event_1
+            - const: pwr_event_2
+            - const: pwr_event_3
+            - const: pwr_event_4
+            - const: hs_phy_1
+            - const: hs_phy_2
+            - const: hs_phy_3
+            - const: hs_phy_4
+            - const: dp_hs_phy_1
+            - const: dm_hs_phy_1
+            - const: dp_hs_phy_2
+            - const: dm_hs_phy_2
+            - const: dp_hs_phy_3
+            - const: dm_hs_phy_3
+            - const: dp_hs_phy_4
+            - const: dm_hs_phy_4
+            - const: ss_phy_1
+            - const: ss_phy_2
+
 additionalProperties: false
 
 examples:
-- 
2.34.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