Add compatible string and lpi pinctrl variant data structure for adsp
enabled sc7280 platforms.
This variant data structure rnd compatible name required for
distingushing ADSP based platforms and ADSP bypass platforms.
In case of ADSP enabled platforms, where audio is routed through ADSP
macro and decodec GDSC Switches are triggered as clocks by pinctrl
driver and ADSP firmware controls them. So It's mandatory to enable
them in ADSP based solutions.
In case of ADSP bypass platforms clock voting is optional as these macro
and dcodec GDSC switches are maintained as power domains and operated from
lpass clock drivers.
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Co-developed-by: Venkata Prasad Potturu <[email protected]>
Signed-off-by: Venkata Prasad Potturu <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
index 2add9a4..c9e85d9 100644
--- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
@@ -134,6 +134,16 @@ static const struct lpi_function sc7280_functions[] = {
LPI_FUNCTION(wsa_swr_data),
};
+static const struct lpi_pinctrl_variant_data sc7280_adsp_lpi_data = {
+ .pins = sc7280_lpi_pins,
+ .npins = ARRAY_SIZE(sc7280_lpi_pins),
+ .groups = sc7280_groups,
+ .ngroups = ARRAY_SIZE(sc7280_groups),
+ .functions = sc7280_functions,
+ .nfunctions = ARRAY_SIZE(sc7280_functions),
+ .is_clk_optional = false,
+};
+
static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
.pins = sc7280_lpi_pins,
.npins = ARRAY_SIZE(sc7280_lpi_pins),
@@ -149,6 +159,10 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
.compatible = "qcom,sc7280-lpass-lpi-pinctrl",
.data = &sc7280_lpi_data,
},
+ {
+ .compatible = "qcom,sc7280-lpass-adsp-lpi-pinctrl",
+ .data = &sc7280_adsp_lpi_data,
+ },
{ }
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
--
2.7.4
On 6/3/2022 3:58 PM, Linus Walleij wrote:
Thanks for Your time and valuable inputs Linus!!!
> On Wed, Jun 1, 2022 at 12:30 PM Srinivasa Rao Mandadapu
> <[email protected]> wrote:
>
> So one way to just use a propert and avoid more compatible strings:
>
>> Add compatible string and lpi pinctrl variant data structure for adsp
>> enabled sc7280 platforms.
>> This variant data structure rnd compatible name required for
>> distingushing ADSP based platforms and ADSP bypass platforms.
>> In case of ADSP enabled platforms, where audio is routed through ADSP
>> macro and decodec GDSC Switches are triggered as clocks by pinctrl
>> driver and ADSP firmware controls them. So It's mandatory to enable
>> them in ADSP based solutions.
>> In case of ADSP bypass platforms clock voting is optional as these macro
>> and dcodec GDSC switches are maintained as power domains and operated from
>> lpass clock drivers.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> Co-developed-by: Venkata Prasad Potturu <[email protected]>
>> Signed-off-by: Venkata Prasad Potturu <[email protected]>
>> ---
>> drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> index 2add9a4..c9e85d9 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> @@ -134,6 +134,16 @@ static const struct lpi_function sc7280_functions[] = {
>> LPI_FUNCTION(wsa_swr_data),
>> };
>>
>> +static const struct lpi_pinctrl_variant_data sc7280_adsp_lpi_data = {
> Remove static and export this struct in drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
We can remove entire data structure if my below approach is okay.
>
>> + .pins = sc7280_lpi_pins,
>> + .npins = ARRAY_SIZE(sc7280_lpi_pins),
>> + .groups = sc7280_groups,
>> + .ngroups = ARRAY_SIZE(sc7280_groups),
>> + .functions = sc7280_functions,
>> + .nfunctions = ARRAY_SIZE(sc7280_functions),
>> + .is_clk_optional = false,
>> +};
>>
>>
>> static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
>> .pins = sc7280_lpi_pins,
>> .npins = ARRAY_SIZE(sc7280_lpi_pins),
>> @@ -149,6 +159,10 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
>> .compatible = "qcom,sc7280-lpass-lpi-pinctrl",
>> .data = &sc7280_lpi_data,
>> },
>> + {
>> + .compatible = "qcom,sc7280-lpass-adsp-lpi-pinctrl",
>> + .data = &sc7280_adsp_lpi_data,
>> + },
> Drop this and instead add some code in the probe()
> in drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> lines:
>
> if (of_device_is_compatible(np, "qcom,sc7280-lpass-lpi-pinctrl") &&
> of_property_read_bool(np, "qcom,adsp-mode))
> data = &sc7280_adsp_lpi_data;
Here, only diff between ADSP and ADSP bypass variant dats is
"is_clk_optional" field.
So we can keep something like this. Kindly suggest, if it's not making
sense.
if (of_device_is_compatible(np, "qcom,sc7280-lpass-lpi-pinctrl") &&
of_property_read_bool(np, "qcom,adsp-mode))
data->is_clk_optional = false;
>
> Yours,
> Linus Walleij
On Wed, Jun 1, 2022 at 12:30 PM Srinivasa Rao Mandadapu
<[email protected]> wrote:
So one way to just use a propert and avoid more compatible strings:
> Add compatible string and lpi pinctrl variant data structure for adsp
> enabled sc7280 platforms.
> This variant data structure rnd compatible name required for
> distingushing ADSP based platforms and ADSP bypass platforms.
> In case of ADSP enabled platforms, where audio is routed through ADSP
> macro and decodec GDSC Switches are triggered as clocks by pinctrl
> driver and ADSP firmware controls them. So It's mandatory to enable
> them in ADSP based solutions.
> In case of ADSP bypass platforms clock voting is optional as these macro
> and dcodec GDSC switches are maintained as power domains and operated from
> lpass clock drivers.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Co-developed-by: Venkata Prasad Potturu <[email protected]>
> Signed-off-by: Venkata Prasad Potturu <[email protected]>
> ---
> drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> index 2add9a4..c9e85d9 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> @@ -134,6 +134,16 @@ static const struct lpi_function sc7280_functions[] = {
> LPI_FUNCTION(wsa_swr_data),
> };
>
> +static const struct lpi_pinctrl_variant_data sc7280_adsp_lpi_data = {
Remove static and export this struct in drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> + .pins = sc7280_lpi_pins,
> + .npins = ARRAY_SIZE(sc7280_lpi_pins),
> + .groups = sc7280_groups,
> + .ngroups = ARRAY_SIZE(sc7280_groups),
> + .functions = sc7280_functions,
> + .nfunctions = ARRAY_SIZE(sc7280_functions),
> + .is_clk_optional = false,
> +};
> static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
> .pins = sc7280_lpi_pins,
> .npins = ARRAY_SIZE(sc7280_lpi_pins),
> @@ -149,6 +159,10 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
> .compatible = "qcom,sc7280-lpass-lpi-pinctrl",
> .data = &sc7280_lpi_data,
> },
> + {
> + .compatible = "qcom,sc7280-lpass-adsp-lpi-pinctrl",
> + .data = &sc7280_adsp_lpi_data,
> + },
Drop this and instead add some code in the probe()
in drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
lines:
if (of_device_is_compatible(np, "qcom,sc7280-lpass-lpi-pinctrl") &&
of_property_read_bool(np, "qcom,adsp-mode))
data = &sc7280_adsp_lpi_data;
Yours,
Linus Walleij
On Fri, Jun 3, 2022 at 1:03 PM Srinivasa Rao Mandadapu
<[email protected]> wrote:
> >> @@ -149,6 +159,10 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
> >> .compatible = "qcom,sc7280-lpass-lpi-pinctrl",
> >> .data = &sc7280_lpi_data,
> >> },
> >> + {
> >> + .compatible = "qcom,sc7280-lpass-adsp-lpi-pinctrl",
> >> + .data = &sc7280_adsp_lpi_data,
> >> + },
> > Drop this and instead add some code in the probe()
> > in drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> > lines:
> >
> > if (of_device_is_compatible(np, "qcom,sc7280-lpass-lpi-pinctrl") &&
> > of_property_read_bool(np, "qcom,adsp-mode))
> > data = &sc7280_adsp_lpi_data;
>
> Here, only diff between ADSP and ADSP bypass variant dats is
> "is_clk_optional" field.
>
> So we can keep something like this. Kindly suggest, if it's not making
> sense.
>
> if (of_device_is_compatible(np, "qcom,sc7280-lpass-lpi-pinctrl") &&
> of_property_read_bool(np, "qcom,adsp-mode))
> data->is_clk_optional = false;
Looks good to me!
Yours,
Linus Walleij