PXA: Colibri320: Add M41T00 RTC support
Subsystems (3)
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
--- arch/arm/mach-pxa/colibri-pxa320.c | 16 ++++++++++++++++ arch/arm/mach-pxa/colibri-pxa3xx.c | 14 ++++++++++++++ arch/arm/mach-pxa/include/mach/colibri.h | 7 +++++++ 3 files changed, 37 insertions(+), 0 deletions(-)
arch/arm/mach-pxa/colibri-pxa320.c +16 -0
@@ -236,6 +236,21 @@ static void __init colibri_pxa320_init_uart(void)pxa3xx_mfp_config (ARRAY_AND_SIZE (colibri_pxa320_uart_pin_config)); } +#if defined(CONFIG_RTC_DRV_DS1307 ) || defined(CONFIG_RTC_DRV_DS1307_MODULE ) +static mfp_cfg_t colibri_pxa320_i2c_pin_config[]__initdata = { +GPIO32_I2C_SCL , +GPIO33_I2C_SDA , +}; + +static void __init colibri_pxa320_init_rtc (void ) +{ +pxa3xx_mfp_config (ARRAY_AND_SIZE (colibri_pxa320_i2c_pin_config)); +colibri_pxa3xx_init_rtc (); +} +#else +static inline void colibri_pxa320_init_rtc (void ) {} +#endif +void __init colibri_pxa320_init (void ) {pxa_set_ffuart_info (NULL );
@@ -252,6 +267,7 @@ void __init colibri_pxa320_init(void) mfp_to_gpio(MFP_PIN_GPIO28));colibri_pxa320_init_uart ();colibri_pxa320_init_udc (); +colibri_pxa320_init_rtc (); }MACHINE_START (COLIBRI320 ,"Toradex Colibri PXA320" )
arch/arm/mach-pxa/colibri-pxa3xx.c +14 -0
@@ -26,6 +26,7 @@#include <mach/mmc.h> #include <mach/pxafb.h> #include <plat/pxa3xx_nand.h> +#include <plat/i2c.h> #include "generic.h" #include "devices.h"
@@ -198,3 +199,16 @@ void __init colibri_pxa3xx_init_nand(void) }#endif +#if defined(CONFIG_RTC_DRV_DS1307 ) || defined(CONFIG_RTC_DRV_DS1307_MODULE ) +static struct i2c_board_info __initdata colibri_pxa3xx_i2c_devs[] = { + { +I2C_BOARD_INFO ("m41t00" ,0x68 ), + }, +}; + +void __init colibri_pxa3xx_init_rtc (void ) +{ +pxa_set_i2c_info (NULL ); +i2c_register_board_info (0 ,ARRAY_AND_SIZE (colibri_pxa3xx_i2c_devs)); +} +#endif
arch/arm/mach-pxa/include/mach/colibri.h +7 -0
@@ -51,6 +51,13 @@ extern void colibri_pxa3xx_init_nand(void);static inline void colibri_pxa3xx_init_nand (void ) {}#endif +#if defined(CONFIG_RTC_DRV_DS1307 ) || defined(CONFIG_RTC_DRV_DS1307_MODULE ) +extern void colibri_pxa3xx_init_rtc (void ); +#else +static inline void colibri_pxa3xx_init_rtc (void ) {} +#endif + +/* physical memory regions */ #define COLIBRI_SDRAM_BASE 0xa0000000/* SDRAM region */
Signature · 1 line
1.7.1
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
arch/arm/mach-pxa/colibri-pxa320.c static void __init colibri_pxa320_init_uart(void) · line 243
Quoted 6 lines · On Saturday 31 July 2010 04:30:53 Marek Vasut wrote:
+#if
defined(CONFIG_RTC_DRV_DS1307)||defined(CONFIG_RTC_DRV_DS1307_MODULE)arch/arm/mach-pxa/colibri-pxa320.c +4
-0+static mfp_cfg_t colibri_pxa320_i2c_pin_config[]__initdata = { +GPIO32_I2C_SCL , +GPIO33_I2C_SDA , +};
Should the i2c pins really depend on the DS1307 config? On my board, I use a different rtc. So I do need the I2C pins to be configured, but I don't need the DS1307.
A few weeks ago I submitted
[PATCH 4/5] colibri-pxa3xx: add i2c support
which initializes the i2c gpio's, depending on CONFIG_I2C || CONFIG_I2C_MODULE
(but the patch was rejected, argueing that people might want to use different gpio's for i2c)
Rgds, Pieter
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
Dne So 31. ?ervence 2010 09:26:03 pieterg napsal(a):
Quoted 16 lines · On Saturday 31 July 2010 04:30:53 Marek Vasut wrote:
arch/arm/mach-pxa/colibri-pxa320.c static void __init colibri_pxa320_init_uart(void) · line 243
Quoted 6 lines
+#if
defined(CONFIG_RTC_DRV_DS1307)||defined(CONFIG_RTC_DRV_DS1307_MODULE)arch/arm/mach-pxa/colibri-pxa320.c +4
-0+static mfp_cfg_t colibri_pxa320_i2c_pin_config[]__initdata = { +GPIO32_I2C_SCL , +GPIO33_I2C_SDA , +};Should the i2c pins really depend on the DS1307 config? On my board, I use a different rtc. So I do need the I2C pins to be configured, but I don't need the DS1307.
A few weeks ago I submitted
[PATCH 4/5] colibri-pxa3xx: add i2c support
Quoted 7 lines
which initializes the i2c gpio's, depending on
CONFIG_I2C||CONFIG_I2C_MODULE(but the patch was rejected, argueing that people might want to use different gpio's for i2c)
Rgds, Pieter
Well I'd like to rework the colibri pxa3xx stuff later, but I'll have to discuss it with Dan. I believe reworking it the same way as pxa270 colibri would be nice (and it'd solve your problem too as you use custom board I think?).
Actually, there is one thing that puzzles me. Eric/Dan, shall I stick all the MFP configs into one (or two, one for board and once for cpu card) array or keep it split in multiple smaller arrays for each device? I think putting it all in one place would be more readable.
Cheers
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
Quoted 12 lines · Marek Vasut
Dne So 31. ?ervence 2010 09:26:03 pieterg napsal(a):
Quoted 11 lines · On Saturday 31 July 2010 04:30:53 Marek Vasut wrote:
arch/arm/mach-pxa/colibri-pxa320.c static void __init colibri_pxa320_init_uart(void) · line 243
Quoted 6 lines
+#if
defined(CONFIG_RTC_DRV_DS1307)||defined(CONFIG_RTC_DRV_DS1307_MODULE)arch/arm/mach-pxa/colibri-pxa320.c +4
-0+static mfp_cfg_t colibri_pxa320_i2c_pin_config[]__initdata = { +GPIO32_I2C_SCL , +GPIO33_I2C_SDA , +};Should the i2c pins really depend on the DS1307 config? On my board, I use a different rtc. So I do need the I2C pins to be configured, but I don't need the DS1307.
Yes, I think you're right. The I2C pins should only be set up if I2C support for PXA is in fact enabled. If someone manages to build a kernel which has support for this driver (which should depend on the I2C core as its transport layer) but not for PXA I2C, that's an unfortunate exception.
You could think about something like
select I2C_PXA if RTC_DRV_DS1307
in the Colibri block of mach-pxa/Kconfig, but that's somewhat overdone maybe.
Quoted 3 lines
Well I'd like to rework the colibri pxa3xx stuff later, but I'll have to discuss it with Dan. I believe reworking it the same way as pxa270 colibri would be nice (and it'd solve your problem too as you use custom board I think?).
Yes, that'd be nice.
Quoted 4 lines
Actually, there is one thing that puzzles me. Eric/Dan, shall I stick all the MFP configs into one (or two, one for board and once for cpu card) array or keep it split in multiple smaller arrays for each device? I think putting it all in one place would be more readable.
Hmm, GPIOs for peripherals that are assembled on the module itself can safely be configured unconditionally as it is impossible to use them for anything else. For GPIOs that are allocted by a specific base board, things are different though, especially for the the Colibri eval board with it hundreds of jumpers on it. It's a convenience feature to have them free for general purpose use just by disabling a certain config directive.
Daniel
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
Quoted 7 lines · Daniel Mack
Hmm, GPIOs for peripherals that are assembled on the module itself can safely be configured unconditionally as it is impossible to use them for anything else. For GPIOs that are allocted by a specific base board, things are different though, especially for the the Colibri eval board with it hundreds of jumpers on it. It's a convenience feature to have them free for general purpose use just by disabling a certain config directive.
It would probably make sense for their to be a primary table which at least ensures that all pins are setup to safe default values. You really don't want the unconnected pins left floating, especially if they come out to some vulnerable connector or unconnected jumper pin.
I'd suggest they're configured as inputs but with a pull-up or pull-down, not forgetting to consider the low power mode pull setting as well.
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
Quoted 9 lines · Russell King - ARM Linux
Quoted 7 lines · Daniel Mack
Hmm, GPIOs for peripherals that are assembled on the module itself can safely be configured unconditionally as it is impossible to use them for anything else. For GPIOs that are allocted by a specific base board, things are different though, especially for the the Colibri eval board with it hundreds of jumpers on it. It's a convenience feature to have them free for general purpose use just by disabling a certain config directive.
Quoted 8 lines
It would probably make sense for their to be a primary table which at least ensures that all pins are setup to safe default values. ?You really don't want the unconnected pins left floating, especially if they come out to some vulnerable connector or unconnected jumper pin.
I'd suggest they're configured as inputs but with a pull-up or pull-down, not forgetting to consider the low power mode pull setting as well.
Yeah, that's also my opinion. So for those pins which can be connected to different usages (e.g. to two or more different expansion boards), I'd like to have the baseboard code to configure them safely (e.g. inputs with pull as Russell suggested), and perform another mfp config in the expansion board code.
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
In reply to Daniel MackDne So 31. ?ervence 2010 11:05:54 Daniel Mack napsal(a):
Quoted 20 lines · Marek Vasut
Quoted 12 lines
Dne So 31. ?ervence 2010 09:26:03 pieterg napsal(a):
Quoted 11 lines · On Saturday 31 July 2010 04:30:53 Marek Vasut wrote:
arch/arm/mach-pxa/colibri-pxa320.c static void __init colibri_pxa320_init_uart(void) · line 243
Quoted 6 lines
+#if
defined(CONFIG_RTC_DRV_DS1307)||defined(CONFIG_RTC_DRV_DS1307_MODULE)arch/arm/mach-pxa/colibri-pxa320.c +4
-0+static mfp_cfg_t colibri_pxa320_i2c_pin_config[]__initdata = { +GPIO32_I2C_SCL , +GPIO33_I2C_SDA , +};Should the i2c pins really depend on the DS1307 config? On my board, I use a different rtc. So I do need the I2C pins to be configured, but I don't need the DS1307.
Yes, I think you're right. The I2C pins should only be set up if I2C support for PXA is in fact enabled. If someone manages to build a kernel which has support for this driver (which should depend on the I2C core as its transport layer) but not for PXA I2C, that's an unfortunate exception.
Quoted 6 lines
You could think about something like
select
I2C_PXAifRTC_DRV_DS1307in the Colibri block of mach-pxa/Kconfig, but that's somewhat overdone maybe.
You're right, probably. I'll see what I can do about it (maybe I'll just queue this patch after the rework).
Quoted 7 lines
Quoted 5 lines
Well I'd like to rework the colibri pxa3xx stuff later, but I'll have to discuss it with Dan. I believe reworking it the same way as pxa270 colibri would be nice (and it'd solve your problem too as you use custom board I think?).
Yes, that'd be nice.
It's a mess, true indeed. But now I need to get some sleep :-)
Quoted 6 lines
Quoted 5 lines
Actually, there is one thing that puzzles me. Eric/Dan, shall I stick all the MFP configs into one (or two, one for board and once for cpu card) array or keep it split in multiple smaller arrays for each device? I think putting it all in one place would be more readable.
Quoted 7 lines
Hmm, GPIOs for peripherals that are assembled on the module itself can safely be configured unconditionally as it is impossible to use them for anything else. For GPIOs that are allocted by a specific base board, things are different though, especially for the the Colibri eval board with it hundreds of jumpers on it. It's a convenience feature to have them free for general purpose use just by disabling a certain config directive.
Ok, I'd agree on this ;-)
Quoted 2 lines
Daniel
PATCH 1/1 PXA: Colibri320: Add M41T00 RTC support
In reply to Marek VasutQuoted 4 lines · Marek Vasut
Actually, there is one thing that puzzles me. Eric/Dan, shall I stick all the MFP configs into one (or two, one for board and once for cpu card) array or keep it split in multiple smaller arrays for each device? I think putting it all in one place would be more readable.
I've always been of the opinion that the pin configuration should be setup independently of what drivers are built into the kernel or not.
Consider a pin which is used as an output to drive some high impedance signal (eg, an input on another device) and it doesn't have any pull-up or pull down. If on reset, this signal is also configured as a high impedance input, it will float.
Inputs which float cause increased current drain (that's a fact of the high impedance FET input configurations - a floating signal ends up partially turning on both N and P FETs at both ends, which allows current to flow between supply and ground. This is also why the current consumption of devices is related to their clock frequency and disabling clocks saves power.)
So, it is desirable to have the pin configuration correct for the entire board, irrespective of the driver configuration.