Message ID | 20230518113800.339158-1-srinivas.kandagatla@linaro.org |
---|---|
Headers | show |
Series | clk: qcom: sc8280xp: add lpasscc reset control | expand |
On Thu, May 18, 2023 at 12:37:58PM +0100, Srinivas Kandagatla wrote: > Add support for the lpass clock controller found on SC8280XP based devices. > This would allow lpass peripheral loader drivers to control the clocks and > bring the subsystems out of reset. > > Currently this patch only supports resets as the Q6DSP is in control of > LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg > channel. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/clk/qcom/Kconfig | 8 ++++ > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/lpasscc-sc8280xp.c | 71 +++++++++++++++++++++++++++++ > 3 files changed, 80 insertions(+) > create mode 100644 drivers/clk/qcom/lpasscc-sc8280xp.c > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index 12be3e2371b3..8188f4dedf40 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -514,6 +514,14 @@ config SC_GPUCC_8280XP > Say Y if you want to support graphics controller devices and > functionality such as 3D graphics. > > +config SC_LPASSCC_8280XP Should go after SC_LPASSCC_7280. > + tristate "SC8280 Low Power Audio Subsystem (LPASS) Clock Controller" > + select SC_GCC_8280XP > + help > + Support for the LPASS clock controller on SC8280XP devices. > + Say Y if you want to use the LPASS branch clocks of the LPASS clock > + controller to reset the LPASS subsystem. > + > config SC_LPASSCC_7280 > tristate "SC7280 Low Power Audio Subsystem (LPASS) Clock Controller" > select SC_GCC_7280 > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile > index 9ff4c373ad95..dce2dd639524 100644 > --- a/drivers/clk/qcom/Makefile > +++ b/drivers/clk/qcom/Makefile > @@ -71,6 +71,7 @@ obj-$(CONFIG_SC_CAMCC_7280) += camcc-sc7280.o > obj-$(CONFIG_SC_DISPCC_7180) += dispcc-sc7180.o > obj-$(CONFIG_SC_DISPCC_7280) += dispcc-sc7280.o > obj-$(CONFIG_SC_DISPCC_8280XP) += dispcc-sc8280xp.o > +obj-$(CONFIG_SC_LPASSCC_8280XP) += lpasscc-sc8280xp.o This looks misplaced too. > obj-$(CONFIG_SA_GCC_8775P) += gcc-sa8775p.o > obj-$(CONFIG_SA_GPUCC_8775P) += gpucc-sa8775p.o > obj-$(CONFIG_SC_GCC_7180) += gcc-sc7180.o > diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c > new file mode 100644 > index 000000000000..118320f8ee40 > --- /dev/null > +++ b/drivers/clk/qcom/lpasscc-sc8280xp.c > @@ -0,0 +1,71 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022, Linaro Limited > + */ > + > +#include <linux/clk-provider.h> > +#include <linux/err.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > +#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h> > +#include "common.h" > +#include "reset.h" Nit: add newline separators before dt-bindings and local includes, respectively? > +static int __init lpasscc_sc8280xp_init(void) > +{ > + return platform_driver_register(&lpasscc_sc8280xp_driver); > +} > +subsys_initcall(lpasscc_sc8280xp_init); Do you really need subsys init for this? I've been using this driver as a module on the X13s and it seems to work fine. > +static void __exit lpasscc_sc8280xp_exit(void) > +{ > + platform_driver_unregister(&lpasscc_sc8280xp_driver); > +} > +module_exit(lpasscc_sc8280xp_exit); > + > +MODULE_DESCRIPTION("QTI LPASSCC SC8280XP Driver"); > +MODULE_LICENSE("GPL"); Johan
On Thu, May 18, 2023 at 12:37:59PM +0100, Srinivas Kandagatla wrote: > Add support for the lpass audio clock controller found on SC8280XP based > devices. This would allow lpass peripheral loader drivers to control the > clocks and bring the subsystems out of reset. > > Currently this patch only supports resets as the Q6DSP is in control of > LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg > channel. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/clk/qcom/lpasscc-sc8280xp.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c > index 118320f8ee40..e221ae2d40ae 100644 > --- a/drivers/clk/qcom/lpasscc-sc8280xp.c > +++ b/drivers/clk/qcom/lpasscc-sc8280xp.c > @@ -13,6 +13,26 @@ > #include "common.h" > #include "reset.h" > > +static const struct qcom_reset_map lpass_audio_csr_sc8280xp_resets[] = { > + [LPASS_AUDIO_SWR_RX_CGCR] = { 0xa0, 1 }, > + [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 }, > + [LPASS_AUDIO_SWR_WSA2_CGCR] = { 0xd8, 1 }, > +}; > + > +static struct regmap_config lpass_audio_csr_sc8280xp_regmap_config = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > + .name = "lpass-audio-csr", Should you update this name to match the new compatible ("lpassaudiocc")? > + .max_register = 0x1000, > +}; > + > +static const struct qcom_cc_desc lpass_audio_csr_reset_sc8280xp_desc = { Same here (and for the reset struct as well as previous patch). > + .config = &lpass_audio_csr_sc8280xp_regmap_config, > + .resets = lpass_audio_csr_sc8280xp_resets, > + .num_resets = ARRAY_SIZE(lpass_audio_csr_sc8280xp_resets), > +}; > + > static const struct qcom_reset_map lpass_tcsr_sc8280xp_resets[] = { > [LPASS_AUDIO_SWR_TX_CGCR] = { 0xc010, 1 }, > }; > @@ -33,6 +53,9 @@ static const struct qcom_cc_desc lpass_tcsr_reset_sc8280xp_desc = { > > static const struct of_device_id lpasscc_sc8280xp_match_table[] = { > { > + .compatible = "qcom,sc8280xp-lpassaudiocc", > + .data = &lpass_audio_csr_reset_sc8280xp_desc, > + }, { > .compatible = "qcom,sc8280xp-lpasscc", > .data = &lpass_tcsr_reset_sc8280xp_desc, > }, Johan
On Thu, May 18, 2023 at 12:38:00PM +0100, Srinivas Kandagatla wrote: > Soundwire controllers on sc8280xp needs an explicit reset, this > patch adds support for this. s/this patch adds/add/ > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index d2a2224d138a..a2d0f8abe23d 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -6,6 +6,7 @@ > > #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h> > #include <dt-bindings/clock/qcom,gcc-sc8280xp.h> > +#include <dt-bindings/clock/qcom,lpasscc-sc8280xp.h> > #include <dt-bindings/clock/qcom,rpmh.h> > #include <dt-bindings/interconnect/qcom,osm-l3.h> > #include <dt-bindings/interconnect/qcom,sc8280xp.h> > @@ -2548,6 +2549,8 @@ rxmacro: rxmacro@3200000 { > swr1: soundwire-controller@3210000 { > compatible = "qcom,soundwire-v1.6.0"; > reg = <0 0x03210000 0 0x2000>; > + resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>; > + reset-names = "swr_audio_cgcr"; Move after clocks. > interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&rxmacro>; > clock-names = "iface"; > @@ -2647,6 +2650,13 @@ swr0: soundwire-controller@3250000 { > status = "disabled"; > }; > > + lpass_audiocc: clock-controller@3300000 { > + compatible = "qcom,sc8280xp-lpassaudiocc"; > + reg = <0 0x032a9000 0 0x1000>; Either this property or the unit address is wrong as they do not match. The bindings currently mandates that vendor property you added ("qcom,adsp-pil-mode), but you left it out here. > + #reset-cells = <1>; > + #clock-cells = <1>; clock before reset for some sort order. > + }; > + Your preliminary version of this patch also added a reset to swr0, which has been left out here. Was that not needed? > swr2: soundwire-controller@3330000 { > compatible = "qcom,soundwire-v1.6.0"; > reg = <0 0x03330000 0 0x2000>; > @@ -2654,6 +2664,8 @@ swr2: soundwire-controller@3330000 { > <GIC_SPI 520 IRQ_TYPE_LEVEL_HIGH>; > interrupt-names = "core", "wakeup"; > > + resets = <&lpasscc LPASS_AUDIO_SWR_TX_CGCR>; > + reset-names = "swr_audio_cgcr"; Add after clocks. > clocks = <&txmacro>; > clock-names = "iface"; > label = "TX"; > @@ -2849,6 +2861,13 @@ data-pins { > }; > }; > > + lpasscc: clock-controller@33e0000 { > + compatible = "qcom,sc8280xp-lpasscc"; > + reg = <0 0x033e0000 0 0x21000>; Your driver (and the binding example) seems to suggest that the size here should be 0x12000. The vendor property appears to be missing here too (or the binding is incorrect). > + #reset-cells = <1>; > + #clock-cells = <1>; clock before reset > + }; > + > usb_0_qmpphy: phy@88eb000 { > compatible = "qcom,sc8280xp-qmp-usb43dp-phy"; > reg = <0 0x088eb000 0 0x4000>; Johan
On Thu, May 18, 2023 at 12:37:58PM +0100, Srinivas Kandagatla wrote: > +config SC_LPASSCC_8280XP > + tristate "SC8280 Low Power Audio Subsystem (LPASS) Clock Controller" > + select SC_GCC_8280XP > + help > + Support for the LPASS clock controller on SC8280XP devices. > + Say Y if you want to use the LPASS branch clocks of the LPASS clock > + controller to reset the LPASS subsystem. And please include a defconfig update for this one as a separate patch in the next revision as it is needed for audio on the X13s. Johan
On 22/05/2023 09:33, Johan Hovold wrote: > On Thu, May 18, 2023 at 12:37:59PM +0100, Srinivas Kandagatla wrote: >> Add support for the lpass audio clock controller found on SC8280XP based >> devices. This would allow lpass peripheral loader drivers to control the >> clocks and bring the subsystems out of reset. >> >> Currently this patch only supports resets as the Q6DSP is in control of >> LPASS IP which manages most of the clocks via Q6PRM service on GPR rpmsg >> channel. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/clk/qcom/lpasscc-sc8280xp.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/drivers/clk/qcom/lpasscc-sc8280xp.c b/drivers/clk/qcom/lpasscc-sc8280xp.c >> index 118320f8ee40..e221ae2d40ae 100644 >> --- a/drivers/clk/qcom/lpasscc-sc8280xp.c >> +++ b/drivers/clk/qcom/lpasscc-sc8280xp.c >> @@ -13,6 +13,26 @@ >> #include "common.h" >> #include "reset.h" >> >> +static const struct qcom_reset_map lpass_audio_csr_sc8280xp_resets[] = { >> + [LPASS_AUDIO_SWR_RX_CGCR] = { 0xa0, 1 }, >> + [LPASS_AUDIO_SWR_WSA_CGCR] = { 0xb0, 1 }, >> + [LPASS_AUDIO_SWR_WSA2_CGCR] = { 0xd8, 1 }, >> +}; >> + >> +static struct regmap_config lpass_audio_csr_sc8280xp_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 32, >> + .name = "lpass-audio-csr", > > Should you update this name to match the new compatible > ("lpassaudiocc")? This name reflects the name from data sheet, keeping it that way would be useful. --srini > >> + .max_register = 0x1000, >> +}; >> + >> +static const struct qcom_cc_desc lpass_audio_csr_reset_sc8280xp_desc = { > > Same here (and for the reset struct as well as previous patch). > >> + .config = &lpass_audio_csr_sc8280xp_regmap_config, >> + .resets = lpass_audio_csr_sc8280xp_resets, >> + .num_resets = ARRAY_SIZE(lpass_audio_csr_sc8280xp_resets), >> +}; >> + >> static const struct qcom_reset_map lpass_tcsr_sc8280xp_resets[] = { >> [LPASS_AUDIO_SWR_TX_CGCR] = { 0xc010, 1 }, >> }; >> @@ -33,6 +53,9 @@ static const struct qcom_cc_desc lpass_tcsr_reset_sc8280xp_desc = { >> >> static const struct of_device_id lpasscc_sc8280xp_match_table[] = { >> { >> + .compatible = "qcom,sc8280xp-lpassaudiocc", >> + .data = &lpass_audio_csr_reset_sc8280xp_desc, >> + }, { >> .compatible = "qcom,sc8280xp-lpasscc", >> .data = &lpass_tcsr_reset_sc8280xp_desc, >> }, > > Johan