Received-SPF: pass (google.com: domain of linux-kernel+bounces-32093-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sun, 21 Jan 2024 15:36:55 +0000
From: Jonathan Cameron <jic23@kernel.org>
To: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>, Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>, Matti Vaittinen <mazziesaccount@gmail.com>, Andy
 Shevchenko <andriy.shevchenko@linux.intel.com>, Marek Vasut
 <marex@denx.de>, Anshul Dalal <anshulusr@gmail.com>, Javier Carrasco
 <javier.carrasco.cruz@gmail.com>, Matt Ranostay <matt@ranostay.sg>, Stefan
 Windfeldt-Prytz <stefan.windfeldt-prytz@axis.com>,
 linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306
Message-ID: <20240121153655.5f734180@jic23-huawei>
In-Reply-To: <20240121051735.32246-3-subhajit.ghosh@tweaklogic.com>
References: <20240121051735.32246-1-subhajit.ghosh@tweaklogic.com>
	<20240121051735.32246-3-subhajit.ghosh@tweaklogic.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Sun, 21 Jan 2024 15:47:33 +1030
Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote:

> Adding device tree support for APDS9306 Ambient Light Sensor.
> Updating datasheet hyperlinks.
> Adding interrupt definition macro and header file.

This is an unrelated change, so should probably be in a separate patch.

> Adding vdd-supply property.

This one is reasonable to have in same patch as the new device addition
as, whilst I assume it's valid for the existing devices, you are adding it
to incorporate something that device also has.
Could also be a separate precursor patch.

> 
> Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> ---
> v2 -> v5:
>  - Implemented changes as per previous reviews:
>    Link: https://lore.kernel.org/lkml/20231028142944.7e210eb6@jic23-huawei/
>    Link: https://lore.kernel.org/lkml/22e9e5e9-d26a-46e9-8986-5062bbfd72ec@linaro.org/
> ---
>  .../bindings/iio/light/avago,apds9300.yaml        | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> index c610780346e8..bee73a590424 100644
> --- a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> @@ -4,19 +4,21 @@
>  $id: http://devicetree.org/schemas/iio/light/avago,apds9300.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Avago Gesture/RGB/ALS/Proximity sensors
> +title: Avago (Broadcom) Gesture/RGB/ALS/Proximity sensors
>  
>  maintainers:
>    - Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
>  
>  description: |
> -  Datasheet: https://www.avagotech.com/docs/AV02-1077EN
> -  Datasheet: https://www.avagotech.com/docs/AV02-4191EN
> +  Datasheet: https://docs.broadcom.com/doc/AV02-1077EN
> +  Datasheet: https://docs.broadcom.com/doc/AV02-4191EN
> +  Datasheet: https://docs.broadcom.com/doc/AV02-4755EN

Old links seem to still work, so why the change?

>  
>  properties:
>    compatible:
>      enum:
>        - avago,apds9300
> +      - avago,apds9306
>        - avago,apds9960
>  
>    reg:
> @@ -25,6 +27,8 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  vdd-supply: true
> +
>  additionalProperties: false
>  
>  required:
> @@ -33,6 +37,8 @@ required:
>  
>  examples:
>    - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
>      i2c {
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -41,7 +47,8 @@ examples:
>              compatible = "avago,apds9300";
>              reg = <0x39>;
>              interrupt-parent = <&gpio2>;
> -            interrupts = <29 8>;
> +            interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> +            vdd-supply = <&regulator_3v3>;
>          };
>      };
>  ...