Rename struct i3c_priv_xfer to struct i3c_xfer, since private xfer in the
I3C spec refers only to SDR transfers. Ref: i3c spec ver1.2, section 3,
Technical Overview.
i3c_xfer will be used for both SDR and HDR.
Rename enum i3c_hdr_mode to i3c_xfer_mode. Previous definition need match
CCC GET_CAP1 bit position. Use 31 as SDR transfer mode.
Add i3c_device_do_xfers() with an xfer mode argument, while keeping
i3c_device_do_priv_xfers() as a wrapper that calls i3c_device_do_xfers()
with I3C_SDR for backward compatibility.
Introduce a 'cmd' field in struct i3c_xfer as an anonymous union with
'rnw', since HDR mode uses read/write commands instead of the SDR address
bit.
Add .i3c_xfers() callback for master controllers. If not implemented, fall
back to SDR with .priv_xfers(). The .priv_xfers() API can be removed once
all controllers switch to .i3c_xfers().
Add 'mode_mask' bitmask to advertise controller capability.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Why not add hdr mode in struct i3c_priv_xfer because mode can't be mixed in
one i3c transfer. for example, can't send a HDR follow one SDR between
START and STOP.
i3c_priv_xfer should be treat as whole i3c transactions. If user want send
HDR follow SDR, should be call i3c_device_do_priv_xfers_mode() twice,
instead put into a big i3c_priv_xfer[n].
change in v9
- fix typo Deprecated
- remove reduntant master->ops->priv_xfers check.
change in v8
- new API use i3c_xfer instead of i3c_priv_xfer.
change in v7
- explicit set enum I3C_HDR_* to value, which spec required.
- add comments about check priv_xfers and i3c_xfers
change in v5-v6
- none
change in v4
- Rename enum i3c_hdr_mode to i3c_xfer_mode.
change in v3
- Add Deprecated comment for priv_xfers.
change in v2
- don't use 'priv_' since it is refer to sdr mode transfer in spec.
- add 'mode_mask' indicate controller's capibility.
- add helper function to check master's supported transfer mode.
---
drivers/i3c/device.c | 27 ++++++++++++++++++++-------
drivers/i3c/internals.h | 6 +++---
drivers/i3c/master.c | 19 ++++++++++++++-----
include/linux/i3c/device.h | 40 +++++++++++++++++++++++++++++-----------
include/linux/i3c/master.h | 4 ++++
5 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 2396545763ff853097d9f0173787e087f7a6e688..8a156f5ad6929402eb92b152d2e80754dd5a2387 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -15,12 +15,12 @@
#include "internals.h"
/**
- * i3c_device_do_priv_xfers() - do I3C SDR private transfers directed to a
- * specific device
+ * i3c_device_do_xfers() - do I3C transfers directed to a specific device
*
* @dev: device with which the transfers should be done
* @xfers: array of transfers
* @nxfers: number of transfers
+ * @mode: transfer mode
*
* Initiate one or several private SDR transfers with @dev.
*
@@ -33,9 +33,8 @@
* 'xfers' some time later. See I3C spec ver 1.1.1 09-Jun-2021. Section:
* 5.1.2.2.3.
*/
-int i3c_device_do_priv_xfers(struct i3c_device *dev,
- struct i3c_priv_xfer *xfers,
- int nxfers)
+int i3c_device_do_xfers(struct i3c_device *dev, struct i3c_xfer *xfers,
+ int nxfers, enum i3c_xfer_mode mode)
{
int ret, i;
@@ -48,12 +47,12 @@ int i3c_device_do_priv_xfers(struct i3c_device *dev,
}
i3c_bus_normaluse_lock(dev->bus);
- ret = i3c_dev_do_priv_xfers_locked(dev->desc, xfers, nxfers);
+ ret = i3c_dev_do_xfers_locked(dev->desc, xfers, nxfers, mode);
i3c_bus_normaluse_unlock(dev->bus);
return ret;
}
-EXPORT_SYMBOL_GPL(i3c_device_do_priv_xfers);
+EXPORT_SYMBOL_GPL(i3c_device_do_xfers);
/**
* i3c_device_do_setdasa() - do I3C dynamic address assignement with
@@ -260,6 +259,20 @@ i3c_device_match_id(struct i3c_device *i3cdev,
}
EXPORT_SYMBOL_GPL(i3c_device_match_id);
+/**
+ * i3c_device_get_supported_xfer_mode - Returns the supported transfer mode by
+ * connected master controller.
+ * @dev: I3C device
+ *
+ * Return: a bit mask, which supported transfer mode, bit position is defined at
+ * enum i3c_hdr_mode
+ */
+u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
+{
+ return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
+}
+EXPORT_SYMBOL_GPL(i3c_device_get_supported_xfer_mode);
+
/**
* i3c_driver_register_with_owner() - register an I3C device driver
*
diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 79ceaa5f5afd6f8772db114472cfad99d4dd4341..f609e5098137c1b00db1830a176bb44c2802eb6f 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -15,9 +15,9 @@ void i3c_bus_normaluse_lock(struct i3c_bus *bus);
void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
int i3c_dev_setdasa_locked(struct i3c_dev_desc *dev);
-int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
- struct i3c_priv_xfer *xfers,
- int nxfers);
+int i3c_dev_do_xfers_locked(struct i3c_dev_desc *dev,
+ struct i3c_xfer *xfers,
+ int nxfers, enum i3c_xfer_mode mode);
int i3c_dev_disable_ibi_locked(struct i3c_dev_desc *dev);
int i3c_dev_enable_ibi_locked(struct i3c_dev_desc *dev);
int i3c_dev_request_ibi_locked(struct i3c_dev_desc *dev,
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 66513a27e6e776d251203b286bcaecb9d8fc67b9..30c5e5de7963c78735e96605367e9a762d286e86 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2821,10 +2821,14 @@ EXPORT_SYMBOL_GPL(i3c_generic_ibi_recycle_slot);
static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
{
- if (!ops || !ops->bus_init || !ops->priv_xfers ||
+ if (!ops || !ops->bus_init ||
!ops->send_ccc_cmd || !ops->do_daa || !ops->i2c_xfers)
return -EINVAL;
+ /* Must provide one of priv_xfers (SDR only) or i3c_xfers (all modes) */
+ if (!ops->priv_xfers && !ops->i3c_xfers)
+ return -EINVAL;
+
if (ops->request_ibi &&
(!ops->enable_ibi || !ops->disable_ibi || !ops->free_ibi ||
!ops->recycle_ibi_slot))
@@ -3014,9 +3018,8 @@ int i3c_dev_setdasa_locked(struct i3c_dev_desc *dev)
dev->boardinfo->init_dyn_addr);
}
-int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
- struct i3c_priv_xfer *xfers,
- int nxfers)
+int i3c_dev_do_xfers_locked(struct i3c_dev_desc *dev, struct i3c_xfer *xfers,
+ int nxfers, enum i3c_xfer_mode mode)
{
struct i3c_master_controller *master;
@@ -3027,9 +3030,15 @@ int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
if (!master || !xfers)
return -EINVAL;
- if (!master->ops->priv_xfers)
+ if (mode != I3C_SDR && !(master->this->info.hdr_cap & BIT(mode)))
return -EOPNOTSUPP;
+ if (master->ops->i3c_xfers)
+ return master->ops->i3c_xfers(dev, xfers, nxfers, mode);
+
+ if (mode != I3C_SDR)
+ return -EINVAL;
+
return master->ops->priv_xfers(dev, xfers, nxfers);
}
diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
index 7f136de4b73ef839fb4a1837a87b1aebbddbfe93..7f7738041f3809e538816e94f90b99e58eb806f9 100644
--- a/include/linux/i3c/device.h
+++ b/include/linux/i3c/device.h
@@ -39,20 +39,25 @@ enum i3c_error_code {
};
/**
- * enum i3c_hdr_mode - HDR mode ids
+ * enum i3c_xfer_mode - I3C xfer mode ids
* @I3C_HDR_DDR: DDR mode
* @I3C_HDR_TSP: TSP mode
* @I3C_HDR_TSL: TSL mode
+ * @I3C_SDR: SDR mode (NOT HDR mode)
*/
-enum i3c_hdr_mode {
- I3C_HDR_DDR,
- I3C_HDR_TSP,
- I3C_HDR_TSL,
+enum i3c_xfer_mode {
+ /* The below 3 value (I3C_HDR*) must match GETCAP1 Byte bit position */
+ I3C_HDR_DDR = 0,
+ I3C_HDR_TSP = 1,
+ I3C_HDR_TSL = 2,
+ /* Use for default SDR transfer mode */
+ I3C_SDR = 0x31,
};
/**
- * struct i3c_priv_xfer - I3C SDR private transfer
+ * struct i3c_xfer - I3C data transfer
* @rnw: encodes the transfer direction. true for a read, false for a write
+ * @cmd: Read/Write command in HDR mode, read: 0x80 - 0xff, write: 0x00 - 0x7f
* @len: transfer length in bytes of the transfer
* @actual_len: actual length in bytes are transferred by the controller
* @data: input/output buffer
@@ -60,8 +65,11 @@ enum i3c_hdr_mode {
* @data.out: output buffer. Must point to a DMA-able buffer
* @err: I3C error code
*/
-struct i3c_priv_xfer {
- u8 rnw;
+struct i3c_xfer {
+ union {
+ u8 rnw;
+ u8 cmd;
+ };
u16 len;
u16 actual_len;
union {
@@ -71,6 +79,9 @@ struct i3c_priv_xfer {
enum i3c_error_code err;
};
+/* keep back compatible */
+#define i3c_priv_xfer i3c_xfer
+
/**
* enum i3c_dcr - I3C DCR values
* @I3C_DCR_GENERIC_DEVICE: generic I3C device
@@ -297,9 +308,15 @@ static __always_inline void i3c_i2c_driver_unregister(struct i3c_driver *i3cdrv,
i3c_i2c_driver_unregister, \
__i2cdrv)
-int i3c_device_do_priv_xfers(struct i3c_device *dev,
- struct i3c_priv_xfer *xfers,
- int nxfers);
+int i3c_device_do_xfers(struct i3c_device *dev, struct i3c_xfer *xfers,
+ int nxfers, enum i3c_xfer_mode mode);
+
+static inline int i3c_device_do_priv_xfers(struct i3c_device *dev,
+ struct i3c_priv_xfer *xfers,
+ int nxfers)
+{
+ return i3c_device_do_xfers(dev, xfers, nxfers, I3C_SDR);
+}
int i3c_device_do_setdasa(struct i3c_device *dev);
@@ -341,5 +358,6 @@ int i3c_device_request_ibi(struct i3c_device *dev,
void i3c_device_free_ibi(struct i3c_device *dev);
int i3c_device_enable_ibi(struct i3c_device *dev);
int i3c_device_disable_ibi(struct i3c_device *dev);
+u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev);
#endif /* I3C_DEV_H */
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index c52a82dd79a63436c1de6a01c11df9e295c1660e..d0d5b3a9049f0b5ff65ae6c5a7d59444b373edec 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -474,9 +474,13 @@ struct i3c_master_controller_ops {
const struct i3c_ccc_cmd *cmd);
int (*send_ccc_cmd)(struct i3c_master_controller *master,
struct i3c_ccc_cmd *cmd);
+ /* Deprecated, please use i3c_xfers() */
int (*priv_xfers)(struct i3c_dev_desc *dev,
struct i3c_priv_xfer *xfers,
int nxfers);
+ int (*i3c_xfers)(struct i3c_dev_desc *dev,
+ struct i3c_xfer *xfers,
+ int nxfers, enum i3c_xfer_mode mode);
int (*attach_i2c_dev)(struct i2c_dev_desc *dev);
void (*detach_i2c_dev)(struct i2c_dev_desc *dev);
int (*i2c_xfers)(struct i2c_dev_desc *dev,
--
2.34.1Hi Frank,
On Thu, 2025-11-06 at 12:36 -0500, Frank Li wrote:
> Rename struct i3c_priv_xfer to struct i3c_xfer, since private xfer in the
> I3C spec refers only to SDR transfers. Ref: i3c spec ver1.2, section 3,
> Technical Overview.
>
> i3c_xfer will be used for both SDR and HDR.
>
> Rename enum i3c_hdr_mode to i3c_xfer_mode. Previous definition need match
> CCC GET_CAP1 bit position. Use 31 as SDR transfer mode.
>
> Add i3c_device_do_xfers() with an xfer mode argument, while keeping
> i3c_device_do_priv_xfers() as a wrapper that calls i3c_device_do_xfers()
> with I3C_SDR for backward compatibility.
>
> Introduce a 'cmd' field in struct i3c_xfer as an anonymous union with
> 'rnw', since HDR mode uses read/write commands instead of the SDR address
> bit.
>
> Add .i3c_xfers() callback for master controllers. If not implemented, fall
> back to SDR with .priv_xfers(). The .priv_xfers() API can be removed once
> all controllers switch to .i3c_xfers().
>
> Add 'mode_mask' bitmask to advertise controller capability.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Why not add hdr mode in struct i3c_priv_xfer because mode can't be mixed in
> one i3c transfer. for example, can't send a HDR follow one SDR between
> START and STOP.
>
> i3c_priv_xfer should be treat as whole i3c transactions. If user want send
> HDR follow SDR, should be call i3c_device_do_priv_xfers_mode() twice,
> instead put into a big i3c_priv_xfer[n].
>
> change in v9
> - fix typo Deprecated
> - remove reduntant master->ops->priv_xfers check.
>
> change in v8
> - new API use i3c_xfer instead of i3c_priv_xfer.
>
> change in v7
> - explicit set enum I3C_HDR_* to value, which spec required.
> - add comments about check priv_xfers and i3c_xfers
>
> change in v5-v6
> - none
>
> change in v4
> - Rename enum i3c_hdr_mode to i3c_xfer_mode.
>
> change in v3
> - Add Deprecated comment for priv_xfers.
>
> change in v2
> - don't use 'priv_' since it is refer to sdr mode transfer in spec.
> - add 'mode_mask' indicate controller's capibility.
> - add helper function to check master's supported transfer mode.
> ---
> drivers/i3c/device.c | 27 ++++++++++++++++++++-------
> drivers/i3c/internals.h | 6 +++---
> drivers/i3c/master.c | 19 ++++++++++++++-----
> include/linux/i3c/device.h | 40 +++++++++++++++++++++++++++++-----------
> include/linux/i3c/master.h | 4 ++++
> 5 files changed, 70 insertions(+), 26 deletions(-)
>
*snip*
>
> diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> index 7f136de4b73ef839fb4a1837a87b1aebbddbfe93..7f7738041f3809e538816e94f90b99e58eb806f9 100644
> --- a/include/linux/i3c/device.h
> +++ b/include/linux/i3c/device.h
> @@ -39,20 +39,25 @@ enum i3c_error_code {
> };
>
> /**
> - * enum i3c_hdr_mode - HDR mode ids
> + * enum i3c_xfer_mode - I3C xfer mode ids
> * @I3C_HDR_DDR: DDR mode
> * @I3C_HDR_TSP: TSP mode
> * @I3C_HDR_TSL: TSL mode
> + * @I3C_SDR: SDR mode (NOT HDR mode)
> */
> -enum i3c_hdr_mode {
> - I3C_HDR_DDR,
> - I3C_HDR_TSP,
> - I3C_HDR_TSL,
> +enum i3c_xfer_mode {
> + /* The below 3 value (I3C_HDR*) must match GETCAP1 Byte bit position */
> + I3C_HDR_DDR = 0,
> + I3C_HDR_TSP = 1,
> + I3C_HDR_TSL = 2,
> + /* Use for default SDR transfer mode */
> + I3C_SDR = 0x31,
0x31 is 49 - is that really what you intend here? For instance,
building this patch for ARM32 produces:
In file included from ../include/linux/bits.h:5,
from ../include/linux/ratelimit_types.h:5,
from ../include/linux/printk.h:9,
from ../include/asm-generic/bug.h:31,
from ../arch/arm/include/asm/bug.h:60,
from ../include/linux/bug.h:5,
from ../drivers/i3c/device.c:9:
../drivers/i3c/device.c: In function ‘i3c_device_get_supported_xfer_mode’:
../include/vdso/bits.h:7:40: warning: left shift count >= width of type [-Wshift-count-overflow]
7 | #define BIT(nr) (UL(1) << (nr))
| ^~
../drivers/i3c/device.c:272:68: note: in expansion of macro ‘BIT’
272 | return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
| ^~~
Should this be decimal 31, rather than hex 31?
Andrew
On Thu, Dec 04, 2025 at 10:47:57AM +1030, Andrew Jeffery wrote:
> Hi Frank,
>
> On Thu, 2025-11-06 at 12:36 -0500, Frank Li wrote:
> > Rename struct i3c_priv_xfer to struct i3c_xfer, since private xfer in the
> > I3C spec refers only to SDR transfers. Ref: i3c spec ver1.2, section 3,
> > Technical Overview.
> >
> > i3c_xfer will be used for both SDR and HDR.
> >
> > Rename enum i3c_hdr_mode to i3c_xfer_mode. Previous definition need match
> > CCC GET_CAP1 bit position. Use 31 as SDR transfer mode.
> >
> > Add i3c_device_do_xfers() with an xfer mode argument, while keeping
> > i3c_device_do_priv_xfers() as a wrapper that calls i3c_device_do_xfers()
> > with I3C_SDR for backward compatibility.
> >
> > Introduce a 'cmd' field in struct i3c_xfer as an anonymous union with
> > 'rnw', since HDR mode uses read/write commands instead of the SDR address
> > bit.
> >
> > Add .i3c_xfers() callback for master controllers. If not implemented, fall
> > back to SDR with .priv_xfers(). The .priv_xfers() API can be removed once
> > all controllers switch to .i3c_xfers().
> >
> > Add 'mode_mask' bitmask to advertise controller capability.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Why not add hdr mode in struct i3c_priv_xfer because mode can't be mixed in
> > one i3c transfer. for example, can't send a HDR follow one SDR between
> > START and STOP.
> >
> > i3c_priv_xfer should be treat as whole i3c transactions. If user want send
> > HDR follow SDR, should be call i3c_device_do_priv_xfers_mode() twice,
> > instead put into a big i3c_priv_xfer[n].
> >
> > change in v9
> > - fix typo Deprecated
> > - remove reduntant master->ops->priv_xfers check.
> >
> > change in v8
> > - new API use i3c_xfer instead of i3c_priv_xfer.
> >
> > change in v7
> > - explicit set enum I3C_HDR_* to value, which spec required.
> > - add comments about check priv_xfers and i3c_xfers
> >
> > change in v5-v6
> > - none
> >
> > change in v4
> > - Rename enum i3c_hdr_mode to i3c_xfer_mode.
> >
> > change in v3
> > - Add Deprecated comment for priv_xfers.
> >
> > change in v2
> > - don't use 'priv_' since it is refer to sdr mode transfer in spec.
> > - add 'mode_mask' indicate controller's capibility.
> > - add helper function to check master's supported transfer mode.
> > ---
> > drivers/i3c/device.c | 27 ++++++++++++++++++++-------
> > drivers/i3c/internals.h | 6 +++---
> > drivers/i3c/master.c | 19 ++++++++++++++-----
> > include/linux/i3c/device.h | 40 +++++++++++++++++++++++++++++-----------
> > include/linux/i3c/master.h | 4 ++++
> > 5 files changed, 70 insertions(+), 26 deletions(-)
> >
>
> *snip*
>
> >
> > diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> > index 7f136de4b73ef839fb4a1837a87b1aebbddbfe93..7f7738041f3809e538816e94f90b99e58eb806f9 100644
> > --- a/include/linux/i3c/device.h
> > +++ b/include/linux/i3c/device.h
> > @@ -39,20 +39,25 @@ enum i3c_error_code {
> > };
> >
> > /**
> > - * enum i3c_hdr_mode - HDR mode ids
> > + * enum i3c_xfer_mode - I3C xfer mode ids
> > * @I3C_HDR_DDR: DDR mode
> > * @I3C_HDR_TSP: TSP mode
> > * @I3C_HDR_TSL: TSL mode
> > + * @I3C_SDR: SDR mode (NOT HDR mode)
> > */
> > -enum i3c_hdr_mode {
> > - I3C_HDR_DDR,
> > - I3C_HDR_TSP,
> > - I3C_HDR_TSL,
> > +enum i3c_xfer_mode {
> > + /* The below 3 value (I3C_HDR*) must match GETCAP1 Byte bit position */
> > + I3C_HDR_DDR = 0,
> > + I3C_HDR_TSP = 1,
> > + I3C_HDR_TSL = 2,
> > + /* Use for default SDR transfer mode */
> > + I3C_SDR = 0x31,
>
> 0x31 is 49 - is that really what you intend here? For instance,
> building this patch for ARM32 produces:
>
> In file included from ../include/linux/bits.h:5,
> from ../include/linux/ratelimit_types.h:5,
> from ../include/linux/printk.h:9,
> from ../include/asm-generic/bug.h:31,
> from ../arch/arm/include/asm/bug.h:60,
> from ../include/linux/bug.h:5,
> from ../drivers/i3c/device.c:9:
> ../drivers/i3c/device.c: In function ‘i3c_device_get_supported_xfer_mode’:
> ../include/vdso/bits.h:7:40: warning: left shift count >= width of type [-Wshift-count-overflow]
> 7 | #define BIT(nr) (UL(1) << (nr))
> | ^~
> ../drivers/i3c/device.c:272:68: note: in expansion of macro ‘BIT’
> 272 | return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
> | ^~~
>
> Should this be decimal 31, rather than hex 31?
Yes, fixed patch
https://lore.kernel.org/linux-i3c/aS8YKfhPAxvpj6xy@lizhi-Precision-Tower-5810/T/#m7367e8845c31b924672445dcf639eea39112aac0
Frank
>
> Andrew
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
© 2016 - 2025 Red Hat, Inc.