| Message ID | 20250219192102.423850-3-daniel.baluta@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | imx8mp: Add support to Run/Stall DSP via reset API | expand |
On Wed, 19 Feb 2025 21:20:56 +0200, Daniel Baluta wrote: > On i.MX8MP we introduced support for using a reset controller > to control DSP operation. > > This patch adds reset property which is required for i.MX8MP. > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > --- > .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/dsp/fsl,dsp.yaml:183:11: [error] string value is redundantly quoted with any quotes (quoted-strings) dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/dsp/fsl,dsp.example.dts:85.37-38 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/dsp/fsl,dsp.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1511: dt_binding_check] Error 2 make: *** [Makefile:251: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250219192102.423850-3-daniel.baluta@nxp.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Wed, Feb 19, 2025 at 09:20:56PM +0200, Daniel Baluta wrote: > On i.MX8MP we introduced support for using a reset controller > to control DSP operation. > > This patch adds reset property which is required for i.MX8MP. Avoid words "this patch" according to kernel submit patch document. Just said: Add reset property ... > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > --- > .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > index ab93ffd3d2e5..923e7f079f1b 100644 > --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > @@ -82,6 +82,13 @@ properties: > description: > Phandle to syscon block which provide access for processor enablement > > + resets: > + description: > + A pair consisting of phandle to audio-blk-control and an index referencing > + the DSP Run/Stall bit in audiomix registers. > + See include/dt-bindings/reset/imx8mp-reset-audiomix.h for each index meaning. Generally, needn't description for such common property. Or just said for example, i.MX8MP, ... , because it may change to difference reset providor in future. Anyway, I think it is fine leave here. Reviewed-by: Frank Li <Frank.Li@nxp.com> > + maxItems: 1 > + > required: > - compatible > - reg > @@ -164,6 +171,16 @@ allOf: > - const: txdb1 > - const: rxdb0 > - const: rxdb1 > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx8mp-dsp > + - fsl,imx8mp-hifi4 > + then: > + required: > + - "resets" > > additionalProperties: false > > @@ -220,5 +237,5 @@ examples: > <&mu2 3 0>; > memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>, > <&dsp_vdev0vring1>, <&dsp_reserved>; > - fsl,dsp-ctrl = <&audio_blk_ctrl>; > + resets = <&audio_blk_ctrl IMX8MP_AUDIOMIX_DSP>; > }; > -- > 2.25.1 >
Hi, Am Mittwoch, 19. Februar 2025, 20:20:56 CET schrieb Daniel Baluta: > On i.MX8MP we introduced support for using a reset controller > to control DSP operation. > > This patch adds reset property which is required for i.MX8MP. > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > --- > .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > index ab93ffd3d2e5..923e7f079f1b 100644 > --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > @@ -82,6 +82,13 @@ properties: > description: > Phandle to syscon block which provide access for processor enablement > > + resets: > + description: > + A pair consisting of phandle to audio-blk-control and an index referencing > + the DSP Run/Stall bit in audiomix registers. > + See include/dt-bindings/reset/imx8mp-reset-audiomix.h for each index meaning. > + maxItems: 1 > + > required: > - compatible > - reg > @@ -164,6 +171,16 @@ allOf: > - const: txdb1 > - const: rxdb0 > - const: rxdb1 > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx8mp-dsp > + - fsl,imx8mp-hifi4 > + then: > + required: > + - "resets" > > additionalProperties: false > > @@ -220,5 +237,5 @@ examples: > <&mu2 3 0>; > memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>, > <&dsp_vdev0vring1>, <&dsp_reserved>; > - fsl,dsp-ctrl = <&audio_blk_ctrl>; > + resets = <&audio_blk_ctrl IMX8MP_AUDIOMIX_DSP>; Am I missing something here? fsl,dsp-ctrl is used to get the regmap from syscon. Best regards, Alexander > }; >
On Thu, Feb 20, 2025 at 12:37 PM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi, > > Am Mittwoch, 19. Februar 2025, 20:20:56 CET schrieb Daniel Baluta: > > On i.MX8MP we introduced support for using a reset controller > > to control DSP operation. > > > > This patch adds reset property which is required for i.MX8MP. > > > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > > --- > > .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- > > 1 file changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > index ab93ffd3d2e5..923e7f079f1b 100644 > > --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > @@ -82,6 +82,13 @@ properties: > > description: > > Phandle to syscon block which provide access for processor enablement > > > > + resets: > > + description: > > + A pair consisting of phandle to audio-blk-control and an index referencing > > + the DSP Run/Stall bit in audiomix registers. > > + See include/dt-bindings/reset/imx8mp-reset-audiomix.h for each index meaning. > > + maxItems: 1 > > + > > required: > > - compatible > > - reg > > @@ -164,6 +171,16 @@ allOf: > > - const: txdb1 > > - const: rxdb0 > > - const: rxdb1 > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx8mp-dsp > > + - fsl,imx8mp-hifi4 > > + then: > > + required: > > + - "resets" > > > > additionalProperties: false > > > > @@ -220,5 +237,5 @@ examples: > > <&mu2 3 0>; > > memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>, > > <&dsp_vdev0vring1>, <&dsp_reserved>; > > - fsl,dsp-ctrl = <&audio_blk_ctrl>; > > + resets = <&audio_blk_ctrl IMX8MP_AUDIOMIX_DSP>; > > Am I missing something here? fsl,dsp-ctrl is used to get the regmap from syscon. fsl,dsp-ctrl was used to control the DSP. This functionality for imx8mp have been now replaced by using the reset controller. The example where the diff happens is for imx8mp so for this reason I changed it to use the new way of doing the rest.
Hi, Am Donnerstag, 20. Februar 2025, 14:11:30 CET schrieb Daniel Baluta: > On Thu, Feb 20, 2025 at 12:37 PM Alexander Stein > <alexander.stein@ew.tq-group.com> wrote: > > > > Hi, > > > > Am Mittwoch, 19. Februar 2025, 20:20:56 CET schrieb Daniel Baluta: > > > On i.MX8MP we introduced support for using a reset controller > > > to control DSP operation. > > > > > > This patch adds reset property which is required for i.MX8MP. > > > > > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > > > --- > > > .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- > > > 1 file changed, 18 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > > index ab93ffd3d2e5..923e7f079f1b 100644 > > > --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > > +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > > @@ -82,6 +82,13 @@ properties: > > > description: > > > Phandle to syscon block which provide access for processor enablement > > > > > > + resets: > > > + description: > > > + A pair consisting of phandle to audio-blk-control and an index referencing > > > + the DSP Run/Stall bit in audiomix registers. > > > + See include/dt-bindings/reset/imx8mp-reset-audiomix.h for each index meaning. > > > + maxItems: 1 > > > + > > > required: > > > - compatible > > > - reg > > > @@ -164,6 +171,16 @@ allOf: > > > - const: txdb1 > > > - const: rxdb0 > > > - const: rxdb1 > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + enum: > > > + - fsl,imx8mp-dsp > > > + - fsl,imx8mp-hifi4 > > > + then: > > > + required: > > > + - "resets" > > > > > > additionalProperties: false > > > > > > @@ -220,5 +237,5 @@ examples: > > > <&mu2 3 0>; > > > memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>, > > > <&dsp_vdev0vring1>, <&dsp_reserved>; > > > - fsl,dsp-ctrl = <&audio_blk_ctrl>; > > > + resets = <&audio_blk_ctrl IMX8MP_AUDIOMIX_DSP>; > > > > Am I missing something here? fsl,dsp-ctrl is used to get the regmap from syscon. > > fsl,dsp-ctrl was used to control the DSP. This functionality for > imx8mp have been now replaced by > using the reset controller. > > The example where the diff happens is for imx8mp so for this reason I > changed it to use the > new way of doing the rest. Okay, maybe add another example for 8ulp then. Best regards, Alexander
On Mi, 2025-02-19 at 21:20 +0200, Daniel Baluta wrote: > On i.MX8MP we introduced support for using a reset controller > to control DSP operation. > > This patch adds reset property which is required for i.MX8MP. > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > --- > .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > index ab93ffd3d2e5..923e7f079f1b 100644 > --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > @@ -82,6 +82,13 @@ properties: > description: > Phandle to syscon block which provide access for processor enablement > > + resets: > + description: > + A pair consisting of phandle to audio-blk-control and an index referencing > + the DSP Run/Stall bit in audiomix registers. > + See include/dt-bindings/reset/imx8mp-reset-audiomix.h for each index meaning. > + maxItems: 1 This is going to be confusing when there is an actual (undocumented?) DSP core reset that is not described in the device tree bindings, see patch 8. To me this looks like a bit of a gray zone, as I don't know how the hardware actually works, but if you wouldn't call the Run/Stall bit a reset, it probably shouldn't be described as such in the device tree bindings. I'm not sure. Should both core and runstall reset be described in the device tree? Or only the core reset, or neither? Either way we should try not to lie about the hardware here. regards Philipp
On Thu, Feb 20, 2025 at 04:45:42PM +0100, Philipp Zabel wrote: > On Mi, 2025-02-19 at 21:20 +0200, Daniel Baluta wrote: > > On i.MX8MP we introduced support for using a reset controller > > to control DSP operation. > > > > This patch adds reset property which is required for i.MX8MP. > > > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > > --- > > .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- > > 1 file changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > index ab93ffd3d2e5..923e7f079f1b 100644 > > --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > > @@ -82,6 +82,13 @@ properties: > > description: > > Phandle to syscon block which provide access for processor enablement > > > > + resets: > > + description: > > + A pair consisting of phandle to audio-blk-control and an index referencing > > + the DSP Run/Stall bit in audiomix registers. > > + See include/dt-bindings/reset/imx8mp-reset-audiomix.h for each index meaning. > > + maxItems: 1 > > This is going to be confusing when there is an actual (undocumented?) > DSP core reset that is not described in the device tree bindings, see > patch 8. > > To me this looks like a bit of a gray zone, as I don't know how the > hardware actually works, but if you wouldn't call the Run/Stall bit a > reset, it probably shouldn't be described as such in the device tree > bindings. According to some hardware common sense for cpu core. Generally release RESET Pin to let core runnings. Difference system use difference signal name. Spec/RM generally copy from hardware sign name. The functionaltiy is work as core reset. Release 'reset' let core go. The module is abstract layer for the function. > > I'm not sure. Should both core and runstall reset be described in the > device tree? Or only the core reset, or neither? Either way we should > try not to lie about the hardware here. Not lie about hardware. Try match hardware behavior to existed abstract module. Hardware is back box, which we often only observe it from outside. But I can assume hardware implement like swtich(state) { ... state = new state; ... case RUNSTALL: state = state; } Of course, hardware may use simple gate a input clock to run/stall a core. Frank > > regards > Philipp
diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml index ab93ffd3d2e5..923e7f079f1b 100644 --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml @@ -82,6 +82,13 @@ properties: description: Phandle to syscon block which provide access for processor enablement + resets: + description: + A pair consisting of phandle to audio-blk-control and an index referencing + the DSP Run/Stall bit in audiomix registers. + See include/dt-bindings/reset/imx8mp-reset-audiomix.h for each index meaning. + maxItems: 1 + required: - compatible - reg @@ -164,6 +171,16 @@ allOf: - const: txdb1 - const: rxdb0 - const: rxdb1 + - if: + properties: + compatible: + contains: + enum: + - fsl,imx8mp-dsp + - fsl,imx8mp-hifi4 + then: + required: + - "resets" additionalProperties: false @@ -220,5 +237,5 @@ examples: <&mu2 3 0>; memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>, <&dsp_vdev0vring1>, <&dsp_reserved>; - fsl,dsp-ctrl = <&audio_blk_ctrl>; + resets = <&audio_blk_ctrl IMX8MP_AUDIOMIX_DSP>; };
On i.MX8MP we introduced support for using a reset controller to control DSP operation. This patch adds reset property which is required for i.MX8MP. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> --- .../devicetree/bindings/dsp/fsl,dsp.yaml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)