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.

Sign up for GitHub

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

[RFC]Template prototype with updated edts #10873

Merged
merged 5 commits into from
Oct 30, 2018
Merged

[RFC]Template prototype with updated edts #10873

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f4110cd
Switched from COG to jinja2 based code generation. Ported instantiation
evenl Oct 17, 2018
71840bc
Modifying codegen documentation
evenl Oct 22, 2018
5f61191
Updated prototype to generate a .h file which is included in a .c file
evenl Oct 25, 2018
754097a
* Can now set and get parameters instead of direcly populating a dict…
evenl Oct 29, 2018
274ba16
Exposing EDTSDatabase and EDTSDevice API for templates through the ed…
evenl Oct 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions cmake/extensions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,20 +374,20 @@ function(target_sources_codegen
target # The cmake target that depends on the generated file
)
set(options)
set(oneValueArgs)
set(oneValueArgs CODEGEN_OUTPUT)
set(multiValueArgs CODEGEN_DEFINES SEARCH_PATH)



cmake_parse_arguments(CODEGEN "${options}" "${oneValueArgs}"
"${multiValueArgs}" ${ARGN})

# prepend -D to all defines
string(REGEX REPLACE "([^;]+)" "-D;\\1"
CODEGEN_CODEGEN_DEFINES "${CODEGEN_CODEGEN_DEFINES}")

string(REGEX REPLACE "([^;]+)" "-I;\\1"
CODEGEN_SEARCH_PATH "${CODEGEN_SEARCH_PATH}")

message(${CODEGEN_SEARCH_PATH})

# Get all the files that make up codegen for dependency
file(GLOB CODEGEN_SOURCES
${ZEPHYR_BASE}/scripts/codegen/*.py
Expand All @@ -400,15 +400,15 @@ function(target_sources_codegen
foreach(arg ${CODEGEN_UNPARSED_ARGUMENTS})
if(IS_ABSOLUTE ${arg})
set(template_file ${arg})
get_filename_component(generated_file_name ${arg} NAME_WE)
set(generated_file ${CMAKE_CURRENT_BINARY_DIR}/${generated_file_name}.c)
set(generated_file ${CMAKE_CURRENT_BINARY_DIR}/${CODEGEN_CODEGEN_OUTPUT})
else()
set(template_file ${CMAKE_CURRENT_SOURCE_DIR}/${arg})
get_filename_component(generated_file_name ${arg} NAME_WE)
set(generated_file ${CMAKE_CURRENT_BINARY_DIR}/${generated_file_name}.c)
set(generated_file ${CMAKE_CURRENT_BINARY_DIR}/${CODEGEN_CODEGEN_OUTPUT})
endif()
get_filename_component(template_dir ${template_file} DIRECTORY)

include_directories( ${CMAKE_CURRENT_BINARY_DIR} )

if(IS_DIRECTORY ${template_file})
message(FATAL_ERROR "target_sources_codegen() was called on a directory")
endif()
Expand Down Expand Up @@ -487,7 +487,7 @@ function(zephyr_sources_codegen_ifdef feature_toggle)
endfunction()

function(zephyr_library_sources_codegen)
target_sources_codegen(${ZEPHYR_CURRENT_LIBRARY} ${ARGN})
target_sources_codegen(${ZEPHYR_CURRENT_LIBRARY} ${ARGN})
endfunction()

function(zephyr_library_sources_codegen_ifdef feature_toggle)
Expand Down
9 changes: 7 additions & 2 deletions drivers/i2c/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ zephyr_library_sources_ifdef(CONFIG_I2C_SBCON i2c_sbcon.c)
zephyr_library_sources_ifdef(CONFIG_I2C_NIOS2 i2c_nios2.c)
zephyr_library_sources_ifdef(CONFIG_I2C_STM32_V1 i2c_ll_stm32_v1.c)
zephyr_library_sources_ifdef(CONFIG_I2C_STM32_V2 i2c_ll_stm32_v2.c)
zephyr_library_sources_codegen_ifdef(CONFIG_I2C_STM32_V1 i2c_ll_stm32.c SEARCH_PATH ${PROJECT_SOURCE_DIR}"/templates/")
zephyr_library_sources_codegen_ifdef(CONFIG_I2C_STM32_V2 i2c_ll_stm32.c SEARCH_PATH ${PROJECT_SOURCE_DIR}"/templates/")

zephyr_library_sources_ifdef(CONFIG_I2C_STM32_V1 i2c_ll_stm32.c)
zephyr_library_sources_ifdef(CONFIG_I2C_STM32_V2 i2c_ll_stm32.c)


zephyr_library_sources_codegen_ifdef(CONFIG_I2C_STM32_V1 i2c_ll_stm32_device_template.h CODEGEN_OUTPUT i2c_ll_stm32_device.h SEARCH_PATH ${PROJECT_SOURCE_DIR}"/templates/")
zephyr_library_sources_codegen_ifdef(CONFIG_I2C_STM32_V2 i2c_ll_stm32_device_template.h CODEGEN_OUTPUT i2c_ll_stm32_device.h SEARCH_PATH ${PROJECT_SOURCE_DIR}"/templates/")

zephyr_library_sources_ifdef(CONFIG_USERSPACE i2c_handlers.c)

Expand Down
5 changes: 2 additions & 3 deletions drivers/i2c/i2c_ll_stm32.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
{% import 'i2c_ll_stm32.meta.jinja2' as i2c_stm32 %}

/*
* Copyright (c) 2016 BayLibre, SAS
* Copyright (c) 2017 Linaro Ltd
Expand Down Expand Up @@ -214,4 +212,5 @@ static int i2c_stm32_init(struct device *dev)
return 0;
}

{{ i2c_stm32.device(data, ['st,stm32-i2c-v1', 'st,stm32-i2c-v2']) }}
#include "i2c_ll_stm32_device.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i2c_ll_stm32_devices.h would make more sense.

Then, should we include .c or .h file?
I'm not sure to have any preference, but question should be asked.
@carlescufi ? @galak ?
Any preference?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I go back and forth about this, but maybe .c, and adding a comment about so something like:

/* Include generated code */
#include "i2c_ll_stm32_device.c"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of don't like either and would prefer to render the content directly into the driver file instead.

Like I did here https://github.com/evenl/zephyr/blob/843cfad25ea6c1c6347b77365f8e4d5e171c868d/drivers/spi/spi_ll_stm32.c.jinja2

But I know that I am probably in minority here ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evenl
After thinking about this and discussing it with people, I really dislike the templated C file. I would like my .c file to be a .c file, clean of templating and containing only .c code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I am anyway generating .h file as you requested in the first place.


8 changes: 8 additions & 0 deletions drivers/i2c/i2c_ll_stm32_device_template.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{% import 'i2c_ll_stm32.meta.jinja2' as i2c_stm32 %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an .h file that should really be an .h.jinja2 file. See my comment here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably misunderstood you, but I though you wanted me to have the .h so that the IDE's would recognize it as .h file, but that was maybe for when I rendered the whole driver file and not this file that is included in the driver file?


#ifndef _I2C_LL_STM32_DEVICE_H_
#define _I2C_LL_STM32_DEVICE_H_

{{ i2c_stm32.device(data, ['st,stm32-i2c-v1', 'st,stm32-i2c-v2']) }}

#endif
2 changes: 1 addition & 1 deletion drivers/spi/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ zephyr_library()

zephyr_library_sources_ifdef(CONFIG_SPI_DW spi_dw.c)
zephyr_library_sources_ifdef(CONFIG_SPI_INTEL spi_intel.c)
zephyr_library_sources_codegen_ifdef(CONFIG_SPI_STM32 spi_ll_stm32.c.jinja2 SEARCH_PATH ${PROJECT_SOURCE_DIR}"/templates/")
zephyr_library_sources_codegen_ifdef(CONFIG_SPI_STM32 spi_ll_stm32_device_template.h CODEGEN_OUTPUT spi_ll_stm32_device.h SEARCH_PATH ${PROJECT_SOURCE_DIR}"/templates/")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a more easy to use macro?
Ideally one should only call zephyr_library_sources_jinja2_ifdef(CONFIG_SPI_STM32 spi_ll_stm32.c)
Then jinja2 should generate spi_ll_stm32_devices.c/h from spi_ll_stm32_devices.c.jinja

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, you can omit CODEGEN_OUTPUT and SEARCH_PATH if you don't need them

CODEGEN_OUTPUT is to control the name of the rendered file which can be nice in many cases

SEARCH_PATH is set "include_paths", so if you have templates in other folders that you want to reuse in your template (with the include/import or extends keywords)

zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_DSPI spi_mcux_dspi.c)
zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI spi_mcux_lpspi.c)
zephyr_library_sources_ifdef(CONFIG_SPI_SAM spi_sam.c)
Expand Down
5 changes: 2 additions & 3 deletions drivers/spi/spi_ll_stm32.c.jinja2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
{% import 'spi_ll_stm32.meta.jinja2' as spi_stm32 %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you still need to have jinja2 extension on this file?


/*
* Copyright (c) 2016 BayLibre, SAS
* Copyright (c) 2018 Nordic Semiconductor ASA
Expand Down Expand Up @@ -480,4 +478,5 @@ static int spi_stm32_init(struct device *dev)
return 0;
}

{{ spi_stm32.device(data, ['st,stm32-spi', 'st,stm32-spi-fifo']) }}
#include "spi_ll_stm32_device.h"

4 changes: 0 additions & 4 deletions scripts/render_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ def write_generated_output(self, output_str):
def render(self):
return self.template_handle.render(data=self.data)


def main(self, argv):

argv = argv[1:]
Expand All @@ -71,15 +70,12 @@ def main(self, argv):
include_paths.append(input_path)
include_paths.extend(self.data['runtime']['include_path'])

print(json.dumps(self.data, sort_keys=True, indent=4))

loader = FileSystemLoader(include_paths)
env = Environment(loader=loader, extensions=['jinja2.ext.do','jinja2.ext.loopcontrols'])
self.template_handle = env.get_template(input_filename)

self.write_generated_output(self.render())


if __name__ == '__main__':

ret = CodeGen().main(sys.argv)
Expand Down