On Thu, 25 Jul 2019 12:47:16 +0100 Lee Jones <lee.jones@xxxxxxxxxx> wrote: > On Thu, 13 Jun 2019, Thomas Bogendoerfer wrote: > > +/* > > + * On IP30 the RTC (a DS1687) is behind the IOC3 on the generic > > + * ByteBus regions. We have to write the RTC address of interest to > > + * IOC3_BYTEBUS_DEV1, then read the data from IOC3_BYTEBUS_DEV2. > > + * rtc->regs already points to IOC3_BYTEBUS_DEV1. > > + */ > > +#define IP30_RTC_ADDR(rtc) (rtc->regs) > > +#define IP30_RTC_DATA(rtc) ((rtc->regs) + IOC3_BYTEBUS_DEV2 - IOC3_BYTEBUS_DEV1) > > + > > +static u8 ip30_rtc_read(struct ds1685_priv *rtc, int reg) > > +{ > > + writeb((reg & 0x7f), IP30_RTC_ADDR(rtc)); > > + return readb(IP30_RTC_DATA(rtc)); > > +} > > + > > +static void ip30_rtc_write(struct ds1685_priv *rtc, int reg, u8 value) > > +{ > > + writeb((reg & 0x7f), IP30_RTC_ADDR(rtc)); > > + writeb(value, IP30_RTC_DATA(rtc)); > > +} > > Why is this not in the RTC driver? because rtc1685 is used in different systems and accessing the chip differs between those systems. > > +static struct ds1685_rtc_platform_data ip30_rtc_platform_data = { > > + .bcd_mode = false, > > + .no_irq = false, > > + .uie_unsupported = true, > > + .alloc_io_resources = true, > > > + .plat_read = ip30_rtc_read, > > + .plat_write = ip30_rtc_write, > > Call-backs in a non-subsystem API is pretty ugly IMHO. I agree > Where are these called from? drivers/rtc/rtc-ds1685.c I could do the same as done for serial8250 and add an additional .c file in drivers/rtc which handles this for SGI-IP30. Alexandre would this work for you as well ? > > +#define IOC3_SID(_name, _sid, _setup) \ > > + { \ > > + .name = _name, \ > > + .sid = (PCI_VENDOR_ID_SGI << 16) | IOC3_SUBSYS_ ## _sid, \ > > + .setup = _setup, \ > > + } > > + > > +static struct { > > + const char *name; > > + u32 sid; > > + int (*setup)(struct ioc3_priv_data *ipd); > > +} ioc3_infos[] = { > > IMHO it's neater if you separate the definition and static data part. I don't quite understand what you mean here. Should I move the #define at the beginning of the file ? Why is it neater ? Thomas. -- SUSE Linux GmbH GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)