Re: [PATCH] firmware: arm_scmi: Rename scmi_{msg_,}clock_config_{get,set}_{2,21}

From: Sudeep Holla <sudeep.holla@arm.com>
To: Cristian Marussi <cristian.marussi@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	Sudeep Holla <sudeep.holla@arm.com>
Subject: Re: [PATCH] firmware: arm_scmi: Rename scmi_{msg_,}clock_config_{get,set}_{2,21}
Date: Wed, 27 Sep 2023 12:50:23 +0100	[thread overview]
Message-ID: <20230927115023.wjxvnpdruv37hid2@bogus> (raw)
In-Reply-To: <ZRQOxGmR8NQVMrwi@pluto>

On Wed, Sep 27, 2023 at 12:15:16PM +0100, Cristian Marussi wrote:
> On Mon, Sep 25, 2023 at 11:15:57AM +0100, Sudeep Holla wrote:
> > It is very confusing to use *_v2 for everything applicable until SCMI
> > clock protocol version v2.0 including v1.0 for example. So let us rename
> > such that *_v2 is used only for SCMI clock protocol v2.1 onwards. Also
> > add comment to indicate the same explicitly.
> > 
> 
> Hi Sudeep,
> 
> looking back at this, indeed, I remember being unsure if it was better
> to use the v2/v21 naming scheme or the one that this patch propose.
> 
> Revisiting this now, I have to say that I agree with you, but why you
> have also renamed _v21 to v2 ? The idea was to match the exact protocol
> version ( I see that you added a comment anyway...)
>

OK we can do that too. I was thinking of continuous increment in the structure
version independent of the spec version. Having _v21, _v53, ...etc looks odd
to me. I was thinking more like _v2(v2.1 onwards) and _v3(v5.3 onwards) for
example. Hope that clarifies and let me know if that is still confusing in
your opinion.

> IOW, the day some further new non-backward compatible features will be
> possibly introduced (say clock v3), we could go like:
> 
>  - _config_set_v21: only v2.1 (the one you have renamed to v2)
>  - _config_set_v3: only v3
>  - _config_set : everything else, i.e. up to v2.0 (as you've renamed
>    now)
>

Correct. My main worry is if things get changed at random minor version
like v2.1, v4.5, v5.3, ...etc. Unlikely to happen but not ruled out 😉,
blame spec authors.

> I have no string opinions anyway, so I am fine also with this version of
> the patch...better than the original brain-dead naming scheme that I had
> chosen indeed :P
> 

I am not saying what I did is easier to follow, but a comment with the
driver structure version must help.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel