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

From: Vivek Gautam
Date: Tue Mar 07 2017 - 04:27:28 EST


Hi,


On 03/07/2017 02:34 PM, Kishon Vijay Abraham I wrote:
Hi,

On Thursday 02 March 2017 10:10 PM, Vivek Gautam wrote:
Hi Kishon,


On Wed, Feb 22, 2017 at 9:29 AM, Vivek Gautam
<vivek.gautam@xxxxxxxxxxxxxx> wrote:
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.
Gentle ping. Any thoughts on this ?
sure, lets hold the phy configuration binding and complete the rest of the patch.

Sure, thanks. Will post out the next version of patches soon.

Regards
Vivek


Thanks
Kishon

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