Re: [PATCH v2 3/8] misc: amd-sbi: Add support for AMD_SBI IOCTL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/17/2024 2:14 PM, Greg KH wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Wed, Jul 17, 2024 at 08:10:22AM +0000, Akshay Gupta wrote:
+/* These are byte indexes into data_in and data_out arrays */
+#define RD_WR_DATA_INDEX     0
+#define REG_OFF_INDEX                0
+#define REG_VAL_INDEX                4
+#define RD_FLAG_INDEX                7
+
+#define MB_DATA_SIZE         4
These are VERY badly named global defines for all of the kernel and
userspace.  But most importantly, do you even need them here?  If so,
please provide something sane.

These are required as used as-is by userspace. We will modify the names of the MACROS.

We will update the MACROS with AMD specific prefix.

+
+struct apml_message {
+     /* message ids:
+      * Mailbox Messages:    0x0 ... 0x999
+      */
+     __u32 cmd;
+
+     /*
+      * 8 bit data for reg read,
+      * 32 bit data in case of mailbox,
+      */
+     union {
+             __u32 mb_out[2];
+             __u8 reg_out[8];
+     } data_out;
+
+     /*
+      * [0]...[3] mailbox 32bit input
+      * [7] read/write functionality
+      */
+     union {
+             __u32 mb_in[2];
+             __u8 reg_in[8];
+     } data_in;
+} __attribute__((packed));
+
+/* ioctl command for mailbox msgs using generic _IOWR */
+#define AMD_SBI_BASE_IOCTL_NR        0xF9
+#define SBRMI_IOCTL_CMD              _IOWR(AMD_SBI_BASE_IOCTL_NR, 0, struct apml_message)
Why is the ioctl command not prefixed with AMD_SBMI_BASE as well?
Existing APML library uses the "SBRMI_IOCTL_CMD", so will rename the AMD_SBI_BASE_IOCTL_NR to match with ioctl command.

thanks,

greg k-h

Do you want me to make these changes before you review the rest of the patches?

Thanks,

Akshay





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux