How to set external pin state / usage of AVR_IOCTL_IOPORT_SET_EXTERNAL / bug?

[kittennbfive]

Hello,

as the IRC seems dead i try it here: What's the correct way to set an external pin state for an input, like if the AVR-pin is connected to some other device? Is AVR_IOCTL_IOPORT_SET_EXTERNAL the right way to do it?

Here is my testcode that does not work:

#include <stdlib.h>
#include <stdio.h>
#include <signal.h>
#include <stdbool.h>

#include "sim_avr.h"
#include "sim_elf.h"

#include "avr_ioport.h"

avr_t * avr = NULL;

volatile bool ctrl_c=false;

void handler_sigint(int s)
{
	(void)s;
	printf("received CTRL+C\n");
	ctrl_c=true;
}

int main(void)
{
	signal(SIGINT, &handler_sigint);
	
	elf_firmware_t firmware;

	if(elf_read_firmware("avr_debug.elf", &firmware))
	{
		printf("elf_read_firmware failed\n");
		return 1;
	}

	avr = avr_make_mcu_by_name("atmega1284p");
	if (!avr) {
		printf("avr_make_mcu_by_name failed\n");
		return 1;
	}
	
	avr_init(avr);
	avr->frequency=18432000;
	avr_load_firmware(avr, &firmware);

	avr_ioport_external_t io_ext;
	io_ext.name='B';
	io_ext.mask=0xff;
	io_ext.value=0x55;
	avr_ioctl(avr, AVR_IOCTL_IOPORT_SET_EXTERNAL('B'), &io_ext);
	
	int state;
	do
	{
		state=avr_run(avr);
	} while(state!=cpu_Done && state!=cpu_Crashed && !ctrl_c);

	avr_terminate(avr);

	printf("simulation finished\n");

	return 0;
}

#include <stdio.h>
#include <stdint.h>
#include <avr/io.h>
#define BAUD 115200
#include <util/setbaud.h>
#include <util/delay.h>
#include <avr/sleep.h>

int uart_putchar(char c, FILE *stream);

int main(void)
{
	UBRR0=UBRR_VALUE;
	UCSR0B|=(1<<TXEN0);
	
	FILE uart_output = FDEV_SETUP_STREAM(uart_putchar, NULL, _FDEV_SETUP_WRITE);
	stdout = &uart_output;
	
	printf("PINB is 0x%x\r\n", PINB); //expected: 0x55 - real: 0x00
	
	_delay_ms(500);
	
	sleep_mode();
		
	return 0;
}

int uart_putchar(char c, FILE *stream)
{
	(void)stream;
	while(!(UCSR0A&(1<<UDRE0)));
	UDR0=c;
	return 0;
}

If somebody could tell me if this is a bug in simavr or a bug in front of the computer (=i am doing it wrong) i would appreciate. If it is indeed a bug in simavr i have a patch ready and will open a PR.

Thank you.

[gatk555]

Bug (IMHO). The ioctl does nothing but store the passed values, so has no effect on PINB. If I were you, I would not try to fix it as I think that IOCTL was never a good idea. The simulator should not know anything about the voltage on its input pins that does not come directly from the external circuit. The "right" way is to connect to IOPORT_IRQ_REG_PIN and supply the correct value when the simulator reads it. Unfortunately that does not work at present, but should be fixed in my recent "IOPORT" pull request. Also raise IOPORT_IRQ_PINx on input changes in case of enabled interrupts.

It is not pretty, but probably can not be.

[kittennbfive]

Thank you. The patch is was talking about introduced another bug so i discarded it. I got it working (i think) using avr_raise_irq(avr_io_getirq(avr, AVR_IOCTL_IOPORT_GETIRQ('B'), i), (val>>i)&1); inside a loop (0..7). I am using the latest git, not a release.
If AVR_IOCTL_IOPORT_SET_EXTERNAL() does basically nothing useful i think it should be removed or at least there should be a better description somewhere in the code / the doc.

[gatk555]

That should work, but the values will be overridden if the firmware writes one bits to the output register. That is because the simavr code is trying to implement the pull-up feature of the chip. In my view that is a mistake, as the effect of the pull-up depends entirely on the external circuit. The only real use for AVR_IOCTL_IOPORT_SET_EXTERNAL is to override that behaviour and, as you found, it has its own bug.

I have recently been bitten by the pull-up feature again, so I expect to submit a pull request to remove both internal and external forms.

[Nuxar1]

I personally found using both gave the best results.
First set the default pin value and then raising the IRQ like this:

void Emulator::SetPortPullupPin(char name, uint8_t mask, uint8_t val) {
	avr_ioport_external_t io_ext;
	io_ext.name = name;
	io_ext.mask = mask;
	io_ext.value = val;
	avr_ioctl(avr, AVR_IOCTL_IOPORT_SET_EXTERNAL(name), &io_ext);
	for (uint8_t i = 0; i < 8; i++) {
		if (mask & (1 << i)) {
			avr_raise_irq(avr_io_getirq(avr, AVR_IOCTL_IOPORT_GETIRQ(name), i), val & (1 << i));
		}
	}
}

[Nuxar1]

I think the external pull-up feature should be modified to work on a per-pin basis, just like avr_raise_irq. The issue I was experiencing is that if the simulated code writes the pull-ups after the external hardware did, it would reset the value.