linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC DNM PATCH] i2c: busses: qcom-cci: Add support for passing data in DT
@ 2022-03-19 20:05 Konrad Dybcio
  2022-03-21 15:16 ` Bjorn Andersson
  0 siblings, 1 reply; 3+ messages in thread
From: Konrad Dybcio @ 2022-03-19 20:05 UTC (permalink / raw)
  To: ~postmarketos/upstreaming
  Cc: martin.botka, angelogioacchino.delregno, marijn.suijten,
	jamipkettunen, Konrad Dybcio, Loic Poulain, Robert Foss,
	linux-i2c, linux-arm-msm, linux-kernel

Some vendors, such as SONY, fine-tune CCI parameters to better suit their
specific combinations of devices and camera sensors, mostly in order to save a
tiny bit more power. Add support for specifying all the CCI parameters in DT to
avoid adding millions of structs in the driver itself.

---
This is a ridiculously blatant RFC and PoC just to start the discussion.
(thus it's missing the S-o-b on purpose)

My point is that our favourite (cough cough) phone vendor likes to
fine-tune every bit of the hardware and they are probably not alone
doing this. Moving the properties into the DT would allow for more
flexibility with the configuration, instead of having to add a separate
set of structs for each one.

If it were to make it in any form, it would probably be much saner
to represent each mode as an individual subnode in the dt, specifying
its parameters over there (which would incidentally also allow for
adding more/less modes if need be), something like this:

cci@0badbeef {
	/* compatible and stuff */

	mode-standard {
		parameter-1 = <1>;
	};

	mode-fast {
		parameter-1 = <1>;
	};

	mode-supercustomlightspeed {
		parameter-1 = <1>;
		parameter-2 = <1337>;
	};
};

What are your thoughts about this, and do you think the form shown above
(and probably not the one in the patch) would be fitting, or is there a
better approach to this?

 drivers/i2c/busses/i2c-qcom-cci.c | 274 ++++++++++++++++++++++++++++++
 1 file changed, 274 insertions(+)

diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
index 07c11e2a446d..6754b5d11c52 100644
--- a/drivers/i2c/busses/i2c-qcom-cci.c
+++ b/drivers/i2c/busses/i2c-qcom-cci.c
@@ -117,6 +117,7 @@ struct cci_master {
 };
 
 struct cci_data {
+	bool read_params_from_dt;
 	unsigned int num_masters;
 	struct i2c_adapter_quirks quirks;
 	u16 queue_size[NUM_QUEUES];
@@ -520,11 +521,20 @@ static const struct dev_pm_ops qcom_cci_pm = {
 	SET_RUNTIME_PM_OPS(cci_suspend_runtime, cci_resume_runtime, NULL)
 };
 
+static struct cci_data cci_dt_data = {
+	.read_params_from_dt = true,
+	.quirks = {},
+	.params[I2C_MODE_STANDARD] = {},
+	.params[I2C_MODE_FAST] = {},
+	.params[I2C_MODE_FAST_PLUS] = {},
+};
+
 static int cci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	unsigned long cci_clk_rate = 0;
 	struct device_node *child;
+	struct cci_data *dt_data;
 	struct resource *r;
 	struct cci *cci;
 	int ret, i;
@@ -540,6 +550,267 @@ static int cci_probe(struct platform_device *pdev)
 	if (!cci->data)
 		return -ENOENT;
 
+	if (cci->data->read_params_from_dt) {
+		dt_data = &cci_dt_data;
+
+		/* CCI params */
+		ret = of_property_read_u32(dev->of_node, "num-masters", &val);
+		if (ret) {
+			dev_err(dev, "Error reading num-masters from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->num_masters = val;
+
+		ret = of_property_read_u16_array(dev->of_node, "queue-size", dt_data->queue_size,
+			(size_t)&dt_data->num_masters);
+		if (ret) {
+			dev_err(dev, "Error reading queue-size from DT, ret = %d", ret);
+			return ret;
+		}
+
+		if (ARRAY_SIZE(dt_data->queue_size) != dt_data->num_masters) {
+			dev_err(dev, "num-masters doesn't match the number of queue-size elements!");
+			return -EINVAL;
+		}
+
+		ret = of_property_read_u32(dev->of_node, "max-write-len", &val);
+		if (ret) {
+			dev_err(dev, "Error reading max-write-len from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->quirks.max_write_len = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "max-read-len", &val);
+		if (ret) {
+			dev_err(dev, "Error reading max-read-len from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->quirks.max_read_len = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "cci-clk-rate", &val);
+		if (ret) {
+			dev_err(dev, "Error reading cci-clk-rate from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->cci_clk_rate = (unsigned long)val;
+
+		/* STANDARD mode params */
+		ret = of_property_read_u32(dev->of_node, "thigh-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thigh-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].thigh = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tlow-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tlow-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].tlow = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsu-sto-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsu-sto-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].tsu_sto = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsu-sta-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsu-sta-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].tsu_sta = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "thd-dat-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thd-dat-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].thd_dat = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "thd-sta-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thd-sta-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].thd_sta = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tbuf-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tbuf-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].tbuf = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading scl-stretch-en-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].scl_stretch_en = (u8)val;
+
+		ret = of_property_read_u32(dev->of_node, "trdhld-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading trdhld-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].trdhld = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsp-standard", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsp-standard from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_STANDARD].tsp = (u16)val;
+
+		/* FAST mode params */
+		ret = of_property_read_u32(dev->of_node, "thigh-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thigh-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].thigh = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tlow-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tlow-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].tlow = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsu-sto-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsu-sto-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].tsu_sto = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsu-sta-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsu-sta-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].tsu_sta = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "thd-dat-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thd-dat-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].thd_dat = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "thd-sta-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thd-sta-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].thd_sta = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tbuf-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tbuf-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].tbuf = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading scl-stretch-en-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].scl_stretch_en = (u8)val;
+
+		ret = of_property_read_u32(dev->of_node, "trdhld-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading trdhld-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].trdhld = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsp-fast", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsp-fast from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST].tsp = (u16)val;
+
+		/* FAST_PLUS mode params */
+		ret = of_property_read_u32(dev->of_node, "thigh-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thigh-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].thigh = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tlow-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tlow-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].tlow = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsu-sto-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsu-sto-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].tsu_sto = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsu-sta-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsu-sta-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].tsu_sta = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "thd-dat-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thd-dat-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].thd_dat = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "thd-sta-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading thd-sta-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].thd_sta = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tbuf-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tbuf-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].tbuf = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading scl-stretch-en-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].scl_stretch_en = (u8)val;
+
+		ret = of_property_read_u32(dev->of_node, "trdhld-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading trdhld-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].trdhld = (u16)val;
+
+		ret = of_property_read_u32(dev->of_node, "tsp-fast-plus", &val);
+		if (ret) {
+			dev_err(dev, "Error reading tsp-fast-plus from DT, ret = %d", ret);
+			return ret;
+		}
+		dt_data->params[I2C_MODE_FAST_PLUS].tsp = (u16)val;
+
+		/* Let's ship it! */
+		cci->data = dt_data;
+	}
+
 	for_each_available_child_of_node(dev->of_node, child) {
 		u32 idx;
 
@@ -818,6 +1089,9 @@ static const struct cci_data cci_msm8994_data = {
 };
 
 static const struct of_device_id cci_dt_match[] = {
+	{ .compatible = "qcom,cci", .data = &cci_dt_data },
+
+	/* Legacy compatibles for older DTs */
 	{ .compatible = "qcom,msm8916-cci", .data = &cci_v1_data},
 	{ .compatible = "qcom,msm8994-cci", .data = &cci_msm8994_data},
 	{ .compatible = "qcom,msm8996-cci", .data = &cci_v2_data},
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC DNM PATCH] i2c: busses: qcom-cci: Add support for passing data in DT
  2022-03-19 20:05 [RFC DNM PATCH] i2c: busses: qcom-cci: Add support for passing data in DT Konrad Dybcio
@ 2022-03-21 15:16 ` Bjorn Andersson
  2022-03-21 22:20   ` Konrad Dybcio
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Andersson @ 2022-03-21 15:16 UTC (permalink / raw)
  To: Konrad Dybcio, devicetree
  Cc: ~postmarketos/upstreaming, martin.botka,
	angelogioacchino.delregno, marijn.suijten, jamipkettunen,
	Loic Poulain, Robert Foss, linux-i2c, linux-arm-msm,
	linux-kernel

On Sat 19 Mar 15:05 CDT 2022, Konrad Dybcio wrote:

+devicestree@vger.kernel.org

> Some vendors, such as SONY, fine-tune CCI parameters to better suit their
> specific combinations of devices and camera sensors, mostly in order to save a
> tiny bit more power. Add support for specifying all the CCI parameters in DT to
> avoid adding millions of structs in the driver itself.
> 
> ---
> This is a ridiculously blatant RFC and PoC just to start the discussion.
> (thus it's missing the S-o-b on purpose)
> 
> My point is that our favourite (cough cough) phone vendor likes to
> fine-tune every bit of the hardware and they are probably not alone
> doing this. Moving the properties into the DT would allow for more
> flexibility with the configuration, instead of having to add a separate
> set of structs for each one.
> 
> If it were to make it in any form, it would probably be much saner
> to represent each mode as an individual subnode in the dt, specifying
> its parameters over there (which would incidentally also allow for
> adding more/less modes if need be), something like this:
> 
> cci@0badbeef {
> 	/* compatible and stuff */
> 
> 	mode-standard {
> 		parameter-1 = <1>;
> 	};
> 
> 	mode-fast {
> 		parameter-1 = <1>;
> 	};
> 
> 	mode-supercustomlightspeed {
> 		parameter-1 = <1>;
> 		parameter-2 = <1337>;
> 	};

We put clock-frequency in the i2c bus node today, doesn't this serve the
same purpose, but with more details? Why would one want to specify the
timing details for each mode and then select one? Perhaps I'm
missing some detail here?


Are these timing details specified per bus in the CCI, or are the
settings shared between the two buses of the controller?

Regards,
Bjorn

> };
> 
> What are your thoughts about this, and do you think the form shown above
> (and probably not the one in the patch) would be fitting, or is there a
> better approach to this?
> 
>  drivers/i2c/busses/i2c-qcom-cci.c | 274 ++++++++++++++++++++++++++++++
>  1 file changed, 274 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index 07c11e2a446d..6754b5d11c52 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -117,6 +117,7 @@ struct cci_master {
>  };
>  
>  struct cci_data {
> +	bool read_params_from_dt;
>  	unsigned int num_masters;
>  	struct i2c_adapter_quirks quirks;
>  	u16 queue_size[NUM_QUEUES];
> @@ -520,11 +521,20 @@ static const struct dev_pm_ops qcom_cci_pm = {
>  	SET_RUNTIME_PM_OPS(cci_suspend_runtime, cci_resume_runtime, NULL)
>  };
>  
> +static struct cci_data cci_dt_data = {
> +	.read_params_from_dt = true,
> +	.quirks = {},
> +	.params[I2C_MODE_STANDARD] = {},
> +	.params[I2C_MODE_FAST] = {},
> +	.params[I2C_MODE_FAST_PLUS] = {},
> +};
> +
>  static int cci_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	unsigned long cci_clk_rate = 0;
>  	struct device_node *child;
> +	struct cci_data *dt_data;
>  	struct resource *r;
>  	struct cci *cci;
>  	int ret, i;
> @@ -540,6 +550,267 @@ static int cci_probe(struct platform_device *pdev)
>  	if (!cci->data)
>  		return -ENOENT;
>  
> +	if (cci->data->read_params_from_dt) {
> +		dt_data = &cci_dt_data;
> +
> +		/* CCI params */
> +		ret = of_property_read_u32(dev->of_node, "num-masters", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading num-masters from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->num_masters = val;
> +
> +		ret = of_property_read_u16_array(dev->of_node, "queue-size", dt_data->queue_size,
> +			(size_t)&dt_data->num_masters);
> +		if (ret) {
> +			dev_err(dev, "Error reading queue-size from DT, ret = %d", ret);
> +			return ret;
> +		}
> +
> +		if (ARRAY_SIZE(dt_data->queue_size) != dt_data->num_masters) {
> +			dev_err(dev, "num-masters doesn't match the number of queue-size elements!");
> +			return -EINVAL;
> +		}
> +
> +		ret = of_property_read_u32(dev->of_node, "max-write-len", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading max-write-len from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->quirks.max_write_len = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "max-read-len", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading max-read-len from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->quirks.max_read_len = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "cci-clk-rate", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading cci-clk-rate from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->cci_clk_rate = (unsigned long)val;
> +
> +		/* STANDARD mode params */
> +		ret = of_property_read_u32(dev->of_node, "thigh-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thigh-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].thigh = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tlow-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tlow-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].tlow = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsu-sto-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsu-sto-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].tsu_sto = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsu-sta-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsu-sta-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].tsu_sta = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "thd-dat-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thd-dat-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].thd_dat = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "thd-sta-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thd-sta-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].thd_sta = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tbuf-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tbuf-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].tbuf = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading scl-stretch-en-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].scl_stretch_en = (u8)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "trdhld-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading trdhld-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].trdhld = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsp-standard", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsp-standard from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_STANDARD].tsp = (u16)val;
> +
> +		/* FAST mode params */
> +		ret = of_property_read_u32(dev->of_node, "thigh-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thigh-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].thigh = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tlow-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tlow-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].tlow = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsu-sto-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsu-sto-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].tsu_sto = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsu-sta-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsu-sta-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].tsu_sta = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "thd-dat-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thd-dat-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].thd_dat = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "thd-sta-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thd-sta-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].thd_sta = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tbuf-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tbuf-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].tbuf = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading scl-stretch-en-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].scl_stretch_en = (u8)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "trdhld-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading trdhld-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].trdhld = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsp-fast", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsp-fast from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST].tsp = (u16)val;
> +
> +		/* FAST_PLUS mode params */
> +		ret = of_property_read_u32(dev->of_node, "thigh-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thigh-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].thigh = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tlow-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tlow-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].tlow = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsu-sto-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsu-sto-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].tsu_sto = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsu-sta-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsu-sta-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].tsu_sta = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "thd-dat-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thd-dat-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].thd_dat = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "thd-sta-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading thd-sta-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].thd_sta = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tbuf-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tbuf-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].tbuf = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading scl-stretch-en-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].scl_stretch_en = (u8)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "trdhld-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading trdhld-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].trdhld = (u16)val;
> +
> +		ret = of_property_read_u32(dev->of_node, "tsp-fast-plus", &val);
> +		if (ret) {
> +			dev_err(dev, "Error reading tsp-fast-plus from DT, ret = %d", ret);
> +			return ret;
> +		}
> +		dt_data->params[I2C_MODE_FAST_PLUS].tsp = (u16)val;
> +
> +		/* Let's ship it! */
> +		cci->data = dt_data;
> +	}
> +
>  	for_each_available_child_of_node(dev->of_node, child) {
>  		u32 idx;
>  
> @@ -818,6 +1089,9 @@ static const struct cci_data cci_msm8994_data = {
>  };
>  
>  static const struct of_device_id cci_dt_match[] = {
> +	{ .compatible = "qcom,cci", .data = &cci_dt_data },
> +
> +	/* Legacy compatibles for older DTs */
>  	{ .compatible = "qcom,msm8916-cci", .data = &cci_v1_data},
>  	{ .compatible = "qcom,msm8994-cci", .data = &cci_msm8994_data},
>  	{ .compatible = "qcom,msm8996-cci", .data = &cci_v2_data},
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC DNM PATCH] i2c: busses: qcom-cci: Add support for passing data in DT
  2022-03-21 15:16 ` Bjorn Andersson
@ 2022-03-21 22:20   ` Konrad Dybcio
  0 siblings, 0 replies; 3+ messages in thread
From: Konrad Dybcio @ 2022-03-21 22:20 UTC (permalink / raw)
  To: Bjorn Andersson, devicetree
  Cc: ~postmarketos/upstreaming, martin.botka,
	angelogioacchino.delregno, marijn.suijten, jamipkettunen,
	Loic Poulain, Robert Foss, linux-i2c, linux-arm-msm,
	linux-kernel


On 21/03/2022 16:16, Bjorn Andersson wrote:
> On Sat 19 Mar 15:05 CDT 2022, Konrad Dybcio wrote:
>
> +devicestree@vger.kernel.org
>
>> Some vendors, such as SONY, fine-tune CCI parameters to better suit their
>> specific combinations of devices and camera sensors, mostly in order to save a
>> tiny bit more power. Add support for specifying all the CCI parameters in DT to
>> avoid adding millions of structs in the driver itself.
>>
>> ---
>> This is a ridiculously blatant RFC and PoC just to start the discussion.
>> (thus it's missing the S-o-b on purpose)
>>
>> My point is that our favourite (cough cough) phone vendor likes to
>> fine-tune every bit of the hardware and they are probably not alone
>> doing this. Moving the properties into the DT would allow for more
>> flexibility with the configuration, instead of having to add a separate
>> set of structs for each one.
>>
>> If it were to make it in any form, it would probably be much saner
>> to represent each mode as an individual subnode in the dt, specifying
>> its parameters over there (which would incidentally also allow for
>> adding more/less modes if need be), something like this:
>>
>> cci@0badbeef {
>> 	/* compatible and stuff */
>>
>> 	mode-standard {
>> 		parameter-1 = <1>;
>> 	};
>>
>> 	mode-fast {
>> 		parameter-1 = <1>;
>> 	};
>>
>> 	mode-supercustomlightspeed {
>> 		parameter-1 = <1>;
>> 		parameter-2 = <1337>;
>> 	};
> We put clock-frequency in the i2c bus node today, doesn't this serve the
> same purpose, but with more details? Why would one want to specify the
> timing details for each mode and then select one?

As far as I understand, imaging sensors can choose different modes under 
different bandwidth requirements, in order to save power where possible 
and consume the full bandwidth where necessary. The parameters can be 
changed to better suit the individual board designs.


> Perhaps I'm
> missing some detail here?
>
>
> Are these timing details specified per bus in the CCI, or are the
> settings shared between the two buses of the controller?

The details are shared between both buses, as is the case in the current 
state of the driver and downstream.


By the way, I got word that at least Lumias also have some tweaks done 
to these values.


Konrad


>
> Regards,
> Bjorn
>
>> };
>>
>> What are your thoughts about this, and do you think the form shown above
>> (and probably not the one in the patch) would be fitting, or is there a
>> better approach to this?
>>
>>   drivers/i2c/busses/i2c-qcom-cci.c | 274 ++++++++++++++++++++++++++++++
>>   1 file changed, 274 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
>> index 07c11e2a446d..6754b5d11c52 100644
>> --- a/drivers/i2c/busses/i2c-qcom-cci.c
>> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
>> @@ -117,6 +117,7 @@ struct cci_master {
>>   };
>>   
>>   struct cci_data {
>> +	bool read_params_from_dt;
>>   	unsigned int num_masters;
>>   	struct i2c_adapter_quirks quirks;
>>   	u16 queue_size[NUM_QUEUES];
>> @@ -520,11 +521,20 @@ static const struct dev_pm_ops qcom_cci_pm = {
>>   	SET_RUNTIME_PM_OPS(cci_suspend_runtime, cci_resume_runtime, NULL)
>>   };
>>   
>> +static struct cci_data cci_dt_data = {
>> +	.read_params_from_dt = true,
>> +	.quirks = {},
>> +	.params[I2C_MODE_STANDARD] = {},
>> +	.params[I2C_MODE_FAST] = {},
>> +	.params[I2C_MODE_FAST_PLUS] = {},
>> +};
>> +
>>   static int cci_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	unsigned long cci_clk_rate = 0;
>>   	struct device_node *child;
>> +	struct cci_data *dt_data;
>>   	struct resource *r;
>>   	struct cci *cci;
>>   	int ret, i;
>> @@ -540,6 +550,267 @@ static int cci_probe(struct platform_device *pdev)
>>   	if (!cci->data)
>>   		return -ENOENT;
>>   
>> +	if (cci->data->read_params_from_dt) {
>> +		dt_data = &cci_dt_data;
>> +
>> +		/* CCI params */
>> +		ret = of_property_read_u32(dev->of_node, "num-masters", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading num-masters from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->num_masters = val;
>> +
>> +		ret = of_property_read_u16_array(dev->of_node, "queue-size", dt_data->queue_size,
>> +			(size_t)&dt_data->num_masters);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading queue-size from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +
>> +		if (ARRAY_SIZE(dt_data->queue_size) != dt_data->num_masters) {
>> +			dev_err(dev, "num-masters doesn't match the number of queue-size elements!");
>> +			return -EINVAL;
>> +		}
>> +
>> +		ret = of_property_read_u32(dev->of_node, "max-write-len", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading max-write-len from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->quirks.max_write_len = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "max-read-len", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading max-read-len from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->quirks.max_read_len = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "cci-clk-rate", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading cci-clk-rate from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->cci_clk_rate = (unsigned long)val;
>> +
>> +		/* STANDARD mode params */
>> +		ret = of_property_read_u32(dev->of_node, "thigh-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thigh-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].thigh = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tlow-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tlow-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].tlow = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsu-sto-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsu-sto-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].tsu_sto = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsu-sta-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsu-sta-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].tsu_sta = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "thd-dat-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thd-dat-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].thd_dat = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "thd-sta-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thd-sta-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].thd_sta = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tbuf-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tbuf-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].tbuf = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading scl-stretch-en-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].scl_stretch_en = (u8)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "trdhld-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading trdhld-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].trdhld = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsp-standard", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsp-standard from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_STANDARD].tsp = (u16)val;
>> +
>> +		/* FAST mode params */
>> +		ret = of_property_read_u32(dev->of_node, "thigh-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thigh-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].thigh = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tlow-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tlow-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].tlow = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsu-sto-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsu-sto-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].tsu_sto = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsu-sta-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsu-sta-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].tsu_sta = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "thd-dat-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thd-dat-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].thd_dat = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "thd-sta-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thd-sta-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].thd_sta = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tbuf-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tbuf-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].tbuf = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading scl-stretch-en-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].scl_stretch_en = (u8)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "trdhld-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading trdhld-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].trdhld = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsp-fast", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsp-fast from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST].tsp = (u16)val;
>> +
>> +		/* FAST_PLUS mode params */
>> +		ret = of_property_read_u32(dev->of_node, "thigh-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thigh-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].thigh = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tlow-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tlow-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].tlow = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsu-sto-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsu-sto-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].tsu_sto = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsu-sta-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsu-sta-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].tsu_sta = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "thd-dat-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thd-dat-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].thd_dat = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "thd-sta-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading thd-sta-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].thd_sta = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tbuf-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tbuf-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].tbuf = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "scl-stretch-en-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading scl-stretch-en-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].scl_stretch_en = (u8)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "trdhld-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading trdhld-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].trdhld = (u16)val;
>> +
>> +		ret = of_property_read_u32(dev->of_node, "tsp-fast-plus", &val);
>> +		if (ret) {
>> +			dev_err(dev, "Error reading tsp-fast-plus from DT, ret = %d", ret);
>> +			return ret;
>> +		}
>> +		dt_data->params[I2C_MODE_FAST_PLUS].tsp = (u16)val;
>> +
>> +		/* Let's ship it! */
>> +		cci->data = dt_data;
>> +	}
>> +
>>   	for_each_available_child_of_node(dev->of_node, child) {
>>   		u32 idx;
>>   
>> @@ -818,6 +1089,9 @@ static const struct cci_data cci_msm8994_data = {
>>   };
>>   
>>   static const struct of_device_id cci_dt_match[] = {
>> +	{ .compatible = "qcom,cci", .data = &cci_dt_data },
>> +
>> +	/* Legacy compatibles for older DTs */
>>   	{ .compatible = "qcom,msm8916-cci", .data = &cci_v1_data},
>>   	{ .compatible = "qcom,msm8994-cci", .data = &cci_msm8994_data},
>>   	{ .compatible = "qcom,msm8996-cci", .data = &cci_v2_data},
>> -- 
>> 2.35.1
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-03-21 22:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-19 20:05 [RFC DNM PATCH] i2c: busses: qcom-cci: Add support for passing data in DT Konrad Dybcio
2022-03-21 15:16 ` Bjorn Andersson
2022-03-21 22:20   ` Konrad Dybcio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).