On Fri, Feb 28, 2025 at 05:35:52PM -0800, Shannon Nelson wrote:
> +static int pdsfc_identify(struct pdsfc_dev *pdsfc)
> +{
> + struct device *dev = &pdsfc->fwctl.dev;
> + union pds_core_adminq_comp comp = {0};
> + union pds_core_adminq_cmd cmd;
> + struct pds_fwctl_ident *ident;
> + dma_addr_t ident_pa;
> + int err = 0;
> +
> + ident = dma_alloc_coherent(dev->parent, sizeof(*ident), &ident_pa, GFP_KERNEL);
> + err = dma_mapping_error(dev->parent, ident_pa);
> + if (err) {
> + dev_err(dev, "Failed to map ident buffer\n");
> + return err;
> + }
> +
> + cmd = (union pds_core_adminq_cmd) {
> + .fwctl_ident = {
> + .opcode = PDS_FWCTL_CMD_IDENT,
> + .version = 0,
> + .len = cpu_to_le32(sizeof(*ident)),
> + .ident_pa = cpu_to_le64(ident_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 err: %d\n",
> + cmd.fwctl_ident.opcode, err);
> + else
> + pdsfc->ident = *ident;
> +
> + dma_free_coherent(dev->parent, sizeof(*ident), ident, ident_pa);
> +
> + return 0;
Is it intential to loose the pds_client_adminq_cmd err? Maybe needs a
comment if so
> +/**
> + * struct pds_fwctl_cmd - Firmware control command structure
> + * @opcode: Opcode
> + * @rsvd: Word boundary padding
> + * @ep: Endpoint identifier.
> + * @op: Operation identifier.
> + */
> +struct pds_fwctl_cmd {
> + u8 opcode;
> + u8 rsvd[3];
> + __le32 ep;
> + __le32 op;
> +} __packed;
What's your plan for the scope indication? Right now this would be
restricted to the most restricted FWCTL_RPC_CONFIGURATION scope in FW.
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* Copyright(c) Advanced Micro Devices, Inc */
> +
> +/*
> + * fwctl interface info for pds_fwctl
> + */
> +
> +#ifndef _UAPI_FWCTL_PDS_H_
> +#define _UAPI_FWCTL_PDS_H_
> +
> +#include <linux/types.h>
> +
> +/*
> + * struct fwctl_info_pds
> + *
> + * Return basic information about the FW interface available.
> + */
> +struct fwctl_info_pds {
> + __u32 uid;
I think Jonathon remarked, the uid should go since it isn't used.
Jason
