Re: [Librtas-devel] [PATCH 5/5] librtas: Fix conversion of 64bit values for LE

[Thomas F. <tlf...@li...>]

On 08/21/2014 07:30 AM, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <su...@in...>
>
> When we pass _hi and _lo 32bits of a 64bit value, we
> should convert the host endian value carefully.
>
> i.e,
> 	X_hi = htobe32(BITS32_HI(X))
> 	X_lo = htobe32(BITS32_LO(X))
>
> and not :
> 	X_hi = BITS32_HI(htobe64(X))

Maybe I am mistaken, but you take the high and low 32 bits of a 64-bit 
LE integer, then convert each to 32-bit BE integer.  Each half would be 
the correct endianness, but would not be the correct orientation?  
Wouldn't the hi bits then actually be the lo bits?

Ex.

LE: AB BC CD DE

BE: DE CD BC AB

In the current implementation, hi is DECD.  In this implementation, BCAB.

tom
> NOTE: This patch is untested, but I believe this patch
> is good to have than the current faulty conversion.
>
> This patch fixes all such occurrences in librtas.
>
> Signed-off-by: Suzuki K. Poulose <su...@in...>
> ---
>   librtas_src/syscall_calls.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/librtas_src/syscall_calls.c b/librtas_src/syscall_calls.c
> index 29ab613..dc52370 100644
> --- a/librtas_src/syscall_calls.c
> +++ b/librtas_src/syscall_calls.c
> @@ -532,8 +532,8 @@ int sc_get_config_addr_info2(int token, uint32_t config_addr,
>
>   	do {
>   		rc = sc_rtas_call(token, 4, 2, htobe32(config_addr),
> -				  BITS32_HI(htobe64(phb_id)),
> -				  BITS32_LO(htobe64(phb_id)),
> +				  htobe32(BITS32_HI(phb_id)),
> +				  htobe32(BITS32_LO(phb_id)),
>   				  htobe32(func), &status, &be_info);
>   		if (rc)
>   			break;
> @@ -1005,8 +1005,8 @@ sc_read_slot_reset(int token, uint32_t cfg_addr, uint64_t phbid, int *state,
>
>   	do {
>   		rc = sc_rtas_call(token, 3, 3, htobe32(cfg_addr),
> -				  BITS32_HI(htobe64(phbid)),
> -				  BITS32_LO(htobe64(phbid)), &status,
> +				  htobe32(BITS32_HI(phbid)),
> +				  htobe32(BITS32_LO(phbid)), &status,
>   				  state, eeh);
>   		if (rc)
>   			return rc;
> @@ -1123,8 +1123,8 @@ sc_set_eeh_option(int token, uint32_t cfg_addr, uint64_t phbid, int function)
>
>   	do {
>   		rc = sc_rtas_call(token, 4, 1, htobe32(cfg_addr),
> -				  BITS32_HI(htobe64(phbid)),
> -				  BITS32_LO(htobe64(phbid)),
> +				  htobe32(BITS32_HI(phbid)),
> +				  htobe32(BITS32_LO(phbid)),
>   				  htobe32(function), &status);
>   		if (rc)
>   			return rc;
> @@ -1321,8 +1321,8 @@ int sc_suspend_me(int token, uint64_t streamid)
>   	int rc;
>
>   	do {
> -		rc = sc_rtas_call(token, 2, 1, BITS32_HI(htobe64(streamid)),
> -				  BITS32_LO(htobe64(streamid)), &status);
> +		rc = sc_rtas_call(token, 2, 1, htobe32(BITS32_HI(streamid)),
> +				  htobe32(BITS32_LO(streamid)), &status);
>   		if (rc)
>   			return rc;
>

--