Tms570 support #1
Conversation
hw/arm/hercules.c
Outdated
| qdev_prop_set_drive(DEVICE(dev), "eeprom", | ||
| eeprom ? blk_by_legacy_dinfo(eeprom) : NULL, | ||
| &error_abort); | ||
| object_property_set_bool(dev, is_tms570, "is_tms570", &error_fatal); |
There was a problem hiding this comment.
regarding instance_init vs realize, is this appropriate?
There was a problem hiding this comment.
I think so, but I am also sure we'd get a lecture from upstream if it isn't :-) I don't know why, but for some reason I think property names are supposed to use - instead of _, so we might want to call it is-tms570
There was a problem hiding this comment.
Figured out we can just use object_new_with_props instead
| /* | ||
| * FIXME: Is this the best way to deal with endianness RM57 vs | ||
| * TMS570 | ||
| */ |
There was a problem hiding this comment.
@kamathba we should tests this one on RM57
| /* | ||
| * Current assumptions in this MIBSPI implementation | ||
| * | ||
| * - Only MIBSPI mode is implemented, no compatability mode |
There was a problem hiding this comment.
I don't think this is true anymore. I think I had to implement compatibility mode too. Right?
There was a problem hiding this comment.
Yea, we use it on mibspi5 + dma
| static void hercules_spi_ram_write(HerculesMibSpiState *s, uint32_t *word, | ||
| uint32_t value) | ||
| { | ||
| if (hercules_spi_ram_big_endian(s)) { |
There was a problem hiding this comment.
Another spot that we need to check on RM57
There was a problem hiding this comment.
yea... I dug into this a big more and will comment separately
There was a problem hiding this comment.
I at least checked that spi transfers complete in an expected way without the external chardev doing much (you can setup/start transfers)
| { | ||
| HerculesMibSpiState *s = opaque; | ||
|
|
||
| /* FIXME: This assumes BE */ |
There was a problem hiding this comment.
Oh god, if only I could remember what this meant
kamathba
force-pushed
the
tms570-support
branch
6 times, most recently
from
753ee68 to
727b746
Compare
|
So, the BE/LE problem with the hercules peripherals is pretty irritating. nothing outside of hw/arm/* has include access to target/arm/ (for cpu.h). I think the way to do this is make "cpu" opaque in HerculesState, and like you said: check the parent object or global CPU objects endianness to set the MemoryRegionOps. I hate it. |
Change bswap_code()'s parameter to CPUARMState * in order to accomodate upcoming changes. No functional change intended. Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
ARMv7-R can support legacy BE32 code, which can be configured by SoC vendor via SCTLR.IE bit. Add various bit and pieces needed to emulate that feature. Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
We need to swap endiannes of register value data sent to GDB when SCTLR.IE is set, since GDB will be running in BE32 and all of the instruction memory is going to be big-endian, so it'll expect register data to be as well. Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Add for_each_set_bit macro as found in Linux kernel Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]> Signed-off-by: Benjamin Kamath <[email protected]>
Can probably make this a helper function to reduce some boilerplate. What I would really want to do is allocate the MemoryRegionOps and have it return a 'const' version, but that should probably happen in init rather than realize...
Signed-off-by: Benjamin Kamath <[email protected]>
| }; | ||
|
|
||
| if (parent->is_tms570) | ||
| { |
There was a problem hiding this comment.
Dumbest of nits: here and in other if statements { is on a new line for some reason. Should be on the same line per coding style.
Review patchset