Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

coresight: discovery: Avoid double offset in root component creation #1717

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

coresight: discovery: Avoid double offset in root component creation #1717

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyocd/coresight/discovery.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def _create_root_component(self, cmpid):
memif = APAccessMemoryInterface(self.dp, ap_address)

# Instantiate the component and attach to the target.
component = cmpid.factory(memif, cmpid, cmpid.address)
component = cmpid.factory(memif, cmpid, 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tracked down this parametter to be set to CoreSightComponent._address.

So if I understand correctly, this should be set to None instead.

I can see that the AP Address is used as an offset in APAccessMemoryInterface. Could that be the issue ?
Are all read/write operations to this root AP meant to be relative to the base address?

EG, hypothetically, if there’s an SCS right on the root AP, then there could be some read/write in the 0xExxx_xxxx space even if the root’s base address is in 0xFxxx_xxxx, couldn’t it?

Could be please point to where it is applied a second time?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tracked down this parametter to be set to CoreSightComponent._address.

So if I understand correctly, this should be set to None instead.

None makes sense instead of 0 here.

I can see that the AP Address is used as an offset in APAccessMemoryInterface. Could that be the issue ? Are all read/write operations to this root AP meant to be relative to the base address?

EG, hypothetically, if there’s an SCS right on the root AP, then there could be some read/write in the 0xExxx_xxxx space even if the root’s base address is in 0xFxxx_xxxx, couldn’t it?

Could be please point to where it is applied a second time?

I was debugging an issue where a funnel was faulting on access: https://github.com/pyocd/pyOCD/blob/main/pyocd/coresight/funnel.py#L47

Since it's using the instantiated APAccessMemoryInterface which is applying cmpid.address as an offset to all accesses the self.address + TraceFunnel.DEVID results in a double offset.

I don't have enough of a high level view to understand which application of this makes sense. Perhaps instantiating APAccessMemoryIterface w/o an offset (or an offset derived from somewhere else ) makes sense?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's today instead of yesterday I can say that the chip I was debugging was the rp2350 (see #1718). The error ends up being ignored and you can flash, erase, reset the rp2350 fine.

self.target.add_child(component)
component.init()
except exceptions.Error as e:
Expand Down