2024-01-21 15:27:43

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] dt-bindings: iio: light: Squash APDS9300 and APDS9960 schemas

On Sun, 21 Jan 2024 15:47:32 +1030
Subhajit Ghosh <[email protected]> wrote:

> Squashing Avago (Broadcom) APDS9300 and APDS9960 schemas into one
> file and removing the other. This is done as per the below review:
> Link: https://lore.kernel.org/all/[email protected]/

Sounds like a Suggested-by tag to reflect the ideas would be sensible here.

>
> This patch series adds the driver support and device tree binding schemas
> for APDS9306 Ambient Light Sensor.

This sentence isn't relevant to this patch, so I'd drop it.
We don't need additional motivation.

>It was pointed out in earlier reviews
> that the schemas for APDS9300 and APDS9960 looks similar and should be
> merged. This particular patch does the first operation of merging
> APDS9300 and APDS9960 schema files.
You have a reference above which is enough.

"Merge very similar schemas for APDS9300 and APDS9960."
is sufficient description alongside a suggested by tag and if you like
a link tag to as above. Note however that Link is an official tag
so belongs in the tag block at the end, not inline in the text.

>
Link: ...
> Signed-off-by: Subhajit Ghosh <[email protected]>
> ---
> v2 -> v5:
> - Removed 'required' for Interrupts and 'oneOf' for compatibility strings
> as per below reviews:
> Link: https://lore.kernel.org/lkml/20231028142944.7e210eb6@jic23-huawei/
> Link: https://lore.kernel.org/lkml/[email protected]/
> ---
> .../bindings/iio/light/avago,apds9300.yaml | 11 +++--
> .../bindings/iio/light/avago,apds9960.yaml | 44 -------------------
> 2 files changed, 7 insertions(+), 48 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/iio/light/avago,apds9960.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> index 206af44f2c43..c610780346e8 100644
> --- a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> @@ -4,17 +4,20 @@
> $id: http://devicetree.org/schemas/iio/light/avago,apds9300.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: Avago APDS9300 ambient light sensor
> +title: Avago Gesture/RGB/ALS/Proximity sensors
>
> maintainers:
> - - Jonathan Cameron <[email protected]>
> + - Subhajit Ghosh <[email protected]>
>
> description: |
> - Datasheet at https://www.avagotech.com/docs/AV02-1077EN
> + Datasheet: https://www.avagotech.com/docs/AV02-1077EN
> + Datasheet: https://www.avagotech.com/docs/AV02-4191EN
>
> properties:
> compatible:
> - const: avago,apds9300
> + enum:
> + - avago,apds9300
> + - avago,apds9960
>
> reg:
> maxItems: 1
> diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9960.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9960.yaml
> deleted file mode 100644
> index f06e0fda5629..000000000000
> --- a/Documentation/devicetree/bindings/iio/light/avago,apds9960.yaml
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> -%YAML 1.2
> ----
> -$id: http://devicetree.org/schemas/iio/light/avago,apds9960.yaml#
> -$schema: http://devicetree.org/meta-schemas/core.yaml#
> -
> -title: Avago APDS9960 gesture/RGB/ALS/proximity sensor
> -
> -maintainers:
> - - Matt Ranostay <[email protected]>
> -
> -description: |
> - Datasheet at https://www.avagotech.com/docs/AV02-4191EN
> -
> -properties:
> - compatible:
> - const: avago,apds9960
> -
> - reg:
> - maxItems: 1
> -
> - interrupts:
> - maxItems: 1
> -
> -additionalProperties: false
> -
> -required:
> - - compatible
> - - reg
> -
> -examples:
> - - |
> - i2c {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - light-sensor@39 {
> - compatible = "avago,apds9960";
> - reg = <0x39>;
> - interrupt-parent = <&gpio1>;
> - interrupts = <16 1>;
> - };
> - };
> -...



2024-01-24 05:27:11

by Subhajit Ghosh

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] dt-bindings: iio: light: Squash APDS9300 and APDS9960 schemas

On 22/1/24 01:57, Jonathan Cameron wrote:
> On Sun, 21 Jan 2024 15:47:32 +1030
> Subhajit Ghosh <[email protected]> wrote:
>
>> Squashing Avago (Broadcom) APDS9300 and APDS9960 schemas into one
>> file and removing the other. This is done as per the below review:
>> Link: https://lore.kernel.org/all/[email protected]/
>
> Sounds like a Suggested-by tag to reflect the ideas would be sensible here.
>
>>
>> This patch series adds the driver support and device tree binding schemas
>> for APDS9306 Ambient Light Sensor.
>
> This sentence isn't relevant to this patch, so I'd drop it.
> We don't need additional motivation.
>
>> It was pointed out in earlier reviews
>> that the schemas for APDS9300 and APDS9960 looks similar and should be
>> merged. This particular patch does the first operation of merging
>> APDS9300 and APDS9960 schema files.
> You have a reference above which is enough.
>
> "Merge very similar schemas for APDS9300 and APDS9960."
> is sufficient description alongside a suggested by tag and if you like
> a link tag to as above. Note however that Link is an official tag
> so belongs in the tag block at the end, not inline in the text.
>
>>
> Link: ...
Thank you so much Jonathan for taking time to explain these things.

Regards,
Subhajit Ghosh