Add cpu device PIC16x8x - Add PIC16F84 to magicle into misc/magicard.cpp #12092
Conversation
| , m_data_config("data", ENDIANNESS_LITTLE, 8, 8, 0, | ||
| ((pic_model == 0x83) ? address_map_constructor(FUNC(pic16x8x_device::ram_6), this) : | ||
| address_map_constructor(FUNC(pic16x8x_device::ram_7), this))) |
There was a problem hiding this comment.
It's probably better just to pass the address map constructor here and eliminate the artificial pic_model constants.
There was a problem hiding this comment.
Done. Deleted pic_model and and use program_width as selector.
There was a problem hiding this comment.
Second revision. Fixed.
Added new base classes and instances for 16x83 and 16x84 to define properly the address maps.
| // Get default NVRAM data from memory region | ||
| memcpy(m_buff, m_region->base() + eeprom_dump, m_internal_eeprom_size * 2); | ||
|
|
||
| // Data width conversion | ||
| for (u8 i=0; i < m_internal_eeprom_size; i++) | ||
| m_eeprom_data[i] = m_buff[i] & 0x00ff; |
There was a problem hiding this comment.
This code doesn't seem endian-safe.
There was a problem hiding this comment.
Since the rom load of this region is (and "must be") "ROM_REGION16_LE( 0x4280, "pic16f84", 0 )", I understand that the proposed code is correct.
The origin and destination data have been duly verified.
The memcpy() function loads u16 type data into m_buff from m_region, which in turn
have been read from the source in little_endian mode, therefore the conversion loop and its corresponding mask are correct.
Even so, if you consider that the code requires some change, I am completely willing to make it.
| #define RBPU_FLAG 0x80 // RBPU PORTB Pull-up Enable bit | ||
| #define INTEDG_FLAG 0x40 // INTEDG Interrupt Edge Select bit | ||
| // 1 = Interrupt on rising edge of RB0/INT pin | ||
| // 0 = Interrupt on falling edge of RB0/INT pin | ||
| #define T0CS_FLAG 0x20 // T0CS Timer 0 clock source select | ||
| // 1 = Transition on RA4/T0CKI pin | ||
| // 0 = Internal instruction cycle clock (CLKOUT) | ||
| #define T0SE_FLAG 0x10 // T0SE Timer 0 clock source edge select | ||
| // 1 = Increment on high-to-low transition on RA4/T0CKI pin | ||
| // 0 = Increment on low-to-high transition on RA4/T0CKI pin | ||
| #define PSA_FLAG 0x08 // PSA Prescaler Assignment bit | ||
| // 1 = Prescaler is assigned to the WDT | ||
| // 0 = Prescaler is assigned to the Timer0 module | ||
| #define PS_REG 0x07 // PS Prescaler Rate select |
There was a problem hiding this comment.
It's generally recommended to use constexpr instead of #define for constants in new code.
| if(ADDR == 0x0b) | ||
| logerror("ANDWF -> INTCON - data:%x\n", m_ALU); |
There was a problem hiding this comment.
This seems to be a leftover from debugging. If you're running MAME with -debug, you can use wpdset 0x0b, 1, w to catch writes to this register.
There was a problem hiding this comment.
You're right. Will delete this and other two following logerrors calls.
There was a problem hiding this comment.
Done. Deleted all leftovers.
| m_STACK[m_stack_pointer] = data & m_program_mask; | ||
| m_stack_pointer++; |
There was a problem hiding this comment.
It's better to check for stack overflow here. (I haven't double-checked the datasheet for this microcontroller, but PIC17 does recognize stack overflows, though it doesn't handle them very gracefully.)
There was a problem hiding this comment.
will check the datasheet
There was a problem hiding this comment.
Done. Added some datasheet notes and add stack overflow check.
Unify EEPROM size. Delete model selector.
Unify EEPROM size. Deleted pic model selector. Deleted all logerror() leftover Changed #defines by constexpr Fixed and add datashhet notes about stack overflow control
Added new base classes and instances for 16x83 and 16x84 to define properly the address maps.
Added new base classes and instances for 16x83 and 16x84 to define properly the address maps.
|
Hi @ajrhacker. |
Add cpu device PIC16x8x
Add PIC16F84 to magicle game into misc/magicard.cpp