mbox series

[v2,0/2] Add pinctrl support adsp based platforms

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

Message

Srinivasa Rao Mandadapu June 1, 2022, 10:30 a.m. UTC
This patch set is to add pinctrl support adsp enabled sc7280 platforms.

Changes Since V1:
    -- Update commit message.
 
Srinivasa Rao Mandadapu (2):
  dt-bindings: pinctrl: qcom: sc7280: Add compatible string for adsp
    based platforms
  pinctrl: qcom: sc7280: Add lpi pinctrl variant data for adsp based
    targets

 .../bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml    |  4 +++-
 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c            | 14 ++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Stephen Boyd June 2, 2022, 1:13 a.m. UTC | #1
Quoting Srinivasa Rao Mandadapu (2022-06-01 03:30:14)
> Add compatible string to support adsp enabled sc7280 platforms.
>
> 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>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  .../devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml    | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
> index d32ee32..53c2c59 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
> @@ -17,7 +17,9 @@ description: |
>
>  properties:
>    compatible:
> -    const: qcom,sc7280-lpass-lpi-pinctrl
> +    enum:
> +      - qcom,sc7280-lpass-lpi-pinctrl
> +      - qcom,sc7280-lpass-adsp-lpi-pinctrl

Can you confirm that this is the same hardware (i.e. same reg property)
but just a different compatible string used to convey that the device is
using "adsp" mode or not? If so, this looks to be a common pattern for
the audio hardware here, where we have two "views" of the hardware, one
for adsp mode and one for not adsp mode. I guess the not adsp mode is
called "adsp bypass"?

Is that right? Why are we conveying this information via the compatible
string?
Srinivasa Rao Mandadapu June 3, 2022, 6:09 a.m. UTC | #2
On 6/2/2022 6:43 AM, Stephen Boyd wrote:
> Quoting Srinivasa Rao Mandadapu (2022-06-01 03:30:14)
>> Add compatible string to support adsp enabled sc7280 platforms.
>>
>> 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>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>   .../devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml    | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
>> index d32ee32..53c2c59 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
>> @@ -17,7 +17,9 @@ description: |
>>
>>   properties:
>>     compatible:
>> -    const: qcom,sc7280-lpass-lpi-pinctrl
>> +    enum:
>> +      - qcom,sc7280-lpass-lpi-pinctrl
>> +      - qcom,sc7280-lpass-adsp-lpi-pinctrl
> Can you confirm that this is the same hardware (i.e. same reg property)
> but just a different compatible string used to convey that the device is
> using "adsp" mode or not? If so, this looks to be a common pattern for
> the audio hardware here, where we have two "views" of the hardware, one
> for adsp mode and one for not adsp mode. I guess the not adsp mode is
> called "adsp bypass"?

Yes Your understanding is correct. The same hardware in scenario not using ADSP,

and in another enabling DSP.
>
> Is that right? Why are we conveying this information via the compatible
> string?

Could you please suggest better way!.  As pin control driver is the 
first one to probe, I am not getting better approach.

While up-streaming these drivers, concluded to use this approach.
Linus Walleij June 3, 2022, 10:19 a.m. UTC | #3
On Fri, Jun 3, 2022 at 8:09 AM Srinivasa Rao Mandadapu
<quic_srivasam@quicinc.com> wrote:
> On 6/2/2022 6:43 AM, Stephen Boyd wrote:

> >> +    enum:
> >> +      - qcom,sc7280-lpass-lpi-pinctrl
> >> +      - qcom,sc7280-lpass-adsp-lpi-pinctrl
> > Can you confirm that this is the same hardware (i.e. same reg property)
> > but just a different compatible string used to convey that the device is
> > using "adsp" mode or not? If so, this looks to be a common pattern for
> > the audio hardware here, where we have two "views" of the hardware, one
> > for adsp mode and one for not adsp mode. I guess the not adsp mode is
> > called "adsp bypass"?
>
> Yes Your understanding is correct. The same hardware in scenario not using ADSP,
>
> and in another enabling DSP.
> >
> > Is that right? Why are we conveying this information via the compatible
> > string?
>
> Could you please suggest better way!.  As pin control driver is the
> first one to probe, I am not getting better approach.
>
> While up-streaming these drivers, concluded to use this approach.

The device tree conveys hardware description and some configuration.

If this is configuration thing, either you could perhaps determine it from the
hardware (if set up in hardware or boot loader) and if that is not possible
it should just be a boolean property of the device
node:

{
    compatible = "...";
    qcom.adsp-mode;
}

If you are probing two different drivers depending on the mode, then there is a
problem of course, but it is a Linux problem not a device tree problem.

Yours,
Linus Walleij