On 02/16/2011 18:23, Lars-Peter Clausen wrote:
I think you should really use readb(pdata->regs + REG) instead of the following
structs. Maybe add a helper function in the form of:
static uint8_t ds1685_read(struct ds1685_priv *ds1685, unsigned int reg) {
return readb(pdata->regs + REG);
}
That should also help with the different paddings introduced in patch 2.
Lots of good feedback, thanks! Ralf already suggested using offsets instead of
a struct. I'm tinkering now with getting this to work, as I have to have this
done before I can address many of your other points.
I have determined the following formula specific to the SGI O2 to read the RTC
registers:
readb(pdata->regs + RTC_<REGISTER> * 0x100);
is equivalent to
readb(pdata->regs.time.<REGISTER>);
I'll assume writeb() changes are the same. The question is, how do I wire in
the 0x100 padding value in such a way that I keep the IP32-specific bits out of
generic code? Ralf mentioned using some field in platform_data, but I haven't
quite learned the platform stuff (this is my first real attempt at a kernle driver).
Also, one thing I can quickly address, I put the ds1685.h file under
include/linux/rtc because I saw that folder as already existing. I figured
that's where rtc header files went. Right now, nothing outside of the driver
uses it, but SGI O2 will need to eventually, as it uses the RTC to trigger a
system poweroff by accessing a few of the extended control registers.
It currently uses similarly-duplicated #defines in a local header file, but I
figured, if I can get this driver fully working, and other platforms could
theoretically use the same trick, would not include/linux/rtc be the best place
for the header? If there's a better place, please let me know!
Thanks,
--
Joshua Kinard
Gentoo/MIPS
kumba@xxxxxxxxxx
"The past tempts us, the present confuses us, the future frightens us. And our
lives slip away, moment by moment, lost in that vast, terrible in-between."
--Emperor Turhan, Centauri Republic
