next steps

U-Boot Marek Vasut 12 participants 43 messages Oct 7, 2014 – Oct 20, 2014

Share & tools

Marek Vasut

author

2014-10-07 12:45 UTC #0 lore raw

Hey,

given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.

First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .

As you probably noticed, there is a lot of topic branches in the u-boot-socfpga
[1] repository, each of them with a date tag. This decision came to be so that rebasing of a branch can be avoided. I will most likely garbage-collect the old and useless topic branches at some point, when they become irrelevant due to the code being merged into u-boot-arm/master (and then to u-boot/master).

I think that's about it for the organization part. Now for the remaining part, we are still missing a few things. Some of them are ready, some of them, not so much:

SPL: This is something we still miss from mainline. We will need to discuss
this thoroughly, but one thing is obvious already -- we need to figure out how to interoperate with Quartus resp. the bsp-editor generated header files to assemble the SPL properly.
USB: This is scheduled for the next merge window. The DWC2 driver is cleaned
up and seems to be in rather good state. The USB driver currently resides in [2]
EPCQ: This is something I prepared and tested real quick. The EPCS/EPCQ SPI NOR
can be operated via the Altera SPI driver, which is currently unused at all and thus suffering bitrot. The current cleanup is here [3]
NET: Does the SoCDK use EMAC0 or EMAC1 ? I believe we still have this issue
open, I recall Chin was complaining about this.

You can find the latest combined version of all the patches at [4] to ease up your testing.

Please feel free to add into this list, it would be really good to keep track of what's still open and what's missing.

Thank you!

[1] http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=summary
[2] http://git.denx.de/?p=u-boot/u-boot-
usb.git;a=shortlog;h=refs/heads/topic/dwc2-20140930
[3] http://git.denx.de/?p=u-boot/u-boot-
socfpga.git;a=shortlog;h=refs/heads/topic/drivers/spi-20141006
[4] http://git.denx.de/?p=u-boot/u-boot-
socfpga.git;a=shortlog;h=refs/heads/topic/arm/socfpga-next-20141007

Michal Simek

2014-10-08 08:58 UTC #1 lore raw

Hi,

Quoted 6 lines · Marek Vasut

Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.

Quoted 6 lines

First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .

Platform was in this shape for a while that's why I can't see the reason why this happen.

Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.

Thanks,
Michal

Signature · 14 lines

Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


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

Marek Vasut

author

2014-10-08 10:39 UTC #2 lore raw

Quoted 17 lines · Michal Simek

Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 7 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .

Quoted 6 lines

Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.

I believe this was an exception here. Also, the socfpga patches were posted on the list for a while (beginning of september) and they were tested on multiple devices by multiple parties, so there is clearly interest to get the platform in shape.

The initial patch was posted all the way back as "[RFC] mainline u-boot on socfpga" on Sept. 3rd 2014, which is in the middle of the release cycle and which still gave us enough time to clean the patches up still.

Best regards,
Marek Vasut

Pavel Machek

2014-10-08 11:17 UTC #3 lore raw

In reply to Michal Simek

Quoted 16 lines · Michal Simek

Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 6 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .

Quoted 4 lines

Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.

Well, the platform was in such a wrong shape that it would be hard to cause regressions, and code was self-contained enough that it would not break anything.

I'm not a Tom, but it makes sense to me.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Tom Rini

2014-10-08 20:09 UTC #4 lore raw

In reply to Michal Simek

Quoted 16 lines · Michal Simek

Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 6 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .

Quoted 6 lines

Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.

So, it's a long standing thing where for non-core changes, deferring to the relevant custodian about what's going to come in close to the release is what's done. So yes, I grilled Marek about what non-socfpga things would be impacted by the changes (RPi) and if he'd tested things there. It all had been through a few post/review cycles.

There's an argument to be made that we shouldn't have let socfpga in, back in 2012 or should have pushed harder, sooner, to get more progress made on "real" platform support.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141008/315a0930/attachment.pgp>

Michal Simek

2014-10-09 08:37 UTC #5 lore raw

Hi,

Quoted 30 lines · Tom Rini

Quoted 16 lines · Michal Simek
Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 6 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .
Quoted 6 lines
Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.
So, it's a long standing thing where for non-core changes, deferring to the relevant custodian about what's going to come in close to the release is what's done. So yes, I grilled Marek about what non-socfpga things would be impacted by the changes (RPi) and if he'd tested things there. It all had been through a few post/review cycles.

Quoted 3 lines

There's an argument to be made that we shouldn't have let socfpga in, back in 2012 or should have pushed harder, sooner, to get more progress made on "real" platform support.

AFAIK if platform is working in certain state and you are able to get for example console than there is no problem to be in. There is nowhere written what exactly should work that's why I can't see any problem that socfpga is in if it is not causing compilation issues and have at least minimum functionality.

The question was if the problem was that Altera just failed because didn't collect patches to any repo and sending it to Albert. Or there was just misunderstanding that Albert expected that Altera will do that and Altera expected that Albert will pick it up because he is ARM custodian and none was listed for socfpga. I have to defend Altera guys because if none is listed for SocFpga the nearest maintainer is collecting patches.

Then there was discussion that none did care about socfpga patches and problem was resolved by creating socfpga repository and Marek became custodian for it. Marek collected that patches to the new repo and also I believe add new one and rebased them on the top of current tree and send them out as one big 51 series which is not possible to even properly review.
IMHO they should be sent separated to target exact audience which do care about spi/i2c/watchdog/fpga/soc etc. But maybe that's just matter of taste.

But I am still missing the point why that patches was that urgent that they were merged to rc3 when it was claimed that socfpga was in a wrong shape for a while. It means v2014.10 should be just another broken version for socfpga and all this mess should be solved properly in 2015.01 via socfpga repo.

And because patches went into rc3 and yesterday Jeroen is reporting problem on FreeBSD because of this merge.(http://patchwork.ozlabs.org/patch/397453/)

Regarding your point that all "It all had been through a few post/review cycles." I don't think all things have been fixed.
Personally me I have reported here
http://lists.denx.de/pipermail/u-boot/2014-September/189741.html
(sha1: 0ae16cbb40a2881f6dfbe00fcb023ee7b548bc5c)
issue with checkpatch.pl which hasn't been fixed.

Here is my ACK for one patch which is not present in mainline commit
http://lists.denx.de/pipermail/u-boot/2014-September/189747.html
(sha1: 2f210639c4f003b0d5310273979441f1bfc88eae)

Make no sense to go through all patches but this is just an example.

I think it is something what we should discuss at u-boot mini summit on Monday. I discussed this with Marek over IRC yesterday and I expect he will ping me today (because of this email :-) ).

If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?

Thanks,
Michal

Signature · 14 lines

Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


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

Jagan Teki

2014-10-09 11:20 UTC #6 lore raw

Quoted 83 lines · Michal Simek

Hi,
Quoted 30 lines · Tom Rini
Quoted 16 lines · Michal Simek
Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 6 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .
Quoted 6 lines
Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.
So, it's a long standing thing where for non-core changes, deferring to the relevant custodian about what's going to come in close to the release is what's done. So yes, I grilled Marek about what non-socfpga things would be impacted by the changes (RPi) and if he'd tested things there. It all had been through a few post/review cycles.
Quoted 3 lines
There's an argument to be made that we shouldn't have let socfpga in, back in 2012 or should have pushed harder, sooner, to get more progress made on "real" platform support.
AFAIK if platform is working in certain state and you are able to get for example console than there is no problem to be in. There is nowhere written what exactly should work that's why I can't see any problem that socfpga is in if it is not causing compilation issues and have at least minimum functionality.
The question was if the problem was that Altera just failed because didn't collect patches to any repo and sending it to Albert. Or there was just misunderstanding that Albert expected that Altera will do that and Altera expected that Albert will pick it up because he is ARM custodian and none was listed for socfpga. I have to defend Altera guys because if none is listed for SocFpga the nearest maintainer is collecting patches.
Then there was discussion that none did care about socfpga patches and problem was resolved by creating socfpga repository and Marek became custodian for it. Marek collected that patches to the new repo and also I believe add new one and rebased them on the top of current tree and send them out as one big 51 series which is not possible to even properly review.
IMHO they should be sent separated to target exact audience which do care about spi/i2c/watchdog/fpga/soc etc. But maybe that's just matter of taste.
But I am still missing the point why that patches was that urgent that they were merged to rc3 when it was claimed that socfpga was in a wrong shape for a while. It means v2014.10 should be just another broken version for socfpga and all this mess should be solved properly in 2015.01 via socfpga repo.
And because patches went into rc3 and yesterday Jeroen is reporting problem on FreeBSD because of this merge.(http://patchwork.ozlabs.org/patch/397453/)
Regarding your point that all "It all had been through a few post/review cycles." I don't think all things have been fixed.
Personally me I have reported here
http://lists.denx.de/pipermail/u-boot/2014-September/189741.html
(sha1: 0ae16cbb40a2881f6dfbe00fcb023ee7b548bc5c)
issue with checkpatch.pl which hasn't been fixed.
Here is my ACK for one patch which is not present in mainline commit
http://lists.denx.de/pipermail/u-boot/2014-September/189747.html
(sha1: 2f210639c4f003b0d5310273979441f1bfc88eae)
Make no sense to go through all patches but this is just an example.

Quoted 6 lines

I think it is something what we should discuss at u-boot mini summit on Monday. I discussed this with Marek over IRC yesterday and I expect he will ping me today (because of this email :-) ).
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?

I think this traversing the actual development process in a different direction and it must be a valid point that need to discuss.

Apart from this, I'm experienced an another isuue where some of the subsystem patches (say for example spi stuff) are pushed in a different direction.
http://patchwork.ozlabs.org/patch/346015/

These are the qspi stuff from freescale, and I didn't understand why these goes into
u-boot-arm/master. And there is no intimation of mine as well.

Issue is that the driver itself is not in a proper shape, why does subsystem patches were
pushed without the the review tag from a respective custodians.

Please try to discuss this point as well "Each subsystem patch(es) should be pushed
if and only if the respective custodian should marked the review tag"

thanks!

Signature · 1 line

Jagan.

Marek Vasut

author

2014-10-09 13:42 UTC #7 lore raw

Quoted 98 lines · Jagan Teki

Quoted 86 lines · Michal Simek
Hi,
Quoted 31 lines · Tom Rini
Quoted 17 lines · Michal Simek
Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 7 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .
Quoted 6 lines
Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.
So, it's a long standing thing where for non-core changes, deferring to the relevant custodian about what's going to come in close to the release is what's done. So yes, I grilled Marek about what non-socfpga things would be impacted by the changes (RPi) and if he'd tested things there. It all had been through a few post/review cycles.
Quoted 3 lines
There's an argument to be made that we shouldn't have let socfpga in, back in 2012 or should have pushed harder, sooner, to get more progress made on "real" platform support.
AFAIK if platform is working in certain state and you are able to get for example console than there is no problem to be in. There is nowhere written what exactly should work that's why I can't see any problem that socfpga is in if it is not causing compilation issues and have at least minimum functionality.
The question was if the problem was that Altera just failed because didn't collect patches to any repo and sending it to Albert. Or there was just misunderstanding that Albert expected that Altera will do that and Altera expected that Albert will pick it up because he is ARM custodian and none was listed for socfpga. I have to defend Altera guys because if none is listed for SocFpga the nearest maintainer is collecting patches.
Then there was discussion that none did care about socfpga patches and problem was resolved by creating socfpga repository and Marek became custodian for it. Marek collected that patches to the new repo and also I believe add new one and rebased them on the top of current tree and send them out as one big 51 series which is not possible to even properly review.
IMHO they should be sent separated to target exact audience which do care about spi/i2c/watchdog/fpga/soc etc. But maybe that's just matter of taste.
But I am still missing the point why that patches was that urgent that they were merged to rc3 when it was claimed that socfpga was in a wrong shape for a while. It means v2014.10 should be just another broken version for socfpga and all this mess should be solved properly in 2015.01 via socfpga repo.
And because patches went into rc3 and yesterday Jeroen is reporting problem on FreeBSD because of this merge.(http://patchwork.ozlabs.org/patch/397453/)
Regarding your point that all "It all had been through a few post/review cycles." I don't think all things have been fixed. Personally me I have reported here
http://lists.denx.de/pipermail/u-boot/2014-September/189741.html
(sha1: 0ae16cbb40a2881f6dfbe00fcb023ee7b548bc5c)
issue with checkpatch.pl which hasn't been fixed.
Here is my ACK for one patch which is not present in mainline commit
http://lists.denx.de/pipermail/u-boot/2014-September/189747.html
(sha1: 2f210639c4f003b0d5310273979441f1bfc88eae)
Make no sense to go through all patches but this is just an example.
Quoted 7 lines
I think it is something what we should discuss at u-boot mini summit on Monday. I discussed this with Marek over IRC yesterday and I expect he will ping me today (because of this email :-) ).
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?
I think this traversing the actual development process in a different direction and it must be a valid point that need to discuss.

Quoted 7 lines

Apart from this, I'm experienced an another isuue where some of the subsystem patches (say for example spi stuff) are pushed in a different direction. http://patchwork.ozlabs.org/patch/346015/
These are the qspi stuff from freescale, and I didn't understand why these goes into
u-boot-arm/master. And there is no intimation of mine as well.

Did you comment on them at all please ? While I disagree with them bypassing your tree, I see they were rotting on the ML for a month and then Albert then picked those.

Quoted 3 lines

Issue is that the driver itself is not in a proper shape, why does subsystem patches were
pushed without the the review tag from a respective custodians.

I produced a hypothesis above.

Can you retroactively comment on them and ask the author to fix the code?

Quoted 3 lines

Please try to discuss this point as well "Each subsystem patch(es) should be pushed
if and only if the respective custodian should marked the review tag"

I agree we have an issue here, but I would suggest we move this discussion into a separate thread now. The subject of the email does not match the topic of the thread by far.

Jagan Teki

2014-10-09 16:11 UTC #8 lore raw

Quoted 103 lines · Marek Vasut

Quoted 102 lines · Jagan Teki
Quoted 86 lines · Michal Simek
Hi,
Quoted 31 lines · Tom Rini
Quoted 17 lines · Michal Simek
Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 7 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .
Quoted 6 lines
Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.
So, it's a long standing thing where for non-core changes, deferring to the relevant custodian about what's going to come in close to the release is what's done. So yes, I grilled Marek about what non-socfpga things would be impacted by the changes (RPi) and if he'd tested things there. It all had been through a few post/review cycles.
Quoted 3 lines
There's an argument to be made that we shouldn't have let socfpga in, back in 2012 or should have pushed harder, sooner, to get more progress made on "real" platform support.
AFAIK if platform is working in certain state and you are able to get for example console than there is no problem to be in. There is nowhere written what exactly should work that's why I can't see any problem that socfpga is in if it is not causing compilation issues and have at least minimum functionality.
The question was if the problem was that Altera just failed because didn't collect patches to any repo and sending it to Albert. Or there was just misunderstanding that Albert expected that Altera will do that and Altera expected that Albert will pick it up because he is ARM custodian and none was listed for socfpga. I have to defend Altera guys because if none is listed for SocFpga the nearest maintainer is collecting patches.
Then there was discussion that none did care about socfpga patches and problem was resolved by creating socfpga repository and Marek became custodian for it. Marek collected that patches to the new repo and also I believe add new one and rebased them on the top of current tree and send them out as one big 51 series which is not possible to even properly review.
IMHO they should be sent separated to target exact audience which do care about spi/i2c/watchdog/fpga/soc etc. But maybe that's just matter of taste.
But I am still missing the point why that patches was that urgent that they were merged to rc3 when it was claimed that socfpga was in a wrong shape for a while. It means v2014.10 should be just another broken version for socfpga and all this mess should be solved properly in 2015.01 via socfpga repo.
And because patches went into rc3 and yesterday Jeroen is reporting problem on FreeBSD because of this merge.(http://patchwork.ozlabs.org/patch/397453/)
Regarding your point that all "It all had been through a few post/review cycles." I don't think all things have been fixed. Personally me I have reported here
http://lists.denx.de/pipermail/u-boot/2014-September/189741.html
(sha1: 0ae16cbb40a2881f6dfbe00fcb023ee7b548bc5c)
issue with checkpatch.pl which hasn't been fixed.
Here is my ACK for one patch which is not present in mainline commit
http://lists.denx.de/pipermail/u-boot/2014-September/189747.html
(sha1: 2f210639c4f003b0d5310273979441f1bfc88eae)
Make no sense to go through all patches but this is just an example.
Quoted 7 lines
I think it is something what we should discuss at u-boot mini summit on Monday. I discussed this with Marek over IRC yesterday and I expect he will ping me today (because of this email :-) ).
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?
I think this traversing the actual development process in a different direction and it must be a valid point that need to discuss.
Apart from this, I'm experienced an another isuue where some of the subsystem patches (say for example spi stuff) are pushed in a different direction. http://patchwork.ozlabs.org/patch/346015/

Quoted 7 lines

Quoted 3 lines
These are the qspi stuff from freescale, and I didn't understand why these goes into
u-boot-arm/master. And there is no intimation of mine as well.
Did you comment on them at all please ? While I disagree with them bypassing your tree, I see they were rotting on the ML for a month and then Albert then picked those.

This is not a question of commenting - but - about the process.

Yes, I asked the author to test the changes later for a while this got pushed. I never thought this could happen so suddenly without any ping or something.

I guess some times it happens few of the patches will rotted for a while on ML due to some delays, but taking them with/out any ping causes over head if the respective owner will look at the code for later modifications.

Quoted 8 lines

Quoted 4 lines
Issue is that the driver itself is not in a proper shape, why does subsystem patches were
pushed without the the review tag from a respective custodians.
I produced a hypothesis above.
Can you retroactively comment on them and ask the author to fix the code?

Yes - I asked the author for fixing those for few of the patches against that change.

Quoted 8 lines

Quoted 4 lines
Please try to discuss this point as well "Each subsystem patch(es) should be pushed
if and only if the respective custodian should marked the review tag"
I agree we have an issue here, but I would suggest we move this discussion into a separate thread now. The subject of the email does not match the topic of the thread by far.

Agreed - I mentioned this on this tread only for listing item on meeting, that it.

Thanks for your comments.

thanks!

Signature · 1 line

Jagan.

Marek Vasut

author

2014-10-09 16:15 UTC #9 lore raw

[U-Boot] Discussion topics / issues

On Thursday, October 09, 2014 at 06:11:37 PM, Jagan Teki wrote:

[...]

Quoted 10 lines

Quoted 7 lines
Quoted 3 lines
These are the qspi stuff from freescale, and I didn't understand why these goes into
u-boot-arm/master. And there is no intimation of mine as well.
Did you comment on them at all please ? While I disagree with them bypassing your tree, I see they were rotting on the ML for a month and then Albert then picked those.
This is not a question of commenting - but - about the process.

Quoted 7 lines

Yes, I asked the author to test the changes later for a while this got pushed. I never thought this could happen so suddenly without any ping or something.
I guess some times it happens few of the patches will rotted for a while on ML due to some delays, but taking them with/out any ping causes over head if the respective owner will look at the code for later modifications.

I agree with you that there is a problem where custodians get bypassed and that such a thing happened to me as well. This is sporadic, but apparently annoying enough, so this should be added.

Quoted 10 lines

Quoted 7 lines
Quoted 3 lines
Issue is that the driver itself is not in a proper shape, why does subsystem patches were
pushed without the the review tag from a respective custodians.
I produced a hypothesis above.
Can you retroactively comment on them and ask the author to fix the code?
Yes - I asked the author for fixing those for few of the patches against that change.

Thanks!

Quoted 10 lines

Quoted 7 lines
Quoted 3 lines
Please try to discuss this point as well "Each subsystem patch(es) should be pushed
if and only if the respective custodian should marked the review tag"
I agree we have an issue here, but I would suggest we move this discussion into a separate thread now. The subject of the email does not match the topic of the thread by far.
Agreed - I mentioned this on this tread only for listing item on meeting, that it.

Will you join us as well? (sorry, I lost track of who will and who won't)

Best regards,
Marek Vasut

Jagan Teki

2014-10-09 16:41 UTC #10 lore raw

[U-Boot] Discussion topics / issues

Quoted 18 lines · Marek Vasut

On Thursday, October 09, 2014 at 06:11:37 PM, Jagan Teki wrote:
[...]
Quoted 14 lines
Quoted 7 lines
Quoted 3 lines
These are the qspi stuff from freescale, and I didn't understand why these goes into
u-boot-arm/master. And there is no intimation of mine as well.
Did you comment on them at all please ? While I disagree with them bypassing your tree, I see they were rotting on the ML for a month and then Albert then picked those.
This is not a question of commenting - but - about the process.
Yes, I asked the author to test the changes later for a while this got pushed. I never thought this could happen so suddenly without any ping or something.

Quoted 7 lines

Quoted 3 lines
I guess some times it happens few of the patches will rotted for a while on ML due to some delays, but taking them with/out any ping causes over head if the respective owner will look at the code for later modifications.
I agree with you that there is a problem where custodians get bypassed and that such a thing happened to me as well. This is sporadic, but apparently annoying enough, so this should be added.

Ok - thanks.

Quoted 18 lines

Quoted 11 lines
Quoted 8 lines
Quoted 4 lines
Issue is that the driver itself is not in a proper shape, why does subsystem patches were
pushed without the the review tag from a respective custodians.
I produced a hypothesis above.
Can you retroactively comment on them and ask the author to fix the code?
Yes - I asked the author for fixing those for few of the patches against that change.
Thanks!
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Please try to discuss this point as well "Each subsystem patch(es) should be pushed
if and only if the respective custodian should marked the review tag"

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
I agree we have an issue here, but I would suggest we move this discussion into a separate thread now. The subject of the email does not match the topic of the thread by far.
Agreed - I mentioned this on this tread only for listing item on meeting, that it.
Will you join us as well? (sorry, I lost track of who will and who won't)

I'm unable to join this, hope you guys have more fun than last year :)

thanks!

Signature · 1 line

Jagan.

Marek Vasut

author

2014-10-09 14:03 UTC #11 lore raw

[U-Boot] Discussion topics / issues

In reply to Michal Simek

Quoted 1 line · Michal Simek

Hi,

Hi!

I changed the subject, since it long didn't match the topic.

Quoted 36 lines · Tom Rini

Quoted 31 lines · Michal Simek
Quoted 17 lines
Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 7 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .
Quoted 6 lines
Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.
So, it's a long standing thing where for non-core changes, deferring to the relevant custodian about what's going to come in close to the release is what's done. So yes, I grilled Marek about what non-socfpga things would be impacted by the changes (RPi) and if he'd tested things there. It all had been through a few post/review cycles.
Quoted 3 lines
There's an argument to be made that we shouldn't have let socfpga in, back in 2012 or should have pushed harder, sooner, to get more progress made on "real" platform support.

Quoted 5 lines

AFAIK if platform is working in certain state and you are able to get for example console than there is no problem to be in. There is nowhere written what exactly should work that's why I can't see any problem that socfpga is in if it is not causing compilation issues and have at least minimum functionality.

This was not the case in here. The platform was completely unusable.

Quoted 7 lines

The question was if the problem was that Altera just failed because didn't collect patches to any repo and sending it to Albert. Or there was just misunderstanding that Albert expected that Altera will do that and Altera expected that Albert will pick it up because he is ARM custodian and none was listed for socfpga. I have to defend Altera guys because if none is listed for SocFpga the nearest maintainer is collecting patches.

It was not Altera guys who started submitting patches to put this thing in shape. Altera only realized that things started to move after Pavel sent the initial blob-of-a-patch . Since then, we are moving forward.

Quoted 6 lines

Then there was discussion that none did care about socfpga patches and problem was resolved by creating socfpga repository and Marek became custodian for it. Marek collected that patches to the new repo and also I believe add new one and rebased them on the top of current tree and send them out as one big 51 series which is not possible to even properly review.

Patches which were contained to the architecture for the most part, mind you. The rest were fixes for issues which appeared during this development, thus fixing issues in U-Boot.

Quoted 2 lines

IMHO they should be sent separated to target exact audience which do care about spi/i2c/watchdog/fpga/soc etc. But maybe that's just matter of taste.

I fully agree on this part.

Quoted 5 lines

But I am still missing the point why that patches was that urgent that they were merged to rc3 when it was claimed that socfpga was in a wrong shape for a while. It means v2014.10 should be just another broken version for socfpga and all this mess should be solved properly in 2015.01 via socfpga repo.

Yes, this would mean that another broken version would be released even though the fixes were available. This doesn't sound right to me.

Quoted 3 lines

And because patches went into rc3 and yesterday Jeroen is reporting problem on FreeBSD because of this merge.(http://patchwork.ozlabs.org/patch/397453/)

This problem was discovered in a patch which was first posted to the list on Feb. 19 ( http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/180797 ), which is before 2014.04 release and was not discovered through the review process.

Quoted 6 lines

Regarding your point that all "It all had been through a few post/review cycles." I don't think all things have been fixed. Personally me I have reported here
http://lists.denx.de/pipermail/u-boot/2014-September/189741.html
(sha1: 0ae16cbb40a2881f6dfbe00fcb023ee7b548bc5c)
issue with checkpatch.pl which hasn't been fixed.

And I explicitly noted that the FPGA stuff still has a couple of checkpatch issues right in the subsequent email [1] . Processing all the checkpatch issues of that file would make the resulting patch a horrid mess instead of producing a well contained set of patches.

[1] http://lists.denx.de/pipermail/u-boot/2014-September/189745.html

Quoted 3 lines

Here is my ACK for one patch which is not present in mainline commit
http://lists.denx.de/pipermail/u-boot/2014-September/189747.html
(sha1: 2f210639c4f003b0d5310273979441f1bfc88eae)

Apologies, this one was indeed missed.

Quoted 6 lines

Make no sense to go through all patches but this is just an example.
I think it is something what we should discuss at u-boot mini summit on Monday. I discussed this with Marek over IRC yesterday and I expect he will ping me today (because of this email :-) ).

I agree we should discuss this on the minisummit. But what is "this" topic exactly? Do we have a list of topics we need to discuss somewhere, so this one can be added there ?

Quoted 3 lines

If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?

I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...

Michal Simek

2014-10-09 14:45 UTC #12 lore raw

[U-Boot] Discussion topics / issues

Quoted 43 lines · Marek Vasut

Quoted 1 line · Michal Simek
Hi,
Hi!
I changed the subject, since it long didn't match the topic.
Quoted 36 lines · Tom Rini
Quoted 34 lines · Michal Simek
Quoted 17 lines
Hi,
Quoted 6 lines · Marek Vasut
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Quoted 7 lines
First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .
Quoted 6 lines
Platform was in this shape for a while that's why I can't see the reason why this happen.
Tom: Does it mean that every platform which is not in good shape can go directly to the mainline in any time? It is definitely something which is good to know.
So, it's a long standing thing where for non-core changes, deferring to the relevant custodian about what's going to come in close to the release is what's done. So yes, I grilled Marek about what non-socfpga things would be impacted by the changes (RPi) and if he'd tested things there. It all had been through a few post/review cycles.
There's an argument to be made that we shouldn't have let socfpga in, back in 2012 or should have pushed harder, sooner, to get more progress made on "real" platform support.

Quoted 7 lines

Quoted 5 lines
AFAIK if platform is working in certain state and you are able to get for example console than there is no problem to be in. There is nowhere written what exactly should work that's why I can't see any problem that socfpga is in if it is not causing compilation issues and have at least minimum functionality.
This was not the case in here. The platform was completely unusable.

I think that doesn't matter too much now because it was just merged in.

Also I think that a lot of platforms are in u-boot and only one person has tested it. For completely new platform this is likely the case. Simply we have to trust to submitter than this is working.

Quoted 11 lines

Quoted 7 lines
The question was if the problem was that Altera just failed because didn't collect patches to any repo and sending it to Albert. Or there was just misunderstanding that Albert expected that Altera will do that and Altera expected that Albert will pick it up because he is ARM custodian and none was listed for socfpga. I have to defend Altera guys because if none is listed for SocFpga the nearest maintainer is collecting patches.
It was not Altera guys who started submitting patches to put this thing in shape. Altera only realized that things started to move after Pavel sent the initial blob-of-a-patch . Since then, we are moving forward.

Progress is important but should be done in a way which is standard.

Quoted 7 lines

Quoted 6 lines
Then there was discussion that none did care about socfpga patches and problem was resolved by creating socfpga repository and Marek became custodian for it. Marek collected that patches to the new repo and also I believe add new one and rebased them on the top of current tree and send them out as one big 51 series which is not possible to even properly review.

Quoted 8 lines

Patches which were contained to the architecture for the most part, mind you. The rest were fixes for issues which appeared during this development, thus fixing issues in U-Boot.
Quoted 2 lines
IMHO they should be sent separated to target exact audience which do care about spi/i2c/watchdog/fpga/soc etc. But maybe that's just matter of taste.
I fully agree on this part.

great.

Quoted 9 lines

Quoted 6 lines
But I am still missing the point why that patches was that urgent that they were merged to rc3 when it was claimed that socfpga was in a wrong shape for a while. It means v2014.10 should be just another broken version for socfpga and all this mess should be solved properly in 2015.01 via socfpga repo.
Yes, this would mean that another broken version would be released even though the fixes were available. This doesn't sound right to me.

Aug 2 merge windows was closed and Pavel sent RFC to mailing list Sep 3. And Sep 8 I have replied to him to move things to drivers/fpga.

http://lists.denx.de/pipermail/u-boot/2014-September/188311.html

That's why I don't think that at least fpga part was sent to the mailing list at proper time. Maybe I am just wrong and you will find out any really ancient commit with fpga code that I have no problem to admit that I was wrong with fpga part.

Quoted 8 lines

Quoted 3 lines
And because patches went into rc3 and yesterday Jeroen is reporting problem on FreeBSD because of this merge.(http://patchwork.ozlabs.org/patch/397453/)
This problem was discovered in a patch which was first posted to the list on Feb. 19 ( http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/180797 ), which is before 2014.04 release and was not discovered through the review process.

No problem with timing but then why it is not the part of that series if you are aware about that. :-) Introduction new build error between rc2/rc3 is not the best thing to do.

Quoted 7 lines

Quoted 6 lines
Regarding your point that all "It all had been through a few post/review cycles." I don't think all things have been fixed. Personally me I have reported here
http://lists.denx.de/pipermail/u-boot/2014-September/189741.html
(sha1: 0ae16cbb40a2881f6dfbe00fcb023ee7b548bc5c)
issue with checkpatch.pl which hasn't been fixed.

Quoted 6 lines

And I explicitly noted that the FPGA stuff still has a couple of checkpatch issues right in the subsequent email [1] . Processing all the checkpatch issues of that file would make the resulting patch a horrid mess instead of producing a well contained set of patches.
[1] http://lists.denx.de/pipermail/u-boot/2014-September/189745.html

If I look at that patch 27/51 it is just simple sed s/__FUNCTION__/__func__/g and s/PRINTF/debug_cond/g you do also a lot of changes like s/printf (/printf(/g and also for others functions that's why I tend to say that also that 5 warnings should be resolved.

Quoted 5 lines

Quoted 3 lines
Here is my ACK for one patch which is not present in mainline commit
http://lists.denx.de/pipermail/u-boot/2014-September/189747.html
(sha1: 2f210639c4f003b0d5310273979441f1bfc88eae)
Apologies, this one was indeed missed.

no problem at all. As you know I sent it just for fun. :-)

Quoted 11 lines

Quoted 7 lines
Make no sense to go through all patches but this is just an example.
I think it is something what we should discuss at u-boot mini summit on Monday. I discussed this with Marek over IRC yesterday and I expect he will ping me today (because of this email :-) ).
I agree we should discuss this on the minisummit. But what is "this" topic exactly? Do we have a list of topics we need to discuss somewhere, so this one can be added there ?

Last time IRC Detlev had some sort of list of topics which needs to be discussed.

Quoted 8 lines

Quoted 4 lines
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?
I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...

+1 on this.

Thanks,
Michal

Signature · 14 lines

Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


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

Tom Rini

2014-10-09 15:57 UTC #13 lore raw

[U-Boot] Discussion topics / issues

Quoted 8 lines · Michal Simek

Quoted 7 lines · Marek Vasut
Quoted 1 line · Michal Simek
Hi,
Hi!
I changed the subject, since it long didn't match the topic.

[snip]

Quoted 9 lines

Quoted 7 lines
Quoted 3 lines
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?
I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...
+1 on this.

Yes, lets talk more about this.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141009/4ed553ed/attachment.pgp>

Marek Vasut

author

2014-10-09 16:10 UTC #14 lore raw

[U-Boot] Discussion topics / issues

Quoted 15 lines · Tom Rini

Quoted 7 lines · Michal Simek
Quoted 6 lines · Marek Vasut
Quoted 1 line · Michal Simek
Hi,
Hi!
I changed the subject, since it long didn't match the topic.
[snip]
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...
+1 on this.
Yes, lets talk more about this.

Also a CI would really help. Vadim was already proposing one last year too, we should certainly review the outcome on that.

Best regards,
Marek Vasut

Tom Rini

2014-10-09 16:25 UTC #15 lore raw

[U-Boot] Discussion topics / issues

Quoted 21 lines · Marek Vasut

Quoted 20 lines · Tom Rini
Quoted 7 lines · Michal Simek
Quoted 6 lines · Marek Vasut
Quoted 1 line · Michal Simek
Hi,
Hi!
I changed the subject, since it long didn't match the topic.
[snip]
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?
I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
+1 on this.
Yes, lets talk more about this.
Also a CI would really help. Vadim was already proposing one last year too, we should certainly review the outcome on that.

I don't think anything went too far on that, but in a follow-up elsewhere I'm not having buildman do something I thought it did, but perhaps I imagined, which was only show me new problems since the last time it ran.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141009/6e7689e4/attachment.pgp>

Marek Vasut

author

2014-10-09 16:29 UTC #16 lore raw

[U-Boot] Discussion topics / issues

Quoted 26 lines · Tom Rini

Quoted 25 lines · Marek Vasut
Quoted 23 lines · Tom Rini
Quoted 7 lines · Michal Simek
Quoted 6 lines · Marek Vasut
Quoted 1 line · Michal Simek
Hi,
Hi!
I changed the subject, since it long didn't match the topic.
[snip]
Quoted 10 lines
Quoted 8 lines
Quoted 3 lines
If there is a problem because Albert is just too busy we should at least try to find out any reasonable way how to handle this. Like in Linux ARM-SOC custodian?
I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...
+1 on this.
Yes, lets talk more about this.

Quoted 7 lines

Quoted 2 lines
Also a CI would really help. Vadim was already proposing one last year too, we should certainly review the outcome on that.
I don't think anything went too far on that, but in a follow-up elsewhere I'm not having buildman do something I thought it did, but perhaps I imagined, which was only show me new problems since the last time it ran.

Tom, I think we should start building a list for the discussion. There seem to be an awful lot of ideas on what to discuss spurring here, but is anyone taking notes ?

Should I or shall we start logging this into some wiki ?

Best regards,
Marek Vasut

Pavel Machek

2014-10-09 22:11 UTC #17 lore raw

[U-Boot] Discussion topics / issues

In reply to Marek Vasut

Hi!

Quoted 3 lines

I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...

As I said privately, I believe we have way too many custodians...

Anyway, u-boot code looks similar to kernel code, but patch submission rules are really different.

Something like this could help..?
Pavel

SubmittingPatches new file +11 -0

@@ -0,0 +1,11 @@
+Differences from kernel:
+
+* SPDX license headers are required.
+
+* puts() is preffered over single-argument prinf()
+
+* later versions of patch should come with "diff changelog" below "---"
+
+* subject should begin with tags, such as "arm: socfpga:"
+
+* should pass checkpatch
\ No newline at end of file

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Wolfgang Denk

2014-10-09 22:24 UTC #18 lore raw

[U-Boot] Discussion topics / issues

Dear Pavel,

SubmittingPatches

Quoted 6 lines · In message <20141009221154.GA24774@amd> you wrote:

Something like this could help..?
Pavel
SubmittingPatches new file +0 -0

Is there anything wrong with [1] ?

[1] http://www.denx.de/wiki/U-Boot/Patches

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
One possible reason that things aren't going  according  to  plan  is
that there never was a plan in the first place.

Pavel Machek

2014-10-09 23:00 UTC #19 lore raw

[U-Boot] Discussion topics / issues

Hi!

Quoted 9 lines

SubmittingPatches
Quoted 5 lines
Something like this could help..?
Pavel
SubmittingPatches new file +0 -0
Is there anything wrong with [1] ?
[1] http://www.denx.de/wiki/U-Boot/Patches

It should really go into tree.

It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Wolfgang Denk

2014-10-10 12:22 UTC #20 lore raw

[U-Boot] Discussion topics / issues

Dear Pavel,

Quoted 4 lines · In message <20141009230004.GA25685@amd> you wrote:

Quoted 2 lines
[1] http://www.denx.de/wiki/U-Boot/Patches
It should really go into tree.

If you think so...

Quoted 2 lines

It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.

This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.

Don't we always try to use the smallest, most efficient tool that is suited for a task?

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A consultant is a person who borrows your watch, tells you what  time
it is, pockets the watch, and sends you a bill for it.

Jeroen Hofstee

2014-10-10 14:04 UTC #21 lore raw

[U-Boot] Discussion topics / issues

Hello Wolfgang,

Quoted 10 lines · Wolfgang Denk

Quoted 2 lines
It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.
Don't we always try to use the smallest, most efficient tool that is suited for a task?

calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary.

Regards,
Jeroen

Marek Vasut

author

2014-10-10 14:26 UTC #22 lore raw

[U-Boot] Discussion topics / issues

Quoted 12 lines · Jeroen Hofstee

Hello Wolfgang,
Quoted 9 lines · Wolfgang Denk
Quoted 2 lines
It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.

Quoted 5 lines

Quoted 2 lines
Don't we always try to use the smallest, most efficient tool that is suited for a task?
calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary.

Is this LLVM specific or does GCC do that too ? This is interesting information.

Best regards,
Marek Vasut

Fabio Estevam

2014-10-10 14:35 UTC #23 lore raw

[U-Boot] Discussion topics / issues

Hi Marek,

Quoted 4 lines · Marek Vasut

Quoted 2 lines
calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary.
Is this LLVM specific or does GCC do that too ? This is interesting information.

Just did a quick test here with gcc and the resulting u-boot binary size is the same when I use puts or printf.

Jeroen Hofstee

2014-10-10 16:09 UTC #24 lore raw

[U-Boot] Discussion topics / issues

In reply to Marek Vasut

Hello Marek,

Quoted 12 lines · Marek Vasut

Quoted 11 lines · Jeroen Hofstee
Hello Wolfgang,
Quoted 8 lines · Wolfgang Denk
Quoted 2 lines
It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.

Quoted 5 lines

Quoted 4 lines
Quoted 2 lines
Don't we always try to use the smallest, most efficient tool that is suited for a task?
calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary
Is this LLVM specific or does GCC do that too ? This is interesting information.

I was talking about gcc, it has been doing such since ages ago (unless you purposely disable it). clang does it as well.

Regards,
Jeroen

Albert ARIBAUD

2014-10-10 19:51 UTC #25 lore raw

[U-Boot] Discussion topics / issues

Hi Jeroen,

Quoted 15 lines · Jeroen Hofstee

Hello Marek,
Quoted 12 lines · Marek Vasut
Quoted 11 lines · Jeroen Hofstee
Hello Wolfgang,
Quoted 8 lines · Wolfgang Denk
Quoted 2 lines
It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.

Quoted 8 lines

Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Don't we always try to use the smallest, most efficient tool that is suited for a task?
calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary
Is this LLVM specific or does GCC do that too ? This is interesting information.
I was talking about gcc, it has been doing such since ages ago (unless you purposely disable it). clang does it as well.

That's a good thing, but generally speaking, I think that just because the compiler is being clever doesn't mean we are allowed to rely on that, because if we do take a habit of relying on the compiler being clever, two things will happen:

1) we will keep thinking the compiler is being clever even when for some reason it will stop being clever -- for instance, because someone decided to disable the clever feature;

2) we will begin thinking the compiler is clever in situations where it never has and never will.

IMO, a quick cost/benefit comparison of choosing between manually turning printf() into puts whenever doable vs letting the compiler do the changes automatically, the manual option wins -- it's bit like Pascal's Wager: you don't lose much but you can only win.

Quoted 6 lines

Regards,
Jeroen
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Amicalement,

Signature · 1 line

Albert.

Jeroen Hofstee

2014-10-10 20:40 UTC #26 lore raw

[U-Boot] Discussion topics / issues

Hello Albert,

Quoted 24 lines · Albert ARIBAUD

Hi Jeroen,
Quoted 19 lines · Jeroen Hofstee
Hello Marek,
Quoted 16 lines · Marek Vasut
Quoted 15 lines · Jeroen Hofstee
Hello Wolfgang,
Quoted 10 lines · Wolfgang Denk
Quoted 2 lines
It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.
Don't we always try to use the smallest, most efficient tool that is suited for a task?
calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Is this LLVM specific or does GCC do that too ? This is interesting information.
I was talking about gcc, it has been doing such since ages ago (unless you purposely disable it). clang does it as well.
That's a good thing, but generally speaking, I think that just because the compiler is being clever doesn't mean we are allowed to rely on that, because if we do take a habit of relying on the compiler being clever, two things will happen:

Why can't this be relied on, I gave up digging if this is a gcc 3 or 2 feature. It is old at least, museum stuff if it is not supported.

Quoted 3 lines

1) we will keep thinking the compiler is being clever even when for some reason it will stop being clever -- for instance, because someone decided to disable the clever feature;

If you ask to disable it, it is good if it does so, don't see a problem with that. Anyway, it is not an u-boot issue, anything below -O2 is not supported anyway.

Quoted 2 lines

2) we will begin thinking the compiler is clever in situations where it never has and never will.

I would almost take this as an insult, I hope u-boot folks know or at least check before they assume a compiler does XYZ. And yes compilers will replace simple printf call with their simpler equivalent and has been doing so for quite a while (and that is an understatement).

Quoted 4 lines

IMO, a quick cost/benefit comparison of choosing between manually turning printf() into puts whenever doable vs letting the compiler do the changes automatically, the manual option wins -- it's bit like Pascal's Wager: you don't lose much but you can only win.

No it is the other way around; why on earth do you want demand patch submitters to make changes which result in the exactly same binary; you waste time of reviewers / patch submitter and it doesn't serve a goal.

So to turn it around: just use printf: "you don't lose much but you can only win."

Regards,
Jeroen

Albert ARIBAUD

2014-10-10 21:13 UTC #27 lore raw

[U-Boot] Discussion topics / issues

Hi Jeroen,

Quoted 30 lines · Jeroen Hofstee

Hello Albert,
Quoted 27 lines · Albert ARIBAUD
Hi Jeroen,
Quoted 22 lines · Jeroen Hofstee
Hello Marek,
Quoted 12 lines · Marek Vasut
Quoted 11 lines · Jeroen Hofstee
Hello Wolfgang,
Quoted 8 lines · Wolfgang Denk
Quoted 2 lines
It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Don't we always try to use the smallest, most efficient tool that is suited for a task?
calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary
Is this LLVM specific or does GCC do that too ? This is interesting information.
I was talking about gcc, it has been doing such since ages ago (unless you purposely disable it). clang does it as well.

Quoted 7 lines

Quoted 4 lines
That's a good thing, but generally speaking, I think that just because the compiler is being clever doesn't mean we are allowed to rely on that, because if we do take a habit of relying on the compiler being clever, two things will happen:
Why can't this be relied on, I gave up digging if this is a gcc 3 or 2 feature. It is old at least, museum stuff if it is not supported.

It's a general habit I have of never assuming anything is forever.

Quoted 7 lines

Quoted 3 lines
1) we will keep thinking the compiler is being clever even when for some reason it will stop being clever -- for instance, because someone decided to disable the clever feature;
If you ask to disable it, it is good if it does so, don't see a problem with that. Anyway, it is not an u-boot issue, anything below -O2 is not supported anyway.

There is no problem if the person disabling the feature is also the one doing assumptions on the feature state. Problems arise when these are two different persons, because the one relying on the feature might not know or notice that the other one disabled it.

Quoted 7 lines

Quoted 2 lines
2) we will begin thinking the compiler is clever in situations where it never has and never will.
I would almost take this as an insult, I hope u-boot folks know or at least check before they assume a compiler does XYZ. And yes compilers will replace simple printf call with their simpler equivalent and has been doing so for quite a while (and that is an understatement).

I certainly did not intend this as an insult. But no, if I personally had encountered a printf("%s\n", s) in the course of some development, I would not have gone and looked whether the compiler might turn it into a puts(s). I'd have assumed it does not, because assuming so was the fastest and safest approach.

Don't misunderstand me: I'm quite pleased that gcc and LLVM do that. And I'm also quite convinced that it's a good thing -- actually, I did start by saying so. The problem I see is not specifically with the printf()/puts() case, it is with the *general* assumption that a compiler is clever -- and again, I did start my opinion with "generally speaking".

The problem I see is that a developer will not necessarily go and test every single thing that the compiler *might* be clever enough to do (or dump enough not to); a developer will assume some of it, and whenever he assumes, he should assume a dumb compiler.

Quoted 5 lines

Quoted 4 lines
IMO, a quick cost/benefit comparison of choosing between manually turning printf() into puts whenever doable vs letting the compiler do the changes automatically, the manual option wins -- it's bit like Pascal's Wager: you don't lose much but you can only win.

Quoted 7 lines

No it is the other way around; why on earth do you want demand patch submitters to make changes which result in the exactly same binary; you waste time of reviewers / patch submitter and it doesn't serve a goal.
So to turn it around: just use printf: "you don't lose much but you can only win."

In the specific case of printf()/puts(), a wager is meaningless since the compiler capability is already known. Therefore, there is no reason to "wager on 'just us[ing] printf'"; there is reason to "just use printf because it is known to turn into puts() if possible".

But please remember that I considered the wager *only* in a *general* case where a developer could replace some code with some other, more efficient code, and does *not* know whether the compiler could be clever enough to do the replacement by itself. The developer /might/ check the compiler... or he might not check, and instead, wager on the compiler being clever or dumb; and in this *general* case, he should wager on a dumb compiler.

Of course, if the case is specific and known to him, i.e. he already knows whether the compiler will be clever or dumb in that specific case, then there is *no* wager to be done; the developer should just act on his knowledge (and check it again from time to time, but that's somewhat beside the point).

Quoted 2 lines

Regards,
Jeroen

Amicalement,

Signature · 1 line

Albert.

Wolfgang Denk

2014-10-11 15:03 UTC #28 lore raw

[U-Boot] Discussion topics / issues

In reply to Jeroen Hofstee

Dear Jeroen,

Quoted 4 lines · In message <[email protected]> you wrote:

If you ask to disable it, it is good if it does so, don't see a problem with that. Anyway, it is not an u-boot issue, anything below -O2 is not supported anyway.

I'm not sure what you mean here. Gcc certainly does this replacement with -Os as used for U-Boot.

Quoted 4 lines

I would almost take this as an insult, I hope u-boot folks know or at least check before they assume a compiler does XYZ. And yes compilers will replace simple printf call with their simpler equivalent and has been doing so for quite a while (and that is an understatement).

I wonder how many people know about this - and where it is documented?

Quoted 2 lines

So to turn it around: just use printf: "you don't lose much but you can only win."

Sorry, but I disagree here.

First, we have a compatibility problem here. GCC assumes that puts() will add a newline character after the string; U-Boot puts() does NOT do this. So the GCC auto-converted printf()s will all be wrong, as they are missing the newline. [1]

Second, using puts() insteas of printf() is also a means of documenting your code. It shows your intention to print a constant string. Just one example: compare

	void add_header(const char *s)
	{
		printf("MY HEADER: %s\n", s);
	}

versus

	void add_header(const char *s)
	{
		puts("MY HEADER: ");
		puts(s);
		/* Assuming U-Boot puts() - no automatic \n added */
		putc('\n');
	}

Which is "better"? A is obviously much shorter and more elegant; but B is much more robust - A will happily crash your system when you try to print a string like "s%s%s%s%s%s%s%s%s%s%s" (not to mention that this may open a classic attack vector to break into a running system).

So yes, it does make sense to explicitly use puts().

[1] One might argue that this is a bug in U-Boot and should be fixed, but that is another topic.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A verbal contract isn't worth the paper it's written on.
                                                    -- Samuel Goldwyn

Wolfgang Denk

2014-10-11 15:16 UTC #29 lore raw

[U-Boot] Discussion topics / issues

Dear Jeroen,

Quoted 5 lines · In message <[email protected]> i wrote:

Which is "better"? A is obviously much shorter and more elegant; but B is much more robust - A will happily crash your system when you try to print a string like "s%s%s%s%s%s%s%s%s%s%s" (not to mention that this may open a classic attack vector to break into a running system).

Ignore me. This example was obviously crap. What I had in mind was something where you would use

	char *s;
	...
	printf(s);

Sorry...

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Documentation is like sex: when it is good, it is  very,  very  good;
and when it is bad, it is better than nothing.         - Dick Brandon

Pavel Machek

2014-10-15 08:40 UTC #30 lore raw

[U-Boot] puts() and newlines (was Re: Discussion topics / issues)

In reply to Wolfgang Denk

Hi!

Quoted 7 lines

First, we have a compatibility problem here. GCC assumes that puts() will add a newline character after the string; U-Boot puts() does NOT do this. So the GCC auto-converted printf()s will all be wrong, as they are missing the newline. [1]
[1] One might argue that this is a bug in U-Boot and should be fixed, but that is another topic.

I believe we should fix that, yes.

I did quick grep,

pavel at duo:~/wagabuibui/u-boot$ grep -ri puts . | wc -l 4287

and that is probably too much to change in one go. So what about this?

Best regards,
Pavel
---

Introduce __puts() that puts strings without trailing newline. Plan is to move the existing puts() users into __puts(), when no puts() users are left, fix the puts() to add the newline, and move users that want newline back to puts().

Signed-off-by Pavel Machek <[email protected]>

include/common.h +2 -0

@@ -836,6 +836,8 @@ int	tstc(void);
 /* stdout */
 void	putc(const char c);
 void	puts(const char *s);
+static inline void __puts(const char *s) { puts(s); }
+
 int	printf(const char *fmt, ...)
 		__attribute__ ((format (__printf__, 1, 2)));
 int	vprintf(const char *fmt, va_list args);

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2014-10-15 09:42 UTC #31 lore raw

[U-Boot] puts() and newlines (was Re: Discussion topics / issues)

Quoted 12 lines · Pavel Machek

Hi!
Quoted 7 lines
First, we have a compatibility problem here. GCC assumes that puts() will add a newline character after the string; U-Boot puts() does NOT do this. So the GCC auto-converted printf()s will all be wrong, as they are missing the newline. [1]
[1] One might argue that this is a bug in U-Boot and should be fixed, but that is another topic.
I believe we should fix that, yes.

Quoted 7 lines

I did quick grep,
pavel at duo:~/wagabuibui/u-boot$ grep -ri puts . | wc -l 4287
and that is probably too much to change in one go. So what about this?

Next step is probably

include/common.h +3 -0

index d5020c8..95b2377 100644
@@ -836,6 +836,9 @@ int	tstc(void);
 /* stdout */
 void	putc(const char c);
 void	puts(const char *s);
+static inline void __puts(const char *s) { puts(s); }
+static inline void putsnl(const char *s) { puts(s); putc('\n'); }
+
 int	printf(const char *fmt, ...)
 		__attribute__ ((format (__printf__, 1, 2)));
 int	vprintf(const char *fmt, va_list args);

Then run

sed 's/puts($.*$\\n")/putsnl(\1")/' find . -name "*.[ch]"

... to get existing users converted. Then we can convert the rest to to either __puts or putsnl, and finally get rid of putsnl and convert it back to puts.

(Patch is >400KB, so I'll not post it here).

Best regards,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Tom Rini

2014-10-20 15:51 UTC #32 lore raw

[U-Boot] puts() and newlines (was Re: Discussion topics / issues)

In reply to Pavel Machek

Quoted 10 lines · Pavel Machek

Hi!
Quoted 7 lines
First, we have a compatibility problem here. GCC assumes that puts() will add a newline character after the string; U-Boot puts() does NOT do this. So the GCC auto-converted printf()s will all be wrong, as they are missing the newline. [1]
[1] One might argue that this is a bug in U-Boot and should be fixed, but that is another topic.

Quoted 8 lines

I believe we should fix that, yes.
I did quick grep,
pavel at duo:~/wagabuibui/u-boot$ grep -ri puts . | wc -l 4287
and that is probably too much to change in one go. So what about this?

I'm thinking, now that we know that with $(CC) -ffreestanding printf is not converted to puts, we should (a) globally change puts("no newline") to printf (b) fix puts behavior to conform to standards and correct puts("newline\n") to puts("newline"). And then let people use whatever of the print functions they want, while we sort out making people use debug(...) / error(...) more and come up with a clever output scheme like we talked about in person.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141020/112db177/attachment.pgp>

Wolfgang Denk

2014-10-11 14:44 UTC #33 lore raw

[U-Boot] Discussion topics / issues

In reply to Jeroen Hofstee

Dear Jeroen,

Quoted 3 lines · In message <[email protected]> you wrote:

calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary.

Interesting, I didn't know that. Is this somewhere documented?

Is there any comprehensive list of such "magic" optimizations?

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Research is what I'm doing when I don't know what I'm doing.
                                                 -- Wernher von Braun

Jeroen Hofstee

2014-10-12 15:06 UTC #34 lore raw

[U-Boot] Discussion topics / issues

In reply to Jeroen Hofstee

Hello Wolfgang / Albert / others,

Quoted 11 lines · Jeroen Hofstee

Hello Wolfgang,
Quoted 8 lines · Wolfgang Denk
Quoted 2 lines
It does not mention puts() vs. printf(), if it is indeed meant to be u-boot policy.
This is not just U-Boot philosophy, but something that I would consider a matter of course when writing code - using the appropriate tools for the task at hand. If all you want to do is sendout a constant string to the utput device, there is no need to invoke a function that provides fancy formatting options.

Quoted 5 lines

Quoted 2 lines
Don't we always try to use the smallest, most efficient tool that is suited for a task?
calling printf("%s\n", "string") gets translated into puts by the compiler. There should be no difference in the binary.

mumbles: while this is true in general it won't hold for u-boot since -ffreestanding disables such rewrites and u-boot is compiled with that flag. On the bright side, perhaps I educated some people a bit that they are wasting time rewriting such lines in normal, hosted applications.

Regards,
Jeroen

Pavel Machek

2014-10-09 23:05 UTC #35 lore raw

[U-Boot] Discussion topics / issues

In reply to Wolfgang Denk

Quoted 7 lines · Wolfgang Denk

Dear Pavel,
Quoted 4 lines · In message <20141009221154.GA24774@amd> you wrote:
Something like this could help..?
Pavel

Quoted 6 lines

SubmittingPatches
Quoted 2 lines
SubmittingPatches new file +0 -0
Is there anything wrong with [1] ?
[1] http://www.denx.de/wiki/U-Boot/Patches

..and actually... it makes submitting patches rather hard.

[PATCH] fix compilation on socfpga

Quoted 1 line

Please add tags to the subject

[PATCHv2] arm: socfpga: fix compilation on socfpga

Quoted 1 line

Please add diff from previous version

[PATCHv3] arm: socfpga: fix compilation on socfpga

---

v2: added tags to the subject

v3: added diffs to previous version

. (From memory, but IIRC something very similar to this happened before).

This scares of all but the most determined patch submitters, and does not really improve code quality.

I'd argue that if only changelog is updated, it is _not_ a new version of patch, and does not need changelog diff. Or maybe be less strict policy / less strict enforcement of the policy in trivial cases.

Best regards,

Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Albert ARIBAUD

2014-10-10 11:05 UTC #36 lore raw

[U-Boot] Discussion topics / issues

Hi Pavel,

Quoted 23 lines · Pavel Machek

Quoted 7 lines · Wolfgang Denk
Dear Pavel,
Quoted 4 lines · In message <20141009221154.GA24774@amd> you wrote:
Something like this could help..?
Pavel
Quoted 6 lines
SubmittingPatches
Quoted 2 lines
SubmittingPatches new file +0 -0
Is there anything wrong with [1] ?
[1] http://www.denx.de/wiki/U-Boot/Patches
..and actually... it makes submitting patches rather hard.
[PATCH] fix compilation on socfpga
Quoted 1 line
Please add tags to the subject
[PATCHv2] arm: socfpga: fix compilation on socfpga

Quoted 7 lines

Quoted 1 line
Please add diff from previous version
[PATCHv3] arm: socfpga: fix compilation on socfpga
---
v2: added tags to the subject

Tags can be useful in automating CC: lists from Patman through doc/git-mailrc, and as a filtering key in e.g. gitk, hence the suggestion to add them. Guessing which tags a patch could use is indeed a tedious and uncertain process, but I don't think it is requested of many patches, is it?

Quoted 3 lines

v3: added diffs to previous version
. (From memory, but IIRC something very similar to this happened before).

At least it happened that I requested the change logs when they were missing entirely in a v2-or-later series. The reason is that with these logs, reviewers can see what change requests were acknowledged by the submitter and what other changes were spontaneous additions.

Quoted 2 lines

This scares of all but the most determined patch submitters, and does not really improve code quality.

One can argue that it improves code /review/, by both making sure the submitter has involved the relevant custodians (tags) and provided a follow-up on their previous remarks (diffs).

Note that patman help a lot about maintaining the change log and tags.

Quoted 3 lines

I'd argue that if only changelog is updated, it is _not_ a new version of patch, and does not need changelog diff. Or maybe be less strict policy / less strict enforcement of the policy in trivial cases.

Well, if only a changelog is updated, then a [PATCH vN RESEND] should be as ok as a [PATCH vN+1], and anyway both will end up as "a new patch" for Patchwork, so the difference is not really major IMO -- meaning both should be accepted and, I believe, are accepted in practice.

Quoted 3 lines

Best regards,
Pavel

Amicalement,

Signature · 1 line

Albert.

Wolfgang Denk

2014-10-10 12:34 UTC #37 lore raw

[U-Boot] Discussion topics / issues

In reply to Pavel Machek

Dear Pavel,

Quoted 4 lines · In message <20141009230559.GB25685@amd> you wrote:

v2: added tags to the subject
v3: added diffs to previous version
. (From memory, but IIRC something very similar to this happened before).

Yes, this happens when people repeatedly ignore to read the patch posting rules.

Quoted 3 lines

I'd argue that if only changelog is updated, it is _not_ a new version of patch, and does not need changelog diff. Or maybe be less strict policy / less strict enforcement of the policy in trivial cases.

You ignore the fact that you are supposed to miimize the work load you create on reviewers. If I try to review a patch, I need to understand what has been changed compared to the previous version. Yes, of course I can apply both versions and run a diff (diffs over diffs are too difficult to read in general), but this costs time. And each reviewer has to spend this time.

On the other hand, you should know exactly what you've changes, so it is minimal work for you to add such an explanation. It you use the provided tools for the task (patman comes to mind), this is even well supported.

So what is better for the community: if you spend a little time once, or if every reviewer spends much more time trying to figure out what you might have changed? So it all depends whether you take the egotistic or the community point of view....

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I think animal testing is a terrible idea; they get all  nervous  and
give the wrong answers.

Tom Rini

2014-10-10 00:12 UTC #38 lore raw

[U-Boot] Discussion topics / issues

In reply to Pavel Machek

Quoted 7 lines · Pavel Machek

Hi!
Quoted 3 lines
I don't this Albert is the problem, I am starting to suspect we simply lack custodian manpower in general. And I also suspect we're not quite inviting and attractive crowd, which is something we should discuss too ...
As I said privately, I believe we have way too many custodians...

I think perhaps I need to better spell out custodian expectations. But as to having too many? No, I need some convincing in that direction.

SubmittingPatches line 1

Quoted 10 lines

Anyway, u-boot code looks similar to kernel code, but patch submission rules are really different.
Something like this could help..?
Pavel
SubmittingPatches new file +1 -0
@@ -0,0 +1,11 @@
+Differences from kernel:

Perhaps. If not in tree, a concice reminder to follow all of the rules spelled out on the wiki to avoid the ping-ponging you noted in a follow up later.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141009/62d82fd3/attachment.pgp>

Dinh Nguyen

2014-10-08 13:18 UTC #39 lore raw

In reply to Marek Vasut

Hi Marek,

Quoted 5 lines · Marek Vasut

Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.

Thanks for all of your hard work on this!

Quoted 14 lines

First thing I should probably clarify is the late acceptance of the socfpga patches. This is certainly not something we do regularly and is one of the worst possible practices to do, but this time it felt rather important to get the platform in shape, so this exception happened. Furthermore, all of the code in u-boot-socfpga should be based on u-boot-arm and should be submitted through the u-boot-arm repository, not directly to u-boot .
As you probably noticed, there is a lot of topic branches in the u-boot-socfpga
[1] repository, each of them with a date tag. This decision came to be so that rebasing of a branch can be avoided. I will most likely garbage-collect the old and useless topic branches at some point, when they become irrelevant due to the code being merged into u-boot-arm/master (and then to u-boot/master).

Quoted 8 lines

I think that's about it for the organization part. Now for the remaining part, we are still missing a few things. Some of them are ready, some of them, not so much:
SPL: This is something we still miss from mainline. We will need to discuss
this thoroughly, but one thing is obvious already -- we need to figure out how to interoperate with Quartus resp. the bsp-editor generated header files to assemble the SPL properly.

Have you or anyone else already started on this work? If not, then this is an area that I can start to work on.

Quoted 8 lines

USB: This is scheduled for the next merge window. The DWC2 driver is cleaned
up and seems to be in rather good state. The USB driver currently resides in [2]
EPCQ: This is something I prepared and tested real quick. The EPCS/EPCQ SPI NOR
can be operated via the Altera SPI driver, which is currently unused at all and thus suffering bitrot. The current cleanup is here [3]
NET: Does the SoCDK use EMAC0 or EMAC1 ? I believe we still have this issue
open, I recall Chin was complaining about this.

The SoCDK is using EMAC1.

BR,
Dinh

Marek Vasut

author

2014-10-08 19:05 UTC #40 lore raw

Quoted 1 line · Dinh Nguyen

Hi Marek,

Hi all!

Quoted 8 lines · Marek Vasut

Quoted 5 lines
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Thanks for all of your hard work on this!

I only did what needed to be done and I'm looking forward to you taking over shortly ;-)

[...]

Quoted 9 lines

Quoted 6 lines
SPL: This is something we still miss from mainline. We will need to discuss
this thoroughly, but one thing is obvious already -- we need to figure out how to interoperate with Quartus resp. the bsp-editor generated header files to assemble the SPL properly.
Have you or anyone else already started on this work? If not, then this is an area that I can start to work on.

No, noone did and I'd be really happy if you took this one.

Quoted 13 lines

Quoted 13 lines
USB: This is scheduled for the next merge window. The DWC2 driver is cleaned
up and seems to be in rather good state. The USB driver currently resides in [2]
EPCQ: This is something I prepared and tested real quick. The EPCS/EPCQ SPI NOR
can be operated via the Altera SPI driver, which is currently unused at all and thus suffering bitrot. The current cleanup is here [3]

Quoted 6 lines

Quoted 4 lines
NET: Does the SoCDK use EMAC0 or EMAC1 ? I believe we still have this issue
open, I recall Chin was complaining about this.
The SoCDK is using EMAC1.

I guess we'd need one more quick patch for current mainline then, since this seems to be broken ever since. I will bake one shortly.

Also, let me add two more goals:
- Migrate to driver model
- Support probing of the whole board from Device Tree Blob

This would make our lives so much easier, so we should aim for this. I don't know if it is realistic to make this happen for 2015.01, but I would like to keep this in mind.

Best regards,
Marek Vasut

Masahiro YAMADA

2014-10-11 18:22 UTC #41 lore raw

In reply to Marek Vasut

Hi Marek,

2014-10-07 21:45 GMT+09:00 Marek Vasut <[email protected]>:

Quoted 5 lines

Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.

Can you refactor
arch/arm/include/asm/spl.h and arch/arm/include/asm/arch-socfpga/spl.h ?

I mean, I want all the SoCs to use BOOT_DEVICE_* defined in arch/arm/include/asm/spl.h rather than duplicating it in arch/arm/include/asm/arch-*/spl.h

I am expecting something like what Michal did for Zynq in commit dbc31f6a and commit ae2ee77f98.

Signature · 2 lines

Best Regards
Masahiro Yamada

Marek Vasut

author

2014-10-19 21:19 UTC #42 lore raw

Quoted 1 line · Masahiro YAMADA

Hi Marek,

Hello!

Quoted 11 lines

2014-10-07 21:45 GMT+09:00 Marek Vasut <[email protected]>:
Quoted 5 lines
Hey,
given that we now have most of the u-boot socfpga stuff in mainline, I decided it would be a good idea to list what we're still missing and we should also decide how to move on now.
Can you refactor
arch/arm/include/asm/spl.h and arch/arm/include/asm/arch-socfpga/spl.h ?

Quoted 7 lines

I mean, I want all the SoCs to use BOOT_DEVICE_* defined in arch/arm/include/asm/spl.h rather than duplicating it in arch/arm/include/asm/arch-*/spl.h
I am expecting something like what Michal did for Zynq in commit dbc31f6a and commit ae2ee77f98.

I just sent out a patch:
Re: [PATCH] arm: socfpga: Zap spl.h and ad-hoc related syms This should resolve it.

Best regards,
Marek Vasut