HALifying the RTC

I’ve now got I2C working on my beagleboard, and have successfully communicated with the TWL/TPS companion chip that manages the RTC, audio, etc. So since I’ve already read up on most of the stuff, I thought I’d take a look at what would be needed to get the RTC working.

At the moment it looks like RISC OS only supports RTC hardware that’s attached to I2C bus 0, and is programmed using BCD day/month/year/minute/second/etc. values. Which is convenient for the OMAP3 since the TWL/TPS is on I2C bus 0 and also uses BCD values for the RTC. Although I could extend the existing code to detect the TWL/TPS and read/write the RTC accordingly, the code would simply become more complex and confusing, and it would increase the chance of failure if an I2C device on a future platform uses the same address as one of the supported RTCs.

So instead I’m thinking it’s about time the RTC was HALified.

There are two approaches to HALifying the RTC - using standard HAL calls or using the HAL device API. The HAL device API is probably preferable, but may cause some problems since at the moment the RTC is first read inside ExecuteInit, which happens before the call to HAL_InitDevices. Although I suspect it won’t cause any problems to re-initialise the system clock after HAL_InitDevices, if the RTC init code inside ExecuteInit is to remain then an extra flag might be needed in somewhere like HAL_PlatformInfo to indicate that ExecuteInit should avoid probing the I2C bus.

• PreReadTime – constructs an iic_transfer list for reading the time
• PostReadTime – after the successful processing of the iic_transfer list, reads out the time value in some specified format (raw BCD, integers, 5-byte time, etc)
• PreWriteTime – constructs an iic_transfer list for writing the time, using the values in some specified format (raw BCD, integers, 5-byte time, etc)
• PostWriteTime – if required, examines the updated iic_transfer to check the time was written properly

• I2C bus number. This can be encoded as part of the HAL device ‘location’ field
• Amount of memory required for read iic_transfer
• Amount of memory required for write iic_transfer
• Preferred time format (BCD year/month/day/hour/minute/second/centisecond, integer year/month/day/hour/minute/second/centisecond, 5-byte RISC OS time, 32bit unix time, etc) – the only danger here is that there could be many BCD formats required depending on whether fields are 0- or 1-based
• Minimum representable time (as 5-byte RISC OS time?)
• Maximum representable time (as 5-byte RISC OS time?)

The min/max represntable time values would be important for checking that a time is valid before allowing to be committed to hardware (e.g. the TWL/TPS only has a 2-digit year field, so it would be sensible to only allow the system clock to run from years from 2000-2099).

Thoughts?

I’m not intimately familiar with all the variants of RTC chips that are around, but I think I recall that the kernel has a database of IIC addresses and it tries them all until it finds one it knows. I’d agree that it would be much more elegant and less prone to address clashes if the HAL describes the actual RTC chip in use – possibly with a way to signal that the kernel should fall back to the old behaviour for the case of legacy platforms.

One thing I would mention though, when inventing HAL interfaces, is to try to keep the HAL layer only as deep as is necessary to support all current hardware platforms: third-party software should in general not be using the HAL interfaces (the exceptions being the few timer- and IRQ-related calls that were documented before the open sourcing of RISC OS) so it is practical to extend HAL interfaces to a more general form as and when the need arises.

Keep the APIs as close to the hardware as possible – it helps to think of the HAL interface as being OS-independent. So for example in this case, I’d warn against using either RISC OS or Unix time formats, especially if all RTC chips use some variant of a BCD encoding. This also has the benefit that all that complicated and potentially error-prone code to deal with conversions (leap years in particular) only has to live in one place, in the OS, rather than one implementation in each HAL.

The policy of probing known addresses should work fine, since addresses are allocated by Philips (now NXP) http://www.nxp.com/products/interface_control/i2c/support/requestform/ http://www.nxp.com/products/interface_control/i2c/licensing/

so the clocks are all at known addresses. However, there is some fun knowing which clock it fitted since addresses can overlap (as you wouldn’t normally have 2 clocks at the same time Philips is presumably saving address space by dishing out the same number twice).

When I added support for the Dallas part to the kernel it turned out all but one registers were the same as the Philips PCF part so a probe followed by some conditionality worked. This would get progressively more complicated over time!

The policy of probing known addresses should work fine, since addresses are allocated by Philips (now NXP)

“should work fine”, yes. But in reality I suspect it’s a bit more complicated. I haven’t checked their facts, but the Wikipedia article about I2C states that two protocol-incompatible EEPROM chips and an RTC chip all respond to the same slave address.

As far as the HAL implementation goes, I’m thinking the first implementation would be as per my original post, with 4 function calls used to construct & parse iic_transfer structures (which are already HALified as they get used by the HAL I2C API). The ‘preferred time format’ I mentioned should really have been ‘supported time format’, as it’s intended that the driver code would only support one time format when communicating with RISC OS. For this initial implementation this format would be BCD, with a set of flags to toggle 0- or 1-based behaviour of some of the fields. As Ben says there’s no point supporting formats which aren’t presently needed (or could cause issues, e.g. the potential non-monotonicity of RISC OS/unix time), so BCD will be the only format for the moment. I may also have to rethink the format the min/max times are stored in – or just have the PreWriteTime function return an error code if the requested time is outside of range (much simpler from an implementation standpoint, even if it doesn’t provide some feedback as to what a valid time would be)

I’ll add a compile-time flag to either the Kernel source or Hdr:Machine.Machine to indicate whether the kernel should probe the I2C bus for an RTC. So for Cortex the existing I2C RTC code can be removed. Unless someone can think up a reason why, I won’t add a flag to HAL_PlatformFeatures to indicate that a HAL RTC is present; I’ll just keep things simple by having the kernel look for it automatically if its own I2C RTC support is disabled. Then over time if we want we can enable the HAL RTC code for the other HAL machine types.

So the next question would be: How does one go about allocating a new HAL device type & ID? Also a new serial bus sub-type would be required for I2C (unless the doc I was reading was out of date and there’s already one defined)

How does one go about allocating a new HAL device type & ID?

The definitive list is actually in castle/RiscOS/Sources/Kernel/hdr/HALDevice (and yes, there aren’t all that many definitions as yet). We should probably nominate one branch as the definitive list to avoid possible conflicting allocations – logically, I’d suggest this should be the HAL branch version of this file.

I’ve got the HAL RTC code implemented now and working with the beagleboard – although in the end I have changed the API a fair bit. Basically the ‘PreXXXTime’ and ‘PostXXXTime’ method was too simplistic and was preventing more complex interactions from occuring (e.g. safely updating the RTC by stopping the clock, writing the new time, then restarting it). So instead I’ve gone with a much simpler approach of just passing a function pointer to the RTC driver to allow it to perform as many IIC ops as it wants. This should also make it easier to adapt the API to support other RTCs. The driver is now also responsible for allocating its own workspace (workspace allocation isn’t really a problem for a simple IIC driver anyway since it can easily fit the data on the stack)

While looking through the RISC OS code I also spotted that the OS already handles devices which can’t store the full two-byte year – The Dallas RTC only seems to have a 2-bit year field, so RISC OS augments this with a 2 byte year stored in CMOS and compares the two to see if anything has changed (which should be fine as long as you don’t leave the computer turned off for more than 4 years!). So for the sake of the API, each RTC driver identifies itself as either supporting a 2 byte year or a 2 bit year.

The code should be in CVS in the next day or two – after that I think I’ll quickly move the HAL documentation into the wiki and add the final RTC API documentation there.

Of course, while preparing the wiki HAL docs, I’ve now spotted that IIC_OpV is made available to HAL code via the entry RISCOS_IICOpV, which makes the function pointer the HALDevice RTC API uses pretty much redundant. Doh! (Although the HALDevice wrapper has the advantage of returning a HAL IIC error code instead of a RISC OS error block, and the possibility of being extended to support IIC transfers on bus numbers other than 0)

I guess this will be one of the things to go on the “list of things to fix”, then…