| Message ID | 20240906191438.4104329-4-quic_msavaliy@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable shared SE support over I2C | expand |
On 6.09.2024 9:14 PM, Mukesh Kumar Savaliya wrote: > 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. > > Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com> > --- Well, i2c is probably not the only type of client you'll want to share and the current approach requires changes in all protocol drivers. How about adding a parameter like `bool shared_se` to geni_se_resources_off() and changing the pinctrl state conditionally? Konrad
Hi Konrad, Thanks ! On 9/9/2024 5:05 PM, Konrad Dybcio wrote: > On 6.09.2024 9:14 PM, Mukesh Kumar Savaliya wrote: >> 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. >> >> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com> >> --- > > Well, i2c is probably not the only type of client you'll want > to share and the current approach requires changes in all protocol > drivers. > That's true, it may require for other drivers too in future. But logically seems more aligning to be handle at client driver side. Meaning if it's required by SPI client like touch, they can add this flag into touch client DT node. > How about adding a parameter like `bool shared_se` to > geni_se_resources_off() and changing the pinctrl state conditionally? > Sure, Looks good design. This needs a change in common struct geni_se which can add this flag and set to true from i2c driver struct geni_i2c_dev->se->shared_se. Then geni_se_resources_off() can bypass pinctrl state based on this flag. if this is aligning, please confirm so can make the change in V3. > Konrad
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 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. 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(-)