* [PATCH] crypto: ccp: Remove forward declaration
@ 2018-09-24 17:26 Nathan Chancellor
2018-09-24 19:18 ` Gary R Hook
2018-10-05 2:27 ` Herbert Xu
0 siblings, 2 replies; 11+ messages in thread
From: Nathan Chancellor @ 2018-09-24 17:26 UTC (permalink / raw)
To: Tom Lendacky, Gary Hook, Herbert Xu
Cc: linux-crypto, linux-kernel, Nick Desaulniers, Nathan Chancellor
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 <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 17:26 [PATCH] crypto: ccp: Remove forward declaration Nathan Chancellor
@ 2018-09-24 19:18 ` Gary R Hook
2018-09-24 19:40 ` Nathan Chancellor
2018-09-24 20:24 ` Nick Desaulniers
2018-10-05 2:27 ` Herbert Xu
1 sibling, 2 replies; 11+ messages in thread
From: Gary R Hook @ 2018-09-24 19:18 UTC (permalink / raw)
To: Nathan Chancellor, Lendacky, Thomas, Hook, Gary, Herbert Xu
Cc: linux-crypto, linux-kernel, Nick Desaulniers
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 <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> 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",
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 19:18 ` Gary R Hook
@ 2018-09-24 19:40 ` Nathan Chancellor
2018-09-24 21:22 ` Gary R Hook
2018-09-24 20:24 ` Nick Desaulniers
1 sibling, 1 reply; 11+ messages in thread
From: Nathan Chancellor @ 2018-09-24 19:40 UTC (permalink / raw)
To: Gary R Hook
Cc: Lendacky, Thomas, Hook, Gary, Herbert Xu, linux-crypto,
linux-kernel, Nick Desaulniers
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 <ndesaulniers@google.com>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > 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",
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 19:18 ` Gary R Hook
2018-09-24 19:40 ` Nathan Chancellor
@ 2018-09-24 20:24 ` Nick Desaulniers
2018-09-24 21:27 ` Gary R Hook
1 sibling, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2018-09-24 20:24 UTC (permalink / raw)
To: ghook
Cc: Nathan Chancellor, Thomas.Lendacky, Gary.Hook, Herbert Xu,
linux-crypto, LKML
On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <ghook@amd.com> 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 <ndesaulniers@google.com>
>
> 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 <ndesaulniers@google.com>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 19:40 ` Nathan Chancellor
@ 2018-09-24 21:22 ` Gary R Hook
2018-09-24 21:42 ` Nick Desaulniers
0 siblings, 1 reply; 11+ messages in thread
From: Gary R Hook @ 2018-09-24 21:22 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Lendacky, Thomas, Hook, Gary, Herbert Xu, linux-crypto,
linux-kernel, 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 20:24 ` Nick Desaulniers
@ 2018-09-24 21:27 ` Gary R Hook
2018-09-24 21:44 ` Nick Desaulniers
0 siblings, 1 reply; 11+ messages in thread
From: Gary R Hook @ 2018-09-24 21:27 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Lendacky, Thomas, Hook, Gary, Herbert Xu,
linux-crypto, LKML
On 09/24/2018 03:24 PM, Nick Desaulniers wrote:
> On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <ghook@amd.com> 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 <ndesaulniers@google.com>
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 <gary.hook@amd.com>
>
>>
>> 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 <ndesaulniers@google.com>
>>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>>> ---
>>> 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",
>>>
>>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 21:22 ` Gary R Hook
@ 2018-09-24 21:42 ` Nick Desaulniers
2018-09-24 21:44 ` Nathan Chancellor
0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2018-09-24 21:42 UTC (permalink / raw)
To: ghook
Cc: Nathan Chancellor, Thomas.Lendacky, Gary.Hook, Herbert Xu,
linux-crypto, LKML
On Mon, Sep 24, 2018 at 2:22 PM Gary R Hook <ghook@amd.com> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 21:27 ` Gary R Hook
@ 2018-09-24 21:44 ` Nick Desaulniers
2018-09-24 23:41 ` Gary R Hook
0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2018-09-24 21:44 UTC (permalink / raw)
To: ghook
Cc: Nathan Chancellor, Thomas.Lendacky, Gary.Hook, Herbert Xu,
linux-crypto, LKML
On Mon, Sep 24, 2018 at 2:27 PM Gary R Hook <ghook@amd.com> wrote:
>
> On 09/24/2018 03:24 PM, Nick Desaulniers wrote:
> > On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <ghook@amd.com> 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 <ndesaulniers@google.com>
>
>
> 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 <gary.hook@amd.com>
>
>
> >
> >>
> >> 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 <ndesaulniers@google.com>
> >>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> >>> ---
> >>> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 21:42 ` Nick Desaulniers
@ 2018-09-24 21:44 ` Nathan Chancellor
0 siblings, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2018-09-24 21:44 UTC (permalink / raw)
To: Nick Desaulniers
Cc: ghook, Thomas.Lendacky, Gary.Hook, Herbert Xu, linux-crypto, LKML
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 <ghook@amd.com> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 21:44 ` Nick Desaulniers
@ 2018-09-24 23:41 ` Gary R Hook
0 siblings, 0 replies; 11+ messages in thread
From: Gary R Hook @ 2018-09-24 23:41 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Lendacky, Thomas, Hook, Gary, Herbert Xu,
linux-crypto, LKML
On 09/24/2018 04:44 PM, Nick Desaulniers wrote:
> On Mon, Sep 24, 2018 at 2:27 PM Gary R Hook <ghook@amd.com> wrote:
>>
>> On 09/24/2018 03:24 PM, Nick Desaulniers wrote:
>>> On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <ghook@amd.com> 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 <ndesaulniers@google.com>
>>
>>
>> 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 <gary.hook@amd.com>
>>
>>
>>>
>>>>
>>>> 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 <ndesaulniers@google.com>
>>>>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>>>>> ---
>>>>> 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",
>>>>>
>>>>
>>>
>>>
>>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] crypto: ccp: Remove forward declaration
2018-09-24 17:26 [PATCH] crypto: ccp: Remove forward declaration Nathan Chancellor
2018-09-24 19:18 ` Gary R Hook
@ 2018-10-05 2:27 ` Herbert Xu
1 sibling, 0 replies; 11+ messages in thread
From: Herbert Xu @ 2018-10-05 2:27 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Tom Lendacky, Gary Hook, linux-crypto, linux-kernel, Nick Desaulniers
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 <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 28 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-10-05 2:27 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-24 17:26 [PATCH] crypto: ccp: Remove forward declaration Nathan Chancellor
2018-09-24 19:18 ` Gary R Hook
2018-09-24 19:40 ` Nathan Chancellor
2018-09-24 21:22 ` Gary R Hook
2018-09-24 21:42 ` Nick Desaulniers
2018-09-24 21:44 ` Nathan Chancellor
2018-09-24 20:24 ` Nick Desaulniers
2018-09-24 21:27 ` Gary R Hook
2018-09-24 21:44 ` Nick Desaulniers
2018-09-24 23:41 ` Gary R Hook
2018-10-05 2:27 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).