linux-kernel - Re: [PATCH v9 05/10] regulator: IRQ based event/error notification helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4d980efbec6a63d3241c14c7afdbaf14459d3fb3.camel@gmail.com>
Date:   Tue, 11 May 2021 07:54:22 +0300
From:   Matti Vaittinen <mazziesaccount@...il.com>
To:     Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc:     Mark Brown <broonie@...nel.org>, Kees Cook <keescook@...omium.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Zhang Rui <rui.zhang@...el.com>,
        Guenter Roeck <linux@...ck-us.net>,
        "agross@...nel.org" <agross@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        linux-power <linux-power@...rohmeurope.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>
Subject: Re: [PATCH v9 05/10] regulator: IRQ based event/error notification
 helpers


On Mon, 2021-05-10 at 14:29 +0300, Matti Vaittinen wrote:
> Provide helper function for IC's implementing regulator notifications
> when an IRQ fires. The helper also works for IRQs which can not be
> acked.
> Helper can be set to disable the IRQ at handler and then re-enabling
> it
> on delayed work later. The helper also adds
> regulator_get_error_flags()
> errors in cache for the duration of IRQ disabling.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> 
> 
> +static irqreturn_t regulator_notifier_isr(int irq, void *data)
> +{

//snip

> +	/*
> +	 * IRQ seems to be for us. Let's fire correct notifiers / store
> error
> +	 * flags
> +	 */
> +	for_each_set_bit(i, &rdev_map, num_rdevs) {
> +		unsigned long evt;
> +		struct regulator_err_state *stat;
> +		struct regulator_dev *rdev;
> +
> +		stat = &rid->states[i];
> +		rdev = stat->rdev;
> +
> +		for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat-
> >notifs)) {
> +			evt =  BIT(j);
> +			pr_dbg("Sending regulator notification EVT
> 0x%lx\r\n",
> +			       stat->notifs, evt);
> +			regulator_notifier_call_chain(rdev, evt, NULL);
> +		}

This construct sends own notification for each of the event flagged by
the driver. My thinking was that sending each event separately ensures
all of them are handled. OTOH, the comment in the even description
states:

> * NOTE: These events can be OR'ed together when passed into handler.

So... Should I actually simplify this and just punt out all the
stat->notifs in one event? That would get rid of this one extra loop.

> +		rdev_flag_err(rdev, stat->errors);
> +	}

Best Regards
	Matti Vaittinen