diff mbox series

[1/1] dt-bindings: media: imx-jpeg: add clocks,clock-names,slot to fix warning

Message ID 20240404035205.59492-1-Frank.Li@nxp.com (mailing list archive)
State Not Applicable
Headers show
Series [1/1] dt-bindings: media: imx-jpeg: add clocks,clock-names,slot to fix warning | expand

Commit Message

Frank Li April 4, 2024, 3:52 a.m. UTC
Fix below DTB_CHECK warning.

make CHECK_DTBS=y freescale/imx8qxp-mek.dtb
  DTC_CHK arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb
arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#

Add 'clocks' and 'clock-names' property.
Add 'slot' to choose which physical jpeg slot.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Pass dtb_binding check
    
    make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8  dt_binding_check DT_SCHEMA_FILES=nxp,imx8-jpeg.yaml
      LINT    Documentation/devicetree/bindings
      DTEX    Documentation/devicetree/bindings/media/nxp,imx8-jpeg.example.dts
      CHKDT   Documentation/devicetree/bindings/processed-schema.json
      SCHEMA  Documentation/devicetree/bindings/processed-schema.json
      DTC_CHK Documentation/devicetree/bindings/media/nxp,imx8-jpeg.example.dtb

 .../devicetree/bindings/media/nxp,imx8-jpeg.yaml  | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Krzysztof Kozlowski April 4, 2024, 6:26 a.m. UTC | #1
On 04/04/2024 05:52, Frank Li wrote:
> Fix below DTB_CHECK warning.
> 
> make CHECK_DTBS=y freescale/imx8qxp-mek.dtb
>   DTC_CHK arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
> 	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#

No, that's not the reason to add properties. Add them if they are valid.



>  
> +  slot:
> +    description: Certain slot number is used.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3

NAK. Every time.

Fix your DTS instead.

Please read the feedback instead of pushing this stuff for the third time!

https://lore.kernel.org/all/bbb1875b-7980-46aa-80b4-dbaf2a2d5755@linaro.org/

Can NXP take responsibility for this piece of code?

Best regards,
Krzysztof
Krzysztof Kozlowski April 4, 2024, 6:54 a.m. UTC | #2
On 04/04/2024 08:26, Krzysztof Kozlowski wrote:
> On 04/04/2024 05:52, Frank Li wrote:
>> Fix below DTB_CHECK warning.
>>
>> make CHECK_DTBS=y freescale/imx8qxp-mek.dtb
>>   DTC_CHK arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb
>> arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
>> 	from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
> 
> No, that's not the reason to add properties. Add them if they are valid.

And for the clocks, instead pick up this patch:
https://lore.kernel.org/all/20230721111020.1234278-3-alexander.stein@ew.tq-group.com/

Please check for same work on lore before working on old issues. dfn in
lore is your friend.

Best regards,
Krzysztof
Fabio Estevam April 4, 2024, 11:03 a.m. UTC | #3
On Thu, Apr 4, 2024 at 3:54 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:

> And for the clocks, instead pick up this patch:
> https://lore.kernel.org/all/20230721111020.1234278-3-alexander.stein@ew.tq-group.com/

Or maybe this one:
https://lore.kernel.org/linux-devicetree/DB9PR04MB923493D0DA82C9EC4386BC2A8FF1A@DB9PR04MB9234.eurprd04.prod.outlook.com/
Krzysztof Kozlowski April 4, 2024, 11:59 a.m. UTC | #4
On 04/04/2024 13:03, Fabio Estevam wrote:
> On Thu, Apr 4, 2024 at 3:54 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> 
>> And for the clocks, instead pick up this patch:
>> https://lore.kernel.org/all/20230721111020.1234278-3-alexander.stein@ew.tq-group.com/
> 
> Or maybe this one:
> https://lore.kernel.org/linux-devicetree/DB9PR04MB923493D0DA82C9EC4386BC2A8FF1A@DB9PR04MB9234.eurprd04.prod.outlook.com/


Three people were fixing same clocks issue... and three or more people
were trying to fix the slot property.

This is really bad binding maintenance and driver upstreaming, NXP.

Best regards,
Krzysztof
Frank Li April 4, 2024, 1:54 p.m. UTC | #5
On Thu, Apr 04, 2024 at 01:59:57PM +0200, Krzysztof Kozlowski wrote:
> On 04/04/2024 13:03, Fabio Estevam wrote:
> > On Thu, Apr 4, 2024 at 3:54 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > 
> >> And for the clocks, instead pick up this patch:
> >> https://lore.kernel.org/all/20230721111020.1234278-3-alexander.stein@ew.tq-group.com/
> > 
> > Or maybe this one:
> > https://lore.kernel.org/linux-devicetree/DB9PR04MB923493D0DA82C9EC4386BC2A8FF1A@DB9PR04MB9234.eurprd04.prod.outlook.com/
> 
> 
> Three people were fixing same clocks issue... and three or more people
> were trying to fix the slot property.
> 
> This is really bad binding maintenance and driver upstreaming, NXP.

Thanks everyone help make imx dts and binding better. I should google
before send. 

Patchwork for imx already was created.
https://patchwork.kernel.org/project/imx/list/?series=&submitter=150701&state=&q=&archive=&delegate=

I hope to patchwork help reduce duplicate work.

Frank

> 
> Best regards,
> Krzysztof
>
Mirela Rabulea April 8, 2024, 3:15 a.m. UTC | #6
Hi Krzysztof,

On 04.04.2024 09:26, Krzysztof Kozlowski wrote:
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
>
>
> On 04/04/2024 05:52, Frank Li wrote:
>> Fix below DTB_CHECK warning.
>>
>> make CHECK_DTBS=y freescale/imx8qxp-mek.dtb
>>    DTC_CHK arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb
>> arch/arm64/boot/dts/freescale/imx8qxp-mek.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks', 'slot' do not match any of the regexes: 'pinctrl-[0-9]+'
>>        from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
> No, that's not the reason to add properties. Add them if they are valid.
>
>
>
>> +  slot:
>> +    description: Certain slot number is used.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 3
> NAK. Every time.
>
> Fix your DTS instead.
>
> Please read the feedback instead of pushing this stuff for the third time!
>
> https://lore.kernel.org/all/bbb1875b-7980-46aa-80b4-dbaf2a2d5755@linaro.org/
>
> Can NXP take responsibility for this piece of code?

Thanks for feedback.

For the clocks issue, I looked at the patches sent previously by 
Alexander Stein and Fabio Estevam, and the current one:

https://lore.kernel.org/linux-devicetree/?q=dfblob%3A3d9d1db3704+dfblob%3A7899e17aff3

As I also said in the past, I think Fabio's patch was more complete, so 
I took his _v3, I tried to incorporate all the feedback given, and I 
sent a subsequent _v4, here (bindings & dtb):

https://lore.kernel.org/linux-devicetree/20240408030734.1191069-1-mirela.rabulea@nxp.com/


For the slots issue, I will consult with Ming and get back.

Thanks for your patience, and sorry for the inconvenience.


Regards,

Mirela

>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
index 3d9d1db37040d..32820ec42de9d 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
@@ -31,6 +31,15 @@  properties:
   reg:
     maxItems: 1
 
+  clocks:
+    minItems: 2
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: per
+      - const: ipg
+
   interrupts:
     description: |
       There are 4 slots available in the IP, which the driver may use
@@ -46,6 +55,12 @@  properties:
     minItems: 2               # Wrapper and 1 slot
     maxItems: 5               # Wrapper and 4 slots
 
+  slot:
+    description: Certain slot number is used.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 3
+
 required:
   - compatible
   - reg