[U-Boot] [SoCFPGA] next steps

[U-Boot] [SoCFPGA] next steps

[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.

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

[U-Boot] [SoCFPGA] next steps

[Michal Simek]

Hi,

On 10/07/2014 02:45 PM, Marek Vasut wrote:
> 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 .

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

-- 
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>

[U-Boot] [SoCFPGA] next steps

[Marek Vasut]

On Wednesday, October 08, 2014 at 10:58:24 AM, Michal Simek wrote:
> Hi,
> 
> On 10/07/2014 02:45 PM, Marek Vasut wrote:
> > 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 .
> 
> 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

[U-Boot] [SoCFPGA] next steps

[Pavel Machek]

On Wed 2014-10-08 10:58:24, Michal Simek wrote:
> Hi,
> 
> On 10/07/2014 02:45 PM, Marek Vasut wrote:
> > 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 .
> 
> 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [SoCFPGA] next steps

[Dinh Nguyen]

Hi Marek,

On 10/7/14, 7:45 AM, Marek Vasut wrote:
> 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!

> 
> 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.

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

>  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

[U-Boot] [SoCFPGA] next steps

[Marek Vasut]

On Wednesday, October 08, 2014 at 03:18:10 PM, Dinh Nguyen wrote:
> Hi Marek,

Hi all!

> On 10/7/14, 7:45 AM, Marek Vasut wrote:
> > 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 ;-)

[...]

> >  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.

> >  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.

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

[U-Boot] [SoCFPGA] next steps

[Tom Rini]

On Wed, Oct 08, 2014 at 10:58:24AM +0200, Michal Simek wrote:
> Hi,
> 
> On 10/07/2014 02:45 PM, Marek Vasut wrote:
> > 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 .
> 
> 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.

-- 
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>

[U-Boot] [SoCFPGA] next steps

[Michal Simek]

Hi,

On 10/08/2014 10:09 PM, Tom Rini wrote:
> On Wed, Oct 08, 2014 at 10:58:24AM +0200, Michal Simek wrote:
>> Hi,
>>
>> On 10/07/2014 02:45 PM, Marek Vasut wrote:
>>> 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 .
>>
>> 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.

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

-- 
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>

[U-Boot] [SoCFPGA] next steps

[Jagan Teki]

On 9 October 2014 14:07, Michal Simek <monstr@monstr.eu> wrote:
> Hi,
>
> On 10/08/2014 10:09 PM, Tom Rini wrote:
>> On Wed, Oct 08, 2014 at 10:58:24AM +0200, Michal Simek wrote:
>>> Hi,
>>>
>>> On 10/07/2014 02:45 PM, Marek Vasut wrote:
>>>> 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 .
>>>
>>> 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.
>
> 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?

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!
-- 
Jagan.

[U-Boot] [SoCFPGA] next steps

[Marek Vasut]

On Thursday, October 09, 2014 at 01:20:23 PM, Jagan Teki wrote:
> On 9 October 2014 14:07, Michal Simek <monstr@monstr.eu> wrote:
> > Hi,
> > 
> > On 10/08/2014 10:09 PM, Tom Rini wrote:
> >> On Wed, Oct 08, 2014 at 10:58:24AM +0200, Michal Simek wrote:
> >>> Hi,
> >>> 
> >>> On 10/07/2014 02:45 PM, Marek Vasut wrote:
> >>>> 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 .
> >>> 
> >>> 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.
> > 
> > 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?
> 
> 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.

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.

> 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?

> 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.

[U-Boot] Discussion topics / issues

[Marek Vasut]

On Thursday, October 09, 2014 at 10:37:52 AM, Michal Simek wrote:
> Hi,

Hi!

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

> On 10/08/2014 10:09 PM, Tom Rini wrote:
> > On Wed, Oct 08, 2014 at 10:58:24AM +0200, Michal Simek wrote:
> >> Hi,
> >> 
> >> On 10/07/2014 02:45 PM, Marek Vasut wrote:
> >>> 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 .
> >> 
> >> 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.
> 
> 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.

> 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.

> 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.

> 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.

> 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.

> 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.

> 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

> 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.

> 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 ?

> 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 ...

[U-Boot] Discussion topics / issues

[Michal Simek]

On 10/09/2014 04:03 PM, Marek Vasut wrote:
> On Thursday, October 09, 2014 at 10:37:52 AM, Michal Simek wrote:
>> Hi,
> 
> Hi!
> 
> I changed the subject, since it long didn't match the topic.
> 
>> On 10/08/2014 10:09 PM, Tom Rini wrote:
>>> On Wed, Oct 08, 2014 at 10:58:24AM +0200, Michal Simek wrote:
>>>> Hi,
>>>>
>>>> On 10/07/2014 02:45 PM, Marek Vasut wrote:
>>>>> 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 .
>>>>
>>>> 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.
>>
>> 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.

>> 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.

>> 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.
> 
>> 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.

> 
>> 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.

>> 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.

>> 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

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.

>> 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. :-)

> 
>> 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.

> 
>> 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

-- 
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>

[U-Boot] Discussion topics / issues

[Tom Rini]

On Thu, Oct 09, 2014 at 04:45:07PM +0200, Michal Simek wrote:
> On 10/09/2014 04:03 PM, Marek Vasut wrote:
> > On Thursday, October 09, 2014 at 10:37:52 AM, Michal Simek wrote:
> >> Hi,
> > 
> > Hi!
> > 
> > I changed the subject, since it long didn't match the topic.
> > 
[snip]
> >> 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.

-- 
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>

[U-Boot] Discussion topics / issues

[Marek Vasut]

On Thursday, October 09, 2014 at 05:57:47 PM, Tom Rini wrote:
> On Thu, Oct 09, 2014 at 04:45:07PM +0200, Michal Simek wrote:
> > On 10/09/2014 04:03 PM, Marek Vasut wrote:
> > > On Thursday, October 09, 2014 at 10:37:52 AM, Michal Simek wrote:
> > >> Hi,
> > > 
> > > Hi!
> > > 
> > > I changed the subject, since it long didn't match the topic.
> 
> [snip]
> 
> > >> 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.

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

[U-Boot] [SoCFPGA] next steps

[Jagan Teki]

On 9 October 2014 19:12, Marek Vasut <marex@denx.de> wrote:
> On Thursday, October 09, 2014 at 01:20:23 PM, Jagan Teki wrote:
>> On 9 October 2014 14:07, Michal Simek <monstr@monstr.eu> wrote:
>> > Hi,
>> >
>> > On 10/08/2014 10:09 PM, Tom Rini wrote:
>> >> On Wed, Oct 08, 2014 at 10:58:24AM +0200, Michal Simek wrote:
>> >>> Hi,
>> >>>
>> >>> On 10/07/2014 02:45 PM, Marek Vasut wrote:
>> >>>> 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 .
>> >>>
>> >>> 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.
>> >
>> > 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?
>>
>> 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.
>
> 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.

>
>> 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.

>
>> 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!
-- 
Jagan.

[U-Boot] Discussion topics / issues

[Marek Vasut]

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

[...]

> >> 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.

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.

> >> 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!

> >> 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

[U-Boot] Discussion topics / issues

[Tom Rini]

On Thu, Oct 09, 2014 at 06:10:12PM +0200, Marek Vasut wrote:
> On Thursday, October 09, 2014 at 05:57:47 PM, Tom Rini wrote:
> > On Thu, Oct 09, 2014 at 04:45:07PM +0200, Michal Simek wrote:
> > > On 10/09/2014 04:03 PM, Marek Vasut wrote:
> > > > On Thursday, October 09, 2014 at 10:37:52 AM, Michal Simek wrote:
> > > >> Hi,
> > > > 
> > > > Hi!
> > > > 
> > > > I changed the subject, since it long didn't match the topic.
> > 
> > [snip]
> > 
> > > >> 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.
> 
> 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
-------------- 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>

[U-Boot] Discussion topics / issues

[Marek Vasut]

On Thursday, October 09, 2014 at 06:25:30 PM, Tom Rini wrote:
> On Thu, Oct 09, 2014 at 06:10:12PM +0200, Marek Vasut wrote:
> > On Thursday, October 09, 2014 at 05:57:47 PM, Tom Rini wrote:
> > > On Thu, Oct 09, 2014 at 04:45:07PM +0200, Michal Simek wrote:
> > > > On 10/09/2014 04:03 PM, Marek Vasut wrote:
> > > > > On Thursday, October 09, 2014 at 10:37:52 AM, Michal Simek wrote:
> > > > >> Hi,
> > > > > 
> > > > > Hi!
> > > > > 
> > > > > I changed the subject, since it long didn't match the topic.
> > > 
> > > [snip]
> > > 
> > > > >> 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.
> > 
> > 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

[U-Boot] Discussion topics / issues

[Jagan Teki]

On 9 October 2014 21:45, Marek Vasut <marex@denx.de> wrote:
> On Thursday, October 09, 2014 at 06:11:37 PM, Jagan Teki wrote:
>
> [...]
>
>> >> 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.
>
> 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.

>
>> >> 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!
>
>> >> 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)

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

thanks!
-- 
Jagan.

[U-Boot] Discussion topics / issues

[Pavel Machek]

Hi!

> 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

--- /dev/null	2014-10-09 01:15:57.354292026 +0200
+++ doc/SubmittingPatches	2014-10-09 23:58:53.058883776 +0200
@@ -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




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

[U-Boot] Discussion topics / issues

[Wolfgang Denk]

Dear Pavel,

In message <20141009221154.GA24774@amd> you wrote:
> 
> Something like this could help..?
> 									Pavel
> 
> --- /dev/null	2014-10-09 01:15:57.354292026 +0200
> +++ doc/SubmittingPatches	2014-10-09 23:58:53.058883776 +0200

Is there anything wrong with [1] ?

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

Best regards,

Wolfgang Denk

-- 
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.

[U-Boot] Discussion topics / issues

[Pavel Machek]

Hi!

> > Something like this could help..?
> > 									Pavel
> > 
> > --- /dev/null	2014-10-09 01:15:57.354292026 +0200
> > +++ doc/SubmittingPatches	2014-10-09 23:58:53.058883776 +0200
> 
> 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

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

[U-Boot] Discussion topics / issues

[Pavel Machek]

On Fri 2014-10-10 00:24:46, Wolfgang Denk wrote:
> Dear Pavel,
> 
> In message <20141009221154.GA24774@amd> you wrote:
> > 
> > Something like this could help..?
> > 									Pavel
> > 
> > --- /dev/null	2014-10-09 01:15:57.354292026 +0200
> > +++ doc/SubmittingPatches	2014-10-09 23:58:53.058883776 +0200
> 
> 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

> Please add tags to the subject

[PATCHv2] arm: socfpga: fix compilation on socfpga

> 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] Discussion topics / issues

[Tom Rini]

On Fri, Oct 10, 2014 at 12:11:54AM +0200, Pavel Machek wrote:
> Hi!
> 
> > 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.

> Anyway, u-boot code looks similar to kernel code, but patch submission
> rules are really different.
> 
> Something like this could help..?
> 									Pavel
> 
> --- /dev/null	2014-10-09 01:15:57.354292026 +0200
> +++ doc/SubmittingPatches	2014-10-09 23:58:53.058883776 +0200
> @@ -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.

-- 
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>

[U-Boot] Discussion topics / issues

[Albert ARIBAUD]

Hi Pavel,

On Fri, 10 Oct 2014 01:05:59 +0200, Pavel Machek <pavel@denx.de> wrote:

> On Fri 2014-10-10 00:24:46, Wolfgang Denk wrote:
> > Dear Pavel,
> > 
> > In message <20141009221154.GA24774@amd> you wrote:
> > > 
> > > Something like this could help..?
> > > 									Pavel
> > > 
> > > --- /dev/null	2014-10-09 01:15:57.354292026 +0200
> > > +++ doc/SubmittingPatches	2014-10-09 23:58:53.058883776 +0200
> > 
> > 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
> 
> > Please add tags to the subject
> 
> [PATCHv2] arm: socfpga: fix compilation on socfpga
> 
> > 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?

> 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.

> 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.

> 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.

> Best regards,
> 
> 									Pavel

Amicalement,
-- 
Albert.

[U-Boot] Discussion topics / issues

[Wolfgang Denk]

Dear Pavel,

In message <20141009230004.GA25685@amd> you wrote:
> 
> > [1] http://www.denx.de/wiki/U-Boot/Patches
> 
> It should really go into tree.

If you think so...

> 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

-- 
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.

[U-Boot] Discussion topics / issues

[Wolfgang Denk]

Dear Pavel,

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.

> 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

-- 
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.

[U-Boot] Discussion topics / issues

[Jeroen Hofstee]

Hello Wolfgang,

On 10-10-14 14:22, Wolfgang Denk wrote:
>> 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

[U-Boot] Discussion topics / issues

[Marek Vasut]

On Friday, October 10, 2014 at 04:04:40 PM, Jeroen Hofstee wrote:
> Hello Wolfgang,
> 
> On 10-10-14 14:22, Wolfgang Denk wrote:
> >> 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.

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

Best regards,
Marek Vasut

[U-Boot] Discussion topics / issues

[Fabio Estevam]

Hi Marek,

On Fri, Oct 10, 2014 at 11:26 AM, Marek Vasut <marex@denx.de> wrote:

>> 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.

[U-Boot] Discussion topics / issues

[Jeroen Hofstee]

Hello Marek,

On 10-10-14 16:26, Marek Vasut wrote:
> On Friday, October 10, 2014 at 04:04:40 PM, Jeroen Hofstee wrote:
>> Hello Wolfgang,
>>
>> On 10-10-14 14:22, Wolfgang Denk wrote:
>>>> 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
> 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

[U-Boot] Discussion topics / issues

[Albert ARIBAUD]

Hi Jeroen,

On Fri, 10 Oct 2014 18:09:19 +0200, Jeroen Hofstee
<jeroen@myspectrum.nl> wrote:

> Hello Marek,
> 
> On 10-10-14 16:26, Marek Vasut wrote:
> > On Friday, October 10, 2014 at 04:04:40 PM, Jeroen Hofstee wrote:
> >> Hello Wolfgang,
> >>
> >> On 10-10-14 14:22, Wolfgang Denk wrote:
> >>>> 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
> > 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.

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


Amicalement,
-- 
Albert.

[U-Boot] Discussion topics / issues

[Jeroen Hofstee]

Hello Albert,

On 10-10-14 21:51, Albert ARIBAUD wrote:
> Hi Jeroen,
>
> On Fri, 10 Oct 2014 18:09:19 +0200, Jeroen Hofstee
> <jeroen@myspectrum.nl> wrote:
>
>> Hello Marek,
>>
>> On 10-10-14 16:26, Marek Vasut wrote:
>>> On Friday, October 10, 2014 at 04:04:40 PM, Jeroen Hofstee wrote:
>>>> Hello Wolfgang,
>>>>
>>>> On 10-10-14 14:22, Wolfgang Denk wrote:
>>>>>> 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
>>> 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.

> 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.

> 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).

> 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

[U-Boot] Discussion topics / issues

[Albert ARIBAUD]

Hi Jeroen,

On Fri, 10 Oct 2014 22:40:48 +0200, Jeroen Hofstee
<dasuboot@myspectrum.nl> wrote:

> Hello Albert,
> 
> On 10-10-14 21:51, Albert ARIBAUD wrote:
> > Hi Jeroen,
> >
> > On Fri, 10 Oct 2014 18:09:19 +0200, Jeroen Hofstee
> > <jeroen@myspectrum.nl> wrote:
> >
> >> Hello Marek,
> >>
> >> On 10-10-14 16:26, Marek Vasut wrote:
> >>> On Friday, October 10, 2014 at 04:04:40 PM, Jeroen Hofstee wrote:
> >>>> Hello Wolfgang,
> >>>>
> >>>> On 10-10-14 14:22, Wolfgang Denk wrote:
> >>>>>> 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
> >>> 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.

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

> > 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.

> > 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.

> > 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."

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).

> Regards,
> Jeroen

Amicalement,
-- 
Albert.

[U-Boot] Discussion topics / issues

[Wolfgang Denk]

Dear Jeroen,

In message <5437E778.3050306@myspectrum.nl> 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

-- 
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

[U-Boot] Discussion topics / issues

[Wolfgang Denk]

Dear Jeroen,

In message <54384450.3000204@myspectrum.nl> 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.

> 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?


> 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

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

versus

B:
	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

-- 
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

[U-Boot] Discussion topics / issues

[Wolfgang Denk]

Dear Jeroen,

In message <20141011150346.150C038352A@gemini.denx.de> 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

-- 
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

[U-Boot] [SoCFPGA] next steps

[Masahiro YAMADA]

Hi Marek,


2014-10-07 21:45 GMT+09:00 Marek Vasut <marex@denx.de>:
> 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.



-- 
Best Regards
Masahiro Yamada

[U-Boot] Discussion topics / issues

[Jeroen Hofstee]

Hello Wolfgang / Albert / others,

On 10-10-14 16:04, Jeroen Hofstee wrote:
> Hello Wolfgang,
>
> On 10-10-14 14:22, Wolfgang Denk wrote:
>>> 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.

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

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

[Pavel Machek]

Hi!

> 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 <pavel@denx.de>

--- a/include/common.h
+++ b/include/common.h
@@ -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);




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

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

[Pavel Machek]

On Wed 2014-10-15 10:40:12, Pavel Machek wrote:
> Hi!
> 
> > 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?

Next step is probably

diff --git a/include/common.h b/include/common.h
index d5020c8..95b2377 100644
--- a/include/common.h
+++ b/include/common.h
@@ -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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [SoCFPGA] next steps

[Marek Vasut]

On Saturday, October 11, 2014 at 08:22:29 PM, Masahiro YAMADA wrote:
> Hi Marek,

Hello!

> 2014-10-07 21:45 GMT+09:00 Marek Vasut <marex@denx.de>:
> > 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.

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

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

[Tom Rini]

On Wed, Oct 15, 2014 at 10:40:12AM +0200, Pavel Machek wrote:
> Hi!
> 
> > 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?

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.

-- 
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>