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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b9cc6c7a-0819-fa6a-d4f9-b057dddda5bb@linaro.org>
Date:   Wed, 2 Nov 2016 09:33:41 +0000
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Vivek Gautam <vivek.gautam@...eaurora.org>
Cc:     kishon <kishon@...com>, robh+dt <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 2/2] phy: qcom-qmp: new qmp phy driver for qcom-chipsets



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