Re: [PATCH v2 1/2] drm/xe/gsc: Initialize GSC proxy

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

On Tue, 2024-01-09 at 16:44 -0800, Daniele Ceraolo Spurio wrote:
> The GSC uC needs to communicate with the CSME to perform certain
> operations. Since the GSC can't perform this communication directly on

alan:snip


> +static int proxy_query(struct xe_gsc *gsc)
> +{
> +	struct xe_gt *gt = gsc_to_gt(gsc);
> +	struct xe_device *xe = gt_to_xe(gt);
> +	struct xe_gsc_proxy_header *to_csme_hdr = gsc->proxy.to_csme;
> +	void *to_csme_payload = gsc->proxy.to_csme + PROXY_HDR_SIZE;
> +	u32 wr_offset;
> +	u32 reply_offset;
> +	u32 size;
> +	int ret;
> +
> +	wr_offset = xe_gsc_emit_header(xe, &gsc->proxy.to_gsc, 0,
> +				       HECI_MEADDRESS_PROXY, 0, PROXY_HDR_SIZE);
> +	wr_offset = emit_proxy_header(xe, &gsc->proxy.to_gsc, wr_offset);
> +
> +	size = wr_offset;
> +
> +	while (1) {
> +		/*
> +		 * Poison the GSC response header space to make sure we don't
> +		 * read a stale reply.
> +		 */
> +		xe_gsc_poison_header(xe, &gsc->proxy.from_gsc, 0);
> +
> +		/* send proxy message to GSC */
> +		ret = proxy_send_to_gsc(gsc, size);
> +		if (ret)
> +			goto proxy_error;
> +
> +		/* check the reply from GSC */
> +		ret = xe_gsc_read_out_header(xe, &gsc->proxy.from_gsc, 0,
> +					     PROXY_HDR_SIZE, &reply_offset);
> +		if (ret) {
> +			xe_gt_err(gt, "Invalid gsc header in proxy reply (%pe)\n",
> +				  ERR_PTR(ret));
> +			goto proxy_error;
> +		}
> +
> +		/* copy the proxy header reply from GSC */
> +		xe_map_memcpy_from(xe, to_csme_hdr, &gsc->proxy.from_gsc,
> +				   reply_offset, PROXY_HDR_SIZE);
> +
> +		/* stop if this was the last message */
> +		if (FIELD_GET(GSC_PROXY_TYPE, to_csme_hdr->hdr) == GSC_PROXY_MSG_TYPE_PROXY_END)
> +			break;
alan:snip

> proxy_error:
> +	return ret < 0 ? ret : 0;
> +}

alan: nit: with the fixing of the ret for the case where ret < expected header size,
i dont see anymore cases where ret can be >0 and not an error within the loops points
of "goto proxy_error" or the break. In which case, we can simply remove "proxy_error:"
and return ret at the end and in all prior cases, where there is a "goto proxy_error"
replace with a direct "return ret". 

But this is trivial, so no point holding off:
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>