[U-Boot] [PATCH] wait_bit: add big endian version of wait_for_bit function

[Álvaro Fernández Rojas]

Hello Tom,


El 01/01/2018 a las 14:41, Tom Rini escribió:
> On Sat, Dec 30, 2017 at 11:12:36AM +0100, Álvaro Fernández Rojas wrote:
>
>> The only difference with the existing wait_for_bit function is the fact that
>> wait_for_bit_be expects the register size to be read.
>>
>> Signed-off-by: Álvaro Fernández Rojas <noltari at gmail.com>
>> ---
>>   include/wait_bit.h | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 69 insertions(+)
>>
>> diff --git a/include/wait_bit.h b/include/wait_bit.h
>> index 06ad43a122..828fdcda02 100644
>> --- a/include/wait_bit.h
>> +++ b/include/wait_bit.h
>> @@ -69,5 +69,74 @@ static inline int wait_for_bit(const char *prefix, const u32 *reg,
>>   	return -ETIMEDOUT;
>>   }
>>   
>> +/**
>> + * wait_for_bit_be()	waits for bit set/cleared in BE register
>> + *
>> + * Function polls register waiting for specific bit(s) change
>> + * (either 0->1 or 1->0). It can fail under two conditions:
>> + * - Timeout
>> + * - User interaction (CTRL-C)
>> + * Function succeeds only if all bits of masked register are set/cleared
>> + * (depending on set option).
>> + *
>> + * @param prefix	Prefix added to timeout messagge (message visible only
>> + *			with debug enabled)
>> + * @param reg		Register that will be read (using read_be())
>> + * @param size		Size of the register that will be read
>> + * @param mask		Bit(s) of register that must be active
>> + * @param set		Selects wait condition (bit set or clear)
>> + * @param timeout_ms	Timeout (in miliseconds)
>> + * @param breakable	Enables CTRL-C interruption
>> + * @return		0 on success, -EINVAL, -ETIMEDOUT or -EINTR on failure
>> + */
>> +static inline int wait_for_bit_be(const char *prefix,
>> +				  const u32 *reg, const u8 size,
>> +				  const u32 mask, const bool set,
>> +				  const unsigned int timeout_ms,
>> +				  const bool breakable)
>> +{
>> +	u32 val;
>> +	unsigned long start = get_timer(0);
>> +
>> +	while (1) {
>> +		switch(size)
>> +		{
>> +			case 8:
>> +				val = readb_be(reg);
>> +				break;
>> +			case 16:
>> +				val = readw_be(reg);
>> +				break;
>> +			case 32:
>> +				val = readl_be(reg);
>> +				break;
>> +			default:
>> +				debug("%s: invalid size %u\n", prefix, size);
>> +				return -EINVAL;
>> +		}
>> +
>> +		if (!set)
>> +			val = ~val;
>> +
>> +		if ((val & mask) == mask)
>> +			return 0;
>> +
>> +		if (get_timer(start) > timeout_ms)
>> +			break;
>> +
>> +		if (breakable && ctrlc()) {
>> +			puts("Abort\n");
>> +			return -EINTR;
>> +		}
>> +
>> +		udelay(1);
>> +		WATCHDOG_RESET();
>> +	}
>> +
>> +	debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n", prefix, reg, mask,
>> +	      set);
>> +
>> +	return -ETIMEDOUT;
>> +}
> Where is this used?  Thanks!
I was planning on using this for both brcm63xx SPI drivers, as stated here:
https://lists.denx.de/pipermail/u-boot/2017-December/315263.html
https://lists.denx.de/pipermail/u-boot/2017-December/315264.html

Maybe it would be better to define a macro and use it for both LE/BE, 
with functions like:
wait_for_bit_8
wait_for_bit_le16/wait_for_bit_be16
wait_for_bit_le32/wait_for_bit_be32

What do you think?

Regards,
Álvaro.