lkml.org 
[lkml]   [2021]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH v3 3/7] regulator: IRQ based event/error notification helpers
On Thu, Mar 11, 2021 at 12:22:36PM +0200, Matti Vaittinen wrote:

> @@ -0,0 +1,423 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 ROHM Semiconductors

Please make the entire comment a C++ one so things look more consistent.

> +static void regulator_notifier_isr_work(struct work_struct *work)
> +{

> + if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
> + if (d->die)
> + ret = d->die(rid);
> + else
> + BUG();
> +
> + /*
> + * If the 'last resort' IC recovery failed we will have
> + * nothing else left to do...
> + */
> + BUG_ON(ret);

This isn't good... we should be trying to provide more system level
handling of this, if nothing else it's quite possibly not a software bug
here but rather a hardware failure. An explicit message about what
happened would be more likely to be understood as a hardware failure,
and something which allows handling such as initiating a system shutdown
would be good as well - I'm not sure if there's any existing mechanism
to plumb userspace into, or perhaps some sort of policy configurable via
sysfs. That could be built on later though, I think the main thing here
is that the logging should be clearer and distinguishable from a random
software fault which is what BUG_ON() looks like. The backtrace and
whatnot that BUG_ON() provides aren't useful here and the message isn't
going to be very distinctive, some custom prints will attract more
attention.

> + /* Disable IRQ if HW keeps line asserted */
> + if (d->irq_off_ms)
> + disable_irq_nosync(irq);
> + /*
> + * IRQ seems to be for us. Let's fire correct notifiers / store error

Missing blank lines in the file.

> + * This structure is passed to map_event and renable for reporting reulator

regulator.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-04-02 19:14    [W:0.109 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site