| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <93eec05a-588a-468d-885e-5432c8c26d0a@oss.qualcomm.com>
Date: Wed, 21 Jan 2026 13:17:20 +0530
From: Viken Dadhaniya <viken.dadhaniya@....qualcomm.com>
To: Praveen Talari <praveen.talari@....qualcomm.com>,
Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Mukesh Kumar Savaliya <mukesh.savaliya@....qualcomm.com>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, linux-arm-msm@...r.kernel.org,
linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, bryan.odonoghue@...aro.org,
dmitry.baryshkov@....qualcomm.com, bjorn.andersson@....qualcomm.com
Cc: prasad.sodagudi@....qualcomm.com, quic_vtanuku@...cinc.com,
aniket.randive@....qualcomm.com, chandana.chiluveru@....qualcomm.com
Subject: Re: [PATCH v3 08/12] i2c: qcom-geni: Isolate serial engine setup
Minor comment.
Acked-by: Viken Dadhaniya <viken.dadhaniya@....qualcomm.com>
On 1/12/2026 4:17 PM, Praveen Talari wrote:
> 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 like
> hibernation and deep sleep features where hardware context is lost.
>
> Signed-off-by: Praveen Talari <praveen.talari@....qualcomm.com>
> ---
> v1->v2:
> Bjorn:
> - Updated commit text.
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 154 ++++++++++++++---------------
> 1 file changed, 73 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 3a04016db2c3..58c32ffbd150 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,100 +1124,27 @@ 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);
Double space.
> + if (ret < 0) {
> + pm_runtime_disable(gi2c->se.dev);
> + return ret;
> + }
> +
> ret = i2c_add_adapter(&gi2c->adap);
> if (ret) {
> dev_err_probe(dev, ret, "Error adding i2c adapter\n");
> pm_runtime_disable(gi2c->se.dev);
> - goto err_dma;
> + return ret;
> }
>
> dev_dbg(dev, "Geni-I2C adaptor successfully added\n");
>
> - 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);
> -
> return ret;
> }
>
Powered by blists - more mailing lists