Received-SPF: pass (google.com: domain of linux-kernel+bounces-131028-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <f7d454ac-6ecb-4431-a1de-c9b5d1240969@gmail.com>
Date: Thu, 4 Apr 2024 10:26:34 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH 0/6] Support ROHM BD96801 scalable PMIC
Content-Language: en-US, en-GB
To: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc: Lee Jones <lee@kernel.org>, Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Liam Girdwood <lgirdwood@gmail.com>,
 Mark Brown <broonie@kernel.org>, Wim Van Sebroeck <wim@linux-watchdog.org>,
 Guenter Roeck <linux@roeck-us.net>, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org
References: <cover.1712058690.git.mazziesaccount@gmail.com>
From: Matti Vaittinen <mazziesaccount@gmail.com>
In-Reply-To: <cover.1712058690.git.mazziesaccount@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 4/2/24 16:07, Matti Vaittinen wrote:
> Another "oddity" is that the PMIC has two physical IRQ lines. When I
> last wrote this patch in 2021 I had some naming collison in debugfs for
> the IRQ domains. Back then I used:
> irq_domain_update_bus_token(intb_domain, DOMAIN_BUS_WIRED);
> to work-around the issue. Now, when rebasing to v6.9-rc1 the naming
> collision was gone and things seemed to work. However, it'd be great if
> the IRQ code in MFD driver was reviewed by greater minds :)

It appears my statement "things seemed to work" is a bit too optimistic. 
I am afraid my approach of having two separate IRQ domains for one 
device (and DT-node) is just somehow fundamentally wrong. It'd be great 
to learn what's the correct "ideology" here.

It appears the naming collision is still there. My config just had the 
CONFIG_GENERIC_IRQ_DEBUGFS disabled. Enabling it shows the same naming 
collison:
debugfs: File 
':ocp:interconnect@48000000:segment@100000:target-module@9c000:i2c@0:pmic@60' 
in directory 'domains' already present!

If I'm not mistaken the debugfs file name is generated from the 
device-tree node path+name. This is a subtle hint that it is not 
expected there are more than 1 IRQ-domain / device. I guess this kind of 
makes sense if we can have more than 1 HWIRQ handled by a single domain 
(I don't recall having to ever write such domain/IRQ-controller before, 
but I think it should be possible).

I have now 3 new questions =)

1. Should we be able to have more than 1 IRQ domain / device?
2. Should regmap_irq support having more than 1 HWIRQ
3. If answer to 1 is "no" - should we protect against this somehow? (see 
why below).

When CONFIG_GENERIC_IRQ_DEBUGFS is disabled, adding the two IRQ 
controllers with own IRQ domains (intb and errb here) to a single device 
is seemingly successful. I see no complaints / errors. Also, most of the 
IRQs seem to work - but not all. In my case trying to issue:

cat /proc/interrupts

will oops. Also, looking in the /sys/kernel/irq/ lists folders for all 
the "intb" and "errb" IRQs - but reading the files contained in these 
directories will cause an oops for all "errb" interrupts except for the 
first 16.

Finally, if I use the
irq_domain_update_bus_token(intb_domain, DOMAIN_BUS_WIRED);

to add "-1" at the end of the "intb" - domain name resulting domains:

:ocp:interconnect@48000000:segment@100000:target-module@9c000:i2c@0:pmic@60
:ocp:interconnect@48000000:segment@100000:target-module@9c000:i2c@0:pmic@60-1

then it seems that reading the IRQ information from the /proc/interrupts 
works as expected. Here I am making a wild guess that the name of the 
domain is used as a key for some data-lookups, and having two domains 
with a same name will either overwrite something or cause wrong domain 
data to be fetched. (This is just guessing for now).

Any tips, hints or thoughts on this?

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~