Re: [PATCH 03/10] soc: fujitsu: hwb: Add IOC_BB_ALLOC ioctl

On Tue, Jan 12, 2021 at 12:02 PM misono.tomohiro@fujitsu.com
[off-list ref] wrote:

quoted

On Fri, Jan 8, 2021 at 11:52 AM Misono Tomohiro
[off-list ref] wrote:

quoted

quoted

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.

I think msr/mrs instructions needs register names at compile time so
it cannot be calculate dynamically. Or am I misunderstood?

You are correct, I didn't realize it was implemented using a string
concatenation macro and that an inline function version would be
tricky.

quoted

quoted

@@ -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;

A64FX does not support 32-bit mode (aarch32 state).
So I think unlockd_ioctl is enough or is it better to use compat_ioctl anyway?

It's a good point that this is not supported, but I would suggest adding
it anyway out of principle. It's better to always write code in a portable
way even if you do not expect to need the portability.

       Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel