diff mbox series

[v2,1/2] i3c: Remove the const qualifier from i2c_msg pointer in i2c_xfers API

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

Commit Message

Billy Tsai Feb. 4, 2025, 9:17 a.m. UTC
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(-)

Comments

Thomas Weißschuh Feb. 4, 2025, 9:50 a.m. UTC | #1
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/
Billy Tsai Feb. 4, 2025, 10:09 a.m. UTC | #2
> 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.
Thomas Weißschuh Feb. 4, 2025, 4:50 p.m. UTC | #3
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
Alexandre Belloni Feb. 4, 2025, 4:56 p.m. UTC | #4
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.
Frank Li Feb. 5, 2025, 4:50 p.m. UTC | #5
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 mbox series

Patch

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