Date: Mon, 25 Jul 2016 07:05:02 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Michal Meloun <mmel@FreeBSD.org>, Svatopluk Kraus <skra@FreeBSD.org>, freebsd-arch@FreeBSD.org, freebsd-arm@freebsd.org Subject: Re: INTRNG (Was: svn commit: r301453....) Message-ID: <e2cace17-0924-2084-5fcf-626f87e41cc3@freebsd.org> In-Reply-To: <57961549.4020105@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> <57961549.4020105@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 07/25/16 06:34, Michal Meloun wrote:
> It seems that you accidentally moved this thread to arm@, so I moved him
> again to arch@.
(Thanks -- I meant it to be both arm@ and arch@, like the other email)
> 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...
That wasn't my question. Are these particular device drivers allocating
interrupts both on the GPIOs in their "interrupts" property (which are
entirely GPIOs in this example) *and* on the GPIOs listed as resources
but not listed as interrupts? If they are, then you need a switching
mechanism, but that seems pretty unlikely given the names of the
non-interrupt GPIOs (they look like outputs). It would also be a
somewhat deranged way to set up a device tree -- not that that rules it
out or anything.
>> 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.
Which is totally fine. The question is if you need to set up interrupts
on the GPIOs both the "interrupts" and non-interrupts properties, which
is easy enough to handle, but kind of weird.
>
>> 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).
It's not a Linux thing, it's from the OpenPIC spec. Probably it just
means the interrupt isn't configurable, no that it matters.
>
>> (#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.
Fair enough. My core issue is basically that this code connects
interrupts to the newbus hierarchy, which simply doesn't work in a lot
of cases when the newbus hierarchy doesn't match the interrupt one,
which is quite common (see my other mail). Unfortunately, that's a
fundamental architectural issue with r301453 and so I'm not sure how to
proceed here.
The current mechanism I was discussing is not OFW-centric either,
although it is true that the only current mapping method
(ofw_bus_map_intr()) is (though it can be abused for non-OFW data
sources successfully), but you could easily add an acpi_bus_map_intr()
or whatever that takes different data and connects in the MD interrupt
code. I have no objections to that at all. I'm still confused about how
your GPIO interrupts work (above), but that's a detail. Is that
something that you think would be workable? I'd be more than happy to
write some prototype code.
-Nathan
>
> 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?e2cace17-0924-2084-5fcf-626f87e41cc3>