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