On Mon, Aug 23, 2021 at 09:44:51AM +0000, Al Grant wrote:
Hi Al and Mike,
16 (0xF) should work for all silicon, as AXI allows burst sizes up to 16. So unless we've missed something, this is an implementation non-compliance and we should not be penalising compliant implementations by reducing the default burst size - the question is how we can enable the upstream kernel
to
workaround the issue on this chip only, and to me that sounds like it
needs
something that can be triggered by a setting in DT/ACPI.
Another possibility would be to introduce an errata workaround in Kconfig for your silicon. There are a number of these already in KConfig for PE issues e.g. CONFIG_ARM64_ERRATUM_826319, and we have introduced CONFIG_ETM4X_IMPDEF_FEATURE for silicon specific variants in the ETMv4 driver.
The latter config compiles in implementation defined workarounds which operate on the basis of matching the AMBA ID for the silicon. This means they will operate only if configured in KConfig and only on silicon where the workaround is needed.
Thanks a lot for the suggestions.
We are thinking of using "part-number" and "designer" rather than device tree property. While we use standard ARM core IP and Coresight SoC-600 IP, we cannot differentiate our silicon from others using ETR AMBA ID, PIDR and CPU MIDR registers.
We are proposing to expose Coresight ROM region to the driver and determine part number and designer by reading the following fields. part_number = (PIDR1.PART_1 << 8) | PIDR0.PART_0; designer = ((PIDR4.DES_2 << 7) & 0xf) | ((PIDR2.DES_1 << 4) & 0x7) | ((PIDR1.DES_0 & 0xf));
Using a combination of part number and designer from ROM region would help in identifying the Marvell implementation.
This option would be generic and could be helpful for other silicon with similar issues and can be applied across Coresight components like ETF/ETR.
What are your thoughts on this approach ?
It's a valid approach if you can locate the CoreSight ROM region. Some CPUs don't set the CPU DBGDRAR (ROM region base address) register, so as a generic mechanism, there would need to be some alternative way of locating the ROM region, e.g. through DT or ACPI.
Sorry for sudden jumping into this topic, since I have some experience for DT binding, I agree with Al that DT (or ACPI) is the right way to move forward.
You could see there have some examples for DT binding for DMA controller and memory controller for burst size, e.g.
in Documentation/devicetree/bindings/dma/arm-pl08x.yaml:
memcpy-burst-size: $ref: /schemas/types.yaml#/definitions/uint32 enum: - 1 - 4 - 8 - 16 - 32 - 64 - 128 - 256 description: the size of the bursts for memcpy
Just remind, DT binding is not used to set any specific register value, on the other hand, DT property is to describe the hardware character. So I think it's reasonable to add a new property to describe burst size for sink, if DT binding has no the property, then driver will fallback to use the default value '16'.
Thanks, Leo