| Message ID | E1ol97m-00EDSR-46@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: sfp: improve high power module implementation | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Wed, 19 Oct 2022 14:28:46 +0100, Russell King (Oracle) wrote: > Add a minimum and default for the maximum-power-milliwatt option; > module power levels were originally up to 1W, so this is the default > and the minimum power level we can have for a functional SFP cage. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ > 1 file changed, 2 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.yaml: properties:maximum-power-milliwatt: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']} hint: Scalar and array keywords cannot be mixed from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ 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.
On Wed, Oct 19, 2022 at 06:31:53PM -0500, Rob Herring wrote: > On Wed, 19 Oct 2022 14:28:46 +0100, Russell King (Oracle) wrote: > > Add a minimum and default for the maximum-power-milliwatt option; > > module power levels were originally up to 1W, so this is the default > > and the minimum power level we can have for a functional SFP cage. > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ > > 1 file changed, 2 insertions(+) > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.yaml: properties:maximum-power-milliwatt: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']} > hint: Scalar and array keywords cannot be mixed > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# I'm reading that error message and it means absolutely nothing to me. Please can you explain it (and also re-word it to be clearer)? Thanks.
On Thu, Oct 20, 2022 at 09:28:25AM +0100, Russell King (Oracle) wrote: > On Wed, Oct 19, 2022 at 06:31:53PM -0500, Rob Herring wrote: > > On Wed, 19 Oct 2022 14:28:46 +0100, Russell King (Oracle) wrote: > > > Add a minimum and default for the maximum-power-milliwatt option; > > > module power levels were originally up to 1W, so this is the default > > > and the minimum power level we can have for a functional SFP cage. > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > --- > > > Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.yaml: properties:maximum-power-milliwatt: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']} > > hint: Scalar and array keywords cannot be mixed > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > I'm reading that error message and it means absolutely nothing to me. > Please can you explain it (and also re-word it to be clearer)? 'maxItems' is a constraint for arrays. 'maximum' is a constraint for scalar values. Mixing them does not make sense. I have little control over the 1st line as that comes from jsonschema package. 'hint' is what I've added to explain things a bit more. Rob
On Thu, Oct 20, 2022 at 9:19 AM Rob Herring <robh@kernel.org> wrote: > > On Thu, Oct 20, 2022 at 09:28:25AM +0100, Russell King (Oracle) wrote: > > On Wed, Oct 19, 2022 at 06:31:53PM -0500, Rob Herring wrote: > > > On Wed, 19 Oct 2022 14:28:46 +0100, Russell King (Oracle) wrote: > > > > Add a minimum and default for the maximum-power-milliwatt option; > > > > module power levels were originally up to 1W, so this is the default > > > > and the minimum power level we can have for a functional SFP cage. > > > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > --- > > > > Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > > > yamllint warnings/errors: > > > > > > dtschema/dtc warnings/errors: > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.yaml: properties:maximum-power-milliwatt: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']} > > > hint: Scalar and array keywords cannot be mixed > > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > > > I'm reading that error message and it means absolutely nothing to me. > > Please can you explain it (and also re-word it to be clearer)? > > 'maxItems' is a constraint for arrays. 'maximum' is a constraint for > scalar values. Mixing them does not make sense. TBC, dropping 'maxItems' is what is needed here. Rob
On Thu, Oct 20, 2022 at 09:19:23AM -0500, Rob Herring wrote: > On Thu, Oct 20, 2022 at 09:28:25AM +0100, Russell King (Oracle) wrote: > > On Wed, Oct 19, 2022 at 06:31:53PM -0500, Rob Herring wrote: > > > On Wed, 19 Oct 2022 14:28:46 +0100, Russell King (Oracle) wrote: > > > > Add a minimum and default for the maximum-power-milliwatt option; > > > > module power levels were originally up to 1W, so this is the default > > > > and the minimum power level we can have for a functional SFP cage. > > > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > --- > > > > Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > > > yamllint warnings/errors: > > > > > > dtschema/dtc warnings/errors: > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.yaml: properties:maximum-power-milliwatt: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']} > > > hint: Scalar and array keywords cannot be mixed > > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > > > I'm reading that error message and it means absolutely nothing to me. > > Please can you explain it (and also re-word it to be clearer)? > > 'maxItems' is a constraint for arrays. 'maximum' is a constraint for > scalar values. Mixing them does not make sense. > > I have little control over the 1st line as that comes from jsonschema > package. 'hint' is what I've added to explain things a bit more. Given that maximum-power-milliwatt is a single value and has never been an array, it seems then that the original conversion to yaml was wrong. What should it have been? (I'm clueless what the difference is between an array and scalar in this yaml stuff.)
On Thu, Oct 20, 2022 at 09:27:44AM -0500, Rob Herring wrote: > On Thu, Oct 20, 2022 at 9:19 AM Rob Herring <robh@kernel.org> wrote: > > > > On Thu, Oct 20, 2022 at 09:28:25AM +0100, Russell King (Oracle) wrote: > > > On Wed, Oct 19, 2022 at 06:31:53PM -0500, Rob Herring wrote: > > > > On Wed, 19 Oct 2022 14:28:46 +0100, Russell King (Oracle) wrote: > > > > > Add a minimum and default for the maximum-power-milliwatt option; > > > > > module power levels were originally up to 1W, so this is the default > > > > > and the minimum power level we can have for a functional SFP cage. > > > > > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > > --- > > > > > Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > > > > > yamllint warnings/errors: > > > > > > > > dtschema/dtc warnings/errors: > > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.yaml: properties:maximum-power-milliwatt: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']} > > > > hint: Scalar and array keywords cannot be mixed > > > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > > > > > I'm reading that error message and it means absolutely nothing to me. > > > Please can you explain it (and also re-word it to be clearer)? > > > > 'maxItems' is a constraint for arrays. 'maximum' is a constraint for > > scalar values. Mixing them does not make sense. > > TBC, dropping 'maxItems' is what is needed here. So how does this work? maxItems: 1 tells it that there should be an array of one property, which is at the DT level fundamentally the same as a scalar property. minimum: default: maximum: tells it that this is a scalar property, so there should be exactly one item or the property should not be mentioned?
On Thu, Oct 20, 2022 at 11:08 AM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Thu, Oct 20, 2022 at 09:27:44AM -0500, Rob Herring wrote: > > On Thu, Oct 20, 2022 at 9:19 AM Rob Herring <robh@kernel.org> wrote: > > > > > > On Thu, Oct 20, 2022 at 09:28:25AM +0100, Russell King (Oracle) wrote: > > > > On Wed, Oct 19, 2022 at 06:31:53PM -0500, Rob Herring wrote: > > > > > On Wed, 19 Oct 2022 14:28:46 +0100, Russell King (Oracle) wrote: > > > > > > Add a minimum and default for the maximum-power-milliwatt option; > > > > > > module power levels were originally up to 1W, so this is the default > > > > > > and the minimum power level we can have for a functional SFP cage. > > > > > > > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > > > --- > > > > > > Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ > > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > > > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > > > > > > > yamllint warnings/errors: > > > > > > > > > > dtschema/dtc warnings/errors: > > > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.yaml: properties:maximum-power-milliwatt: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']} > > > > > hint: Scalar and array keywords cannot be mixed > > > > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# > > > > > > > > I'm reading that error message and it means absolutely nothing to me. > > > > Please can you explain it (and also re-word it to be clearer)? > > > > > > 'maxItems' is a constraint for arrays. 'maximum' is a constraint for > > > scalar values. Mixing them does not make sense. > > > > TBC, dropping 'maxItems' is what is needed here. > > So how does this work? Do you really want to know? ;) > > maxItems: 1 json-schema happily ignores any keywords that it doesn't understand or don't make sense for a specific context. The DT meta-schema tries to prevent that. > tells it that there should be an array of one property, which is at the > DT level fundamentally the same as a scalar property. Yes, it is true that the YAML encoded DT and (currently) the internal encoding used by the tools encode everything as matrices simply because dtc doing the YAML encoding doesn't know the types beyond what DTS source level provides, so everything has to be the same encoding. Now we use the type information in the schemas to decode the DTBs directly and don't have that limitation. Once I remove the YAML encoding, we can stop encoding everything as a matrix and having to fixup the schemas from scalar -> array -> matrix. > minimum: > default: > maximum: > > tells it that this is a scalar property, so there should be exactly one > item or the property should not be mentioned? Not sure I follow the question. As the property is defined as a scalar, it only needs scalar keywords. Internally, the schema gets expanded to: prop: minItems: 1 maxItems: 1 items: - maxItems: 1 minItems: 1 items: - maximum: ??? minimum: ??? default: ??? This is what processed-schemas.json will contain if you just have the scalar keywords. It's a bit more messy now with the unit suffixes as initially they were all scalars, but over time we've had to allow for arrays. So it's really they default to scalars unless you need an array in which you can define: prop: maxItems: 2 items: maximum: ??? Could you do 'maxItems: 1' here? Yes, that would be a valid schema, but IIRC we'll still complain because it is redundant. Rob
diff --git a/Documentation/devicetree/bindings/net/sff,sfp.yaml b/Documentation/devicetree/bindings/net/sff,sfp.yaml index 06c66ab81c01..20d30cccc95e 100644 --- a/Documentation/devicetree/bindings/net/sff,sfp.yaml +++ b/Documentation/devicetree/bindings/net/sff,sfp.yaml @@ -23,6 +23,8 @@ title: Small Form Factor (SFF) Committee Small Form-factor Pluggable (SFP) maximum-power-milliwatt: maxItems: 1 + minimum: 1000 + default: 1000 description: Maximum module power consumption Specifies the maximum power consumption allowable by a module in the slot, in milli-Watts. Presently, modules can
Add a minimum and default for the maximum-power-milliwatt option; module power levels were originally up to 1W, so this is the default and the minimum power level we can have for a functional SFP cage. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- Documentation/devicetree/bindings/net/sff,sfp.yaml | 2 ++ 1 file changed, 2 insertions(+)