It's just noise, really, lots of systems don't have it.
Reported-by: Kamlakant Patel <kamlakantp@marvell.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
According to ipmi spec, block number is a number that is incremented,
starting with 0, for each new block of message data returned using the
middle transaction.
Here, the 'blocknum' is data[0] which always starts from zero(0) and
'ssif_info->multi_pos' starts from 1.
So, we need to add +1 to blocknum while comparing with multi_pos.
Fixes: 7d6380cd40 ("ipmi:ssif: Fix handling of multi-part return messages").
Reported-by: Kiran Kolukuluru <kirank@ami.com>
Signed-off-by: Kamlakant Patel <kamlakantp@marvell.com>
Message-Id: <1556106615-18722-1-git-send-email-kamlakantp@marvell.com>
[Also added a debug log if the block numbers don't match.]
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: stable@vger.kernel.org # 4.4
Trivial fix to clean up an indentation issue, remove space
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
The code to tell the lower layer to enable or disable watching for
certain things was lazy in disabling, it waited until a timer tick
to see if a disable was necessary. Not a really big deal, but it
could be improved.
Modify the code to enable and disable watching immediately and don't
do it from the background timer any more.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Tested-by: Kamlakant Patel <kamlakant.patel@cavium.com>
The IPMI driver has a mechanism to tell the lower layers it needs
to watch for messages, commands, and watchdogs (so it doesn't
needlessly poll). However, it needed some extensions, it needed
a way to tell what is being waited for so it could set the timeout
appropriately.
The update to the lower layer was also being done once a second
at best because it was done in the main timeout handler. However,
if a command is sent and a response message is coming back,
it needed to be started immediately. So modify the code to
update immediately if it needs to be enabled. Disable is still
lazy.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Tested-by: Kamlakant Patel <kamlakant.patel@cavium.com>
Commit 89986496de ("ipmi: Turn off all activity on an idle ipmi
interface") modified the IPMI code to only request events when the
driver had somethine waiting for events. The SSIF code, however,
was using the event fetch request to also fetch the flags.
Add a timer and the proper handling for the upper layer telling
whether flags fetches are required.
Reported-by: Kamlakant Patel <Kamlakant.Patel@cavium.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Tested-by: Kamlakant Patel <kamlakant.patel@cavium.com>
The block number was not being compared right, it was off by one
when checking the response.
Some statistics wouldn't be incremented properly in some cases.
Check to see if that middle-part messages always have 31 bytes of
data.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: stable@vger.kernel.org # 4.4
The spec was fairly confusing about how multi-part transmit messages
worked, so the original implementation only added support for two
part messages. But after talking about it with others and finding
something I missed, I think it makes more sense.
The spec mentions smbus command 8 in a table at the end of the
section on SSIF support as the end transaction. If that works,
then all is good and as it should be. However, some implementations
seem to use a middle transaction <32 bytes tomark the end because of the
confusion in the spec, even though that is an SMBus violation if
the number of bytes is zero.
So this change adds some tests, if command=8 works, it uses that,
otherwise if an empty end transaction works, it uses a middle
transaction <32 bytes to mark the end. If neither works, then
it limits the size to 63 bytes as it is now.
Cc: Harri Hakkarainen <harri@cavium.com>
Cc: Bazhenov, Dmitry <dmitry.bazhenov@auriga.com>
Cc: Mach, Dat <Dat.Mach@cavium.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
The IPMI DMI code was adding platform overrides, which is not
really an ideal solution. Switch to using the id_table in
the drivers to identify the devices.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Add and use #define pr_fmt/dev_fmt, and remove #define PFX
This also prefixes some messages that were not previously prefixed.
Miscellanea:
o Convert printk(KERN_<level> to pr_<level>(
o Use %s, __func__ where appropriate
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
There is a potential execution path in which function ssif_info_find()
returns NULL, hence there is a NULL pointer dereference when accessing
pointer *addr_info*
Fix this by null checking *addr_info* before dereferencing it.
Addresses-Coverity-ID: 1473145 ("Explicit null dereferenced")
Fixes: e333054a91d1 ("ipmi: Fix I2C client removal in the SSIF driver")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
There were certain situations where ipmi_register_smi() would
return a failure, but the interface would still be registered
and would need to be unregistered. This is obviously a bad
design and resulted in an oops in certain failure cases.
If the interface is started up in ipmi_register_smi(), then
an error occurs, shut down the interface there so the
cleanup can be done properly.
Fix the various smi users, too.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reported-by: Justin Ernst <justin.ernst@hpe.com>
Tested-by: Justin Ernst <justin.ernst@hpe.com>
Cc: Andrew Banman <abanman@hpe.com>
Cc: Russ Anderson <russ.anderson@hpe.com>
Cc: <stable@vger.kernel.org> # 4.18.x
Currently, function ssif_remove returns _rv_, which is a variable that
is never initialized.
Fix this by removing variable _rv_ and return 0 instead.
Addresses-Coverity-ID: 1467999 ("Uninitialized scalar variable")
Fixes: 6a0d23ed33 ("ipmi: ipmi_unregister_smi() cannot fail, have it
return void")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Due to changes in the way shutdown is done, it is no longer
required to check that the interface is set.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Move the shutdown handling to a shutdown function called from
the IPMI core code. That makes for a cleaner shutdown.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
This happens when BMC doesn't return any data and the code is trying
to print the value of data[2].
Getting following crash:
[ 484.728410] Unable to handle kernel NULL pointer dereference at virtual address 00000002
[ 484.736496] pgd = ffff0000094a2000
[ 484.739885] [00000002] *pgd=00000047fcffe003, *pud=00000047fcffd003, *pmd=0000000000000000
[ 484.748158] Internal error: Oops: 96000005 [#1] SMP
[...]
[ 485.101451] Call trace:
[...]
[ 485.188473] [<ffff000000a46e68>] msg_done_handler+0x668/0x700 [ipmi_ssif]
[ 485.195249] [<ffff000000a456b8>] ipmi_ssif_thread+0x110/0x128 [ipmi_ssif]
[ 485.202038] [<ffff0000080f1430>] kthread+0x108/0x138
[ 485.206994] [<ffff0000080838e0>] ret_from_fork+0x10/0x30
[ 485.212294] Code: aa1903e1 aa1803e0 b900227f 95fef6a5 (39400aa3)
Adding a check to validate the data len before printing data[2] to fix this issue.
Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
The IPMI spec states:
The purpose of the SPMI Table is to provide a mechanism that can
be used by the OSPM (an ACPI term for “OS Operating System-directed
configuration and Power Management” essentially meaning an ACPI-aware
OS or OS loader) very early in the boot process, e.g., before the
ability to execute ACPI control methods in the OS is available.
When we are probing IPMI in Linux, ACPI control methods are available,
so we shouldn't be probing using SPMI. It could cause some confusion
during the probing process.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Tested-by: Jiandi An <anjiandi@codeaurora.org>
And get rid of the license text that is no longer necessary.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Alistair Popple <alistair@popple.id.au>
Cc: Jeremy Kerr <jk@ozlabs.org>
Cc: Joel Stanley <joel@jms.id.au>
Cc: Rocky Craig <rocky.craig@hp.com>
Since i2c_unregister_device() became NULL-aware we may remove duplicate
NULL check.
Cc: Corey Minyard <minyard@acm.org>
Cc: openipmi-developer@lists.sourceforge.net
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Rework the DMI probe function to be a generic platform probe, and
then rework the DMI code (and a few other things) to use the more
generic information. This is so other things can declare platform
IPMI devices.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Create a device attribute for everything we show in proc, getting
ready for removing the proc stuff.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Currently, ipmi_demagle_device_id requires a full response buffer in its
data argument. This means we can't use it to parse a response in a
struct ipmi_recv_msg, which has the netfn and cmd as separate bytes.
This change alters the definition and users of ipmi_demangle_device_id
to use a split netfn, cmd and data buffer, so it can be used with
non-sequential responses.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Fixed the ipmi_ssif.c and ipmi_si_intf.c changes to use data from the
response, not the data from the message, when passing info to the
ipmi_demangle_device_id() function.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
When getting flags, a response to a different message would
result in a deadlock because of a missing unlock. Add that
unlock and a comment. Found by static analysis.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: stable@vger.kernel.org # 3.19
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Now that the IPMI DMI code creates a platform device for IPMI devices
in the firmware, use that instead of handling all the DMI work
in the IPMI drivers themselves.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: Andy Lutomirski <luto@kernel.org>
The null check on client->adapter->name is redundant as name is an
array of I2C_NAME_SIZE chars and hence can never be null. We may as
well remove this redundant check.
Detected by CoverityScan, CID#1375918 ("Array compared against 0")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
We should unlock and re-enable IRQs if this allocation fails.
Fixes: 259307074b ("ipmi: Add SMBus interface driver (SSIF) ")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Use setup_timer() instead of init_timer() to simplify the code.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
msg_written_handler() may set ssif_info->multi_data to NULL
when using ipmitool to write fru.
Before setting ssif_info->multi_data to NULL, add new local
pointer "data_to_send" and store correct i2c data pointer to
it to fix NULL pointer kernel panic and incorrect ssif_info->multi_pos.
Signed-off-by: Joeseph Chang <joechang@codeaurora.org>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: stable@vger.kernel.org # 3.19-
When added by ACPI, the information does not contain the slave address
of the BMC. However, that information is available from SMBIOS. So
if we add a device that doesn't have a slave address, look at the other
devices that are duplicate interfaces and see if they have a slave
address.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
.alert() is meant to be generic, but there is currently no way
for the device driver to know which protocol generated the alert.
Add a parameter in .alert() to help the device driver to understand
what is given in data.
This patch is required to have the support of SMBus Host Notify protocol
through .alert().
Tested-by: Andrew Duggan <aduggan@synaptics.com>
For hwmon:
Acked-by: Guenter Roeck <linux@roeck-us.net>
For IPMI:
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Unlike everywhere else in the IPMI specification, the I2C address
specified in the SPMI table is not shifted to the left one bit with
the LSB zero. Instead it is not shifted with the MSB zero.
Reported-by: Sanjeev <singhsan@codeaurora.org>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
