[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<TY2PR01MB3322181DE9D456B3255756BCCD9D2@TY2PR01MB3322.jpnprd01.prod.outlook.com>
Date: Thu, 5 Sep 2024 22:34:18 +0800
From: Zhang Ning <zhangn1985@...look.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org,
linux-kernel@...r.kernel.org, lee@...nel.org
Subject: Re: mfd: intel_soc_pmic_bxtwc: irq 0 issue, tmu and typec components
fail to probe.
On Thu, Sep 05, 2024 at 03:33:14PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 05, 2024 at 07:27:25PM +0800, Zhang Ning wrote:
> > On Wed, Sep 04, 2024 at 05:36:35PM +0300, Andy Shevchenko wrote:
> > > On Wed, Sep 4, 2024 at 5:29 PM Zhang Ning <zhangn1985@...look.com> wrote:
> > > > On Fri, Aug 09, 2024 at 05:09:49PM +0300, Andy Shevchenko wrote:
> > > > > On Fri, Aug 09, 2024 at 08:53:24PM +0800, Zhang Ning wrote:
> > > > > > On Fri, Aug 09, 2024 at 03:33:33PM +0300, Andy Shevchenko wrote:
> > > > > > > On Fri, Aug 09, 2024 at 08:02:43PM +0800, Zhang Ning wrote:
> > > > > > > > Hi, Greg & Rafael
> > > > > > > >
> > > > > > > > recently, when I try to enable mfd components for intel_soc_pmic_bxtwc
> > > > > > > > for debian kernel[0]. I find tmu and typec failed to probe.
> > > > > > > >
> > > > > > > > after check source code, I find irq for these two devices are 0, when
> > > > > > > > use platform_get_irq, it will alway fail.
> > > > > > > >
> > > > > > > > if (WARN(!ret, "0 is an invalid IRQ number\n"))
> > > > > > > > return -EINVAL;
> > > > > > > > return ret;
> > > > > > > >
> > > > > > > > My workaround for debian is to hardcode irq to 0, instead to use api.
> > > > > > > >
> > > > > > > > I don't know how to write a good solution, thus send an email to you.
> > > > > > >
> > > > > > > Hold on, how the heck you got 0 in the first place?A
> > > > > >
> > > > > > use tmu as an example
> > > > > >
> > > > > > enum bxtwc_irqs_tmu {
> > > > > > BXTWC_TMU_IRQ = 0,
> > > > > > };
> > > > > >
> > > > > > static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
> > > > > > REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, GENMASK(2, 1)),
> > > > > > };
> > > > > >
> > > > > > static const struct resource tmu_resources[] = {
> > > > > > DEFINE_RES_IRQ_NAMED(BXTWC_TMU_IRQ, "TMU"),
> > > > > > };
> > > > > >
> > > > > > {
> > > > > > .name = "bxt_wcove_tmu",
> > > > > > .num_resources = ARRAY_SIZE(tmu_resources),
> > > > > > .resources = tmu_resources,
> > > > > > },
> > > > > >
> > > > > > this is why I got 0, and I don't do any hack.
> > > > >
> > > > > Thanks for elaboration, I will look at this a bit later (may be next or one
> > > > > after next week, just returned from vacations).
> > >
> > > > could you share the patch link to the fix? then I could port it to
> > > > debian.
> > >
> > > Sorry I was busy with other stuff (as well), I am still in the middle
> > > of debugging that.
> > > The issue is that the leaf drivers for some reason do not request
> > > proper vIRQ (that should come from the secondary IRQ chip). OTOH there
> > > is only one _similar_ design in the kernel besides this one. And when
> > > I tried to test the version where all this appears, I couldn't boot
> > > (yeah, I spent some time on bisecting things) the board I have (One of
> > > pre-production variants of Intel Joule SoM).
> >
> > Yes, me too. I'm trying to enable Joule on Debian. thus found this
> > issue. you can use debian sid with linux 6.11 to debug this issue.
> >
> > and another issue is Joule HDA pci id is removed from Linux kernel, thus
> > no sound, but I don't plan to submit an issue.
> >
> > > Do you have any (most recent) kernel version that works as expected?
> > I don't try any old kernel, but from git log, I think bad commit is:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=57129044f5044dcd73c22d91491906104bd331fd`
>
> No, it does the right thing from architectural point of view. It might be that
> it was never tested or it was a regression somewhere. That's why I wanted to find
> the newest possible kernel that works on that machine.
from the first point btx pmic driver merged to mainline kernel:
enum bxtwc_irqs_level2 {
/* Level 2 */
BXTWC_THRM0_IRQ = 0,
BXTWC_THRM1_IRQ,
BXTWC_THRM2_IRQ,
BXTWC_BCU_IRQ,
BXTWC_ADC_IRQ,
BXTWC_CHGR0_IRQ,
BXTWC_CHGR1_IRQ,
BXTWC_GPIO0_IRQ,
BXTWC_GPIO1_IRQ,
BXTWC_CRIT_IRQ,
};
static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
REGMAP_IRQ_REG(BXTWC_THRM0_IRQ, 0, 0xff),
REGMAP_IRQ_REG(BXTWC_THRM1_IRQ, 1, 0xbf),
static struct resource thermal_resources[] = {
DEFINE_RES_IRQ(BXTWC_THRM0_IRQ),
DEFINE_RES_IRQ(BXTWC_THRM1_IRQ),
DEFINE_RES_IRQ(BXTWC_THRM2_IRQ),
};
it would meet irq 0 issue.
another Intel pmic driver: intel_soc_pmic_chtdc_ti.c
enum {
CHTDC_TI_PWRBTN = 0, /* power button */
CHTDC_TI_DIETMPWARN, /* thermal */
CHTDC_TI_ADCCMPL, /* ADC */
/* No IRQ 3 */
CHTDC_TI_VBATLOW = 4, /* battery */
CHTDC_TI_VBUSDET, /* power source */
/* No IRQ 6 */
CHTDC_TI_CCEOCAL = 7, /* battery */
};
static const struct resource power_button_resources[] = {
DEFINE_RES_IRQ(CHTDC_TI_PWRBTN),
};
static struct mfd_cell chtdc_ti_dev[] = {
{
.name = "chtdc_ti_pwrbtn",
.num_resources = ARRAY_SIZE(power_button_resources),
.resources = power_button_resources,
}, {
#define DEFINE_RES_IRQ_NAMED(_irq, _name) \
DEFINE_RES_NAMED((_irq), 1, (_name), IORESOURCE_IRQ)
#define DEFINE_RES_IRQ(_irq) \
DEFINE_RES_IRQ_NAMED((_irq), NULL)
in this driver, CHTDC_TI_PWRBTN is 0, thus chtdc_ti_pwrbtn will also got
irq 0 issue.
I think it's very easy to find lot of drivers have irq 0 issue, not only
intel drivers.
wm8994 driver as example:
in wm8994-irq.c
static const struct regmap_irq wm8994_irqs[] = {
[WM8994_IRQ_TEMP_SHUT] = {
.reg_offset = 1,
.mask = WM8994_TEMP_SHUT_EINT,
},
WM8994_IRQ_TEMP_SHUT is 0
static const struct resource wm8994_codec_resources[] = {
{
.start = WM8994_IRQ_TEMP_SHUT,
.end = WM8994_IRQ_TEMP_WARN,
.flags = IORESOURCE_IRQ,
},
};
static const struct mfd_cell wm8994_devs[] = {
{
.name = "wm8994-codec",
.num_resources = ARRAY_SIZE(wm8994_codec_resources),
.resources = wm8994_codec_resources,
},
in the wm8994 codec driver:
wm8994_request_irq(wm8994->wm8994, WM8994_IRQ_TEMP_SHUT,
wm8994_temp_shut, "Thermal shutdown", component);
static inline int wm8994_request_irq(struct wm8994 *wm8994, int irq,
irq_handler_t handler, const char *name,
void *data)
{
if (!wm8994->irq_data)
return -EINVAL;
return request_threaded_irq(regmap_irq_get_virq(wm8994->irq_data, irq),
NULL, handler, IRQF_TRIGGER_RISING, name,
data);
}
lucky is wm8994 codec driver doesn't catch the return value.
>
> > > > > > > > [0]: https://salsa.debian.org/kernel-team/linux/-/merge_requests/1156/diffs
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Powered by blists - more mailing lists