diff mbox series

[v2,2/2] pinctrl: qcom: sc7280: Add lpi pinctrl variant data for adsp based targets

Message ID 1654079415-26217-3-git-send-email-quic_srivasam@quicinc.com
State New
Headers show
Series Add pinctrl support adsp based platforms | expand

Commit Message

Srinivasa Rao Mandadapu June 1, 2022, 10:30 a.m. UTC
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 <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Linus Walleij June 3, 2022, 10:28 a.m. UTC | #1
On Wed, Jun 1, 2022 at 12:30 PM Srinivasa Rao Mandadapu
<quic_srivasam@quicinc.com> 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 <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---
>  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
Srinivasa Rao Mandadapu June 3, 2022, 11:03 a.m. UTC | #2
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
> <quic_srivasam@quicinc.com> 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 <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> ---
>>   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
diff mbox series

Patch

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);