| Message ID | 20230816152210.4080779-2-devarsht@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add V4L2 M2M Driver for E5010 JPEG Encoder | expand |
On 16/08/2023 17:22, Devarsh Thakkar wrote: > Add dt-bindings for Imagination E5010 JPEG Encoder driver which is > implemented as stateful V4L2 M2M driver. > > Co-developed-by: David Huang <d-huang@ti.com> > Signed-off-by: David Huang <d-huang@ti.com> > Signed-off-by: Devarsh Thakkar <devarsht@ti.com> > --- > V2: No change > V3: > - Add vendor specific compatible > - Fix commit title and message > - Update reg names > - Update clocks to 1 > - Fix dts example with proper naming I do not see improvements in the subject. > > .../bindings/media/img,e5010-jpeg-enc.yaml | 81 +++++++++++++++++++ > MAINTAINERS | 5 ++ > 2 files changed, 86 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml > > diff --git a/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml > new file mode 100644 > index 000000000000..d105a71ee2ea > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml > @@ -0,0 +1,81 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/img,e5010-jpeg-enc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Imagination E5010 JPEG Encoder > + > +maintainers: > + - Devarsh Thakkar <devarsht@ti.com> > + > +description: | > + The E5010 is a JPEG encoder from Imagination Technologies implemented on > + TI's AM62A SoC. It is capable of real time encoding of YUV420 and YUV422 > + inputs to JPEG and M-JPEG. It supports baseline JPEG Encoding up to > + 8Kx8K resolution. > + > +properties: > + compatible: > + oneOf: > + - items: > + - const: ti,e5010-jpeg-enc TI did not make e5010. Use SoC-based compatible. > + - const: img,e5010-jpeg-enc > + - const: img,e5010-jpeg-enc img,e5010-jpeg-enc cannot be compatible with img,e5010-jpeg-enc. It does not make sense. I guess I did not expect you are going to use what you wrote in v1 directly... I thought it is just about syntax. > + > + reg: > + items: > + - description: The E5010 core register region > + - description: The E5010 mmu register region > + > + reg-names: > + items: > + - const: core > + - const: mmu > + > + power-domains: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: core_clk Drop _clk or even drop clock-names. It brings little benefit for one-entry list. Best regards, Krzysztof
Hi Krzysztof, Thanks for the review. On 19/08/23 19:30, Krzysztof Kozlowski wrote: > On 16/08/2023 17:22, Devarsh Thakkar wrote: >> Add dt-bindings for Imagination E5010 JPEG Encoder driver which is >> implemented as stateful V4L2 M2M driver. >> >> Co-developed-by: David Huang <d-huang@ti.com> >> Signed-off-by: David Huang <d-huang@ti.com> >> Signed-off-by: Devarsh Thakkar <devarsht@ti.com> >> --- >> V2: No change >> V3: >> - Add vendor specific compatible >> - Fix commit title and message >> - Update reg names >> - Update clocks to 1 >> - Fix dts example with proper naming > > I do not see improvements in the subject. > Sorry, Will correct in v4. >> >> .../bindings/media/img,e5010-jpeg-enc.yaml | 81 +++++++++++++++++++ >> MAINTAINERS | 5 ++ >> 2 files changed, 86 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml >> >> diff --git a/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml >> new file mode 100644 >> index 000000000000..d105a71ee2ea >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml >> @@ -0,0 +1,81 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/img,e5010-jpeg-enc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Imagination E5010 JPEG Encoder >> + >> +maintainers: >> + - Devarsh Thakkar <devarsht@ti.com> >> + >> +description: | >> + The E5010 is a JPEG encoder from Imagination Technologies implemented on >> + TI's AM62A SoC. It is capable of real time encoding of YUV420 and YUV422 >> + inputs to JPEG and M-JPEG. It supports baseline JPEG Encoding up to >> + 8Kx8K resolution. >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - const: ti,e5010-jpeg-enc > > TI did not make e5010. Use SoC-based compatible. > >> + - const: img,e5010-jpeg-enc >> + - const: img,e5010-jpeg-enc > > img,e5010-jpeg-enc cannot be compatible with img,e5010-jpeg-enc. It does > not make sense. I guess I did not expect you are going to use what you > wrote in v1 directly... I thought it is just about syntax. > Sorry but I did not understand this fully, the possible compatibles are: 1) "ti,am62a-jpeg-enc", "img,e5010-jpeg-enc" or 2) "img,e5010-jpeg-enc" anything else will not comply during dtbs_check as shown below : For e.g. If I use below compatible : "img,e5010-jpeg-enc", "img,e5010-jpeg-enc" and run dtbs_check, it throw below error : make CHECK_DTBS=y DT_SCHEMA_FILES=media/img,e5010-jpeg-enc.yaml ti/k3-am62a7-sk.dtb LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTC_CHK arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb /home/devarsht/ti/linux-next2/linux-next/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: jpeg-encoder@fd20000: compatible: 'oneOf' conditional failed, one must be fixed: ['img,e5010-jpeg-enc', 'img,e5010-jpeg-enc'] is too long 'ti,am62a-jpeg-enc' was expected From schema: /home/devarsht/ti/linux-next2/linux-next/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml Similarly, if I use below compatible : "ti,am62a-jpeg-enc", It throw below error : make CHECK_DTBS=y DT_SCHEMA_FILES=media/img,e5010-jpeg-enc.yaml ti/k3-am62a7-sk.dtb DTC_CHK arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb /home/devarsht/ti/linux-next2/linux-next/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: jpeg-encoder@fd20000: compatible: 'oneOf' conditional failed, one must be fixed: ['ti,am62a-jpeg-enc'] is too short 'img,e5010-jpeg-enc' was expected From schema: /home/devarsht/ti/linux-next2/linux-next/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml But If I use either 1) or 2) it does not throw any error. Please let me know if I missed to understand your point. >> + >> + reg: >> + items: >> + - description: The E5010 core register region >> + - description: The E5010 mmu register region >> + >> + reg-names: >> + items: >> + - const: core >> + - const: mmu >> + >> + power-domains: >> + maxItems: 1 >> + >> + resets: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-names: >> + items: >> + - const: core_clk > > Drop _clk or even drop clock-names. It brings little benefit for > one-entry list. > Agreed, will drop clock-names altogether. Regards Devarsh > > Best regards, > Krzysztof >
On 20/08/2023 18:46, Devarsh Thakkar wrote: >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - const: ti,e5010-jpeg-enc >> >> TI did not make e5010. Use SoC-based compatible. >> >>> + - const: img,e5010-jpeg-enc >>> + - const: img,e5010-jpeg-enc >> >> img,e5010-jpeg-enc cannot be compatible with img,e5010-jpeg-enc. It does >> not make sense. I guess I did not expect you are going to use what you >> wrote in v1 directly... I thought it is just about syntax. >> > > Sorry but I did not understand this fully, the possible compatibles are: > > 1) "ti,am62a-jpeg-enc", "img,e5010-jpeg-enc" > or > 2) "img,e5010-jpeg-enc" > > anything else will not comply during dtbs_check as shown below : Ah, you are right, ENOTENOUGHCOFFEE or some other issue on my side. > > For e.g. If I use below compatible : > "img,e5010-jpeg-enc", "img,e5010-jpeg-enc" > > and run dtbs_check, it throw below error : > > make CHECK_DTBS=y DT_SCHEMA_FILES=media/img,e5010-jpeg-enc.yaml > ti/k3-am62a7-sk.dtb > LINT Documentation/devicetree/bindings > CHKDT Documentation/devicetree/bindings/processed-schema.json > SCHEMA Documentation/devicetree/bindings/processed-schema.json > DTC_CHK arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb > /home/devarsht/ti/linux-next2/linux-next/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: > jpeg-encoder@fd20000: compatible: 'oneOf' conditional failed, one must > be fixed: > ['img,e5010-jpeg-enc', 'img,e5010-jpeg-enc'] is too long > 'ti,am62a-jpeg-enc' was expected > From schema: > /home/devarsht/ti/linux-next2/linux-next/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml > > > Similarly, if I use below compatible : > > "ti,am62a-jpeg-enc", > It throw below error : > > make CHECK_DTBS=y DT_SCHEMA_FILES=media/img,e5010-jpeg-enc.yaml > ti/k3-am62a7-sk.dtb > DTC_CHK arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb > /home/devarsht/ti/linux-next2/linux-next/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: > jpeg-encoder@fd20000: compatible: 'oneOf' conditional failed, one must > be fixed: > ['ti,am62a-jpeg-enc'] is too short > 'img,e5010-jpeg-enc' was expected > From schema: > /home/devarsht/ti/linux-next2/linux-next/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml > > > But If I use either 1) or 2) it does not throw any error. > Please let me know if I missed to understand your point. Yes, you are right, sorry for that. However it still should be "ti,am62a-jpeg-enc", not ti,e5010... Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml new file mode 100644 index 000000000000..d105a71ee2ea --- /dev/null +++ b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/img,e5010-jpeg-enc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Imagination E5010 JPEG Encoder + +maintainers: + - Devarsh Thakkar <devarsht@ti.com> + +description: | + The E5010 is a JPEG encoder from Imagination Technologies implemented on + TI's AM62A SoC. It is capable of real time encoding of YUV420 and YUV422 + inputs to JPEG and M-JPEG. It supports baseline JPEG Encoding up to + 8Kx8K resolution. + +properties: + compatible: + oneOf: + - items: + - const: ti,e5010-jpeg-enc + - const: img,e5010-jpeg-enc + - const: img,e5010-jpeg-enc + + reg: + items: + - description: The E5010 core register region + - description: The E5010 mmu register region + + reg-names: + items: + - const: core + - const: mmu + + power-domains: + maxItems: 1 + + resets: + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + items: + - const: core_clk + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - reg-names + - interrupts + - clocks + - clock-names + +additionalProperties: false + +examples: + - | + #include <dt-bindings/soc/ti,sci_pm_domain.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + jpeg-encoder@fd20000 { + compatible = "img,e5010-jpeg-enc"; + reg = <0x00 0xfd20000 0x00 0x100>, + <0x00 0xfd20200 0x00 0x200>; + reg-names = "core", "mmu"; + clocks = <&k3_clks 201 0>; + clock-names = "core_clk"; + power-domains = <&k3_pds 201 TI_SCI_PD_EXCLUSIVE>; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index d8700b009e22..9425ecf45df6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10206,6 +10206,11 @@ S: Maintained F: Documentation/devicetree/bindings/auxdisplay/img,ascii-lcd.yaml F: drivers/auxdisplay/img-ascii-lcd.c +IMGTEC JPEG ENCODER DRIVER +M: Devarsh Thakkar <devarsht@ti.com> +S: Supported +F: Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml + IMGTEC IR DECODER DRIVER S: Orphan F: drivers/media/rc/img-ir/