[U-Boot] [SoCFPGA] next steps

[Jagan Teki]

On 9 October 2014 19:12, Marek Vasut <marex at 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 at 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.