Re: [PATCH 02/11] firmware: arm_scmi: hide protocols' private data

[Florian Fainelli]

On 10/14/20 8:05 AM, Cristian Marussi wrote:
> Protocols private data were meant to be used exclusively by protocol code
> but they are currently exposed through the handle, so available also to
> SCMI drivers: move them away from handle into instance specific data and
> provide internal helpers to let protocols implementation set/get their own
> private data from protocol code.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>

[snip]


> diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
> index 94bcad9a7d19..4e8dafc36d7e 100644
> --- a/drivers/firmware/arm_scmi/clock.c
> +++ b/drivers/firmware/arm_scmi/clock.c
> @@ -240,7 +240,8 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id,
>  	u32 flags = 0;
>  	struct scmi_xfer *t;
>  	struct scmi_clock_set_rate *cfg;
> -	struct clock_info *ci = handle->clk_priv;
> +	struct clock_info *ci =
> +		scmi_get_proto_priv(handle, SCMI_PROTOCOL_CLOCK);

Having to repeat the protocol identifier is a bit error prone, does not
scale really and it will not change throughout the lifecycle of the
protocol handler. This is the least path of resistance though so it
makes sense you went that route for now.

Instead of exposing scmi_handle towards the various operations what you
could do is just pass down an encapsulating object which is comprised of
a scmi_handle reference and the protocol's private structure already
resolved?
-- 
Florian