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

init: coolant not initialized, no early I/O-init #336

Open
rkoe opened this issue Apr 18, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@rkoe
Copy link

commented Apr 18, 2018

There are two issues with I/O-initialization:

  1. Coolant: The coolant is not initialized. So, when setting the coolant to low-active, the coolant is on at power-on.

  2. late I/O-init: The I/Os are initialized after waiting 400ms for USB. So, for default-low-outputs, this results in a power-on-spike of ~0.4s. It would be better to first init the I/Os ("early I/O-preinit") to make this spike as short as possible, and then wait for USB.

Fixes:

  1. Coolant: Add the following lines to main.cpp:
  #include "spindle.h"
+ #include "coolant.h"
  #include "temperature.h"

  ...

  void application_init_machine(void)
  {
      ...
      spindle_reset();
+     coolant_init();
+     coolant_reset();
      temperature_init();
  1. early I/O-init: Initialize all I/Os directly after power-on, by moving some init-function-calls from
    application_init_machine() and application_init_startup() to an earlier location. I don't know the code in full detail, so I don't know if this has any side-effects, so some core-developer should check before merging. My suggestion for main.cpp:
+ void application_init_io_early(void)
+ {
+     config_init();
+     gpio_init();
+     gpio_reset();
+     spindle_init();
+     spindle_reset();
+     coolant_init();
+     coolant_reset();
+ }

+ }
  void setup(void)
  {
      // application setup
      application_init_services();
+     // early I/O-init
+     application_init_io_early();
+
      while (SysTickTimer_getValue() < 400);  // delay 400 ms for USB to come up

      application_init_machine();
      application_init_startup();
  }
@justinclift

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2018

With the coolant bit, would you be ok to create a Pull Request with that change?

For the early IO bit... yeah that does sound like more discussion is needed. 😄

@justinclift

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2018

As a data point, the PR to address the first bit is in #339. 😄

giseburt added a commit that referenced this issue May 6, 2018

@giseburt

This comment has been minimized.

Copy link
Member

commented May 6, 2018

PR #339 has been closed. What else is needed for this issue?

@justinclift

This comment has been minimized.

Copy link
Contributor

commented May 7, 2018

The second part to the problem still needs thinking about, and probably some code changes (hopefully minor):

The I/Os are initialized after waiting 400ms for USB. So, for default-low-outputs, this results in a power-on-spike of ~0.4s. It would be better to first init the I/Os ("early I/O-preinit") to make this spike as short as possible, and then wait for USB.

Potential solution:

Initialize all I/Os directly after power-on, by moving some init-function-calls from
application_init_machine() and application_init_startup() to an earlier location.

Potential solution code from @rkoe (main.cpp):

+ void application_init_io_early(void)
+ {
+     config_init();
+     gpio_init();
+     gpio_reset();
+     spindle_init();
+     spindle_reset();
+     coolant_init();
+     coolant_reset();
+ }

+ }
  void setup(void)
  {
      // application setup
      application_init_services();
+     // early I/O-init
+     application_init_io_early();
+
      while (SysTickTimer_getValue() < 400);  // delay 400 ms for USB to come up

      application_init_machine();
      application_init_startup();
  }

Note that @rkoe mentions since he doesn't know the codebase in depth, he'd really like someone familiar with it to chime in. If the above code snippet seems ok, he can then turn it into a PR for merging.

Also note that if @rkoe doesn't have time creating said PR, I can do it pretty easily for him too to save time. 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.