All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Gautam <vivek.gautam@codeaurora.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Kishon Vijay Abraham I <kishon@ti.com>
Cc: robh+dt <robh+dt@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
Date: Mon, 23 Jan 2017 15:43:06 +0530	[thread overview]
Message-ID: <CAFp+6iG3KxX=G64EyJFgGn9ZffzjCtEeR4HUPdOK-uhE8A=PwA@mail.gmail.com> (raw)
In-Reply-To: <20170118180347.GO10531@minitux>

On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Wed 18 Jan 01:13 PST 2017, Vivek Gautam wrote:
>> On 01/16/2017 02:15 PM, Kishon Vijay Abraham I wrote:
>> > Hi,
>> >
>> > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> [..]
>> > > +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
>> > > +};
>> > I wish all this data comes from device tree and one API in phy-core can do all
>> > these settings. Your other driver qcom-qmp also seems to have a bunch of
>> > similar settings.
>> >
>> > The problem is every vendor driver adds a bunch of code to perform the same
>> > thing again and again when all of these settings can be done by a single phy API.
>>
>> Yes, i understand this. You have commented similar thing in the patch from
>> Jaehoon -
>> [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
>>
>> I would like to understand the requirements here.
>> Would you like me to get all this information from the device tree -
>> an array of register offset and value pair, which we can then program
>> by calling a phy_ops (may be calibrate) ? Something of this sort:
>>
>> phy-calibrate-data = <val1, register_offset1>,
>>                                   <val2, register_offset2>,
>>                                   <val3, register_offset3>,
>>                                   ....
>>
>> I am sure having such information in the driver (like i have in my patch)
>> makes the driver look more clumsy.
>> But, all this data can be pretty huge - a set of some 100+ register-value
>> pairs
>> for QMP phy, for example. So, will it be okay to get this from device tree ?
>> We also note here that such information changes from one IP version to
>> another.
>> I remember Rob having some concerns about it.
>>
>
> The devicetree is supposed to describe which hardware components a
> certain device has, most of the time this carries a set of properties to
> describe how this piece is connected and configured.
>
> A dump of magic register values does not describe how the QMP is
> connected to anything and is, as far as this patch shows, static for
> this particular hardware block.

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.

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

That's right. These register-value pairs (100+ for qmp) don't give a human
understandable data when put in dts.

Kishon,
Please let me know if you have concerns.

If this looks good otherwise, please consider taking this for 4.11.


Regards
Vivek
>
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> 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

  reply	other threads:[~2017-01-23 10:13 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-10 10:51 [PATCH v4 0/4] phy: USB and PCIe phy drivers for Qcom chipsets Vivek Gautam
2017-01-10 10:51 ` Vivek Gautam
2017-01-10 10:51 ` [PATCH v4 1/4] dt-bindings: phy: Add support for QUSB2 phy Vivek Gautam
2017-01-10 10:51 ` [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips Vivek Gautam
     [not found]   ` <1484045519-19030-3-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-01-16  8:45     ` Kishon Vijay Abraham I
2017-01-16  8:45       ` Kishon Vijay Abraham I
2017-01-18  9:13       ` Vivek Gautam
2017-01-18 18:03         ` Bjorn Andersson
2017-01-23 10:13           ` Vivek Gautam [this message]
2017-01-24  9:19             ` Kishon Vijay Abraham I
2017-01-24  9:19               ` Kishon Vijay Abraham I
2017-01-26 18:15               ` Bjorn Andersson
2017-01-27  6:24                 ` Vivek Gautam
2017-02-22  3:59                   ` Vivek Gautam
2017-03-02 16:40                     ` Vivek Gautam
2017-03-07  9:04                       ` Kishon Vijay Abraham I
2017-03-07  9:04                         ` Kishon Vijay Abraham I
     [not found]                         ` <58BE77A1.6090502-l0cyMroinI0@public.gmane.org>
2017-03-07  9:26                           ` Vivek Gautam
2017-03-07  9:26                             ` Vivek Gautam
2017-01-10 10:51 ` [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy Vivek Gautam
2017-01-16  8:49   ` Kishon Vijay Abraham I
2017-01-16  8:49     ` Kishon Vijay Abraham I
2017-01-18  6:54     ` Vivek Gautam
     [not found]       ` <50612693-5345-55da-8207-8c5e721fb68a-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-01-18 18:22         ` Bjorn Andersson
2017-01-18 18:22           ` Bjorn Andersson
2017-01-19  0:40           ` Stephen Boyd
     [not found]             ` <20170119004028.GA4857-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-01-19  5:12               ` Vivek Gautam
2017-01-19  5:12                 ` Vivek Gautam
2017-01-19 21:42                 ` Stephen Boyd
2017-01-23 12:22                   ` Vivek Gautam
     [not found]                   ` <20170119214258.GD7829-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-01-24  9:33                     ` Kishon Vijay Abraham I
2017-01-24  9:33                       ` Kishon Vijay Abraham I
2017-01-24 14:05                       ` Vivek Gautam
     [not found]                         ` <CAFp+6iGBzUEFM-MjqerhoVWjFF8wahgwC_rnB6GMd2VsMuDm6g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-01-24 14:15                           ` Kishon Vijay Abraham I
2017-01-24 14:15                             ` Kishon Vijay Abraham I
     [not found]                             ` <5887619B.70108-l0cyMroinI0@public.gmane.org>
2017-01-24 16:40                               ` Vivek Gautam
2017-01-24 16:40                                 ` Vivek Gautam
2017-01-26 23:43                         ` Stephen Boyd
2017-01-27  5:16                           ` Vivek Gautam
2017-03-07 14:00                             ` Stephen Boyd
2017-03-08  6:45                               ` Vivek Gautam
2017-01-10 10:51 ` [PATCH v4 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets Vivek Gautam
     [not found]   ` <1484045519-19030-5-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-01-10 23:20     ` Andy Gross
2017-01-10 23:20       ` Andy Gross
2017-01-11  3:36       ` Vivek Gautam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFp+6iG3KxX=G64EyJFgGn9ZffzjCtEeR4HUPdOK-uhE8A=PwA@mail.gmail.com' \
    --to=vivek.gautam@codeaurora.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kishon@ti.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=srinivas.kandagatla@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.