2017-10-12 05:53:51

by Ganapathi Bhat

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH] mwifiex: Random MAC address during scanning

Hi Kalle,
>
> > ----------------------------------------------------------------------
> > On Fri, Sep 29, 2017 at 04:23:10PM +0530, Ganapathi Bhat wrote:
> > > From: Karthik Ananthapadmanabha <[email protected]>
> > >
> > > Driver will advertise RANDOM_MAC support only if the device supports
> > > this feature.
> > >
> > > Signed-off-by: Karthik Ananthapadmanabha <[email protected]>
> > > Signed-off-by: Ganapathi Bhat <[email protected]>
> >
> > I'd just like to point out that this is a very bad commit subject:
> >
> > "[PATCH] mwifiex: Random MAC address during scanning"
> >
> > It's borderline wrong, really. "Random MAC address during scanning" is
> > already supported. This patch is just adding a feature-flag check for
> > it, since some firmwares in the wild don't support it. A more accurate
> > description would be something like:
> >
> > "[PATCH] mwifiex: Add feature flag support for MAC randomization"
> >
> > The patch is already applied, so I'd only worry about it for future
> > submissions (no need to resend).
>
> I'm Really Sorry. I will take care of this in future.
>
The firmware which advertises this capability flag is yet to be shared in upstream. The latest available firmware does not contain this flag.
So, is it possible to revert this change, so that we will resubmit this (with proper subject) once our firmware is uploaded.
> >
> > Brian

Regards,
Ganapathi


2017-10-12 07:59:47

by Kalle Valo

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH] mwifiex: Random MAC address during scanning

Ganapathi Bhat <[email protected]> writes:

> Hi Kalle,
>>
>> > ----------------------------------------------------------------------
>> > On Fri, Sep 29, 2017 at 04:23:10PM +0530, Ganapathi Bhat wrote:
>> > > From: Karthik Ananthapadmanabha <[email protected]>
>> > >
>> > > Driver will advertise RANDOM_MAC support only if the device supports
>> > > this feature.
>> > >
>> > > Signed-off-by: Karthik Ananthapadmanabha <[email protected]>
>> > > Signed-off-by: Ganapathi Bhat <[email protected]>
>> >
>> > I'd just like to point out that this is a very bad commit subject:
>> >
>> > "[PATCH] mwifiex: Random MAC address during scanning"
>> >
>> > It's borderline wrong, really. "Random MAC address during scanning" is
>> > already supported. This patch is just adding a feature-flag check for
>> > it, since some firmwares in the wild don't support it. A more accurate
>> > description would be something like:
>> >
>> > "[PATCH] mwifiex: Add feature flag support for MAC randomization"
>> >
>> > The patch is already applied, so I'd only worry about it for future
>> > submissions (no need to resend).

Brian, thanks for pointing out that. I had missed it.

>> I'm Really Sorry. I will take care of this in future.
>>
> The firmware which advertises this capability flag is yet to be shared
> in upstream. The latest available firmware does not contain this flag.
> So, is it possible to revert this change, so that we will resubmit
> this (with proper subject) once our firmware is uploaded.

Like Brian said, I don't think there's any point reverting it and
resending the patch. Just something to learn for the future, for both of
us. (I need to also check the titles more carefully.)

--
Kalle Valo