Applied. Thanks!
On Tue, Oct 20, 2026 at 3:37 AM Omokefe Emmanuel Onanaroghene <[email protected]> wrote: > > kernel-doc reports prototype mismatches for the cgs register accessor > typedefs, e.g.: > > drivers/gpu/drm/amd/include/cgs_common.h:89: expecting prototype > for cgs_read_register(). Prototype was for uint32_t() instead > > Each of the four typedefs is used exactly once, as the type of a struct > cgs_ops member, so they are merely an unnecessary layer of indirection. > Drop the typedefs along with their kernel-doc comments and write the > function pointer types directly in struct cgs_ops. > > While at it, drop the unused cgs_handle_t and cgs_get_firmware_info > typedefs, and document the members of struct cgs_firmware_info. > > Suggested-by: Christian König <[email protected]> > Link: > https://lore.kernel.org/all/[email protected]/ > Signed-off-by: Omokefe Emmanuel Onanaroghene <[email protected]> > Tested-by: Arnaud Bonnet <[email protected]> > --- > added kernel-doc comments for struct cgs_firmware_info > > drivers/gpu/drm/amd/include/cgs_common.h | 74 +++++++++--------------- > 1 file changed, 26 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/amd/include/cgs_common.h > b/drivers/gpu/drm/amd/include/cgs_common.hZZ > index f40b6a03fe63..79522e1bec4f 100644 > --- a/drivers/gpu/drm/amd/include/cgs_common.h > +++ b/drivers/gpu/drm/amd/include/cgs_common.h > @@ -63,6 +63,14 @@ enum cgs_ucode_id { > > /** > * struct cgs_firmware_info - Firmware information > + * @version: firmware header version > + * @fw_version: firmware version reported by the IP block > + * @feature_version: firmware feature version > + * @image_size: firmware image size in bytes > + * @mc_addr: GPU address of the firmware image > + * @ucode_start_address: ucode start address (SMU firmware only) > + * @kptr: kernel virtual address of the firmware image > + * @is_kicker: whether this is a "kicker" (k) firmware variant > */ > struct cgs_firmware_info { > uint16_t version; > @@ -78,45 +86,6 @@ struct cgs_firmware_info { > bool is_kicker; > }; > > -typedef unsigned long cgs_handle_t; > - > -/** > - * cgs_read_register() - Read an MMIO register > - * @cgs_device: opaque device handle > - * @offset: register offset > - * > - * Return: register value > - */ > -typedef uint32_t (*cgs_read_register_t)(struct cgs_device *cgs_device, > unsigned offset); > - > -/** > - * cgs_write_register() - Write an MMIO register > - * @cgs_device: opaque device handle > - * @offset: register offset > - * @value: register value > - */ > -typedef void (*cgs_write_register_t)(struct cgs_device *cgs_device, unsigned > offset, > - uint32_t value); > - > -/** > - * cgs_read_ind_register() - Read an indirect register > - * @cgs_device: opaque device handle > - * @offset: register offset > - * > - * Return: register value > - */ > -typedef uint32_t (*cgs_read_ind_register_t)(struct cgs_device *cgs_device, > enum cgs_ind_reg space, > - unsigned index); > - > -/** > - * cgs_write_ind_register() - Write an indirect register > - * @cgs_device: opaque device handle > - * @offset: register offset > - * @value: register value > - */ > -typedef void (*cgs_write_ind_register_t)(struct cgs_device *cgs_device, enum > cgs_ind_reg space, > - unsigned index, uint32_t value); > - > #define CGS_REG_FIELD_SHIFT(reg, field) reg##__##field##__SHIFT > #define CGS_REG_FIELD_MASK(reg, field) reg##__##field##_MASK > > @@ -133,18 +102,27 @@ typedef void (*cgs_write_ind_register_t)(struct > cgs_device *cgs_device, enum cgs > #define CGS_WREG32_FIELD_IND(device, space, reg, field, val) \ > cgs_write_ind_register(device, space, ix##reg, > (cgs_read_ind_register(device, space, ix##reg) & ~CGS_REG_FIELD_MASK(reg, > field)) | (val) << CGS_REG_FIELD_SHIFT(reg, field)) > > -typedef int (*cgs_get_firmware_info)(struct cgs_device *cgs_device, > - enum cgs_ucode_id type, > - struct cgs_firmware_info *info); > - > +/** > + * struct cgs_ops - Common Graphics Services operations > + * @read_register: read register value > + * @write_register: write register value > + * @read_ind_register: read indirect register value > + * @write_ind_register: write indirect register value > + * @get_firmware_info: get firmware information > + */ > struct cgs_ops { > /* MMIO access */ > - cgs_read_register_t read_register; > - cgs_write_register_t write_register; > - cgs_read_ind_register_t read_ind_register; > - cgs_write_ind_register_t write_ind_register; > + uint32_t (*read_register)(struct cgs_device *cgs_device, unsigned int > offset); > + void (*write_register)(struct cgs_device *cgs_device, unsigned int > offset, > + uint32_t value); > + uint32_t (*read_ind_register)(struct cgs_device *cgs_device, enum > cgs_ind_reg space, > + unsigned int index); > + void (*write_ind_register)(struct cgs_device *cgs_device, enum > cgs_ind_reg space, > + unsigned int index, uint32_t value); > /* Firmware Info */ > - cgs_get_firmware_info get_firmware_info; > + int (*get_firmware_info)(struct cgs_device *cgs_device, > + enum cgs_ucode_id type, > + struct cgs_firmware_info *info); > }; > > struct cgs_os_ops; /* To be define in OS-specific CGS header */ > -- > 2.43.0 >
