Skip to content

Have platform-dependent code in only one place #19

Description

@piranna

Have a "platform" folder with subdirs for each platform with their platform dependent code inside (i.e., no asembler code on the core) like PyMite does. This will make more clear where to look and how to port it to another platforms, and also would easier to have a unique central makefile (just define as a param what platform do you want to compile).

Activity

  1. piranna commented on Jan 1, 2014

    @piranna
    Author

    Also would be nice to have to code stored on subfolders for each task to identify it better: lexer, parser, runtime...

  2. pfalcon commented on Jan 1, 2014

    @pfalcon
    Contributor

    I love structures and hierarchies, but then I know that you can go "too much" with it. Lexer and parser is ~1 file each. Having bunch of subdirs means extra work navigating among them. I'm not saying stuff can't be improved, it just should be balanced with practicality.

  3. KeithJRome commented on Jan 1, 2014

    @KeithJRome

    The most practical thing is to organize it well. I'm not aware of this "navigation cost". Folders are free, and development IDEs don't care.

    And just because it is a single file today doesn't mean it will always be a single file. In fact, a single code file for a lexer or parser for any dynamic language sounds VERY overweight at first blush.

  4. pfalcon commented on Jan 1, 2014

    @pfalcon
    Contributor

    development IDEs don't care.

    Yeah, but real men don't use finicky "ides", they use cd + vi, right? ;-)

  5. piranna commented on Jan 1, 2014

    @piranna
    Author

    Well, for that one file cases we can keep going this way, only that in fact
    they are two: .c & .h. Anyway, the biggest problem is for assembler and
    platform dependent code: they must go to their own folder.

    Send from my Samsung Galaxy Note II
    El 01/01/2014 19:08, "Paul Sokolovsky" [email protected] escribió:

    development IDEs don't care.

    Yeah, but real men don't use finicky "ides", they use cd + vi, right? ;-)

    —
    Reply to this email directly or view it on GitHubhttps://github.com/dpgeorge/issues/19#issuecomment-31427032
    .

  6. dpgeorge commented on Jan 2, 2014

    @dpgeorge
    Member

    Agree that assembler code should go in a platform-specific folder.

  7. tylerwilson commented on Jan 2, 2014

    @tylerwilson

    Just my 2 cents: I am not a huge fan of making platform folders, especially for code that is designed to be portable in the first place. Too much code duplication happens in my experience. Better to try to use a set of #defines and macros first.

    Even for the assembler, if we can do #defines there and use macros, that would be preferable. For example, the porting of the nlrx64.S to OS X was simply a removal of some unsupported keywords. If we made those into defines we ought to be able to use one file for multiple platforms and not duplicate the majority of the code in different places.

  8. piranna commented on Jan 2, 2014

    @piranna
    Author

    Even for the assembler, if we can do #defines there and use macros, that
    would be preferable. For example, the porting of the nlrx64.S to OS X was
    simply a removal of some unsupported keywords. If we made those into
    defines we ought to be able to use one file for multiple platforms and not
    duplicate the majority of the code in different places.

    Obviously, this multi-platform, half-portable code could go on the core
    folder, and the platform specific files would only need to set #defines
    that enabled them... Anyway, assembler code is not portable by definition,
    so although it could be used by several platforms, it should be removed
    from core folder (although not necesarily from the core code) or
    substituted by an equivalent C code, and in my opinion anything that would
    require a platform-dependent #define.

    "Si quieres viajar alrededor del mundo y ser invitado a hablar en un monton
    de sitios diferentes, simplemente escribe un sistema operativo Unix."
    – Linus Tordvals, creador del sistema operativo Linux

  9. iabdalkader commented on Jan 3, 2014

    @iabdalkader
    Contributor

    This is very interesting to me as I want to start porting the code to the STM32F407 but not sure how the code should be structured, but here's an idea, it looks like each driver has two parts, the micropython binding, which is platform independent, I think, and a platform-dependent low-level I/O code, it seems like the first step would be to split each driver into a micropython binding and a target specific implementation of the low level stuff ?

    take for example the i2c driver, there will be an stm/i2c_pyobj.c file and an stm/STM32F405/i2c_impl.c, this folder will also contain the STM32F405 linker script, so the hierarchy looks something like this:

    stm/
    |--i2c.h
    |--i2c_pyobj.c  <--micropython binding
    |--STM32F405/   
       |---stm32f405.ld   <-- linker script
       |---i2c_impl.c       <-- i2c lowlevel stuff
    |--STM32F407/
       |---stm32f407.ld
       |---i2c_impl.c
    

    and so on, now the Makefile would be changed to accept a target on the command line, and based on that it links that target's implementation files, so external calls in the *_pyobj.c would be resolved to the target specific implementation...

  10. pfalcon commented on Jan 3, 2014

    @pfalcon
    Contributor

    Few comments here:

    I want to start porting the code to the STM32F407

    Given your example, are you seriously thinking that F407 would have different I2C implementation than F405? Can't be that way unless proven otherwise (then STM worth an entry in some "f$#^^ up design" hall-of).

    the micropython binding, which is platform independent
    stm/i2c_pyobj.

    If it's "platform independent", why it's under "stm/" ?

    All in all, I'd say there's no need to have http://en.wikipedia.org/wiki/Analysis_paralysis , best practice is to start implementing new stuff in such a way as to require minimal changes to existing stuff, and then take further action based on actual experience with that. For example, I'd expect F407 port to require ~zero duplication of code, just adding drivers for new hardware blocks and parametrizing existing code (i.e. #ifdef's) - for example, for case like F407 has 3 I2C blocks, while F405 - 2 (even that's unlikely per my crystal ball).

  11. iabdalkader commented on Jan 3, 2014

    @iabdalkader
    Contributor

    I'm not sure, it probably doesn't have a different implementation, but it might have a different pin mapping ? that and the fact that it has a different memory layout, which means there's a different linker script that needs to go somewhere...
    and also consider other families that will most likely have different implementations, e.g. STM32F3xx, now, are you still for using #ifdef for every micro in every family of micros to be supported ? can you "under analyze" something :D

    The micropython binding looks platform independent, someone more familiar with the code might be able to confirm this, if so, then it needs to be separated from the implementation, ok maybe stm/ is a bad example.

  12. hagenkaye commented on Jan 3, 2014

    @hagenkaye
    Contributor

    #26 (comment)

    This is how eLua implemented different platforms.

  13. Neon22 commented on Jan 4, 2014

    @Neon22
    Contributor

    Looking at the data sheets the 407 is a minum 100 pin device vs the 405's 64 pins.
    So all pins will be different and must be re-mapped.
    But internally alomst everything else is the same, just more ports and extra camera functionality.
    So IMHO it could inherit from 405 but with changes to pin mapping, etc
    This 'improvement' could be done as an optimisation when we get 405 working ? In the meantime (and possibly forever) it could be its own folder.

    The elua layout is interesting. See https://github.com/elua/elua/blob/master/src/platform/stm32f4/stm32f4xx.h and compare with the adjacent folder for the stm32f2

  14. dpgeorge commented on Jan 12, 2014

    @dpgeorge
    Member

    As for the F407, @Neon22 is correct: it's basically the same inside, just has more pins and hence more GPIO and peripherals.

    The STM library is the same for all F4's, and that's the one I'm using in the stm directory. So there should be almost no change to the code to get it running on an F407.

    If anything, we just have some #define etc directives to enable additional I2Cs (or whatever is available) and remap the GPIO, LEDs, switches, etc.

    I don't want to get bogged down trying to work out a perfect file structure etc for ports. I need to work on the pyboard port and make sure the pyboard works very well and has all the features that it should.

  15. iabdalkader commented on Jan 12, 2014

    @iabdalkader
    Contributor

    Speaking of the STM libraries and portability, I think an old version is used, the system_stm32f4xx.c has some code missing, for example in the latest version (V1.3) there are a few conditionals for setting the clock dividers based on the micro:

    #if defined (STM32F40_41xxx) || defined (STM32F427_437xx) || defined (STM32F429_439xx)      
        /* PCLK2 = HCLK / 2*/
        RCC->CFGR |= RCC_CFGR_PPRE2_DIV2;
    
        /* PCLK1 = HCLK / 4*/
        RCC->CFGR |= RCC_CFGR_PPRE1_DIV4;
    #endif /* STM32F40_41xxx || STM32F427_437x || STM32F429_439xx */
    
    #if defined (STM32F401xx)
        /* PCLK2 = HCLK / 2*/
        RCC->CFGR |= RCC_CFGR_PPRE2_DIV1;
    
        /* PCLK1 = HCLK / 4*/
        RCC->CFGR |= RCC_CFGR_PPRE1_DIV2;
    #endif /* STM32F401xx */
    

    The one in stm/ doesn't have that..it might be wise to update at some point, same applies to the USB libraries.

  16. dpgeorge commented on Jan 19, 2014

    @dpgeorge
    Member

    The one in stm/ doesn't have that..it might be wise to update at some point, same applies to the USB libraries.

    Thanks @iabdalkader for the hint: I've now upgraded the peripheral libraries to V1.3.0.

    I'm using V2.1.0 for the USB drivers. Is there a newer one?

  17. iabdalkader commented on Jan 19, 2014

    @iabdalkader
    Contributor

    I'm using V2.1.0 for the USB drivers. Is there a newer one?

    No, that's the latest one :)

  18. dpgeorge commented on Oct 3, 2014

    @dpgeorge
    Member

    Closing because this issue is out of date. The structure of the uPy repo is now more mature and has proven itself (so far), so any issues should be directed at its current state.

  19. added a commit that references this issue on Jan 22, 2017
  20. added a commit that references this issue on Aug 20, 2020
  21. added a commit that references this issue on Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    portsRelates to multiple ports, or a new/proposed portrfcRequest for Comment

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions