| Message ID | 20240712113200.2468249-2-wangshuaijie@awinic.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Awinic SAR sensor | expand |
On 12/07/2024 13:31, wangshuaijie@awinic.com wrote: > From: shuaijie wang <wangshuaijie@awinic.com> > > Add the awinic,aw96xxx.yaml file to adapt to the awinic proximity sensor driver. > Addressing the issues raised in the previous version. > 1. Add a description about the hardware device. > 2. Remove inappropriate configuration items. > 3. Modify the formatting issues. That's commit msg or changelog? Don't mix both. Read submitting patches. > > Signed-off-by: shuaijie wang <wangshuaijie@awinic.com> > --- > .../iio/proximity/awinic,aw96xxx.yaml | 127 ++++++++++++++++++ > 1 file changed, 127 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml > > diff --git a/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml b/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml > new file mode 100644 > index 000000000000..459cb1644d3c > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml > @@ -0,0 +1,127 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/awinic,aw96xxx.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Awinic's AW96XXX capacitive proximity sensor > + > +maintainers: > + - Shuaijie Wang <wangshuaijie@awinic.com> > + > +description: | > + Awinic's AW96XXX proximity sensor. > + The specific absorption rate (SAR) is a metric that measures > + the degree of absorption of electromagnetic radiation emitted by wireless > + devices, such as mobile phones and tablets, by human tissue. > + In mobile phone applications, the proximity sensor is primarily used to detect > + the proximity of the human body to the phone. When the phone approaches the human body, > + it will actively reduce the transmit power of the antenna to keep the SAR within a safe > + range. Therefore, we also refer to the proximity sensor as a SAR sensor. Wrap at 80 (see Coding style). > + > +properties: > + compatible: > + enum: > + - awinic,aw96103 > + - awinic,aw96105 > + - awinic,aw96303 > + - awinic,aw96305 > + - awinic,aw96308 > + > + reg: > + maxItems: 1 > + > + interrupts: > + description: > + Generated by the device to announce that a close/far > + proximity event has happened. > + maxItems: 1 > + > + awinic,sar-num: Drop the property. I already asked. > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Set the number 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. > + In an application where a device utilizes multiple proximity sensors, > + it is used to retrieve the names of the register configuration files > + that the drivers need to load. For example: aw963xx_reg_0.bin/aw963xx_reg_1.bin > + > + awinic,regulator-power-supply: It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. > + 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: Aren't there existing IIO properties like this? > + $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,monitor-esd: > + type: boolean > + description: > + Choose if you want to monitor ESD. Why this is a choice? How does this describe hardware. > + > + awinic,pin-set-inter-pull-up: > + type: boolean > + description: > + Choose if you want to set the interrupt pin to internal pull-up. > + > + awinic,using-pm-ops: NAK, drop all such OS policies. > + type: boolean > + description: > + Choose if you want to change the chip mode on suppend and resume. > + > + awinic,use-plug-cail:> + type: boolean > + description: > + Choose If you want to perform calibration when plugging and unplugging the charger. And why this is board dependent? > + > + awinic,irq-mux: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [2, 5] > + description: > + You only need to set this configuration item if you are using AW96308 adn AW96305BFOR. > + If CS2 is used as the interrupt pin, this item should be set to 2. > + If CS5 is used as the interrupt pin, this item should be set to 5. > + Best regards, Krzysztof
On Fri, 12 Jul 2024 11:31:59 +0000, wangshuaijie@awinic.com wrote: > From: shuaijie wang <wangshuaijie@awinic.com> > > Add the awinic,aw96xxx.yaml file to adapt to the awinic proximity sensor driver. > Addressing the issues raised in the previous version. > 1. Add a description about the hardware device. > 2. Remove inappropriate configuration items. > 3. Modify the formatting issues. > > Signed-off-by: shuaijie wang <wangshuaijie@awinic.com> > --- > .../iio/proximity/awinic,aw96xxx.yaml | 127 ++++++++++++++++++ > 1 file changed, 127 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml:50:49: [error] syntax error: mapping values are not allowed here (syntax) dtschema/dtc warnings/errors: make[2]: *** Deleting file 'Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.example.dts' Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml:50:49: mapping values are not allowed in this context make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml:50:49: mapping values are not allowed in this context /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml: ignoring, error parsing file make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2 make: *** [Makefile:240: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240712113200.2468249-2-wangshuaijie@awinic.com 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.
> > > + 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: > > Aren't there existing IIO properties like this? > I'm not sure what this is. If it's about the chip support then the compatible is enough. If is is about what is wired, then use child nodes per channel to describe what is connected up. See adc.yaml for an example - similar can be used for other device types. > > + $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,monitor-esd: > > + type: boolean > > + description: > > + Choose if you want to monitor ESD.
Hi Krzysztof, On Fri, 12 Jul 2024 13:55:09 +0200, krzk@kernel.org wrote: >On 12/07/2024 13:31, wangshuaijie@awinic.com wrote: >> From: shuaijie wang <wangshuaijie@awinic.com> >> >> Add the awinic,aw96xxx.yaml file to adapt to the awinic proximity sensor driver. >> Addressing the issues raised in the previous version. >> 1. Add a description about the hardware device. >> 2. Remove inappropriate configuration items. >> 3. Modify the formatting issues. > >That's commit msg or changelog? Don't mix both. Read submitting patches. > The patch for v4 will fix these issues. >> >> Signed-off-by: shuaijie wang <wangshuaijie@awinic.com> >> --- >> .../iio/proximity/awinic,aw96xxx.yaml | 127 ++++++++++++++++++ >> 1 file changed, 127 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml >> >> diff --git a/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml b/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml >> new file mode 100644 >> index 000000000000..459cb1644d3c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml >> @@ -0,0 +1,127 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/input/awinic,aw96xxx.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Awinic's AW96XXX capacitive proximity sensor >> + >> +maintainers: >> + - Shuaijie Wang <wangshuaijie@awinic.com> >> + >> +description: | >> + Awinic's AW96XXX proximity sensor. >> + The specific absorption rate (SAR) is a metric that measures >> + the degree of absorption of electromagnetic radiation emitted by wireless >> + devices, such as mobile phones and tablets, by human tissue. >> + In mobile phone applications, the proximity sensor is primarily used to detect >> + the proximity of the human body to the phone. When the phone approaches the human body, >> + it will actively reduce the transmit power of the antenna to keep the SAR within a safe >> + range. Therefore, we also refer to the proximity sensor as a SAR sensor. > >Wrap at 80 (see Coding style). > The patch for v4 will fix these issues. >> + >> +properties: >> + compatible: >> + enum: >> + - awinic,aw96103 >> + - awinic,aw96105 >> + - awinic,aw96303 >> + - awinic,aw96305 >> + - awinic,aw96308 >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + description: >> + Generated by the device to announce that a close/far >> + proximity event has happened. >> + maxItems: 1 >> + >> + awinic,sar-num: > >Drop the property. I already asked. > The patch for v4 will remove this property. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Set the number 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. >> + In an application where a device utilizes multiple proximity sensors, >> + it is used to retrieve the names of the register configuration files >> + that the drivers need to load. For example: aw963xx_reg_0.bin/aw963xx_reg_1.bin >> + >> + awinic,regulator-power-supply: > >It does not look like you tested the bindings, at least after quick >look. Please run `make dt_binding_check` (see >Documentation/devicetree/bindings/writing-schema.rst for instructions). >Maybe you need to update your dtschema and yamllint. > The patch for v4 will fix these issues. >> + 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: > >Aren't there existing IIO properties like this? The patch for v4 will remove this property. > >> + $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,monitor-esd: >> + type: boolean >> + description: >> + Choose if you want to monitor ESD. > >Why this is a choice? How does this describe hardware. > The patch for v4 will remove this property. >> + >> + awinic,pin-set-inter-pull-up: >> + type: boolean >> + description: >> + Choose if you want to set the interrupt pin to internal pull-up. >> + >> + awinic,using-pm-ops: > >NAK, drop all such OS policies. > The patch for v4 will remove this property. >> + type: boolean >> + description: >> + Choose if you want to change the chip mode on suppend and resume. >> + >> + awinic,use-plug-cail:> + type: boolean >> + description: >> + Choose If you want to perform calibration when plugging and unplugging the charger. > >And why this is board dependent? > The patch for v4 will remove this property. >> + >> + awinic,irq-mux: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + enum: [2, 5] >> + description: >> + You only need to set this configuration item if you are using AW96308 adn AW96305BFOR. >> + If CS2 is used as the interrupt pin, this item should be set to 2. >> + If CS5 is used as the interrupt pin, this item should be set to 5. >> + > > >Best regards, >Krzysztof Thank you very much for your valuable advice. I will address these issues in the next version of the patch. Kind regards, Wang Shuaijie
On 25/07/2024 14:47, wangshuaijie@awinic.com wrote: > >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + Set the number 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. >>> + In an application where a device utilizes multiple proximity sensors, >>> + it is used to retrieve the names of the register configuration files >>> + that the drivers need to load. For example: aw963xx_reg_0.bin/aw963xx_reg_1.bin >>> + >>> + awinic,regulator-power-supply: >> >> It does not look like you tested the bindings, at least after quick >> look. Please run `make dt_binding_check` (see >> Documentation/devicetree/bindings/writing-schema.rst for instructions). >> Maybe you need to update your dtschema and yamllint. >> > > The patch for v4 will fix these issues. Really? I commented here that you did not test it, so let's look at v4 and see if you tested it. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml b/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml new file mode 100644 index 000000000000..459cb1644d3c --- /dev/null +++ b/Documentation/devicetree/bindings/iio/proximity/awinic,aw96xxx.yaml @@ -0,0 +1,127 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/awinic,aw96xxx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Awinic's AW96XXX capacitive proximity sensor + +maintainers: + - Shuaijie Wang <wangshuaijie@awinic.com> + +description: | + Awinic's AW96XXX proximity sensor. + The specific absorption rate (SAR) is a metric that measures + the degree of absorption of electromagnetic radiation emitted by wireless + devices, such as mobile phones and tablets, by human tissue. + In mobile phone applications, the proximity sensor is primarily used to detect + the proximity of the human body to the phone. When the phone approaches the human body, + it will actively reduce the transmit power of the antenna to keep the SAR within a safe + range. Therefore, we also refer to the proximity sensor as a SAR sensor. + +properties: + compatible: + enum: + - awinic,aw96103 + - awinic,aw96105 + - awinic,aw96303 + - awinic,aw96305 + - awinic,aw96308 + + reg: + maxItems: 1 + + interrupts: + description: + Generated by the device to announce that a close/far + proximity event has happened. + maxItems: 1 + + awinic,sar-num: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Set the number 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. + In an application where a device utilizes multiple proximity sensors, + it is used to retrieve the names of the register configuration files + that the drivers need to load. For example: aw963xx_reg_0.bin/aw963xx_reg_1.bin + + 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,monitor-esd: + type: boolean + description: + Choose if you want to monitor ESD. + + awinic,pin-set-inter-pull-up: + type: boolean + description: + Choose if you want to set the interrupt pin to internal pull-up. + + awinic,using-pm-ops: + type: boolean + description: + Choose if you want to change the chip mode on suppend and resume. + + awinic,use-plug-cail: + type: boolean + description: + Choose If you want to perform calibration when plugging and unplugging the charger. + + awinic,irq-mux: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [2, 5] + description: + You only need to set this configuration item if you are using AW96308 adn AW96305BFOR. + If CS2 is used as the interrupt pin, this item should be set to 2. + If CS5 is used as the interrupt pin, this item should be set to 5. + +required: + - compatible + - reg + - interrupts + - awinic,sar-num + - awinic,channel-use-mask + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + proximity@12 { + compatible = "awinic,aw96308"; + reg = <0x12>; + interrupt-parent = <&gpio>; + interrupts = <23 IRQ_TYPE_EDGE_FALLING>; + awinic,sar-num = <0>; + awinic,channel-use-mask = <0xff>; + awinic,irq-mux = <2>; + }; + };