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

PULPino port #58

Merged
merged 59 commits into from Jul 3, 2017

Conversation

Projects
None yet
3 participants
@suehtamacv
Contributor

suehtamacv commented Jun 16, 2017

Solves issue #55.

jlbirccyn and others added some commits May 15, 2017

[pulpino] Acknowledging interruption no matter what service was calle…
…d, and calling tpl_central_interrupt_handler_2 to treat ISR2s
[pulpino] Saving status of interrupt enable bit between context switc…
…hes, and depending on running_id to ack interruption in tpl_ack_irq()
@jlbirccyn

This comment has been minimized.

Show comment
Hide comment
@jlbirccyn

jlbirccyn Jun 21, 2017

Member

Hi

I did a quick review. In file goil/templates/build/build_py.goilTemplate you added code if target is riscv.

+cSourceList.append(projfile.ProjectFile("% !PROJECT %/tpl_vectors.c"))
+cSourceList.append(projfile.ProjectFile("% !PROJECT %/tpl_primary_irq.S"))

should be set using the OIL attribute GENERATED_FILE. Check the other targets in the templates/.../config hierarchy

+%
+if TARGET == "riscv/pulpino" then
+%
+PULPINO_PATH = os.environ.get('PULPINO_PATH','MUST_SET_PULPINO_PATH_ENV_VARIABLE')
+%
+end if
+%

Maybe this kind of thing can use an additional OIL attribute like NEEDS_ENV in the templates/.../config hierarchy to list the environment variables needed. In build.py.goilTemplate, a test could be generated for each of these attribute.

Member

jlbirccyn commented Jun 21, 2017

Hi

I did a quick review. In file goil/templates/build/build_py.goilTemplate you added code if target is riscv.

+cSourceList.append(projfile.ProjectFile("% !PROJECT %/tpl_vectors.c"))
+cSourceList.append(projfile.ProjectFile("% !PROJECT %/tpl_primary_irq.S"))

should be set using the OIL attribute GENERATED_FILE. Check the other targets in the templates/.../config hierarchy

+%
+if TARGET == "riscv/pulpino" then
+%
+PULPINO_PATH = os.environ.get('PULPINO_PATH','MUST_SET_PULPINO_PATH_ENV_VARIABLE')
+%
+end if
+%

Maybe this kind of thing can use an additional OIL attribute like NEEDS_ENV in the templates/.../config hierarchy to list the environment variables needed. In build.py.goilTemplate, a test could be generated for each of these attribute.

@suehtamacv

This comment has been minimized.

Show comment
Hide comment
@suehtamacv

suehtamacv Jun 21, 2017

Contributor
  1. The first issue was solved, i was weirdly not aware of the GENERATED_FILES option.
  2. The PULPINO_PATH variable is necessary to correctly compile the code, as it contains Modelsim simulation files and tools to program the FPGA board. I refer to $PULPINO_PATH in my build script. There is always the need to actually export this variable, as precised in my README, but no need to make the Python systems aware of its existence. I dropped these lines.

Believe it was the single root Trampoline file that I touched.

Contributor

suehtamacv commented Jun 21, 2017

  1. The first issue was solved, i was weirdly not aware of the GENERATED_FILES option.
  2. The PULPINO_PATH variable is necessary to correctly compile the code, as it contains Modelsim simulation files and tools to program the FPGA board. I refer to $PULPINO_PATH in my build script. There is always the need to actually export this variable, as precised in my README, but no need to make the Python systems aware of its existence. I dropped these lines.

Believe it was the single root Trampoline file that I touched.

@suehtamacv

This comment has been minimized.

Show comment
Hide comment
@suehtamacv

suehtamacv Jun 23, 2017

Contributor

Another idea to solve the second point would be to include PULPino's repo as a submodule to Trampoline. The submodule would be cloned only after a user request with git submodule update (thus would not pollute this repository) and at least Trampoline would be self-contained. With PULPino in a known path, the user would be no longer be expected to indicate PULPino's location with $PULPINO_PATH...

Which solution do you think that works better with Trampoline ? Even with submodules Trampoline would still expect the user to "clone" PULPino and to run setup.csh, so I am not sure the gain is relevant enough to justify the change.

Regards.

Contributor

suehtamacv commented Jun 23, 2017

Another idea to solve the second point would be to include PULPino's repo as a submodule to Trampoline. The submodule would be cloned only after a user request with git submodule update (thus would not pollute this repository) and at least Trampoline would be self-contained. With PULPino in a known path, the user would be no longer be expected to indicate PULPino's location with $PULPINO_PATH...

Which solution do you think that works better with Trampoline ? Even with submodules Trampoline would still expect the user to "clone" PULPino and to run setup.csh, so I am not sure the gain is relevant enough to justify the change.

Regards.

@KamelHacene

This comment has been minimized.

Show comment
Hide comment
@KamelHacene

KamelHacene Jun 26, 2017

Contributor

Hi !

In fact we may need some way to generate checks in the build.py with goil (and maybe a goil option to force them to a value).

For the meantime, you can replicate the generation of the build.py file in goil/templates/build/riscv{/pulpino,}/ directory and do whatever you want in this file. Here's a patch that does this :

0002-pulpino-check-Add-PULPINO_PATH-environnment-variable.patch.txt

Plus, you can add other architecture-dependant oil relative checking in the goil/templates/check/riscv{/pulpino,} directory. Here's a patch that returns an error if trying to use Memory or Timing protection :

0001-pulpino-check-Warn-if-trying-to-use-Memory-Timing-pr.patch.txt

Contributor

KamelHacene commented Jun 26, 2017

Hi !

In fact we may need some way to generate checks in the build.py with goil (and maybe a goil option to force them to a value).

For the meantime, you can replicate the generation of the build.py file in goil/templates/build/riscv{/pulpino,}/ directory and do whatever you want in this file. Here's a patch that does this :

0002-pulpino-check-Add-PULPINO_PATH-environnment-variable.patch.txt

Plus, you can add other architecture-dependant oil relative checking in the goil/templates/check/riscv{/pulpino,} directory. Here's a patch that returns an error if trying to use Memory or Timing protection :

0001-pulpino-check-Warn-if-trying-to-use-Memory-Timing-pr.patch.txt

@suehtamacv

This comment has been minimized.

Show comment
Hide comment
@suehtamacv

suehtamacv Jun 30, 2017

Contributor

Hello,

The patches were applied !

Regards.

Contributor

suehtamacv commented Jun 30, 2017

Hello,

The patches were applied !

Regards.

@jlbirccyn

It is ok for me

@jlbirccyn jlbirccyn removed request for mbriday and sefau Jul 3, 2017

@jlbirccyn jlbirccyn merged commit 43dadf1 into TrampolineRTOS:master Jul 3, 2017

@jlbirccyn

This comment has been minimized.

Show comment
Hide comment
@jlbirccyn

jlbirccyn Jul 3, 2017

Member

Hello,

I merged the pull-request. Thank you very much for your contribution !

Member

jlbirccyn commented Jul 3, 2017

Hello,

I merged the pull-request. Thank you very much for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment