Received-SPF: pass (google.com: domain of linux-kernel+bounces-205267-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240606142424.129709-1-krzysztof.kozlowski@linaro.org>
In-Reply-To: <20240606142424.129709-1-krzysztof.kozlowski@linaro.org>
From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Date: Fri, 7 Jun 2024 12:26:13 +0900
Message-ID: <CAMZ6RqKCWzzbd-P7rOMEryd=31pdD_PJJvtQFYcmS+wAf8q+CQ@mail.gmail.com>
Subject: Re: [PATCH net-next 1/3] can: hi311x: simplify with spi_get_device_match_data()
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>, "David S. Miller" <davem@davemloft.net>, 
	Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, 
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>, Thomas Kopp <thomas.kopp@microchip.com>, 
	linux-can@vger.kernel.org, netdev@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Krzysztof,

On Thu. 6 Jun. 2024 =C3=A0 23:24, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.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@linaro.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 =3D &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 =3D device_get_match_data(dev);
> -       if (match)
> -               priv->model =3D (enum hi3110_model)(uintptr_t)match;
> -       else
> -               priv->model =3D spi_get_device_id(spi)->driver_data;
> +       priv->model =3D (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@pengutron=
ix.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 =3D (enum
hi3110_model)(uintptr_t)spi_get_device_match_data(spi);

>         priv->net =3D net;
>         priv->clk =3D clk;

Yours sincerely,
Vincent Mailhol