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

[PATCH v4 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 v4:
- Added Reviewed-by tags.
- Added return check for dev_data variable.
- Link to v3: https://lore.kernel.org/all/20260604-enable-spi-on-sa8255p-v3-0-43984eac4c67@oss.qualcomm.com

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>

---
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                        | 88 +++++++++++-----------
 2 files changed, 106 insertions(+), 46 deletions(-)
---
base-commit: 4fa3f5fabb30bf00d7475d5a33459ea83d639bf9
change-id: 20260529-enable-spi-on-sa8255p-8166eaa226cb

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

[PATCH v4 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>
---
 .../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 v4 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.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
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 26e723cfea61..09feb6577efc 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1045,7 +1045,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);
@@ -1060,10 +1059,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
@@ -1079,17 +1074,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;
@@ -1115,24 +1103,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

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

[sashiko-bot]

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

New issues:
- [Low] The `geni_se_resources_init()` function is missing from the kernel tree.

Pre-existing issues:
- [High] Asynchronous `pm_runtime_put()` is used on error paths that immediately call `pm_runtime_disable()`, risking the hardware being left in an invalid active state.
--

commit e8e291bf8f15d73b4da4bf256d08b86fec92a093
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
spi: qcom-geni: Use geni_se_resources_init() for resource initialization

This commit replaces individual resource initializations such as clocks, ICC
paths, and OPP tables with a common geni_se_resources_init() function. This
aims to reduce code duplication and ensure consistent resource management
across GENI SE drivers.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 26e723cfea61e..09feb6577efcb 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -1079,17 +1074,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);

[Severity: Low]
Is this function defined anywhere in the kernel tree?

It doesn't appear to be declared or defined in this commit or the existing
codebase, which results in an implicit declaration error and linker failure.

>  	if (ret)
>  		return ret;

[ ... ]

> @@ -1115,24 +1103,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);

[Severity: High]
This is a pre-existing issue, but I noticed a potential resource leak related
to how runtime PM is disabled.

If probe fails later on, the devres framework will immediately call
pm_runtime_disable() to undo devm_pm_runtime_enable().

Does this cause the asynchronous pm_runtime_put() called inside
spi_geni_init() to have its queued idle/suspend work cancelled? This could
leave the device permanently powered on after a probe failure.

>  	if (ret)
>  		return ret;

[ ... ]

>  	ret = spi_geni_init(mas);
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com?part=2

[PATCH v4 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.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 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 09feb6577efc..025165c4b952 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1130,16 +1130,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)
@@ -1148,15 +1140,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

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

[sashiko-bot]

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

New issues:
- [Low] The patch relies on APIs and structure fields that do not exist in the baseline kernel tree or the rest of the patch series, causing fatal compilation errors.

Pre-existing issues:
- [High] Hardware registers are accessed without checking if `pm_runtime_get_sync()` successfully resumed the device.
--

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

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

This commit updates the geni SPI driver's runtime power management
functions to use streamlined resource activation and deactivation
helpers. It aims to eliminate code duplication across drivers.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 09feb6577efcb..025165c4b9527 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -1130,16 +1130,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);

[Severity: Low]
It appears the geni_se_resources_deactivate() and
geni_se_resources_activate() functions are not implemented in the baseline
tree, which could lead to immediate build failures.

Are these new helper APIs defined elsewhere in this patch series?

>  }
>  
>  static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
> @@ -1148,15 +1140,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)

[Severity: Low]
The has_opp field appears to be missing from the definition of
struct geni_se, which would cause a compilation error.

Does the has_opp field exist in struct geni_se in another patch within
this series?

> +		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 looking at spi_geni_init() in
drivers/spi/spi-geni-qcom.c:

	pm_runtime_get_sync(mas->dev);

	proto = geni_se_read_proto(se);

If pm_runtime_get_sync() fails to resume the device, accessing unclocked
or powered-off registers via geni_se_read_proto() could trigger a
synchronous external abort and result in a kernel panic.

Should the return value of pm_runtime_get_sync() be checked before
accessing hardware registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com?part=3

[PATCH v4 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>
---
v3->v4
- Added return check for dev_data.
---
 drivers/spi/spi-geni-qcom.c | 49 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 025165c4b952..88ac0833351c 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)
@@ -825,7 +834,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;
 
@@ -1075,7 +1084,11 @@ 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);
+	if (!mas->dev_data)
+		return -EINVAL;
+
+	ret = mas->dev_data->resources_init(&mas->se);
 	if (ret)
 		return ret;
 
@@ -1131,7 +1144,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)
@@ -1140,9 +1154,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);
@@ -1188,8 +1204,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 v4 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

[sashiko-bot]

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

New issues:
- [High] Incomplete abstraction in `setup_gsi_xfer()` breaks GPI DMA transfers for SA8255p.
- [High] `spi_geni_runtime_resume()` hardcodes legacy OPP clock restore, breaking performance state restoration on SA8255p.
- [Low] Compilation fails due to undeclared identifiers `geni_se_domain_attach` and `geni_se_set_perf_opp`.

Pre-existing issues:
- [High] Ignored `pm_runtime_get_sync()` return value in `spi_geni_init()` causes kernel panic on initialization failure.
--

commit 41d8ba8fbc50744c00c196e2484d690b4592cc0a
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
Subject: spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

This commit adds support for the SA8255p SoC to the qcom-geni SPI driver.
It abstracts resources initialization and rate setting into a new
geni_spi_desc structure to handle SCMI-based power and performance protocols
required by this platform.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 025165c4b9527..88ac0833351c0 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -825,7 +834,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]
Since setup_se_xfer() now uses the new set_rate callback, should
setup_gsi_xfer() be updated as well?

Currently, setup_gsi_xfer() appears to hardcode a call to the legacy
get_spi_clk_cfg() function, which bypasses mas->dev_data->set_rate.
Could this break GPI DMA transfers for the new SA8255p platform by leaving
the hardware misconfigured?

[ ... ]

> @@ -1075,7 +1084,11 @@ 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);
> +	if (!mas->dev_data)
> +		return -EINVAL;
> +
> +	ret = mas->dev_data->resources_init(&mas->se);
>  	if (ret)
>  		return ret;

[Severity: High]
This is a pre-existing issue, but does spi_geni_init() safely handle power
domain or clock enablement failures?

In spi_geni_init(), which is called shortly after this probe setup, the
return value of pm_runtime_get_sync() is ignored before accessing hardware:

    pm_runtime_get_sync(mas->dev);
    proto = geni_se_read_proto(se);

If pm_runtime_get_sync() fails, could the subsequent read from the
GENI_FW_REVISION_RO register via geni_se_read_proto() cause an asynchronous
external abort and panic the kernel?

[ ... ]

> @@ -1140,9 +1154,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);

[Severity: High]
Does this hardcoded OPP restore bypass the new set_rate abstraction for the
SA8255p platform?

For SA8255p, mas->dev_data->set_rate is geni_se_set_perf_opp(), which
does not update the legacy mas->cur_sclk_hz variable. If has_opp is true,
could this accidentally set the clock rate to 0 and stall the device?
Alternatively, if has_opp is false, could this fail to restore the
performance state entirely?

[ ... ]

> @@ -1188,8 +1204,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,
> +};

[Severity: Low]
Are geni_se_domain_attach and geni_se_set_perf_opp declared elsewhere?

It looks like these identifiers are not defined in this file or the included
headers, which might cause a build failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com?part=4