On 27/10/2021 19:05:27+0200, Emmanuel Gil Peyrot wrote:
> > On 15/10/2021 00:05:24+0200, Emmanuel Gil Peyrot wrote:
> > > These three consoles share a device, the MX23L4005, which contains a
> > > clock and 64 bytes of SRAM storage, and is exposed on the EXI bus
> > > (similar to SPI) on channel 0, device 1. This driver allows it to be
> > > used as a Linux RTC device, where time can be read and set.
> > >
> > > The hardware also exposes two timers, one which shuts down the console
> > > and one which powers it on, but these aren’t supported currently.
> > >
> > > On the Wii U, the counter bias is stored in a XML file, /config/rtc.xml,
> > > encrypted in the SLC (eMMC storage), using a proprietary filesystem. In
> > > order to avoid having to implement all that, this driver assumes a
> > > bootloader will parse this XML file and write the bias into the SRAM, at
> > > the same location the other two consoles have it.
> > >
> > > Signed-off-by: Emmanuel Gil Peyrot <linkmauve@xxxxxxxxxxxx>
> > > ---
> > > drivers/rtc/Kconfig | 10 ++
> > > drivers/rtc/Makefile | 1 +
> > > drivers/rtc/rtc-nintendo.c | 305 +++++++++++++++++++++++++++++++++++++
> >
> > I'm not convinced this is a good name, seeing that the switch will
> > certainly not use this driver (neither is the snes mini).
>
> Other subsystem maintainers have requested this to be changed, so I
> reflected it here too. For instance hid requested a Wii U-specific
> driver to be merged with the Switch one.
>
> Would rtc-gamecube be fine then? So far I have only tested on Wii U,
> but this driver is expected to support all three generations of
> GameCube, Wii and Wii U.
>
I think this is appropriate, that would have been my suggestion.
> > > +static int nintendo_rtc_set_time(struct device *dev, struct rtc_time *t)
> > > +{
> > > + time64_t timestamp;
> > > + struct nintendo_rtc_drvdata *d = dev_get_drvdata(dev);
> > > +
> > > + /* Subtract the timestamp and the bias to obtain the counter value */
> > > + timestamp = rtc_tm_to_time64(t);
> > > + exi_write(d->iob, RTC_COUNTER, timestamp - d->rtc_bias);
> >
> > As you are able to update RTC_COUNTER, I'm not sure why you actually
> > need rtc_bias.
>
> The proprietary firmware sets the counter based on the bias, so if we
> want to get the correct time out of the box we have to sum them. It
> wouldn’t even be possible to set bias on all of the consoles from Linux,
> for instance on the Wii U it is stored in an XML file, in a proprietary
> filesystem, and encrypted (using keys for which the drivers are already
> in mainline at least).
>
Ok, fine by me.
> > > +#ifdef DEBUG
> > > +static void nintendo_rtc_dumpregs(void __iomem *iob)
> > > +{
> > > + int i;
> > > + u32 sram_addr = RTC_SRAM;
> > > +
> > > + printk("RTC_COUNTER: %08X\n", exi_read(iob, RTC_COUNTER));
> > > + printk("RTC_SNAPSHOT: %08X\n", exi_read(iob, RTC_SNAPSHOT));
> > > + printk("RTC_ONTMR: %08X\n", exi_read(iob, RTC_ONTMR));
> > > + printk("RTC_OFFTMR: %08X\n", exi_read(iob, RTC_OFFTMR));
> > > + printk("RTC_TEST0: %08X\n", exi_read(iob, RTC_TEST0));
> > > + printk("RTC_TEST1: %08X\n", exi_read(iob, RTC_TEST1));
> > > + printk("RTC_TEST2: %08X\n", exi_read(iob, RTC_TEST2));
> > > + printk("RTC_TEST3: %08X\n", exi_read(iob, RTC_TEST3));
> > > + printk("RTC_CONTROL0: %08X\n", exi_read(iob, RTC_CONTROL0));
> > > + printk("RTC_CONTROL1: %08X\n", exi_read(iob, RTC_CONTROL1));
> > > + printk("RTC_SRAM:\n");
> > > + for(i = 0; i < 4; i++) {
> > > + printk("%08X %08X %08X %08X\n",
> > > + exi_read(iob, sram_addr + 0x100 * 0),
> > > + exi_read(iob, sram_addr + 0x100 * 1),
> > > + exi_read(iob, sram_addr + 0x100 * 2),
> > > + exi_read(iob, sram_addr + 0x100 * 3));
> > > + sram_addr += 0x400;
> >
> > Something great to do would be to convert the driver to regmap, provding
> > custom regmap_read and regmap_write functions to access the EXI bus.
> > Then you'd get this directly in debugfs. And this could be split ou once
> > other drivers need access to the bus (I guess power/reset at some
> > point).
>
> Will do, I wasn’t aware of regmap, thanks!
>
This is feature creep pushed to the max, I would take your driver even
if you don't do that.
> >
> > Ideally, you should also expose LOW_BATT from RTC_CONTROL0 using the
> > VL_READ ioctl as I'm guessing many console will start to have a depleted
> > battery.
>
> I won’t be able to test that yet, but I’ll try to implement it as it is
> documented.
>
This can also come in a second patch. I'll try to boot up my own
(platinum edition) GC, I'm wondering what is the battery status :)
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
