drivers: SPI: sunxi SPL: fix warning
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
Somehow an int returning function without a return statement sneaked in. Fix it.
--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mtd/spi/sunxi_spi_spl.c +2 -1
@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT)); } -static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +if (IS_ENABLED (CONFIG_MACH_SUN50I )) pin_function =SUN50I_GPC_SPI0 ;
Signature · 1 line
2.8.2
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
Hi,
Quoted 4 lines · Andre Przywara
Somehow an int returning function without a return statement sneaked in. Fix it.
Signed-off-by Andre Przywara <[email protected]>
LGTM:
Regards,
Hans
Quoted 21 lines
--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/spi/sunxi_spi_spl.c +2
-1@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT)); } -static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +if (IS_ENABLED (CONFIG_MACH_SUN50I )) pin_function =SUN50I_GPC_SPI0 ;
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
In reply to Andre PrzywaraQuoted 13 lines · Andre Przywara
Somehow an int returning function without a return statement sneaked in. Fix it.
Signed-off-by Andre Przywara <[email protected]>--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/spi/sunxi_spi_spl.c +0
-0
drivers/mtd/spi/sunxi_spi_spl.c static void spi0_disable_clock(void) · line 164
Quoted 8 lines
drivers/mtd/spi/sunxi_spi_spl.c +2
-1@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT)); } -static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +
Space not needed or unrelated, please remove this.
thanks!
Signature · 4 lines
Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India.
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
Hi,
Quoted 17 lines · Jagan Teki
Quoted 16 lines · Andre Przywara
Somehow an int returning function without a return statement sneaked in. Fix it.
Signed-off-by Andre Przywara <[email protected]>--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/spi/sunxi_spi_spl.c +0
-0@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT)); }
Quoted 7 lines
drivers/mtd/spi/sunxi_spi_spl.c static void spi0_disable_clock(void) · line 164
Quoted 5 lines
drivers/mtd/spi/sunxi_spi_spl.c +2
-1-static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +Space not needed or unrelated, please remove this.
This is Linux coding style, which U-Boot adheres to. "WARNING: Missing a blank line after declarations"
I thought I should fix this since this is was in the context of this very simple patch and it improves readability.
If this is too much, then please remove the line before committing.
Thanks!
Andre.
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
Quoted 28 lines · Andre Przywara
Hi,
Quoted 17 lines · Jagan Teki
Quoted 16 lines · Andre Przywara
Somehow an int returning function without a return statement sneaked in. Fix it.
Signed-off-by Andre Przywara <[email protected]>--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/spi/sunxi_spi_spl.c +0
-0@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT)); }Quoted 7 lines
drivers/mtd/spi/sunxi_spi_spl.c static void spi0_disable_clock(void) · line 164
Quoted 5 lines
drivers/mtd/spi/sunxi_spi_spl.c +2
-1-static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +Space not needed or unrelated, please remove this.
Quoted 6 lines
This is Linux coding style, which U-Boot adheres to. "WARNING: Missing a blank line after declarations"
I thought I should fix this since this is was in the context of this very simple patch and it improves readability.
If this is too much, then please remove the line before committing.
Making things checkpatch clean is good, in the future please also mention that in commit messages. Thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161114/57c0e803/attachment.sig>
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
Quoted 32 lines · On Mon, 14 Nov 2016 11:56:50 -0500 Tom Rini <[email protected]> wrote:
Quoted 31 lines · Andre Przywara
Hi,
Quoted 17 lines · Jagan Teki
Quoted 16 lines · Andre Przywara
Somehow an int returning function without a return statement sneaked in. Fix it.
Signed-off-by Andre Przywara <[email protected]>--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/spi/sunxi_spi_spl.c +0
-0@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT)); }Quoted 7 lines
drivers/mtd/spi/sunxi_spi_spl.c static void spi0_disable_clock(void) · line 164
Quoted 5 lines
drivers/mtd/spi/sunxi_spi_spl.c +2
-1-static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +Space not needed or unrelated, please remove this.
This is Linux coding style, which U-Boot adheres to. "WARNING: Missing a blank line after declarations"
Quoted 6 lines
Quoted 3 lines
I thought I should fix this since this is was in the context of this very simple patch and it improves readability.
If this is too much, then please remove the line before committing.Making things checkpatch clean is good, in the future please also mention that in commit messages. Thanks!
How can I get this checkpatch warning?
$ scripts/checkpatch.pl 0001-sunxi-Support-booting-from-SPI-flash.patch total: 0 errors, 0 warnings, 0 checks, 361 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE
0001-sunxi-Support-booting-from-SPI-flash.patch has no obvious style problems and is ready for submission.
Signature · 2 lines
Best regards, Siarhei Siamashka
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
In reply to Andre PrzywaraQuoted 14 lines · On Thu, 3 Nov 2016 00:58:12 +0000 Andre Przywara <[email protected]>…
Somehow an int returning function without a return statement sneaked in. Fix it.
Signed-off-by Andre Przywara <[email protected]>--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/spi/sunxi_spi_spl.c +0
-0@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT));
drivers/mtd/spi/sunxi_spi_spl.c static void spi0_disable_clock(void) · line 167
Quoted 10 lines
drivers/mtd/spi/sunxi_spi_spl.c +2
-1} -static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +if (IS_ENABLED (CONFIG_MACH_SUN50I )) pin_function =SUN50I_GPC_SPI0 ;
Thanks for spotting and fixing this compilation warning.
This was a last minute change. Originally there was also a check for the pins state and the function returned an error code in the case if the pins are already configured for NAND. But I removed this code before sending the patch.
The idea is that probing the SPI flash may be useful in the future even if booting from some other media. We may store some board-specific configuration in the on-board SPI flash (for example, the DRAM and CPU speed grade information). But this functionality definitely belongs to a separate patch and can be always contributed later. There is also the SPL size concern and we don't want to unnecessarily increase the code size.
Signature · 2 lines
Best regards, Siarhei Siamashka
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
Quoted 19 lines · Siarhei Siamashka
Quoted 16 lines · On Thu, 3 Nov 2016 00:58:12 +0000 Andre Przywara <[email protected]>…
Somehow an int returning function without a return statement sneaked in. Fix it.
Signed-off-by Andre Przywara <[email protected]>--- drivers/mtd/spi/sunxi_spi_spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mtd/spi/sunxi_spi_spl.c +0
-0@@ -158,9 +158,10 @@ static void spi0_disable_clock(void) (1 << AHB_RESET_SPI0_SHIFT)); }
Quoted 9 lines
drivers/mtd/spi/sunxi_spi_spl.c static void spi0_disable_clock(void) · line 167
Quoted 8 lines
drivers/mtd/spi/sunxi_spi_spl.c +2
-1-static int spi0_init (void ) +static void spi0_init (void ) {unsigned int pin_function =SUNXI_GPC_SPI0 ; +if (IS_ENABLED (CONFIG_MACH_SUN50I )) pin_function =SUN50I_GPC_SPI0 ;
Hi Siarhei,
Quoted 6 lines
Thanks for spotting and fixing this compilation warning.
This was a last minute change. Originally there was also a check for the pins state and the function returned an error code in the case if the pins are already configured for NAND. But I removed this code before sending the patch.
please, no need to apologize or explain, those things happen, especially when shuffling around patches.
The only learning that we should take from it that at least those compilation warnings should be spotted on a more automated base. I think the problem here is that the feature is not enabled in any defconfig atm, but I think I will enable it in my Opi PC 2 series.
On a related matter, I ran buildman on HEAD for armv8 today and GCC 6.2 found quite some issues (will send out the fixes ASAP).
So is there some buildbot somewhere that runs buildman? If yes, with what compilers?
Quoted 7 lines
The idea is that probing the SPI flash may be useful in the future even if booting from some other media. We may store some board-specific configuration in the on-board SPI flash (for example, the DRAM and CPU speed grade information). But this functionality definitely belongs to a separate patch and can be always contributed later. There is also the SPL size concern and we don't want to unnecessarily increase the code size.
Totally agree, and as I said: No worries. Actually thanks a lot for this series, as booting from NOR flash is really a cool feature.
Cheers,
Andre.
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
On Mon, Nov 14, 2016 at 07:43:40PM +0000, Andr? Przywara wrote:
[snip]
Quoted 4 lines
On a related matter, I ran buildman on HEAD for armv8 today and GCC 6.2 found quite some issues (will send out the fixes ASAP).
So is there some buildbot somewhere that runs buildman? If yes, with what compilers?
What gcc-6.2 compilers are you using? The travis-ci setup is using the Ubuntu 'Trusty' compilers, which are not that recent. I also have been running debian/unstable in a chroot, but it's old enough of a copy to be using gcc-5.3.1 currently. I'm going to kick off a new debootstrap now'ish to see what toolchain versions I can get there. Do you know if these gcc-6.2 problems you see would also show up for rpi3 in 64bit mode? That's my current easiest test platform for aarch64. Thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161114/d9cf91c9/attachment.sig>
PATCH 1/1 drivers: SPI: sunxi SPL: fix warning
Quoted 9 lines · Tom Rini
On Mon, Nov 14, 2016 at 07:43:40PM +0000, Andr? Przywara wrote:
[snip]
Quoted 4 lines
On a related matter, I ran buildman on HEAD for armv8 today and GCC 6.2 found quite some issues (will send out the fixes ASAP).
So is there some buildbot somewhere that runs buildman? If yes, with what compilers?What gcc-6.2 compilers are you using?
I just built my own, in fact for Trusty. I have some easy build scripts here [1], though the README needs some update. With this I create .deb packages for binutils and gcc-stage1 of (almost) any vanilla GCC version you throw at it.
I don't know of any newer GCC in some proper repo, historically not many people cared and either built their own or used a Linaro drop.
Quoted 7 lines
The travis-ci setup is using the
Ubuntu 'Trusty' compilers, which are not that recent. I also have been running debian/unstable in a chroot, but it's old enough of a copy to be using gcc-5.3.1 currently. I'm going to kick off a new debootstrap now'ish to see what toolchain versions I can get there. Do you know if these gcc-6.2 problems you see would also show up for rpi3 in 64bit mode?
I didn't observe any runtime issues (though at least someone else did in one occasion).
The compiler warnings are mostly due to the new GCC6 option -Wmisleading-indentation, which seems to be enabled by default. Some of them are really just wrong indentation, so cosmetic, but others are genuine bugs. I will send patches later today ($work here atm).
Quoted 1 line
That's my current easiest test platform for aarch64. Thanks!
As said this was observed only by running buildman, so you shouldn't need any actual hardware to see this.
Cheers,
Andre.