Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753967AbbFDRnU (ORCPT ); Thu, 4 Jun 2015 13:43:20 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:36904 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752617AbbFDRnT convert rfc822-to-8bit (ORCPT ); Thu, 4 Jun 2015 13:43:19 -0400 MIME-Version: 1.0 In-Reply-To: <55708139.7010900@vodafone.de> References: <55708139.7010900@vodafone.de> From: Valentin Rothberg Date: Thu, 4 Jun 2015 19:42:47 +0200 Message-ID: Subject: Re: drm/amdkfd: bad CONFIG_ prefix for enum entries To: =?UTF-8?Q?Christian_K=C3=B6nig?= Cc: Alex Deucher , Paul Bolle , Andreas Ruprecht , Maling list - DRI developers , LKML , hengelein Stefan Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6032 Lines: 188 Hi Christian, On Thu, Jun 4, 2015 at 6:47 PM, Christian König wrote: > On 04.06.2015 17:09, Alex Deucher wrote: >> >> On Thu, Jun 4, 2015 at 10:04 AM, Valentin Rothberg >> wrote: >>> >>> Hi Alex, >>> >>> On Thu, Jun 4, 2015 at 4:01 PM, Alex Deucher >>> wrote: >>>> >>>> On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay >>>> 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 >>>>> 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 wrote: > On 04.06.2015 17:09, Alex Deucher wrote: >> >> On Thu, Jun 4, 2015 at 10:04 AM, Valentin Rothberg >> wrote: >>> >>> Hi Alex, >>> >>> On Thu, Jun 4, 2015 at 4:01 PM, Alex Deucher >>> wrote: >>>> >>>> On Thu, Jun 4, 2015 at 9:48 AM, Oded Gabbay >>>> 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 >>>>> 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/