From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?=c3=81lvaro_Fern=c3=a1ndez_Rojas?= Date: Thu, 25 Jan 2018 17:00:28 +0100 Subject: [U-Boot] Pull request: u-boot-spi/master In-Reply-To: References: <1516600256-8152-1-git-send-email-jagan@amarulasolutions.com> <20180122125844.GA32220@bill-the-cat> <7fd360b9-0494-9927-d456-91c4d3c0cb1a@gmail.com> <20180122171400.GN32220@bill-the-cat> Message-ID: <7cf92c05-af10-b41e-cd09-539969dd6dad@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi Jagan, El 24/01/2018 a las 7:57, Jagan Teki escribió: > On Tue, Jan 23, 2018 at 3:58 PM, Daniel Schwierzeck > wrote: >> >> 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 >>>>>>> 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 >>>>>>> Reviewed-by: Daniel Schwierzeck >>>>>>> Reviewed-by: Jagan Teki >>>>>>> >>>>>>> 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. > Recent patch from Alvaro, seems fine to built all [1] > > [1] https://travis-ci.org/openedev/u-boot-spi/builds/332390626 I ended up adding the changes proposed by Daniel to avoid nested ifdefs. So I guess it can be merged now :).