On Thu, Jul 29, 2021 at 10:18:28AM +0100, Srinivas Kandagatla wrote: > On 28/07/2021 18:36, Rob Herring wrote: > > This all looks fairly similar to the prior Qcom audio binding(s). It > > would be nice to not see this all re-invented. > AudioReach is a new DSP signal processing framework Which is different to > its previous DSP firmware(aka Elite). > It makes use of ASoC Topology to load audio graphs on to the DSP which is > then managed by APM (Audio Processing Manager) service. > So internals are not exactly same. > From device tree side we might end up with similar layout, but there are > some subtle differences like clocks are managed by q6prm service instead of > q6afe service in old firmware, front-end pcm dais definitions come from ASoC > topology. The software we're running on the hardware shouldn't impact how the hardware is described, it should be posible to switch DSP frameworks on the same hardware - look at what Intel have done with SoF. > > Are you suggesting that we should reuse the old bindings (q6afe, q6asm) by > add new compatible strings along with differences ? > > > > > > > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> > > > --- > > > .../devicetree/bindings/sound/qcom,q6apm.yaml | 87 +++++++++++++++++++ > > > include/dt-bindings/sound/qcom,q6apm.h | 8 ++ > > > 2 files changed, 95 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/sound/qcom,q6apm.yaml > > > create mode 100644 include/dt-bindings/sound/qcom,q6apm.h > > > > > > diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm.yaml > > > new file mode 100644 > > > index 000000000000..6f27567523a9 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm.yaml > > > @@ -0,0 +1,87 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/sound/qcom,q6apm.yaml#"; > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > > > + > > > +title: Qualcomm Audio Process Manager binding > > > + > > > +maintainers: > > > + - Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> > > > + > > > +description: | > > > + This binding describes the Qualcomm Audio Process Manager service in DSP > > > + > > > +properties: > > > + compatible: > > > + const: qcom,q6apm > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + '#address-cells': > > > + const: 1 > > > + > > > + '#size-cells': > > > + const: 0 > > > + > > > +#APM Services > > > +patternProperties: > > > + 'apm@[0-9]+$': > > > > This means '.*apm' for the node name. Did you need a '^'? > > > yes we need begins with '^' , will add that in next version. > > > > + type: object > > > + description: > > > + APM devices use subnodes for services. > > > + > > > + properties: > > > + compatible: > > > + enum: > > > + - qcom,q6apm-dais > > > + - qcom,q6apm-bedais > > > + > > > + iommus: > > > + maxItems: 1 > > > + > > > + "#sound-dai-cells": > > > + const: 1 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + required: > > > + - compatible > > > + - reg > > > + - '#sound-dai-cells' > > > + > > > + additionalProperties: false > > > + > > > +required: > > > + - compatible > > > + - reg > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + gpr { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + gprservice@1 { > > > + compatible = "qcom,q6apm"; > > > + reg = <1>; > > > + > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + apm@1 { > > > + compatible = "qcom,q6apm-dais"; > > > + #sound-dai-cells = <1>; > > > + reg = <1>; > > > + }; > > > + > > > + apm@2 { > > > + compatible = "qcom,q6apm-bedais"; > > > + #sound-dai-cells = <1>; > > > + reg = <2>; > > > + }; > > > + }; > > > + }; > > > diff --git a/include/dt-bindings/sound/qcom,q6apm.h b/include/dt-bindings/sound/qcom,q6apm.h > > > new file mode 100644 > > > index 000000000000..3c3987eb6e95 > > > --- /dev/null > > > +++ b/include/dt-bindings/sound/qcom,q6apm.h > > > @@ -0,0 +1,8 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +#ifndef __DT_BINDINGS_Q6_APM_H__ > > > +#define __DT_BINDINGS_Q6_APM_H__ > > > + > > > +/* Audio Process Manager (APM) virtual ports IDs */ > > > +#include <dt-bindings/sound/qcom,q6afe.h> > > > > Why add this indirection? Rename the file if you need something to cover > > both. > > Thats a good idea, > > These are basically audio endpoint device ids which should be same across > different audio firmwares. > > I can rename this to dt-bindings/sound/qcom,adsp-audio-ports.h or something > more generic to be able to reuse. > > --srini > > > > > + > > > +#endif /* __DT_BINDINGS_Q6_APM_H__ */ > > > -- > > > 2.21.0 > > > > > >
Attachment:
signature.asc
Description: PGP signature
