Re: [PATCH 4/5] clk: zynqmp: cleanup sizes of firmware responses

[Stephen Boyd <sboyd@kernel.org>]

Quoting Michael Tretter (2019-03-12 04:00:15)
> @@ -248,7 +251,8 @@ static int zynqmp_pm_clock_get_topology(u32 clock_id, u32 index, u32 *topology)
>         qdata.arg2 = index;
>  
>         ret = eemi_ops->query_data(qdata, ret_payload);
> -       memcpy(topology, &ret_payload[1], CLK_GET_TOPOLOGY_RESP_WORDS * 4);
> +       memcpy(topology, &ret_payload[1],
> +              CLK_GET_TOPOLOGY_RESP_WORDS * sizeof(*topology));

It would be even clearer to have a set of structs that encoded what the
responses looked like, instead of having to unmarshal the response
somewhat manually on top of a u32 buffer.

So something like

	struct topology_resp {
		u32 node[3];
	};

and then just use sizeof(*topology_resp). Also, it's odd that we make a
stack local variable for the response in this function and then copy it
onto the stack again for the caller of this function
(zynqmp_clock_get_topology()) and then pass it to the unmarshal function
that copies it the third time into the struct clk_topology. It would be
better to do the copy at most once into some structure that has the data
all packed together and then use the macros to access the fields
wherever the response is used.