> -----Original Message----- > From: Kevin Hilman [mailto:khilman@xxxxxx] > Sent: Thursday, March 03, 2011 4:00 AM > To: Santosh Shilimkar > Cc: linux-omap@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 08/17] omap4: pm: Add GIC save/restore support [...] > > @@ -67,6 +82,17 @@ struct omap4_cpu_pm_info { > > > > static DEFINE_PER_CPU(struct omap4_cpu_pm_info, omap4_pm_info); > > > > +/* Helper functions */ > > +static inline void sar_writel(u32 val, u32 offset, u8 idx) > > +{ > > + __raw_writel(val, sar_ram_base + offset + 4 * idx); > > +} > > aha, this is what I was thinking of in the earlier SAR patch. > > Something like this should be part of the SAR code, not here. > If you remember during the internal review, this helpers are added to improve the readability and they are inline functions. Same is the case with wakeupgen save code. If you move this code to SAR file and GIC helper to gic file, the save routine will become highly un-optimal. In General Save is like below. Read_Harfware_register() Write_it_sar_location() This happens for every GIC and wakeupgen registers and hence moving this across files and calling them from there is going add un-necessary stack overhead. I already got rid-off the global address pointers. I will get those now using omap4_get_*_base() and store it for local use in the file. > > +static inline u32 gic_readl(u32 offset, u8 idx) > > +{ > > + return __raw_readl(gic_dist_base_addr + offset + 4 * idx); > > +} > > Similarily, it would be nice tos see this as part of GIC code so > this code doesn't have to access a global base address pointer. > Same comment applies here too. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html