[U-Boot] Pull request: u-boot-spi/master

[Daniel Schwierzeck]

On 22.01.2018 21:55, Álvaro Fernández Rojas wrote:
> Hi Daniel,
> 
> 
> El 22/01/2018 a las 21:26, Daniel Schwierzeck escribió:
>>
>> On 22.01.2018 18:14, Tom Rini wrote:
>>> On Mon, Jan 22, 2018 at 05:49:39PM +0100, Daniel Schwierzeck wrote:
>>>>
>>>> On 22.01.2018 13:58, Tom Rini wrote:
>>>>> On Mon, Jan 22, 2018 at 11:20:56AM +0530, Jagan Teki wrote:
>>>>>
>>>>>> Hi Tom,
>>>>>>
>>>>>> Please pull this PR.
>>>>>>
>>>>>> thanks!
>>>>>> Jagan.
>>>>>>
>>>>>> The following changes since commit
>>>>>> 98691a60abffb44303d7dae6e9e699d0daded930:
>>>>>>
>>>>>>    Merge git://git.denx.de/u-boot-rockchip (2018-01-09 13:28:51
>>>>>> -0500)
>>>>>>
>>>>>> are available in the git repository at:
>>>>>>
>>>>>>    git://git.denx.de/u-boot-spi.git master
>>>>>>
>>>>>> for you to fetch changes up to
>>>>>> b23c685c6f295da3c01dd487f0e003b70299af91:
>>>>>>
>>>>>>    mips: bmips: enable the SPI flash on the Comtrend AR-5387un
>>>>>> (2018-01-22 10:39:13 +0530)
>>>>>>
>>>>> NAK:
>>>>>
>>>>> commit 19e3a4856c1cba751a9ecb3931ff0d96a7f169be
>>>>> Author: Álvaro Fernández Rojas <noltari at gmail.com>
>>>>> Date:   Sat Jan 20 02:11:34 2018 +0100
>>>>>
>>>>>      wait_bit: add 8/16/32 BE/LE versions of wait_for_bit
>>>>>
>>>>>      Add 8/16/32 bits and BE/LE versions of wait_for_bit.
>>>>>      This is needed for reading registers that are not aligned to
>>>>> 32 bits, and for
>>>>>      Big Endian platforms.
>>>>>
>>>>>      Signed-off-by: Álvaro Fernández Rojas <noltari at gmail.com>
>>>>>      Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>>>>      Reviewed-by: Jagan Teki <jagan at openedev.com>
>>>>>
>>>>> Adds warnings on almost all platforms:
>>>>> w+(ls1088ardb_qspi_SECURE_BOOT) ../include/wait_bit.h: In function
>>>>> ?wait_for_bit_be16?:
>>>>> w+(ls1088ardb_qspi_SECURE_BOOT) ../include/wait_bit.h:76:31:
>>>>> warning: implicit declaration of function ?readw_be?
>>>>> [-Wimplicit-function-declaration]
>>>>> w+(ls1088ardb_qspi_SECURE_BOOT) ../include/wait_bit.h: In function
>>>>> ?wait_for_bit_be32?:         w+(ls1088ardb_qspi_SECURE_BOOT)
>>>>> ../include/wait_bit.h:78:31: warning: implicit declaration of
>>>>> function ?readl_be? [-Wimplicit-function-declaration]
>>>>>
>>>>>
>>>> Tom, would this change to the patch be acceptable?
>>>>
>>>> --- a/include/wait_bit.h
>>>> +++ b/include/wait_bit.h
>>>> @@ -73,8 +73,12 @@ static inline int wait_for_bit_##sfx(const void
>>>> *reg,            \
>>>>
>>>>   BUILD_WAIT_FOR_BIT(8, u8, readb)
>>>>   BUILD_WAIT_FOR_BIT(le16, u16, readw)
>>>> +#ifdef readw_be
>>>>   BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
>>>> +#endif
>>>>   BUILD_WAIT_FOR_BIT(le32, u32, readl)
>>>> +#ifdef readl_be
>>>>   BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
>>>> +#endif
>>>>
>>>>   #endif
>>>>
>>>> This wouldn't define wait_bit_be*() on archs which doesn't implement
>>>> readw_be or readl_be.
>>>>
>>>> A build with the updated patch is scheduled at
>>>> https://travis-ci.org/danielschwierzeck/u-boot/builds/331899381
>>> That seems reasonable, thanks!
>>>
>> Álvaro, could you send a v10 where the patch "wait_bit: add 8/16/32
>> BE/LE versions of wait_for_bit" is fixed like above? Thanks.
> Sure, but I think this alternative would be much cleaner:
> https://gist.github.com/Noltari/3e6ed4648b87484c73ca22e2f533f9b0
> 
> What do you think?
> 

I had a similar idea at first but a #ifdef within a #define is not possible and AFAIK not implemented in C standards. 

Have you tried it? With nested #ifdef I'm getting:

$ ./tools/buildman/buildman -o /tmp/bm -b master..wait_for_bit aarch64 
Building 1 commit for 111 boards (8 threads, 1 job per thread)
   50   11   50 /111    lion-rk3368
 
vs.

$ ./tools/buildman/buildman -o /tmp/bm -b master..wait_for_bit_2 aarch64 
Building 1 commit for 111 boards (8 threads, 1 job per thread)
   98   13    0 /111    0:00:06  : ls1046ardb_emm

Also a Travis CI build already shows several broken builds:
https://travis-ci.org/danielschwierzeck/u-boot/builds/332205310

Maybe a yet better solution would be if each arch implements the same common set of I/O primitives like Linux is doing.

-- 
- Daniel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180123/beee2404/attachment.sig>