[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAhV-H6tUvwN9UejTRf0zKXdhGG4o5X4ZOppE+1oQhL-rADFHg@mail.gmail.com>
Date: Tue, 8 Apr 2025 20:37:36 +0800
From: Huacai Chen <chenhuacai@...il.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Huacai Chen <chenhuacai@...ngson.cn>, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org, Xuefeng Li <lixuefeng@...ngson.cn>,
Jiaxun Yang <jiaxun.yang@...goat.com>, stable@...r.kernel.org,
Yinbo Zhu <zhuyinbo@...ngson.cn>
Subject: Re: [PATCH] irqchip/loongson-liointc: Support to set IRQ_TYPE_EDGE_BOTH
On Sun, Apr 6, 2025 at 10:20 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Sun, Apr 06 2025 at 20:46, Huacai Chen wrote:
> > On Sun, Apr 6, 2025 at 6:18 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> >> On Sun, Apr 06 2025 at 17:46, Huacai Chen wrote:
> >> > On Thu, Apr 3, 2025 at 11:48 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> >> >> But it won't trigger on both. So no, you cannot claim that this fixes
> >> >> anything.
> >> > Yes, it won't trigger on both (not perfect), but it allows drivers
> >> > that request "both" work (better than fail to request), and there are
> >>
> >> By some definition of 'work'. There is probably a good technical reason
> >> why those drivers expect EDGE_BOTH to work correctly and otherwise fail
> >> to load.
> > The real problem we encounter is the MMC driver. In
> > drivers/mmc/core/slot-gpio.c there is
> > devm_request_threaded_irq(host->parent, irq,
> > NULL, ctx->cd_gpio_isr,
> > IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
> > IRQF_ONESHOT,
> > ctx->cd_label, host);
> >
> > "IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING" is an alias of
> > "IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING", and
> > "IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING" is
> > "IRQ_TYPE_EDGE_BOTH".
>
> I know that.
>
> > Except MMC, "grep IRQ_TYPE_EDGE_BOTH drivers" can give some more examples.
>
> Sure, but you still do not explain why this works even when the driver
> is obviously depending on EDGE_BOTH. If it does not then the driver is
> bogus.
>
> Looking at it, there is obviously a reason for this particular driver to
> request BOTH. Why?
>
> This is the card change detection and it uses a GPIO. Insert raises one
> edge and remove the opposite one.
>
> Which means whatever edge you chose randomly the detection will only
> work in one direction. Please don't tell me that this is correct by any
> meaning of correct. It's not.
>From experiments, either setting to EDGE_RISING or EDGE_FALLING, card
detection (inserting and removing) works. Maybe the driver request
"BOTH", but it really need "ANY"? I've searched git log, but I haven't
get any useful information.
Huacai
>
> The driver is perfectly fine, when the request fails. It then does the
> obvious right thing to poll the card detection pin.
>
> So your change makes it worse as it screws up the detection mechanism.
>
> What are you actually making "work"?
>
> Thanks,
>
> tglx
Powered by blists - more mailing lists