| Message ID | 1617881469-31965-1-git-send-email-skakit@codeaurora.org |
|---|---|
| Headers | show |
| Series | Add support for PMK8350 PON_HLOS PMIC peripheral | expand |
Hi Sebastian, On 2021-04-08 18:30, Sebastian Reichel wrote: > Hi, > > On Thu, Apr 08, 2021 at 05:01:08PM +0530, satya priya wrote: >> Convert qcom PON binding from .txt to .yaml format. >> >> Signed-off-by: satya priya <skakit@codeaurora.org> >> --- > > Thanks for doing this. > >> Changes in V2: >> - As per Rob's comments, converted the main PON binding and added in >> V2. >> >> .../devicetree/bindings/power/reset/qcom,pon.txt | 49 >> ---------------------- >> .../devicetree/bindings/power/reset/qcom,pon.yaml | 41 >> ++++++++++++++++++ >> 2 files changed, 41 insertions(+), 49 deletions(-) >> delete mode 100644 >> Documentation/devicetree/bindings/power/reset/qcom,pon.txt >> create mode 100644 >> Documentation/devicetree/bindings/power/reset/qcom,pon.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/power/reset/qcom,pon.txt >> b/Documentation/devicetree/bindings/power/reset/qcom,pon.txt >> deleted file mode 100644 >> index 0c0dc3a..0000000 >> --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.txt >> +++ /dev/null >> @@ -1,49 +0,0 @@ >> -Qualcomm PON Device >> - >> -The Power On device for Qualcomm PM8xxx is MFD supporting pwrkey >> -and resin along with the Android reboot-mode. >> - >> -This DT node has pwrkey and resin as sub nodes. >> - >> -Required Properties: >> --compatible: Must be one of: >> - "qcom,pm8916-pon" >> - "qcom,pms405-pon" >> - "qcom,pm8998-pon" >> - >> --reg: Specifies the physical address of the pon register >> - >> -Optional subnode: >> --pwrkey: Specifies the subnode pwrkey and should follow the >> - qcom,pm8941-pwrkey.txt description. >> --resin: Specifies the subnode resin and should follow the >> - qcom,pm8xxx-pwrkey.txt description. >> - >> -The rest of the properties should follow the generic reboot-mode >> description >> -found in reboot-mode.txt >> - >> -Example: >> - >> - pon@800 { >> - compatible = "qcom,pm8916-pon"; >> - >> - reg = <0x800>; >> - mode-bootloader = <0x2>; >> - mode-recovery = <0x1>; >> - >> - pwrkey { >> - compatible = "qcom,pm8941-pwrkey"; >> - interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>; >> - debounce = <15625>; >> - bias-pull-up; >> - linux,code = <KEY_POWER>; >> - }; >> - >> - resin { >> - compatible = "qcom,pm8941-resin"; >> - interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>; >> - debounce = <15625>; >> - bias-pull-up; >> - linux,code = <KEY_VOLUMEDOWN>; >> - }; >> - }; >> diff --git >> a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml >> b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml >> new file mode 100644 >> index 0000000..40eea5f1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml >> @@ -0,0 +1,41 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/power/reset/qcom,pon.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm PON Device >> + >> +maintainers: >> + - Vinod Koul <vkoul@kernel.org> >> + >> +description: | >> + The Power On device for Qualcomm PM8xxx is MFD supporting pwrkey >> + and resin along with the Android reboot-mode. >> + >> + This DT node has pwrkey and resin as sub nodes. >> + >> +properties: >> + compatible: >> + enum: >> + - qcom,pm8916-pon >> + - qcom,pms405-pon >> + - qcom,pm8998-pon >> + >> + reg: >> + description: Specifies the physical address of the pon register > > That description is obvious and pointless. Instead add > > maxItems: 1 > Okay. >> + pwrkey: >> + type: object >> + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" >> + >> + resin: >> + type: object >> + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: true > > Instead of allowing arbitrary properties, only valid modes > should be allowed. So drop additionalProperties and do this > instead: > > allOf: > - $ref: reboot-mode.yaml# > > unevaluatedProperties: false > Okay. >> +... > > Please do not drop the example :) > As per my understanding on Rob's comments [1] I have added one complete example in qcom,pm8941-pwrkey.yaml (see patch 4/4) and dropped it here. [1] https://lore.kernel.org/patchwork/patch/1390062/#1588027 > -- Sebastian Thanks, Satya Priya
On Thu, 08 Apr 2021 17:01:07 +0530, satya priya wrote: > From: David Collins <collinsd@codeaurora.org> > > Add power key and resin compatible strings for the PMK8350 PMIC. > These are needed to distinguish key PON_HLOS register differences > between PMK8350 and previous PMIC PON modules. > > Signed-off-by: David Collins <collinsd@codeaurora.org> > Signed-off-by: satya priya <skakit@codeaurora.org> > --- > Chnages in V2: > - Moved this patch before the conversion patches. > > Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring <robh@kernel.org>
Hi Sebastian, On 2021-04-09 13:48, skakit@codeaurora.org wrote: > Hi Sebastian, > > On 2021-04-08 18:30, Sebastian Reichel wrote: >> Hi, >> >> On Thu, Apr 08, 2021 at 05:01:08PM +0530, satya priya wrote: >>> Convert qcom PON binding from .txt to .yaml format. >>> >>> Signed-off-by: satya priya <skakit@codeaurora.org> >>> --- >> >> Thanks for doing this. >> >>> Changes in V2: >>> - As per Rob's comments, converted the main PON binding and added in >>> V2. >>> [...] >>> + reg: >>> + description: Specifies the physical address of the pon register >> >> That description is obvious and pointless. Instead add >> >> maxItems: 1 >> > > Okay. > >>> + pwrkey: >>> + type: object >>> + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" >>> + >>> + resin: >>> + type: object >>> + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" >>> + >>> +required: >>> + - compatible >>> + - reg >>> + >>> +additionalProperties: true >> >> Instead of allowing arbitrary properties, only valid modes >> should be allowed. So drop additionalProperties and do this >> instead: >> >> allOf: >> - $ref: reboot-mode.yaml# >> >> unevaluatedProperties: false >> > > Okay. > I am not able to use 'allOf' to refer reboot-mode.yaml as some of the properties do not match with reboot-mode.yaml properties. Can we use oneOf like below? oneOf: - $ref: "reboot-mode.yaml#" - $ref: "../../input/qcom,pm8941-pwrkey.yaml#" Also, If I drop additionalProperties I am getting below error. kernel/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml: 'additionalProperties' is a required property Thanks, Satya Priya
Hi, On Tue, Apr 27, 2021 at 11:45:44AM +0530, skakit@codeaurora.org wrote: > Hi Sebastian, > > On 2021-04-09 13:48, skakit@codeaurora.org wrote: > > Hi Sebastian, > > > > On 2021-04-08 18:30, Sebastian Reichel wrote: > > > Hi, > > > > > > On Thu, Apr 08, 2021 at 05:01:08PM +0530, satya priya wrote: > > > > Convert qcom PON binding from .txt to .yaml format. > > > > > > > > Signed-off-by: satya priya <skakit@codeaurora.org> > > > > --- > > > > > > Thanks for doing this. > > > > > > > Changes in V2: > > > > - As per Rob's comments, converted the main PON binding and > > > > added in V2. > > > > > [...] > > > > + reg: > > > > + description: Specifies the physical address of the pon register > > > > > > That description is obvious and pointless. Instead add > > > > > > maxItems: 1 > > > > > > > Okay. > > > > > > + pwrkey: > > > > + type: object > > > > + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" > > > > + > > > > + resin: > > > > + type: object > > > > + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + > > > > +additionalProperties: true > > > > > > Instead of allowing arbitrary properties, only valid modes > > > should be allowed. So drop additionalProperties and do this > > > instead: > > > > > > allOf: > > > - $ref: reboot-mode.yaml# > > > > > > unevaluatedProperties: false > > > > > > > Okay. > > I am not able to use 'allOf' to refer reboot-mode.yaml as some of the > properties do not match with reboot-mode.yaml properties. Can we use oneOf > like below? > > oneOf: > - $ref: "reboot-mode.yaml#" > - $ref: "../../input/qcom,pm8941-pwrkey.yaml#" That does not make sense. The reference to reboot-mode.yaml is needed because it adds valid mode properties, so that you can set unevaluatedProperties to false. You need it at the root of the PON binding. They are not added to the required list, so it's fine if not all of them are used. Also there can (and usually is) more than one mode, so using oneOf is not ok. Last but not least the pwrkey reference is needed to describe specific nodes (resin, pwrkey) and should not appear at the root of the PON binding. > Also, If I drop additionalProperties I am getting below error. > > kernel/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml: > 'additionalProperties' is a required property You need to add 'unevaluatedProperties: false'. It is basically the same as 'additionalProperties: false', but also accepts properties from the referenced bindings. Thanks, -- Sebastian
On 2021-04-27 14:07, Sebastian Reichel wrote: > Hi, > > On Tue, Apr 27, 2021 at 11:45:44AM +0530, skakit@codeaurora.org wrote: >> Hi Sebastian, >> >> On 2021-04-09 13:48, skakit@codeaurora.org wrote: >> > Hi Sebastian, >> > >> > On 2021-04-08 18:30, Sebastian Reichel wrote: >> > > Hi, >> > > >> > > On Thu, Apr 08, 2021 at 05:01:08PM +0530, satya priya wrote: >> > > > Convert qcom PON binding from .txt to .yaml format. >> > > > >> > > > Signed-off-by: satya priya <skakit@codeaurora.org> >> > > > --- >> > > >> > > Thanks for doing this. >> > > >> > > > Changes in V2: >> > > > - As per Rob's comments, converted the main PON binding and >> > > > added in V2. >> > > > >> [...] >> > > > + reg: >> > > > + description: Specifies the physical address of the pon register >> > > >> > > That description is obvious and pointless. Instead add >> > > >> > > maxItems: 1 >> > > >> > >> > Okay. >> > >> > > > + pwrkey: >> > > > + type: object >> > > > + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" >> > > > + >> > > > + resin: >> > > > + type: object >> > > > + $ref: "../../input/qcom,pm8941-pwrkey.yaml#" >> > > > + >> > > > +required: >> > > > + - compatible >> > > > + - reg >> > > > + >> > > > +additionalProperties: true >> > > >> > > Instead of allowing arbitrary properties, only valid modes >> > > should be allowed. So drop additionalProperties and do this >> > > instead: >> > > >> > > allOf: >> > > - $ref: reboot-mode.yaml# >> > > >> > > unevaluatedProperties: false >> > > >> > >> > Okay. >> >> I am not able to use 'allOf' to refer reboot-mode.yaml as some of the >> properties do not match with reboot-mode.yaml properties. Can we use >> oneOf >> like below? >> >> oneOf: >> - $ref: "reboot-mode.yaml#" >> - $ref: "../../input/qcom,pm8941-pwrkey.yaml#" > > That does not make sense. > > The reference to reboot-mode.yaml is needed because it adds valid > mode properties, so that you can set unevaluatedProperties to false. > You need it at the root of the PON binding. They are not added to > the required list, so it's fine if not all of them are used. Also > there can (and usually is) more than one mode, so using oneOf is not > ok. > Okay, but I am getting errors like below during make dtbs_check. kernel/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-ganges-kirin.dt.yaml: pon@800: 'compatible', 'pwrkey', 'reg' do not match any of the regexes: '^mode-.*$', 'pinctrl-[0-9]+' As suggested I have added allOf: - $ref: reboot-mode.yaml# at the root of binding and also added unevaluatedProperties: false > Last but not least the pwrkey reference is needed to describe > specific nodes (resin, pwrkey) and should not appear at the root > of the PON binding. > >> Also, If I drop additionalProperties I am getting below error. >> >> kernel/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml: >> 'additionalProperties' is a required property > > You need to add 'unevaluatedProperties: false'. It is basically > the same as 'additionalProperties: false', but also accepts > properties from the referenced bindings. > > Thanks, > > -- Sebastian
Hi, [...] >>> > > > + >>> > > > +required: >>> > > > + - compatible >>> > > > + - reg >>> > > > + >>> > > > +additionalProperties: true >>> > > >>> > > Instead of allowing arbitrary properties, only valid modes >>> > > should be allowed. So drop additionalProperties and do this >>> > > instead: >>> > > >>> > > allOf: >>> > > - $ref: reboot-mode.yaml# >>> > > >>> > > unevaluatedProperties: false >>> > > >>> > >>> > Okay. >>> >>> I am not able to use 'allOf' to refer reboot-mode.yaml as some of the >>> properties do not match with reboot-mode.yaml properties. Can we use >>> oneOf >>> like below? >>> >>> oneOf: >>> - $ref: "reboot-mode.yaml#" >>> - $ref: "../../input/qcom,pm8941-pwrkey.yaml#" >> >> That does not make sense. >> >> The reference to reboot-mode.yaml is needed because it adds valid >> mode properties, so that you can set unevaluatedProperties to false. >> You need it at the root of the PON binding. They are not added to >> the required list, so it's fine if not all of them are used. Also >> there can (and usually is) more than one mode, so using oneOf is not >> ok. >> > > Okay, but I am getting errors like below during make dtbs_check. > > kernel/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-ganges-kirin.dt.yaml: > pon@800: 'compatible', 'pwrkey', 'reg' do not match any of the > regexes: '^mode-.*$', 'pinctrl-[0-9]+' > Seems like I have to make 'additionalProperties' as true in reboot-mode.yaml I have checked other yaml binding docs where allOf is used, and they have 'additionalProperties' as true in the file which is being referred. Please let me know if this is not correct way to do it. > As suggested I have added > > allOf: > - $ref: reboot-mode.yaml# > > at the root of binding and also added unevaluatedProperties: false > >> Last but not least the pwrkey reference is needed to describe >> specific nodes (resin, pwrkey) and should not appear at the root >> of the PON binding. >> >>> Also, If I drop additionalProperties I am getting below error. >>> >>> kernel/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml: >>> 'additionalProperties' is a required property >> >> You need to add 'unevaluatedProperties: false'. It is basically >> the same as 'additionalProperties: false', but also accepts >> properties from the referenced bindings. >> >> Thanks, >> >> -- Sebastian Thanks, Satya Priya
Hi, On Fri, May 07, 2021 at 03:15:55PM +0530, skakit@codeaurora.org wrote: > Seems like I have to make 'additionalProperties' as true in reboot-mode.yaml > I have checked other yaml binding docs where allOf is used, and they have > 'additionalProperties' as true in the file which is being referred. Please > let me know if this is not correct way to do it. Yes, reboot-mode.yaml should have additionalProperties = true. I think Rob missed, that the binding is a generic one when he added it in f84e2c5c528d. -- Sebastian
David Collins (2): input: pm8941-pwrkey: add support for PMK8350 PON_HLOS PMIC peripheral dt-bindings: input: pm8941-pwrkey: add pmk8350 compatible strings satya priya (2): dt-bindings: power: reset: qcom-pon: Convert qcom PON binding to yaml dt-bindings: input: pm8941-pwrkey: Convert pm8941 power key binding to yaml .../bindings/input/qcom,pm8941-pwrkey.txt | 53 ----------- .../bindings/input/qcom,pm8941-pwrkey.yaml | 88 ++++++++++++++++++ .../devicetree/bindings/power/reset/qcom,pon.txt | 49 ---------- .../devicetree/bindings/power/reset/qcom,pon.yaml | 41 ++++++++ drivers/input/misc/pm8941-pwrkey.c | 103 ++++++++++++++------- 5 files changed, 201 insertions(+), 133 deletions(-) delete mode 100644 Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.yaml delete mode 100644 Documentation/devicetree/bindings/power/reset/qcom,pon.txt create mode 100644 Documentation/devicetree/bindings/power/reset/qcom,pon.yaml