On Sun, May 15, 2022 at 01:47:11PM +0300, Yaşar Arabacı wrote:
> Currently this driver uses ioctl for reading/writing per-device and
> per-client configuration. Per-client configuration can be handled by
> usespace and sent to driver with each write() call. Doing so does not
> introduce extra overhead because we copy tx config to fifo for each
> transmit anyway. This way, we don't have to introduce new ioctl's.
>
> This has not been tested as I don't have access to hardware.
>
> Signed-off-by: Yaşar Arabacı <yasar11732@xxxxxxxxx>
This commit is confusing and does a number of unrelated things. It's
not explained well what the motivation is for the patch.
If I remember this correctly, the current API is broken. It used a
too small type or something? People wanted fix it by making
incompatible changes which would have broken user space. I had said
that the right thing would be to not using ioctls at all but instead use
sysfs.
So I kind of remember that there was a motivation to get rid of the
ioctl, but I don't remember what it was and it's not explained here.
I had imagined adding the sysfs configuration along side the ioctl to
start with and then deleting the ioctl when userspace was updated. If
you're saying that we don't need any configuration at all then that's
great but that has to come from someone who has tested the code.
What is this part of the commit for?
> --- a/drivers/staging/pi433/pi433_if.h
> +++ b/drivers/staging/pi433/pi433_if.h
> @@ -75,6 +75,8 @@ struct pi433_tx_cfg {
>
> __u8 sync_pattern[8];
> __u8 address_byte;
> + __u32 payload_size;
> + __u8 payload[];
> };
regards,
dan carpenter
