diff mbox series

net: ipa: don't configure IDLE_INDICATION on v3.1

Message ID 20221024234850.4049778-1-caleb.connolly@linaro.org (mailing list archive)
State Accepted
Commit 95a0396a0642d3c28b6cefdc76697e0b8f594825
Delegated to: Netdev Maintainers
Headers show
Series net: ipa: don't configure IDLE_INDICATION on v3.1 | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Caleb Connolly Oct. 24, 2022, 11:48 p.m. UTC
IPA v3.1 doesn't support the IDLE_INDICATION_CFG register, this was
causing a harmless splat in ipa_idle_indication_cfg(), add a version
check to prevent trying to fetch this register on v3.1

Fixes: 6a244b75cfab ("net: ipa: introduce ipa_reg()")
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
This will need to wait for Jami's Tested-by as I don't have any v3.1 hardware.
---
 drivers/net/ipa/ipa_main.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Alex Elder Oct. 25, 2022, 12:13 a.m. UTC | #1
On 10/24/22 6:48 PM, Caleb Connolly wrote:
> IPA v3.1 doesn't support the IDLE_INDICATION_CFG register, this was
> causing a harmless splat in ipa_idle_indication_cfg(), add a version
> check to prevent trying to fetch this register on v3.1
> 
> Fixes: 6a244b75cfab ("net: ipa: introduce ipa_reg()")

Actually, the bug first appeared long ago.  This commit:
   1bb1a117878b9 ("net: ipa: add IPA v3.1 configuration data")
marked IPA v3.1 supported.  But it did not update the code
to avoid accessing the IDLE_INDICATION register for IPA v3.1
(in ipa_reg_idle_indication_cfg_offset()).  That being said,
we have no evidence that it caused harm, and until we do I'd
rather not try to fix the problem that far back.

The commit you point out is the one where we actually
start checking (and WARNing), and I think it's reasonable
to say that's what this fixes.

> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> This will need to wait for Jami's Tested-by as I don't have any v3.1 hardware.

I would very much like to get a Tested-by tag.

But from my perspective, this patch looks good.  Previously
offset 0x220 was used, even though it was not well-defined
for IPA v3.1.

This is a bug fix destined for the net/master branch.

Reviewed-by: Alex Elder <elder@linaro.org>

Thank you.

> ---
>   drivers/net/ipa/ipa_main.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
> index 3461ad3029ab..49537fccf6ad 100644
> --- a/drivers/net/ipa/ipa_main.c
> +++ b/drivers/net/ipa/ipa_main.c
> @@ -434,6 +434,9 @@ static void ipa_idle_indication_cfg(struct ipa *ipa,
>   	const struct ipa_reg *reg;
>   	u32 val;
>   
> +	if (ipa->version < IPA_VERSION_3_5_1)
> +		return;
> +
>   	reg = ipa_reg(ipa, IDLE_INDICATION_CFG);
>   	val = ipa_reg_encode(reg, ENTER_IDLE_DEBOUNCE_THRESH,
>   			     enter_idle_debounce_thresh);
Jami Kettunen Oct. 25, 2022, 8:01 p.m. UTC | #2
On 25.10.2022 02:48, Caleb Connolly wrote:
> IPA v3.1 doesn't support the IDLE_INDICATION_CFG register, this was
> causing a harmless splat in ipa_idle_indication_cfg(), add a version
> check to prevent trying to fetch this register on v3.1
> 
> Fixes: 6a244b75cfab ("net: ipa: introduce ipa_reg()")
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>

Tested-by: Jami Kettunen <jami.kettunen@somainline.org>

> ---
> This will need to wait for Jami's Tested-by as I don't have any v3.1 
> hardware.
> ---
>  drivers/net/ipa/ipa_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
> index 3461ad3029ab..49537fccf6ad 100644
> --- a/drivers/net/ipa/ipa_main.c
> +++ b/drivers/net/ipa/ipa_main.c
> @@ -434,6 +434,9 @@ static void ipa_idle_indication_cfg(struct ipa 
> *ipa,
>  	const struct ipa_reg *reg;
>  	u32 val;
> 
> +	if (ipa->version < IPA_VERSION_3_5_1)
> +		return;
> +
>  	reg = ipa_reg(ipa, IDLE_INDICATION_CFG);
>  	val = ipa_reg_encode(reg, ENTER_IDLE_DEBOUNCE_THRESH,
>  			     enter_idle_debounce_thresh);
patchwork-bot+netdevbpf@kernel.org Oct. 26, 2022, 3 a.m. UTC | #3
Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 25 Oct 2022 00:48:50 +0100 you wrote:
> IPA v3.1 doesn't support the IDLE_INDICATION_CFG register, this was
> causing a harmless splat in ipa_idle_indication_cfg(), add a version
> check to prevent trying to fetch this register on v3.1
> 
> Fixes: 6a244b75cfab ("net: ipa: introduce ipa_reg()")
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> 
> [...]

Here is the summary with links:
  - net: ipa: don't configure IDLE_INDICATION on v3.1
    https://git.kernel.org/netdev/net/c/95a0396a0642

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
index 3461ad3029ab..49537fccf6ad 100644
--- a/drivers/net/ipa/ipa_main.c
+++ b/drivers/net/ipa/ipa_main.c
@@ -434,6 +434,9 @@  static void ipa_idle_indication_cfg(struct ipa *ipa,
 	const struct ipa_reg *reg;
 	u32 val;
 
+	if (ipa->version < IPA_VERSION_3_5_1)
+		return;
+
 	reg = ipa_reg(ipa, IDLE_INDICATION_CFG);
 	val = ipa_reg_encode(reg, ENTER_IDLE_DEBOUNCE_THRESH,
 			     enter_idle_debounce_thresh);