[v2,2/2] media: dt-bindings: media: remove nokia,n900-ir as pwm-ir-tx is compatible

Commit Message

Sean Young Aug. 22, 2023, 9:12 a.m. UTC

The generic pwm-ir-tx driver works for the Nokia n900, so nokia,n900-ir
can be removed.

Cc: Sicelo <[e-mail]>
Cc: Rob Herring <[e-mail]>
Cc: Krzysztof Kozlowski <[e-mail]>
Cc: Conor Dooley <[e-mail]>
Cc: [e-mail]
Cc: Ivaylo Dimitrov <[e-mail]>
Cc: Pali Rohár <[e-mail]>
Cc: Pavel Machek <[e-mail]>
Cc: Timo Kokkonen <[e-mail]>
Cc: Tony Lindgren <[e-mail]>
Signed-off-by: Sean Young <[e-mail]>
---
 .../bindings/leds/irled/pwm-ir-tx.yaml        |  4 +++-
 .../devicetree/bindings/media/nokia,n900-ir   | 20 -------------------
 2 files changed, 3 insertions(+), 21 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/nokia,n900-ir

Comments

Krzysztof Kozlowski Aug. 22, 2023, 9:28 a.m. UTC | #1

On 22/08/2023 11:12, Sean Young wrote:
> The generic pwm-ir-tx driver works for the Nokia n900, so nokia,n900-ir
> can be removed.
> 
> Cc: Sicelo <[e-mail]>
> Cc: Rob Herring <[e-mail]>
> Cc: Krzysztof Kozlowski <[e-mail]>
> Cc: Conor Dooley <[e-mail]>
> Cc: [e-mail]
> Cc: Ivaylo Dimitrov <[e-mail]>
> Cc: Pali Rohár <[e-mail]>
> Cc: Pavel Machek <[e-mail]>
> Cc: Timo Kokkonen <[e-mail]>
> Cc: Tony Lindgren <[e-mail]>
> Signed-off-by: Sean Young <[e-mail]>

Unfortunately not much improved here, so:
1. Please use scripts/get_maintainers.pl to get a list of necessary
people and lists to CC. It might happen, that command when run on an
older kernel, gives you outdated entries. Therefore please be sure you
base your patches on recent Linux kernel.

You missed at least DT list (maybe more), so this won't be tested by
automated tooling. Performing review on untested code might be a waste
of time, thus I will skip this patch entirely till you follow the
process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.

2. Test your bindings before sending. Especially if you decide to
opt-out from automatic testing :/

Best regards,
Krzysztof

Krzysztof Kozlowski Aug. 22, 2023, 9:36 a.m. UTC | #2

On 22/08/2023 11:28, Krzysztof Kozlowski wrote:
> On 22/08/2023 11:12, Sean Young wrote:
>> The generic pwm-ir-tx driver works for the Nokia n900, so nokia,n900-ir
>> can be removed.
>>
>> Cc: Sicelo <[e-mail]>
>> Cc: Rob Herring <[e-mail]>
>> Cc: Krzysztof Kozlowski <[e-mail]>
>> Cc: Conor Dooley <[e-mail]>
>> Cc: [e-mail]
>> Cc: Ivaylo Dimitrov <[e-mail]>
>> Cc: Pali Rohár <[e-mail]>
>> Cc: Pavel Machek <[e-mail]>
>> Cc: Timo Kokkonen <[e-mail]>
>> Cc: Tony Lindgren <[e-mail]>
>> Signed-off-by: Sean Young <[e-mail]>
> 
> Unfortunately not much improved here, so:
> 1. Please use scripts/get_maintainers.pl to get a list of necessary
> people and lists to CC. It might happen, that command when run on an
> older kernel, gives you outdated entries. Therefore please be sure you
> base your patches on recent Linux kernel.
> 
> You missed at least DT list (maybe more), so this won't be tested by
> automated tooling. Performing review on untested code might be a waste
> of time, thus I will skip this patch entirely till you follow the
> process allowing the patch to be tested.
> 
> Please kindly resend and include all necessary To/Cc entries.

Ykes, my bad, the DT list is there. Therefore you can expect Rob's bot
with complains.

> 
> 2. Test your bindings before sending. Especially if you decide to
> opt-out from automatic testing :/

Testing before sending is still needed, though.


Best regards,
Krzysztof

Sean Young Aug. 22, 2023, 9:56 a.m. UTC | #3

On Tue, Aug 22, 2023 at 11:28:00AM +0200, Krzysztof Kozlowski wrote:
> On 22/08/2023 11:12, Sean Young wrote:
> > The generic pwm-ir-tx driver works for the Nokia n900, so nokia,n900-ir
> > can be removed.
> > 
> > Cc: Sicelo <[e-mail]>
> > Cc: Rob Herring <[e-mail]>
> > Cc: Krzysztof Kozlowski <[e-mail]>
> > Cc: Conor Dooley <[e-mail]>
> > Cc: [e-mail]
> > Cc: Ivaylo Dimitrov <[e-mail]>
> > Cc: Pali Rohár <[e-mail]>
> > Cc: Pavel Machek <[e-mail]>
> > Cc: Timo Kokkonen <[e-mail]>
> > Cc: Tony Lindgren <[e-mail]>
> > Signed-off-by: Sean Young <[e-mail]>
> 
> Unfortunately not much improved here, so:
> 1. Please use scripts/get_maintainers.pl to get a list of necessary
> people and lists to CC. It might happen, that command when run on an
> older kernel, gives you outdated entries. Therefore please be sure you
> base your patches on recent Linux kernel.
> 
> You missed at least DT list (maybe more), so this won't be tested by
> automated tooling.

So what tree do you want me to run scripts/get_maintainers.pl on? Apparently
Linus' master or media staging master does not suffice.

> Performing review on untested code might be a waste
> of time, thus I will skip this patch entirely till you follow the
> process allowing the patch to be tested.
> 
> Please kindly resend and include all necessary To/Cc entries.

Please kindly tell me how follow your thiefdom's rules. What tree do
you want me to run scripts/get_maintainers.pl on? What is the DT list
that I missed out?

> 2. Test your bindings before sending. Especially if you decide to
> opt-out from automatic testing :/

I ran:

	make dt_binding_check

And it ran successfully. Anything else?

Thanks,

Sean

Rob Herring (Arm) Aug. 22, 2023, 10:14 a.m. UTC | #4

On Tue, 22 Aug 2023 10:12:45 +0100, Sean Young wrote:
> The generic pwm-ir-tx driver works for the Nokia n900, so nokia,n900-ir
> can be removed.
> 
> Cc: Sicelo <[e-mail]>
> Cc: Rob Herring <[e-mail]>
> Cc: Krzysztof Kozlowski <[e-mail]>
> Cc: Conor Dooley <[e-mail]>
> Cc: [e-mail]
> Cc: Ivaylo Dimitrov <[e-mail]>
> Cc: Pali Rohár <[e-mail]>
> Cc: Pavel Machek <[e-mail]>
> Cc: Timo Kokkonen <[e-mail]>
> Cc: Tony Lindgren <[e-mail]>
> Signed-off-by: Sean Young <[e-mail]>
> ---
>  .../bindings/leds/irled/pwm-ir-tx.yaml        |  4 +++-
>  .../devicetree/bindings/media/nokia,n900-ir   | 20 -------------------
>  2 files changed, 3 insertions(+), 21 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/nokia,n900-ir
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/leds/irled/pwm-ir-tx.yaml:19:6: [warning] wrong indentation: expected 6 but found 5 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/irled/pwm-ir-tx.example.dtb: irled: compatible: ['pwm-ir-tx'] is too short
	from schema $id: http://devicetree.org/schemas/leds/irled/pwm-ir-tx.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[e-mail]

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.

Krzysztof Kozlowski Aug. 22, 2023, 11:37 a.m. UTC | #5

On 22/08/2023 11:56, Sean Young wrote:
>> Performing review on untested code might be a waste
>> of time, thus I will skip this patch entirely till you follow the
>> process allowing the patch to be tested.
>>
>> Please kindly resend and include all necessary To/Cc entries.
> 
> Please kindly tell me how follow your thiefdom's rules. What tree do
> you want me to run scripts/get_maintainers.pl on? What is the DT list
> that I missed out?

That was my mistake, and I responded correcting myself 20 minutes before
you sent this question.

> 
>> 2. Test your bindings before sending. Especially if you decide to
>> opt-out from automatic testing :/
> 
> I ran:
> 
> 	make dt_binding_check
> 
> And it ran successfully. Anything else?

This should be enough, but I doubt that it was successful.

Best regards,
Krzysztof

Sean Young Aug. 22, 2023, 2:18 p.m. UTC | #6

On Tue, Aug 22, 2023 at 01:37:08PM +0200, Krzysztof Kozlowski wrote:
> On 22/08/2023 11:56, Sean Young wrote:
> >> 2. Test your bindings before sending. Especially if you decide to
> >> opt-out from automatic testing :/
> > 
> > I ran:
> > 
> > 	make dt_binding_check
> > 
> > And it ran successfully. Anything else?
> 
> This should be enough, but I doubt that it was successful.

The command succeeds just fine with a 0 exit code, however I failed to spot
the error in the output. My bad. I'll send a v3 soon.

Thanks,

Sean

Patch

diff --git a/Documentation/devicetree/bindings/leds/irled/pwm-ir-tx.yaml b/Documentation/devicetree/bindings/leds/irled/pwm-ir-tx.yaml
index f2a6fa140f38..05d8e3bb1ef3 100644
--- a/Documentation/devicetree/bindings/leds/irled/pwm-ir-tx.yaml
+++ b/Documentation/devicetree/bindings/leds/irled/pwm-ir-tx.yaml
@@ -15,7 +15,9 @@  description:
 
 properties:
   compatible:
-    const: pwm-ir-tx
+    items:
+     - const: pwm-ir-tx
+     - const: nokia,n900-ir
 
   pwms:
     maxItems: 1
diff --git a/Documentation/devicetree/bindings/media/nokia,n900-ir b/Documentation/devicetree/bindings/media/nokia,n900-ir
deleted file mode 100644
index 13a18ce37dd1..000000000000
--- a/Documentation/devicetree/bindings/media/nokia,n900-ir
+++ /dev/null
@@ -1,20 +0,0 @@ 
-Device-Tree bindings for LIRC TX driver for Nokia N900(RX51)
-
-Required properties:
-	- compatible: should be "nokia,n900-ir".
-	- pwms: specifies PWM used for IR signal transmission.
-
-Example node:
-
-	pwm9: dmtimer-pwm@9 {
-		compatible = "ti,omap-dmtimer-pwm";
-		ti,timers = <&timer9>;
-		ti,clock-source = <0x00>; /* timer_sys_ck */
-		#pwm-cells = <3>;
-	};
-
-	ir: n900-ir {
-		compatible = "nokia,n900-ir";
-
-		pwms = <&pwm9 0 26316 0>; /* 38000 Hz */
-	};