Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum { + CONFIG_REG_BASE = 0x2000, /* in dwords */ + CONFIG_REG_END = 0x2B00, + CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE +};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
Hi Oded,
On Thu, Jun 4, 2015 at 3:48 PM, Oded Gabbay oded.gabbay@gmail.com wrote:
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
With pleasure, I am happy if I can help. Do you have any preference to change the prefix to something else? As there are three other symbols SH_REG_{BASE,SIZE,END}, I would rename CONFIG_ to CONF_ to avoid mix-ups.
Kind regards, Valentin
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay oded.gabbay@gmail.com wrote:
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
Please try and keep CONFIG in the name since this range of registers are called CONFIG registers.
Alex
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Alex,
On Thu, Jun 4, 2015 at 4:01 PM, Alex Deucher alexdeucher@gmail.com wrote:
On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay oded.gabbay@gmail.com wrote:
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
Please try and keep CONFIG in the name since this range of registers are called CONFIG registers.
I cannot force changing those symbols, but point out that it's violating naming conventions. I would suggest to s/CONFIG_/CONF_/ to make clear that it's config registers. Would you be fine with that?
Kind regards, Valentin
Alex
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 4, 2015 at 10:04 AM, Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Alex,
On Thu, Jun 4, 2015 at 4:01 PM, Alex Deucher alexdeucher@gmail.com wrote:
On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay oded.gabbay@gmail.com wrote:
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
Please try and keep CONFIG in the name since this range of registers are called CONFIG registers.
I cannot force changing those symbols, but point out that it's violating naming conventions. I would suggest to s/CONFIG_/CONF_/ to make clear that it's config registers. Would you be fine with that?
What about something like AMD_CONFIG_REG?
Kind regards, Valentin
Alex
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 04.06.2015 17:09, Alex Deucher wrote:
On Thu, Jun 4, 2015 at 10:04 AM, Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Alex,
On Thu, Jun 4, 2015 at 4:01 PM, Alex Deucher alexdeucher@gmail.com wrote:
On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay oded.gabbay@gmail.com wrote:
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
Please try and keep CONFIG in the name since this range of registers are called CONFIG registers.
I cannot force changing those symbols, but point out that it's violating naming conventions. I would suggest to s/CONFIG_/CONF_/ to make clear that it's config registers. Would you be fine with that?
What about something like AMD_CONFIG_REG?
For the background: The register headers will be auto generated in the future and if the hardware designer named the register CONFIG_* the name will show up in our headers as such.
Prefixing it with AMD_ sounds like a good solution to me, too.
Regards, Christian.
Kind regards, Valentin
Alex
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Christian,
On Thu, Jun 4, 2015 at 6:47 PM, Christian König deathsimple@vodafone.de wrote:
On 04.06.2015 17:09, Alex Deucher wrote:
On Thu, Jun 4, 2015 at 10:04 AM, Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Alex,
On Thu, Jun 4, 2015 at 4:01 PM, Alex Deucher alexdeucher@gmail.com wrote:
On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay oded.gabbay@gmail.com wrote:
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
Please try and keep CONFIG in the name since this range of registers are called CONFIG registers.
I cannot force changing those symbols, but point out that it's violating naming conventions. I would suggest to s/CONFIG_/CONF_/ to make clear that it's config registers. Would you be fine with that?
What about something like AMD_CONFIG_REG?
For the background: The register headers will be auto generated in the future and if the hardware designer named the register CONFIG_* the name will show up in our headers as such.
Prefixing it with AMD_ sounds like a good solution to me, too.
Okay. I will prepare and send a patch tomorrow.
Kind regards, Valentin
Regards, Christian.
Kind regards, Valentin
Alex
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 4, 2015 at 6:47 PM, Christian König deathsimple@vodafone.de wrote:
On 04.06.2015 17:09, Alex Deucher wrote:
On Thu, Jun 4, 2015 at 10:04 AM, Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Alex,
On Thu, Jun 4, 2015 at 4:01 PM, Alex Deucher alexdeucher@gmail.com wrote:
On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay oded.gabbay@gmail.com wrote:
Hi Valentin, Thanks for catching that. I would be grateful if you could fix this yourself.
Please try and keep CONFIG in the name since this range of registers are called CONFIG registers.
I cannot force changing those symbols, but point out that it's violating naming conventions. I would suggest to s/CONFIG_/CONF_/ to make clear that it's config registers. Would you be fine with that?
What about something like AMD_CONFIG_REG?
For the background: The register headers will be auto generated in the future and if the hardware designer named the register CONFIG_* the name will show up in our headers as such.
Prefixing it with AMD_ sounds like a good solution to me, too.
Regards, Christian.
Kind regards, Valentin
Alex
Oded
On Thu, Jun 4, 2015 at 4:45 PM Valentin Rothberg valentinrothberg@gmail.com wrote:
Hi Yair,
your commit fbeb661bfa89 ("drm/amdkfd: Add skeleton H/W debugger module support") has shown up in today's linux-next tree (i.e., next-20150604). The commit adds the following lines of code to drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h:
+/* CONFIG reg space definition */ +enum {
CONFIG_REG_BASE = 0x2000, /* in dwords */CONFIG_REG_END = 0x2B00,CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE+};
There is a problem with the 'CONFIG_' prefix of those entries. This prefix is reserved for Kconfig options in Make/Kbuild and CPP syntax, so that static analysis tools (and readers of the code) may mistakenly assume that the symbol is defined somewhere in a Kconfig file.
I detected the issue with ./scripts/checkkconfigsymbols.py. Would you mind renaming those entries to something without the 'CONFIG_' prefix? I can also take care of it if you wish to.
Kind regards, Valentin
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König -
Oded Gabbay -
Valentin Rothberg