[PATCH] drm/amd/display: fix kernel-doc warnings for cgs register typedefs

[PATCH] drm/amd/display: fix kernel-doc warnings for cgs register typedefs

[Omokefe Emmanuel Onanaroghene]

kernel-doc reports prototype mismatches for the cgs register accessor
typedefs, whose comments do not use the documented typedef format, e.g.:

  drivers/gpu/drm/amd/include/cgs_common.h:89 ... expecting prototype
  for cgs_read_register(). Prototype was for uint32_t() instead

Use the typedef comment format for all four accessors (also restoring
the dropped _t suffix) and replace the wrong @offset with the real
@space/@index params in the two indirect-register comments.

Signed-off-by: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>
---
 drivers/gpu/drm/amd/include/cgs_common.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/cgs_common.h b/drivers/gpu/drm/amd/include/cgs_common.h
index f40b6a03fe63..3a25c3c75a3a 100644
--- a/drivers/gpu/drm/amd/include/cgs_common.h
+++ b/drivers/gpu/drm/amd/include/cgs_common.h
@@ -81,7 +81,7 @@ struct cgs_firmware_info {
 typedef unsigned long cgs_handle_t;
 
 /**
- * cgs_read_register() - Read an MMIO register
+ * typedef cgs_read_register_t - Read an MMIO register
  * @cgs_device:	opaque device handle
  * @offset:	register offset
  *
@@ -90,7 +90,7 @@ typedef unsigned long cgs_handle_t;
 typedef uint32_t (*cgs_read_register_t)(struct cgs_device *cgs_device, unsigned offset);
 
 /**
- * cgs_write_register() - Write an MMIO register
+ * typedef cgs_write_register_t - Write an MMIO register
  * @cgs_device:	opaque device handle
  * @offset:	register offset
  * @value:	register value
@@ -99,9 +99,10 @@ typedef void (*cgs_write_register_t)(struct cgs_device *cgs_device, unsigned off
 				     uint32_t value);
 
 /**
- * cgs_read_ind_register() - Read an indirect register
+ * typedef cgs_read_ind_register_t - Read an indirect register
  * @cgs_device:	opaque device handle
- * @offset:	register offset
+ * @space:	indirect register space
+ * @index:	register index
  *
  * Return:  register value
  */
@@ -109,9 +110,10 @@ typedef uint32_t (*cgs_read_ind_register_t)(struct cgs_device *cgs_device, enum
 					    unsigned index);
 
 /**
- * cgs_write_ind_register() - Write an indirect register
+ * typedef cgs_write_ind_register_t - Write an indirect register
  * @cgs_device:	opaque device handle
- * @offset:	register offset
+ * @space:	indirect register space
+ * @index:	register index
  * @value:	register value
  */
 typedef void (*cgs_write_ind_register_t)(struct cgs_device *cgs_device, enum cgs_ind_reg space,
-- 
2.43.0

Re: [PATCH] drm/amd/display: fix kernel-doc warnings for cgs register typedefs

[Christian König]

On 10/2/26 04:17, Omokefe Emmanuel Onanaroghene wrote:
> kernel-doc reports prototype mismatches for the cgs register accessor
> typedefs, whose comments do not use the documented typedef format, e.g.:
> 
>   drivers/gpu/drm/amd/include/cgs_common.h:89 ... expecting prototype
>   for cgs_read_register(). Prototype was for uint32_t() instead
> 
> Use the typedef comment format for all four accessors (also restoring
> the dropped _t suffix) and replace the wrong @offset with the real
> @space/@index params in the two indirect-register comments.
> 
> Signed-off-by: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>

We should potentially just completely nuke those typedefs since they are actually an unnecessarily layer of indirection.

Regards,
Christian.

> ---
>  drivers/gpu/drm/amd/include/cgs_common.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/include/cgs_common.h b/drivers/gpu/drm/amd/include/cgs_common.h
> index f40b6a03fe63..3a25c3c75a3a 100644
> --- a/drivers/gpu/drm/amd/include/cgs_common.h
> +++ b/drivers/gpu/drm/amd/include/cgs_common.h
> @@ -81,7 +81,7 @@ struct cgs_firmware_info {
>  typedef unsigned long cgs_handle_t;
> 
>  /**
> - * cgs_read_register() - Read an MMIO register
> + * typedef cgs_read_register_t - Read an MMIO register
>   * @cgs_device:        opaque device handle
>   * @offset:    register offset
>   *
> @@ -90,7 +90,7 @@ typedef unsigned long cgs_handle_t;
>  typedef uint32_t (*cgs_read_register_t)(struct cgs_device *cgs_device, unsigned offset);
> 
>  /**
> - * cgs_write_register() - Write an MMIO register
> + * typedef cgs_write_register_t - Write an MMIO register
>   * @cgs_device:        opaque device handle
>   * @offset:    register offset
>   * @value:     register value
> @@ -99,9 +99,10 @@ typedef void (*cgs_write_register_t)(struct cgs_device *cgs_device, unsigned off
>                                      uint32_t value);
> 
>  /**
> - * cgs_read_ind_register() - Read an indirect register
> + * typedef cgs_read_ind_register_t - Read an indirect register
>   * @cgs_device:        opaque device handle
> - * @offset:    register offset
> + * @space:     indirect register space
> + * @index:     register index
>   *
>   * Return:  register value
>   */
> @@ -109,9 +110,10 @@ typedef uint32_t (*cgs_read_ind_register_t)(struct cgs_device *cgs_device, enum
>                                             unsigned index);
> 
>  /**
> - * cgs_write_ind_register() - Write an indirect register
> + * typedef cgs_write_ind_register_t - Write an indirect register
>   * @cgs_device:        opaque device handle
> - * @offset:    register offset
> + * @space:     indirect register space
> + * @index:     register index
>   * @value:     register value
>   */
>  typedef void (*cgs_write_ind_register_t)(struct cgs_device *cgs_device, enum cgs_ind_reg space,
> --
> 2.43.0
> 

Re: [PATCH] drm/amd/display: fix kernel-doc warnings for cgs register typedefs

[Omokefe Emmanuel Onanaroghene]

Hi Christian,

On 10/2/26 13:38, Christian König wrote:
> We should potentially just completely nuke those typedefs since they
> are actually an unnecessarily layer of indirection.

Yes, the cleanup is necessary, but the four comments don't just name the typedefs, they're the only prose describing the operations (MMIO vs indirect access, offset/space/index semantics, return values). Should i relocated onto the struct cgs_ops members or the cgs_read_register macros?

Thanks,
Omokefe

[PATCH v2] drm/amd/display: remove unused cgs register accessor typedefs

[Omokefe Emmanuel Onanaroghene]

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.

Suggested-by: Christian König <christian.koenig@amd.com>
Link: https://lore.kernel.org/all/981f01e1-0c3b-4854-9d72-8bf22af9fc64@amd.com/
Signed-off-by: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>
---
 drivers/gpu/drm/amd/include/cgs_common.h | 60 +++++++-----------------
 1 file changed, 18 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/cgs_common.h b/drivers/gpu/drm/amd/include/cgs_common.h
index f40b6a03fe63..1750401c7d08 100644
--- a/drivers/gpu/drm/amd/include/cgs_common.h
+++ b/drivers/gpu/drm/amd/include/cgs_common.h
@@ -80,43 +80,6 @@ struct cgs_firmware_info {
 
 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
 
@@ -137,14 +100,27 @@ 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

Re: [PATCH v2] drm/amd/display: remove unused cgs register accessor typedefs

[Arnaud Bonnet]

On 10/17/26 3:00 PM, Omokefe Emmanuel Onanaroghene 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.
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Link: https://lore.kernel.org/all/981f01e1-0c3b-4854-9d72-8bf22af9fc64@amd.com/
> Signed-off-by: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>
> ---
>  drivers/gpu/drm/amd/include/cgs_common.h | 60 +++++++-----------------
>  1 file changed, 18 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/include/cgs_common.h b/drivers/gpu/drm/amd/include/cgs_common.h
> index f40b6a03fe63..1750401c7d08 100644
> --- a/drivers/gpu/drm/amd/include/cgs_common.h
> +++ b/drivers/gpu/drm/amd/include/cgs_common.h
> @@ -80,43 +80,6 @@ struct cgs_firmware_info {
>  
>  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
>  
> @@ -137,14 +100,27 @@ typedef int (*cgs_get_firmware_info)(struct cgs_device *cgs_device,
>  				     enum cgs_ucode_id type,
>  				     struct cgs_firmware_info *info);

Hi Omokefe, you probably forgot
-typedef unsigned long cgs_handle_t;
-typedef int (*cgs_get_firmware_info)(struct cgs_device *cgs_device,
-                                    enum cgs_ucode_id type,
-                                    struct cgs_firmware_info *info);

As discussed I have built and ran some IGT on hardware for the versions
with and without these two, so you can still use the tag if you send v3:

Tested-by: Arnaud Bonnet <abo@medichon.fr>

>  
> +/**
> + * 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 */

[PATCH v3] drm/amd/display: remove unused cgs register accessor typedefs

[Omokefe Emmanuel Onanaroghene]

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 <christian.koenig@amd.com>
Link: https://lore.kernel.org/all/981f01e1-0c3b-4854-9d72-8bf22af9fc64@amd.com/
Signed-off-by: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>
Tested-by: Arnaud Bonnet <abo@medichon.fr>
---
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

Re: [PATCH v2] drm/amd/display: remove unused cgs register accessor typedefs

[Christian König]

On 10/17/26 15:00, Omokefe Emmanuel Onanaroghene 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.
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Link: https://lore.kernel.org/all/981f01e1-0c3b-4854-9d72-8bf22af9fc64@amd.com/
> Signed-off-by: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/include/cgs_common.h | 60 +++++++-----------------
>  1 file changed, 18 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/include/cgs_common.h b/drivers/gpu/drm/amd/include/cgs_common.h
> index f40b6a03fe63..1750401c7d08 100644
> --- a/drivers/gpu/drm/amd/include/cgs_common.h
> +++ b/drivers/gpu/drm/amd/include/cgs_common.h
> @@ -80,43 +80,6 @@ struct cgs_firmware_info {
> 
>  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
> 
> @@ -137,14 +100,27 @@ 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
>