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.

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

docs: Add note about passing peripheral addresses as arguments to inline assembler #1768

Closed
wants to merge 1 commit into from

Conversation

dhylands
Copy link
Contributor

@dhylands dhylands commented Jan 6, 2016

No description provided.

@dhylands dhylands changed the title docs: Add note about passing peripheral addresses as arguments docs: Add note about passing peripheral addresses as arguments to inline assembler Jan 6, 2016
@dpgeorge
Copy link
Member

dpgeorge commented Jan 7, 2016

I admit that it's really ugly having to do this masking of the high bit all over the place (eg also in the machine.mem* functions). We could actually make these constants big-num-constants, and it would solve, I think, all of the problems. It would take more ROM to store the big-num, 5 extra words per big-num. And the parser would need to be modified to be able to fold big-num's. But maybe it's worth it? pyboard has the ROM....

@dhylands
Copy link
Contributor Author

dhylands commented Jan 7, 2016

Thinking about this some more, I'd like to propose another approach. Create some type of native class, lets say RegisterBank which has a single uint32_t * member. The type could have an accessor that returns a python int. It could also have its own mem8/16/32 functions that just take an offset as an argument.

This would only require 2 words for each register. If we extended this and created sub-types for each type of register (USART, SPI, I2C, etc) it would allow the register offsets to be grouped with the register bank that they belonged to and it would allow the prefix to be dropped. I think that this would yield a net drop in flash space required to store all of the registers and their corresponding fieldnames. It would also make for a much cleaner interface (trying to find stuff in the long list of stm constants is a pain.

Then you would do something like:

uart = stm.USART1
sr = uart.mem16[uart.SR]
uart.mem16[uart.SR] = sr | 0x08)

mp_convert_obj_to_native could recognize these register types and automatically pass in the correct address.

I think we could make constants for register offsets available as stm.USART.SR rather than stm.USART_SR by just having the class hierarchy correct. i.e. USART type derives from RegisterBlock type, and USART1 type derives from USART type.

@dpgeorge
Copy link
Member

dpgeorge commented Jan 7, 2016

I think what you are describing can be easily implemented using uctypes (which was one of the original goals for its use).

I had a go at making stm constants big ints, and it was much easier than I thought it would be. I also modified the parser so it can fold constants which are big ints. In the end, everything is greatly improved and simplified: constants are all positive and proper numbers, no more &0x7fffffff needed anywhere, movwt can take a full 32-bit arg, inline asm functions and viper functions can take big ints, and machine mem functions can use the default address helper functions (ie no more masking).

See 2621f8a and 22b2265 for the biggest changes (which are not that big). @peterhinch you might like these ones: 47dc592 and ea8be37.

I'll close this PR because it's no longer relevant. The example code given in the PR now runs without the &7fffffff stuff.

@dpgeorge dpgeorge closed this Jan 7, 2016
@peterhinch
Copy link
Contributor

Very nice! Is the return value from an asm function still subject to the constraint?

@dhylands
Copy link
Contributor Author

dhylands commented Jan 7, 2016

Not quite sure how uctypes would fit in at all (so we're probably thinking about something totally differnt).

Right now, each constant in the stm module takes up a qstr and 4 bytes for the small int. Using bit ints, the 4 bytes gets replaced by 5 words (20 bytes). Since there are currently about 200 contants, that equates to an increase of 3K, and the constants can't be used by IRQ routines.

What I was thinking was to create a struct something like:

typedef struct {
    mp_obj_base_t base;
    uint32_t val;
} mp_register_base_t;

for each constant that is a register base address. And you would have one of those per base. This would yield an increase of 800 bytes. Being able to drop the prefix off the other constants winds up saving about 800 bytes.

@dpgeorge
Copy link
Member

dpgeorge commented Jan 7, 2016

Very nice! Is the return value from an asm function still subject to the constraint?

Yes, still constrained. Before fixing it one needs to make a decision if the return value is signed or not... eg if you return 0xffffffff does that mean -1 or a large positive number? A solution could be to provide a type annotation to the asm function saying which one you want (or if you want it to be a pointer so you can return actual objects). Eg def f(r0) -> uint. It would then work similar to viper.

Not quite sure how uctypes would fit in at all

Can't you define a uctypes structure similar to the CMSIS structures for a peripheral, then you can do things like:

uart = stm.UART1 # this is some uctypes object
uart.DR = 42 # write to uart
print(uart.DR) # read from uart

and the constants can't be used by IRQ routines.

Hmm, interesting point. You should still be able to use stm.GPIOA even though it's a big int. And stm.GPIOA + stm.BSRRL will be folded to a true constant, so you can still do that in IRQ handlers.

Can you think of a case (do you have existing code) that is now broken due to the recent change to make the constants big ints?

@dhylands
Copy link
Contributor Author

dhylands commented Jan 7, 2016

I was able to do a uctypes based IRQ handler that worked:

import pyb
import micropython
import stm
import uctypes

micropython.alloc_emergency_exception_buf(100)

gpio_desc = {
    'MODER':    uctypes.UINT16 | 0x00,
    'OTYPER':   uctypes.UINT16 | 0x04,
    'OPSEEDR':  uctypes.UINT16 | 0x08,
    'PUPDR':    uctypes.UINT16 | 0x0C,
    'IDR':      uctypes.UINT16 | 0x10,
    'ODR':      uctypes.UINT16 | 0x14,
    'BSRRL':    uctypes.UINT16 | 0x18,
    'BSRRH':    uctypes.UINT16 | 0x1A,
}

gpio = uctypes.struct(stm.GPIOA & 0x7fffffff, gpio_desc, uctypes.NATIVE)

def toggle(tim):
    on = (gpio.ODR & (1 << 15)) != 0
    if on:
        gpio.BSRRH = 1 << 15
    else:
        gpio.BSRRL = 1 << 15

timer = pyb.Timer(4, freq=2, callback=toggle)

so losing the constants in an IRQ isn't that big a deal (if the folding didn't work for some case).

nickzoic pushed a commit to nickzoic/micropython that referenced this pull request Apr 16, 2019
nickzoic pushed a commit to nickzoic/micropython that referenced this pull request Apr 16, 2019
nickzoic pushed a commit to nickzoic/micropython that referenced this pull request Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants