On 4/7/2011 1:23 PM, Balbi, Felipe wrote:
Hi,

On Thu, Apr 07, 2011 at 03:54:47PM +0530, Basheer, Mansoor Ahamed wrote:
@@ -100,6 +102,129 @@ static int __init omap4_l3_init(void)
  }
  postcore_initcall(omap4_l3_init);

+#if defined(CONFIG_SATA_AHCI_PLATFORM) || \
+       defined(CONFIG_SATA_AHCI_PLATFORM_MODULE)
+
+static struct ahci_platform_data omap_sata_pdata;
+static u64 omap_sata_dmamask = DMA_BIT_MASK(32);
+static struct clk *omap_sata_clk;
+
+/* SATA PHY control register offsets */
+#define SATA_P0PHYCR_REG       0x178
+#define SATA_P1PHYCR_REG       0x1F8

prepend all with TI816X_

+#define SATA_PHY_ENPLL(x)      ((x)<<  0)
+#define SATA_PHY_MPY(x)                ((x)<<  1)
+#define SATA_PHY_LB(x)         ((x)<<  5)
+#define SATA_PHY_CLKBYP(x)     ((x)<<  7)
+#define SATA_PHY_RXINVPAIR(x)  ((x)<<  9)
+#define SATA_PHY_LBK(x)                ((x)<<  10)
+#define SATA_PHY_RXLOS(x)      ((x)<<  12)
+#define SATA_PHY_RXCDR(x)      ((x)<<  13)
+#define SATA_PHY_RXEQ(x)       ((x)<<  16)
+#define SATA_PHY_RXENOC(x)     ((x)<<  20)
+#define SATA_PHY_TXINVPAIR(x)  ((x)<<  21)
+#define SATA_PHY_TXCM(x)       ((x)<<  22)
+#define SATA_PHY_TXSWING(x)    ((x)<<  23)

the ones which are single bits, you define as:

#define TI816X_SATA_PHY_TXINVPAIR       (1<<  21)

or

#define TI816X_SATA_PHY_TXINVPAIR       BIT(21)

+#define SATA_PHY_TXDE(x)       ((x)<<  27)
+
+#define TI816X_SATA_BASE       0x4A140000

you should probably define these on some header file. Also SATA_BASE
should be an increment to the global base.

In fact, you should even use hwmod data for that.

+
+static int ti816x_ahci_plat_init(struct device *dev, void __iomem *base)
+{
+       unsigned int phy_val;
+       int ret;
+
+       omap_sata_clk = clk_get(dev, NULL);
+       if (IS_ERR(omap_sata_clk)) {
+               pr_err("ahci : Failed to get SATA clock\n");
+               return PTR_ERR(omap_sata_clk);
+       }

can't you use pm_runtime do achieve this ?

+
+       if (!base) {
+               pr_err("ahci : SATA reg space not mapped, PHY enable failed\n");
+               ret = -ENOMEM;
+               goto err;
+       }
+
+       ret = clk_enable(omap_sata_clk);
+       if (ret) {
+               pr_err("ahci : Clock enable failed\n");
+               goto err;
+       }

As Felipe suggested, pm_runtime is probably the right API for that.
It should even potentially be done in the driver directly since it is a generic API and most platform should probably have to enable "something" at that time.

Benoit


+
+       phy_val = SATA_PHY_ENPLL(1) |
+                       SATA_PHY_MPY(8) |
+                       SATA_PHY_LB(0) |
+                       SATA_PHY_CLKBYP(0) |
+                       SATA_PHY_RXINVPAIR(0) |
+                       SATA_PHY_LBK(0) |
+                       SATA_PHY_RXLOS(1) |
+                       SATA_PHY_RXCDR(4) |
+                       SATA_PHY_RXEQ(1) |
+                       SATA_PHY_RXENOC(1) |
+                       SATA_PHY_TXINVPAIR(0) |
+                       SATA_PHY_TXCM(0) |
+                       SATA_PHY_TXSWING(7) |
+                       SATA_PHY_TXDE(0);

if it's 0, it's same as not even adding them. Please remove the ones
which are 0.

+       writel(phy_val, base + SATA_P0PHYCR_REG);
+       writel(phy_val, base + SATA_P1PHYCR_REG);
+
+       return 0;
+err:
+       clk_put(omap_sata_clk);
+       return ret;
+}
+
+static void ti816x_ahci_plat_exit(struct device *dev)
+{
+       clk_disable(omap_sata_clk);
+       clk_put(omap_sata_clk);
+}
+
+/* resources will be filled by soc specific init routine */
+static struct resource omap_ahci_resources[] = {
+       {
+               .flags  = IORESOURCE_MEM,
+       },
+       {
+               .flags  = IORESOURCE_IRQ,
+       }
+};
+
+static struct platform_device omap_ahci_device = {
+       .name   = "ahci",
+       .dev    = {
+               .platform_data =&omap_sata_pdata,
+               .coherent_dma_mask      = DMA_BIT_MASK(32),
+               .dma_mask               =&omap_sata_dmamask,
+       },
+       .num_resources  = ARRAY_SIZE(omap_ahci_resources),
+       .resource       = omap_ahci_resources,
+};
+
+static void ti816x_ahci_init(void)
+{
+       /* fixup platform device info for TI816X */
+       omap_ahci_resources[0].start    = TI816X_SATA_BASE;
+       omap_ahci_resources[0].end      = TI816X_SATA_BASE + 0x10fff;

weird resource size... 68k... Ok, anyway.. as long as it's correct :-p

+       omap_ahci_resources[1].start    = 16; /* SATA IRQ */
+       omap_sata_pdata.init            = ti816x_ahci_plat_init;
+       omap_sata_pdata.exit            = ti816x_ahci_plat_exit;

why didn't you initialize these when defining the resources ?

you could even drop this function altogether.

+}
+
+static inline void omap_init_ahci(void)
+{
+       if (cpu_is_ti816x()) {
+               ti816x_ahci_init();
+               platform_device_register(&omap_ahci_device);
+       }

this is better the other way around:

        if (!cpu_is_ti816x())
                return;

        platform_device_register(&omap_ahci_device);


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to