linux-toradex.git/drivers/char/ipmi/ipmi_si_platform.c, branch v6.17 Linux kernel for Apalis and Colibri modules ipmi:si: Move SI type information into an info structure 2025-05-07T22:25:47+00:00 Corey Minyard corey@minyard.net 2025-04-15T19:30:58+00:00 9cba938aecbbffe95286e0b07f21ec6d4b12b42c Andy reported: Debian clang version 19.1.7 is not happy when compiled with `make W=1` (note, CONFIG_WERROR=y is the default): ipmi_si_platform.c:268:15: error: cast to smaller integer type 'enum si_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] 268 | io.si_type = (enum si_type)device_get_match_data(&pdev->dev); The IPMI SI type is an enum that was cast into a pointer that was then cast into an enum again. That's not the greatest style, so instead create an info structure to hold the data and use that. Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Closes: https://lore.kernel.org/lkml/20250415085156.446430-1-andriy.shevchenko@linux.intel.com/ Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <corey@minyard.net> 
Andy reported:

Debian clang version 19.1.7 is not happy when compiled with
`make W=1` (note, CONFIG_WERROR=y is the default):

ipmi_si_platform.c:268:15: error: cast to smaller integer type 'enum si_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
  268 |         io.si_type      = (enum si_type)device_get_match_data(&pdev->dev);

The IPMI SI type is an enum that was cast into a pointer that was
then cast into an enum again.  That's not the greatest style, so
instead create an info structure to hold the data and use that.

Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Closes: https://lore.kernel.org/lkml/20250415085156.446430-1-andriy.shevchenko@linux.intel.com/
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Corey Minyard <corey@minyard.net>
Get rid of 'remove_new' relic from platform driver struct 2024-12-01T23:12:43+00:00 Linus Torvalds torvalds@linux-foundation.org 2024-12-01T23:12:43+00:00 e70140ba0d2b1a30467d4af6bcfe761327b9ec95 The continual trickle of small conversion patches is grating on me, and is really not helping. Just get rid of the 'remove_new' member function, which is just an alias for the plain 'remove', and had a comment to that effect: /* * .remove_new() is a relic from a prototype conversion of .remove(). * New drivers are supposed to implement .remove(). Once all drivers are * converted to not use .remove_new any more, it will be dropped. */ This was just a tree-wide 'sed' script that replaced '.remove_new' with '.remove', with some care taken to turn a subsequent tab into two tabs to make things line up. I did do some minimal manual whitespace adjustment for places that used spaces to line things up. Then I just removed the old (sic) .remove_new member function, and this is the end result. No more unnecessary conversion noise. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> 
The continual trickle of small conversion patches is grating on me, and
is really not helping.  Just get rid of the 'remove_new' member
function, which is just an alias for the plain 'remove', and had a
comment to that effect:

  /*
   * .remove_new() is a relic from a prototype conversion of .remove().
   * New drivers are supposed to implement .remove(). Once all drivers are
   * converted to not use .remove_new any more, it will be dropped.
   */

This was just a tree-wide 'sed' script that replaced '.remove_new' with
'.remove', with some care taken to turn a subsequent tab into two tabs
to make things line up.

I did do some minimal manual whitespace adjustment for places that used
spaces to line things up.

Then I just removed the old (sic) .remove_new member function, and this
is the end result.  No more unnecessary conversion noise.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
ipmi: ipmi_si_platform: Convert to platform remove callback returning void 2024-04-17T19:55:04+00:00 Uwe Kleine-König u.kleine-koenig@pengutronix.de 2024-03-05T16:27:00+00:00 f99a996574275d33d64ee1fdee6b49369bc07a9f The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Message-Id: <789cd7876780241430dd5604bc4322453fe4e581.1709655755.git.u.kleine-koenig@pengutronix.de> Signed-off-by: Corey Minyard <minyard@acm.org> 
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Message-Id: <789cd7876780241430dd5604bc4322453fe4e581.1709655755.git.u.kleine-koenig@pengutronix.de>
Signed-off-by: Corey Minyard <minyard@acm.org>
ipmi: si: Use device_get_match_data() 2023-12-19T12:32:27+00:00 Rob Herring robh@kernel.org 2023-11-15T21:02:29+00:00 5be50eb5ae99d890cc22ab49753330cce8731599 Use preferred device_get_match_data() instead of of_match_device() to get the driver match data. With this, adjust the includes to explicitly include the correct headers. Signed-off-by: Rob Herring <robh@kernel.org> Message-Id: <20231115210230.3744198-1-robh@kernel.org> Signed-off-by: Corey Minyard <minyard@acm.org> 
Use preferred device_get_match_data() instead of of_match_device() to
get the driver match data. With this, adjust the includes to explicitly
include the correct headers.

Signed-off-by: Rob Herring <robh@kernel.org>
Message-Id: <20231115210230.3744198-1-robh@kernel.org>
Signed-off-by: Corey Minyard <minyard@acm.org>
ipmi_si: fix -Wvoid-pointer-to-enum-cast warning 2023-08-15T20:46:06+00:00 Justin Stitt justinstitt@google.com 2023-08-09T21:05:17+00:00 d40f09c1a23024f0e550d9423f4d389672e1dfaf With W=1 we see the following warning: | drivers/char/ipmi/ipmi_si_platform.c:272:15: error: \ | cast to smaller integer type 'enum si_type' from \ | 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] | 272 | io.si_type = (enum si_type) match->data; | | ^~~~~~~~~~~~~~~~~~~~~~~~~~ This is due to the fact that the `si_type` enum members are int-width and a cast from pointer-width down to int will cause truncation and possible data loss. Although in this case `si_type` has only a few enumerated fields and thus there is likely no data loss occurring. Nonetheless, this patch is necessary to the goal of promoting this warning out of W=1. Link: https://github.com/ClangBuiltLinux/linux/issues/1902 Link: https://lore.kernel.org/llvm/202308081000.tTL1ElTr-lkp@intel.com/ Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Justin Stitt <justinstitt@google.com> Message-Id: <20230809-cbl-1902-v1-1-92def12d1dea@google.com> Signed-off-by: Corey Minyard <minyard@acm.org> 
With W=1 we see the following warning:

|  drivers/char/ipmi/ipmi_si_platform.c:272:15: error: \
|       cast to smaller integer type 'enum si_type' from \
|       'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
|    272 |         io.si_type      = (enum si_type) match->data;
|        |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~

This is due to the fact that the `si_type` enum members are int-width
and a cast from pointer-width down to int will cause truncation and
possible data loss. Although in this case `si_type` has only a few
enumerated fields and thus there is likely no data loss occurring.
Nonetheless, this patch is necessary to the goal of promoting this
warning out of W=1.

Link: https://github.com/ClangBuiltLinux/linux/issues/1902
Link: https://lore.kernel.org/llvm/202308081000.tTL1ElTr-lkp@intel.com/
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Message-Id: <20230809-cbl-1902-v1-1-92def12d1dea@google.com>
Signed-off-by: Corey Minyard <minyard@acm.org>
ipmi: Change request_module to request_module_nowait 2023-06-20T14:59:53+00:00 Corey Minyard minyard@acm.org 2023-06-20T14:59:53+00:00 e87443a5f68da7bacefe933c90453ae7215263b3 When probing for an ACPI-specified IPMI device, the code would request that the acpi_ipmi module be loaded ACPI operations through IPMI can be performed. This could happen through module load context, for instance, if an I2C module is loaded that caused the IPMI interface to be probed. This is not allowed because a synchronous module load in this context can result in an deadlock, and I was getting a warning: [ 23.967853] WARNING: CPU: 0 PID: 21 at kernel/module/kmod.c:144 __request_module+0x1de/0x2d0 [ 23.968852] Modules linked in: i2c_i801 ipmi_ssif The IPMI driver is not dependent on acpi_ipmi, so just change the called to request_module_nowait to make the load asynchronous. Signed-off-by: Corey Minyard <minyard@acm.org> 
When probing for an ACPI-specified IPMI device, the code would request
that the acpi_ipmi module be loaded ACPI operations through IPMI can be
performed.  This could happen through module load context, for instance,
if an I2C module is loaded that caused the IPMI interface to be probed.

This is not allowed because a synchronous module load in this context
can result in an deadlock, and I was getting a warning:

[   23.967853] WARNING: CPU: 0 PID: 21 at kernel/module/kmod.c:144 __request_module+0x1de/0x2d0
[   23.968852] Modules linked in: i2c_i801 ipmi_ssif

The IPMI driver is not dependent on acpi_ipmi, so just change the called
to request_module_nowait to make the load asynchronous.

Signed-off-by: Corey Minyard <minyard@acm.org>
parisc: Make struct parisc_driver::remove() return void 2021-08-30T08:18:25+00:00 Uwe Kleine-König u.kleine-koenig@pengutronix.de 2021-08-07T09:19:27+00:00 87875c1084a28364dad8cd4f9ecbfdfe0b845ad5 The caller of this function (parisc_driver_remove() in arch/parisc/kernel/drivers.c) ignores the return value, so better don't return any value at all to not wake wrong expectations in driver authors. The only function that could return a non-zero value before was ipmi_parisc_remove() which returns the return value of ipmi_si_remove_by_dev(). Make this function return void, too, as for all other callers the value is ignored, too. Also fold in a small checkpatch fix for: WARNING: Unnecessary space before function pointer arguments + void (*remove) (struct parisc_device *dev); Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> (for drivers/input) Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com> Acked-by: Jiri Slaby <jirislaby@kernel.org> Signed-off-by: Helge Deller <deller@gmx.de> 
The caller of this function (parisc_driver_remove() in
arch/parisc/kernel/drivers.c) ignores the return value, so better don't
return any value at all to not wake wrong expectations in driver authors.

The only function that could return a non-zero value before was
ipmi_parisc_remove() which returns the return value of
ipmi_si_remove_by_dev(). Make this function return void, too, as for all
other callers the value is ignored, too.

Also fold in a small checkpatch fix for:

WARNING: Unnecessary space before function pointer arguments
+	void (*remove) (struct parisc_device *dev);

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> (for drivers/input)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by:  Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Acked-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Helge Deller <deller@gmx.de>
ipmi_si: Join string literals back 2021-04-02T17:53:42+00:00 Andy Shevchenko andriy.shevchenko@linux.intel.com 2021-04-02T17:43:34+00:00 07cbd87b0416d7b6b8419b2a56bc63659de5d066 For easy grepping on debug purposes join string literals back in the messages. No functional change. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Message-Id: <20210402174334.13466-11-andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com> 
For easy grepping on debug purposes join string literals back in
the messages.

No functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Message-Id: <20210402174334.13466-11-andriy.shevchenko@linux.intel.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
ipmi_si: Use proper ACPI macros to check error code for failures 2021-04-02T17:53:25+00:00 Andy Shevchenko andriy.shevchenko@linux.intel.com 2021-04-02T17:43:28+00:00 bd7a33bfa1f1d0295058003899568570a7de0998 Instead of direct comparison, use proper ACPI macros to check error code for failures. While at it, drop unneeded 'else' keyword. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Message-Id: <20210402174334.13466-5-andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com> 
Instead of direct comparison, use proper ACPI macros to check error code
for failures.

While at it, drop unneeded 'else' keyword.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Message-Id: <20210402174334.13466-5-andriy.shevchenko@linux.intel.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
ipmi_si: Utilize temporary variable to hold device pointer 2021-04-02T17:53:22+00:00 Andy Shevchenko andriy.shevchenko@linux.intel.com 2021-04-02T17:43:27+00:00 96c4d0de912c6a7cfc8f7aae1aadff9467d7b996 Introduce a temporary variable to hold a device pointer. It can be utilized in the ->probe() and save a bit of LOCs. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Message-Id: <20210402174334.13466-4-andriy.shevchenko@linux.intel.com> Signed-off-by: Corey Minyard <cminyard@mvista.com> 
Introduce a temporary variable to hold a device pointer.
It can be utilized in the ->probe() and save a bit of LOCs.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Message-Id: <20210402174334.13466-4-andriy.shevchenko@linux.intel.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>