[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cad69841-9ca2-45af-9db2-4c4aced63d5e@linaro.org>
Date: Fri, 26 Jan 2024 08:24:19 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Sam Protsenko <semen.protsenko@...aro.org>
Cc: broonie@...nel.org, andi.shyti@...nel.org, arnd@...db.de,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
alim.akhtar@...sung.com, linux-spi@...r.kernel.org,
linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-arch@...r.kernel.org, andre.draszik@...aro.org,
peter.griffin@...aro.org, kernel-team@...roid.com, willmcvicker@...gle.com
Subject: Re: [PATCH v2 07/28] spi: s3c64xx: remove unneeded (void *) casts in
of_match_table
Thanks for the review feedback, Sam, great catches so far!
On 1/25/24 19:04, Sam Protsenko wrote:
> On Thu, Jan 25, 2024 at 8:50 AM Tudor Ambarus <tudor.ambarus@...aro.org> wrote:
>>
>> of_device_id::data is an opaque pointer. No explicit cast is needed.
>> Remove unneeded (void *) casts in of_match_table. While here align the
>> compatible and data members.
>>
>> Reviewed-by: Andi Shyti <andi.shyti@...nel.org>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@...aro.org>
>> ---
>> drivers/spi/spi-s3c64xx.c | 45 +++++++++++++++++++++++----------------
>> 1 file changed, 27 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index 230fda2b3417..137faf9f2697 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -1511,32 +1511,41 @@ static const struct platform_device_id s3c64xx_spi_driver_ids[] = {
>> };
>>
>> static const struct of_device_id s3c64xx_spi_dt_match[] = {
>> - { .compatible = "samsung,s3c2443-spi",
>> - .data = (void *)&s3c2443_spi_port_config,
>
> I support removing (void *) cast. But this new braces style:
>
> },
> {
this style was there before my patch.
>
> seems to bloat the code a bit. For my taste, having something like },
> { on the same line would be more compact, and more canonical so to
I don't lean towards neither of the styles, I'm ok with both
> speak. Or even preserving the existing style would be ok too, for that
> matter.
>
seeing .compatible and .data unaligned hurt my eyes and I think that
aligning them while dropping the cast is fine. I don't really want to do
the style change unless you, Andi or Mark insist. Would you please come
with a patch on top if you really want them changed?
> Assuming the braces style is fixed, you can add:
>
> Reviewed-by: Sam Protsenko <semen.protsenko@...aro.org>
>
>> + {
>> + .compatible = "samsung,s3c2443-spi",
>> + .data = &s3c2443_spi_port_config,
>> },
>> - { .compatible = "samsung,s3c6410-spi",
>> - .data = (void *)&s3c6410_spi_port_config,
>> + {
>> + .compatible = "samsung,s3c6410-spi",
>> + .data = &s3c6410_spi_port_config,
>> },
>> - { .compatible = "samsung,s5pv210-spi",
>> - .data = (void *)&s5pv210_spi_port_config,
>> + {
>> + .compatible = "samsung,s5pv210-spi",
>> + .data = &s5pv210_spi_port_config,
>> },
>> - { .compatible = "samsung,exynos4210-spi",
>> - .data = (void *)&exynos4_spi_port_config,
>> + {
>> + .compatible = "samsung,exynos4210-spi",
>> + .data = &exynos4_spi_port_config,
>> },
>> - { .compatible = "samsung,exynos7-spi",
>> - .data = (void *)&exynos7_spi_port_config,
>> + {
>> + .compatible = "samsung,exynos7-spi",
>> + .data = &exynos7_spi_port_config,
>> },
>> - { .compatible = "samsung,exynos5433-spi",
>> - .data = (void *)&exynos5433_spi_port_config,
>> + {
>> + .compatible = "samsung,exynos5433-spi",
>> + .data = &exynos5433_spi_port_config,
>> },
>> - { .compatible = "samsung,exynos850-spi",
>> - .data = (void *)&exynos850_spi_port_config,
>> + {
>> + .compatible = "samsung,exynos850-spi",
>> + .data = &exynos850_spi_port_config,
>> },
>> - { .compatible = "samsung,exynosautov9-spi",
>> - .data = (void *)&exynosautov9_spi_port_config,
>> + {
>> + .compatible = "samsung,exynosautov9-spi",
>> + .data = &exynosautov9_spi_port_config,
>> },
>> - { .compatible = "tesla,fsd-spi",
>> - .data = (void *)&fsd_spi_port_config,
>> + {
>> + .compatible = "tesla,fsd-spi",
>> + .data = &fsd_spi_port_config,
>> },
>> { },
>> };
>> --
>> 2.43.0.429.g432eaa2c6b-goog
>>
Powered by blists - more mailing lists