[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [avr-libc-dev] [bug #43828] wdt.h: Wrong inline assembler arguments

From: Georg-Johann Lay
Subject: Re: [avr-libc-dev] [bug #43828] wdt.h: Wrong inline assembler arguments
Date: Thu, 30 Apr 2015 15:34:07 +0200
User-agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

Am 04/30/2015 um 12:09 PM schrieb Sivanupandi, Pitchumani:
-----Original Message-----
From: Georg-Johann Lay [mailto:address@hidden
Sent: Wednesday, April 29, 2015 3:11 PM
To: Sivanupandi, Pitchumani
Cc: address@hidden
Subject: Re: [bug #43828] wdt.h: Wrong inline assembler arguments

Am 04/28/2015 um 12:53 PM schrieb Sivanupandi, Pitchumani:
Hi Johann,

Below is the patch to fix this issue.

diff --git a/avr-libc/include/avr/wdt.h b/avr-libc/include/avr/wdt.h
index [...]

Is this change Ok?

That patch is malformed, presumably because of missing / spurious spaces or
new lines.

Attached the patch.

Several notes:

Some asm use ORI with operand of constraint r, e.g. temp or temp_reg in wdt_disable(). Correct constraint is d (or a subset thereof).

Some flag values are using constraint "I" which is 0 <= * <= 63. Dunno whether there are devices that have flags located in bit 6 or 7. Proposed constraint is "n", "I" doesn't have any benefit here.

Some memory (non-I/O) addresses use "M" as constraint which is 0 <= * <= 0xff. Dunno whether there are devices that have SFRs with memory addresses outside that range. LDS and STS are fine with "i" (known at link time) or even "n" (known at compile time). "M" has no advantage here.

All the asms are changing memory but don't mention that. Usually it is not wanted that (non-volatile) memory accesses are dragged across the inline asm. Easy fix is to clobber memory, more exact is to explicitly describe the effect on memory. For example, one sequence reads:

  "sts %0, %2"
  : /* no outputs */

Explicit memory modelling would read something like:

  "sts %1, %3"
  : "+m" (_WD_CONTROL_REG)

There are magic values like 0xd8, 0x8, 0x7. Can't these be represented by macros from io.h?

Named asm operands are easier to grasp, but they use / expose identifiers which are in the namespace of the application, not in the namespace of the implementation. For example, the code will raise strange errors if

#include <avr/wdh.t>

#define tmp
#define temp
#define SIGNATURE 0x1234

void nofun (void)
    wdt_enable (0);


reply via email to

[Prev in Thread] Current Thread [Next in Thread]