Re: [PATCH] brcmfmac: use direct data pointer in NVRAM parser struct

[Arend van Spriel <arend@broadcom.com>]

On 05/29/15 10:03, Arend van Spriel wrote:
> On 05/29/15 07:20, Rafał Miłecki wrote:
>> On 28 May 2015 at 23:24, Arend van Spriel<arend@broadcom.com> wrote:
>>> On 05/28/15 14:34, Rafał Miłecki wrote:
>>>>
>>>> On 28 May 2015 at 13:54, Arend van Spriel<arend@broadcom.com> wrote:
>>>>>
>>>>> On 05/28/15 13:37, Rafał Miłecki wrote:
>>>>>>
>>>>>>
>>>>>> As we plan to add support for platform NVRAM we should store direct
>>>>>> data pointer without the extra struct firmware layer. This will allow
>>>>>> us to support other sources with the only requirement being u8
>>>>>> buffer.
>>>>>>
>>>>>> Signed-off-by: Rafał Miłecki<zajec5@gmail.com>
>>>>>> Signed-off-by: Hante Meuleman<meuleman@broadcom.com>
>>>>>> Signed-off-by: Arend van Spriel<arend@broadcom.com>
>>>>>> ---
>>>>>> Tested on router with BCM43602-s using
>>>>>> /lib/firmware/brcm/brcmfmac43602-pcie.txt
>>>>>>
>>>>>> I've written this patch from scratch, it's inspired by the dropped:
>>>>>> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.
>>>>>
>>>>>
>>>>>
>>>>> Hi Rafał,
>>>>>
>>>>> So what is your goal here. The inspirational patch was dropped so
>>>>> it can
>>>>> be
>>>>> resubmitted when the mips change it relies on has made its way
>>>>> upstream.
>>>>> So
>>>>> I have to rebase the patch over here and your patch will just give me
>>>>> conflicts during that rebase. So can we please wait or do you need
>>>>> this
>>>>> change right now.
>>>>
>>>>
>>>> The dropped patch will require rebasing/rewriting anyway. There were
>>>> few changes to firmware.c already, I've few more planned, you'll have
>>>> to drop some code form your patch (parts that will go into MIPS tree)
>>>> and probably apply few changes as requested in comments.
>>>
>>>
>>> I already did that and submitted the mips part to Ralf. I am sorry to
>>> say
>>> this but what annoys me is that since then you started submitting
>>> patches
>>> that seem to be taken from the dropped patch. So I have a problem
>>> seeing the
>>> bright side. If Ralf takes the mips part it ends up in linux-next and
>>> we can
>>> submit the brcmfmac part.
>>
>> This is truly the first patch based on your dropped one. All other 5
>> patches were addressing problems I noticed by myself. One of them
>> fixed the same issue you /silently/ did in the dropped one, but I
>> wasn't even aware of that until I started rebasing your patch. We
>> simply noticed the same problem and fixed it on our owns.
>>
>> So I think this patch is the only one that could annoy you, but
>> *honestly* my intention was exactly the opposite. As already said, I
>> just wanted to do possible cleanup early and let you maintain 50% of
>> your original patch instead the whole one.
>>
>> I really don't want to have you annoyed because the need of rebasing
>> your patch. What you said about Ralf's tree and linux-next isn't
>> exactly true. I do believe Kalle won't merge linux-next into
>> wireless-driver-next, so we have to wait for 4.2-rc1, then for Dave
>> merging Linus's tree, then Kalle merging Dave's tree.
>
> Well, it is an integration issue. So yes, wireless-drivers-next (and
> net-next) would not build for CONFIG_BCM47XX with the brcmfmac patch,
> because of the missing mips patch. However, the linux-next tree will
> have no build issue and consequently 4.2-rc1 will have no build issue. I
> think this is acceptable although I would like to hear the opinion of
> Kalle on this.

Hi Kalle,

Do you have an opinion on this? mips-next and linux-next now have the 
function brcmfmac needs for CONFIG_BCM47XX so I could submit brcmfmac 
patch, but that means wireless-drivers-next (and net-next) will not 
build for CONFIG_BCM47XX until after the 4.2 merge window.

Regards,
Arend

> Regards,
> Arend
>
>> We really
>> shouldn't stop development for weeks because of having some patch
>> prepared for later submitting.
>> I think the only think you can do to make your life easier is to
>> submit cleanup part of your prepared patch. This is exactly what I
>> tried to do for you.
>>
>