| Message ID | 20240829092418.2863659-4-quic_msavaliy@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable shared SE support over I2C | expand |
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@quicinc.com> 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
On 8/29/2024 3:49 PM, Bryan O'Donoghue wrote: > 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. ACKed.
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); /** * geni_se_resources_off() - Turn off resources associated with the serial diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h index 0f038a1a0330..caf2c0c4505b 100644 --- a/include/linux/soc/qcom/geni-se.h +++ b/include/linux/soc/qcom/geni-se.h @@ -1,6 +1,7 @@ /* 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. */ #ifndef _LINUX_QCOM_GENI_SE @@ -494,6 +495,8 @@ int geni_se_resources_off(struct geni_se *se); int geni_se_resources_on(struct geni_se *se); +void geni_se_clks_off(struct geni_se *se); + int geni_se_clk_tbl_get(struct geni_se *se, unsigned long **tbl); int geni_se_clk_freq_match(struct geni_se *se, unsigned long req_freq,
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@quicinc.com> --- drivers/soc/qcom/qcom-geni-se.c | 4 +++- include/linux/soc/qcom/geni-se.h | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-)