Re: [PATCH v1 3/4] soc: qcom: geni-se: Export function geni_se_clks_off()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29/08/2024 10:24, Mukesh Kumar Savaliya wrote:
Currently driver provides geni_se_resources_off() function to turn
off SE resources like clocks, pinctrl. But we don't have function to
control clock separately, hence export function geni_se_clks_off()
to turn off clocks separately without disturbing GPIO.

The client drivers like i2c requires this function for use case where
i2c SE is shared between two subsystems.

Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx>

Suggest:

Currently the driver provides a function called geni_serial_resources_off() to turn off resources like clocks and pinctrl. We don't have a function to control clocks separately hence, export the function geni_se_clks_off() to turn off clocks separately without disturbing GPIO.

Client drivers like I2C require this function for use-cases where the I2C SE is shared between two subsystems.

---
  drivers/soc/qcom/qcom-geni-se.c  | 4 +++-
  include/linux/soc/qcom/geni-se.h | 3 +++
  2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 2e8f24d5da80..20166c8fc919 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -1,5 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  // Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+// Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
/* Disable MMIO tracing to prevent excessive logging of unwanted MMIO traces */
  #define __DISABLE_TRACE_MMIO__
@@ -482,13 +483,14 @@ void geni_se_config_packing(struct geni_se *se, int bpw, int pack_words,
  }
  EXPORT_SYMBOL_GPL(geni_se_config_packing);
-static void geni_se_clks_off(struct geni_se *se)
+void geni_se_clks_off(struct geni_se *se)
  {
  	struct geni_wrapper *wrapper = se->wrapper;
clk_disable_unprepare(se->clk);
  	clk_bulk_disable_unprepare(wrapper->num_clks, wrapper->clks);
  }
+EXPORT_SYMBOL_GPL(geni_se_clks_off);


Does it make sense to have geni_se_clks_off() exported without having geni_se_clks_on() similarly exported ?

Two exported functions already appear to wrapper this functionality for you.

geni_se_resources_off -> gensi_se_clks_off
geni_se_resources_on -> gensi_se_clks_on

Seems like a usage violation to have geni_se_resources_on() switch the clocks on but then have something else directly call gensi_se_clks_off() without going through geni_se_resources_off();

?

---
bod




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux