On 06/17/2014 06:01 PM, Greg KH wrote:
On Thu, Jun 05, 2014 at 01:56:16PM -0500, Ken Cox wrote:
Added I/O version for the function ultra_vbus_init_channel() to get rid
of noderef sparse warnings when accessing I/O space.
Signed-off-by: Ken Cox <jkc@xxxxxxxxxx>
---
.../common-spar/include/channels/vbuschannel.h | 40 ++++++++++++++++++++--
drivers/staging/unisys/uislib/uislib.c | 2 +-
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/unisys/common-spar/include/channels/vbuschannel.h b/drivers/staging/unisys/common-spar/include/channels/vbuschannel.h
index 000182c..af5a1ff 100644
--- a/drivers/staging/unisys/common-spar/include/channels/vbuschannel.h
+++ b/drivers/staging/unisys/common-spar/include/channels/vbuschannel.h
@@ -95,12 +95,46 @@ typedef struct _ULTRA_VBUS_CHANNEL_PROTOCOL {
#define VBUS_CH_SIZE(MAXDEVICES) COVER(VBUS_CH_SIZE_EXACT(MAXDEVICES), 4096)
static inline void
-ultra_vbus_init_channel(ULTRA_VBUS_CHANNEL_PROTOCOL __iomem *x,
- int bytesAllocated)
+ultra_vbus_init_channel(ULTRA_VBUS_CHANNEL_PROTOCOL *x,
+ int bytes)
{
/* Please note that the memory at <x> does NOT necessarily have space
* for DevInfo structs allocated at the end, which is why we do NOT use
- * <bytesAllocated> to clear. */
+ * <bytes> to clear. */
+ memset(x, 0, sizeof(ULTRA_VBUS_CHANNEL_PROTOCOL));
+ if (bytes < (int) sizeof(ULTRA_VBUS_CHANNEL_PROTOCOL))
+ return;
+
+ x->ChannelHeader.VersionId = ULTRA_VBUS_CHANNEL_PROTOCOL_VERSIONID;
+ x->ChannelHeader.Signature = ULTRA_VBUS_CHANNEL_PROTOCOL_SIGNATURE;
+ x->ChannelHeader.SrvState = CHANNELSRV_READY;
+ x->ChannelHeader.HeaderSize = sizeof(x->ChannelHeader);
+ x->ChannelHeader.Size = bytes;
+ memcpy(&x->ChannelHeader.Type, &UltraVbusChannelProtocolGuid,
+ sizeof(x->ChannelHeader.Type));
+ memcpy(&x->ChannelHeader.ZoneGuid, &NULL_UUID_LE, sizeof(uuid_le));
+ x->HdrInfo.structBytes = sizeof(ULTRA_VBUS_HEADERINFO);
+ x->HdrInfo.chpInfoByteOffset = sizeof(ULTRA_VBUS_HEADERINFO);
+ x->HdrInfo.busInfoByteOffset += sizeof(ULTRA_VBUS_DEVICEINFO);
+ x->HdrInfo.devInfoByteOffset += sizeof(ULTRA_VBUS_DEVICEINFO);
+ x->HdrInfo.deviceInfoStructBytes = sizeof(ULTRA_VBUS_DEVICEINFO);
+ bytes -= (sizeof(ULTRA_CHANNEL_PROTOCOL) +
+ x->HdrInfo.devInfoByteOffset);
+ x->HdrInfo.devInfoCount = bytes / x->HdrInfo.deviceInfoStructBytes;
+}
That's a _huge_ inline function, why is it inline? Shouldn't it just be
a "real" function instead?
And given that you are duplicating most of the logic in
ULTRA_VBUS_init_channel(), isn't there some other way to share the logic
here? Hm, no, I see why you did this, that's crazy. The same logic to
write to a structure or a iomemory-based structure? Something feels
really wrong here with what you are doing. It's as if you init the
channel, then write it all out to memory, right?
No, wait, no one calls this function ever now. Why is it even needed?
Originally, this function was here in preparation for an additional
driver that has not been submitted. After thinking about your comments
and reviewing the code, I can make some changes to the calling functions
and remove both versions of the function completely.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
