| Message ID | 790da5fe60eb3bcd190830770866147bbb5f8143.1615198094.git.matti.vaittinen@fi.rohmeurope.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Support ROHM BD71815 PMIC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Mon, 08 Mar 2021 12:40:50 +0200, Matti Vaittinen wrote: > Document DT bindings for ROHM BD71815. > > BD71815 is a single-chip power management IC mainly for battery-powered > portable devices. The IC integrates 5 bucks, 7 LDOs, a boost driver for > LED, a battery charger with a Coulomb counter, a real-time clock, a 32kHz > clock and two general-purpose outputs although only one is documented by > the data-sheet. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > .../bindings/mfd/rohm,bd71815-pmic.yaml | 201 ++++++++++++++++++ > 1 file changed, 201 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/regulator/rohm,bd71815-regulator.yaml' xargs: dt-doc-validate: exited with status 255; aborting Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/regulator/rohm,bd71815-regulator.yaml' make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.example.dt.yaml] Error 255 make[1]: *** Deleting file 'Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.example.dt.yaml' make: *** [Makefile:1380: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1448970 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. 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.
Hello Rob, On Mon, 2021-03-08 at 10:39 -0700, Rob Herring wrote: > On Mon, 08 Mar 2021 12:40:50 +0200, Matti Vaittinen wrote: > > Document DT bindings for ROHM BD71815. > > > > BD71815 is a single-chip power management IC mainly for battery- > > powered > > portable devices. The IC integrates 5 bucks, 7 LDOs, a boost driver > > for > > LED, a battery charger with a Coulomb counter, a real-time clock, a > > 32kHz > > clock and two general-purpose outputs although only one is > > documented by > > the data-sheet. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > .../bindings/mfd/rohm,bd71815-pmic.yaml | 201 > > ++++++++++++++++++ > > 1 file changed, 201 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml > > > > My bot found errors running 'make dt_binding_check' on your patch: I am sorry to bother but I've spent a while trying to reproduce this. For some reason I can't trigger the error from 'make dt_binding_check' or 'make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mfd/rohm,bd71815- pmic.yaml' even after I ran 'pip3 install dtschema --upgrade --user'. I should also have yamllint installed. > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > Unknown file referenced: [Errno 2] No such file or directory: > '/usr/local/lib/python3.8/dist- > packages/dtschema/schemas/regulator/rohm,bd71815-regulator.yaml' This bothers me slightly. The patch 04/15 should bring-in the rohm,bd71815-regulator.yaml. Does this error indicate that file is missing or is my $ref somehow invalid? *** opinion follows - not sure if it just me but... *** I know I should probably keep my mouth shut but... I am more and more thinking that the yaml bindings are yet another 'excessive unit-test' type solution. Tooling which should "force doing things correctly" is eventually hindering development and causing the end result being sub- optimal. I mean that creating binding docs takes way too much time from someone like me who is "yaml-illiterate". And when I eventually get yaml done - the end result is far less descriptive for human eyes than the "good old" free-text format would've been. I know one can add comments - but I don't see much of them in the binding docs... Best Regards -- Matti Vaittinen
On Tue, Mar 9, 2021 at 5:51 AM Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> wrote: > > Hello Rob, > > On Mon, 2021-03-08 at 10:39 -0700, Rob Herring wrote: > > On Mon, 08 Mar 2021 12:40:50 +0200, Matti Vaittinen wrote: > > > Document DT bindings for ROHM BD71815. > > > > > > BD71815 is a single-chip power management IC mainly for battery- > > > powered > > > portable devices. The IC integrates 5 bucks, 7 LDOs, a boost driver > > > for > > > LED, a battery charger with a Coulomb counter, a real-time clock, a > > > 32kHz > > > clock and two general-purpose outputs although only one is > > > documented by > > > the data-sheet. > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > --- > > > .../bindings/mfd/rohm,bd71815-pmic.yaml | 201 > > > ++++++++++++++++++ > > > 1 file changed, 201 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > I am sorry to bother but I've spent a while trying to reproduce this. > For some reason I can't trigger the error from > > 'make dt_binding_check' or > 'make dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/mfd/rohm,bd71815- > pmic.yaml' > > even after I ran > > 'pip3 install dtschema --upgrade --user'. > > I should also have yamllint installed. > > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > Unknown file referenced: [Errno 2] No such file or directory: > > '/usr/local/lib/python3.8/dist- > > packages/dtschema/schemas/regulator/rohm,bd71815-regulator.yaml' > > This bothers me slightly. The patch 04/15 should bring-in the > rohm,bd71815-regulator.yaml. Does this error indicate that file is > missing or is my $ref somehow invalid? Then it's simply a false positive. I usually check these and try to only send the email if the dependency is not in the series so the dependency is clear. It's a balance of replying quickly and my time reviewing the errors. > *** opinion follows - not sure if it just me but... *** > > I know I should probably keep my mouth shut but... I am more and more > thinking that the yaml bindings are yet another 'excessive unit-test' > type solution. Tooling which should "force doing things correctly" is > eventually hindering development and causing the end result being sub- > optimal. It's about validating DTS files actually do what the bindings say. It's pretty clear that the free form text bindings left a lot of things ambiguous. How would you propose we can check every property in a DTS file has a definition (minimally of it's type)? Freeform text can simply never do that. > I mean that creating binding docs takes way too much time from someone > like me who is "yaml-illiterate". And when I eventually get yaml done - > the end result is far less descriptive for human eyes than the "good > old" free-text format would've been. I know one can add comments - but > I don't see much of them in the binding docs... Because people do the minimum? The only comments/description I object to are duplicating generic descriptions of common properties. There's certainly lots of things we could do. There are tools which generate pretty docs out of json-schema. Not sure how useful they would be OOTB. But I simply don't have the bandwidth to look into them. I can barely keep up with reviews... Rob
Hello Rob, (I dropped other personal mails from CC - kept only the lists) On Tue, 2021-03-09 at 08:11 -0700, Rob Herring wrote: > On Tue, Mar 9, 2021 at 5:51 AM Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> wrote: > > Hello Rob, > > > > On Mon, 2021-03-08 at 10:39 -0700, Rob Herring wrote: > > > On Mon, 08 Mar 2021 12:40:50 +0200, Matti Vaittinen wrote: > > > > > > > dtschema/dtc warnings/errors: > > > Unknown file referenced: [Errno 2] No such file or directory: > > > '/usr/local/lib/python3.8/dist- > > > packages/dtschema/schemas/regulator/rohm,bd71815-regulator.yaml' > > > > This bothers me slightly. The patch 04/15 should bring-in the > > rohm,bd71815-regulator.yaml. Does this error indicate that file is > > missing or is my $ref somehow invalid? > > Then it's simply a false positive. I usually check these and try to > only send the email if the dependency is not in the series so the > dependency is clear. It's a balance of replying quickly and my time > reviewing the errors. Oh, good to know. I was assuming I got it wrong once again... Thanks! Rest of the mail is further discussion why I wonder if yaml bindings are worth it - you can skip it if you're not in a mood for babbling ;) Best Regards Matti > > *** opinion follows - not sure if it just me but... *** > > > > I know I should probably keep my mouth shut but... I am more and > > more > > thinking that the yaml bindings are yet another 'excessive unit- > > test' > > type solution. Tooling which should "force doing things correctly" > > is > > eventually hindering development and causing the end result being > > sub- > > optimal. > > It's about validating DTS files actually do what the bindings say. > It's pretty clear that the free form text bindings left a lot of > things ambiguous. > > How would you propose we can check every property in a DTS file has a > definition (minimally of it's type)? Freeform text can simply never > do > that. True. But I am not at all sure that the benefit of verifying the .dts files programmatically exceeds cost (work + lost readability for humans + impact of increased "not that fun" work). Anyways, as I wrote - this is just my biased opinion. Other people may have different opinions :] > > I mean that creating binding docs takes way too much time from > > someone > > like me who is "yaml-illiterate". And when I eventually get yaml > > done - > > the end result is far less descriptive for human eyes than the > > "good > > old" free-text format would've been. I know one can add comments - > > but > > I don't see much of them in the binding docs... > > Because people do the minimum? I know. This is the very basic nature of most human beings. I think this must be accepted. And likelihood for doing bare minimum sky- rockets when people feel the work they do is dull/boring/not fun. And I just have a feeling that many who enjoy writing drivers find writing the yaml bindings quite not fun. (I have absolutely no statistics to back up this statement - it's just a feeling). Big question is how to get best results in this not-so-perfect world when we know that people are both lazy and make mistakes? I have kind of deja-vu here. I've seen many attempts of tightening mechanical checks to get rid of human errors. In my opinion it rarely works well. To pick few I've seen: - demand for 100 % UT test coverage. => lazy people started to avoid code changes => autogenerated test cases which assumed current code was correct and brought no value but incredible inertia to all changes. - demand for heavy(ish) RCA/EDA process when severe faults were fixed => lazy people started to mark severe faults as minor - demand for peculiar, hard to remember syntax for commit messages to automatically fill in different statistics/management data => lazy people started to commit all changes in same category (syntax they had memorized/had alias for). - demand for DT docs which can be verified => ... ? Common thing is that all of these have valid, good intention - but overhead of doing them exceeded the (visible) benefits and lazy people tried to get around of them / did bare minimum. And as a side impact - part of the work got more boring - which also has (bad) consequences. I am not surprized if people try skipping creating DT docs. Or if the people do bare minimum with the DT docs. > The only comments/description I object > to are duplicating generic descriptions of common properties. I know. And I don't think you are to blame here. > There's certainly lots of things we could do. There are tools which > generate pretty docs out of json-schema. Not sure how useful they > would be OOTB. But I simply don't have the bandwidth to look into > them. I can barely keep up with reviews... Yes. I understand this. And now I am just taking more of that time - sorry. I just gave an opinion (knowing it probably won't change anything - but at least I've told what I am afraid of). Thanks for the open discussion :) I appreciate it. And I do really admire the amount of effort you (have) put in this! I do also admire your skills here. And I absolutely respect your opinions what comes to how the DT should look like. I just see the usefulness of the bindings verification differently. I hope you don't find this as a personal attack.
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml new file mode 100644 index 000000000000..fe265bcab50d --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml @@ -0,0 +1,201 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/rohm,bd71815-pmic.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ROHM BD71815 Power Management Integrated Circuit bindings + +maintainers: + - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> + +description: | + BD71815AGW is a single-chip power management ICs for battery-powered + portable devices. It integrates 5 buck converters, 8 LDOs, a boost driver + for LED and a 500 mA single-cell linear charger. Also included is a Coulomb + counter, a real-time clock (RTC), and a 32.768 kHz clock gate and two GPOs. + +properties: + compatible: + const: rohm,bd71815 + + reg: + description: + I2C slave address. + maxItems: 1 + + interrupts: + maxItems: 1 + + gpio-controller: true + + "#gpio-cells": + const: 2 + description: | + The first cell is the pin number and the second cell is used to specify + flags. See ../gpio/gpio.txt for more information. + + clocks: + maxItems: 1 + + "#clock-cells": + const: 0 + + clock-output-names: + const: bd71815-32k-out + + rohm,clkout-open-drain: + description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos". + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 0 + maximum: 1 + + rohm,charger-sense-resistor-ohms: + minimum: 10000000 + maximum: 50000000 + description: | + BD71827 and BD71828 have SAR ADC for measuring charging currents. + External sense resistor (RSENSE in data sheet) should be used. If + something other but 30MOhm resistor is used the resistance value + should be given here in Ohms. + default: 30000000 + + regulators: + $ref: ../regulator/rohm,bd71815-regulator.yaml + description: + List of child nodes that specify the regulators. + + gpio-reserved-ranges: + description: | + Usage of BD71828 GPIO pins can be changed via OTP. This property can be + used to mark the pins which should not be configured for GPIO. Please see + the ../gpio/gpio.txt for more information. + + rohm,enable-hidden-gpo: + description: | + The BD71815 has undocumented GPO at pin E5. Pin is marked as GND at the + data-sheet as it's location in the middle of GND pins makes it hard to + use on PCB. If your board has managed to use this pin you can enable the + second GPO by defining this property. Dont enable this if you are unsure + about how the E5 pin is connected on your board. + type: boolean + +required: + - compatible + - reg + - interrupts + - clocks + - "#clock-cells" + - regulators + - gpio-controller + - "#gpio-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/leds/common.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + pmic: pmic@4b { + compatible = "rohm,bd71815"; + reg = <0x4b>; + + interrupt-parent = <&gpio1>; + interrupts = <29 IRQ_TYPE_LEVEL_LOW>; + + clocks = <&osc 0>; + #clock-cells = <0>; + clock-output-names = "bd71815-32k-out"; + + gpio-controller; + #gpio-cells = <2>; + + rohm,charger-sense-resistor-ohms = <10000000>; + + regulators { + buck1: buck1 { + regulator-name = "buck1"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <2000000>; + regulator-always-on; + regulator-ramp-delay = <1250>; + rohm,dvs-run-voltage = <1150000>; + rohm,dvs-suspend-voltage = <950000>; + }; + buck2: buck2 { + regulator-name = "buck2"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <2000000>; + regulator-always-on; + regulator-ramp-delay = <1250>; + rohm,dvs-run-voltage = <1150000>; + rohm,dvs-suspend-voltage = <950000>; + }; + buck3: buck3 { + regulator-name = "buck3"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <2700000>; + regulator-always-on; + }; + buck4: buck4 { + regulator-name = "buck4"; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1850000>; + regulator-always-on; + }; + buck5: buck5 { + regulator-name = "buck5"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + ldo1: ldo1 { + regulator-name = "ldo1"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + ldo2: ldo2 { + regulator-name = "ldo2"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + ldo3: ldo3 { + regulator-name = "ldo3"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + ldo4: ldo4 { + regulator-name = "ldo4"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + ldo5: ldo5 { + regulator-name = "ldo5"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + ldo6: ldodvref { + regulator-name = "ldodvref"; + regulator-always-on; + }; + ldo7: ldolpsr { + regulator-name = "ldolpsr"; + regulator-always-on; + }; + + boost: wled { + regulator-name = "wled"; + regulator-min-microamp = <10>; + regulator-max-microamp = <25000>; + }; + }; + }; + };