On 05/06/2024 11:11, wangshuaijie@xxxxxxxxxx wrote: > From: shuaijie wang <wangshuaijie@xxxxxxxxxx> > > Add the awinic,aw_sar.yaml file to adapt to the awinic sar sensor driver. Subject: drop final stops. From all your patches. > > Signed-off-by: shuaijie wang <wangshuaijie@xxxxxxxxxx> > --- No changelog, so nothing improved? > .../bindings/input/awinic,aw_sar.yaml | 125 ++++++++++++++++++ No underscores, but rather awinic,aw96103.yaml > 1 file changed, 125 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/awinic,aw_sar.yaml > > diff --git a/Documentation/devicetree/bindings/input/awinic,aw_sar.yaml b/Documentation/devicetree/bindings/input/awinic,aw_sar.yaml > new file mode 100644 > index 000000000000..2560ef09d3d0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/awinic,aw_sar.yaml > @@ -0,0 +1,125 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/awinic,aw_sar.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Awinic sar sensor driver family driver as Linux driver or some other hardware meaning? If first, then drop and describe hardware. > + > +maintainers: > + - Shuaijie Wang <wangshuaijie@xxxxxxxxxx> Missing description. You already got question about meaning of sar and indeed nothing improved. > + > +properties: > + compatible: > + enum: > + - awinic,aw96103 > + - awinic,aw96105 > + - awinic,aw96303 > + - awinic,aw96305 > + - awinic,aw96308 > + > + reg: > + maxItems: 1 > + > + irq-gpio: > + maxItems: 1 > + > + awinic,sar-label: label is a string, not number. > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Set the label of the SAR(Specific Absorption Rate) sensor. > + It is set to 0 if one awinic sar chip is used. > + If two awinic sar chips are used, awinic,sar-label in the first > + awinic-sar should be set to 0 and awinic,sar-label in the second > + awinic-sar should be set to 1. Sorry, no instance indexing. Drop. > + > + No need for two line breaks. > + awinic,regulator-power-supply: > + description: > + Choose if you want to use a regulator to power the chip. Then the > + vccX-supply has to be set. > + > + vcc0-supply: > + description: > + Optional regulator for chip, 1.7V-3.6V. > + If two awinic sar chips are used, the first regulator > + should set the ID to vcc0-supply and the second regulator > + should set the ID to vcc1-supply. > + > + awinic,channel-use-mask: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + The mask of channels used. > + Configure according to the specific chip channel used. > + Bit[31:0] Each bit represents a channel. > + If the customer uses ch0 and ch2, then channel_use_mask=<0x05> > + For a 3-channel chip, the maximum value is 0x07; > + For a 5-channel chip, the maximum value is 0x1F; > + For a 8-channel chip, the maximum value is 0xFF; > + > + awinic,update-fw: > + type: boolean > + description: > + Choose if you want to update the firmware. Not much improve in explanation or rationale. Why do you want to update FW every time? Explain this in property description. I mostly skipped the rest, because it does not look like you addresses previous feedback. ... > + > +required: > + - compatible > + - reg > + - irq-gpio > + - awinic,sar-label > + - awinic,channel-use-mask > + > +unevaluatedProperties: false Missing some ref, like dai-common or component... or this is supposed to be additionalProperties: false instead. > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + awinic-sar@12 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation I still have no clue what is sar and there is no description in this binding. > + compatible = "awinic,aw96308"; > + reg = <0x12>; > + irq-gpio = <&tlmm 72 0>; Use proper defines. > + awinic,sar-label = < 0 >; Do not introduce different coding style. Drop spaces. See DTS coding style. Best regards, Krzysztof
