| Message ID | 20240213160521.15925-2-quic_jinlmao@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | coresight: tpdm: Change qcom,dsb-element-size to qcom,dsb-elem-bits | expand |
On Tue, 13 Feb 2024 08:05:17 -0800, Mao Jinlong wrote: > Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is > bit. > > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > --- > .../devicetree/bindings/arm/qcom,coresight-tpdm.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > 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/arm/qcom,coresight-tpdm.yaml: properties:qcom,dsb-element-bits: '$ref' should not be valid under {'const': '$ref'} hint: Standard unit suffix properties don't need a type $ref from schema $id: http://devicetree.org/meta-schemas/core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.example.dtb: tpdm@684c000: qcom,dsb-element-bits:0: [0, 0, 0, 32] is too long from schema $id: http://devicetree.org/schemas/arm/qcom,coresight-tpdm.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.example.dtb: tpdm@684c000: qcom,dsb-element-bits:0:0: 0 is not one of [32, 64] from schema $id: http://devicetree.org/schemas/arm/qcom,coresight-tpdm.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.example.dtb: tpdm@684c000: qcom,dsb-element-bits:0: [0, 0, 0, 32] is too long from schema $id: http://devicetree.org/schemas/arm/qcom,coresight-tpdm.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.example.dtb: tpdm@684c000: qcom,dsb-element-bits: size is 8, expected 32 from schema $id: http://devicetree.org/schemas/property-units.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240213160521.15925-2-quic_jinlmao@quicinc.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.
On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: > Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is > bit. That may be, but this is an ABI and you are stuck with it. Unless, you can justify why that doesn't matter. (IIRC, this is new, so maybe no users yet?)
On 2/14/2024 6:29 AM, Rob Herring wrote: > On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: >> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is >> bit. > That may be, but this is an ABI and you are stuck with it. Unless, you > can justify why that doesn't matter. (IIRC, this is new, so maybe no > users yet?) Hi Rob, Because for CMB type, it uses qcom,cmb-element-bits. So I change the format to be the same as CMB. Thanks Jinlong Mao
On 14/02/2024 01:43, Jinlong Mao wrote: > > On 2/14/2024 6:29 AM, Rob Herring wrote: >> On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: >>> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is >>> bit. >> That may be, but this is an ABI and you are stuck with it. Unless, you >> can justify why that doesn't matter. (IIRC, this is new, so maybe no >> users yet?) > > Hi Rob, > > Because for CMB type, it uses qcom,cmb-element-bits. So I change the > format to be the same as > CMB. > > Thanks > Jinlong Mao > I think what Rob was trying to say was that in the interest of not breaking existing DTs it's best to leave the existing names as they are, even if they aren't technically correct. And to only add new parameters with the -bits suffix, even if it's inconsistent with what's already there.
On 2/14/2024 5:36 PM, James Clark wrote: > > On 14/02/2024 01:43, Jinlong Mao wrote: >> On 2/14/2024 6:29 AM, Rob Herring wrote: >>> On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: >>>> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is >>>> bit. >>> That may be, but this is an ABI and you are stuck with it. Unless, you >>> can justify why that doesn't matter. (IIRC, this is new, so maybe no >>> users yet?) >> Hi Rob, >> >> Because for CMB type, it uses qcom,cmb-element-bits. So I change the >> format to be the same as >> CMB. >> >> Thanks >> Jinlong Mao >> > I think what Rob was trying to say was that in the interest of not > breaking existing DTs it's best to leave the existing names as they are, > even if they aren't technically correct. And to only add new parameters > with the -bits suffix, even if it's inconsistent with what's already there. Hi Rob & James, There is no tpdm nodes in any DT as of now. So I want to make this change before any tpdm node is added in DT. Thanks Jinlong Mao > > _______________________________________________ > CoreSight mailing list -- coresight@lists.linaro.org > To unsubscribe send an email to coresight-leave@lists.linaro.org
On Wed, Feb 14, 2024 at 8:18 AM Jinlong Mao <quic_jinlmao@quicinc.com> wrote: > > > On 2/14/2024 5:36 PM, James Clark wrote: > > > > On 14/02/2024 01:43, Jinlong Mao wrote: > >> On 2/14/2024 6:29 AM, Rob Herring wrote: > >>> On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: > >>>> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is > >>>> bit. > >>> That may be, but this is an ABI and you are stuck with it. Unless, you > >>> can justify why that doesn't matter. (IIRC, this is new, so maybe no > >>> users yet?) > >> Hi Rob, > >> > >> Because for CMB type, it uses qcom,cmb-element-bits. So I change the > >> format to be the same as > >> CMB. > >> > >> Thanks > >> Jinlong Mao > >> > > I think what Rob was trying to say was that in the interest of not > > breaking existing DTs it's best to leave the existing names as they are, > > even if they aren't technically correct. And to only add new parameters > > with the -bits suffix, even if it's inconsistent with what's already there. > > Hi Rob & James, > > There is no tpdm nodes in any DT as of now. So I want to make this > change before any tpdm > node is added in DT. Then the commit msg needs to state that detail. Rob
On 13/02/2024 22:29, Rob Herring wrote: > On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: >> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is >> bit. > > That may be, but this is an ABI and you are stuck with it. Unless, you > can justify why that doesn't matter. (IIRC, this is new, so maybe no > users yet?) This was added and support queued in v6.8. This change won't make it to v6.8 (given it has to go via two levels and is technically not a fix). As James also pointed out, it doesn't matter what the name is (now that it has been published). Suzuki
On Wed, Feb 14, 2024 at 9:56 AM Suzuki K Poulose <suzuki.poulose@arm.com> wrote: > > On 13/02/2024 22:29, Rob Herring wrote: > > On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: > >> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is > >> bit. > > > > That may be, but this is an ABI and you are stuck with it. Unless, you > > can justify why that doesn't matter. (IIRC, this is new, so maybe no > > users yet?) > > This was added and support queued in v6.8. This change won't make it to > v6.8 (given it has to go via two levels and is technically not a fix). I'd argue it is a fix. But given no users yet, delaying is fine. > As James also pointed out, it doesn't matter what the name is (now that > it has been published). v6.8 final is what we consider published. Rob
On 14/02/2024 16:03, Rob Herring wrote: > On Wed, Feb 14, 2024 at 9:56 AM Suzuki K Poulose <suzuki.poulose@arm.com> wrote: >> >> On 13/02/2024 22:29, Rob Herring wrote: >>> On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: >>>> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is >>>> bit. >>> >>> That may be, but this is an ABI and you are stuck with it. Unless, you >>> can justify why that doesn't matter. (IIRC, this is new, so maybe no >>> users yet?) >> >> This was added and support queued in v6.8. This change won't make it to >> v6.8 (given it has to go via two levels and is technically not a fix). > > I'd argue it is a fix. But given no users yet, delaying is fine. I agree it is a fix, but not something that maintainers would like to pull it during an rc cycle. As you said, since there are no real users for this yet (and given it is all under a single vendor), it may be fine to queue this if the DT maintainers are OK with this. > >> As James also pointed out, it doesn't matter what the name is (now that >> it has been published). > > v6.8 final is what we consider published. I can't send this to Greg as a fix. For v6.8. We can fix it for v6.9 cycle. Suzuki > > Rob
On 2/15/2024 12:18 AM, Suzuki K Poulose wrote: > On 14/02/2024 16:03, Rob Herring wrote: >> On Wed, Feb 14, 2024 at 9:56 AM Suzuki K Poulose >> <suzuki.poulose@arm.com> wrote: >>> >>> On 13/02/2024 22:29, Rob Herring wrote: >>>> On Tue, Feb 13, 2024 at 08:05:17AM -0800, Mao Jinlong wrote: >>>>> Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is >>>>> bit. >>>> >>>> That may be, but this is an ABI and you are stuck with it. Unless, you >>>> can justify why that doesn't matter. (IIRC, this is new, so maybe no >>>> users yet?) >>> >>> This was added and support queued in v6.8. This change won't make it to >>> v6.8 (given it has to go via two levels and is technically not a fix). >> >> I'd argue it is a fix. But given no users yet, delaying is fine. > > I agree it is a fix, but not something that maintainers would like to > pull it during an rc cycle. As you said, since there are no real users > for this yet (and given it is all under a single vendor), it may be fine > to queue this if the DT maintainers are OK with this. > > >> >>> As James also pointed out, it doesn't matter what the name is (now that >>> it has been published). >> >> v6.8 final is what we consider published. > > I can't send this to Greg as a fix. For v6.8. We can fix it for v6.9 > cycle. > > Suzuki > Thanks all for the comments. I will update the commit message and fix the warning. Thanks Jinlong Mao >> >> Rob >
diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml index d0647ffaed71..62188c51ceb5 100644 --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml @@ -44,7 +44,7 @@ properties: minItems: 1 maxItems: 2 - qcom,dsb-element-size: + qcom,dsb-element-bits: description: Specifies the DSB(Discrete Single Bit) element size supported by the monitor. The associated aggregator will read this size before it @@ -111,7 +111,7 @@ examples: compatible = "qcom,coresight-tpdm", "arm,primecell"; reg = <0x0684c000 0x1000>; - qcom,dsb-element-size = /bits/ 8 <32>; + qcom,dsb-element-bits = <32>; qcom,dsb-msrs-num = <16>; clocks = <&aoss_qmp>;
Change qcom,dsb-element-size to qcom,dsb-element-bits as the unit is bit. Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> --- .../devicetree/bindings/arm/qcom,coresight-tpdm.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)