On 20/02/2024 16:11, Mark Brown wrote:
> On Tue, Feb 20, 2024 at 10:09:13AM +0000, James Clark wrote:
>> Add separate definitions for ELx and EL2 as TRFCR_EL1 doesn't have CX.
>> This also mirrors the previous definition so no code change is required.
>
> This is also converting to automatic generation in the process.
>
>> +SysregFields TRFCR_EL2
>> +Res0 63:7
>> +UnsignedEnum 6:5 TS
>> + 0b0000 USE_TRFCR_EL1_TS
>> + 0b0001 VIRTUAL
>> + 0b0010 GUEST_PHYSICAL
>> + 0b0011 PHYSICAL
>> +EndEnum
>> +Res0 4
>> +Field 3 CX
>> +Res0 2
>> +Field 1 E2TRE
>> +Field 0 E0HTRE
>> +EndSysregFields
>
> This has exactly one user and I'd not expect more so why have a separate
> SysregFields?
>
No reason, probably just a copy paste thing. I'll change it to a Sysreg.
>> +# TRFCR_EL1 doesn't have the CX bit so redefine it without CX instead of
>> +# using a shared definition between TRFCR_EL2 and TRFCR_EL1
>
> This comment is reflecting the default state?
>
True, will remove.
>> +Sysreg TRFCR_EL1 3 0 1 2 1
>> +Fields TRFCR_ELx
>> +EndSysreg
>> +
>> +Sysreg TRFCR_EL2 3 4 1 2 1
>> +Fields TRFCR_EL2
>> +EndSysreg
>> +
>> +Sysreg TRFCR_EL12 3 5 1 2 1
>> +Fields TRFCR_ELx
>> +EndSysreg
>
> These are generally sorted by encoding (simiarly to how sysreg.h was
> sorted historically).
Ah I didn't know that. Can I add a comment to the top of the file saying
that it should be kept sorted?
As unit of dsb element size is bit, change qcom,dsb-element-size to
qcom,dsb-elem-bits. And CMB uses -bits suffix as well. There is no
TPDM node in any DT now. Make such change before any TPDM node is added
to DT.
Change since V1:
1. Update the commit message for dt-binding.
2. Fix the dt_binding_check error for dt-binding change.
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 | 5 ++---
drivers/hwtracing/coresight/coresight-tpda.c | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
--
2.41.0
On 14/02/2024 16:03, Rob Herring wrote:
> On Wed, Feb 14, 2024 at 9:56 AM Suzuki K Poulose <suzuki.poulose(a)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 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 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 13/02/2024 10:45, Arnd Bergmann wrote:
> On Tue, Feb 13, 2024, at 11:22, Suzuki K Poulose wrote:
>
>> coresight: tpdm: Fix build break due to uninitialised field
>>
>> {CMB/DSB}_PATT_ENABLE_TS attributes do not use an index field.
>> But some compilers complain for uninitialised "index" field in
>> the struct tpdm_dataset_attribute:
>>
> ...
>>
>> Reported-by: Arnd Bergmann <arnd(a)arndb.de>
>> Cc: Tao Zhang <quic_taozha(a)quicinc.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>
> Works fine here with this, thanks for the fix!
>
> Acked-by: Arnd Bergmann <arnd(a)arndb.de>
>
> I had actually missed that this is a warning option that
> is normally disabled even at W=1 level, but it does get
> added by drivers/hwtracing/coresight/Makefile setting
> -Wextra again because that comes after the
> -Wno-missing-field-initializers from scripts/Makefile.extrawarn.
>
> Arnd
Thanks for verifying, I have queued this.
https://git.kernel.org/coresight/c/c099fdd218a0
Suzuki
On 13/02/2024 10:45, Arnd Bergmann wrote:
> On Tue, Feb 13, 2024, at 11:22, Suzuki K Poulose wrote:
>
>> coresight: tpdm: Fix build break due to uninitialised field
>>
>> {CMB/DSB}_PATT_ENABLE_TS attributes do not use an index field.
>> But some compilers complain for uninitialised "index" field in
>> the struct tpdm_dataset_attribute:
>>
> ...
>>
>> Reported-by: Arnd Bergmann <arnd(a)arndb.de>
>> Cc: Tao Zhang <quic_taozha(a)quicinc.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
>
> Works fine here with this, thanks for the fix!
>
> Acked-by: Arnd Bergmann <arnd(a)arndb.de>
>
> I had actually missed that this is a warning option that
> is normally disabled even at W=1 level, but it does get
> added by drivers/hwtracing/coresight/Makefile setting
> -Wextra again because that comes after the
> -Wno-missing-field-initializers from scripts/Makefile.extrawarn.
>
> Arnd
Thanks, I will queue this.
Suzuki