Message ID | 20250204091702.4014466-1-billy_tsai@aspeedtech.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [v2,1/2] i3c: Remove the const qualifier from i2c_msg pointer in i2c_xfers API | expand |
Hi Billy, On 2025-02-04 17:17:01+0800, Billy Tsai wrote: > The change is necessary to enable the use of the > `i2c_get_dma_safe_msg_buf()` API, which requires a non-const > `struct i2c_msg *` to operate. The `i2c_get_dma_safe_msg_buf()` function > ensures safe handling of I2C messages when using DMA, making it essential > for scenarios where DMA transfers are involved. By removing the `const` > qualifier, this patch allows drivers to prepare and manage DMA-safe > buffers directly. This is missing a changelog to v1 of the series. Also I asked before why it is not possible to change the signature of i2c_get_dma_safe_msg_buf() to accept 'const struct i2c_msg *' [0]. That looks like the nicer solution to me. > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > --- > drivers/i3c/master/dw-i3c-master.c | 2 +- > drivers/i3c/master/i3c-master-cdns.c | 2 +- > drivers/i3c/master/mipi-i3c-hci/core.c | 2 +- > drivers/i3c/master/svc-i3c-master.c | 2 +- > include/linux/i3c/master.h | 2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) <snip> [0] https://lore.kernel.org/lkml/23854752-435e-432e-ba21-caf690c0cecc@t-8ch.de/
> On 2025-02-04 17:17:01+0800, Billy Tsai wrote: > > The change is necessary to enable the use of the > > `i2c_get_dma_safe_msg_buf()` API, which requires a non-const > > `struct i2c_msg *` to operate. The `i2c_get_dma_safe_msg_buf()` function > > ensures safe handling of I2C messages when using DMA, making it essential > > for scenarios where DMA transfers are involved. By removing the `const` > > qualifier, this patch allows drivers to prepare and manage DMA-safe > > buffers directly. > This is missing a changelog to v1 of the series. > Also I asked before why it is not possible to change the signature of > i2c_get_dma_safe_msg_buf() to accept 'const struct i2c_msg *' [0]. > That looks like the nicer solution to me. Hi Thomas, The i2c_get_dma_safe_msg_buf() function has existed for a long time, but I do not know the original reason why it declares struct i2c_msg * without const. However, I believe this is because the i2c_put_dma_safe_msg_buf() function modifies the buffer data, so for consistency, it is declared without const. Best regards, Billy Tsai.
Hi Billy, On 2025-02-04 10:09:31+0000, Billy Tsai wrote: > > On 2025-02-04 17:17:01+0800, Billy Tsai wrote: > > > The change is necessary to enable the use of the > > > `i2c_get_dma_safe_msg_buf()` API, which requires a non-const > > > `struct i2c_msg *` to operate. The `i2c_get_dma_safe_msg_buf()` function > > > ensures safe handling of I2C messages when using DMA, making it essential > > > for scenarios where DMA transfers are involved. By removing the `const` > > > qualifier, this patch allows drivers to prepare and manage DMA-safe > > > buffers directly. > > > This is missing a changelog to v1 of the series. > > > Also I asked before why it is not possible to change the signature of > > i2c_get_dma_safe_msg_buf() to accept 'const struct i2c_msg *' [0]. > > That looks like the nicer solution to me. > The i2c_get_dma_safe_msg_buf() function has existed for a long time, > but I do not know the original reason why it declares struct i2c_msg * > without const. I would guess that nobody ever cared before. > However, I believe this is because the > i2c_put_dma_safe_msg_buf() function modifies the buffer data, so for > consistency, it is declared without const. IMO that's a weak argument. Maybe the maintainers have a preference? Thomas
On 04/02/2025 17:50:14+0100, Thomas Weißschuh wrote: > Hi Billy, > > On 2025-02-04 10:09:31+0000, Billy Tsai wrote: > > > On 2025-02-04 17:17:01+0800, Billy Tsai wrote: > > > > The change is necessary to enable the use of the > > > > `i2c_get_dma_safe_msg_buf()` API, which requires a non-const > > > > `struct i2c_msg *` to operate. The `i2c_get_dma_safe_msg_buf()` function > > > > ensures safe handling of I2C messages when using DMA, making it essential > > > > for scenarios where DMA transfers are involved. By removing the `const` > > > > qualifier, this patch allows drivers to prepare and manage DMA-safe > > > > buffers directly. > > > > > This is missing a changelog to v1 of the series. > > > > > Also I asked before why it is not possible to change the signature of > > > i2c_get_dma_safe_msg_buf() to accept 'const struct i2c_msg *' [0]. > > > That looks like the nicer solution to me. > > > The i2c_get_dma_safe_msg_buf() function has existed for a long time, > > but I do not know the original reason why it declares struct i2c_msg * > > without const. > > I would guess that nobody ever cared before. > > > However, I believe this is because the > > i2c_put_dma_safe_msg_buf() function modifies the buffer data, so for > > consistency, it is declared without const. > > IMO that's a weak argument. > Maybe the maintainers have a preference? > I guess Wolfram will be the one have the preference, I'm fine using the existing API as-is.
On Tue, Feb 04, 2025 at 05:17:01PM +0800, Billy Tsai wrote: > The change is necessary to enable the use of the > `i2c_get_dma_safe_msg_buf()` API, which requires a non-const > `struct i2c_msg *` to operate. The `i2c_get_dma_safe_msg_buf()` function > ensures safe handling of I2C messages when using DMA, making it essential > for scenarios where DMA transfers are involved. By removing the `const` > qualifier, this patch allows drivers to prepare and manage DMA-safe > buffers directly. > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> > --- Reviewed-by: Frank Li <Frank.Li@nxp.com> > drivers/i3c/master/dw-i3c-master.c | 2 +- > drivers/i3c/master/i3c-master-cdns.c | 2 +- > drivers/i3c/master/mipi-i3c-hci/core.c | 2 +- > drivers/i3c/master/svc-i3c-master.c | 2 +- > include/linux/i3c/master.h | 2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c > index d09ea5b6467c..54c7d86997d5 100644 > --- a/drivers/i3c/master/dw-i3c-master.c > +++ b/drivers/i3c/master/dw-i3c-master.c > @@ -1079,7 +1079,7 @@ static void dw_i3c_master_detach_i3c_dev(struct i3c_dev_desc *dev) > } > > static int dw_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, > - const struct i2c_msg *i2c_xfers, > + struct i2c_msg *i2c_xfers, > int i2c_nxfers) > { > struct dw_i3c_i2c_dev_data *data = i2c_dev_get_master_data(dev); > diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c > index 8d69a34986d9..c9e4c129454c 100644 > --- a/drivers/i3c/master/i3c-master-cdns.c > +++ b/drivers/i3c/master/i3c-master-cdns.c > @@ -813,7 +813,7 @@ static int cdns_i3c_master_priv_xfers(struct i3c_dev_desc *dev, > } > > static int cdns_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, > - const struct i2c_msg *xfers, int nxfers) > + struct i2c_msg *xfers, int nxfers) > { > struct i3c_master_controller *m = i2c_dev_get_master(dev); > struct cdns_i3c_master *master = to_cdns_i3c_master(m); > diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c > index e6e482a259b4..a408feac3e9e 100644 > --- a/drivers/i3c/master/mipi-i3c-hci/core.c > +++ b/drivers/i3c/master/mipi-i3c-hci/core.c > @@ -367,7 +367,7 @@ static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev, > } > > static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev, > - const struct i2c_msg *i2c_xfers, int nxfers) > + struct i2c_msg *i2c_xfers, int nxfers) > { > struct i3c_master_controller *m = i2c_dev_get_master(dev); > struct i3c_hci *hci = to_i3c_hci(m); > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index c1ee3828e7ee..24bd701b5de0 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -1584,7 +1584,7 @@ static int svc_i3c_master_priv_xfers(struct i3c_dev_desc *dev, > } > > static int svc_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, > - const struct i2c_msg *xfers, > + struct i2c_msg *xfers, > int nxfers) > { > struct i3c_master_controller *m = i2c_dev_get_master(dev); > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > index 12d532b012c5..c67922ece617 100644 > --- a/include/linux/i3c/master.h > +++ b/include/linux/i3c/master.h > @@ -475,7 +475,7 @@ struct i3c_master_controller_ops { > 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, > - const struct i2c_msg *xfers, int nxfers); > + struct i2c_msg *xfers, int nxfers); > int (*request_ibi)(struct i3c_dev_desc *dev, > const struct i3c_ibi_setup *req); > void (*free_ibi)(struct i3c_dev_desc *dev); > -- > 2.25.1 > > > -- > linux-i3c mailing list > linux-i3c@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-i3c
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c index d09ea5b6467c..54c7d86997d5 100644 --- a/drivers/i3c/master/dw-i3c-master.c +++ b/drivers/i3c/master/dw-i3c-master.c @@ -1079,7 +1079,7 @@ static void dw_i3c_master_detach_i3c_dev(struct i3c_dev_desc *dev) } static int dw_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, - const struct i2c_msg *i2c_xfers, + struct i2c_msg *i2c_xfers, int i2c_nxfers) { struct dw_i3c_i2c_dev_data *data = i2c_dev_get_master_data(dev); diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c index 8d69a34986d9..c9e4c129454c 100644 --- a/drivers/i3c/master/i3c-master-cdns.c +++ b/drivers/i3c/master/i3c-master-cdns.c @@ -813,7 +813,7 @@ static int cdns_i3c_master_priv_xfers(struct i3c_dev_desc *dev, } static int cdns_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, - const struct i2c_msg *xfers, int nxfers) + struct i2c_msg *xfers, int nxfers) { struct i3c_master_controller *m = i2c_dev_get_master(dev); struct cdns_i3c_master *master = to_cdns_i3c_master(m); diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c index e6e482a259b4..a408feac3e9e 100644 --- a/drivers/i3c/master/mipi-i3c-hci/core.c +++ b/drivers/i3c/master/mipi-i3c-hci/core.c @@ -367,7 +367,7 @@ static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev, } static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev, - const struct i2c_msg *i2c_xfers, int nxfers) + struct i2c_msg *i2c_xfers, int nxfers) { struct i3c_master_controller *m = i2c_dev_get_master(dev); struct i3c_hci *hci = to_i3c_hci(m); diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index c1ee3828e7ee..24bd701b5de0 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -1584,7 +1584,7 @@ static int svc_i3c_master_priv_xfers(struct i3c_dev_desc *dev, } static int svc_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, - const struct i2c_msg *xfers, + struct i2c_msg *xfers, int nxfers) { struct i3c_master_controller *m = i2c_dev_get_master(dev); diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 12d532b012c5..c67922ece617 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -475,7 +475,7 @@ struct i3c_master_controller_ops { 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, - const struct i2c_msg *xfers, int nxfers); + struct i2c_msg *xfers, int nxfers); int (*request_ibi)(struct i3c_dev_desc *dev, const struct i3c_ibi_setup *req); void (*free_ibi)(struct i3c_dev_desc *dev);
The change is necessary to enable the use of the `i2c_get_dma_safe_msg_buf()` API, which requires a non-const `struct i2c_msg *` to operate. The `i2c_get_dma_safe_msg_buf()` function ensures safe handling of I2C messages when using DMA, making it essential for scenarios where DMA transfers are involved. By removing the `const` qualifier, this patch allows drivers to prepare and manage DMA-safe buffers directly. Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> --- drivers/i3c/master/dw-i3c-master.c | 2 +- drivers/i3c/master/i3c-master-cdns.c | 2 +- drivers/i3c/master/mipi-i3c-hci/core.c | 2 +- drivers/i3c/master/svc-i3c-master.c | 2 +- include/linux/i3c/master.h | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)