Re: [PATCH 2/2] phy: qcom-qmp: new qmp phy driver for qcom-chipsets

[Srinivas Kandagatla]

On 02/11/16 07:26, Vivek Gautam wrote:
+               phy_desc->tx = base + qphy->cfg->tx_offsets[id];
>> +               phy_desc->rx = base + qphy->cfg->rx_offsets[id];
>> +               phy_desc->pcs = base + qphy->cfg->pcs_offsets[id];
>> +
>> +               /*
>> +                * Get PHY's Pipe clock, if any; USB3 and PCIe are PIPE
>> +                * based phys, so they essentially have pipe clock
>> +                */
>> +               memset(&prop_name, 0, sizeof(prop_name));
>> +               snprintf(prop_name, MAX_PROP_NAME, "pipe%d", id);
>> +               phy_desc->pipe_clk = devm_clk_get(dev, prop_name);
>> +               if (IS_ERR(phy_desc->pipe_clk)) {
>> +                       if (qphy->cfg->type == PHY_TYPE_PCIE ||
>> +                           qphy->cfg->type == PHY_TYPE_USB3) {
>> +                               ret = PTR_ERR(phy_desc->pipe_clk);
>> +                               if (ret != -EPROBE_DEFER)
>> +                                       dev_err(dev,
>> +                                       "failed to get lane%d pipe_clk\n",
>> id);
>> +                               return ret;
>> +                       } else {
>> +                               phy_desc->pipe_clk = NULL;
>> +                       }
>> +               }
>
>
> Do we need to really need phy type for this?
> You should proably make a flag in cfg to mark this rather then using phy
> type. Hopefully it will also simplify the logic.
The pipe clock is something that's specific to PCIe and USB3 type
phys (both are PIPE3 specs based phys). So i think it is okay
to use the PHY_TYPE_{*} macros here. Looks a bit cleaner rather
than having a cfg flag. No ?
Yes, it makes sense, I was not sure of full details of the pipe3 specs, 
now that you made it more clear.
Remember to add this in comments.

Thanks,
srini

Using PHY_TYPE_{*} macros will also keep the doors open to
other PHY types that are added at a later point in time.

>