libv has joined #linux-exynos
libv has quit [Read error: Connection reset by peer]
libv_ has quit [Ping timeout: 250 seconds]
libv has joined #linux-exynos
libv has quit [Read error: Connection reset by peer]
libv has joined #linux-exynos
libv has quit [Read error: Connection reset by peer]
libv has joined #linux-exynos
MrBIOS has joined #linux-exynos
MrBIOS has quit [Quit: MrBIOS]
tilmann has joined #linux-exynos
dlezcano has joined #linux-exynos
<sjoerd> mdrjr: Hey, we're thinking about getting some XU3 boards for 4K fun and games, I was wondiring if you could tell with which monitors the 4K output has been tested
Wizzup has quit [Ping timeout: 250 seconds]
liquidAcid has joined #linux-exynos
Wizzup has joined #linux-exynos
<tfiga> javier__: ping
<javier__> tfiga: hi, I'm quite confused about the irq trigger type issue I'm having...
<tfiga> are you testing this on linux-next?
<mdrjr> sjoerd: hmm.. some Samsung 28" display.. I can't remember exactly the model
<javier__> tfiga: yes, well is yesterday's next when the atmel patches were initially posted
<tfiga> well, I (re)wrote most of the pinctrl-exynos IRQ handling code some time ago and I can't think of any reason why exynos_irq_set_type() wouldn't work
<tfiga> as long as the type argument is valid it always does whatever it should
<tfiga> I'm rechecking the code right now, though
<javier__> tfiga: yes, that's my understanding as well from reading the code and as I said on my latest email all the variables on exynos_irq_set_type() have the same value
<tfiga> the only thing that comes to my mind
<tfiga> is that somehow reconfigures the pin back to GPIO
<tfiga> something somehow*
<javier__> tfiga: exactly, that's my suspicious as well
<javier__> tfiga: that or maybe is not configured before OF parses the "interrupt" propery
<javier__> *not properly muxed
<tfiga> actually I had a patch to prevent this, but I found a better way to do that and in the middle of implementing it got preempted with other things to do
<javier__> tfiga: any hints will be highly appreciated :)
<tfiga> care to try this patch?
<tfiga> I wonder if it still applies cleanly, though
<javier__> tfiga: don't worry if it does not apply, I'll rebase it
<javier__> tfiga: I did exactly the same for the gpio omap driver on 2f56e0a ("gpio/omap: use gpiolib API to mark a GPIO used as an IRQ") so I get your logic ;)
<tfiga> although gpio_lock_as_irq() doesn't fully do what I wanted with that patch
<tfiga> because on Exynos even calling gpio_direction_input() on a configured GPIO interrupt is destructive
<javier__> tfiga: I see, interesting
<tfiga> this is because GPIO interrupt is another pinmux setting than GPIO input
<tfiga> currently pinmux is being configured in exynos_set_irq_type()
<tfiga> and if something reconfigures it after the type is set, you end up without interrupts
<javier__> got it
<tfiga> I wouldn't mind seeing a patch that finally makes handling of this proper
<tfiga> ;)
<tfiga> as I might not be able to finish the second version of mine
<javier__> :)
<javier__> let me try yours first before leave for the weekend and then I can think better about this
<tfiga> OK
<javier__> tfiga: took me some time to figure it out since I'm not as familiar with the irq subystem as I would want to
<tfiga> I won't be at work until 25.08, so I wouldn't be able to look myself into this earlier anyway
<javier__> ok
<tfiga> well, there is also a possibility that I screwed something up with recent code consolidation
MrBIOS has joined #linux-exynos
<javier__> tfiga: your patch fixes it!
<javier__> just forward ported and tested it
<tfiga> OK, so this probably confirms that something is changing the pinmux after initial set_type
<tfiga> because with this patch it is moved to IRQ startup which happens in request_irq
<javier__> tfiga: nod while used to be on set_type so it never was about the flags but the pin being mis configured
<tfiga> yep
<tfiga> now if we could also figure out what was changing the pinmux
<tfiga> but I have one wild guess, is the dts you use in your testing already in mainline?
<javier__> tfiga: yes, is arch/arm/boot/dts/exynos5420-peach-pit.dts + https://patchwork.kernel.org/patch/4688921/
<tfiga> ok, this explains everything
<tfiga> see the "samsung,pin-function = <0>;" in trackpad_irq?
<tfiga> this is GPIO input
<tfiga> and is being activated before probing the atmel driver
<javier__> tfiga: oh, those DTS snippets were taken from the chrome os downstream kernel which also hardcodes the trigger type when passing to request_threaded_irq()
<javier__> tfiga: that explains everything
<tfiga> well, the driver should be able to catch this and error out
<tfiga> also this is the reason I said that gpio_lock_as_irq is not sufficient
<javier__> yes, gpio_lock_as_irq() is thought just to avoid to set a gpio direction as output
<tfiga> I guess that irqchip's request_resources should just request the pin as special function
<tfiga> hmm, this is a bit more complex
<javier__> tfiga: right since how do you know how the pin is going to be used?
<javier__> in which function I mean
<tfiga> well, if something requests it as an interrupt then it can be only used as an interrupt and GPIO input
<tfiga> without any ability to change its function
<tfiga> the problem is that even if we claim the pin as special function in pinctrl, this won't prevent gpio_direction_*()
<tfiga> moreover, I believe gpio_direction_input() should be able to report success, just do nothing
<tfiga> so probably handling of this should be done at the lowest level in pinctrl-samsung driver
<javier__> tfiga: yes I was about to say that it would be complex since the gpio handling was made in pinctrl-samsung
<javier__> tfiga: so what about this 1) a variation of your patch that does the same but in .irq_request_resources instead of .irq_startup
<javier__> 2) some logic in pinctrl-samsung to prevent gpio_direction_*() to modify pins if was already requested as an IRQ
<tfiga> today I would probably add a bit mask to samsung_pin_bank which would prevent any pinmux reconfiguration
<javier__> tfiga: I guess 2) is possible but I've to read pinctrl-samsung since I'm not familiar with
<javier__> tfiga: nice, that gives us 2)
<tfiga> although moving IRQ pinmux setup to .irq_request_resources might be useful as well
<tfiga> because its current location in exynos_set_irq_type is just a hack
<javier__> tfiga: agreed
<tfiga> also 2) would have to cover the little subtlety of gpio_direction_input() succeeding when the pin is configured as an interrupt
<javier__> tfiga: which according to linusw is ok
<javier__> I remember a very long discussion about this since board code assumed that a GPIO was always requested to be used as an IRQ
<tfiga> I too think it is OK, because the pin can be used as an input too in this state
<javier__> while is not true with DT anymore
<tfiga> well, I'm not really sure it was always requested with board code
<javier__> yeah so that assumption was deep on the gpio omap driver and irq-gpio failed when converted to DT
<tfiga> you couldn't really expect anything back then
<tfiga> I mean, it heavily depended on the platform
<javier__> indeed
<tfiga> and even probably on the author of particular board fiie
<tfiga> file*
<tfiga> anyway
<tfiga> I guess it would be a good idea to notify the MLs about our finding
<tfiga> would you mind doing it or I should?
<javier__> tfiga: sure, I can do it if you want
<tfiga> great, thanks
<javier__> tfiga: thanks to you, this took me a lot to debug, I should had asked you before ;)
<tfiga> no problem
liquidAcid has quit [Quit: Leaving]
<javier__> tfiga: mail sent, sorry couldn't send it before
<javier__> tfiga: leaving for today, again thanks a lot for your help and have a great weekend!
<tfiga> javier__: thanks
<tfiga> have a great weekend as well
<javier__> tfiga: thanks
dlezcano has quit [Ping timeout: 255 seconds]
tilmann has quit [Quit: leaving]
<sjoerd> mdrjr: I was mostly wondering whether it would correctly handle a display that needs MST
ssvb has quit [Ping timeout: 245 seconds]
ssvb has joined #linux-exynos
liquidAcid has joined #linux-exynos
MrBIOS has quit [Ping timeout: 240 seconds]
liquidAcid has quit [Quit: Leaving]