mbox series

[0/3] SM6115 LPASSCC

Message ID 20230825-topic-6115_lpasscc-v1-0-d4857be298e3@linaro.org
Headers show
Series SM6115 LPASSCC | expand

Message

Konrad Dybcio Aug. 25, 2023, 6:13 p.m. UTC
This series brings support for the LPASS clock controllers on the SM6115
and similar. It provides resets that need to be toggled as part of
soundwire bringup routines.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (3):
      dt-bindings: clock: Add Qualcomm SM6115 LPASS clock controller
      clk: qcom: reset: Increase max reset delay
      clk: qcom: Add SM6115 LPASSCC

 .../bindings/clock/qcom,sm6115-lpasscc.yaml        | 53 ++++++++++++++
 drivers/clk/qcom/Kconfig                           |  9 +++
 drivers/clk/qcom/Makefile                          |  1 +
 drivers/clk/qcom/lpasscc-sm6115.c                  | 84 ++++++++++++++++++++++
 drivers/clk/qcom/reset.h                           |  2 +-
 include/dt-bindings/clock/qcom,sm6115-lpasscc.h    | 15 ++++
 6 files changed, 163 insertions(+), 1 deletion(-)
---
base-commit: 6269320850097903b30be8f07a5c61d9f7592393
change-id: 20230825-topic-6115_lpasscc-02a4f5793acd

Best regards,

Comments

Dmitry Baryshkov Aug. 26, 2023, 2:09 p.m. UTC | #1
On Sat, 26 Aug 2023 at 12:23, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 25/08/2023 20:13, Konrad Dybcio wrote:
> > SM6115 (and its derivatives or similar SoCs) have a LPASS clock
> > controller block which provides audio-related resets.
> >
> > Add the required code to support them.
> >
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > ---
> >  drivers/clk/qcom/Kconfig          |  9 +++++
> >  drivers/clk/qcom/Makefile         |  1 +
> >  drivers/clk/qcom/lpasscc-sm6115.c | 84 +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 94 insertions(+)
> >
> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> > index bd9bfb11b328..df9cf112e4b6 100644
> > --- a/drivers/clk/qcom/Kconfig
> > +++ b/drivers/clk/qcom/Kconfig
> > @@ -1001,6 +1001,15 @@ config SM_GPUCC_8550
> >         Say Y if you want to support graphics controller devices and
> >         functionality such as 3D graphics.
> >
> > +config SM_LPASSCC_6115
> > +     tristate "SM6115 Low Power Audio Subsystem (LPASS) Clock Controller"
> > +     depends on ARM64 || COMPILE_TEST
> > +     select SM_GCC_6115
> > +     help
> > +       Support for the LPASS clock controller on SM6115 devices.
> > +       Say Y if you want to toggle LPASS-adjacent resets within
> > +       this clock controller to reset the LPASS subsystem.
> > +
> >  config SM_TCSRCC_8550
> >       tristate "SM8550 TCSR Clock Controller"
> >       depends on ARM64 || COMPILE_TEST
> > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> > index 4790c8cca426..61e3c72fe954 100644
> > --- a/drivers/clk/qcom/Makefile
> > +++ b/drivers/clk/qcom/Makefile
> > @@ -128,6 +128,7 @@ obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
> >  obj-$(CONFIG_SM_GPUCC_8350) += gpucc-sm8350.o
> >  obj-$(CONFIG_SM_GPUCC_8450) += gpucc-sm8450.o
> >  obj-$(CONFIG_SM_GPUCC_8550) += gpucc-sm8550.o
> > +obj-$(CONFIG_SM_LPASSCC_6115) += lpasscc-sm6115.o
> >  obj-$(CONFIG_SM_TCSRCC_8550) += tcsrcc-sm8550.o
> >  obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
> >  obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o
> > diff --git a/drivers/clk/qcom/lpasscc-sm6115.c b/drivers/clk/qcom/lpasscc-sm6115.c
> > new file mode 100644
> > index 000000000000..6aa19e16c53b
> > --- /dev/null
> > +++ b/drivers/clk/qcom/lpasscc-sm6115.c
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2022, 2023 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,sm6115-lpasscc.h>
> > +
> > +#include "common.h"
> > +#include "reset.h"
> > +
> > +static const struct qcom_reset_map lpass_audiocc_sm6115_resets[] = {
> > +     [LPASS_AUDIO_SWR_RX_CGCR] =  { .reg = 0x98, .bit = 1, .udelay = 500 },
> > +};
> > +
> > +static struct regmap_config lpass_audiocc_sm6115_regmap_config = {
> > +     .reg_bits = 32,
> > +     .reg_stride = 4,
> > +     .val_bits = 32,
> > +     .name = "lpass-audio-csr",
> > +     .max_register = 0x1000,
> > +};
> > +
> > +static const struct qcom_cc_desc lpass_audiocc_sm6115_reset_desc = {
> > +     .config = &lpass_audiocc_sm6115_regmap_config,
> > +     .resets = lpass_audiocc_sm6115_resets,
> > +     .num_resets = ARRAY_SIZE(lpass_audiocc_sm6115_resets),
> > +};
> > +
> > +static const struct qcom_reset_map lpasscc_sm6115_resets[] = {
> > +     [LPASS_SWR_TX_CONFIG_CGCR] = { .reg = 0x100, .bit = 1, .udelay = 500 },
> > +};
> > +
> > +static struct regmap_config lpasscc_sm6115_regmap_config = {
> > +     .reg_bits = 32,
> > +     .reg_stride = 4,
> > +     .val_bits = 32,
> > +     .name = "lpass-tcsr",
> > +     .max_register = 0x1000,
> > +};
> > +
> > +static const struct qcom_cc_desc lpasscc_sm6115_reset_desc = {
> > +     .config = &lpasscc_sm6115_regmap_config,
> > +     .resets = lpasscc_sm6115_resets,
> > +     .num_resets = ARRAY_SIZE(lpasscc_sm6115_resets),
> > +};
> > +
> > +static const struct of_device_id lpasscc_sm6115_match_table[] = {
> > +     {
> > +             .compatible = "qcom,sm6115-lpassaudiocc",
> > +             .data = &lpass_audiocc_sm6115_reset_desc,
> > +     }, {
> > +             .compatible = "qcom,sm6115-lpasscc",
> > +             .data = &lpasscc_sm6115_reset_desc,
> > +     },
> > +     { },
> > +};
> > +MODULE_DEVICE_TABLE(of, lpasscc_sm6115_match_table);
>
> Everything here is almost the same as sc8280xp one, so this should be
> added to sc8280xp. You cut some boilerplate and additional driver.

We have been there. It quickly becomes a nightmare to maintain.
Consider dispcc-sm8250.c

But I agree with you, this code looks too similar. If we expect more
similar lpasscc drivers, which provide no clocks, just several resets,
maybe we can create a common generic wrapper and make resets lists
corresponding driver data?
Krzysztof Kozlowski Aug. 26, 2023, 2:29 p.m. UTC | #2
On 26/08/2023 16:09, Dmitry Baryshkov wrote:

>>> +MODULE_DEVICE_TABLE(of, lpasscc_sm6115_match_table);
>>
>> Everything here is almost the same as sc8280xp one, so this should be
>> added to sc8280xp. You cut some boilerplate and additional driver.
> 
> We have been there. It quickly becomes a nightmare to maintain.
> Consider dispcc-sm8250.c

Because too much was added. I do not propose to keep all resets here.

> 
> But I agree with you, this code looks too similar. If we expect more
> similar lpasscc drivers, which provide no clocks, just several resets,
> maybe we can create a common generic wrapper and make resets lists
> corresponding driver data?

This would also work.

Best regards,
Krzysztof
Konrad Dybcio Aug. 28, 2023, 11:45 a.m. UTC | #3
On 26.08.2023 16:29, Krzysztof Kozlowski wrote:
> On 26/08/2023 16:09, Dmitry Baryshkov wrote:
> 
>>>> +MODULE_DEVICE_TABLE(of, lpasscc_sm6115_match_table);
>>>
>>> Everything here is almost the same as sc8280xp one, so this should be
>>> added to sc8280xp. You cut some boilerplate and additional driver.
>>
>> We have been there. It quickly becomes a nightmare to maintain.
>> Consider dispcc-sm8250.c
> 
> Because too much was added. I do not propose to keep all resets here.
> 
>>
>> But I agree with you, this code looks too similar. If we expect more
>> similar lpasscc drivers, which provide no clocks, just several resets,
>> maybe we can create a common generic wrapper and make resets lists
>> corresponding driver data?
> 
> This would also work.
Sounds like a good idea until somebody at qualcomm decides to add
support for bypassing adsp that only works on chromebooks that may get
cancelled or super secret internal devboards and the driver will gain
support for clocks..

But I guess that person will have to worry about squaring this out.

Konrad