[PATCH v1 08/12] i2c: qcom-geni: Isolate serial engine setup

Praveen Talari posted 12 patches 1 week, 2 days ago
[PATCH v1 08/12] i2c: qcom-geni: Isolate serial engine setup
Posted by Praveen Talari 1 week, 2 days ago
Move serial engine configuration from probe to geni_i2c_init().

Relocating the serial engine setup to a dedicated initialization function
enhances code clarity and simplifies future modifications.

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/i2c/busses/i2c-qcom-geni.c | 148 ++++++++++++++---------------
 1 file changed, 73 insertions(+), 75 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 3a04016db2c3..4111afe2713e 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -976,10 +976,75 @@ static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
 	return ret;
 }
 
+static int geni_i2c_init(struct geni_i2c_dev *gi2c)
+{
+	const struct geni_i2c_desc *desc = NULL;
+	u32 proto, tx_depth;
+	bool fifo_disable;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(gi2c->se.dev);
+	if (ret < 0) {
+		dev_err(gi2c->se.dev, "error turning on device :%d\n", ret);
+		return ret;
+	}
+
+	proto = geni_se_read_proto(&gi2c->se);
+	if (proto == GENI_SE_INVALID_PROTO) {
+		ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
+		if (ret) {
+			dev_err_probe(gi2c->se.dev, ret, "i2c firmware load failed ret: %d\n", ret);
+			goto err;
+		}
+	} else if (proto != GENI_SE_I2C) {
+		ret = dev_err_probe(gi2c->se.dev, -ENXIO, "Invalid proto %d\n", proto);
+		goto err;
+	}
+
+	desc = device_get_match_data(gi2c->se.dev);
+	if (desc && desc->no_dma_support)
+		fifo_disable = false;
+	else
+		fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
+
+	if (fifo_disable) {
+		/* FIFO is disabled, so we can only use GPI DMA */
+		gi2c->gpi_mode = true;
+		ret = setup_gpi_dma(gi2c);
+		if (ret)
+			goto err;
+
+		dev_dbg(gi2c->se.dev, "Using GPI DMA mode for I2C\n");
+	} else {
+		gi2c->gpi_mode = false;
+		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
+
+		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
+		if (!tx_depth && desc)
+			tx_depth = desc->tx_fifo_depth;
+
+		if (!tx_depth) {
+			ret = dev_err_probe(gi2c->se.dev, -EINVAL,
+					    "Invalid TX FIFO depth\n");
+			goto err;
+		}
+
+		gi2c->tx_wm = tx_depth - 1;
+		geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
+		geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
+				       PACKING_BYTES_PW, true, true, true);
+
+		dev_dbg(gi2c->se.dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
+	}
+
+err:
+	pm_runtime_put(gi2c->se.dev);
+	return ret;
+}
+
 static int geni_i2c_probe(struct platform_device *pdev)
 {
 	struct geni_i2c_dev *gi2c;
-	u32 proto, tx_depth, fifo_disable;
 	int ret;
 	struct device *dev = &pdev->dev;
 	const struct geni_i2c_desc *desc = NULL;
@@ -1059,79 +1124,19 @@ static int geni_i2c_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(gi2c->core_clk);
-	if (ret)
-		return ret;
-
-	ret = geni_se_resources_on(&gi2c->se);
-	if (ret) {
-		dev_err_probe(dev, ret, "Error turning on resources\n");
-		goto err_clk;
-	}
-	proto = geni_se_read_proto(&gi2c->se);
-	if (proto == GENI_SE_INVALID_PROTO) {
-		ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
-		if (ret) {
-			dev_err_probe(dev, ret, "i2c firmware load failed ret: %d\n", ret);
-			goto err_resources;
-		}
-	} else if (proto != GENI_SE_I2C) {
-		ret = dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
-		goto err_resources;
-	}
-
-	if (desc && desc->no_dma_support)
-		fifo_disable = false;
-	else
-		fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
-
-	if (fifo_disable) {
-		/* FIFO is disabled, so we can only use GPI DMA */
-		gi2c->gpi_mode = true;
-		ret = setup_gpi_dma(gi2c);
-		if (ret)
-			goto err_resources;
-
-		dev_dbg(dev, "Using GPI DMA mode for I2C\n");
-	} else {
-		gi2c->gpi_mode = false;
-		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
-
-		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
-		if (!tx_depth && desc)
-			tx_depth = desc->tx_fifo_depth;
-
-		if (!tx_depth) {
-			ret = dev_err_probe(dev, -EINVAL,
-					    "Invalid TX FIFO depth\n");
-			goto err_resources;
-		}
-
-		gi2c->tx_wm = tx_depth - 1;
-		geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
-		geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
-				       PACKING_BYTES_PW, true, true, true);
-
-		dev_dbg(dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
-	}
-
-	clk_disable_unprepare(gi2c->core_clk);
-	ret = geni_se_resources_off(&gi2c->se);
-	if (ret) {
-		dev_err_probe(dev, ret, "Error turning off resources\n");
-		goto err_dma;
-	}
-
-	ret = geni_icc_disable(&gi2c->se);
-	if (ret)
-		goto err_dma;
-
 	gi2c->suspended = 1;
 	pm_runtime_set_suspended(gi2c->se.dev);
 	pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
 	pm_runtime_use_autosuspend(gi2c->se.dev);
 	pm_runtime_enable(gi2c->se.dev);
 
+	ret =  geni_i2c_init(gi2c);
+	if (ret < 0) {
+		dev_err(gi2c->se.dev, "I2C init failed :%d\n", ret);
+		pm_runtime_disable(gi2c->se.dev);
+		goto err_dma;
+	}
+
 	ret = i2c_add_adapter(&gi2c->adap);
 	if (ret) {
 		dev_err_probe(dev, ret, "Error adding i2c adapter\n");
@@ -1143,13 +1148,6 @@ static int geni_i2c_probe(struct platform_device *pdev)
 
 	return ret;
 
-err_resources:
-	geni_se_resources_off(&gi2c->se);
-err_clk:
-	clk_disable_unprepare(gi2c->core_clk);
-
-	return ret;
-
 err_dma:
 	release_gpi_dma(gi2c);
 
-- 
2.34.1
Re: [PATCH v1 08/12] i2c: qcom-geni: Isolate serial engine setup
Posted by Bjorn Andersson 5 days, 9 hours ago
On Sat, Nov 22, 2025 at 10:30:14AM +0530, Praveen Talari wrote:
> Move serial engine configuration from probe to geni_i2c_init().
> 
> Relocating the serial engine setup to a dedicated initialization function
> enhances code clarity and simplifies future modifications.

Please enhance commit message clarity. I don't think "code clarity" is
your most significant reason for this change, and "simplifies future
modification" is completely vague.

Be specific, the reader of this commit message hasn't implemented the
next set of commits, so they don't understand why this helps.

If the reason is that this simplifies the error handling around the
resource acquisition in the next patches, write that.

If my guess is wrong and the sole reason for you change is that you
don't like 179 lines long functions, then just say that.

Regards,
Bjorn

> 
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
>  drivers/i2c/busses/i2c-qcom-geni.c | 148 ++++++++++++++---------------
>  1 file changed, 73 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 3a04016db2c3..4111afe2713e 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -976,10 +976,75 @@ static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
>  	return ret;
>  }
>  
> +static int geni_i2c_init(struct geni_i2c_dev *gi2c)
> +{
> +	const struct geni_i2c_desc *desc = NULL;
> +	u32 proto, tx_depth;
> +	bool fifo_disable;
> +	int ret;
> +
> +	ret = pm_runtime_resume_and_get(gi2c->se.dev);
> +	if (ret < 0) {
> +		dev_err(gi2c->se.dev, "error turning on device :%d\n", ret);
> +		return ret;
> +	}
> +
> +	proto = geni_se_read_proto(&gi2c->se);
> +	if (proto == GENI_SE_INVALID_PROTO) {
> +		ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
> +		if (ret) {
> +			dev_err_probe(gi2c->se.dev, ret, "i2c firmware load failed ret: %d\n", ret);
> +			goto err;
> +		}
> +	} else if (proto != GENI_SE_I2C) {
> +		ret = dev_err_probe(gi2c->se.dev, -ENXIO, "Invalid proto %d\n", proto);
> +		goto err;
> +	}
> +
> +	desc = device_get_match_data(gi2c->se.dev);
> +	if (desc && desc->no_dma_support)
> +		fifo_disable = false;
> +	else
> +		fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
> +
> +	if (fifo_disable) {
> +		/* FIFO is disabled, so we can only use GPI DMA */
> +		gi2c->gpi_mode = true;
> +		ret = setup_gpi_dma(gi2c);
> +		if (ret)
> +			goto err;
> +
> +		dev_dbg(gi2c->se.dev, "Using GPI DMA mode for I2C\n");
> +	} else {
> +		gi2c->gpi_mode = false;
> +		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
> +
> +		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
> +		if (!tx_depth && desc)
> +			tx_depth = desc->tx_fifo_depth;
> +
> +		if (!tx_depth) {
> +			ret = dev_err_probe(gi2c->se.dev, -EINVAL,
> +					    "Invalid TX FIFO depth\n");
> +			goto err;
> +		}
> +
> +		gi2c->tx_wm = tx_depth - 1;
> +		geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
> +		geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
> +				       PACKING_BYTES_PW, true, true, true);
> +
> +		dev_dbg(gi2c->se.dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
> +	}
> +
> +err:
> +	pm_runtime_put(gi2c->se.dev);
> +	return ret;
> +}
> +
>  static int geni_i2c_probe(struct platform_device *pdev)
>  {
>  	struct geni_i2c_dev *gi2c;
> -	u32 proto, tx_depth, fifo_disable;
>  	int ret;
>  	struct device *dev = &pdev->dev;
>  	const struct geni_i2c_desc *desc = NULL;
> @@ -1059,79 +1124,19 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	ret = clk_prepare_enable(gi2c->core_clk);
> -	if (ret)
> -		return ret;
> -
> -	ret = geni_se_resources_on(&gi2c->se);
> -	if (ret) {
> -		dev_err_probe(dev, ret, "Error turning on resources\n");
> -		goto err_clk;
> -	}
> -	proto = geni_se_read_proto(&gi2c->se);
> -	if (proto == GENI_SE_INVALID_PROTO) {
> -		ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
> -		if (ret) {
> -			dev_err_probe(dev, ret, "i2c firmware load failed ret: %d\n", ret);
> -			goto err_resources;
> -		}
> -	} else if (proto != GENI_SE_I2C) {
> -		ret = dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
> -		goto err_resources;
> -	}
> -
> -	if (desc && desc->no_dma_support)
> -		fifo_disable = false;
> -	else
> -		fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
> -
> -	if (fifo_disable) {
> -		/* FIFO is disabled, so we can only use GPI DMA */
> -		gi2c->gpi_mode = true;
> -		ret = setup_gpi_dma(gi2c);
> -		if (ret)
> -			goto err_resources;
> -
> -		dev_dbg(dev, "Using GPI DMA mode for I2C\n");
> -	} else {
> -		gi2c->gpi_mode = false;
> -		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
> -
> -		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
> -		if (!tx_depth && desc)
> -			tx_depth = desc->tx_fifo_depth;
> -
> -		if (!tx_depth) {
> -			ret = dev_err_probe(dev, -EINVAL,
> -					    "Invalid TX FIFO depth\n");
> -			goto err_resources;
> -		}
> -
> -		gi2c->tx_wm = tx_depth - 1;
> -		geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
> -		geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
> -				       PACKING_BYTES_PW, true, true, true);
> -
> -		dev_dbg(dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
> -	}
> -
> -	clk_disable_unprepare(gi2c->core_clk);
> -	ret = geni_se_resources_off(&gi2c->se);
> -	if (ret) {
> -		dev_err_probe(dev, ret, "Error turning off resources\n");
> -		goto err_dma;
> -	}
> -
> -	ret = geni_icc_disable(&gi2c->se);
> -	if (ret)
> -		goto err_dma;
> -
>  	gi2c->suspended = 1;
>  	pm_runtime_set_suspended(gi2c->se.dev);
>  	pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
>  	pm_runtime_use_autosuspend(gi2c->se.dev);
>  	pm_runtime_enable(gi2c->se.dev);
>  
> +	ret =  geni_i2c_init(gi2c);
> +	if (ret < 0) {
> +		dev_err(gi2c->se.dev, "I2C init failed :%d\n", ret);
> +		pm_runtime_disable(gi2c->se.dev);
> +		goto err_dma;
> +	}
> +
>  	ret = i2c_add_adapter(&gi2c->adap);
>  	if (ret) {
>  		dev_err_probe(dev, ret, "Error adding i2c adapter\n");
> @@ -1143,13 +1148,6 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  
>  	return ret;
>  
> -err_resources:
> -	geni_se_resources_off(&gi2c->se);
> -err_clk:
> -	clk_disable_unprepare(gi2c->core_clk);
> -
> -	return ret;
> -
>  err_dma:
>  	release_gpi_dma(gi2c);
>  
> -- 
> 2.34.1
>
Re: [PATCH v1 08/12] i2c: qcom-geni: Isolate serial engine setup
Posted by Praveen Talari 3 days, 18 hours ago
Hi Bjorn,

Thank you for review.

On 11/26/2025 9:00 PM, Bjorn Andersson wrote:
> On Sat, Nov 22, 2025 at 10:30:14AM +0530, Praveen Talari wrote:
>> Move serial engine configuration from probe to geni_i2c_init().
>>
>> Relocating the serial engine setup to a dedicated initialization function
>> enhances code clarity and simplifies future modifications.
> 
> Please enhance commit message clarity. I don't think "code clarity" is
> your most significant reason for this change, and "simplifies future
> modification" is completely vague.
> 
> Be specific, the reader of this commit message hasn't implemented the
> next set of commits, so they don't understand why this helps.
> 
> If the reason is that this simplifies the error handling around the
> resource acquisition in the next patches, write that.
> 
> If my guess is wrong and the sole reason for you change is that you
> don't like 179 lines long functions, then just say that.
> 

Moving the serial engine setup to geni_i2c_init() API for a cleaner
probe function and utilizes the PM runtime API to control resources 
instead of direct clock-related APIs for better resource management.

Enables reusability of the serial engine initialization in future use 
cases like hibernation and deep sleep features where hardware context is 
lost.

I hope the commit text above should be appropriate.

Thanks,
Praveen

> Regards,
> Bjorn
> 
>>
>> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
>> ---
>>   drivers/i2c/busses/i2c-qcom-geni.c | 148 ++++++++++++++---------------
>>   1 file changed, 73 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
>> index 3a04016db2c3..4111afe2713e 100644
>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>> @@ -976,10 +976,75 @@ static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
>>   	return ret;
>>   }
>>   
>> +static int geni_i2c_init(struct geni_i2c_dev *gi2c)
>> +{
>> +	const struct geni_i2c_desc *desc = NULL;
>> +	u32 proto, tx_depth;
>> +	bool fifo_disable;
>> +	int ret;
>> +
>> +	ret = pm_runtime_resume_and_get(gi2c->se.dev);
>> +	if (ret < 0) {
>> +		dev_err(gi2c->se.dev, "error turning on device :%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	proto = geni_se_read_proto(&gi2c->se);
>> +	if (proto == GENI_SE_INVALID_PROTO) {
>> +		ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
>> +		if (ret) {
>> +			dev_err_probe(gi2c->se.dev, ret, "i2c firmware load failed ret: %d\n", ret);
>> +			goto err;
>> +		}
>> +	} else if (proto != GENI_SE_I2C) {
>> +		ret = dev_err_probe(gi2c->se.dev, -ENXIO, "Invalid proto %d\n", proto);
>> +		goto err;
>> +	}
>> +
>> +	desc = device_get_match_data(gi2c->se.dev);
>> +	if (desc && desc->no_dma_support)
>> +		fifo_disable = false;
>> +	else
>> +		fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
>> +
>> +	if (fifo_disable) {
>> +		/* FIFO is disabled, so we can only use GPI DMA */
>> +		gi2c->gpi_mode = true;
>> +		ret = setup_gpi_dma(gi2c);
>> +		if (ret)
>> +			goto err;
>> +
>> +		dev_dbg(gi2c->se.dev, "Using GPI DMA mode for I2C\n");
>> +	} else {
>> +		gi2c->gpi_mode = false;
>> +		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
>> +
>> +		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
>> +		if (!tx_depth && desc)
>> +			tx_depth = desc->tx_fifo_depth;
>> +
>> +		if (!tx_depth) {
>> +			ret = dev_err_probe(gi2c->se.dev, -EINVAL,
>> +					    "Invalid TX FIFO depth\n");
>> +			goto err;
>> +		}
>> +
>> +		gi2c->tx_wm = tx_depth - 1;
>> +		geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
>> +		geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
>> +				       PACKING_BYTES_PW, true, true, true);
>> +
>> +		dev_dbg(gi2c->se.dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
>> +	}
>> +
>> +err:
>> +	pm_runtime_put(gi2c->se.dev);
>> +	return ret;
>> +}
>> +
>>   static int geni_i2c_probe(struct platform_device *pdev)
>>   {
>>   	struct geni_i2c_dev *gi2c;
>> -	u32 proto, tx_depth, fifo_disable;
>>   	int ret;
>>   	struct device *dev = &pdev->dev;
>>   	const struct geni_i2c_desc *desc = NULL;
>> @@ -1059,79 +1124,19 @@ static int geni_i2c_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> -	ret = clk_prepare_enable(gi2c->core_clk);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = geni_se_resources_on(&gi2c->se);
>> -	if (ret) {
>> -		dev_err_probe(dev, ret, "Error turning on resources\n");
>> -		goto err_clk;
>> -	}
>> -	proto = geni_se_read_proto(&gi2c->se);
>> -	if (proto == GENI_SE_INVALID_PROTO) {
>> -		ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
>> -		if (ret) {
>> -			dev_err_probe(dev, ret, "i2c firmware load failed ret: %d\n", ret);
>> -			goto err_resources;
>> -		}
>> -	} else if (proto != GENI_SE_I2C) {
>> -		ret = dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
>> -		goto err_resources;
>> -	}
>> -
>> -	if (desc && desc->no_dma_support)
>> -		fifo_disable = false;
>> -	else
>> -		fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
>> -
>> -	if (fifo_disable) {
>> -		/* FIFO is disabled, so we can only use GPI DMA */
>> -		gi2c->gpi_mode = true;
>> -		ret = setup_gpi_dma(gi2c);
>> -		if (ret)
>> -			goto err_resources;
>> -
>> -		dev_dbg(dev, "Using GPI DMA mode for I2C\n");
>> -	} else {
>> -		gi2c->gpi_mode = false;
>> -		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
>> -
>> -		/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
>> -		if (!tx_depth && desc)
>> -			tx_depth = desc->tx_fifo_depth;
>> -
>> -		if (!tx_depth) {
>> -			ret = dev_err_probe(dev, -EINVAL,
>> -					    "Invalid TX FIFO depth\n");
>> -			goto err_resources;
>> -		}
>> -
>> -		gi2c->tx_wm = tx_depth - 1;
>> -		geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
>> -		geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
>> -				       PACKING_BYTES_PW, true, true, true);
>> -
>> -		dev_dbg(dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
>> -	}
>> -
>> -	clk_disable_unprepare(gi2c->core_clk);
>> -	ret = geni_se_resources_off(&gi2c->se);
>> -	if (ret) {
>> -		dev_err_probe(dev, ret, "Error turning off resources\n");
>> -		goto err_dma;
>> -	}
>> -
>> -	ret = geni_icc_disable(&gi2c->se);
>> -	if (ret)
>> -		goto err_dma;
>> -
>>   	gi2c->suspended = 1;
>>   	pm_runtime_set_suspended(gi2c->se.dev);
>>   	pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
>>   	pm_runtime_use_autosuspend(gi2c->se.dev);
>>   	pm_runtime_enable(gi2c->se.dev);
>>   
>> +	ret =  geni_i2c_init(gi2c);
>> +	if (ret < 0) {
>> +		dev_err(gi2c->se.dev, "I2C init failed :%d\n", ret);
>> +		pm_runtime_disable(gi2c->se.dev);
>> +		goto err_dma;
>> +	}
>> +
>>   	ret = i2c_add_adapter(&gi2c->adap);
>>   	if (ret) {
>>   		dev_err_probe(dev, ret, "Error adding i2c adapter\n");
>> @@ -1143,13 +1148,6 @@ static int geni_i2c_probe(struct platform_device *pdev)
>>   
>>   	return ret;
>>   
>> -err_resources:
>> -	geni_se_resources_off(&gi2c->se);
>> -err_clk:
>> -	clk_disable_unprepare(gi2c->core_clk);
>> -
>> -	return ret;
>> -
>>   err_dma:
>>   	release_gpi_dma(gi2c);
>>   
>> -- 
>> 2.34.1
>>