mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alex Elder <elder@ieee.org>
To: Luca Weiss <luca.weiss@fairphone.com>,
	Alex Elder <elder@kernel.org>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: ~postmarketos/upstreaming@lists.sr.ht,
	phone-devel@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: ipa: add IPA v5.1 and v5.5 to ipa_version_string()
Date: Mon, 28 Jul 2025 10:53:49 -0500	[thread overview]
Message-ID: <e07f0407-84e1-4efa-868d-5853b7e9ab4e@ieee.org> (raw)
In-Reply-To: <20250728-ipa-5-1-5-5-version_string-v1-1-d7a5623d7ece@fairphone.com>

On 7/28/25 3:35 AM, Luca Weiss wrote:
> Handle the case for v5.1 and v5.5 instead of returning "0.0".

This makes sense for IPA v5.5.

I have comments below, but I'll do this up front:

Reviewed-by: Alex Elder <elder@riscstar.com>

> Also reword the comment below since I don't see any evidence of such a
> check happening, and - since 5.5 has been missing - can happen.

You are correct.  Commit dfdd70e24e388 ("net: ipa: kill
ipa_version_supported()") removed the test that guaranteed
that the version would be good.  So your comment update
should have done then.

> Fixes: 3aac8ec1c028 ("net: ipa: add some new IPA versions")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>   drivers/net/ipa/ipa_sysfs.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ipa/ipa_sysfs.c b/drivers/net/ipa/ipa_sysfs.c
> index a59bd215494c9b7cbdd1f000d9f23e100c18ee1e..a53e9e6f6cdf50103e94e496fd06b55636ba02f4 100644
> --- a/drivers/net/ipa/ipa_sysfs.c
> +++ b/drivers/net/ipa/ipa_sysfs.c
> @@ -37,8 +37,12 @@ static const char *ipa_version_string(struct ipa *ipa)
>   		return "4.11";
>   	case IPA_VERSION_5_0:
>   		return "5.0";
> +	case IPA_VERSION_5_1:
> +		return "5.1";

IPA v5.1 is not actually supported yet, and this won't be
used until it is.  Only those platforms with compatible
strings defined in the ipa_match array in "ipa_main.c" will
probe successfully.

That said...  I guess it's OK to define this at the same time
things are added to enum ipa_version.  There are still too
many little things like this that need to be updated when a
new version is supported.

Thanks for the patch.

					-Alex

> +	case IPA_VERSION_5_5:
> +		return "5.5";
>   	default:
> -		return "0.0";	/* Won't happen (checked at probe time) */
> +		return "0.0";	/* Should not happen */
>   	}
>   }
>   
> 
> ---
> base-commit: 038d61fd642278bab63ee8ef722c50d10ab01e8f
> change-id: 20250728-ipa-5-1-5-5-version_string-a557dc8bd91a
> 
> Best regards,


  parent reply	other threads:[~2025-07-28 15:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-28  8:35 Luca Weiss
2025-07-28 14:11 ` Dawid Osuchowski
2025-07-28 15:53 ` Alex Elder [this message]
2025-07-31 10:31   ` Luca Weiss
2025-07-31  1:20 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e07f0407-84e1-4efa-868d-5853b7e9ab4e@ieee.org \
    --to=elder@ieee.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=elder@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luca.weiss@fairphone.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=phone-devel@vger.kernel.org \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®