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.

> +
> +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;
> +     }
> +
> +     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);

-- 
balbi
--
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