lkml.org 
[lkml]   [2014]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/2] phy: qcom: Add driver for QCOM APQ8064 SATA PHY
Thankyou for the comments,

On 11/07/14 13:38, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Wednesday, July 09, 2014 12:04:06 PM Srinivas Kandagatla wrote:
>> Add a PHY driver for uses with AHCI based SATA controller driver on the
>> APQ8064 family of SoCs.
>>
>> This patch is a forward port from Qualcomm's v3.4 andriod kernel.
>
> Android?
yep.
>
> unused define, please remove it
>
Will fix these instances in next version.
>> +
>> +#define UNIPHY_PLL_LOCK BIT(0)
>> +#define SATA_PHY_TX_CAL BIT(0)
>> +#define SATA_PHY_RX_CAL BIT(0)
>> +
>> +/* default timeout set to 1 sec */
>> +#define TIMEOUT_MS 10000
>> +
>> +struct qcom_apq8064_sata_phy {
>> + void __iomem *mmio;
>> + struct clk *cfg_clk;
>> + struct device *dev;
>> +};
>> +
>> +/* Helper function to do poll and timeout */
>> +static int read_poll_timeout(void __iomem *addr, u32 mask)
>> +{
>> + unsigned long timeout = jiffies + msecs_to_jiffies(TIMEOUT_MS);
>> + u32 val;
>> +
>> + do {
>> + cpu_relax();
>> + val = readl_relaxed(addr);
>> + if (val & mask)
>> + break;
>> + } while (!time_after(jiffies, timeout));
>
> It would be better to use usleep_[range]() (or even msleep() if needed)
> instead of just using cpu_relax().

We really want to poll the register here, usleep/msleep would be useful
if we know already know how much delay is required, but in this case the
its not true.

>> +
>> +static int qcom_apq8064_sata_phy_remove(struct platform_device *pdev)
>> +{
>> + struct qcom_apq8064_sata_phy *phy = platform_get_drvdata(pdev);
>
> Doesn't this require to use platform_set_drvdata() first in the ->probe
> method?
Good spot, I will fix this in next version.

Thanks,
srini


\
 
 \ /
  Last update: 2014-07-11 16:21    [W:0.074 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site