| Message ID | 20240606142424.129709-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,1/3] can: hi311x: simplify with spi_get_device_match_data() | expand |
Hi Krzysztof, On Thu. 6 Jun. 2024 à 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 = &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
On 07.06.2024 12:26:13, Vincent MAILHOL wrote: > Hi Krzysztof, > > On Thu. 6 Jun. 2024 à 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 = &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. I've re-added the intermediate cast to uintptr_t while applying. Thanks, Marc
On 20.06.2024 14:20:53, Marc Kleine-Budde wrote: > On 07.06.2024 12:26:13, Vincent MAILHOL wrote: > > Hi Krzysztof, > > > > On Thu. 6 Jun. 2024 à 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 = &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. > > I've re-added the intermediate cast to uintptr_t while applying. Applied to linux-can-next. Thanks, Marc
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); priv->net = net; priv->clk = clk;
Use spi_get_device_match_data() helper to simplify a bit the driver. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/net/can/spi/hi311x.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)