Re: [PATCH] OMAP: TI816X: Add SATA support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux