Re: [PATCH v4 5/6] pds_fwctl: add rpc and query support

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Wed, Mar 19, 2025 at 02:32:36PM -0700, Shannon Nelson wrote:
> +static struct pds_fwctl_query_data *pdsfc_get_endpoints(struct pdsfc_dev *pdsfc,
> +							dma_addr_t *pa)
> +{
> +	struct device *dev = &pdsfc->fwctl.dev;
> +	union pds_core_adminq_comp comp = {0};
> +	struct pds_fwctl_query_data *data;
> +	union pds_core_adminq_cmd cmd;
> +	dma_addr_t data_pa;
> +	int err;
> +
> +	data = dma_alloc_coherent(dev->parent, PAGE_SIZE, &data_pa, GFP_KERNEL);
> +	err = dma_mapping_error(dev, data_pa);
> +	if (err) {
> +		dev_err(dev, "Failed to map endpoint list\n");
> +		return ERR_PTR(err);
> +	}

This doesn't work.  The dma_alloc_coherent() function doesn't necessarily
initialize data_pa.  I don't know very much about DMA but can't we just
check:

	data = dma_alloc_coherent(dev->parent, PAGE_SIZE, &data_pa, GFP_KERNEL);
	if (!data)
		return ERR_PTR(-ENOMEM);

regards,
dan carpenter

> +
> +	cmd = (union pds_core_adminq_cmd) {
> +		.fwctl_query = {
> +			.opcode = PDS_FWCTL_CMD_QUERY,
> +			.entity = PDS_FWCTL_RPC_ROOT,
> +			.version = 0,
> +			.query_data_buf_len = cpu_to_le32(PAGE_SIZE),
> +			.query_data_buf_pa = cpu_to_le64(data_pa),
> +		}
> +	};
> +
> +	err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
> +	if (err) {
> +		dev_err(dev, "Failed to send adminq cmd opcode: %u entity: %u err: %d\n",
> +			cmd.fwctl_query.opcode, cmd.fwctl_query.entity, err);
> +		dma_free_coherent(dev->parent, PAGE_SIZE, data, data_pa);
> +		return ERR_PTR(err);
> +	}
> +
> +	*pa = data_pa;
> +
> +	return data;
> +}