Date: Mon, 25 Jul 2016 15:34:01 +0200 From: Michal Meloun <mmel@FreeBSD.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org>, Svatopluk Kraus <skra@FreeBSD.org>, freebsd-arch@freebsd.org Subject: Re: INTRNG (Was: svn commit: r301453....) Message-ID: <57961549.4020105@FreeBSD.org> In-Reply-To: <f82018ee-51e7-60fa-2682-f0ef307a52b5@freebsd.org> References: <201606051620.u55GKD5S066398@repo.freebsd.org> <b9606755-69cb-2cb0-04d7-6be32e4cb89e@freebsd.org> <578E0B5D.3070105@FreeBSD.org> <e026f6fc-76ed-5dbe-00fc-365b6d7bcf94@freebsd.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <f82018ee-51e7-60fa-2682-f0ef307a52b5@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
It seems that you accidentally moved this thread to arm@, so I moved him
again to arch@.
Dne 25.07.2016 v 0:38 Nathan Whitehorn napsal(a):
>
>
> On 07/24/16 10:51, Michal Meloun wrote:
>
> [snip]
>
>>>>
>>>>>
>>>>>> For this reason we makes domain key composite, in our model, the
>>>>>> domain key consist of "domain key type"
>>>>>> and "domain key value". This composite key guarantees uniqueness
>>>>>> and it also allows to select proper parser for byte string.
>>>>>
>>>>> Yes, but this solves what is a nonexistant problem by making the
>>>>> system substantially less flexible and more invasive. Which is not
>>>>> a good tradeoff.
>>>>>
>>>> I think that existence of problem is confirmed in the above example .
>>>> Quote from previous paragraph:
>>>> "We could solve this in a number of ways, ... , or adding another
>>>> value (domain type, for example)."
>>>> What can I say more ...
>>>
>>> Except that the example you gave *is not an example* of the problem
>>> you are describing. You would only end up with a problem if:
>>> 1) You had interrupts in a GPIO property rather than in an
>>> interrupts property (or equivalent).
>>> 2) *And* you had interrupts on GPIOs in an interrupts property (or
>>> equivalent)
>>> 3) *and* those are encoded differently
>>> 4) *and* the different encodings use the same number of cells
>>> 5) *and* are not otherwise distinguishable
>>>
>>> Does that ever actually happen, in the real world, on any device
>>> tree? You could imagine any kind of messed up thing you want, but we
>>> shouldn't structure APIs around them, especially given that the
>>> current alternative you are proposing has real, concrete problems on
>>> real hardware that actually exists.
>>>
>> Allow me to respond to this issue only. I think that this main
>> issue, everything else looks resolvable for me (or, in worst case,
>> can be converted to MD code).
>>
>> from linux/arch/arm/boot/dts/am335x-evm.dts (it's first file that I'm
>> searched)
>>
>> ----------------------------------------------------------------------------------------------------------------------------
>>
>> &gpmc {
>> status = "okay";
>> pinctrl-names = "default";
>> pinctrl-0 = <&nandflash_pins_s0>;
>> ranges = <0 0 0x08000000 0x1000000>; /* CS0: 16MB for NAND */
>> nand@0,0 {
>> compatible = "ti,omap2-nand";
>> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */
>> interrupt-parent = <&gpmc>;
>> interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
>> <1 IRQ_TYPE_NONE>; /* termcount */
>> rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
>> ----------------------------------------------------------------------------------------------------------------------------
>>
>> so we have
>> rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
>> and
>> interrupt-parent = <&gpmc>;
>> interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
>>
>>
>> --- or ---
>> from linux/arch/arm/boot/dts/exynos5420-peach-pit.dts
>>
>> max98090: codec@10 {
>> compatible = "maxim,max98090";
>> reg = <0x10>;
>> interrupts = <2 0>;
>> interrupt-parent = <&gpx0>;
>> pinctrl-names = "default";
>> and
>> sleep-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>;
>>
>> also, in this case, first cell in 'interrupts' property have
>> different meaning that second cell in 'sleep-gpio'.
>> (on exynos, not all gpios supports interrrupts)
>>
>>
>> In general, binding for 'interrupts ' and '<foo>-gpios ' are
>> different, but can have same size .
>
> Thanks so much for the concrete example! This is very helpful. Three
> questions:
>
> 1. Are the drivers for these devices actually taking interrupts on the
> lines defined in the *-gpios property? It seems weird to me that there
> would be interrupts on GPIOs and, simultaneously, implicit interrupts
> on other GPIOs. It looks like the things on the *-gpios properties are
> things you set rather than things you would request an interrupt on
> (sleep for the second case, a wait pin in the first).
Yes, interrupts on GPIO pins are relatively widely used. For example,
SDHCI driver uses this for handling of card detect pin. Moreover, the
SDHCI driver have zero knowledge about given GPIO pin
interrupt capabilities. He simply tries to allocate interruption, and
when it fails, it switches to a pooling mode.
Other usage is HDMI cable/monitor detect input, USB VBUS pin or so...
> 2. What is the different meaning? I assumed the second cell in both
> cases is the GPIO pin number, because anything else would be crazy.
No, on exynos (if memory serves me), not all GPIOs have interrupt
capability, and they are numbered independently.
But again, please note that both *-gpios and interrupts properties are
defined independently, they can be formatted differently even within a
single controller.
>
> 3. What on earth is IRQ_TYPE_NONE? That's an OpenPIC flag that just
> disables the interrupt entirely. Why would you want to configure
> things that way?
In Linux DT, the IRQ_TYPE_NONE is equivalent of our
<INTR_POLARITY_CONFORM, INTR_TRIGGER_CONFORM> pair -> don't change
actual interrupt configuration, i want to reuse default or already
preset configuration (from uboot for example).
>
> (#1 is the only one that really matters here; the rest are personal
> curiosity)
>
> If the answer to (1) is "yes", there would be a good case for adding
> something like a ofw_bus_map_gpio_as_intr() or the like that takes the
> same arguments as ofw_bus_map_intr() and propagates them to the
> control in some way but with a flag. It's an easy extension and this
> is the reason the function has "ofw" in the name. My one qualm about
> that is that there is no guarantee that a GPIO controller can actually
> provide interrupts on a random GPIO (nor is it clear that that is
> actually what is wanted here). That qualm applies to both APIs, of
> course, and I still don't see a rationale for throwing away the
> existing code even if (1) is true.
But current INTRNG is no longer OFW centric, it can easily handle any
other 'configuration source'. We removed it (from INTRNG core) at
r297539, at request from MIPS folks.
But again, all what I want in this area is (for me) simple change of
your ofw_bus_map_intr() method to something like:
enum intr_map_data_type {
INTR_MAP_DATA_OFW,
INTR_MAP_DATA_GPIO,
...
};
int
bus_map_intr(device_t dev, enum intr_map_data_type type, uintptr_t
pic_id, void *config_data, int config_size)
Please, don't take current implementation of bus_map_intr() in account.
>From my current point of view, the method is badly named and his name
doesn't reflect its functionality.
Michal
>
> I've changed the mailing list here since the SVN list really isn't the
> best place. For people who want context and are seeing this for the
> first time, please see the other email I just sent.
> -Nathan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?57961549.4020105>