[PATCH v3 0/4] Enable SPI on SA8255p Qualcomm platforms

[PATCH v3 0/4] Enable SPI on SA8255p Qualcomm platforms

[Praveen Talari]

The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power
and performance protocols.

The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power states(on/off).

The SCMI performance protocol manages SPI frequency, with each
frequency rate represented by a performance level. The driver uses
geni_se_set_perf_opp() API to request the desired frequency rate.

As part of geni_se_set_perf_opp(), the OPP for the requested frequency
is obtained using dev_pm_opp_find_freq_floor() and the performance
level is set using dev_pm_opp_set_opp().

Praveen Talari (4):
  spi: dt-bindings: describe SA8255p
  spi: qcom-geni: Use geni_se_resources_init() for resource
    initialization
  spi: qcom-geni: Use resources helper APIs in runtime PM functions
  spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

.../bindings/spi/qcom,sa8255p-geni-spi.yaml   | 63 ++++++++++++++
 drivers/spi/spi-geni-qcom.c                   | 83 ++++++++-----------
 2 files changed, 97 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml

---
Changes in v3:
- Added the OPP rate restoration in resume callback.
- Added missed dma-names in example node.
- Link to v2: https://patch.msgid.link/20260530-enable-spi-on-sa8255p-v2-0-17574601bd63@oss.qualcomm.com

Changes in v2:
- Rebased patches on latest linux-next.
- Link to v1: https://lore.kernel.org/all/20260112190134.1526646-1-praveen.talari@oss.qualcomm.com/

To: Mark Brown <broonie@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
To: bjorn.andersson@oss.qualcomm.com
To: Praveen Talari <praveen.talari@oss.qualcomm.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: mukesh.savaliya@oss.qualcomm.com
Cc: aniket.randive@oss.qualcomm.com
Cc: chandana.chiluveru@oss.qualcomm.com
Cc: jyothi.seerapu@oss.qualcomm.com
Cc: chiluka.harish@oss.qualcomm.com

---
Praveen Talari (4):
      spi: dt-bindings: describe SA8255p
      spi: qcom-geni: Use geni_se_resources_init() for resource initialization
      spi: qcom-geni: Use resources helper APIs in runtime PM functions
      spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

 .../bindings/spi/qcom,sa8255p-geni-spi.yaml        | 64 ++++++++++++++++
 drivers/spi/spi-geni-qcom.c                        | 86 ++++++++++------------
 2 files changed, 104 insertions(+), 46 deletions(-)
---
base-commit: f7af91adc230aa99e23330ecf85bc9badd9780ad
change-id: 20260529-enable-spi-on-sa8255p-8166eaa226cb

Best regards,
--  
Praveen Talari <praveen.talari@oss.qualcomm.com>

[PATCH v3 1/4] spi: dt-bindings: describe SA8255p

[Praveen Talari]

Add DT bindings for the QUP GENI SPI controller on sa8255p platform.

SA8255p platform abstracts resources such as clocks, interconnect and
GPIO pins configuration in Firmware. SCMI power and perf protocols are
utilized to request resource configurations.

SA8255p platform does not require the Serial Engine (SE) common properties
as the SE firmware is loaded and managed by the TrustZone (TZ) secure
environment.

Co-developed-by: Nikunj Kela <quic_nkela@quicinc.com>
Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v2->v3:
- Added missed dma-names in example node.
---
 .../bindings/spi/qcom,sa8255p-geni-spi.yaml        | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml b/Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml
new file mode 100644
index 000000000000..d9347d780ca4
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/qcom,sa8255p-geni-spi.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/qcom,sa8255p-geni-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
+
+maintainers:
+  - Praveen Talari <praveen.talari@oss.qualcomm.com>
+
+properties:
+  compatible:
+    const: qcom,sa8255p-geni-spi
+
+  reg:
+    maxItems: 1
+
+  dmas:
+    maxItems: 2
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+  interrupts:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 2
+
+  power-domain-names:
+    items:
+      - const: power
+      - const: perf
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - power-domains
+  - power-domain-names
+
+allOf:
+  - $ref: /schemas/spi/spi-controller.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/dma/qcom-gpi.h>
+
+    spi@888000 {
+        compatible = "qcom,sa8255p-geni-spi";
+        reg = <0x888000 0x4000>;
+        interrupts = <GIC_SPI 584 IRQ_TYPE_LEVEL_HIGH>;
+        dmas = <&gpi_dma0 0 0 QCOM_GPI_SPI>,
+               <&gpi_dma0 1 0 QCOM_GPI_SPI>;
+        dma-names = "tx", "rx";
+        power-domains = <&scmi0_pd 0>, <&scmi0_dvfs 0>;
+        power-domain-names = "power", "perf";
+    };

-- 
2.34.1

[PATCH v3 2/4] spi: qcom-geni: Use geni_se_resources_init() for resource initialization

[Praveen Talari]

Replace resources initialization such as clocks, ICC path and OPP with the
common geni_se_resources_init() function to avoid code duplication across
all drivers.

The geni_se_resources_init() function handles all these resources
internally, reducing code duplication and ensuring consistent resource
management across GENI SE drivers.

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/spi/spi-geni-qcom.c | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index a04cdc1e5ad4..333216df922e 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1034,7 +1034,6 @@ static int spi_geni_probe(struct platform_device *pdev)
 	struct spi_controller *spi;
 	struct spi_geni_master *mas;
 	void __iomem *base;
-	struct clk *clk;
 	struct device *dev = &pdev->dev;
 
 	irq = platform_get_irq(pdev, 0);
@@ -1049,10 +1048,6 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	clk = devm_clk_get(dev, "se");
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-
 	if (device_property_read_bool(dev, "spi-slave"))
 		spi = devm_spi_alloc_target(dev, sizeof(*mas));
 	else
@@ -1068,17 +1063,10 @@ static int spi_geni_probe(struct platform_device *pdev)
 	mas->se.dev = dev;
 	mas->se.wrapper = dev_get_drvdata(dev->parent);
 	mas->se.base = base;
-	mas->se.clk = clk;
 
-	ret = devm_pm_opp_set_clkname(&pdev->dev, "se");
+	ret = geni_se_resources_init(&mas->se);
 	if (ret)
 		return ret;
-	/* OPP table is optional */
-	ret = devm_pm_opp_of_add_table(&pdev->dev);
-	if (ret && ret != -ENODEV) {
-		dev_err(&pdev->dev, "invalid OPP table in device tree\n");
-		return ret;
-	}
 
 	spi->bus_num = -1;
 	spi->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_CS_HIGH;
@@ -1104,24 +1092,12 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (spi->target)
 		spi->target_abort = spi_geni_target_abort;
 
-	ret = geni_icc_get(&mas->se, NULL);
-	if (ret)
-		return ret;
-
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		return ret;
 
-	/* Set the bus quota to a reasonable value for register access */
-	mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ);
-	mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
-
-	ret = geni_icc_set_bw(&mas->se);
-	if (ret)
-		return ret;
-
 	ret = spi_geni_init(mas);
 	if (ret)
 		return ret;

-- 
2.34.1

[PATCH v3 3/4] spi: qcom-geni: Use resources helper APIs in runtime PM functions

[Praveen Talari]

To manage GENI serial engine resources during runtime power management,
drivers currently need to call functions for ICC, clock, and
SE resource operations in both suspend and resume paths, resulting in
code duplication across drivers.

The new geni_se_resources_activate() and geni_se_resources_deactivate()
helper APIs addresses this issue by providing a streamlined method to
enable or disable all resources based, thereby eliminating redundancy
across drivers.

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v2->v3:
- Added the OPP rate restoration in resume callback.
---
 drivers/spi/spi-geni-qcom.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 333216df922e..420534937695 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1119,16 +1119,8 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
 {
 	struct spi_controller *spi = dev_get_drvdata(dev);
 	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
-	int ret;
-
-	/* Drop the performance state vote */
-	dev_pm_opp_set_rate(dev, 0);
-
-	ret = geni_se_resources_off(&mas->se);
-	if (ret)
-		return ret;
 
-	return geni_icc_disable(&mas->se);
+	return geni_se_resources_deactivate(&mas->se);
 }
 
 static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
@@ -1137,15 +1129,14 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
 	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
 	int ret;
 
-	ret = geni_icc_enable(&mas->se);
+	ret = geni_se_resources_activate(&mas->se);
 	if (ret)
 		return ret;
 
-	ret = geni_se_resources_on(&mas->se);
-	if (ret)
-		return ret;
+	if (mas->se.has_opp)
+		return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
 
-	return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
+	return 0;
 }
 
 static int __maybe_unused spi_geni_suspend(struct device *dev)

-- 
2.34.1

[PATCH v3 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

[Praveen Talari]

The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power
and performance protocols.

The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power states(on/off).

The SCMI performance protocol manages SPI frequency, with each
frequency rate represented by a performance level. The driver uses
geni_se_set_perf_opp() API to request the desired frequency rate.

As part of geni_se_set_perf_opp(), the OPP for the requested frequency
is obtained using dev_pm_opp_find_freq_floor() and the performance
level is set using dev_pm_opp_set_opp().

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/spi/spi-geni-qcom.c | 47 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 420534937695..1b92fb1bd8bf 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -78,6 +78,13 @@
 #define GSI_CPHA		BIT(4)
 #define GSI_CPOL		BIT(5)
 
+struct geni_spi_desc {
+	int (*resources_init)(struct geni_se *se);
+	int (*set_rate)(struct geni_se *se, unsigned long clk_freq);
+	int (*power_on)(struct geni_se *se);
+	int (*power_off)(struct geni_se *se);
+};
+
 struct spi_geni_master {
 	struct geni_se se;
 	struct device *dev;
@@ -105,6 +112,7 @@ struct spi_geni_master {
 	struct dma_chan *tx;
 	struct dma_chan *rx;
 	int cur_xfer_mode;
+	const struct geni_spi_desc *dev_data;
 };
 
 static void spi_slv_setup(struct spi_geni_master *mas)
@@ -305,11 +313,12 @@ static void spi_setup_word_len(struct spi_geni_master *mas, u16 mode,
 	writel(word_len, se->base + SE_SPI_WORD_LEN);
 }
 
-static int geni_spi_set_clock_and_bw(struct spi_geni_master *mas,
-					unsigned long clk_hz)
+static int geni_spi_set_clock_and_bw(struct geni_se *se,
+				     unsigned long clk_hz)
 {
+	struct spi_controller *spi = dev_get_drvdata(se->dev);
+	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
 	u32 clk_sel, m_clk_cfg, idx, div;
-	struct geni_se *se = &mas->se;
 	int ret;
 
 	if (clk_hz == mas->cur_speed_hz)
@@ -820,7 +829,7 @@ static int setup_se_xfer(struct spi_transfer *xfer,
 	}
 
 	/* Speed and bits per word can be overridden per transfer */
-	ret = geni_spi_set_clock_and_bw(mas, xfer->speed_hz);
+	ret = mas->dev_data->set_rate(&mas->se, xfer->speed_hz);
 	if (ret)
 		return ret;
 
@@ -1064,7 +1073,9 @@ static int spi_geni_probe(struct platform_device *pdev)
 	mas->se.wrapper = dev_get_drvdata(dev->parent);
 	mas->se.base = base;
 
-	ret = geni_se_resources_init(&mas->se);
+	mas->dev_data = device_get_match_data(&pdev->dev);
+
+	ret = mas->dev_data->resources_init(&mas->se);
 	if (ret)
 		return ret;
 
@@ -1120,7 +1131,8 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
 	struct spi_controller *spi = dev_get_drvdata(dev);
 	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
 
-	return geni_se_resources_deactivate(&mas->se);
+	return mas->dev_data->power_off ?
+	       mas->dev_data->power_off(&mas->se) : 0;
 }
 
 static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
@@ -1129,9 +1141,11 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
 	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
 	int ret;
 
-	ret = geni_se_resources_activate(&mas->se);
-	if (ret)
-		return ret;
+	if (mas->dev_data->power_on) {
+		ret = mas->dev_data->power_on(&mas->se);
+		if (ret)
+			return ret;
+	}
 
 	if (mas->se.has_opp)
 		return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
@@ -1177,8 +1191,21 @@ static const struct dev_pm_ops spi_geni_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(spi_geni_suspend, spi_geni_resume)
 };
 
+static const struct geni_spi_desc geni_spi = {
+	.resources_init = geni_se_resources_init,
+	.set_rate = geni_spi_set_clock_and_bw,
+	.power_on = geni_se_resources_activate,
+	.power_off = geni_se_resources_deactivate,
+};
+
+static const struct geni_spi_desc sa8255p_geni_spi = {
+	.resources_init = geni_se_domain_attach,
+	.set_rate = geni_se_set_perf_opp,
+};
+
 static const struct of_device_id spi_geni_dt_match[] = {
-	{ .compatible = "qcom,geni-spi" },
+	{ .compatible = "qcom,geni-spi", .data = &geni_spi },
+	{ .compatible = "qcom,sa8255p-geni-spi", .data = &sa8255p_geni_spi },
 	{}
 };
 MODULE_DEVICE_TABLE(of, spi_geni_dt_match);

-- 
2.34.1

Re: [PATCH v3 3/4] spi: qcom-geni: Use resources helper APIs in runtime PM functions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing rollback of activated resources when dev_pm_opp_set_rate() fails during runtime resume.
- [High] Dropping performance state vote before fallible operations leaves the device broken on suspend failure.
- [High] System crash due to unclocked register access when pm_runtime_get_sync() fails in spi_geni_init().
- [High] Silent failure in packing configuration for bits_per_word < 8 leads to inconsistent hardware state and data corruption.
- [High] Asynchronous pm_runtime_put() in probe error path leaves hardware active.
- [Low] Unconditional assignment of dev_pm_ops struct creates dead code when PM is disabled.
--

commit 8aeef46f49b41f7b8567cd06d65f302b46222b6a
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

spi: qcom-geni: Use resources helper APIs in runtime PM functions

This commit introduces the geni_se_resources_activate() and
geni_se_resources_deactivate() helper APIs. These functions reduce
code duplication during runtime power management. The drivers now use
a streamlined method for ICC, clock, and SE resource operations.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 333216df922e8..4205349376955 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -1137,15 +1129,14 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
>  	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
>  	int ret;
>  
> -	ret = geni_icc_enable(&mas->se);
> +	ret = geni_se_resources_activate(&mas->se);
>  	if (ret)
>  		return ret;
>  
> -	ret = geni_se_resources_on(&mas->se);
> -	if (ret)
> -		return ret;
> +	if (mas->se.has_opp)
> +		return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
>  
> -	return dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz);
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but if dev_pm_opp_set_rate() fails here, does
the function return an error without calling geni_se_resources_deactivate()
to roll back the resources enabled by geni_se_resources_activate()? Could this
cause a persistent leak of clock references and interconnect bandwidth?

There are also a few pre-existing issues in this file and related code that
are not directly introduced by this patch, but might be worth addressing:

drivers/soc/qcom/qcom-geni-se.c:geni_se_resources_deactivate() {
    ...
	if (se->has_opp)
		dev_pm_opp_set_rate(se->dev, 0);

	ret = pinctrl_pm_select_sleep_state(se->dev);
	if (ret)
		return ret;
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but dropping the OPP rate to 0
before the fallible pinctrl_pm_select_sleep_state() creates a window for state
inconsistency. If pinctrl fails and an error is returned, PM runtime will leave
the device marked as RPM_ACTIVE. Will subsequent accesses to the device be
unclocked, potentially leading to hardware hangs or asynchronous external aborts?

drivers/spi/spi-geni-qcom.c:spi_geni_driver {
    ...
	.driver = {
		.name = "geni_spi",
		.pm = &spi_geni_pm_ops,
		.of_match_table = spi_geni_dt_match,
	},
    ...
}

[Severity: Low]
This is a pre-existing issue, but does assigning &spi_geni_pm_ops unconditionally
to .pm produce dead code when CONFIG_PM is disabled? Should this be wrapped with
pm_ptr() to ensure the callbacks are properly conditionally compiled out?

drivers/spi/spi-geni-qcom.c:spi_geni_init() {
    ...
	pm_runtime_get_sync(mas->dev);

	proto = geni_se_read_proto(se);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does the unchecked
pm_runtime_get_sync() allow execution to proceed to geni_se_read_proto() even
if it fails? Could this trigger an unclocked register access and cause an
asynchronous external abort?

drivers/soc/qcom/qcom-geni-se.c:geni_se_config_packing() {
    ...
	int iter = (ceil_bpw * pack_words) / BITS_PER_BYTE;
	int i;

	if (iter <= 0 || iter > NUM_PACKING_VECTORS)
		return;
    ...
}

[Severity: High]
This is a pre-existing issue, but for valid bits_per_word values less than 8,
doesn't iter exceed NUM_PACKING_VECTORS (4)? If the function silently returns
without writing to the SE_GENI_TX/RX_PACKING_CFGn registers, will the hardware
operate using stale packing configurations and potentially corrupt data?

drivers/spi/spi-geni-qcom.c:spi_geni_init() {
    ...
out_pm:
	pm_runtime_put(mas->dev);
	return ret;
}

[Severity: High]
This is a pre-existing issue, but doesn't the use of the asynchronous
pm_runtime_put() in the error path allow pm_runtime_disable() (called during
device removal or probe failure) to cancel the pending suspend? Does this leave
the hardware active and permanently leak clock and interconnect resources?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-enable-spi-on-sa8255p-v3-0-43984eac4c67@oss.qualcomm.com?part=3

Re: [PATCH v3 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete abstraction of SPI clock management breaks both GPI DMA and FIFO/SE_DMA modes for SCMI-based platforms.
- [High] Unchecked return value of `device_get_match_data()` leads to a NULL pointer dereference in probe.
--

commit d77f464166f40ea072baeed5f03c9c24a36aac4d
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
This patch adds support for these platforms by introducing an abstraction
for setting the frequency using the SCMI performance protocol.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 4205349376955..1b92fb1bd8bfb 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -820,7 +829,7 @@ static int setup_se_xfer(struct spi_transfer *xfer,
>  	}
>  
>  	/* Speed and bits per word can be overridden per transfer */
> -	ret = geni_spi_set_clock_and_bw(mas, xfer->speed_hz);
> +	ret = mas->dev_data->set_rate(&mas->se, xfer->speed_hz);
>  	if (ret)
>  		return ret;

[Severity: High]
Does this new abstraction break GPI DMA mode for SCMI-based platforms?

In GPI DMA mode, setup_gsi_xfer() completely bypasses this set_rate
abstraction and directly calls get_spi_clk_cfg():

setup_gsi_xfer() {
    ...
    ret = get_spi_clk_cfg(mas->cur_speed_hz, mas,
                          &peripheral.clk_src, &peripheral.clk_div);
    ...
}

Because SCMI platforms do not initialize se->clk, clk_round_rate() inside
geni_se_clk_tbl_get() will return 0 on a NULL clock, unconditionally failing
the transfer.

Also, for FIFO/SE_DMA modes, setup_se_xfer() correctly uses the new
set_rate() callback (mapped to geni_se_set_perf_opp()). However,
geni_se_set_perf_opp() only sets the SCMI performance OPP and completely
omits the required hardware register writes to SE_GENI_CLK_SEL and
GENI_SER_M_CLK_CFG.

Without setting the SER_CLK_EN bit in GENI_SER_M_CLK_CFG, will the internal
serial engine clock remain disabled and prevent the SPI hardware from
functioning?

[ ... ]

> @@ -1064,7 +1073,9 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	mas->se.wrapper = dev_get_drvdata(dev->parent);
>  	mas->se.base = base;
>  
> -	ret = geni_se_resources_init(&mas->se);
> +	mas->dev_data = device_get_match_data(&pdev->dev);
> +
> +	ret = mas->dev_data->resources_init(&mas->se);

[Severity: High]
Could device_get_match_data() return NULL here?

If the driver is instantiated via a method that does not use the OF match
table, such as manual bind via sysfs or legacy platform device instantiation,
device_get_match_data() will return NULL. Dereferencing it immediately to call
resources_init() would lead to a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-enable-spi-on-sa8255p-v3-0-43984eac4c67@oss.qualcomm.com?part=4

Re: [PATCH v3 2/4] spi: qcom-geni: Use geni_se_resources_init() for resource initialization

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver ignores the return value of pm_runtime_get_sync() before accessing hardware registers in spi_geni_init().
--

commit 297907766125fb69231313b230385906933609be
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

spi: qcom-geni: Use geni_se_resources_init() for resource initialization

Replace resources initialization such as clocks, ICC path and OPP with the
common geni_se_resources_init() function to avoid code duplication. This
handles all these resources internally, ensuring consistent resource
management across GENI SE drivers.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index a04cdc1e5ad4d..333216df922e8 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -1104,24 +1092,12 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	if (spi->target)
>  		spi->target_abort = spi_geni_target_abort;
>  
> -	ret = geni_icc_get(&mas->se, NULL);
> -	if (ret)
> -		return ret;
> -
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
>  	ret = devm_pm_runtime_enable(dev);
>  	if (ret)
>  		return ret;
>  
> -	/* Set the bus quota to a reasonable value for register access */
> -	mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ);
> -	mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
> -
> -	ret = geni_icc_set_bw(&mas->se);
> -	if (ret)
> -		return ret;
> -
>  	ret = spi_geni_init(mas);

[Severity: High]
This is a pre-existing issue, but does spi_geni_init() fail to check the
return value of pm_runtime_get_sync() before accessing hardware registers?

drivers/spi/spi-geni-qcom.c:spi_geni_init() {
    ...
    pm_runtime_get_sync(mas->dev);

    proto = geni_se_read_proto(se);
    ...
}

The pm_runtime_get_sync() function can return a negative error code if the
device fails to resume. By not checking the return value, spi_geni_init()
proceeds to read from the hardware via geni_se_read_proto().

On QCOM platforms, reading from unclocked or powered-off peripheral
registers causes a synchronous external abort (bus error), leading to a
kernel panic.

>  	if (ret)
>  		return ret;
>  
> -	/*
> -	 * TX is required per GSI spec, see setup_gsi_xfer().
> -	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-enable-spi-on-sa8255p-v3-0-43984eac4c67@oss.qualcomm.com?part=2