Re: [PATCH 03/10] soc: fujitsu: hwb: Add IOC_BB_ALLOC ioctl
From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-01-08 13:25:08
On Fri, Jan 8, 2021 at 11:52 AM Misono Tomohiro [off-list ref] wrote:
+static void write_init_sync_reg(void *args)
+{
+ struct init_sync_args *sync_args = (struct init_sync_args *)args;
+
+ switch (sync_args->bb) {
+ case 0:
+ write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB0_EL1);
+ break;
+ case 1:
+ write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB1_EL1);
+ break;
+ case 2:
+ write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB2_EL1);
+ break;
+ case 3:
+ write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB3_EL1);
+ break;
+ case 4:
+ write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB4_EL1);
+ break;
+ case 5:
+ write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB5_EL1);
+ break;
+ }
+}(minor style comment I think this could be simplified into a single write_sysreg_s() with the register number calculated based on sync_args->bb, rather than duplicating the same three lines six times.
+static int ioc_bb_alloc(struct file *filp, void __user *argp)
+{
+ struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;A slightly better way to write the ioctl command specific functions is to just give the argument the correct type (struct fujitsu_hwb_ioc_bb_ctl __user*) instead of 'void __user *', to avoid the cast. Similarly, as you don't use 'filp' itself, just pass the struct hwb_private_data pointer as the first argument.
quoted hunk ↗ jump to hunk
@@ -164,6 +386,7 @@ static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp) static const struct file_operations fujitsu_hwb_dev_fops = { .owner = THIS_MODULE, .open = fujitsu_hwb_dev_open, + .unlocked_ioctl = fujitsu_hwb_dev_ioctl, };
All drivers with an ioctl interface should work in compat mode as well,
so please add a
.compat_ioctl = compat_ptr_ioctl;
+#define __FUJITSU_IOCTL_MAGIC 'F'
It's hard to find a non-conflicting range of ioctl numbers, but it would be good to note the conflict in Documentation/userspace-api/ioctl/ioctl-number.rst The 'F' range is also used in framebuffer drivers.
+/* ioctl definitions for hardware barrier driver */
+struct fujitsu_hwb_ioc_bb_ctl {
+ __u8 cmg;
+ __u8 bb;
+ __u8 unused[2];
+ __u32 size;
+ unsigned long __user *pemask;
+};
However, this structure layout is incompatible with 32-bit user
space because of the indirect pointer. See
Documentation/driver-api/ioctl.rst for how to encode a
pointer in a __u64 member.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel