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

Sudeep Holla sudeep.holla at arm.com
Wed Sep 27 05:51:28 PDT 2023


On Wed, Sep 27, 2023 at 01:43:17PM +0100, Cristian Marussi wrote:
> On Wed, Sep 27, 2023 at 12:50:23PM +0100, Sudeep Holla wrote:
> > 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,
> 
> > > 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 think using a progressive continuos increment plus a comment should be
> fine indeed (like this series does): this is anyway a special case, there
> are anyway already many other places in the stack where we have so small
> changes from one version to another that can be handled within the same
> function version just with an if-check
>

Yes that would be ideal. Only when memory layout format changes we need
these structure versioning.

> Reviewed-by: Cristian Marussi <cristian.marussi at arm.com>
>

Thanks!

-- 
Regards,
Sudeep



More information about the linux-arm-kernel mailing list