Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <4d980efbec6a63d3241c14c7afdbaf14459d3fb3.camel@gmail.com>
Subject: Re: [PATCH v9 05/10] regulator: IRQ based event/error notification
 helpers
From:   Matti Vaittinen <mazziesaccount@gmail.com>
To:     Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc:     Mark Brown <broonie@kernel.org>, Kees Cook <keescook@chromium.org>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        Zhang Rui <rui.zhang@intel.com>,
        Guenter Roeck <linux@roeck-us.net>,
        "agross@kernel.org" <agross@kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        linux-power <linux-power@fi.rohmeurope.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>,
        "linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
        "bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
        "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>
Date:   Tue, 11 May 2021 07:54:22 +0300
In-Reply-To: <a22cf56239512f52ae5927f226e79d890d7a1240.1620645507.git.matti.vaittinen@fi.rohmeurope.com>
References: <cover.1620645507.git.matti.vaittinen@fi.rohmeurope.com>
         <a22cf56239512f52ae5927f226e79d890d7a1240.1620645507.git.matti.vaittinen@fi.rohmeurope.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 (3.34.4-1.fc31) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk


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@fi.rohmeurope.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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