[PATCH v2 0/3] staging: most: replace BUG_ON() with proper error handling

[PATCH v2 0/3] staging: most: replace BUG_ON() with proper error handling

[Olle Lukowski]

This small series replaces BUG_ON() calls in three staging MOST drivers
with appropriate error handling.

The affected drivers are:

  - drivers/staging/most/i2c/i2c.c
  - drivers/staging/most/dim2/dim2.c
  - drivers/staging/most/video/video.c

All changes are mechanical and do not alter runtime behavior, except that
the kernel won't panic if a driver invariant is violated.

Tested with checkpatch.pl and compile-tested (make M=drivers/staging/most).

Signed-off-by: Olle Lukowski <olle@lukowski.dev>
---
Changes in v2:
- Drop WARN_ON_ONCE() per review (Greg KH). Many systems enable
  panic_on_warn; use plain checks + error returns instead.
- Link to v1: https://lore.kernel.org/r/20251021-staging-most-warn-v1-0-4cdd3745bbdc@lukowski.dev

---
Olle Lukowski (3):
      staging: most: i2c: replace BUG_ON() with proper checks and error returns
      staging: most: dim2: replace BUG_ON() with proper checks and error returns
      staging: most: video: replace BUG_ON() with proper check

 drivers/staging/most/dim2/dim2.c   | 27 +++++++++++++++++++--------
 drivers/staging/most/i2c/i2c.c     |  9 ++++++---
 drivers/staging/most/video/video.c |  3 ++-
 3 files changed, 27 insertions(+), 12 deletions(-)
---
base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
change-id: 20251021-staging-most-warn-51deb2e454aa

Best regards,
-- 
Olle Lukowski <olle@lukowski.dev>

[PATCH v2 1/3] staging: most: i2c: replace BUG_ON() with proper checks and error returns

[Olle Lukowski]

Replace BUG_ON() checks for invalid channel indices with proper checks
and return -EINVAL to avoid crashing the kernel unnecessarily.

Signed-off-by: Olle Lukowski <olle@lukowski.dev>
---
 drivers/staging/most/i2c/i2c.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index 184b2dd11..db806b00d 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -71,7 +71,8 @@ static int configure_channel(struct most_interface *most_iface,
 	struct hdm_i2c *dev = to_hdm(most_iface);
 	unsigned int delay, pr;
 
-	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
+	if (ch_idx < 0 || ch_idx >= NUM_CHANNELS)
+		return -EINVAL;
 
 	if (channel_config->data_type != MOST_CH_CONTROL) {
 		pr_err("bad data type for channel %d\n", ch_idx);
@@ -125,7 +126,8 @@ static int enqueue(struct most_interface *most_iface,
 	struct hdm_i2c *dev = to_hdm(most_iface);
 	int ret;
 
-	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
+	if (ch_idx < 0 || ch_idx >= NUM_CHANNELS)
+		return -EINVAL;
 
 	if (ch_idx == CH_RX) {
 		/* RX */
@@ -170,7 +172,8 @@ static int poison_channel(struct most_interface *most_iface,
 	struct hdm_i2c *dev = to_hdm(most_iface);
 	struct mbo *mbo;
 
-	BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS);
+	if (ch_idx < 0 || ch_idx >= NUM_CHANNELS)
+		return -EINVAL;
 
 	if (ch_idx == CH_RX) {
 		if (!polling_rate)

-- 
2.51.1

[PATCH v2 2/3] staging: most: dim2: replace BUG_ON() with proper checks and error returns

[Olle Lukowski]

Replace BUG_ON() calls with proper checks to prevent unnecessary kernel
panics. Return appropriate error codes (-EINVAL or -EFAULT) instead of
crashing the system.

Signed-off-by: Olle Lukowski <olle@lukowski.dev>
---
 drivers/staging/most/dim2/dim2.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index dad2abe6c..d0832704b 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -166,8 +166,10 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch)
 	unsigned long flags;
 	struct dim_ch_state st;
 
-	BUG_ON(!hdm_ch);
-	BUG_ON(!hdm_ch->is_initialized);
+	if (!hdm_ch)
+		return -EINVAL;
+	if (!hdm_ch->is_initialized)
+		return -EINVAL;
 
 	spin_lock_irqsave(&dim_lock, flags);
 	if (list_empty(head)) {
@@ -188,7 +190,11 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch)
 		return -EAGAIN;
 	}
 
-	BUG_ON(mbo->bus_address == 0);
+	if (mbo->bus_address == 0) {
+		spin_unlock_irqrestore(&dim_lock, flags);
+		return -EFAULT;
+	}
+
 	if (!dim_enqueue_buffer(&hdm_ch->ch, mbo->bus_address, buf_size)) {
 		list_del(head->next);
 		spin_unlock_irqrestore(&dim_lock, flags);
@@ -269,8 +275,10 @@ static void service_done_flag(struct dim2_hdm *dev, int ch_idx)
 	unsigned long flags;
 	u8 *data;
 
-	BUG_ON(!hdm_ch);
-	BUG_ON(!hdm_ch->is_initialized);
+	if (!hdm_ch)
+		return;
+	if (!hdm_ch->is_initialized)
+		return;
 
 	spin_lock_irqsave(&dim_lock, flags);
 
@@ -455,7 +463,8 @@ static int configure_channel(struct most_interface *most_iface, int ch_idx,
 	int const ch_addr = ch_idx * 2 + 2;
 	struct hdm_channel *const hdm_ch = dev->hch + ch_idx;
 
-	BUG_ON(ch_idx < 0 || ch_idx >= DMA_CHANNELS);
+	if (ch_idx < 0 || ch_idx >= DMA_CHANNELS)
+		return -EINVAL;
 
 	if (hdm_ch->is_initialized)
 		return -EPERM;
@@ -567,7 +576,8 @@ static int enqueue(struct most_interface *most_iface, int ch_idx,
 	struct hdm_channel *hdm_ch = dev->hch + ch_idx;
 	unsigned long flags;
 
-	BUG_ON(ch_idx < 0 || ch_idx >= DMA_CHANNELS);
+	if (ch_idx < 0 || ch_idx >= DMA_CHANNELS)
+		return -EINVAL;
 
 	if (!hdm_ch->is_initialized)
 		return -EPERM;
@@ -643,7 +653,8 @@ static int poison_channel(struct most_interface *most_iface, int ch_idx)
 	u8 hal_ret;
 	int ret = 0;
 
-	BUG_ON(ch_idx < 0 || ch_idx >= DMA_CHANNELS);
+	if (ch_idx < 0 || ch_idx >= DMA_CHANNELS)
+		return -EINVAL;
 
 	if (!hdm_ch->is_initialized)
 		return -EPERM;

-- 
2.51.1

[PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check

[Olle Lukowski]

Replace a BUG_ON() call with a proper check to prevent an unnecessary
kernel panic.

Signed-off-by: Olle Lukowski <olle@lukowski.dev>
---
 drivers/staging/most/video/video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 32f71d9a9..3a7ee2791 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -575,7 +575,8 @@ static void __exit comp_exit(void)
 
 	most_deregister_configfs_subsys(&comp);
 	most_deregister_component(&comp);
-	BUG_ON(!list_empty(&video_devices));
+	if (!list_empty(&video_devices))
+		pr_err("video_devices list not empty during exit\n");
 }
 
 module_init(comp_init);

-- 
2.51.1

Re: [PATCH v2 2/3] staging: most: dim2: replace BUG_ON() with proper checks and error returns

[Greg Kroah-Hartman]

On Tue, Oct 21, 2025 at 04:09:29PM +0300, Olle Lukowski wrote:
> Replace BUG_ON() calls with proper checks to prevent unnecessary kernel
> panics. Return appropriate error codes (-EINVAL or -EFAULT) instead of
> crashing the system.
> 
> Signed-off-by: Olle Lukowski <olle@lukowski.dev>
> ---
>  drivers/staging/most/dim2/dim2.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
> index dad2abe6c..d0832704b 100644
> --- a/drivers/staging/most/dim2/dim2.c
> +++ b/drivers/staging/most/dim2/dim2.c
> @@ -166,8 +166,10 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch)
>  	unsigned long flags;
>  	struct dim_ch_state st;
>  
> -	BUG_ON(!hdm_ch);
> -	BUG_ON(!hdm_ch->is_initialized);
> +	if (!hdm_ch)
> +		return -EINVAL;
> +	if (!hdm_ch->is_initialized)
> +		return -EINVAL;

Can these things ever actually happen?  In looking at the code, I don't
see how that could be, do you?

Let's not check for things that are impossible to ever hit.

Same with the other changes in this series, please verify that these are
actually possible to happen.  If not, then just remove the check.

>  	spin_lock_irqsave(&dim_lock, flags);
>  	if (list_empty(head)) {
> @@ -188,7 +190,11 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch)
>  		return -EAGAIN;
>  	}
>  
> -	BUG_ON(mbo->bus_address == 0);
> +	if (mbo->bus_address == 0) {
> +		spin_unlock_irqrestore(&dim_lock, flags);
> +		return -EFAULT;

You need to do more than just that here :(

Please be very careful on error paths to properly clean up everything.

thanks,

greg k-h

Re: [PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check

[Greg Kroah-Hartman]

On Tue, Oct 21, 2025 at 04:09:30PM +0300, Olle Lukowski wrote:
> Replace a BUG_ON() call with a proper check to prevent an unnecessary
> kernel panic.
> 
> Signed-off-by: Olle Lukowski <olle@lukowski.dev>
> ---
>  drivers/staging/most/video/video.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
> index 32f71d9a9..3a7ee2791 100644
> --- a/drivers/staging/most/video/video.c
> +++ b/drivers/staging/most/video/video.c
> @@ -575,7 +575,8 @@ static void __exit comp_exit(void)
>  
>  	most_deregister_configfs_subsys(&comp);
>  	most_deregister_component(&comp);
> -	BUG_ON(!list_empty(&video_devices));
> +	if (!list_empty(&video_devices))
> +		pr_err("video_devices list not empty during exit\n");

Is this possible to ever have happen?

thanks,

greg k-h