Re: [PATCH v2 net 0/5] net: ipa: minor bug fixes
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 >
Re: [PATCH v2 net 0/5] net: ipa: minor bug fixes
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!
Re: [PATCH v2 net 0/5] net: ipa: minor bug fixes
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. -Alex
Re: [PATCH v2 net 0/5] net: ipa: minor bug fixes
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".
[PATCH v2 net 0/5] net: ipa: minor bug fixes
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. Version 2 improves the commit message for the fourth patch. It also fixes a bug in two spots in the last patch. Both of these changes were suggested by Willem de Bruijn. -Alex Alex Elder (5): net: ipa: assign proper packet context base net: ipa: fix resource group field mask definition net: ipa: assign endpoint to a resource group net: ipa: distinguish between resource group types net: ipa: avoid going past end of resource group array drivers/net/ipa/ipa_data-sc7180.c | 4 + drivers/net/ipa/ipa_data-sdm845.c | 4 + drivers/net/ipa/ipa_data.h| 12 +-- drivers/net/ipa/ipa_endpoint.c| 11 +++ drivers/net/ipa/ipa_main.c| 121 +++--- drivers/net/ipa/ipa_mem.c | 2 +- drivers/net/ipa/ipa_reg.h | 48 +++- 7 files changed, 136 insertions(+), 66 deletions(-) -- 2.20.1
