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

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

 



Hilman, Kevin wrote:
> "Cousson, Benoit" <b-cousson@xxxxxx> writes:
> 
>> 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.
> 
> Just to second what Felipe and Benoit already said:
> 
> Much of what this patch does duplicates what would happen
> "automatcially" if this was done using omap_hwmod/omap_device +
> runtime PM.  For example, platform_device creation is automatic with
> hwmod data + omap_device build, and clock management is done by
> runtime PM. 
> 
> Kevin

Thanks for your comments.
I will work on the hwmod patch and submit it again.

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


[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux