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.

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

provide an enum for registers #46

Closed
Sauci opened this issue Dec 11, 2018 · 10 comments
Closed

provide an enum for registers #46

Sauci opened this issue Dec 11, 2018 · 10 comments

Comments

@Sauci
Copy link
Contributor

Sauci commented Dec 11, 2018

Hi,

I would like to know if it would be possible to provide an enumeration to clarify the calls to register_read(self, register_index). I guess that calling this function with register_index = 0 the value of register r0 will be returned, but I'm not sure what index should be used for CPS register, more generally what index should be used for banked registers. I guess that an enum like shown bellow might help the users:

REG_R0 = 0
...
REG_R7 = 7
REG_R8_USR = ?
REG_R8_FIQ = ?
REG_R9_USR = ?
REG_R9_FIQ = ?

I know it is possible to read the banked register r8_fiq with GDB, even if the controller is not in FIQ mode, but I don't know if it would be possible using the DLL.

here's a link showing different registers available for a cortex-r5 controller (only as example).

Thanks for your feedback

@hkpeprah
Copy link
Contributor

Sounds like a good idea / enhancement. Something like a registers enum with

class Registers(enum):
    ARM_CORTEX_R5_R0 = 0
    <...>
    ARM_CORTEX_R5_CPS

@sstallion
Copy link
Contributor

sstallion commented Dec 11, 2018 via email

@hkpeprah
Copy link
Contributor

There's the register_name() method which takes an index and returns the name of the register at that index. There's also register_list() which will return a list of the register numbers. Together you could probably do something like:

register_map = {}
for reg_index in jlink.register_list():
    register_map[jlink.register_name(reg_index)] = reg_index
print(jlink.register_read(register_map['<REGISTER_NAME>']))

@Sauci
Copy link
Contributor Author

Sauci commented Dec 11, 2018

Oh sorry I didn’t see those two functions...
I guess my proposal doesn’t make sense anymore, as the registers are known at runtime. I was thinking that it would be great for auto completion, but it doesn’t seems to be possible.
Should I close the issue?

Sorry again, RTFD the next time

@Sauci Sauci closed this as completed Sep 9, 2019
@hkpeprah
Copy link
Contributor

hkpeprah commented Sep 9, 2019

Ah, super sorry about getting back to you late on this. I think probably using the list is the best bet. It might be worth modifying the register commands to support both named and indexed registers if that's something you want to take a look at.

@Sauci
Copy link
Contributor Author

Sauci commented Sep 11, 2019

No problem. Ok, I will add this feature. What do you think about this proposition:

  • modifying register_read(self, register_index) signature to
    register_read(self, register_index=None, register_name=None)
    This change would be backward compatible.

  • modifying register_write(self, reg_index, value) signature to
    register_write(self, value, reg_index=None, register_name=None)
    This change wouldn't be backward compatible.

This has the downside to break backward compatibility with current/previous versions of the module, but it keeps a clear interface. Otherwise, if you prefer to keep backward compatibility, I could add functions like read_register_by_name and write_register_by_name.

@Sauci Sauci reopened this Sep 11, 2019
@hkpeprah
Copy link
Contributor

Hm, personally, I would just override register_index and update the docstring to indicate that we take a string value. In a later (major) release, we can rename register_index -> register. Basically, just do something like:

if isinstance(register_index, six.stringtypes):
    register_index = <convert name to index>

hkpeprah pushed a commit that referenced this issue Sep 23, 2019
…ices (#59)

@Sauci: Add support for register methods to accept register names in addition to indices. (#46)
@hkpeprah
Copy link
Contributor

Release with patch should be available in v0.3.0: https://github.com/square/pylink/releases/tag/v0.3.0

@Sauci
Copy link
Contributor Author

Sauci commented Sep 24, 2019

Great, thanks! I guess this issue can be closed?

@hkpeprah
Copy link
Contributor

Sounds good.

hkpeprah pushed a commit that referenced this issue Nov 22, 2019
* refactor register_read method (#46).


refactor register_read_multiple method (#46).


refactor register_write method (#46).


refactor register_write_multiple method (#46).

* refactor according to @hkpeprah review.

* add docstring to _get_register_index_from_name method.

* prevent side effects due to mutability of list.

* add unit tests.

* implement cp15_present function.

* implement cp15_register_read function.

* add test for cp15_present function

* implement cp15_register_write_function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants