Hi,
>> +MODULE_DEVICE_TABLE(of, xgene_ahci_of_match);
>> +
>> +static struct platform_driver xgene_ahci_driver = {
>> + .probe = xgene_ahci_probe,
>> + .remove = ata_platform_remove_one,
>
> It is good to use ata_platform_remove_one() here but some code still needs
> to callback ahci_platform_disable_resources() before the driver is removed
> completely. The way other drivers do it is to define custom ->host_stop
> method and add it to port ops (xgene_ahci_ops in this case). For X-Gene
> AHCI driver ->host_stop function can be quite simple and look like this:
>
> static void xgene_ahci_host_stop(struct ata_host *host)
> {
> struct ahci_host_priv *hpriv = host->private_data;
>
> ahci_platform_disable_resources(hpriv);
> }
Will do.
>
>> + .driver = {
>> + .name = "xgene-ahci",
>> + .owner = THIS_MODULE,
>> + .of_match_table = xgene_ahci_of_match,
>> + },
>
> Power Management support is missing for this driver. If your platform
> doesn't support Suspend to RAM yet it would be good to add a comment
> about this explaining the lack of PM. Otherwise it should be fixed (you
> can take a look at ahci_imx.c and ahci_sunxi.c for examples).
In order to support PM, I will need an other errata fix in libachi.c.
For now PM, will NOT be support.
>
> Otherwise the driver looks good to me and (FWIW) you can add:
>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>
>
> Once the aforementioned issues are fixed.
I will post another version. If you ack'ed that version, Tejun can add
you to the Reviewed-by list.
-Loc
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
