ot_earlgrey: add JTAG support to LC Ctrl
#274
Conversation
luismarques
force-pushed
the
lc_ctrl_jtag
branch
from
310b170 to
b408a44
Compare
| }; | ||
|
|
||
| #define OT_EG_DEBUG_LC_CTRL_ADDR 0x0u | ||
| #define OT_EG_DEBUG_LC_CTRL_SIZE 0x200u |
There was a problem hiding this comment.
Where is this size defined? I can't find a source on earlgrey_1.0.0 but on master this seems like it should be 0x1000 instead?
There was a problem hiding this comment.
Since the bus is private, I just used the maximum allowed size, given the abits=7 (2^7 * 4 == 0x200, with the 4 being for the 1:4 mapping adjustment). That way we can keep the actual size needed known only inside the ot_lc_ctrl.
There was a problem hiding this comment.
It might be worth adding a comment to keep track of this, and/or use a defined constant for the bit width (7) and a macro based on it to compute the actual range?
Signed-off-by: Luís Marques <[email protected]>
luismarques
force-pushed
the
lc_ctrl_jtag
branch
from
b408a44 to
d0e0d84
Compare
| }; | ||
|
|
||
| #define OT_EG_DEBUG_LC_CTRL_ADDR 0x0u | ||
| #define OT_EG_DEBUG_LC_CTRL_SIZE 0x200u |
There was a problem hiding this comment.
It might be worth adding a comment to keep track of this, and/or use a defined constant for the bit width (7) and a macro based on it to compute the actual range?
|
|
||
| #define OT_EG_DEBUG_LC_CTRL_ADDR 0x0u | ||
| #define OT_EG_DEBUG_LC_CTRL_SIZE 0x200u | ||
| #define OT_EG_DEBUG_LC_CTRL_DMI_ADDR (OT_EG_DEBUG_LC_CTRL_ADDR / 4) |
There was a problem hiding this comment.
nit pick: either 4u or sizeof(uint32_t)
| OT_EG_SOC_DEVLINK("tl_dev", LC_CTRL) | ||
| ), | ||
| .prop = IBEXDEVICEPROPDEFS( | ||
| IBEX_DEV_UINT_PROP("dmi_addr", OT_EG_DEBUG_LC_CTRL_DMI_ADDR), |
There was a problem hiding this comment.
nit pick: maybe OT_EG_LC_CTRL_DMI_ADDR to match OT_EG_LC_CTRL_MEMORY_REGION? Dmi already stands for DEBUG.
| OT_EG_LC_CTRL_MEMORY_REGION)); | ||
| Object *oas; | ||
| AddressSpace *as = g_new0(AddressSpace, 1u); | ||
| address_space_init(as, lc_ctrl_mr, "lc-ctrl.as"); |
There was a problem hiding this comment.
Maybe add a comment about the role of this new AS?
| AddressSpace *as = g_new0(AddressSpace, 1u); | ||
| address_space_init(as, lc_ctrl_mr, "lc-ctrl.as"); | ||
| oas = object_new(TYPE_OT_ADDRESS_SPACE); | ||
| object_property_add_child(OBJECT(dev), "ot-lc-ctrl-dmi-as", oas); |
There was a problem hiding this comment.
You could use object_property_add_child(OBJECT(dev), as->name, oas);
for the property name. It is probably easier when debugging to have the AS refered by its name.
| ibex_map_devices(s->devices, mrs, ot_eg_soc_devices, | ||
| ARRAY_SIZE(ot_eg_soc_devices)); | ||
| MemoryRegion *lc_ctrl_mr = g_new0(MemoryRegion, 1u); | ||
| memory_region_init(lc_ctrl_mr, OBJECT(dev), "lc-ctrl-dmi.xbar", |
There was a problem hiding this comment.
Maybe try to use the same radix for all related naming, i.e. "lc-ctrl.xbar", or better:
TYPE_OT_LC_CTRL ".xbar"
TYPE_OT_LC_CTRL ".as"
// see note in another comment about as->name
IBEX_DEV_STRING_PROP("tl_as_name", TYPE_OT_LC_CTRL ".as")
| IBEX_DEV_UINT_PROP("dmi_addr", OT_EG_DEBUG_LC_CTRL_DMI_ADDR), | ||
| IBEX_DEV_UINT_PROP("dmi_size", OT_EG_DEBUG_LC_CTRL_DMI_SIZE), | ||
| IBEX_DEV_UINT_PROP("tl_addr", OT_EG_DEBUG_LC_CTRL_ADDR), | ||
| IBEX_DEV_STRING_PROP("tl_as_name", "ot-lc-ctrl-dmi-as") |
There was a problem hiding this comment.
Nit: this name "ot-lc-ctrl-dmi-as" is repeated as a hardcoded string, maybe extract it out to a:
#define LC_CTRL_ADDR_SPACE "ot-lc-ctrl-dmi-as"or equivalent?
3 participants
No description provided.