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

Fix misuses read and write pointers #89

Merged
merged 9 commits into from
Jan 25, 2022
Merged

Fix misuses read and write pointers #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
54517d0
makefile- allow to override TOOLROOT variable
hagaigold Jan 13, 2022
fceb7a3
fix misuse of REG_S3_RX_RD0 (read pointer)
hagaigold Jan 13, 2022
f99bd4a
fix misuse of REG_S3_TX_WR0 (write pointer)
hagaigold Jan 13, 2022
a0eb3af
rename macros
hagaigold Jan 20, 2022
76660e6
simplified tftpPoll
hagaigold Jan 20, 2022
551f890
decrease TFTP_PACKET_DELAY
hagaigold Jan 20, 2022
9459371
Debug Main TITLE- add WIZnet type
hagaigold Jan 20, 2022
b10bb69
refactoring global (extern) variables definition
hagaigold Jan 21, 2022
1695ac0
fix MAX_ADDR (bootloader size)
hagaigold Jan 22, 2022
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
6 changes: 3 additions & 3 deletions avr/bootloaders/athena/src/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ ifeq ($(ENV), arduino)
# For Arduino, we assume that we're connected to the optiboot directory
# included with the arduino distribution, which means that the full set
# of avr-tools are "right up there" in standard places.
TOOLROOT = ../../../tools
TOOLROOT ?= ../../../tools
GCCROOT = $(TOOLROOT)/avr/bin/
AVRDUDE_CONF = -C$(TOOLROOT)/avr/etc/avrdude.conf

Expand All @@ -80,10 +80,10 @@ else ifeq ($(ENV), arduinodev)
# Arduino IDE source code environment. Use the unpacked compilers created
# by the build (you'll need to do "ant build" first.)
ifeq ($(OS), macosx)
TOOLROOT = ../../../../build/macosx/work/Arduino.app/Contents/Resources/Java/hardware/tools
TOOLROOT ?= ../../../../build/macosx/work/Arduino.app/Contents/Resources/Java/hardware/tools
endif
ifeq ($(OS), windows)
TOOLROOT = ../../../../build/windows/work/hardware/tools
TOOLROOT ?= ../../../../build/windows/work/hardware/tools
endif

GCCROOT = $(TOOLROOT)/avr/bin/
Expand Down
28 changes: 18 additions & 10 deletions avr/bootloaders/athena/src/debug/debug_main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,33 @@ const unsigned char mDebugMain_PREFIX[] PROGMEM = "Main: ";
const unsigned char mDebug_UpdateMode[] PROGMEM = "Update Mode Requested";
const unsigned char mDebug_GoodImage[] PROGMEM = "Good image signature detected";
const unsigned char mDebug_BadImage[] PROGMEM = "BAD image signature detected";

#if defined(__AVR_ATmega328__) || defined(__AVR_ATmega328P__)
#if defined(ARDUINO_ETHERNET)
const unsigned char mDebugMain_TITLE[] PROGMEM =
"Athena for Arduino Ethernet, Version " PP_STRINGIFY(BUILD_TAG);
#define TITLE_BOARD_TYPE "Athena for Arduino Ethernet"
#else
const unsigned char mDebugMain_TITLE[] PROGMEM =
"Athena for Arduino Uno, Version " PP_STRINGIFY(BUILD_TAG);
#define TITLE_BOARD_TYPE "Athena for Arduino Uno"
#endif
#elif defined(__AVR_ATmega2560__)
const unsigned char mDebugMain_TITLE[] PROGMEM =
"Athena for Arduino Mega2560, Version " PP_STRINGIFY(BUILD_TAG);
#define TITLE_BOARD_TYPE "Athena for Arduino Mega2560"
#elif defined(__AVR_ATmega1284P__)
const unsigned char mDebugMain_TITLE[] PROGMEM =
"Athena for Arduino Mega1284P, Version " PP_STRINGIFY(BUILD_TAG);
#define TITLE_BOARD_TYPE "Athena for Arduino Mega1284P"
#else
const unsigned char mDebugMain_TITLE[] PROGMEM =
"Unknown MCU with athena, Version " PP_STRINGIFY(BUILD_TAG);
#define TITLE_BOARD_TYPE "Unknown MCU with athena"
#endif

#if defined(__WIZ_W5100__)
#define TITLE_WIZ_TYPE "W5100"
#elif defined(__WIZ_W5200__)
#define TITLE_WIZ_TYPE "W5200"
#elif defined(__WIZ_W5500__)
#define TITLE_WIZ_TYPE "W5500"
#endif

const unsigned char mDebugMain_TITLE[] PROGMEM =
TITLE_BOARD_TYPE " (" TITLE_WIZ_TYPE "), Version " PP_STRINGIFY(BUILD_TAG);
Copy link
Member

Choose a reason for hiding this comment

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

Really like this new approach!

const unsigned char mDebugMain_EXIT[] PROGMEM = "Start user app";

#if(DEBUG_MAIN > 1)
#undef DBG_MAIN_EX
#define DBG_MAIN_EX(block) block
Expand Down
76 changes: 39 additions & 37 deletions avr/bootloaders/athena/src/ethernet/tftp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ const unsigned char tftp_invalid_image_packet[] PROGMEM = "\0\5"
"\0\0"
"Invalid image file";

uint8_t tftpFlashing;
uint8_t tftpInitError;

#ifndef TFTP_RANDOM_PORT
uint16_t tftpTransferPort;
#endif

uint16_t lastPacket = 0, highPacket = 0;

static void sockInit(uint16_t port)
Expand Down Expand Up @@ -117,17 +124,12 @@ static uint8_t processPacket(void)
DBG_BTN(button();))

// Read data from chip to buffer

// Use uint16_t overflow from 0xFFFF to 0x10000 to follow WIZnet internal pointer
readPointer = spiReadWord(REG_S3_RX_RD0, S3_R_CB);

DBG_TFTP_EX(tracePGMlnTftp(mDebugTftp_RPTR); tracenum(readPointer);)

#if defined(__WIZ_W5500__)
// W5500 auto increments the readpointer by memory mapping a 16bit addr
#else
if(readPointer == 0)
readPointer += S3_RX_START;
#endif

for(count = TFTP_PACKET_MAX_SIZE; count--;)
{
DBG_TFTP_EX(if((count == TFTP_PACKET_MAX_SIZE - 1) || (count == 0)) {
Expand All @@ -137,12 +139,12 @@ static uint8_t processPacket(void)

#if defined(__WIZ_W5500__)
*bufPtr++ = spiReadReg(readPointer++, S3_RXBUF_CB);
// W5500 auto increments the readpointer by memory mapping a 16bit addr
// Use uint16_t overflow from 0xFFFF to 0x10000 to follow W5500 internal pointer
// W5500 have [automaticly] offset address mapping between the read pointer
// to the physical address
#else
*bufPtr++ = spiReadReg(readPointer++, 0);
if(readPointer == S3_RX_END)
readPointer = S3_RX_START;
*bufPtr++ = spiReadReg(S3_map_readPointer_to_phy_address(readPointer++), 0);
// W5100 & W5200 should read from the physical address
// address (relative to the base address) calculate by masking with the buffer size
#endif
}

Expand Down Expand Up @@ -380,12 +382,6 @@ static void sendResponse(uint16_t response)
uint8_t packetLength;
uint16_t writePointer;

#if defined(__WIZ_W5500__)
writePointer = spiReadWord(REG_S3_TX_WR0, S3_R_CB);
#else
writePointer = spiReadWord(REG_S3_TX_WR0, 0) + S3_TX_START;
#endif

switch(response)
{
default:
Expand Down Expand Up @@ -447,20 +443,23 @@ static void sendResponse(uint16_t response)

txPtr = txBuffer;

// Use uint16_t overflow from 0xFFFF to 0x10000 to follow WIZnet internal pointer
writePointer = spiReadWord(REG_S3_TX_WR0, S3_R_CB);

while(packetLength--)
{
spiWriteReg(writePointer++, S3_TXBUF_CB, *txPtr++);
#if defined(__WIZ_W5500__)
// W5500 auto increments the readpointer by memory mapping a 16bit addr
// Use uint16_t overflow from 0xFFFF to 0x10000 to follow W5500 internal pointer
}
spiWriteWord(REG_S3_TX_WR0, S3_W_CB, writePointer);
spiWriteReg(writePointer++, S3_TXBUF_CB, *txPtr++);
// W5500 have [automaticly] offset address mapping between the write pointer
// to the physical address
#else
if(writePointer == S3_TX_END)
writePointer = S3_TX_START;
}
spiWriteWord(REG_S3_TX_WR0, S3_W_CB, writePointer - S3_TX_START);
spiWriteReg(S3_map_writePointer_to_phy_address(writePointer++), S3_TXBUF_CB, *txPtr++);
// W5100 & W5200 should write to the physical address
// address (relative to the base address) calculate by masking with the buffer size
#endif
}

spiWriteWord(REG_S3_TX_WR0, S3_W_CB, writePointer); // Write back new pointer

spiWriteReg(REG_S3_CR, S3_W_CB, CR_SEND);

Expand Down Expand Up @@ -513,25 +512,28 @@ void tftpInit(void)
*/
uint8_t tftpPoll(void)
{
uint16_t packetSize;
uint16_t prev = 0;
uint8_t response = ACK;

// Get the size of the recieved data
uint16_t packetSize = spiReadWord(REG_S3_RX_RSR0, S3_R_CB);
packetSize = spiReadWord(REG_S3_RX_RSR0, S3_R_CB);

// Reading twice (recommended by Wiznet, https://github.com/Wiznet/ioLibrary_Driver)
while(packetSize != prev)
{
_delay_ms(TFTP_PACKET_DELAY);

prev = packetSize;
packetSize = spiReadWord(REG_S3_RX_RSR0, S3_R_CB);
}

if(packetSize)
{
tftpFlashing = TRUE;

while((spiReadReg(REG_S3_IR, S3_R_CB) & IR_RECV))
{
spiWriteReg(REG_S3_IR, S3_W_CB, IR_RECV);
// FIXME: is this right after all? smaller delay but
// still a delay and it still breaks occasionally
_delay_ms(TFTP_PACKET_DELAY);
}

// Process Packet and get TFTP response code
#if(DEBUG_TFTP > 0)
packetSize = spiReadWord(REG_S3_RX_RSR0, S3_R_CB);
response = processPacket(packetSize);
#else
response = processPacket();
Expand Down
16 changes: 6 additions & 10 deletions avr/bootloaders/athena/src/ethernet/tftp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@
#define INVALID_IMAGE 5

#if defined(__AVR_ATmega328P__) || defined(__AVR_ATmega32U4__)
#define MAX_ADDR 0x7000 /// For 328p/32u4 with 2Kword bootloader
#define MAX_ADDR 0x7000 /// For 328p/32u4 with 2Kword (4K-Byte) bootloader
#elif defined(__AVR_ATmega1280__) || defined(__AVR_ATmega1284P__)
#define MAX_ADDR 0x1F000 /// For 1280 with 2Kword bootloader
#define MAX_ADDR 0x1E000 /// For 1280 with 4Kword (8K-Byte) bootloader
#elif defined(__AVR_ATmega2560__)
#define MAX_ADDR 0x3F000 /// For 2560 with 2Kword bootloader
#define MAX_ADDR 0x3E000 /// For 2560 with 4Kword (8K-Byte) bootloader
#endif

#define TFTP_DATA_SIZE 512
Expand All @@ -51,17 +51,13 @@
#define TFTP_PACKET_MAX_SIZE \
(UDP_HEADER_SIZE + TFTP_OPCODE_SIZE + TFTP_BLOCKNO_SIZE + TFTP_MAX_PAYLOAD)

#define TFTP_PACKET_DELAY 400
#define TFTP_PACKET_DELAY 5
Copy link
Member

Choose a reason for hiding this comment

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

Big improvement


/**
* Tftp status flag, it is set to TRUE if flashing from
* tftp is currently active */
uint8_t tftpFlashing;
uint8_t tftpInitError;

#ifndef TFTP_RANDOM_PORT
uint16_t tftpTransferPort;
#endif
extern uint8_t tftpFlashing;
extern uint8_t tftpInitError;

void tftpInit(void);
uint8_t tftpPoll(void);
Expand Down
5 changes: 5 additions & 0 deletions avr/bootloaders/athena/src/ethernet/w5100.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,11 @@
#define S3_RX_START 0x7800
#define S3_RX_END 0x8000

#define S3_map_writePointer_to_phy_address(rptr) \
(S3_TX_START + (rptr & (S3_TX_END - S3_TX_START - 1)))
#define S3_map_readPointer_to_phy_address(rptr) \
(S3_RX_START + (rptr & (S3_RX_END - S3_RX_START - 1)))

#define MR_CLOSED 0x00
#define MR_TCP 0x01
#define MR_UDP 0x02
Expand Down
5 changes: 5 additions & 0 deletions avr/bootloaders/athena/src/ethernet/w5200.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@
#define S3_RX_START 0xD800
#define S3_RX_END 0xE000

#define S3_map_writePointer_to_phy_address(rptr) \
(S3_TX_START + (rptr & (S3_TX_END - S3_TX_START - 1)))
#define S3_map_readPointer_to_phy_address(rptr) \
(S3_RX_START + (rptr & (S3_RX_END - S3_RX_START - 1)))

#define MR_CLOSED 0x00
#define MR_TCP 0x01
#define MR_UDP 0x02
Expand Down
2 changes: 2 additions & 0 deletions avr/bootloaders/athena/src/serial.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#error "Unknown MCU. Cannot find the proper serial bootloader."
#endif

uint8_t serialFlashing;

void serialInit(void)
{
// Double speed mode USART0
Expand Down
2 changes: 1 addition & 1 deletion avr/bootloaders/athena/src/serial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@

/* Serial status flag, it is set to TRUE if flashing from
* serial is currently active */
uint8_t serialFlashing;
extern uint8_t serialFlashing;

/*
* Function definitions
Expand Down