Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Use put_unaligned_le16

[Jes Sorensen <jes.sorensen@gmail.com>]

On 02/18/15 13:08, Vaishali Thakkar wrote:
> On Wed, Feb 18, 2015 at 10:40 PM, Jes Sorensen <jes.sorensen@gmail.com> wrote:
>> On 02/18/15 05:50, Arnd Bergmann wrote:
>>> On Wednesday 18 February 2015 07:02:34 Vaishali Thakkar wrote:
>>>> On Wed, Feb 18, 2015 at 12:42 AM, Jes Sorensen <jes.sorensen@gmail.com> wrote:
>>>>> On 02/17/15 05:11, Vaishali Thakkar wrote:
>>>>> Your use of put_unaligned_le16() if good, the main issue is the above
>>>>> assignment as you mention. Since you got rid of the tim_bitmap_le =
>>>>> portion, the *dst_ie++ = is going to assigning random garbage since
>>>>> tim_bitmap_le is not set.
>>>>>
>>>>> One way to solve the problem would be to make that line say
>>>>>
>>>>>                 if (tim_ielen == 4) {
>>>>> -                       *dst_ie++ = *(u8 *)&tim_bitmap_le;
>>>>> +                       *dst_ie++ = pstapriv->tim_bitmap & 0xff;
>>>>>                 } else if (tim_ielen == 5) {
>>>>> -                       memcpy(dst_ie, &tim_bitmap_le, 2);
>>>>> +                       put_unaligned_le16(pstapriv->tim_bitmap, dst_ie);
>>>>>
>>>>> and hopefully eliminate the tim_bitmap_le variable completely.
>>>>
>>>> Yes. This makes sense. And as there is only one use of tim_bitmap_le
>>>> variable this will eliminate it completely. Should I go for v2 now, with this
>>>> change?? I have one more such case in rtl8723au driver also. I think now
>>>> I can go for it also.
>>>
>>> Yes, please submit the change for both drivers as separate patches.
>>
>> As Arnd says, please use two patches. The reason is that the two drivers
>> are completely independent and also have separate maintainers (me owning
>> rtl8723au fwiw).
> 
> Sorry I didn't get you.
> 
> Yes, I know both drivers are independent. So, I will send 2 different patches.
> Here, I sent only one patch for rtl8188eu. I didn't send any patch for
> rtl8723au.
> I am working on it and supposed to send it now.

Sorry if I wasn't clear here.

The key is to make the changes in two commits. In general you can post
them as part of a multi-commit patch set if you are fixing something in
a large number of places, or changing an API etc. For more independent
changes like these, posting them individually is preferred.

I hope this makes more sense.

Jes