From mboxrd@z Thu Jan  1 00:00:00 1970
From: Kishon Vijay Abraham I <kishon@ti.com>
Subject: Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom
 chips
Date: Tue, 7 Mar 2017 14:34:33 +0530
Message-ID: <58BE77A1.6090502@ti.com>
References: <1484045519-19030-1-git-send-email-vivek.gautam@codeaurora.org>
 <1484045519-19030-3-git-send-email-vivek.gautam@codeaurora.org>
 <587C880E.90803@ti.com> <73a2f6ce-69d6-8d15-b28d-891bdf16672c@codeaurora.org>
 <20170118180347.GO10531@minitux>
 <CAFp+6iG3KxX=G64EyJFgGn9ZffzjCtEeR4HUPdOK-uhE8A=PwA@mail.gmail.com>
 <58871C39.50403@ti.com> <20170126181527.GB10531@minitux>
 <692975a4-e735-975f-7152-fdd81324c42b@codeaurora.org>
 <CAFp+6iHvgPVSbZ-uGRAHK3Wh+k7Apy+U7YcSVrgegx-AJ1=ZTA@mail.gmail.com>
 <CAFp+6iFdSaJYz+eQ0Vwqk0dtqmhAVTZLCNNYWW6gpcJwatgEfA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Return-path: <linux-arm-msm-owner@vger.kernel.org>
Received: from fllnx209.ext.ti.com ([198.47.19.16]:42470 "EHLO
        fllnx209.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1754483AbdCGJGZ (ORCPT
        <rfc822;linux-arm-msm@vger.kernel.org>);
        Tue, 7 Mar 2017 04:06:25 -0500
In-Reply-To: <CAFp+6iFdSaJYz+eQ0Vwqk0dtqmhAVTZLCNNYWW6gpcJwatgEfA@mail.gmail.com>
Sender: linux-arm-msm-owner@vger.kernel.org
List-Id: linux-arm-msm@vger.kernel.org
To: Vivek Gautam <vivek.gautam@codeaurora.org>
Cc: robh+dt <robh+dt@kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.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

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@codeaurora.org> wrote:
>> Hi Kishon,
>>
>>
>> On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
>> <vivek.gautam@codeaurora.org> 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.

Thanks
Kishon

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755214AbdCGJHM (ORCPT <rfc822;w@1wt.eu>);
        Tue, 7 Mar 2017 04:07:12 -0500
Received: from fllnx209.ext.ti.com ([198.47.19.16]:42470 "EHLO
        fllnx209.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1754483AbdCGJGZ (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 7 Mar 2017 04:06:25 -0500
Subject: Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom
 chips
To: Vivek Gautam <vivek.gautam@codeaurora.org>
References: <1484045519-19030-1-git-send-email-vivek.gautam@codeaurora.org>
 <1484045519-19030-3-git-send-email-vivek.gautam@codeaurora.org>
 <587C880E.90803@ti.com> <73a2f6ce-69d6-8d15-b28d-891bdf16672c@codeaurora.org>
 <20170118180347.GO10531@minitux>
 <CAFp+6iG3KxX=G64EyJFgGn9ZffzjCtEeR4HUPdOK-uhE8A=PwA@mail.gmail.com>
 <58871C39.50403@ti.com> <20170126181527.GB10531@minitux>
 <692975a4-e735-975f-7152-fdd81324c42b@codeaurora.org>
 <CAFp+6iHvgPVSbZ-uGRAHK3Wh+k7Apy+U7YcSVrgegx-AJ1=ZTA@mail.gmail.com>
 <CAFp+6iFdSaJYz+eQ0Vwqk0dtqmhAVTZLCNNYWW6gpcJwatgEfA@mail.gmail.com>
CC: robh+dt <robh+dt@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.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>
From: Kishon Vijay Abraham I <kishon@ti.com>
Message-ID: <58BE77A1.6090502@ti.com>
Date: Tue, 7 Mar 2017 14:34:33 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.2
MIME-Version: 1.0
In-Reply-To: <CAFp+6iFdSaJYz+eQ0Vwqk0dtqmhAVTZLCNNYWW6gpcJwatgEfA@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@codeaurora.org> wrote:
>> Hi Kishon,
>>
>>
>> On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
>> <vivek.gautam@codeaurora.org> 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.

Thanks
Kishon