PORT write inadvertently modifying PIN value

[Peter Ross]

Hi,

Simavr appears to be inadvertently setting ioport PIN values when a PORT
write is requested. This was observed while attempting to run software
that uses the same ioport for reading and writing.

The emulation environment is simple:

// initialise atmega644 mcu, load program, set port B0 to 1
avr_raise_irq(avr_io_getirq(avr, AVR_IOCTL_IOPORT_GETIRQ('B'), 0), 1);
// run sim

The avr c program:

main(){
DDRB = 0;
PORTB = 0;
for(timer = 0; timer < 1000; timer++);
value = PINB & 0x1;
...
printf("value: %i\n", value);

Result:
The program in simavr prints out 0, when on real hardware it prints 1.

Stepping through simavr, the PORTB=0 instruction was found to modify *both* the
r_port and r_pin values of ioport B. I believe it should only set the r_port
value, because simavr separately ORs the appropriate r_pin and r_port bits
when a ioport read is requested (see avr_ioport_read in avr_ioport.c).

Digging deeper, the problem is appears to be caused by the callback section of
avr_ioport_write(). Having set r_port with avr_core_watch_write, it then
performs irq callbacks on all eight bits. These callbacks then invoke the
'avr_ioport_irq_notify' function which inadvertently sets the r_pin value.

static void avr_ioport_write(struct avr_t * avr, avr_io_addr_t addr, uint8_t v, void * param)
{...
avr_core_watch_write(avr, addr, v);

// raise the internal IRQ callbacks
for (int i = 0; i < 8; i++)
avr_raise_irq(p->io.irq + i, (v >> i) & 1);

/*
* this is our "main" pin change callback, it can be triggered by either the
* AVR code, or any external piece of code that see fit to do it.
* Either way, this will raise pin change interrupts, if needed
*/
void avr_ioport_irq_notify(struct avr_irq_t * irq, uint32_t value, void * param)

I am not sure how to fix this easily, as the callbacks are being used for
notification purposes, and also modifying the data. (There might also be another
way to achieve what I am trying to do.)

Cheers,

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[Jakob Gruber]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/26/2012 08:56 AM, Peter Ross wrote:
> Hi,
>
> Simavr appears to be inadvertently setting ioport PIN values when a
> PORT write is requested. This was observed while attempting to run
> software that uses the same ioport for reading and writing.
>
> The emulation environment is simple:
>
> // initialise atmega644 mcu, load program, set port B0 to 1
> avr_raise_irq(avr_io_getirq(avr, AVR_IOCTL_IOPORT_GETIRQ('B'), 0),
> 1); // run sim
>
> The avr c program:
>
> main(){ DDRB = 0; PORTB = 0; for(timer = 0; timer < 1000;
> timer++); value = PINB & 0x1; ... printf("value: %i\n", value);
>
> Result: The program in simavr prints out 0, when on real hardware
> it prints 1.

I suppose that example program is simulating an environment in which
some external component is pulling up PB0?

I ran into the same issue a while back (it's covered in section 6.8
and 7 of my thesis [1]), and I don't think there's a decent way to
solve this without making simavr aware of the connected component, the
state of that component, and a resolution function for PIN values
which combines it with the internal state (input/output, pull-up or not).

Michel did not seem to be enthusiastic about that though ;)

I ended up using a hackish workaround for my purposes.

Jakob

[1] https://github.com/schuay/bachelors_thesis
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJQtyNlAAoJEEVsepuRuEKu3joH+gI3Iu6Qx189jO4N1DxGzd0E
amzU3MKbovrWRU5YEUOTmX8mbNlgjkBT5RsDVzJIQh2rWJPdV8pQV7nDyrtONQWh
kQtjfggkZQg7R/byqXq+MEt0IOxOEeZtPgvauyxN/b26vFP5ghW3VumdUSlv7LZf
03BfaAEUVEXGa13DqW8kvQ9aAgT9C6/JkXKT0qCtGZ6Rz7rjqS6jKr2Pzm69oCE4
2eSNExH6JMPSmAzVSOTjIDPdbYnjMAP/omYKPrMLL91G1OjLbNqlKltGGplJEGzs
5d7oYI+h4bEQ0bFX4YmZLSJI6nuW/d/SM9sDLO2UkkwF6Hq+OEYm2cP/igCBAe4=
=FHWg
-----END PGP SIGNATURE-----

[Peter Ross]

On Thu, Nov 29, 2012 at 09:57:09AM +0100, Jakob Gruber wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11/26/2012 08:56 AM, Peter Ross wrote:
> > Hi,
> >
> > Simavr appears to be inadvertently setting ioport PIN values when a
> > PORT write is requested. This was observed while attempting to run
> > software that uses the same ioport for reading and writing.
> >
> > The emulation environment is simple:
> >
> > // initialise atmega644 mcu, load program, set port B0 to 1
> > avr_raise_irq(avr_io_getirq(avr, AVR_IOCTL_IOPORT_GETIRQ('B'), 0),
> > 1); // run sim
> >
> > The avr c program:
> >
> > main(){ DDRB = 0; PORTB = 0; for(timer = 0; timer < 1000;
> > timer++); value = PINB & 0x1; ... printf("value: %i\n", value);
> >
> > Result: The program in simavr prints out 0, when on real hardware
> > it prints 1.
>
> I suppose that example program is simulating an environment in which
> some external component is pulling up PB0?

Yes.

> I ran into the same issue a while back (it's covered in section 6.8
> and 7 of my thesis [1]), and I don't think there's a decent way to
> solve this without making simavr aware of the connected component, the
> state of that component, and a resolution function for PIN values
> which combines it with the internal state (input/output, pull-up or not).
>
> Michel did not seem to be enthusiastic about that though ;)

That proposal does sound complicated. I think all we need is some way to
prevent avr_ioport_irq_notify() from modifying the data[r_pin] value on
PORT writes.

So to restate the problem:

The problem is caused by simavr not internally distinguishing between PORT
writes and PIN writes. e.g. When the MCU executes a 'PORTB |= 1' instruction,
or an external component raises PB0, the _same_ list of avr_raise_irq callbacks
are invoked.

Suggested solution:

Provide separate internal irq lines for PORT writes and PIN writes. Split
avr_ioport_irq_notify() to handle the PIN and PORT writes separately.

> I ended up using a hackish workaround for my purposes.

Have done the same. I am glad someone else has encountered this!

[lem...@gmail.com]

Hi, I'm new to SimAVR. I have encountered a problem that seems related to this one.

I'm trying to add some simple functionality on top of simduino.c.

I set up some callbacks on pin changes,

// register pins 0-7 ( PORTD )

for (int i = 0; i < 8; i++) {

    avr_irq_register_notify(

avr_io_getirq(avr, AVR_IOCTL_IOPORT_GETIRQ('D'), i),

pin_changed_hook, 0);

}

Next, I tried to change the logical value of a pin in the same port

from simduino and from within the program simavr is running.

In simduino, what I am doing is:

avr_raise_irq(avr_io_getirq(avr, AVR_IOCTL_IOPORT_GETIRQ('D'), 1), 1);

This should set the status of D1 to 1, and it seems to do that: the pin_changed_hook is called back.

The firmware is very simple and toggles the status of _another_ pin of port D in a loop. This is also working.

Now the problem:

As soon as the firmware toggles the status of that other pin of port D, the status of pin 1,

which I manually set to 1 calling avr_raise_irq is set back to 0 and I can't understand why.

This is not happening if the firmware writes a pin of a different port, or doesn't perform writes at all.

Is it the same problem we're talking about?

How did you guys workaround it?

Cheers,

Lem

[James Waugh]

Hello! I realize this is over 10 years old, but just putting this here in case anyone comes by.

What I did was to not use the AVR's internal pullup setting for the problem-causing input pins, which as is stated seem to be on ports which are share outputs.

I modified my simulation to default to HIGH values outside the AVR, and had to make a change in my hardware design to add external pull-up resistors on these problem pins to match this firmware change.

I will look to submit a bug report on GitHub to at least provide some more visibility.