Hi Al,
Hi Al and Leo,
Hi Al,
On Tue, Aug 24, 2021 at 02:55:07PM +0000, Al Grant wrote:
[...]
> > > > 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.
[...]
we can add an optional property like below:
Optional property for TMC:
- arm,burst-size: burst size initiated by the TMC on the AXI
master
interface. The burst size can be in the range [0..15], thesetting
supports one data transfer per burst to maximum of 16 data transfers per burst. If don't set this property, the driver will set to 15 (16 data transfers per burst) as defaultvalue.
I don't think it's a right way to use CoreSight ROM to read out part
number and
producer number, and based on these numbers to set burst size. The main reason is this will add SoC specific code into the driver, and DT
binding is just
used to decouple the platform specific with the driver code, so with the
DT
binding, the driver works as common as possible.
Using errata workaround also is not the right thing to do. Based on the
TMC
spec (ARM DDI 0461B), the burst size on AXI bus could be different on
different
SoCs
Hi Leo, where are you reading that this is a property of the AXI bus?
No, I cannot find any description in TMC spec (ARM DDI 0461B) to say AXICTL.WrBurstLen is a property of the AXI bus.
After read the register AXICTL (DDI 0461B 3.3.15), I mixed the concepts, AXICTL.WrBurstLen is an attribution for TMC to initiate the max number of data transfers, it's not a property for AXI bus itself.
Sorry I introduced confusion.
There is a constraint that the burst size must not be greater than the TMC buffer size (see DDI 0461B 3.3.1), but buffer size is indicated by the TMC's MEMWIDTH register and the driver can determine
that.
DEVID.MEMWIDTH: The width of the memory interface databus for ETB/ETF, DEVID.MEMWIDT = 2 * (ATB datawidth) for ETR, DEVID.MEMWIDT = ATB datawidth
AXICTL.WrBurstLen: The maximum number of data transfer that can occur within each burst.
RSZ.RSZ: The size of the local RAM buffer (in 32-bit words).
And there have requirement for burst size and TMC buffer size:
2 ^ (DEVID.MEMWIDTH - 2) * AXICTL.WrBurstLen <= RSZ.RSZ
The situation I thought we were dealing with, is where the TMC presents a burst that the AXI spec says is valid but the downstream component in this SoC can't accept a burst of that size. Our understanding is that this would be a non-compliance in the downstream component - i.e. failure to handle a valid AXI transaction - and it would be appropriate to treat this as a SoC-specific issue triggering a SoC-
specific
workaround. (The alternative would be to represent the AXI topology in DT/ACPI and then drive the workaround from knowing what the downstream component is.)
Okay, I see, the downstream component (e.g. memory controller) has the hardware constraint, it imposes the limitation back on TMC burst size.
The downstream can be a memory controller, or any other component. I understand your suggestion that use DT device node to bind TMC and downstream device, and based on the property in the downstream device we can get to know the constriant for burst size.
But here it's hard to unify property for downstream components; and I checked a bit for the latest Device tree spec, if we create the binding between TMC device and a memory node ('memory' is a general node in DT binding), the 'memory' node doesn't provide property for
burst size.
So seems to me, a feasible (and simple) way is still to add a property for TMC, since the TMC is connected to its downstream component through AXI bus, the developer for a platform has the knowledge for the constraint for the max burst size.
Optional property for TMC:
- arm,max-burst-size: the max burst size initiated by the TMC on the AXI master interface. The burst size can be in the range [0..15], the setting supports one data transfer per burst to maximum of 16
data
transfers per burst. When a platform has specific constriant for the maximum burstsize (e.g. caused by its downstream component), it can set this property for
the
maximum burst size; if this property isn't set, the driver willset to
15 (16 data transfers per burst) as default value.Hope I am not arbitrary for this, so I am curious what's opinions from others. any thoughts?
Thanks for providing all the information and ways to fix this issue.
Based on the suggestions, would it be okay if we introduce the optional DT property as "mrvl,max-burst-size" since it's a SoC specific setting ?
I would suggest, if we're adding a DT property that the TMC driver would look at in all cases, we should go with "arm,max-burst-size" as Leo suggested. I.e. as TMC driver functionality, the ability to override the burst size using a DT property wouldn't be SoC-specific or vendor-specific in any way - even though the motivation for introducing it is to work with a specific SoC and we expect it to only be needed on specific SoCs.
Then, if any other vendor's SoC needs to do the same thing, or even if someone wants to play with burst size as a performance tweak, they can use the same DT property.
Okay sure, I will send a patch for these changes using "arm,max-burst-size" DT binding.
Thanks, Tanmay
Al