[RFC,3/5] i3c: device: expose transfer strutures to uapi

Commit Message

Vitor Soares Dec. 10, 2019, 3:37 p.m. UTC

Move i3c transfer related structures to uapi, so they can be access
by userspace.

Signed-off-by: Vitor Soares <[e-mail]>
---
 include/linux/i3c/device.h      | 54 +--------------------------------
 include/uapi/linux/i3c/device.h | 66 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 53 deletions(-)
 create mode 100644 include/uapi/linux/i3c/device.h

Comments

Greg Kroah-Hartman Dec. 12, 2019, 2:42 p.m. UTC | #1

On Tue, Dec 10, 2019 at 04:37:31PM +0100, Vitor Soares wrote:
> --- /dev/null
> +++ b/include/uapi/linux/i3c/device.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Not a valid uapi SPDX license :(


> +/*
> + * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
> + *
> + * Author: Vitor Soares <[e-mail]>
> + */
> +
> +#ifndef _UAPI_LINUX_I3C_DEVICE_H
> +#define _UAPI_LINUX_I3C_DEVICE_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * enum i3c_error_code - I3C error codes
> + *
> + * These are the standard error codes as defined by the I3C specification.
> + * When -EIO is returned by the i3c_device_do_priv_xfers() or
> + * i3c_device_send_hdr_cmds() one can check the error code in
> + * &struct_i3c_priv_xfer.err or &struct i3c_hdr_cmd.err to get a better idea of
> + * what went wrong.
> + *
> + * @I3C_ERROR_UNKNOWN: unknown error, usually means the error is not I3C
> + *		       related
> + * @I3C_ERROR_M0: M0 error
> + * @I3C_ERROR_M1: M1 error
> + * @I3C_ERROR_M2: M2 error
> + */
> +enum i3c_error_code {
> +	I3C_ERROR_UNKNOWN = 0,
> +	I3C_ERROR_M0 = 1,
> +	I3C_ERROR_M1,
> +	I3C_ERROR_M2,

You have to specify all of these.

> +};
> +
> +/**
> + * enum i3c_hdr_mode - HDR mode ids
> + * @I3C_HDR_DDR: DDR mode
> + * @I3C_HDR_TSP: TSP mode
> + * @I3C_HDR_TSL: TSL mode
> + */
> +enum i3c_hdr_mode {
> +	I3C_HDR_DDR,
> +	I3C_HDR_TSP,
> +	I3C_HDR_TSL,

same here.


> +};
> +
> +/**
> + * struct i3c_priv_xfer - I3C SDR private transfer
> + * @rnw: encodes the transfer direction. true for a read, false for a write
> + * @len: transfer length in bytes of the transfer
> + * @data: input/output buffer
> + * @data.in: input buffer. Must point to a DMA-able buffer
> + * @data.out: output buffer. Must point to a DMA-able buffer
> + * @err: I3C error code
> + */
> +struct i3c_priv_xfer {
> +	u8 rnw;
> +	u16 len;
> +	union {
> +		void *in;
> +		const void *out;
> +	} data;
> +	enum i3c_error_code err;

Ick, that's a horrid user/kernel api structure that will not work at
all.

Please fix.

thanks,

greg k-h

Vitor Soares Dec. 12, 2019, 2:48 p.m. UTC | #2

HI Greg,

From: Greg KH <[e-mail]>
Date: Thu, Dec 12, 2019 at 14:42:33

> On Tue, Dec 10, 2019 at 04:37:31PM +0100, Vitor Soares wrote:
> > --- /dev/null
> > +++ b/include/uapi/linux/i3c/device.h
> > @@ -0,0 +1,66 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> 
> Not a valid uapi SPDX license :(

Sorry, I missed that one. I already got the right SPDX.

> 
> 
> > +/*
> > + * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
> > + *
> > + * Author: Vitor Soares <[e-mail]>
> > + */
> > +
> > +#ifndef _UAPI_LINUX_I3C_DEVICE_H
> > +#define _UAPI_LINUX_I3C_DEVICE_H
> > +
> > +#include <linux/types.h>
> > +
> > +/**
> > + * enum i3c_error_code - I3C error codes
> > + *
> > + * These are the standard error codes as defined by the I3C specification.
> > + * When -EIO is returned by the i3c_device_do_priv_xfers() or
> > + * i3c_device_send_hdr_cmds() one can check the error code in
> > + * &struct_i3c_priv_xfer.err or &struct i3c_hdr_cmd.err to get a better idea of
> > + * what went wrong.
> > + *
> > + * @I3C_ERROR_UNKNOWN: unknown error, usually means the error is not I3C
> > + *		       related
> > + * @I3C_ERROR_M0: M0 error
> > + * @I3C_ERROR_M1: M1 error
> > + * @I3C_ERROR_M2: M2 error
> > + */
> > +enum i3c_error_code {
> > +	I3C_ERROR_UNKNOWN = 0,
> > +	I3C_ERROR_M0 = 1,
> > +	I3C_ERROR_M1,
> > +	I3C_ERROR_M2,
> 
> You have to specify all of these.
> 
> > +};
> > +
> > +/**
> > + * enum i3c_hdr_mode - HDR mode ids
> > + * @I3C_HDR_DDR: DDR mode
> > + * @I3C_HDR_TSP: TSP mode
> > + * @I3C_HDR_TSL: TSL mode
> > + */
> > +enum i3c_hdr_mode {
> > +	I3C_HDR_DDR,
> > +	I3C_HDR_TSP,
> > +	I3C_HDR_TSL,
> 
> same here.
> 
> 
> > +};
> > +
> > +/**
> > + * struct i3c_priv_xfer - I3C SDR private transfer
> > + * @rnw: encodes the transfer direction. true for a read, false for a write
> > + * @len: transfer length in bytes of the transfer
> > + * @data: input/output buffer
> > + * @data.in: input buffer. Must point to a DMA-able buffer
> > + * @data.out: output buffer. Must point to a DMA-able buffer
> > + * @err: I3C error code
> > + */
> > +struct i3c_priv_xfer {
> > +	u8 rnw;
> > +	u16 len;
> > +	union {
> > +		void *in;
> > +		const void *out;
> > +	} data;
> > +	enum i3c_error_code err;
> 
> Ick, that's a horrid user/kernel api structure that will not work at
> all.
> 
> Please fix.
> 
> thanks,
> 
> greg k-h

Thanks for your feedback, I will address it in next version.

Best regards,
Vitor Soares

Patch

diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
index de102e4..1df05c2 100644
--- a/include/linux/i3c/device.h
+++ b/include/linux/i3c/device.h
@@ -14,59 +14,7 @@ 
 #include <linux/kconfig.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
-
-/**
- * enum i3c_error_code - I3C error codes
- *
- * These are the standard error codes as defined by the I3C specification.
- * When -EIO is returned by the i3c_device_do_priv_xfers() or
- * i3c_device_send_hdr_cmds() one can check the error code in
- * &struct_i3c_priv_xfer.err or &struct i3c_hdr_cmd.err to get a better idea of
- * what went wrong.
- *
- * @I3C_ERROR_UNKNOWN: unknown error, usually means the error is not I3C
- *		       related
- * @I3C_ERROR_M0: M0 error
- * @I3C_ERROR_M1: M1 error
- * @I3C_ERROR_M2: M2 error
- */
-enum i3c_error_code {
-	I3C_ERROR_UNKNOWN = 0,
-	I3C_ERROR_M0 = 1,
-	I3C_ERROR_M1,
-	I3C_ERROR_M2,
-};
-
-/**
- * enum i3c_hdr_mode - HDR mode ids
- * @I3C_HDR_DDR: DDR mode
- * @I3C_HDR_TSP: TSP mode
- * @I3C_HDR_TSL: TSL mode
- */
-enum i3c_hdr_mode {
-	I3C_HDR_DDR,
-	I3C_HDR_TSP,
-	I3C_HDR_TSL,
-};
-
-/**
- * struct i3c_priv_xfer - I3C SDR private transfer
- * @rnw: encodes the transfer direction. true for a read, false for a write
- * @len: transfer length in bytes of the transfer
- * @data: input/output buffer
- * @data.in: input buffer. Must point to a DMA-able buffer
- * @data.out: output buffer. Must point to a DMA-able buffer
- * @err: I3C error code
- */
-struct i3c_priv_xfer {
-	u8 rnw;
-	u16 len;
-	union {
-		void *in;
-		const void *out;
-	} data;
-	enum i3c_error_code err;
-};
+#include <uapi/linux/i3c/device.h>
 
 /**
  * enum i3c_dcr - I3C DCR values
diff --git a/include/uapi/linux/i3c/device.h b/include/uapi/linux/i3c/device.h
new file mode 100644
index 0000000..edbb14d
--- /dev/null
+++ b/include/uapi/linux/i3c/device.h
@@ -0,0 +1,66 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <[e-mail]>
+ */
+
+#ifndef _UAPI_LINUX_I3C_DEVICE_H
+#define _UAPI_LINUX_I3C_DEVICE_H
+
+#include <linux/types.h>
+
+/**
+ * enum i3c_error_code - I3C error codes
+ *
+ * These are the standard error codes as defined by the I3C specification.
+ * When -EIO is returned by the i3c_device_do_priv_xfers() or
+ * i3c_device_send_hdr_cmds() one can check the error code in
+ * &struct_i3c_priv_xfer.err or &struct i3c_hdr_cmd.err to get a better idea of
+ * what went wrong.
+ *
+ * @I3C_ERROR_UNKNOWN: unknown error, usually means the error is not I3C
+ *		       related
+ * @I3C_ERROR_M0: M0 error
+ * @I3C_ERROR_M1: M1 error
+ * @I3C_ERROR_M2: M2 error
+ */
+enum i3c_error_code {
+	I3C_ERROR_UNKNOWN = 0,
+	I3C_ERROR_M0 = 1,
+	I3C_ERROR_M1,
+	I3C_ERROR_M2,
+};
+
+/**
+ * enum i3c_hdr_mode - HDR mode ids
+ * @I3C_HDR_DDR: DDR mode
+ * @I3C_HDR_TSP: TSP mode
+ * @I3C_HDR_TSL: TSL mode
+ */
+enum i3c_hdr_mode {
+	I3C_HDR_DDR,
+	I3C_HDR_TSP,
+	I3C_HDR_TSL,
+};
+
+/**
+ * struct i3c_priv_xfer - I3C SDR private transfer
+ * @rnw: encodes the transfer direction. true for a read, false for a write
+ * @len: transfer length in bytes of the transfer
+ * @data: input/output buffer
+ * @data.in: input buffer. Must point to a DMA-able buffer
+ * @data.out: output buffer. Must point to a DMA-able buffer
+ * @err: I3C error code
+ */
+struct i3c_priv_xfer {
+	u8 rnw;
+	u16 len;
+	union {
+		void *in;
+		const void *out;
+	} data;
+	enum i3c_error_code err;
+};
+
+#endif /* _UAPI_LINUX_I3C_DEVICE_H */