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 <j...@redhat.com>
> ---
> .../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?
totally confused,
greg k-h
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
