On Thu, Oct 20, 2016 at 04:39:26PM +0200, Hans de Goede wrote: > On 20-10-16 14:55, Maxime Ripard wrote: > > On Thu, Oct 20, 2016 at 11:17:13AM +0200, Hans de Goede wrote: > > > Yes that is the idea. Although it is not really fake as theoretically > > > the CEC pin could be used as a general gpio. Basically the idea is that > > > if the hardware only offers get and set functionality on the pin and > > > nothing really CEC specific, that we then should really have one generic > > > driver to deal with any such hardware, and not an allwinner specific > > > driver. > > > > > > The natural abstraction for such a driver to talk to the actual hardware > > > (be it allwinner or other hw) would be to use the gpio interface. > > > > There's really no point of having such an abstraction in the first > > place. Theorically, you could use your audio output or PWM as a GPIO, > > yet no one exposes it as a GPIO to then use a generic pwm-gpio driver > > to actually implement a PWM. > > Of course if there is pwm hardware then exporting that as just a > gpio is not sensible (assuming it can be muxed as a regular gpio), > but this is the other way around even though the hardware is labeled > as CEC it cannot do anything a gpio cannot. > > As your rock-chip example points out we are not alone here, so we > really should have a common cec-gpio drivers, just like e.g. > drivers/i2c/busses/i2c-gpio.c drivers/spi/spi-gpio.c, etc. > exist.
I don't know, we're the only users without an interrupt, which changes the logic a bit. > > > > How in that case implement interrupts for such fake gpio? > > > > > > GPIOs can be interrupts too, e.g. the cec driver would do: > > > > > > struct gpio_desc *cec_gpio = gpiod_get(dev, ...); > > > > > > int irq = gpiod_to_irq(cec_gpio); > > > > > > devm_request_threaded_irq(dev, irq, NULL, cec_gpio_irq_handler, > > > IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, > > > "cec_irq", cec_data); > > > > > > To register an irq on rising and falling edges on the gpio > > > pin. If you want to use this you can even make the availability > > > of irq functionality on the gpio mandatory in your cec-gpio driver > > > and then when someone needs things to work without irq functionality > > > they can extend it. > > > > > > The cool thing about this approach is that it will give instant CEC > > > capability to any (new) board where an edge irq capable GPIO is > > > available, just hook up the HMDI connector CEC pin to an edge IRQ > > > capable gpio and even non CEC capable HDMI encoders can do CEC :) > > > > The bad thing about this approach is that it doesn't work for us, > > since we don't have any interrupts on the CEC pins. > > Oh, the question from Jarosław gave me the impression that we did, > but it does not matter really, it just means that the initial > cec-gpio implementation will not use interrupts, and later someone > who actually has an edge irq capable gpio can extend it to handle > both cases, or write another driver for that case, just like we e.g. > have drivers/input/keyboard/gpio_keys.c and > drivers/input/keyboard/gpio_keys_polled.c. > > I'm somewhat surprised we're having so much discussion about this, > if the cec functionality on the A10 is just being able to get / set > some pin(s) then exporting these as gpio's seems the obvious solution > to abstract this in such a way that we can have a generic and *shared* > bit-banged cec driver. As said on some other boards we might very well > use an actual gpio for this, and not all gpio-s are edge irq capable, > so sooner or later we will need a generic bit bang cec driver which > does not use edge irqs on the pin. I'm actually quite surprised too. I don't see how it's different from say the discussion we had a couple of times on the ADCs found in the touchscreen IPs, in the AXP or in the LRADC. With the difference that the ADCs can be used for ADCs (and yet we chose not to support that), while there's no way this pin can be used for a GPIO. Having it in the driver is just the most straightforward way to implement it, and it deals with all the usecases that we have right now. Why would we want to overcomplicate things and trying to fit something in a model that isn't fit? And there's also the side effect that we would expose that pin through sysfs and /dev/, allowing the user to change it any way it wants. And I'm not really confortable with that. Having some kind of generic library to be able to do just that seems like a better design. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
signature.asc
Description: PGP signature
