Re: [PATCH 2.6.27-rc8 1/2] drivers/mfd/twl4030-core.c
* David Brownell [EMAIL PROTECTED] [081002 00:53]: On Wednesday 01 October 2008, David Brownell wrote: NOTE: this is the current code from the linux-omap tree. This chip being used on a whole bunch of boards: - Gumstix Overo - BeagleBoard.org - Omap ZOOM (Labrador) - OMAP 2430 and 3430 SDP, - OMAP2 and OMAP3 EVM - ... more (openpandora.org ?) which will get much closer to usable with mainline kernels when their power management chip works in mainline! :) Cool. I'll post some patches against mainline kernel for booting minimal omap3 boards within next few days. So we should get at least beagle and overo booting to some extent, maybe with serial and musb. Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.27-rc8 1/2] drivers/mfd/twl4030-core.c
On Wednesday 01 October 2008, Tony Lindgren wrote: * David Brownell [EMAIL PROTECTED] [081002 00:53]: On Wednesday 01 October 2008, David Brownell wrote: NOTE: this is the current code from the linux-omap tree. This chip being used on a whole bunch of boards: - Gumstix Overo - BeagleBoard.org - Omap ZOOM (Labrador) - OMAP 2430 and 3430 SDP, - OMAP2 and OMAP3 EVM - ... more (openpandora.org ?) My bad. Openpandora just opened for preorders *today* (PST) and sold out all 3000 units. I'm sure I mentioned it well after most of them were sold, so it's not my fault their server was killed. Really ... which will get much closer to usable with mainline kernels when their power management chip works in mainline! :) Cool. I'll post some patches against mainline kernel for booting minimal omap3 boards within next few days. So we should get at least beagle and overo booting to some extent, maybe with serial and musb. Sounds like it *should* be a nice, modest, achievable goal, right? Something tells me it won't be that easy! More power to you. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.27-rc8 1/2] drivers/mfd/twl4030-core.c
* David Brownell [EMAIL PROTECTED] [081002 09:54]: On Wednesday 01 October 2008, Tony Lindgren wrote: * David Brownell [EMAIL PROTECTED] [081002 00:53]: On Wednesday 01 October 2008, David Brownell wrote: NOTE: this is the current code from the linux-omap tree. This chip being used on a whole bunch of boards: - Gumstix Overo - BeagleBoard.org - Omap ZOOM (Labrador) - OMAP 2430 and 3430 SDP, - OMAP2 and OMAP3 EVM - ... more (openpandora.org ?) My bad. Openpandora just opened for preorders *today* (PST) and sold out all 3000 units. I'm sure I mentioned it well after most of them were sold, so it's not my fault their server was killed. Really ... which will get much closer to usable with mainline kernels when their power management chip works in mainline! :) Cool. I'll post some patches against mainline kernel for booting minimal omap3 boards within next few days. So we should get at least beagle and overo booting to some extent, maybe with serial and musb. Sounds like it *should* be a nice, modest, achievable goal, right? Something tells me it won't be that easy! More power to you. Hmm, well it's really the rest of the already posted omap2-upstream that RMK already commented on, plus one patch for sram for 34xx, then just the board files. So it should be doable yeah. Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.27-rc8 1/2] drivers/mfd/twl4030-core.c
On Wed, 1 Oct 2008 11:44:19 -0700 David Brownell wrote: drivers/mfd/Kconfig| 14 drivers/mfd/Makefile |2 drivers/mfd/twl4030-core.c | 1255 +++ include/linux/i2c/twl4030-gpio.h | 76 ++ include/linux/i2c/twl4030-madc.h | 134 +++ include/linux/i2c/twl4030-pwrirq.h | 37 + include/linux/i2c/twl4030.h| 191 + 7 files changed, 1709 insertions(+) --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -50,6 +50,20 @@ config HTC_PASIC3 HTC Magician devices, respectively. Actual functionality is handled by the leds-pasic3 and ds1wm drivers. +config TWL4030_CORE + bool Texas Instruments TWL4030/TPS659x0 Support + depends on I2C=y (ARCH_OMAP2 || ARCH_OMAP3) + help + Say yes here if you have TWL4030 family chip on your board. + This core driver provides register access and IRQ handling + facilities, and registers devices for the various functions + so that function-specific drivers can bind to them. + + These multi-function chps are found on many OMAP2 and OMAP3 chips + boards, providing power management, RTC, GPIO, keypad, a + high speed USB OTG transceiver, an audio codec (on most + versions) and many other features. + config MFD_TMIO bool default n --- /dev/null +++ b/include/linux/i2c/twl4030-madc.h @@ -0,0 +1,134 @@ +/* + * include/linux/i2c/twl4030-madc.h + * + * TWL4030 MADC module driver header + * + * Copyright (C) 2008 Nokia Corporation + * Mikko Ylinen [EMAIL PROTECTED] +#define TWL4030_MADC_IOC_MAGIC '`' Add that to Documentation/ioctl-number.txt (?) --- ~Randy -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.27-rc8 1/2] drivers/mfd/twl4030-core.c
On Wednesday 01 October 2008, Randy Dunlap wrote: + These multi-function chps are found on many OMAP2 and OMAP3 chips Indeed ... although if e e cummings could write poetry without uppercase, surely in this modern age some literature will be forthcoming that uses no vwls? (Oh wait. That's Hebrew. ;) --- /dev/null +++ b/include/linux/i2c/twl4030-madc.h @@ -0,0 +1,134 @@ +/* + * include/linux/i2c/twl4030-madc.h + * + * TWL4030 MADC module driver header + * + * Copyright (C) 2008 Nokia Corporation + * Mikko Ylinen [EMAIL PROTECTED] +#define TWL4030_MADC_IOC_MAGIC '`' Add that to Documentation/ioctl-number.txt (?) Actually one of my own review comments is that this header should become unneccessary ... and everything associated with it be dealt with when this Multi-channel ADC driver gets pushed upstream. IRQ initialization needs to be done in the core, but none of the other MADC stuff matters here. Thanks. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.27-rc8 1/2] drivers/mfd/twl4030-core.c
On Wednesday 01 October 2008, David Brownell wrote:
NOTE: this is the current code from the linux-omap tree.
This chip being used on a whole bunch of boards:
- Gumstix Overo
- BeagleBoard.org
- Omap ZOOM (Labrador)
- OMAP 2430 and 3430 SDP,
- OMAP2 and OMAP3 EVM
- ... more (openpandora.org ?)
which will get much closer to usable with mainline kernels
when their power management chip works in mainline! :)
I expect a few things still need to be updated...
... ergo, some comments below. I'll hope we won't need
too many more iterations of fixes before we get a baseline
suitable for a 2.6.28-rc0 merge.
This version was posted since it's a good milestone, with
a bunch of recent updates from Felipe and me. I'd think
most of the comments below can be addressed pretty easily.
(Except ones related to threaded IRQ handling in mainline.)
include/linux/i2c/twl4030-gpio.h | 76 ++
include/linux/i2c/twl4030-madc.h | 134 +++
include/linux/i2c/twl4030-pwrirq.h | 37 +
include/linux/i2c/twl4030.h| 191 +
I suspect that's three headers-too-many, and that the fix
should (a) move register decls needed for IRQ setup into
the twl4030.h file, and (b) move other headers into the
relevant driver(s).
+config TWL4030_CORE
+ bool Texas Instruments TWL4030/TPS659x0 Support
+ depends on I2C=y (ARCH_OMAP2 || ARCH_OMAP3)
Hardly any of this code is actually OMAP-specific. I stuck
those dependencies in to prevent build breakage related to
how the current code expects some global IRQ symbols. When
that's all fixed I expect those dependencies to vanish.
--- /dev/null
+++ b/drivers/mfd/twl4030-core.c
@@ -0,0 +1,1255 @@
+/*
+ * twl4030_core.c - driver for TWL4030/TPS659x0 PM and audio CODEC devices
+ *
...
It could probably stand a header comment giving a bit more detail
about the chip and how this driver core supports it.
+/* Slave address */
+#define TWL4030_SLAVENUM_NUM00x00
+#define TWL4030_SLAVENUM_NUM10x01
+#define TWL4030_SLAVENUM_NUM20x02
+#define TWL4030_SLAVENUM_NUM30x03
I suspect these can/should be replaced with 0, 1, 2, and 3
everywhere they get used, referring to the relevant I2C address
(0x48 + 0/1/2/3) used to access a given function. :)
NUM_NUM should be reserved for lolcat usage.
And while I find the register addressing needlessly (?) confusing,
it's not a thing I'd suggest changing right away. It *could* have
been based on a {slave, register} pairing. Instead it's based
mapping {module, offset} to those real addresses. Maybe some
older chip revisions needed different mappings.
+/* TWL4030 BCI registers */
+#define TWL4030_INTERRUPTS_BCIIMR1A 0x2
+#define TWL4030_INTERRUPTS_BCIIMR2A 0x3
+#define TWL4030_INTERRUPTS_BCIIMR1B 0x6
+#define TWL4030_INTERRUPTS_BCIIMR2B 0x7
+#define TWL4030_INTERRUPTS_BCIISR1A 0x0
+#define TWL4030_INTERRUPTS_BCIISR2A 0x1
+#define TWL4030_INTERRUPTS_BCIISR1B 0x4
+#define TWL4030_INTERRUPTS_BCIISR2B 0x5
+
+/* TWL4030 keypad registers */
+#define TWL4030_KEYPAD_KEYP_IMR1 0x12
+#define TWL4030_KEYPAD_KEYP_IMR2 0x14
+#define TWL4030_KEYPAD_KEYP_ISR1 0x11
+#define TWL4030_KEYPAD_KEYP_ISR2 0x13
IMO those IRQ registers should be moved into the twl4030.h
header and shared with the relevant subchip drivers...
+static const u8 __initconst twl4030_keypad_imr_regs[] = {
+ TWL4030_KEYPAD_KEYP_IMR1,
+ TWL4030_KEYPAD_KEYP_IMR2,
+};
+
+/* TWL4030 keypad module interrupt status registers */
+static const u8 __initconst twl4030_keypad_isr_regs[] = {
+ TWL4030_KEYPAD_KEYP_ISR1,
+ TWL4030_KEYPAD_KEYP_ISR2,
+};
This __initconst stuff bothers me a bit. On one hand
it's not strictly correct because the driver could
be unbound from the hardware (e.g. sysfs, or rmmod of
its i2c_adapter) ... and then need this data again if
it gets re-initialized.
On the other hand, anyone trying to break systems like
that will succeed and it's only a question of how quickly
the kablooie happens. I'd rather just ensure the driver
can't ever be unbound.
On yet another hand (it's good to have lots of them!),
keeping that stuff around would let the driver recover
from certain nastiness (see below)...
+/* Structure to define on TWL4030 Slave ID */
+struct twl4030_client {
+ struct i2c_client *client;
+ u8 address;
+ bool inuse;
This inuse thing seems wasteful. Easier to just
have one flag for the whole driver.
+
+ /* max numb of i2c_msg required is for read =2 */
+ struct i2c_msg xfer_msg[2];
+
+ /* To lock access to xfer_msg */
+ struct mutex xfer_lock;
+};
Now the IRQ handling is important, and is a bit messy. It can be
cleaned up a bit ... but since TGLX has threatened to merge some
threaded IRQ support soonish, I'd propose waiting for that support
before doing too much work on the IRQ code here.
While various I2C chips have IRQ handlers -- common on embedded
