NTB: ntb_perf: fix cast to restricted __le32

PATCH Superseded Serge Semin 3 participants 10 messages Jan 19, 2018 – Jan 25, 2018 NTB driver core

lore

Share & tools

lore

Serge Semin

author

2018-01-19 17:30 UTC #0 lore raw

PATCH 1/1 NTB: ntb_perf: fix cast to restricted __le32

Sparse is whining about the u32 and __le32 mixed usage in the driver.

drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32 drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 (different base types)
drivers/ntb/test/ntb_perf.c:295:37: expected unsigned int [unsigned] [usertype] val
drivers/ntb/test/ntb_perf.c:295:37: got restricted __le32 [usertype] <noident>
...

The NTB API can't be changed so ntb_spad_*() methods would return either pure __le32 or __be32, since the scratchpad data can have arbitrary endianness in general. In this case we need to forcibly cast all the u32 to be __le32 and vise-versa where it's supposed to be in accordance with the driver logic.

Fixes b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")

Signed-off-by Serge Semin <[email protected]>

---
 drivers/ntb/test/ntb_perf.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

drivers/ntb/test/ntb_perf.c +16 ~~-16~~

index eaa252b71f82..5590aaf8e8a5 100644
@@ -283,21 +283,21 @@ static int perf_spad_cmd_send(struct perf_peer *peer, enum perf_cmd cmd,
 
 		sts = ntb_peer_spad_read(perf->ntb, peer->pidx,
 					 PERF_SPAD_CMD(perf->gidx));
-		if (le32_to_cpu(sts) != PERF_CMD_INVAL) {
+		if (le32_to_cpu((__force __le32)sts) != PERF_CMD_INVAL) {
 			usleep_range(MSG_UDELAY_LOW, MSG_UDELAY_HIGH);
 			continue;
 		}
 
 		ntb_peer_spad_write(perf->ntb, peer->pidx,
-				    PERF_SPAD_LDATA(perf->gidx),
-				    cpu_to_le32(lower_32_bits(data)));
+				PERF_SPAD_LDATA(perf->gidx),
+				(__force u32)cpu_to_le32(lower_32_bits(data)));
 		ntb_peer_spad_write(perf->ntb, peer->pidx,
-				    PERF_SPAD_HDATA(perf->gidx),
-				    cpu_to_le32(upper_32_bits(data)));
+				PERF_SPAD_HDATA(perf->gidx),
+				(__force u32)cpu_to_le32(upper_32_bits(data)));
 		mmiowb();
 		ntb_peer_spad_write(perf->ntb, peer->pidx,
 				    PERF_SPAD_CMD(perf->gidx),
-				    cpu_to_le32(cmd));
+				    (__force u32)cpu_to_le32(cmd));
 		mmiowb();
 		ntb_peer_db_set(perf->ntb, PERF_SPAD_NOTIFY(peer->gidx));

@@ -331,21 +331,21 @@ static int perf_spad_cmd_recv(struct perf_ctx *perf, int *pidx,
 			continue;
 
 		val = ntb_spad_read(perf->ntb, PERF_SPAD_CMD(peer->gidx));
-		val = le32_to_cpu(val);
+		val = le32_to_cpu((__force __le32)val);
 		if (val == PERF_CMD_INVAL)
 			continue;
 
 		*cmd = val;
 
 		val = ntb_spad_read(perf->ntb, PERF_SPAD_LDATA(peer->gidx));
-		*data = le32_to_cpu(val);
+		*data = le32_to_cpu((__force __le32)val);
 
 		val = ntb_spad_read(perf->ntb, PERF_SPAD_HDATA(peer->gidx));
-		*data |= (u64)le32_to_cpu(val) << 32;
+		*data |= (u64)le32_to_cpu((__force __le32)val) << 32;
 
 		/* Next command can be retrieved from now */
 		ntb_spad_write(perf->ntb, PERF_SPAD_CMD(peer->gidx),
-			       cpu_to_le32(PERF_CMD_INVAL));
+			       (__force u32)cpu_to_le32(PERF_CMD_INVAL));
 
 		dev_dbg(&perf->ntb->dev, "CMD recv: %d 0x%llx\n", *cmd, *data);

@@ -381,7 +381,7 @@ static int perf_msg_cmd_send(struct perf_peer *peer, enum perf_cmd cmd,
 			return ret;
 
 		ntb_peer_msg_write(perf->ntb, peer->pidx, PERF_MSG_LDATA,
-			      cpu_to_le32(lower_32_bits(data)));
+				(__force u32)cpu_to_le32(lower_32_bits(data)));
 
 		if (ntb_msg_read_sts(perf->ntb) & outbits) {
 			usleep_range(MSG_UDELAY_LOW, MSG_UDELAY_HIGH);

@@ -389,12 +389,12 @@ static int perf_msg_cmd_send(struct perf_peer *peer, enum perf_cmd cmd,
 		}
 
 		ntb_peer_msg_write(perf->ntb, peer->pidx, PERF_MSG_HDATA,
-			      cpu_to_le32(upper_32_bits(data)));
+				(__force u32)cpu_to_le32(upper_32_bits(data)));
 		mmiowb();
 
 		/* This call shall trigger peer message event */
 		ntb_peer_msg_write(perf->ntb, peer->pidx, PERF_MSG_CMD,
-			      cpu_to_le32(cmd));
+				   (__force u32)cpu_to_le32(cmd));
 
 		break;
 	}

@@ -414,13 +414,13 @@ static int perf_msg_cmd_recv(struct perf_ctx *perf, int *pidx,
 		return -ENODATA;
 
 	val = ntb_msg_read(perf->ntb, pidx, PERF_MSG_CMD);
-	*cmd = le32_to_cpu(val);
+	*cmd = le32_to_cpu((__force __le32)val);
 
 	val = ntb_msg_read(perf->ntb, pidx, PERF_MSG_LDATA);
-	*data = le32_to_cpu(val);
+	*data = le32_to_cpu((__force __le32)val);
 
 	val = ntb_msg_read(perf->ntb, pidx, PERF_MSG_HDATA);
-	*data |= (u64)le32_to_cpu(val) << 32;
+	*data |= (u64)le32_to_cpu((__force __le32)val) << 32;
 
 	/* Next command can be retrieved from now */
 	ntb_msg_clear_sts(perf->ntb, inbits);

Signature · 1 line

2.12.0

Arnd Bergmann

2018-01-19 20:42 UTC #1 lore raw

Quoted 9 lines · Serge Semin

Sparse is whining about the u32 and __le32 mixed usage in the driver.
drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32 drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 (different base types)
drivers/ntb/test/ntb_perf.c:295:37: expected unsigned int [unsigned] [usertype] val
drivers/ntb/test/ntb_perf.c:295:37: got restricted __le32 [usertype] <noident>
...

Quoted 6 lines

The NTB API can't be changed so ntb_spad_*() methods would return either pure __le32 or __be32, since the scratchpad data can have arbitrary endianness in general. In this case we need to forcibly cast all the u32 to be __le32 and vise-versa where it's supposed to be in accordance with the driver logic.

There's got to be a better way to do this than sprinkling lots of __force typecasts throughout the code.

It looks like all those casts are about ntb_peer_spad_read()/ntb_peer_msg_write() calls, so why not change those function prototypes to work on __le32 types?

There should also be some form of documentation regarding why you need to swap the data twice, since all the ntb drivers later end up doing another cpu_to_le32() on the little-endian data.

Arnd

Serge Semin

author

2018-01-19 21:03 UTC #2 lore raw

Quoted 24 lines · Arnd Bergmann

Quoted 9 lines · Serge Semin
Sparse is whining about the u32 and __le32 mixed usage in the driver.
drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32 drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 (different base types)
drivers/ntb/test/ntb_perf.c:295:37: expected unsigned int [unsigned] [usertype] val
drivers/ntb/test/ntb_perf.c:295:37: got restricted __le32 [usertype] <noident>
...
Quoted 6 lines
The NTB API can't be changed so ntb_spad_*() methods would return either pure __le32 or __be32, since the scratchpad data can have arbitrary endianness in general. In this case we need to forcibly cast all the u32 to be __le32 and vise-versa where it's supposed to be in accordance with the driver logic.
There's got to be a better way to do this than sprinkling lots of __force typecasts throughout the code.
It looks like all those casts are about ntb_peer_spad_read()/ntb_peer_msg_write() calls, so why not change those function prototypes to work on __le32 types?

Quoted 5 lines

There should also be some form of documentation regarding why you need to swap the data twice, since all the ntb drivers later end up doing another cpu_to_le32() on the little-endian data.
Arnd

Actually the provided patch is the best solution I could come up with. The thing is, that the methods can't be changed. Those functions are the part of the NTB API methods used by many drivers. So basically they are like pci_{read,write}_config_{byte,word,dword}() methods. We can't change their prototypes only because it's suit some driver. The methods give an access to the NTB device dummy u32-sized registers, nothing else. So endianness is the transmitted data settings in this case.

NTB is the technology to interconnect some two systems with possibly different endianness (unlike PCI, which interconnect CPU with LE devices). In this case I'd need to set some agreement up between two systems about the endianness of the exchanged data like host and network types in Linux networking. I've chosen the network data to be little-endian, that's why I needed first to convert them from CPU to le32, then on remote side convert them back from le32 to CPU.

If you have any better suggestion how the warning can be fixed, I'd be glad to stick to it.

-Sergey

Serge Semin

author

2018-01-19 21:25 UTC #3 lore raw

Quoted 47 lines · Serge Semin

Quoted 24 lines · Arnd Bergmann
Quoted 9 lines · Serge Semin
Sparse is whining about the u32 and __le32 mixed usage in the driver.
drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32 drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 (different base types)
drivers/ntb/test/ntb_perf.c:295:37: expected unsigned int [unsigned] [usertype] val
drivers/ntb/test/ntb_perf.c:295:37: got restricted __le32 [usertype] <noident>
...
Quoted 6 lines
The NTB API can't be changed so ntb_spad_*() methods would return either pure __le32 or __be32, since the scratchpad data can have arbitrary endianness in general. In this case we need to forcibly cast all the u32 to be __le32 and vise-versa where it's supposed to be in accordance with the driver logic.
There's got to be a better way to do this than sprinkling lots of __force typecasts throughout the code.
It looks like all those casts are about ntb_peer_spad_read()/ntb_peer_msg_write() calls, so why not change those function prototypes to work on __le32 types?
Quoted 5 lines
There should also be some form of documentation regarding why you need to swap the data twice, since all the ntb drivers later end up doing another cpu_to_le32() on the little-endian data.
Arnd
Actually the provided patch is the best solution I could come up with. The thing is, that the methods can't be changed. Those functions are the part of the NTB API methods used by many drivers. So basically they are like pci_{read,write}_config_{byte,word,dword}() methods. We can't change their prototypes only because it's suit some driver. The methods give an access to the NTB device dummy u32-sized registers, nothing else. So endianness is the transmitted data settings in this case.
NTB is the technology to interconnect some two systems with possibly different endianness (unlike PCI, which interconnect CPU with LE devices). In this case I'd need to set some agreement up between two systems about the endianness of the exchanged data like host and network types in Linux networking. I've chosen the network data to be little-endian, that's why I needed first to convert them from CPU to le32, then on remote side convert them back from le32 to CPU.

Quoted 5 lines

If you have any better suggestion how the warning can be fixed, I'd be glad to stick to it.
-Sergey

I meant, everything depends on the NTB hardware driver hidden behind the API. If it does back and forth conversions writing/reading data to/from scratchpad registers (using IO methods like write32/read32), then I don't need to worry about data endianness at all and should have discarded le32_to_cpu()/cpu_to_l32() usage. But if it doesn't do it, then ntb_perf driver will be in trouble. So I sticked with the safest solution. Although the final decision is after the subsystem maintainer.

-Sergey

Arnd Bergmann

2018-01-19 21:26 UTC #4 lore raw

In reply to Serge Semin

Quoted 17 lines · Serge Semin

Actually the provided patch is the best solution I could come up with. The thing is, that the methods can't be changed. Those functions are the part of the NTB API methods used by many drivers. So basically they are like pci_{read,write}_config_{byte,word,dword}() methods. We can't change their prototypes only because it's suit some driver. The methods give an access to the NTB device dummy u32-sized registers, nothing else. So endianness is the transmitted data settings in this case.
NTB is the technology to interconnect some two systems with possibly different endianness (unlike PCI, which interconnect CPU with LE devices). In this case I'd need to set some agreement up between two systems about the endianness of the exchanged data like host and network types in Linux networking. I've chosen the network data to be little-endian, that's why I needed first to convert them from CPU to le32, then on remote side convert them back from le32 to CPU.

Quoted 2 lines

If you have any better suggestion how the warning can be fixed, I'd be glad to stick to it.

I don't think your description matches what you actually do: The underlying ntb hardware drivers (amd, idt, intel, mscc) all treat the incoming data as CPU-endian and convert it to little-endian on the register side, so the framework already assumes that whatever you do here uses a little-endian wire-level protocol.

On a little-endian kernel/CPU, nothing is ever swapped here, neither in the ntb_perf front-end nor in the back-ends. On a big-endian kernel/CPU, they both swap, so you end up with CPU-endian data on the wire, so it should be impossible for a big-endian system to talk to a little-endian one. Have you actually tried that combination with the current code?

If my interpretation is correct, then the best solution would be to completely remove the cpu_to_le32/le32_to_cpu conversions from ntb_perf, and just define that it works like any other PCI device, exchanging little-endian data.

There are two interesting cases to consider though:

- if someone wants to implement an NTB based protocol using big-endian data on the wire, you probably want to add a ntb_peer_spad_read_be()/ntb_peer_msg_write_be()
set of interfaces, to go along with ioread32_be()/iowrite32_be() the same way that ntb_peer_spad_read()/ntb_peer_msg_write() ends up doing ioread32()/iowrite32() with the implied little-endian behavior.

- memcpy_toio()/memcpy_fromio() and ioread32_rep()/iowrite32_rep importantly do not do any byteswap, they are meant to transfer byte streams.

Arnd

Jon Mason

maintainer

2018-01-24 04:18 UTC #5 lore raw

Quoted 27 lines · Arnd Bergmann

Quoted 17 lines · Serge Semin
Actually the provided patch is the best solution I could come up with. The thing is, that the methods can't be changed. Those functions are the part of the NTB API methods used by many drivers. So basically they are like pci_{read,write}_config_{byte,word,dword}() methods. We can't change their prototypes only because it's suit some driver. The methods give an access to the NTB device dummy u32-sized registers, nothing else. So endianness is the transmitted data settings in this case.
NTB is the technology to interconnect some two systems with possibly different endianness (unlike PCI, which interconnect CPU with LE devices). In this case I'd need to set some agreement up between two systems about the endianness of the exchanged data like host and network types in Linux networking. I've chosen the network data to be little-endian, that's why I needed first to convert them from CPU to le32, then on remote side convert them back from le32 to CPU.
Quoted 2 lines
If you have any better suggestion how the warning can be fixed, I'd be glad to stick to it.
I don't think your description matches what you actually do: The underlying ntb hardware drivers (amd, idt, intel, mscc) all treat the incoming data as CPU-endian and convert it to little-endian on the register side, so the framework already assumes that whatever you do here uses a little-endian wire-level protocol.

Quoted 6 lines

On a little-endian kernel/CPU, nothing is ever swapped here, neither in the ntb_perf front-end nor in the back-ends. On a big-endian kernel/CPU, they both swap, so you end up with CPU-endian data on the wire, so it should be impossible for a big-endian system to talk to a little-endian one. Have you actually tried that combination with the current code?

I do not believe anyone has every tried NTB on a big endien system, let alone tried it with one side LE and the other BE. To my knowledge, this has only ever been used on x86 to x86.

Quoted 4 lines

If my interpretation is correct, then the best solution would be to completely remove the cpu_to_le32/le32_to_cpu conversions from ntb_perf, and just define that it works like any other PCI device, exchanging little-endian data.

Yes, this would be the best solution. Thank you for the insight.

Serge, when you get a chance, please make this change and resumbit.

Thanks,
Jon

Quoted 16 lines

There are two interesting cases to consider though:
- if someone wants to implement an NTB based protocol using big-endian data on the wire, you probably want to add a ntb_peer_spad_read_be()/ntb_peer_msg_write_be()
set of interfaces, to go along with ioread32_be()/iowrite32_be() the same way that ntb_peer_spad_read()/ntb_peer_msg_write() ends up doing ioread32()/iowrite32() with the implied little-endian behavior.
- memcpy_toio()/memcpy_fromio() and ioread32_rep()/iowrite32_rep importantly do not do any byteswap, they are meant to transfer byte streams.
Arnd

Serge Semin

author

2018-01-24 07:31 UTC #6 lore raw

Quoted 39 lines · Jon Mason

Quoted 27 lines · Arnd Bergmann
Quoted 17 lines · Serge Semin
Actually the provided patch is the best solution I could come up with. The thing is, that the methods can't be changed. Those functions are the part of the NTB API methods used by many drivers. So basically they are like pci_{read,write}_config_{byte,word,dword}() methods. We can't change their prototypes only because it's suit some driver. The methods give an access to the NTB device dummy u32-sized registers, nothing else. So endianness is the transmitted data settings in this case.
NTB is the technology to interconnect some two systems with possibly different endianness (unlike PCI, which interconnect CPU with LE devices). In this case I'd need to set some agreement up between two systems about the endianness of the exchanged data like host and network types in Linux networking. I've chosen the network data to be little-endian, that's why I needed first to convert them from CPU to le32, then on remote side convert them back from le32 to CPU.
Quoted 2 lines
If you have any better suggestion how the warning can be fixed, I'd be glad to stick to it.
I don't think your description matches what you actually do: The underlying ntb hardware drivers (amd, idt, intel, mscc) all treat the incoming data as CPU-endian and convert it to little-endian on the register side, so the framework already assumes that whatever you do here uses a little-endian wire-level protocol.
Quoted 6 lines
On a little-endian kernel/CPU, nothing is ever swapped here, neither in the ntb_perf front-end nor in the back-ends. On a big-endian kernel/CPU, they both swap, so you end up with CPU-endian data on the wire, so it should be impossible for a big-endian system to talk to a little-endian one. Have you actually tried that combination with the current code?
I do not believe anyone has every tried NTB on a big endien system, let alone tried it with one side LE and the other BE. To my knowledge, this has only ever been used on x86 to x86.

Quoted 9 lines

Quoted 4 lines
If my interpretation is correct, then the best solution would be to completely remove the cpu_to_le32/le32_to_cpu conversions from ntb_perf, and just define that it works like any other PCI device, exchanging little-endian data.
Yes, this would be the best solution. Thank you for the insight.
Serge, when you get a chance, please make this change and resumbit.

Ok. I'll do it in an hour.

Regards,
-Sergey

Quoted 20 lines

Thanks,
Jon
Quoted 16 lines
There are two interesting cases to consider though:
- if someone wants to implement an NTB based protocol using big-endian data on the wire, you probably want to add a ntb_peer_spad_read_be()/ntb_peer_msg_write_be()
set of interfaces, to go along with ioread32_be()/iowrite32_be() the same way that ntb_peer_spad_read()/ntb_peer_msg_write() ends up doing ioread32()/iowrite32() with the implied little-endian behavior.
- memcpy_toio()/memcpy_fromio() and ioread32_rep()/iowrite32_rep importantly do not do any byteswap, they are meant to transfer byte streams.
Arnd

Serge Semin

author

2018-01-24 07:49 UTC #7 lore raw

PATCH v2 1/1 NTB: ntb_perf: fix cast to restricted __le32

In reply to Serge Semin

Sparse is whining about the u32 and __le32 mixed usage in the driver

NTB hardware drivers shall accept CPU-endian data and translate it to the portable formate by internal means, so the explicit conversions are not necessary before Scratchpad/Messages API usage anymore.

Fixes b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")

Signed-off-by Serge Semin <[email protected]>

---
 drivers/ntb/test/ntb_perf.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

drivers/ntb/test/ntb_perf.c +13 ~~-15~~

index 1829a17dd461..3fded6aeda08 100644
@@ -273,21 +273,21 @@ static int perf_spad_cmd_send(struct perf_peer *peer, enum perf_cmd cmd,
 
 		sts = ntb_peer_spad_read(perf->ntb, peer->pidx,
 					 PERF_SPAD_CMD(perf->gidx));
-		if (le32_to_cpu(sts) != PERF_CMD_INVAL) {
+		if (sts != PERF_CMD_INVAL) {
 			usleep_range(MSG_UDELAY_LOW, MSG_UDELAY_HIGH);
 			continue;
 		}
 
 		ntb_peer_spad_write(perf->ntb, peer->pidx,
 				    PERF_SPAD_LDATA(perf->gidx),
-				    cpu_to_le32(lower_32_bits(data)));
+				    lower_32_bits(data));
 		ntb_peer_spad_write(perf->ntb, peer->pidx,
 				    PERF_SPAD_HDATA(perf->gidx),
-				    cpu_to_le32(upper_32_bits(data)));
+				    upper_32_bits(data));
 		mmiowb();
 		ntb_peer_spad_write(perf->ntb, peer->pidx,
 				    PERF_SPAD_CMD(perf->gidx),
-				    cpu_to_le32(cmd));
+				    cmd);
 		mmiowb();
 		ntb_peer_db_set(perf->ntb, PERF_SPAD_NOTIFY(peer->gidx));

@@ -321,21 +321,20 @@ static int perf_spad_cmd_recv(struct perf_ctx *perf, int *pidx,
 			continue;
 
 		val = ntb_spad_read(perf->ntb, PERF_SPAD_CMD(peer->gidx));
-		val = le32_to_cpu(val);
 		if (val == PERF_CMD_INVAL)
 			continue;
 
 		*cmd = val;
 
 		val = ntb_spad_read(perf->ntb, PERF_SPAD_LDATA(peer->gidx));
-		*data = le32_to_cpu(val);
+		*data = val;
 
 		val = ntb_spad_read(perf->ntb, PERF_SPAD_HDATA(peer->gidx));
-		*data |= (u64)le32_to_cpu(val) << 32;
+		*data |= (u64)val << 32;
 
 		/* Next command can be retrieved from now */
 		ntb_spad_write(perf->ntb, PERF_SPAD_CMD(peer->gidx),
-			       cpu_to_le32(PERF_CMD_INVAL));
+			       PERF_CMD_INVAL);
 
 		dev_dbg(&perf->ntb->dev, "CMD recv: %d 0x%llx\n", *cmd, *data);

@@ -371,7 +370,7 @@ static int perf_msg_cmd_send(struct perf_peer *peer, enum perf_cmd cmd,
 			return ret;
 
 		ntb_peer_msg_write(perf->ntb, peer->pidx, PERF_MSG_LDATA,
-			      cpu_to_le32(lower_32_bits(data)));
+				   lower_32_bits(data));
 
 		if (ntb_msg_read_sts(perf->ntb) & outbits) {
 			usleep_range(MSG_UDELAY_LOW, MSG_UDELAY_HIGH);

@@ -379,12 +378,11 @@ static int perf_msg_cmd_send(struct perf_peer *peer, enum perf_cmd cmd,
 		}
 
 		ntb_peer_msg_write(perf->ntb, peer->pidx, PERF_MSG_HDATA,
-			      cpu_to_le32(upper_32_bits(data)));
+				   upper_32_bits(data));
 		mmiowb();
 
 		/* This call shall trigger peer message event */
-		ntb_peer_msg_write(perf->ntb, peer->pidx, PERF_MSG_CMD,
-			      cpu_to_le32(cmd));
+		ntb_peer_msg_write(perf->ntb, peer->pidx, PERF_MSG_CMD, cmd);
 
 		break;
 	}

@@ -404,13 +402,13 @@ static int perf_msg_cmd_recv(struct perf_ctx *perf, int *pidx,
 		return -ENODATA;
 
 	val = ntb_msg_read(perf->ntb, pidx, PERF_MSG_CMD);
-	*cmd = le32_to_cpu(val);
+	*cmd = val;
 
 	val = ntb_msg_read(perf->ntb, pidx, PERF_MSG_LDATA);
-	*data = le32_to_cpu(val);
+	*data = val;
 
 	val = ntb_msg_read(perf->ntb, pidx, PERF_MSG_HDATA);
-	*data |= (u64)le32_to_cpu(val) << 32;
+	*data |= (u64)val << 32;
 
 	/* Next command can be retrieved from now */
 	ntb_msg_clear_sts(perf->ntb, inbits);

Signature · 1 line

2.12.0

Arnd Bergmann

Acked-by on v2 1/1

2018-01-24 08:07 UTC #8 lore raw

Quoted 8 lines · Serge Semin

Sparse is whining about the u32 and __le32 mixed usage in the driver
drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32 drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 (different base types)
drivers/ntb/test/ntb_perf.c:295:37: expected unsigned int [unsigned] [usertype] val
drivers/ntb/test/ntb_perf.c:295:37: got restricted __le32 [usertype] <noident>
...

Quoted 6 lines

NTB hardware drivers shall accept CPU-endian data and translate it to the portable formate by internal means, so the explicit conversions are not necessary before Scratchpad/Messages API usage anymore.
Fixes b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")
Signed-off-by Serge Semin <[email protected]>

Looks good to me,

Acked-by Arnd Bergmann <[email protected]>

Jon Mason

maintainer Applied

2018-01-25 04:13 UTC #9 lore raw

Quoted 13 lines · Arnd Bergmann

Quoted 12 lines · Serge Semin
Sparse is whining about the u32 and __le32 mixed usage in the driver
drivers/ntb/test/ntb_perf.c:288:21: warning: cast to restricted __le32 drivers/ntb/test/ntb_perf.c:295:37: warning: incorrect type in argument 4 (different base types)
drivers/ntb/test/ntb_perf.c:295:37: expected unsigned int [unsigned] [usertype] val
drivers/ntb/test/ntb_perf.c:295:37: got restricted __le32 [usertype] <noident>
...
NTB hardware drivers shall accept CPU-endian data and translate it to the portable formate by internal means, so the explicit conversions are not necessary before Scratchpad/Messages API usage anymore.

Quoted 6 lines

Quoted 2 lines
Fixes b83003b3fdc1 ("NTB: ntb_perf: Add full multi-port NTB API support")
Signed-off-by Serge Semin <[email protected]>
Looks good to me,
Acked-by Arnd Bergmann <[email protected]>

Applied to my ntb-next branch. Thanks for all of the help on this.

Thanks,
Jon