Re: [PATCH v2] ArmPkg/ArmScmiDxe: Add clock enable function

[Jeff Brasen <jbrasen@nvidia.com>]

-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@linaro.org> 
Sent: Thursday, December 6, 2018 9:54 AM
To: Jeff Brasen <jbrasen@nvidia.com>
Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Girish Pathak <girish.pathak@arm.com>
Subject: Re: [PATCH v2] ArmPkg/ArmScmiDxe: Add clock enable function

On Thu, 6 Dec 2018 at 01:37, Jeff Brasen <jbrasen@nvidia.com> wrote:
>
> Leif/Ard,
>
>
>   Any comments on this v2 patch for this?
>
>

Hi Jeff,

I'm not sure what level of bikeshedding is justified when it comes to a driver such as this one, which is very recent, and mostly for platform internal use. However, I will note that the current versioning approach permits a *client* of the old SCMI_CLOCK_PROTOCOL to be built that invokes ->Enable(), which is not defined for it. This somewhat defeats the purpose of the versioning, since the whole point is to avoid invoking ->Enable() on older implementations of the protocol.

I'd be fine with just modifying the protocol, but if we decide we need versioning, we should not modify the public interface of the old one.
How the driver reuses one implementation to back the other is another matter, of course.
[JMB] I can either just change without versioning (that was my original approach but I also changed the guid which would primarily catch new clients running on old platforms from calling an undefined function), I am fine with either that (with maybe a switch back to original guid if we are not concerned about that issue) or a future update that creates a full v2 version of the protocol in the header. 

 

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------