From: Guenter Roeck <linux@roeck-us.net>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: Jean Delvare <jdelvare@suse.com>,
linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org
Subject: Re: [PATCH v1] hwmon: (lm90) Use edge-triggered interrupt
Date: Thu, 17 Jun 2021 08:12:36 -0700 [thread overview]
Message-ID: <20210617151236.GB2676642@roeck-us.net> (raw)
In-Reply-To: <bc3e3595-fe10-c7ae-9560-0c7676facba2@gmail.com>
On Thu, Jun 17, 2021 at 05:46:33PM +0300, Dmitry Osipenko wrote:
> 17.06.2021 17:13, Guenter Roeck пишет:
> ...
> >> This is a device-tree based system, in particular it's NVIDIA Tegra30
> >> Nexus 7. The interrupt support was originally added to the lm90 driver
> >> by Wei Ni who works at NVIDIA and did it for the Tegra boards. The Tegra
> >> device-trees are specifying the trigger mask and apparently they all are
> >> cargo-culted and wrong because they use IRQ_TYPE_LEVEL_HIGH, while it
> >
> > Be fair, no one is perfect.
>
> This is a very minor problem, so no wonder that nobody noticed or
> bothered to fix it yet. I'm just clarifying the status here.
>
> >> should be IRQ_TYPE_EDGE_FALLING.
> >
> > It should probably be both IRQ_TYPE_EDGE_FALLING and IRQ_TYPE_EDGE_RISING,
>
> For now I see that the rising edge isn't needed, the TEMP_ALERT goes
> HIGH by itself when temperature backs to normal. But I will try to
> double check.
>
The point is that a sysfs event should be sent to userspace on both
edges, not only when an alarm is raised. But, you are correct,
IRQ_TYPE_EDGE_RISING is currently not needed since sysfs events
are not generated.
> > and the interrupt handler should call hwmon_notify_event() instead of
> > clogging the kernel log, but that should be done in a separate patch.
>
> Thank you for suggestion, I will take a look.
>
> > Anyway, the tegra30 dts files in the upstream kernel either use
> > IRQ_TYPE_LEVEL_LOW or no interrupts for nct1008. The Nexus 7 dts file
> > in the upstream kernel has no interrupt configured (and coincidentally
> > it was you who added that entry). Where do you see IRQ_TYPE_LEVEL_HIGH ?
>
> I have a patch that will add the interrupt property, it's stashed
> locally for the next kernel release.
>
> IIUC, it's not only the Tegra30 dts, but all the TegraXXX boards that
> use IRQ_TYPE_LEVEL_LOW are in the same position.
I still don't see a IRQ_TYPE_LEVEL_HIGH, though.
Thanks,
Guenter
>
> >> The IRQF flag in devm_request_threaded_irq() overrides the trigger mask
> >> specified in a device-tree. IIUC, the interrupt is used only by OF-based
> >> devices, hence I think we could simply remove the IRQF flag from the
> >> code and fix the device-trees. Does it sound good to you?
> >
> > Yes, that is a better approach.
>
> Thank you for reviewing this patch. I'll prepare v2.
next prev parent reply other threads:[~2021-06-17 15:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-16 19:07 [PATCH v1] hwmon: (lm90) Use edge-triggered interrupt Dmitry Osipenko
2021-06-17 0:12 ` Guenter Roeck
2021-06-17 7:11 ` Dmitry Osipenko
2021-06-17 13:12 ` Guenter Roeck
2021-06-17 13:48 ` Dmitry Osipenko
2021-06-17 14:13 ` Guenter Roeck
2021-06-17 14:46 ` Dmitry Osipenko
2021-06-17 15:12 ` Guenter Roeck [this message]
2021-06-17 15:27 ` Dmitry Osipenko
2021-06-17 21:42 ` Guenter Roeck
2021-06-18 8:55 ` Dmitry Osipenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210617151236.GB2676642@roeck-us.net \
--to=linux@roeck-us.net \
--cc=digetx@gmail.com \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.