[U-Boot] Discussion topics / issues

Thu Oct 9 16:45:07 CEST 2014

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>