On 17 July 2017 at 05:53, Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
> On 16 July 2017 at 13:21, Ian Molton <ian@xxxxxxxxxxxxxx> wrote:
>> I've been doing some cleanups in the broadcome brcmfmac driver, trying to
>> understand how it works.
>>
>> I think this makes the driver a *lot* more readable.
>>
>> Of note:
>>
>> * Gets rid of the arbitrary and completely 1:1 mapping of
>> struct brcmf_{core,chip}_priv/struct brcmf_{core,chip} structures
>> which add unreadability whilst offering no real seperation.
>>
>> * The patch titled HACK - stabilise the value of ->sbwad in use
>> highlights an issue that is either bizarre undocumented behaviour or a
>> genuine bug - without datasheets, I dont know. Essentially the value of sbwad
>> is taken as the base address in several functions, even though it is subject
>> to change should other IO occur that moves the window offset
>>
>> Obviously this is a first cut at this and needs substantial cleanup itself,
>> however I wanted to get some idea of wether I should continue working on this.
>
> I looked at 4 random patches and none got any description. Not to
> mention their chaotic subjects. In this state I can't even review it.
> If you want to have some change accepted, you've to convince us it's
> needed. Work on cleaning your patches and resend them. You also need
> to signed off your changes.
>
> --
> Rafał
As someone who is interested in any bug fixes to this driver (Device
is used on Raspberry Pi3/0W and we have a number of issues reported
which we are actively investigating), it would be very useful to more
clearly split out any actual fixes vs simply tidying up (Yes, I agree
the driver is mostly incomprehensible). Perhaps asking the
list/maintainers for comments on any located issues/bugs fixes would
be a useful starting point, along with ensuring the description gives
a good explanation of what the suspect issue is.
James
