Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1194754 > unrolled thread

Bug: shared usb dt document is incorrect

Back to article view | Back to linux.kernel

Contents

  Bug:  shared usb dt document is incorrect Tim Bird <tim.bird@sonymobile.com> - 2015-07-29 03:10 +0200
    Re: Bug:  shared usb dt document is incorrect Peter Chen <peter.chen@freescale.com> - 2015-07-29 03:40 +0200
    Re: Bug: shared usb dt document is incorrect Rob Herring <robherring2@gmail.com> - 2015-07-29 05:00 +0200
      RE: Bug: shared usb dt document is incorrect Peter Chen <Peter.Chen@freescale.com> - 2015-07-29 05:20 +0200
        Re: Bug: shared usb dt document is incorrect Rob Herring <robherring2@gmail.com> - 2015-07-29 05:30 +0200
      Re: Bug: shared usb dt document is incorrect Rob Herring <robherring2@gmail.com> - 2015-07-29 22:30 +0200

#1194754 — Bug: shared usb dt document is incorrect

Antoine and Rob,

I was just doing some testing with USB on a Qualcomm SoC.

I followed the instructions in the binding document:
Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt

which has a compatible for "qcom,ci-hdrc", and is, in general,
for chipidea-based USB controllers.

It says in the document that the property usb-phy is deprecated, and to
use phys and phy-names instead.  However, the Qualcomm
driver for this still uses usb-phy.  That driver is in:
drivers/usb/chipidea/ci_hdrc_msm.c

I'm guessing I should update the Qualcomm driver to use
phys and phy-names, but wanted to check with you-all to
verify that this is the preferred method of getting 
phys by phandle now.  It's either change the driver
or make an exception in the binding document, I believe.

I presume I should be changing devm_usb_get_phy_by_phandle()
to of_phy_get(), but let me know if there's more to it than that.

In case I change the driver, do I then update the binding doc
to remove the information about the deprecated property, or not?

Please advise.
 -- Tim

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1194763

On Tue, Jul 28, 2015 at 06:06:30PM -0700, Tim Bird wrote:
> Antoine and Rob,
> 
> I was just doing some testing with USB on a Qualcomm SoC.
> 
> I followed the instructions in the binding document:
> Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> 
> which has a compatible for "qcom,ci-hdrc", and is, in general,
> for chipidea-based USB controllers.
> 
> It says in the document that the property usb-phy is deprecated, and to
> use phys and phy-names instead.  However, the Qualcomm
> driver for this still uses usb-phy.  That driver is in:
> drivers/usb/chipidea/ci_hdrc_msm.c
> 
> I'm guessing I should update the Qualcomm driver to use
> phys and phy-names, but wanted to check with you-all to
> verify that this is the preferred method of getting 
> phys by phandle now.  It's either change the driver
> or make an exception in the binding document, I believe.
> 
> I presume I should be changing devm_usb_get_phy_by_phandle()
> to of_phy_get(), but let me know if there's more to it than that.
> 
> In case I change the driver, do I then update the binding doc
> to remove the information about the deprecated property, or not?
> 
> Please advise.

In my opinion, you can just keep both driver and binding doc unchanging
until the generic PHY supports all things the current USB PHY supports,
eg, .notify_disconnect is used in your PHY driver, but it is not
supported in generic PHY framework.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194777 — Re: Bug: shared usb dt document is incorrect

On Tue, Jul 28, 2015 at 8:06 PM, Tim Bird <tim.bird@sonymobile.com> wrote:
> Antoine and Rob,
>
> I was just doing some testing with USB on a Qualcomm SoC.
>
> I followed the instructions in the binding document:
> Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>
> which has a compatible for "qcom,ci-hdrc", and is, in general,
> for chipidea-based USB controllers.
>
> It says in the document that the property usb-phy is deprecated, and to
> use phys and phy-names instead.  However, the Qualcomm
> driver for this still uses usb-phy.  That driver is in:
> drivers/usb/chipidea/ci_hdrc_msm.c

Deprecated means it still exists in the wild and should be maintained,
but don't use it for new dts files.

> I'm guessing I should update the Qualcomm driver to use
> phys and phy-names, but wanted to check with you-all to
> verify that this is the preferred method of getting
> phys by phandle now.  It's either change the driver
> or make an exception in the binding document, I believe.

That would be fine along with updating the dts files, but the doc
should remain for some time.

> I presume I should be changing devm_usb_get_phy_by_phandle()
> to of_phy_get(), but let me know if there's more to qit than that.

devm_phy_get actually. The driver already supports it. See
ci_hdrc_probe in core.c.

> In case I change the driver, do I then update the binding doc
> to remove the information about the deprecated property, or not?

Not.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194784 — RE: Bug: shared usb dt document is incorrect

IA0KPiA+IEkgd2FzIGp1c3QgZG9pbmcgc29tZSB0ZXN0aW5nIHdpdGggVVNCIG9uIGEgUXVhbGNv
bW0gU29DLg0KPiA+DQo+ID4gSSBmb2xsb3dlZCB0aGUgaW5zdHJ1Y3Rpb25zIGluIHRoZSBiaW5k
aW5nIGRvY3VtZW50Og0KPiA+IERvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy91c2Iv
Y2ktaGRyYy11c2IyLnR4dA0KPiA+DQo+ID4gd2hpY2ggaGFzIGEgY29tcGF0aWJsZSBmb3IgInFj
b20sY2ktaGRyYyIsIGFuZCBpcywgaW4gZ2VuZXJhbCwgZm9yDQo+ID4gY2hpcGlkZWEtYmFzZWQg
VVNCIGNvbnRyb2xsZXJzLg0KPiA+DQo+ID4gSXQgc2F5cyBpbiB0aGUgZG9jdW1lbnQgdGhhdCB0
aGUgcHJvcGVydHkgdXNiLXBoeSBpcyBkZXByZWNhdGVkLCBhbmQNCj4gPiB0byB1c2UgcGh5cyBh
bmQgcGh5LW5hbWVzIGluc3RlYWQuICBIb3dldmVyLCB0aGUgUXVhbGNvbW0gZHJpdmVyIGZvcg0K
PiA+IHRoaXMgc3RpbGwgdXNlcyB1c2ItcGh5LiAgVGhhdCBkcml2ZXIgaXMgaW46DQo+ID4gZHJp
dmVycy91c2IvY2hpcGlkZWEvY2lfaGRyY19tc20uYw0KPiANCj4gRGVwcmVjYXRlZCBtZWFucyBp
dCBzdGlsbCBleGlzdHMgaW4gdGhlIHdpbGQgYW5kIHNob3VsZCBiZSBtYWludGFpbmVkLCBidXQg
ZG9uJ3QNCj4gdXNlIGl0IGZvciBuZXcgZHRzIGZpbGVzLg0KPiANCg0KQnV0IGhvdyB0aGUgbmV3
IFNvQyAoZm9yIG5ldyBkdHMpIHdoaWNoIHN0aWxsIHVzZXMgY3VycmVudCBVU0IgUEhZIGRyaXZl
cj8NCg0KTWF5YmUgd2Ugc2hvdWxkIG5vdCBhZGQgYWRkaXRpb25hbCBkZXNjcmlwdGlvbiBmb3Ig
dGhpcyBwcm9wZXJ0eSwNCml0IGRlcGVuZHMgb24gd2hpY2ggUEhZIGRyaXZlciAoR2VuZXJpYyBQ
SFkgdnMgVVNCIFBIWSkgIHRoaXMgY29udHJvbGxlcg0Kd2lsbCB1c2UsIGlmIGl0IHVzZXMgYSBu
ZXcgUEhZIGRyaXZlciwgaXQgbXVzdCB1c2UgZ2VuZXJpYyBQSFkgZnJhbWV3b3JrLCAgYW5kIHdp
bGwNCnVzZSAncGh5JyBhbmQgJ3BoeS1uYW1lcycgY29ycmVzcG9uZGluZy4NCg0KUGV0ZXINCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1194786 — Re: Bug: shared usb dt document is incorrect

On Tue, Jul 28, 2015 at 10:12 PM, Peter Chen <Peter.Chen@freescale.com> wrote:
>
>> > I was just doing some testing with USB on a Qualcomm SoC.
>> >
>> > I followed the instructions in the binding document:
>> > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>> >
>> > which has a compatible for "qcom,ci-hdrc", and is, in general, for
>> > chipidea-based USB controllers.
>> >
>> > It says in the document that the property usb-phy is deprecated, and
>> > to use phys and phy-names instead.  However, the Qualcomm driver for
>> > this still uses usb-phy.  That driver is in:
>> > drivers/usb/chipidea/ci_hdrc_msm.c
>>
>> Deprecated means it still exists in the wild and should be maintained, but don't
>> use it for new dts files.
>>
>
> But how the new SoC (for new dts) which still uses current USB PHY driver?

That's fine to use, but I would encourage people not to.

> Maybe we should not add additional description for this property,
> it depends on which PHY driver (Generic PHY vs USB PHY)  this controller
> will use, if it uses a new PHY driver, it must use generic PHY framework,  and will
> use 'phy' and 'phy-names' corresponding.

We're thinking about this the wrong way around. The driver should not
determine the binding used. The binding should be independent of the
driver used.

Either we should make the generic phy subsystem have feature parity
with usb-phy, or make the usb-phy drivers use the same generic phy
binding. Long term, I think we want the former and need a way to
subclass the generic phy. Other phys have similar needs for additional
protocol specific functions (e.g. configure the # of lanes for PCIe).
The generic phy subsystem needs some love in my brief experience with
it. For the latter case, this would make switching which phy driver is
used transparent to DT.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195477 — Re: Bug: shared usb dt document is incorrect

On Wed, Jul 29, 2015 at 12:29 PM, Tim Bird <tim.bird@sonymobile.com> wrote:
>
>
> On 07/28/2015 07:54 PM, Rob Herring wrote:
>> On Tue, Jul 28, 2015 at 8:06 PM, Tim Bird <tim.bird@sonymobile.com> wrote:
>>> Antoine and Rob,
>>>
>>> I was just doing some testing with USB on a Qualcomm SoC.
>>>
>>> I followed the instructions in the binding document:
>>> Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>>>
>>> which has a compatible for "qcom,ci-hdrc", and is, in general,
>>> for chipidea-based USB controllers.
>>>
>>> It says in the document that the property usb-phy is deprecated, and to
>>> use phys and phy-names instead.  However, the Qualcomm
>>> driver for this still uses usb-phy.  That driver is in:
>>> drivers/usb/chipidea/ci_hdrc_msm.c
>>
>> Deprecated means it still exists in the wild and should be maintained,
>> but don't use it for new dts files.
> OK.  But for the Qualcomm driver, the new binding doesn't work (see below).
> Since the binding doc specifically says: 'Use "phys" instead [of "usb-phy"].',
> it is wrong as currently written, for this controller/phy combination.

Perhaps we need to specify which compatible strings still use the old binding.

>>> I'm guessing I should update the Qualcomm driver to use
>>> phys and phy-names, but wanted to check with you-all to
>>> verify that this is the preferred method of getting
>>> phys by phandle now.  It's either change the driver
>>> or make an exception in the binding document, I believe.
>>
>> That would be fine along with updating the dts files, but the doc
>> should remain for some time.
>
> Does this mean I should try phys/phy-names, and if that doesn't work
> try usb-phy, for backwards compatibility?
>
>>> I presume I should be changing devm_usb_get_phy_by_phandle()
>>> to of_phy_get(), but let me know if there's more to it than that.
>>
>> devm_phy_get actually. The driver already supports it. See
>> ci_hdrc_probe in core.c.
>
> In my case the probe that is running is ci_hdrc_msm_probe(), in ci_hdrc_msm.c
> This probe uses devm_usb_get_phy_by_phandle(..."usb-phy").  If I specify phys/phy-names
> in my dts, the probe fails silently - leading ultimately to USB not working.
> If I specify "usb-phy" in my dts, everything works. In this case, that property
> is NOT deprecated.  It's the only one that works.

In my mind deprecated means "not recommended to be used", not that it
can't be used. If you were doing a new SOC, then I'd tell you don't
use usb-phy, but in this case as usb-phy is still used on QC SOCs I
think it is okay.

> I don't understand what's going on with the different probe routines and how the
> SoC-specific parts interact with the core.  I just know that the kernel behaviour
> and the binding doc don't match, and I'd like to help fix it if I can.

I believe the msm probe calls the core which then creates sub-devices
for the host and device sides. Then the host and device drivers are
probed. The SOC probe can fill in a phy or the core code will do it.
The problem with the core code is that it will not do a deferred
probe. It may still work though if you remove this from the msm probe:

        phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
        if (IS_ERR(phy))
                return PTR_ERR(phy);

        ci_hdrc_msm_platdata.usb_phy = phy;


You can support the old and new bindings just by doing:

phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
if (IS_ERR(phy) == -EPROBE_DEFER)
  return PTR_ERR(phy);
else {
  phy = devm_usb_get_phy_by_phandle(&pdev->dev, "phys", 0);
  if (IS_ERR(phy))
    return PTR_ERR(phy);
}

Not the prettiest code. If the QC maintainers don't care about
compatibility with old dtb's then you could just switch everything
(dts's and driver) to use phys instead.

Perhaps this should be in a wrapper function as I'd like to see all
users of usb-phy migrate to phys and the driver shouldn't really care
what the property name is.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web