mbox series

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

Message ID 1516600256-8152-1-git-send-email-jagan@amarulasolutions.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series [U-Boot] Pull request: u-boot-spi/master | expand

Pull-request

git://git.denx.de/u-boot-spi.git master

Message

Jagan Teki Jan. 22, 2018, 5:50 a.m. UTC
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)

----------------------------------------------------------------
Miquel Raynal (1):
      doc: bindings: soft-spi: update documentation to match the code

Álvaro Fernández Rojas (16):
      wait_bit: add 8/16/32 BE/LE versions of wait_for_bit
      wait_bit: use wait_for_bit_le32 and remove wait_for_bit
      drivers: spi: allow limiting reads
      drivers: spi: consider command bytes when sending transfers
      dm: spi: add BCM63xx SPI driver
      mips: bmips: add bcm63xx-spi driver support for BCM6338
      mips: bmips: add bcm63xx-spi driver support for BCM6348
      mips: bmips: add bcm63xx-spi driver support for BCM6358
      mips: bmips: add bcm63xx-spi driver support for BCM3380
      mips: bmips: add bcm63xx-spi driver support for BCM63268
      mips: bmips: enable the SPI flash on the Sagem F@ST1704
      mips: bmips: enable the SPI flash on the Netgear CG3100D
      dm: spi: add BCM63xx HSSPI driver
      mips: bmips: add bcm63xx-hsspi driver support for BCM6328
      mips: bmips: add bcm63xx-hsspi driver support for BCM63268
      mips: bmips: enable the SPI flash on the Comtrend AR-5387un

 arch/arm/mach-imx/mx6/ddr.c                   |  22 +-
 arch/arm/mach-socfpga/clock_manager.c         |   4 +-
 arch/arm/mach-socfpga/clock_manager_gen5.c    |   6 +-
 arch/arm/mach-socfpga/reset_manager_arria10.c |  36 +--
 arch/mips/dts/brcm,bcm3380.dtsi               |  17 +
 arch/mips/dts/brcm,bcm63268.dtsi              |  38 +++
 arch/mips/dts/brcm,bcm6328.dtsi               |  24 ++
 arch/mips/dts/brcm,bcm6338.dtsi               |  17 +
 arch/mips/dts/brcm,bcm6348.dtsi               |  17 +
 arch/mips/dts/brcm,bcm6358.dtsi               |  17 +
 arch/mips/dts/comtrend,ar-5387un.dts          |  12 +
 arch/mips/dts/netgear,cg3100d.dts             |  12 +
 arch/mips/dts/sagem,f@st1704.dts              |  12 +
 arch/mips/mach-ath79/ar934x/clk.c             |   2 +-
 board/samtec/vining_2000/vining_2000.c        |   4 +-
 configs/comtrend_ar5387un_ram_defconfig       |   8 +
 configs/netgear_cg3100d_ram_defconfig         |   8 +
 configs/sagem_f@st1704_ram_defconfig          |   8 +
 doc/device-tree-bindings/spi/soft-spi.txt     |  24 +-
 drivers/clk/clk_pic32.c                       |  12 +-
 drivers/clk/renesas/clk-rcar-gen3.c           |   4 +-
 drivers/ddr/microchip/ddr2.c                  |   8 +-
 drivers/fpga/socfpga_arria10.c                |  17 +-
 drivers/mmc/msm_sdhci.c                       |   8 +-
 drivers/mtd/pic32_flash.c                     |   4 +-
 drivers/mtd/spi/spi_flash.c                   |   5 +-
 drivers/net/ag7xxx.c                          |  16 +-
 drivers/net/dwc_eth_qos.c                     |  17 +-
 drivers/net/ethoc.c                           |   8 +-
 drivers/net/pic32_eth.c                       |  12 +-
 drivers/net/pic32_mdio.c                      |  28 +-
 drivers/net/ravb.c                            |   4 +-
 drivers/net/xilinx_axi_emac.c                 |   4 +-
 drivers/net/zynq_gem.c                        |  12 +-
 drivers/reset/sti-reset.c                     |   4 +-
 drivers/serial/serial_pic32.c                 |   4 +-
 drivers/spi/Kconfig                           |  16 +
 drivers/spi/Makefile                          |   2 +
 drivers/spi/atmel_spi.c                       |   4 +-
 drivers/spi/bcm63xx_hsspi.c                   | 414 ++++++++++++++++++++++++
 drivers/spi/bcm63xx_spi.c                     | 433 ++++++++++++++++++++++++++
 drivers/spi/cadence_qspi_apb.c                |  14 +-
 drivers/spi/fsl_qspi.c                        |  20 +-
 drivers/spi/mvebu_a3700_spi.c                 |  20 +-
 drivers/usb/host/dwc2.c                       |  24 +-
 drivers/usb/host/ehci-msm.c                   |   3 +-
 drivers/usb/host/ehci-mx6.c                   |   5 +-
 drivers/usb/host/ohci-lpc32xx.c               |  12 +-
 drivers/usb/host/xhci-rcar.c                  |  12 +-
 drivers/video/atmel_hlcdfb.c                  |  64 ++--
 include/spi.h                                 |   5 +-
 include/wait_bit.h                            |  81 ++---
 52 files changed, 1326 insertions(+), 258 deletions(-)
 create mode 100644 drivers/spi/bcm63xx_hsspi.c
 create mode 100644 drivers/spi/bcm63xx_spi.c

Comments

Tom Rini Jan. 22, 2018, 12:58 p.m. UTC | #1
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@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@gmail.com>
    Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
    Reviewed-by: Jagan Teki <jagan@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]
Daniel Schwierzeck Jan. 22, 2018, 2:56 p.m. UTC | #2
Hi Tom,

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@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@gmail.com>
>     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>     Reviewed-by: Jagan Teki <jagan@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]
> 


did this commit alone produce those warnings? The patch series itself
builds successfully on Travis CI [1].

[1] https://travis-ci.org/danielschwierzeck/u-boot/builds/331506036
Tom Rini Jan. 22, 2018, 2:59 p.m. UTC | #3
On Mon, Jan 22, 2018 at 03:56:09PM +0100, Daniel Schwierzeck wrote:
> Hi Tom,
> 
> 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@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@gmail.com>
> >     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> >     Reviewed-by: Jagan Teki <jagan@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]
> > 
> 
> 
> did this commit alone produce those warnings? The patch series itself
> builds successfully on Travis CI [1].
> 
> [1] https://travis-ci.org/danielschwierzeck/u-boot/builds/331506036

It builds, yes.  But it adds that warning too:
https://travis-ci.org/danielschwierzeck/u-boot/jobs/331506059

And I bisect'd down to the above commit being what adds that warning.

And yes, sigh, I need to something-something to get us back to zero
warnings and make -Werror at least a CONFIG option and perhaps default
in travis as this isn't the first warning to come in that wasn't noticed
as travis didn't fail.
Daniel Schwierzeck Jan. 22, 2018, 3:28 p.m. UTC | #4
On 22.01.2018 15:59, Tom Rini wrote:
> On Mon, Jan 22, 2018 at 03:56:09PM +0100, Daniel Schwierzeck wrote:
>> Hi Tom,
>>
>> 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@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@gmail.com>
>>>     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>     Reviewed-by: Jagan Teki <jagan@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]
>>>
>>
>>
>> did this commit alone produce those warnings? The patch series itself
>> builds successfully on Travis CI [1].
>>
>> [1] https://travis-ci.org/danielschwierzeck/u-boot/builds/331506036
> 
> It builds, yes.  But it adds that warning too:
> https://travis-ci.org/danielschwierzeck/u-boot/jobs/331506059
> 
> And I bisect'd down to the above commit being what adds that warning.
> 
> And yes, sigh, I need to something-something to get us back to zero
> warnings and make -Werror at least a CONFIG option and perhaps default
> in travis as this isn't the first warning to come in that wasn't noticed
> as travis didn't fail.
> 

hm, since when are gcc warnings being ignored? I thought only DTC
warnings were suppressed. Thus I still expected to have Travis CI builds
marked as yellow in case of gcc warnings ;)
Tom Rini Jan. 22, 2018, 3:32 p.m. UTC | #5
On Mon, Jan 22, 2018 at 04:28:16PM +0100, Daniel Schwierzeck wrote:
> 
> 
> On 22.01.2018 15:59, Tom Rini wrote:
> > On Mon, Jan 22, 2018 at 03:56:09PM +0100, Daniel Schwierzeck wrote:
> >> Hi Tom,
> >>
> >> 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@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@gmail.com>
> >>>     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> >>>     Reviewed-by: Jagan Teki <jagan@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]
> >>>
> >>
> >>
> >> did this commit alone produce those warnings? The patch series itself
> >> builds successfully on Travis CI [1].
> >>
> >> [1] https://travis-ci.org/danielschwierzeck/u-boot/builds/331506036
> > 
> > It builds, yes.  But it adds that warning too:
> > https://travis-ci.org/danielschwierzeck/u-boot/jobs/331506059
> > 
> > And I bisect'd down to the above commit being what adds that warning.
> > 
> > And yes, sigh, I need to something-something to get us back to zero
> > warnings and make -Werror at least a CONFIG option and perhaps default
> > in travis as this isn't the first warning to come in that wasn't noticed
> > as travis didn't fail.
> 
> hm, since when are gcc warnings being ignored? I thought only DTC
> warnings were suppressed. Thus I still expected to have Travis CI builds
> marked as yellow in case of gcc warnings ;)

... wait, you can have Travis CI do yellow for warnings?  That'd be
handy to get back again.  Especially if we can have it for non-DTC
warnings only.
Daniel Schwierzeck Jan. 22, 2018, 3:47 p.m. UTC | #6
On 22.01.2018 16:32, Tom Rini wrote:
> On Mon, Jan 22, 2018 at 04:28:16PM +0100, Daniel Schwierzeck wrote:
>>
>>
>> On 22.01.2018 15:59, Tom Rini wrote:
>>> On Mon, Jan 22, 2018 at 03:56:09PM +0100, Daniel Schwierzeck wrote:
>>>> Hi Tom,
>>>>
>>>> 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@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@gmail.com>
>>>>>     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>>     Reviewed-by: Jagan Teki <jagan@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]
>>>>>
>>>>
>>>>
>>>> did this commit alone produce those warnings? The patch series itself
>>>> builds successfully on Travis CI [1].
>>>>
>>>> [1] https://travis-ci.org/danielschwierzeck/u-boot/builds/331506036
>>>
>>> It builds, yes.  But it adds that warning too:
>>> https://travis-ci.org/danielschwierzeck/u-boot/jobs/331506059
>>>
>>> And I bisect'd down to the above commit being what adds that warning.
>>>
>>> And yes, sigh, I need to something-something to get us back to zero
>>> warnings and make -Werror at least a CONFIG option and perhaps default
>>> in travis as this isn't the first warning to come in that wasn't noticed
>>> as travis didn't fail.
>>
>> hm, since when are gcc warnings being ignored? I thought only DTC
>> warnings were suppressed. Thus I still expected to have Travis CI builds
>> marked as yellow in case of gcc warnings ;)
> 
> ... wait, you can have Travis CI do yellow for warnings?  That'd be
> handy to get back again.  Especially if we can have it for non-DTC
> warnings only.
> 

hm, I think I saw this in the past but can't find any reference in the
documentation. Sorry for creating false hopes ;)
Daniel Schwierzeck Jan. 22, 2018, 4:49 p.m. UTC | #7
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@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@gmail.com>
>     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>     Reviewed-by: Jagan Teki <jagan@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
Tom Rini Jan. 22, 2018, 5:14 p.m. UTC | #8
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@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@gmail.com>
> >     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> >     Reviewed-by: Jagan Teki <jagan@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!
Daniel Schwierzeck Jan. 22, 2018, 8:26 p.m. UTC | #9
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@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@gmail.com>
>>>     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>     Reviewed-by: Jagan Teki <jagan@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.
Álvaro Fernández Rojas Jan. 22, 2018, 8:55 p.m. UTC | #10
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@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@gmail.com>
>>>>      Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>      Reviewed-by: Jagan Teki <jagan@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?
Tom Rini Jan. 22, 2018, 9:05 p.m. UTC | #11
On Mon, Jan 22, 2018 at 09:55:43PM +0100, Á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@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@gmail.com>
> >>>>     Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> >>>>     Reviewed-by: Jagan Teki <jagan@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?

That is a bit cleaner, thanks!
Daniel Schwierzeck Jan. 23, 2018, 10:28 a.m. UTC | #12
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@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@gmail.com>
>>>>>      Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>>      Reviewed-by: Jagan Teki <jagan@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.
Jagan Teki Jan. 24, 2018, 6:57 a.m. UTC | #13
On Tue, Jan 23, 2018 at 3:58 PM, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> 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 <noltari@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@gmail.com>
>>>>>>      Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>>>      Reviewed-by: Jagan Teki <jagan@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.

Recent patch from Alvaro, seems fine to built all [1]

[1] https://travis-ci.org/openedev/u-boot-spi/builds/332390626
Álvaro Fernández Rojas Jan. 25, 2018, 4 p.m. UTC | #14
Hi Jagan,

El 24/01/2018 a las 7:57, Jagan Teki escribió:
> On Tue, Jan 23, 2018 at 3:58 PM, Daniel Schwierzeck
> <daniel.schwierzeck@gmail.com> 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 <noltari@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@gmail.com>
>>>>>>>       Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>>>>>>       Reviewed-by: Jagan Teki <jagan@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.
> 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 :).