[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMZ6RqKCWzzbd-P7rOMEryd=31pdD_PJJvtQFYcmS+wAf8q+CQ@mail.gmail.com>
Date: Fri, 7 Jun 2024 12:26:13 +0900
From: Vincent MAILHOL <mailhol.vincent@...adoo.fr>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Marc Kleine-Budde <mkl@...gutronix.de>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, Thomas Kopp <thomas.kopp@...rochip.com>,
linux-can@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 1/3] can: hi311x: simplify with spi_get_device_match_data()
Hi Krzysztof,
On Thu. 6 Jun. 2024 à 23:24, Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
> Use spi_get_device_match_data() helper to simplify a bit the driver.
Thanks for this clean up.
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> ---
> drivers/net/can/spi/hi311x.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c
> index e1b8533a602e..5d2c80f05611 100644
> --- a/drivers/net/can/spi/hi311x.c
> +++ b/drivers/net/can/spi/hi311x.c
> @@ -830,7 +830,6 @@ static int hi3110_can_probe(struct spi_device *spi)
> struct device *dev = &spi->dev;
> struct net_device *net;
> struct hi3110_priv *priv;
> - const void *match;
> struct clk *clk;
> u32 freq;
> int ret;
> @@ -874,11 +873,7 @@ static int hi3110_can_probe(struct spi_device *spi)
> CAN_CTRLMODE_LISTENONLY |
> CAN_CTRLMODE_BERR_REPORTING;
>
> - match = device_get_match_data(dev);
> - if (match)
> - priv->model = (enum hi3110_model)(uintptr_t)match;
> - else
> - priv->model = spi_get_device_id(spi)->driver_data;
> + priv->model = (enum hi3110_model)spi_get_device_match_data(spi);
Here, you are dropping the (uintptr_t) cast. Casting a pointer to an
enum type can trigger a zealous -Wvoid-pointer-to-enum-cast clang
warning, and the (uintptr_t) cast is the defacto standard to silence
such warnings, thus the double (enum hi3110_model)(uintptr_t) cast in
the initial version.
Refer to this thread for examples:
https://lore.kernel.org/linux-can/20210527084532.1384031-12-mkl@pengutronix.de/
Unless you are able to add a rationale in the patch description of why
this cast can now be removed, I suggest you to keep it:
priv->model = (enum
hi3110_model)(uintptr_t)spi_get_device_match_data(spi);
> priv->net = net;
> priv->clk = clk;
Yours sincerely,
Vincent Mailhol
Powered by blists - more mailing lists