Re: [PATCH v2 net 0/5] net: ipa: minor bug fixes

From: Alex Elder
Date: Sat Oct 31 2020 - 08:57:43 EST


On 10/30/20 7:23 PM, Jakub Kicinski wrote:
> On Thu, 29 Oct 2020 11:50:52 -0500 Alex Elder wrote:
>> On 10/29/20 11:11 AM, Jakub Kicinski wrote:
>>> On Wed, 28 Oct 2020 14:41:43 -0500 Alex Elder wrote:
>>>> This series fixes several bugs. They are minor, in that the code
>>>> currently works on supported platforms even without these patches
>>>> applied, but they're bugs nevertheless and should be fixed.
>>>
>>> By which you mean "it seems to work just fine most of the time" or "the
>>> current code does not exercise this paths/functionally these bugs don't
>>> matter for current platforms".
>>
>> The latter, although for patch 3 I'm not 100% sure.
>>
>> Case by case:
>> Patch 1:
>> It works. I inquired what the consequence of passing this
>> wrong buffer pointer was, and for the way we are using IPA
>> it seems it's fine--the memory pointer we were assigning is
>> not used, so it's OK. But we're assigning the wrong pointer.
>> Patch 2:
>> It works. Even though the bit field is 1 bit wide (not two)
>> we never actually write a value greater than 1, so we don't
>> cause a problem. But the definition is incorrect.
>> Patch 3:
>> It works, but on the SDM845 we should be assigning the endpoints
>> to use resource group 1 (they are 0 by default). The way we
>> currently use this upstream we don't have other endpoints
>> competing for resources, so I think this is fine. SC7180 we
>> will assign endpoints to resource group 0, which is the default.
>> Patch 4:
>> It works. This is like patch 2; we define the number of these
>> things incorrectly, but the way we currently use them we never
>> exceed the limit in a broken way.
>> Patch 5:
>> It works. The maximum number of supported groups is even,
>> and if a (smaller) odd number are used the remainder are
>> programmed with 0, which is appropriate for undefined
>> fields.
>>
>> If you have any concerns about back-porting these fixes I
>> think I'm comfortable posting them for net-next instead.
>> I debated that before sending them out. Please request that
>> if it's what you think would be best.
>
> Looks like these patches apply cleanly to net-next, so I put them there.
>
> Thanks!

Works for me. Thank you. -Alex

>