Re: [PATCH 1/2] dt-bindings: iio: proximity: Add bindings for Murata IRS-D200

From: Waqar Hameed
Date: Mon Jun 19 2023 - 07:23:58 EST


On Sat, Jun 17, 2023 at 10:55 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote:

> On 16/06/2023 17:10, Waqar Hameed wrote:
>> Murata IRS-D200 is a PIR sensor for human detection. It uses the I2C bus
>> for communication with interrupt support. Add devicetree bindings
>> requiring the compatible string, I2C slave address (reg) and interrupts.
>
> Thank you for your patch. There is something to discuss/improve. I have
> actually only remark about DTS example, but since I expect resend two
> more nits as well.
>
>
> A nit, subject: drop second/last, redundant "bindings for". The
> "dt-bindings" prefix is already stating that these are bindings.

Alright, will change to "dt-bindings: iio: proximity: Add Murata
IRS-D200".

[...]

>> +maintainers:
>> + - Waqar Hameed <waqar.hameed@xxxxxxxx>
>> +
>> +description: |
>
> Nit, do not need '|' unless you need to preserve formatting.

Yes, will remove (a remnant from previous version that had multiple
lines... Sorry!).

[...]

>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + pir@48 {
>
> Node names should be generic. See also explanation and list of examples
> in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
> so I guess: proximity@?

On that list there is "temperature-sensor". Would it make sense then to
call it "proximity-sensor"?