2024-02-13 16:06:47

by Mao Jinlong

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

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



2024-02-13 16:06:57

by Mao Jinlong

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

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

Signed-off-by: Mao Jinlong <[email protected]>
---
.../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


2024-02-13 16:08:07

by Mao Jinlong

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

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

Signed-off-by: Mao Jinlong <[email protected]>
---
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


2024-02-13 18:21:27

by Rob Herring

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


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 <[email protected]>
> ---
> .../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/[email protected]

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.


2024-02-13 22:55:33

by Rob Herring

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

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

2024-02-14 01:43:59

by Mao Jinlong

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


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


2024-02-14 09:36:49

by James Clark

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



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.


2024-02-14 14:19:02

by Mao Jinlong

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


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 -- [email protected]
> To unsubscribe send an email to [email protected]

2024-02-14 15:34:11

by Rob Herring

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

On Wed, Feb 14, 2024 at 8:18 AM Jinlong Mao <[email protected]> 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

2024-02-14 15:57:51

by Suzuki K Poulose

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

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


2024-02-14 16:04:09

by Rob Herring

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

On Wed, Feb 14, 2024 at 9:56 AM Suzuki K Poulose <[email protected]> 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

2024-02-14 16:42:08

by Suzuki K Poulose

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

On 14/02/2024 16:03, Rob Herring wrote:
> On Wed, Feb 14, 2024 at 9:56 AM Suzuki K Poulose <[email protected]> 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


2024-02-15 02:18:17

by Mao Jinlong

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


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
>> <[email protected]> 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
>