On 06/17/2014 06:14 AM, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Wednesday, June 04, 2014 02:32:38 PM Mikko Perttunen wrote: >> This adds support for the integrated AHCI-compliant Serial ATA >> controller present on the NVIDIA Tegra124 system-on-chip. >> +static inline void sata_writel(struct tegra_ahci_priv *tegra, u32 value, >> + unsigned long offset) >> +{ >> + writel(value, tegra->sata_regs + offset); >> +} >> + >> +static inline u32 sata_readl(struct tegra_ahci_priv *tegra, >> + unsigned long offset) >> +{ >> + return readl(tegra->sata_regs + offset); >> +} > > sata_writel() and sata_read() static inlines don't seem to add any value. > > Can they be removed? Such functions are quite idiomatic in drivers, and serve to simplify all the call-sites by removing the need to write out the addition of the base address everywhere. >> +static struct platform_driver tegra_ahci_driver = { >> + .probe = tegra_ahci_probe, >> + .remove = ata_platform_remove_one, >> + .driver = { >> + .name = "tegra-ahci", >> + .owner = THIS_MODULE, >> + .of_match_table = tegra_ahci_of_match, >> + }, > > This driver lacks PM suspend/resume support. I assume that > the Tegra124 platform also doesn't support suspend/resume yet > (if so a comment in the driver code about this would be useful), > otherwise the driver should be fixed. We do have basic system suspend/resume support. However, I don't think missing suspend/resume in an individual driver should stop it from being merged. It can certainly be added later. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html