Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

Hi Kishon,


On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
<vivek.gautam@xxxxxxxxxxxxxx> wrote:
>
>
> On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
>>
>> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>>>
>>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>>>
>>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
>>
>> [..]
>>>>
>>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>>> of static values for a particular IP version. These values hardly give a
>>>> meaningful data to put few phy bindings that could be referenced
>>>> to configure the phy further.
>>>
>>> Not really. You can have clearly defined phy binding to give meaningful
>>> data.
>>> Every driver doing the same configuration bloats the driver and these
>>> configuration values are just magic values which hardly can be reviewed
>>> by anyone.
>>>>>
>>>>> Further more moving this blob to devicetree will not allow us to treat
>>>>> the various QMP configurations as one HW block, as there are other
>>>>> differences as well.
>>>>>
>>>>> Like many other drivers it's possible to create a generic version that
>>>>> has every bit of logic driven by configuration from devicetree, but
>>>>> like
>>>>> most of those cases this is not the way we split things.
>>>>>
>>>>> And this has the side effect of keeping the dts files human readable,
>>>>> human understandable and human maintainable.
>>>
>>> right. That's why I recommend having clearly defined bindings.
>>>         phy,tx-<param1> = <val, offset, mask>
>>>         phy,tx-<param2> = <val, offset, mask>
>>>         phy,tx-<param3> = <val, offset, mask>
>>
>> There's no doubt that this table needs to be encoded somewhere, so the
>> question is should we hard code this in a C file or in a DTSI file.
>>
>> Skimming through [1] I see examples of things that differs based on how
>> the specific component is integrated in a SoC or on a particular board -
>> properties that are relevant to a "system integrator".
>>
>> As far as I can tell this blob will, if ever, only be changed by a
>> driver developer and as such it's not carry information about how this
>> component relates to the rest of the system and should as such not be
>> part of the device tree.
>>
>>
>> If there are properties of the hardware that is affected by how the
>> component is integrated in the system I really would like for those to
>> be exposed as human-readable properties that I can understand and alter
>> without deep knowledge about the register map of the hardware block.
>
>
> I am reaching out to our internal teams to get more information
> on different possible phy configurations, based on which the registers
> values are decided.
> This is something that i tried to understand in the past as well, but
> couldn't
> grab much information that time.
> Will come back with relevant information on this.
>

We have started looking into understanding the PHYs on msm and
eventually create a set of generic phy bindings that can serve multiple
platforms.
But this task, I presume, will take its course and will involve multi-party
discussions.

For these QUSB2 and QMP phy drivers, a good amount of work has
already gone in getting these drivers in upstream state.
The common QMP phy driver supports a bunch of controllers on msm
platforms - USB, PCIe and UFS and there are platforms such as DB820c
and others that want to pull in these changes from upstream.
The future phy controllers also depend on these drivers and we don't want
to hold other developers to contribute to these drivers.
So, we wish to not delay these drivers further because of the phy bindings.
I see that there are phys that still program the registers-value pairs for
phy initialization.

We will keep working on the bindings while these patches
make way to upstream.

Can you consider pulling in these drivers?
I can send the next version of these drivers with other comments addressed.
Please let me know your comments.


Regards
Vivek

>
> Regards
> Vivek
>
>>> [1] -> https://www.linuxplumbersconf.org/2016/ocw/proposals/4047
>>
>> Regards,
>> Bjorn
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
>> in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project