[PATCH v1 0/2] coresight: tpdm: Change qcom,dsb-element-size to qcom,dsb-elem-bits

[PATCH v1 0/2] coresight: tpdm: Change qcom,dsb-element-size to qcom,dsb-elem-bits

[Mao Jinlong]

As unit of dsb element size is bit, change qcom,dsb-element-size to
qcom,dsb-elem-bits.

Mao Jinlong (2):
  dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size
  coresight-tpda: Change qcom,dsb-element-size to qcom,dsb-elem-bits

 .../devicetree/bindings/arm/qcom,coresight-tpdm.yaml          | 4 ++--
 drivers/hwtracing/coresight/coresight-tpda.c                  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.41.0

[PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Mao Jinlong]

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(-)

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>;
-- 
2.41.0

[PATCH v1 2/2] coresight-tpda: Change qcom,dsb-element-size to qcom,dsb-elem-bits

[Mao Jinlong]

Change qcom,dsb-element-size to qcom,dsb-elem-bits as the unit is bit.

Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
---
 drivers/hwtracing/coresight/coresight-tpda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 6863afe7ca94..5d20e10be24b 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -70,7 +70,7 @@ static int tpdm_read_element_size(struct tpda_drvdata *drvdata,
 
 	if (tpdm_has_dsb_dataset(tpdm_data)) {
 		rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
-				"qcom,dsb-element-size", &drvdata->dsb_esize);
+				"qcom,dsb-element-bits", &drvdata->dsb_esize);
 	}
 	if (tpdm_has_cmb_dataset(tpdm_data)) {
 		rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
-- 
2.41.0

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Rob Herring]

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.

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Rob Herring]

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?)

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Jinlong Mao]

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

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[James Clark]

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.

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Jinlong Mao]

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

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Rob Herring]

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

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Suzuki K Poulose]

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

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Rob Herring]

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

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Suzuki K Poulose]

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

Re: [PATCH v1 1/2] dt-bindings: arm: qcom,coresight-tpdm: Rename qcom,dsb-element-size

[Jinlong Mao]

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
>