Clang emits a warning about this construct:
drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
definition assumed to have one element
static const struct acpi_device_id sp_acpi_match[];
^
1 warning generated.
Just remove the forward declarations and move the initializations up
so that they can be used in sp_get_of_version and sp_get_acpi_version.
Reported-by: Nick Desaulniers <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
1 file changed, 25 insertions(+), 28 deletions(-)
diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
index 71734f254fd1..b75dc7db2d4a 100644
--- a/drivers/crypto/ccp/sp-platform.c
+++ b/drivers/crypto/ccp/sp-platform.c
@@ -33,8 +33,31 @@ struct sp_platform {
unsigned int irq_count;
};
-static const struct acpi_device_id sp_acpi_match[];
-static const struct of_device_id sp_of_match[];
+static const struct sp_dev_vdata dev_vdata[] = {
+ {
+ .bar = 0,
+#ifdef CONFIG_CRYPTO_DEV_SP_CCP
+ .ccp_vdata = &ccpv3_platform,
+#endif
+ },
+};
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id sp_acpi_match[] = {
+ { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
+#endif
+
+#ifdef CONFIG_OF
+static const struct of_device_id sp_of_match[] = {
+ { .compatible = "amd,ccp-seattle-v1a",
+ .data = (const void *)&dev_vdata[0] },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sp_of_match);
+#endif
static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
{
@@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
}
#endif
-static const struct sp_dev_vdata dev_vdata[] = {
- {
- .bar = 0,
-#ifdef CONFIG_CRYPTO_DEV_SP_CCP
- .ccp_vdata = &ccpv3_platform,
-#endif
- },
-};
-
-#ifdef CONFIG_ACPI
-static const struct acpi_device_id sp_acpi_match[] = {
- { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
- { },
-};
-MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
-#endif
-
-#ifdef CONFIG_OF
-static const struct of_device_id sp_of_match[] = {
- { .compatible = "amd,ccp-seattle-v1a",
- .data = (const void *)&dev_vdata[0] },
- { },
-};
-MODULE_DEVICE_TABLE(of, sp_of_match);
-#endif
-
static struct platform_driver sp_platform_driver = {
.driver = {
.name = "ccp",
--
2.19.0
On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
> Clang emits a warning about this construct:
>
> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
> definition assumed to have one element
> static const struct acpi_device_id sp_acpi_match[];
> ^
> 1 warning generated.
>
> Just remove the forward declarations and move the initializations up
> so that they can be used in sp_get_of_version and sp_get_acpi_version.
I'm not going to out and out object to this just yet.
I am not a clang expert. Can you please provide a make command that
would explain how you precipitated this complaint?
> Reported-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
> index 71734f254fd1..b75dc7db2d4a 100644
> --- a/drivers/crypto/ccp/sp-platform.c
> +++ b/drivers/crypto/ccp/sp-platform.c
> @@ -33,8 +33,31 @@ struct sp_platform {
> unsigned int irq_count;
> };
>
> -static const struct acpi_device_id sp_acpi_match[];
> -static const struct of_device_id sp_of_match[];
> +static const struct sp_dev_vdata dev_vdata[] = {
> + {
> + .bar = 0,
> +#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> + .ccp_vdata = &ccpv3_platform,
> +#endif
> + },
> +};
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id sp_acpi_match[] = {
> + { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> + { },
> +};
> +MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> +#endif
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id sp_of_match[] = {
> + { .compatible = "amd,ccp-seattle-v1a",
> + .data = (const void *)&dev_vdata[0] },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, sp_of_match);
> +#endif
>
> static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
> {
> @@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
> }
> #endif
>
> -static const struct sp_dev_vdata dev_vdata[] = {
> - {
> - .bar = 0,
> -#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> - .ccp_vdata = &ccpv3_platform,
> -#endif
> - },
> -};
> -
> -#ifdef CONFIG_ACPI
> -static const struct acpi_device_id sp_acpi_match[] = {
> - { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> - { },
> -};
> -MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> -#endif
> -
> -#ifdef CONFIG_OF
> -static const struct of_device_id sp_of_match[] = {
> - { .compatible = "amd,ccp-seattle-v1a",
> - .data = (const void *)&dev_vdata[0] },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, sp_of_match);
> -#endif
> -
> static struct platform_driver sp_platform_driver = {
> .driver = {
> .name = "ccp",
>
On Mon, Sep 24, 2018 at 07:18:23PM +0000, Gary R Hook wrote:
> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
> > Clang emits a warning about this construct:
> >
> > drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
> > definition assumed to have one element
> > static const struct acpi_device_id sp_acpi_match[];
> > ^
> > 1 warning generated.
> >
> > Just remove the forward declarations and move the initializations up
> > so that they can be used in sp_get_of_version and sp_get_acpi_version.
>
> I'm not going to out and out object to this just yet.
>
> I am not a clang expert. Can you please provide a make command that
> would explain how you precipitated this complaint?
>
Hi Gary,
I can produce the warning with Clang 6.0 using the following set of
commands:
make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- allyesconfig
./scripts/config -d CONFIG_CPU_BIG_ENDIAN
make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- olddefconfig
make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- drivers/crypto/ccp/sp-platform.o
Nathan
>
> > Reported-by: Nick Desaulniers <[email protected]>
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > ---
> > drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
> > 1 file changed, 25 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
> > index 71734f254fd1..b75dc7db2d4a 100644
> > --- a/drivers/crypto/ccp/sp-platform.c
> > +++ b/drivers/crypto/ccp/sp-platform.c
> > @@ -33,8 +33,31 @@ struct sp_platform {
> > unsigned int irq_count;
> > };
> >
> > -static const struct acpi_device_id sp_acpi_match[];
> > -static const struct of_device_id sp_of_match[];
> > +static const struct sp_dev_vdata dev_vdata[] = {
> > + {
> > + .bar = 0,
> > +#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> > + .ccp_vdata = &ccpv3_platform,
> > +#endif
> > + },
> > +};
> > +
> > +#ifdef CONFIG_ACPI
> > +static const struct acpi_device_id sp_acpi_match[] = {
> > + { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> > +#endif
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id sp_of_match[] = {
> > + { .compatible = "amd,ccp-seattle-v1a",
> > + .data = (const void *)&dev_vdata[0] },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, sp_of_match);
> > +#endif
> >
> > static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
> > {
> > @@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
> > }
> > #endif
> >
> > -static const struct sp_dev_vdata dev_vdata[] = {
> > - {
> > - .bar = 0,
> > -#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> > - .ccp_vdata = &ccpv3_platform,
> > -#endif
> > - },
> > -};
> > -
> > -#ifdef CONFIG_ACPI
> > -static const struct acpi_device_id sp_acpi_match[] = {
> > - { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> > - { },
> > -};
> > -MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> > -#endif
> > -
> > -#ifdef CONFIG_OF
> > -static const struct of_device_id sp_of_match[] = {
> > - { .compatible = "amd,ccp-seattle-v1a",
> > - .data = (const void *)&dev_vdata[0] },
> > - { },
> > -};
> > -MODULE_DEVICE_TABLE(of, sp_of_match);
> > -#endif
> > -
> > static struct platform_driver sp_platform_driver = {
> > .driver = {
> > .name = "ccp",
> >
>
On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <[email protected]> wrote:
>
> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
> > Clang emits a warning about this construct:
> >
> > drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
> > definition assumed to have one element
> > static const struct acpi_device_id sp_acpi_match[];
> > ^
> > 1 warning generated.
> >
> > Just remove the forward declarations and move the initializations up
> > so that they can be used in sp_get_of_version and sp_get_acpi_version.
>
> I'm not going to out and out object to this just yet.
Tentative array definitions need to have the correct length specified;
it should either be forward declared as the correct length, or as this
patch does, skip the forward declare and move up the definition.
Thanks for this patch Nathan.
Reviewed-by: Nick Desaulniers <[email protected]>
>
> I am not a clang expert. Can you please provide a make command that
> would explain how you precipitated this complaint?
>
>
> > Reported-by: Nick Desaulniers <[email protected]>
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > ---
> > drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
> > 1 file changed, 25 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
> > index 71734f254fd1..b75dc7db2d4a 100644
> > --- a/drivers/crypto/ccp/sp-platform.c
> > +++ b/drivers/crypto/ccp/sp-platform.c
> > @@ -33,8 +33,31 @@ struct sp_platform {
> > unsigned int irq_count;
> > };
> >
> > -static const struct acpi_device_id sp_acpi_match[];
> > -static const struct of_device_id sp_of_match[];
> > +static const struct sp_dev_vdata dev_vdata[] = {
> > + {
> > + .bar = 0,
> > +#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> > + .ccp_vdata = &ccpv3_platform,
> > +#endif
> > + },
> > +};
> > +
> > +#ifdef CONFIG_ACPI
> > +static const struct acpi_device_id sp_acpi_match[] = {
> > + { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> > +#endif
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id sp_of_match[] = {
> > + { .compatible = "amd,ccp-seattle-v1a",
> > + .data = (const void *)&dev_vdata[0] },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, sp_of_match);
> > +#endif
> >
> > static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
> > {
> > @@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
> > }
> > #endif
> >
> > -static const struct sp_dev_vdata dev_vdata[] = {
> > - {
> > - .bar = 0,
> > -#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> > - .ccp_vdata = &ccpv3_platform,
> > -#endif
> > - },
> > -};
> > -
> > -#ifdef CONFIG_ACPI
> > -static const struct acpi_device_id sp_acpi_match[] = {
> > - { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> > - { },
> > -};
> > -MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> > -#endif
> > -
> > -#ifdef CONFIG_OF
> > -static const struct of_device_id sp_of_match[] = {
> > - { .compatible = "amd,ccp-seattle-v1a",
> > - .data = (const void *)&dev_vdata[0] },
> > - { },
> > -};
> > -MODULE_DEVICE_TABLE(of, sp_of_match);
> > -#endif
> > -
> > static struct platform_driver sp_platform_driver = {
> > .driver = {
> > .name = "ccp",
> >
>
--
Thanks,
~Nick Desaulniers
On 09/24/2018 02:40 PM, Nathan Chancellor wrote:
> On Mon, Sep 24, 2018 at 07:18:23PM +0000, Gary R Hook wrote:
>> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
>>> Clang emits a warning about this construct:
>>>
>>> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
>>> definition assumed to have one element
>>> static const struct acpi_device_id sp_acpi_match[];
>>> ^
>>> 1 warning generated.
>>>
>>> Just remove the forward declarations and move the initializations up
>>> so that they can be used in sp_get_of_version and sp_get_acpi_version.
>>
>> I'm not going to out and out object to this just yet.
>>
>> I am not a clang expert. Can you please provide a make command that
>> would explain how you precipitated this complaint?
>>
>
> Hi Gary,
>
> I can produce the warning with Clang 6.0 using the following set of
> commands:
>
> make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- allyesconfig
> ./scripts/config -d CONFIG_CPU_BIG_ENDIAN
> make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- olddefconfig
> make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- drivers/crypto/ccp/sp-platform.o
No, I"m not getting a warning on my system. I get this:
ghook@taos:~/src/cryptodev-2.6/src$ make ARCH=arm64 CC=clang
CROSS_COMPILE=aarch64-linux-gnu- CFLAGS=-v
arch/arm64/Makefile:27: ld does not support --fix-cortex-a53-843419;
kernel may be susceptible to erratum
arch/arm64/Makefile:40: LSE atomics not supported by binutils
arch/arm64/Makefile:48: Detected assembler with broken .inst;
disassembly will be unreliable
CALL scripts/checksyscalls.sh
VDSOA arch/arm64/kernel/vdso/gettimeofday.o
arch/arm64/kernel/vdso/gettimeofday.S: Assembler messages:
arch/arm64/kernel/vdso/gettimeofday.S:28: Error: no such instruction:
`vdso_data .req x6'
arch/arm64/kernel/vdso/gettimeofday.S:29: Error: no such instruction:
`seqcnt .req w7'
arch/arm64/kernel/vdso/gettimeofday.S:30: Error: no such instruction:
`w_tmp .req w8'
...
The only reason I bring this up is that it would be helpful to be able
to recreate results. I figure I'm not set up for this.
That said... please see my response to Nick.
On 09/24/2018 03:24 PM, Nick Desaulniers wrote:
> On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <[email protected]> wrote:
>>
>> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
>>> Clang emits a warning about this construct:
>>>
>>> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
>>> definition assumed to have one element
>>> static const struct acpi_device_id sp_acpi_match[];
>>> ^
>>> 1 warning generated.
>>>
>>> Just remove the forward declarations and move the initializations up
>>> so that they can be used in sp_get_of_version and sp_get_acpi_version.
>>
>> I'm not going to out and out object to this just yet.
>
> Tentative array definitions need to have the correct length specified;
> it should either be forward declared as the correct length, or as this
> patch does, skip the forward declare and move up the definition.
> Thanks for this patch Nathan.
> Reviewed-by: Nick Desaulniers <[email protected]>
Checked my C99, and using both static and [] to create a declaration
(because it's really a definition) apparently isn't valid. I should have
known that. Since I'm old school...
Acked-by: Gary R Hook <[email protected]>
>
>>
>> I am not a clang expert. Can you please provide a make command that
>> would explain how you precipitated this complaint?
>>
>>
>>> Reported-by: Nick Desaulniers <[email protected]>
>>> Signed-off-by: Nathan Chancellor <[email protected]>
>>> ---
>>> drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
>>> 1 file changed, 25 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
>>> index 71734f254fd1..b75dc7db2d4a 100644
>>> --- a/drivers/crypto/ccp/sp-platform.c
>>> +++ b/drivers/crypto/ccp/sp-platform.c
>>> @@ -33,8 +33,31 @@ struct sp_platform {
>>> unsigned int irq_count;
>>> };
>>>
>>> -static const struct acpi_device_id sp_acpi_match[];
>>> -static const struct of_device_id sp_of_match[];
>>> +static const struct sp_dev_vdata dev_vdata[] = {
>>> + {
>>> + .bar = 0,
>>> +#ifdef CONFIG_CRYPTO_DEV_SP_CCP
>>> + .ccp_vdata = &ccpv3_platform,
>>> +#endif
>>> + },
>>> +};
>>> +
>>> +#ifdef CONFIG_ACPI
>>> +static const struct acpi_device_id sp_acpi_match[] = {
>>> + { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
>>> + { },
>>> +};
>>> +MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
>>> +#endif
>>> +
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id sp_of_match[] = {
>>> + { .compatible = "amd,ccp-seattle-v1a",
>>> + .data = (const void *)&dev_vdata[0] },
>>> + { },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sp_of_match);
>>> +#endif
>>>
>>> static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
>>> {
>>> @@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
>>> }
>>> #endif
>>>
>>> -static const struct sp_dev_vdata dev_vdata[] = {
>>> - {
>>> - .bar = 0,
>>> -#ifdef CONFIG_CRYPTO_DEV_SP_CCP
>>> - .ccp_vdata = &ccpv3_platform,
>>> -#endif
>>> - },
>>> -};
>>> -
>>> -#ifdef CONFIG_ACPI
>>> -static const struct acpi_device_id sp_acpi_match[] = {
>>> - { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
>>> - { },
>>> -};
>>> -MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
>>> -#endif
>>> -
>>> -#ifdef CONFIG_OF
>>> -static const struct of_device_id sp_of_match[] = {
>>> - { .compatible = "amd,ccp-seattle-v1a",
>>> - .data = (const void *)&dev_vdata[0] },
>>> - { },
>>> -};
>>> -MODULE_DEVICE_TABLE(of, sp_of_match);
>>> -#endif
>>> -
>>> static struct platform_driver sp_platform_driver = {
>>> .driver = {
>>> .name = "ccp",
>>>
>>
>
>
On Mon, Sep 24, 2018 at 2:22 PM Gary R Hook <[email protected]> wrote:
>
> On 09/24/2018 02:40 PM, Nathan Chancellor wrote:
> > On Mon, Sep 24, 2018 at 07:18:23PM +0000, Gary R Hook wrote:
> >> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
> >>> Clang emits a warning about this construct:
> >>>
> >>> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
> >>> definition assumed to have one element
> >>> static const struct acpi_device_id sp_acpi_match[];
> >>> ^
> >>> 1 warning generated.
> >>>
> >>> Just remove the forward declarations and move the initializations up
> >>> so that they can be used in sp_get_of_version and sp_get_acpi_version.
> >>
> >> I'm not going to out and out object to this just yet.
> >>
> >> I am not a clang expert. Can you please provide a make command that
> >> would explain how you precipitated this complaint?
> >>
> >
> > Hi Gary,
> >
> > I can produce the warning with Clang 6.0 using the following set of
> > commands:
> >
> > make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- allyesconfig
> > ./scripts/config -d CONFIG_CPU_BIG_ENDIAN
> > make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- olddefconfig
> > make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- drivers/crypto/ccp/sp-platform.o
>
> No, I"m not getting a warning on my system. I get this:
>
> ghook@taos:~/src/cryptodev-2.6/src$ make ARCH=arm64 CC=clang
> CROSS_COMPILE=aarch64-linux-gnu- CFLAGS=-v
> arch/arm64/Makefile:27: ld does not support --fix-cortex-a53-843419;
> kernel may be susceptible to erratum
> arch/arm64/Makefile:40: LSE atomics not supported by binutils
./scripts/config -d CONFIG_ARM64_LSE_ATOMICS
> arch/arm64/Makefile:48: Detected assembler with broken .inst;
> disassembly will be unreliable
> CALL scripts/checksyscalls.sh
> VDSOA arch/arm64/kernel/vdso/gettimeofday.o
> arch/arm64/kernel/vdso/gettimeofday.S: Assembler messages:
> arch/arm64/kernel/vdso/gettimeofday.S:28: Error: no such instruction:
> `vdso_data .req x6'
> arch/arm64/kernel/vdso/gettimeofday.S:29: Error: no such instruction:
> `seqcnt .req w7'
> arch/arm64/kernel/vdso/gettimeofday.S:30: Error: no such instruction:
> `w_tmp .req w8'
> ...
>
> The only reason I bring this up is that it would be helpful to be able
> to recreate results. I figure I'm not set up for this.
>
> That said... please see my response to Nick.
--
Thanks,
~Nick Desaulniers
On Mon, Sep 24, 2018 at 2:27 PM Gary R Hook <[email protected]> wrote:
>
> On 09/24/2018 03:24 PM, Nick Desaulniers wrote:
> > On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <[email protected]> wrote:
> >>
> >> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
> >>> Clang emits a warning about this construct:
> >>>
> >>> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
> >>> definition assumed to have one element
> >>> static const struct acpi_device_id sp_acpi_match[];
> >>> ^
> >>> 1 warning generated.
> >>>
> >>> Just remove the forward declarations and move the initializations up
> >>> so that they can be used in sp_get_of_version and sp_get_acpi_version.
> >>
> >> I'm not going to out and out object to this just yet.
> >
> > Tentative array definitions need to have the correct length specified;
> > it should either be forward declared as the correct length, or as this
> > patch does, skip the forward declare and move up the definition.
> > Thanks for this patch Nathan.
> > Reviewed-by: Nick Desaulniers <[email protected]>
>
>
> Checked my C99, and using both static and [] to create a declaration
Note that the kernel uses gnu89, which is ISO C90 + gnu extensions (IIUC).
> (because it's really a definition) apparently isn't valid. I should have
> known that. Since I'm old school...
>
> Acked-by: Gary R Hook <[email protected]>
>
>
> >
> >>
> >> I am not a clang expert. Can you please provide a make command that
> >> would explain how you precipitated this complaint?
> >>
> >>
> >>> Reported-by: Nick Desaulniers <[email protected]>
> >>> Signed-off-by: Nathan Chancellor <[email protected]>
> >>> ---
> >>> drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
> >>> 1 file changed, 25 insertions(+), 28 deletions(-)
> >>>
> >>> diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
> >>> index 71734f254fd1..b75dc7db2d4a 100644
> >>> --- a/drivers/crypto/ccp/sp-platform.c
> >>> +++ b/drivers/crypto/ccp/sp-platform.c
> >>> @@ -33,8 +33,31 @@ struct sp_platform {
> >>> unsigned int irq_count;
> >>> };
> >>>
> >>> -static const struct acpi_device_id sp_acpi_match[];
> >>> -static const struct of_device_id sp_of_match[];
> >>> +static const struct sp_dev_vdata dev_vdata[] = {
> >>> + {
> >>> + .bar = 0,
> >>> +#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> >>> + .ccp_vdata = &ccpv3_platform,
> >>> +#endif
> >>> + },
> >>> +};
> >>> +
> >>> +#ifdef CONFIG_ACPI
> >>> +static const struct acpi_device_id sp_acpi_match[] = {
> >>> + { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> >>> + { },
> >>> +};
> >>> +MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> >>> +#endif
> >>> +
> >>> +#ifdef CONFIG_OF
> >>> +static const struct of_device_id sp_of_match[] = {
> >>> + { .compatible = "amd,ccp-seattle-v1a",
> >>> + .data = (const void *)&dev_vdata[0] },
> >>> + { },
> >>> +};
> >>> +MODULE_DEVICE_TABLE(of, sp_of_match);
> >>> +#endif
> >>>
> >>> static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
> >>> {
> >>> @@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
> >>> }
> >>> #endif
> >>>
> >>> -static const struct sp_dev_vdata dev_vdata[] = {
> >>> - {
> >>> - .bar = 0,
> >>> -#ifdef CONFIG_CRYPTO_DEV_SP_CCP
> >>> - .ccp_vdata = &ccpv3_platform,
> >>> -#endif
> >>> - },
> >>> -};
> >>> -
> >>> -#ifdef CONFIG_ACPI
> >>> -static const struct acpi_device_id sp_acpi_match[] = {
> >>> - { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
> >>> - { },
> >>> -};
> >>> -MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
> >>> -#endif
> >>> -
> >>> -#ifdef CONFIG_OF
> >>> -static const struct of_device_id sp_of_match[] = {
> >>> - { .compatible = "amd,ccp-seattle-v1a",
> >>> - .data = (const void *)&dev_vdata[0] },
> >>> - { },
> >>> -};
> >>> -MODULE_DEVICE_TABLE(of, sp_of_match);
> >>> -#endif
> >>> -
> >>> static struct platform_driver sp_platform_driver = {
> >>> .driver = {
> >>> .name = "ccp",
> >>>
> >>
> >
> >
>
--
Thanks,
~Nick Desaulniers
On Mon, Sep 24, 2018 at 02:42:56PM -0700, Nick Desaulniers wrote:
> On Mon, Sep 24, 2018 at 2:22 PM Gary R Hook <[email protected]> wrote:
> >
> > On 09/24/2018 02:40 PM, Nathan Chancellor wrote:
> > > On Mon, Sep 24, 2018 at 07:18:23PM +0000, Gary R Hook wrote:
> > >> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
> > >>> Clang emits a warning about this construct:
> > >>>
> > >>> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
> > >>> definition assumed to have one element
> > >>> static const struct acpi_device_id sp_acpi_match[];
> > >>> ^
> > >>> 1 warning generated.
> > >>>
> > >>> Just remove the forward declarations and move the initializations up
> > >>> so that they can be used in sp_get_of_version and sp_get_acpi_version.
> > >>
> > >> I'm not going to out and out object to this just yet.
> > >>
> > >> I am not a clang expert. Can you please provide a make command that
> > >> would explain how you precipitated this complaint?
> > >>
> > >
> > > Hi Gary,
> > >
> > > I can produce the warning with Clang 6.0 using the following set of
> > > commands:
> > >
> > > make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- allyesconfig
> > > ./scripts/config -d CONFIG_CPU_BIG_ENDIAN
> > > make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- olddefconfig
> > > make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- drivers/crypto/ccp/sp-platform.o
> >
> > No, I"m not getting a warning on my system. I get this:
> >
> > ghook@taos:~/src/cryptodev-2.6/src$ make ARCH=arm64 CC=clang
> > CROSS_COMPILE=aarch64-linux-gnu- CFLAGS=-v
> > arch/arm64/Makefile:27: ld does not support --fix-cortex-a53-843419;
> > kernel may be susceptible to erratum
> > arch/arm64/Makefile:40: LSE atomics not supported by binutils
>
> ./scripts/config -d CONFIG_ARM64_LSE_ATOMICS
>
D'oh, I keep forgetting to update my gist with my commands...
Thanks!
> > arch/arm64/Makefile:48: Detected assembler with broken .inst;
> > disassembly will be unreliable
> > CALL scripts/checksyscalls.sh
> > VDSOA arch/arm64/kernel/vdso/gettimeofday.o
> > arch/arm64/kernel/vdso/gettimeofday.S: Assembler messages:
> > arch/arm64/kernel/vdso/gettimeofday.S:28: Error: no such instruction:
> > `vdso_data .req x6'
> > arch/arm64/kernel/vdso/gettimeofday.S:29: Error: no such instruction:
> > `seqcnt .req w7'
> > arch/arm64/kernel/vdso/gettimeofday.S:30: Error: no such instruction:
> > `w_tmp .req w8'
> > ...
> >
> > The only reason I bring this up is that it would be helpful to be able
> > to recreate results. I figure I'm not set up for this.
> >
> > That said... please see my response to Nick.
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
On 09/24/2018 04:44 PM, Nick Desaulniers wrote:
> On Mon, Sep 24, 2018 at 2:27 PM Gary R Hook <[email protected]> wrote:
>>
>> On 09/24/2018 03:24 PM, Nick Desaulniers wrote:
>>> On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <[email protected]> wrote:
>>>>
>>>> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
>>>>> Clang emits a warning about this construct:
>>>>>
>>>>> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
>>>>> definition assumed to have one element
>>>>> static const struct acpi_device_id sp_acpi_match[];
>>>>> ^
>>>>> 1 warning generated.
>>>>>
>>>>> Just remove the forward declarations and move the initializations up
>>>>> so that they can be used in sp_get_of_version and sp_get_acpi_version.
>>>>
>>>> I'm not going to out and out object to this just yet.
>>>
>>> Tentative array definitions need to have the correct length specified;
>>> it should either be forward declared as the correct length, or as this
>>> patch does, skip the forward declare and move up the definition.
>>> Thanks for this patch Nathan.
>>> Reviewed-by: Nick Desaulniers <[email protected]>
>>
>>
>> Checked my C99, and using both static and [] to create a declaration
>
> Note that the kernel uses gnu89, which is ISO C90 + gnu extensions (IIUC).
I understand that (and appreciate that ubiquitousness of the dialect)
but in this case it sounds like we are interested in the widest possible
compatibility. Which makes the original patch problematic, and the fix
suggested herein suitable.
>
>> (because it's really a definition) apparently isn't valid. I should have
>> known that. Since I'm old school...
>>
>> Acked-by: Gary R Hook <[email protected]>
>>
>>
>>>
>>>>
>>>> I am not a clang expert. Can you please provide a make command that
>>>> would explain how you precipitated this complaint?
>>>>
>>>>
>>>>> Reported-by: Nick Desaulniers <[email protected]>
>>>>> Signed-off-by: Nathan Chancellor <[email protected]>
>>>>> ---
>>>>> drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
>>>>> 1 file changed, 25 insertions(+), 28 deletions(-)
>>>>>
>>>>> diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
>>>>> index 71734f254fd1..b75dc7db2d4a 100644
>>>>> --- a/drivers/crypto/ccp/sp-platform.c
>>>>> +++ b/drivers/crypto/ccp/sp-platform.c
>>>>> @@ -33,8 +33,31 @@ struct sp_platform {
>>>>> unsigned int irq_count;
>>>>> };
>>>>>
>>>>> -static const struct acpi_device_id sp_acpi_match[];
>>>>> -static const struct of_device_id sp_of_match[];
>>>>> +static const struct sp_dev_vdata dev_vdata[] = {
>>>>> + {
>>>>> + .bar = 0,
>>>>> +#ifdef CONFIG_CRYPTO_DEV_SP_CCP
>>>>> + .ccp_vdata = &ccpv3_platform,
>>>>> +#endif
>>>>> + },
>>>>> +};
>>>>> +
>>>>> +#ifdef CONFIG_ACPI
>>>>> +static const struct acpi_device_id sp_acpi_match[] = {
>>>>> + { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
>>>>> + { },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
>>>>> +#endif
>>>>> +
>>>>> +#ifdef CONFIG_OF
>>>>> +static const struct of_device_id sp_of_match[] = {
>>>>> + { .compatible = "amd,ccp-seattle-v1a",
>>>>> + .data = (const void *)&dev_vdata[0] },
>>>>> + { },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, sp_of_match);
>>>>> +#endif
>>>>>
>>>>> static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
>>>>> {
>>>>> @@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
>>>>> }
>>>>> #endif
>>>>>
>>>>> -static const struct sp_dev_vdata dev_vdata[] = {
>>>>> - {
>>>>> - .bar = 0,
>>>>> -#ifdef CONFIG_CRYPTO_DEV_SP_CCP
>>>>> - .ccp_vdata = &ccpv3_platform,
>>>>> -#endif
>>>>> - },
>>>>> -};
>>>>> -
>>>>> -#ifdef CONFIG_ACPI
>>>>> -static const struct acpi_device_id sp_acpi_match[] = {
>>>>> - { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
>>>>> - { },
>>>>> -};
>>>>> -MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
>>>>> -#endif
>>>>> -
>>>>> -#ifdef CONFIG_OF
>>>>> -static const struct of_device_id sp_of_match[] = {
>>>>> - { .compatible = "amd,ccp-seattle-v1a",
>>>>> - .data = (const void *)&dev_vdata[0] },
>>>>> - { },
>>>>> -};
>>>>> -MODULE_DEVICE_TABLE(of, sp_of_match);
>>>>> -#endif
>>>>> -
>>>>> static struct platform_driver sp_platform_driver = {
>>>>> .driver = {
>>>>> .name = "ccp",
>>>>>
>>>>
>>>
>>>
>>
>
>
On Mon, Sep 24, 2018 at 10:26:15AM -0700, Nathan Chancellor wrote:
> Clang emits a warning about this construct:
>
> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
> definition assumed to have one element
> static const struct acpi_device_id sp_acpi_match[];
> ^
> 1 warning generated.
>
> Just remove the forward declarations and move the initializations up
> so that they can be used in sp_get_of_version and sp_get_acpi_version.
>
> Reported-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 28 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt