On Tue, 2022-03-22 at 10:57 +0100, Marek Vasut wrote: > On 3/22/22 10:49, Andy Shevchenko wrote: > > On Tue, Mar 22, 2022 at 08:43:10AM +0100, Nuno Sá wrote: > > > On Mon, 2022-03-21 at 20:46 +0100, Marek Vasut wrote: > > > > On 3/21/22 17:10, Andy Shevchenko wrote: > > > > > On Mon, Mar 21, 2022 at 03:46:51PM +0100, Marek Vasut wrote: > > > > > > On 3/21/22 11:40, Andy Shevchenko wrote: > > > > > > > On Sun, Mar 20, 2022 at 07:15:42PM +0100, Marek Vasut > > > > > > > wrote: > > > > > > > > Add runtime check to verify whether storagebits are at > > > > > > > > least > > > > > > > > as big > > > > > > > > as shifted realbits. This should help spot broken > > > > > > > > drivers > > > > > > > > which may > > > > > > > > set realbits + shift above storagebits. > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > ... > > > > > > > > > > > > > > > + /* Verify that sample bits fit > > > > > > > > into > > > > > > > > storage */ > > > > > > > > + WARN_ON(channels[i].scan_type.s > > > > > > > > torage > > > > > > > > bits < > > > > > > > > + channels[i].scan_type.r > > > > > > > > ealbit > > > > > > > > s + > > > > > > > > + channels[i].scan_type.s > > > > > > > > hift); > > > > > > > > > > > > > > Not sure WARN is a good level (it might be fatal on some > > > > > > > setups > > > > > > > and we won't that), > > > > > > > besides the fact that we may use dev_WARN(). Perhaps > > > > > > > dev_warn() > > > > > > > would suffice? > > > > > > > > > > > > I was actually thinking about BUG(), but that might crash > > > > > > existing systems. > > > > > > I think we want a strong indicator that something wrong is > > > > > > going > > > > > > on which > > > > > > must be fixed and the splat produced by WARN_ON() is a good > > > > > > indicator of > > > > > > that. It also does not crash existing systems, > > > > > > > > > > It does crash _some_ of them, unfortunately. > > > > > > > > Details please ? > > > > > > > > WARN_ON() shouldn't cause crash outright, or do I miss > > > > something ? > > > > > > Arghh, completely forgot about this... Andy is right, maybe there > > > are > > > other cases (in which case, it would be nice to share :D), but > > > this one > > > is definitely one of them: > > > > > > https://elixir.bootlin.com/linux/latest/source/kernel/panic.c#L579 > > > > > > You can have a cmdline parameter to panic on _WARN() and some > > > systems > > > may have it. > > > > Yes, I meant panic on warning. > > > > And I can't imagine that this driver can be system critical to the > > extent > > that we have to crash the system. > > Is there something which does trigger a backtrace, but without > panic()ing the system ? Nothing I'm aware... Other than directly call 'dump_stack()' - Nuno Sá
