On 08.06.22 18:40, Cristian Marussi wrote:
> Avoid to invoke SENSOR_AXIS_NAME_GET on sensors that have not declared at
> least one of their axes as supporting extended names.
>
> Since the returned list of axes supporting extended names is not
> necessarily comprising all the existing axes of the specified sensor,
> take care also to properly pick the ax descriptor from the id embedded
> in the reply.
>
> Cc: Peter Hilber <[email protected]>
> Cc: Sudeep Holla <[email protected]>
> Fixes: 802b0bed011e ("firmware: arm_scmi: Add SCMI v3.1 SENSOR_AXIS_NAME_GET support")
> Signed-off-by: Cristian Marussi <[email protected]>
Reviewed-by: Peter Hilber <[email protected]>
> ---
> V1 --> v2
> - fixed missing endianity conversion
> ---
> drivers/firmware/arm_scmi/sensors.c | 56 +++++++++++++++++++++++------
> 1 file changed, 46 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
> index 75b9d716508e..8a93dd944c49 100644
> --- a/drivers/firmware/arm_scmi/sensors.c
> +++ b/drivers/firmware/arm_scmi/sensors.c
> @@ -358,15 +358,20 @@ static int scmi_sensor_update_intervals(const struct scmi_protocol_handle *ph,
> return ph->hops->iter_response_run(iter);
> }
>
> +struct scmi_apriv {
> + bool any_axes_support_extended_names;
> + struct scmi_sensor_info *s;
> +};
> +
> static void iter_axes_desc_prepare_message(void *message,
> const unsigned int desc_index,
> const void *priv)
> {
> struct scmi_msg_sensor_axis_description_get *msg = message;
> - const struct scmi_sensor_info *s = priv;
> + const struct scmi_apriv *apriv = priv;
>
> /* Set the number of sensors to be skipped/already read */
> - msg->id = cpu_to_le32(s->id);
> + msg->id = cpu_to_le32(apriv->s->id);
> msg->axis_desc_index = cpu_to_le32(desc_index);
> }
>
> @@ -393,12 +398,14 @@ iter_axes_desc_process_response(const struct scmi_protocol_handle *ph,
> u32 attrh, attrl;
> struct scmi_sensor_axis_info *a;
> size_t dsize = SCMI_MSG_RESP_AXIS_DESCR_BASE_SZ;
> - struct scmi_sensor_info *s = priv;
> + struct scmi_apriv *apriv = priv;
> const struct scmi_axis_descriptor *adesc = st->priv;
>
> attrl = le32_to_cpu(adesc->attributes_low);
> + if (SUPPORTS_EXTENDED_AXIS_NAMES(attrl))
> + apriv->any_axes_support_extended_names = true;
>
> - a = &s->axis[st->desc_index + st->loop_idx];
> + a = &apriv->s->axis[st->desc_index + st->loop_idx];
> a->id = le32_to_cpu(adesc->id);
> a->extended_attrs = SUPPORTS_EXTEND_ATTRS(attrl);
>
> @@ -444,10 +451,19 @@ iter_axes_extended_name_process_response(const struct scmi_protocol_handle *ph,
> void *priv)
> {
> struct scmi_sensor_axis_info *a;
> - const struct scmi_sensor_info *s = priv;
> + const struct scmi_apriv *apriv = priv;
> struct scmi_sensor_axis_name_descriptor *adesc = st->priv;
> + u32 axis_id = le32_to_cpu(adesc->axis_id);
> +
> + if (axis_id >= st->max_resources)
> + return -EPROTO;
>
> - a = &s->axis[st->desc_index + st->loop_idx];
> + /*
> + * Pick the corresponding descriptor based on the axis_id embedded
> + * in the reply since the list of axes supporting extended names
> + * can be a subset of all the axes.
> + */
> + a = &apriv->s->axis[axis_id];
> strscpy(a->name, adesc->name, SCMI_MAX_STR_SIZE);
> st->priv = ++adesc;
>
> @@ -458,21 +474,36 @@ static int
> scmi_sensor_axis_extended_names_get(const struct scmi_protocol_handle *ph,
> struct scmi_sensor_info *s)
> {
> + int ret;
> void *iter;
> struct scmi_iterator_ops ops = {
> .prepare_message = iter_axes_desc_prepare_message,
> .update_state = iter_axes_extended_name_update_state,
> .process_response = iter_axes_extended_name_process_response,
> };
> + struct scmi_apriv apriv = {
> + .any_axes_support_extended_names = false,
> + .s = s,
> + };
>
> iter = ph->hops->iter_response_init(ph, &ops, s->num_axis,
> SENSOR_AXIS_NAME_GET,
> sizeof(struct scmi_msg_sensor_axis_description_get),
> - s);
> + &apriv);
> if (IS_ERR(iter))
> return PTR_ERR(iter);
>
> - return ph->hops->iter_response_run(iter);
> + /*
> + * Do not cause whole protocol initialization failure when failing to
> + * get extended names for axes.
> + */
> + ret = ph->hops->iter_response_run(iter);
> + if (ret)
> + dev_warn(ph->dev,
> + "Failed to get axes extended names for %s (ret:%d).\n",
> + s->name, ret);
> +
> + return 0;
> }
>
> static int scmi_sensor_axis_description(const struct scmi_protocol_handle *ph,
> @@ -486,6 +517,10 @@ static int scmi_sensor_axis_description(const struct scmi_protocol_handle *ph,
> .update_state = iter_axes_desc_update_state,
> .process_response = iter_axes_desc_process_response,
> };
> + struct scmi_apriv apriv = {
> + .any_axes_support_extended_names = false,
> + .s = s,
> + };
>
> s->axis = devm_kcalloc(ph->dev, s->num_axis,
> sizeof(*s->axis), GFP_KERNEL);
> @@ -495,7 +530,7 @@ static int scmi_sensor_axis_description(const struct scmi_protocol_handle *ph,
> iter = ph->hops->iter_response_init(ph, &ops, s->num_axis,
> SENSOR_AXIS_DESCRIPTION_GET,
> sizeof(struct scmi_msg_sensor_axis_description_get),
> - s);
> + &apriv);
> if (IS_ERR(iter))
> return PTR_ERR(iter);
>
> @@ -503,7 +538,8 @@ static int scmi_sensor_axis_description(const struct scmi_protocol_handle *ph,
> if (ret)
> return ret;
>
> - if (PROTOCOL_REV_MAJOR(version) >= 0x3)
> + if (PROTOCOL_REV_MAJOR(version) >= 0x3 &&
> + apriv.any_axes_support_extended_names)
> ret = scmi_sensor_axis_extended_names_get(ph, s);
>
> return ret;