All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
To: Arnd Bergmann <arnd@arndb.de>,
	<linux-arm-kernel@lists.infradead.org>,
	Alan Stern <stern@rowland.harvard.edu>
Cc: Mark Rutland <mark.rutland@arm.com>, <devicetree@vger.kernel.org>,
	"Scott Branden" <sbranden@broadcom.com>,
	Pawel Moll <pawel.moll@arm.com>,
	"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
	Ray Jui <rjui@broadcom.com>, <linux-kernel@vger.kernel.org>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Jonathan Richardson <jonathar@broadcom.com>,
	Rob Herring <robh+dt@kernel.org>,
	<bcm-kernel-feedback-list@broadcom.com>,
	"Dmitry Torokhov" <dtor@google.com>,
	Kumar Gala <galak@codeaurora.org>,
	"Anatol Pomazau" <anatol@google.com>
Subject: Re: [PATCH 1/2] phy: usbphy: Add dt documentation for Broadcom Cygnus USB PHY driver
Date: Wed, 25 Feb 2015 16:24:06 -0800	[thread overview]
Message-ID: <54EE67A6.609@broadcom.com> (raw)
In-Reply-To: <54E53278.70705@broadcom.com>

Hello Alan and Arnd

I wanted to follow up on this patch and ascertain what I would have to 
change. Please see below for my questions

On 15-02-18 04:46 PM, Arun Ramamurthy wrote:
>
>
> On 15-02-18 07:15 AM, Arnd Bergmann wrote:
>> On Tuesday 17 February 2015 13:05:50 Arun Ramamurthy wrote:
>>> On 15-02-17 12:53 PM, Arnd Bergmann wrote:
>>>> On Tuesday 17 February 2015 12:00:49 Arun Ramamurthy wrote:
>>>>> Arnd, I patched the ehci and ohci driver to accept multiple phys so
>>>>> they
>>>>> require different names and cannot both be "usb". That patch was
>>>>> accepted by Alen Stern but I did not update the bindings
>>>>> documentation.
>>>>> I will send out another patch for that. Could we go with the naming
>>>>> scheme of "usb" + "p" + port number or do you have other suggestions?
>>>>
>>>> I don't have a good idea, but I think it would be best if the first
>>>> phy could remain named "usb" for compatibility with the existing
>>>> binding.
>>>>
>>> The patch was written in a way that all the existing and new drivers can
>>> continue to use "usb" if they are using only one phy so that we remain
>>> compatible. The names need to be different only if more than one phy is
>>> specified. In such cases i don't think the first phy should be "usb" as
>>> it would be confusing to have
>>>          phy-names = "usb","usbp1"
>>
>> I see your patch now, as 7e7a0e67f2c ("usb: ehci-platform: add support
>> for
>> multiple phys per controller"), and I'm not too happy about the way you
>> did this.
>
>> We already concluded that there should have been a binding change
>> to go along with this, and that would have caught the fact that you
>> circumvent the API here by reading the phy names manually. That
>> part should never have made it into the kernel.
>>
>> I think we can do this either by defining specific names for the
>> phy, or by changing the generic PHY binding to allow anonymous
>> phy references (leaving out "phy-names" entirely), and adding a
>> proper API for that.
>>
> Thanks Arnd, I will wait for Alan's comments before proceeding. I am
> happy to patch the ehci-platform driver to use a new api instead of
> devm_phy_get if that is the best option.
>
>>> Should I run this by Alan Stern?
>>
>> I've added him to Cc here. He clearly didn't know the background about
>> the DT binding change, and should not need to, but he may have an opinion
>> on what names we should use.
>>
>
Arnd, should I re patch the ehci-platform driver to avoid phy-names 
entirely? Alan, if not do you have an opinion on what the usb phy names 
should be? The current patch uses "usbp" + port number such as "usbp0" , 
"usbp1" etc

>>>> What is the reason for having two phys in your case? Are these
>>>> identical phy devices connected to a single controller or do they
>>>> server different purposes?
>>>>
>>> Yes, we have three identical phys connected to a single host controller
>>> and one of the phys is also connected to the device controller
>>
>> Ok, no problem with that, let's just make sure we come up with a
>> good binding for it.
>>
Arnd do you have any other comments on the phy driver itself? Thank you
>>     Arnd
>>

WARNING: multiple messages have this Message-ID (diff)
From: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
To: Arnd Bergmann <arnd@arndb.de>,
	linux-arm-kernel@lists.infradead.org,
	Alan Stern <stern@rowland.harvard.edu>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Scott Branden <sbranden@broadcom.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Ray Jui <rjui@broadcom.com>,
	linux-kernel@vger.kernel.org,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Jonathan Richardson <jonathar@broadcom.com>,
	Rob Herring <robh+dt@kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	Dmitry Torokhov <dtor@google.com>,
	Kumar Gala <galak@codeaurora.org>,
	Anatol Pomazau <anatol@google.com>
Subject: Re: [PATCH 1/2] phy: usbphy: Add dt documentation for Broadcom Cygnus USB PHY driver
Date: Wed, 25 Feb 2015 16:24:06 -0800	[thread overview]
Message-ID: <54EE67A6.609@broadcom.com> (raw)
In-Reply-To: <54E53278.70705@broadcom.com>

Hello Alan and Arnd

I wanted to follow up on this patch and ascertain what I would have to 
change. Please see below for my questions

On 15-02-18 04:46 PM, Arun Ramamurthy wrote:
>
>
> On 15-02-18 07:15 AM, Arnd Bergmann wrote:
>> On Tuesday 17 February 2015 13:05:50 Arun Ramamurthy wrote:
>>> On 15-02-17 12:53 PM, Arnd Bergmann wrote:
>>>> On Tuesday 17 February 2015 12:00:49 Arun Ramamurthy wrote:
>>>>> Arnd, I patched the ehci and ohci driver to accept multiple phys so
>>>>> they
>>>>> require different names and cannot both be "usb". That patch was
>>>>> accepted by Alen Stern but I did not update the bindings
>>>>> documentation.
>>>>> I will send out another patch for that. Could we go with the naming
>>>>> scheme of "usb" + "p" + port number or do you have other suggestions?
>>>>
>>>> I don't have a good idea, but I think it would be best if the first
>>>> phy could remain named "usb" for compatibility with the existing
>>>> binding.
>>>>
>>> The patch was written in a way that all the existing and new drivers can
>>> continue to use "usb" if they are using only one phy so that we remain
>>> compatible. The names need to be different only if more than one phy is
>>> specified. In such cases i don't think the first phy should be "usb" as
>>> it would be confusing to have
>>>          phy-names = "usb","usbp1"
>>
>> I see your patch now, as 7e7a0e67f2c ("usb: ehci-platform: add support
>> for
>> multiple phys per controller"), and I'm not too happy about the way you
>> did this.
>
>> We already concluded that there should have been a binding change
>> to go along with this, and that would have caught the fact that you
>> circumvent the API here by reading the phy names manually. That
>> part should never have made it into the kernel.
>>
>> I think we can do this either by defining specific names for the
>> phy, or by changing the generic PHY binding to allow anonymous
>> phy references (leaving out "phy-names" entirely), and adding a
>> proper API for that.
>>
> Thanks Arnd, I will wait for Alan's comments before proceeding. I am
> happy to patch the ehci-platform driver to use a new api instead of
> devm_phy_get if that is the best option.
>
>>> Should I run this by Alan Stern?
>>
>> I've added him to Cc here. He clearly didn't know the background about
>> the DT binding change, and should not need to, but he may have an opinion
>> on what names we should use.
>>
>
Arnd, should I re patch the ehci-platform driver to avoid phy-names 
entirely? Alan, if not do you have an opinion on what the usb phy names 
should be? The current patch uses "usbp" + port number such as "usbp0" , 
"usbp1" etc

>>>> What is the reason for having two phys in your case? Are these
>>>> identical phy devices connected to a single controller or do they
>>>> server different purposes?
>>>>
>>> Yes, we have three identical phys connected to a single host controller
>>> and one of the phys is also connected to the device controller
>>
>> Ok, no problem with that, let's just make sure we come up with a
>> good binding for it.
>>
Arnd do you have any other comments on the phy driver itself? Thank you
>>     Arnd
>>

WARNING: multiple messages have this Message-ID (diff)
From: arun.ramamurthy@broadcom.com (Arun Ramamurthy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] phy: usbphy: Add dt documentation for Broadcom Cygnus USB PHY driver
Date: Wed, 25 Feb 2015 16:24:06 -0800	[thread overview]
Message-ID: <54EE67A6.609@broadcom.com> (raw)
In-Reply-To: <54E53278.70705@broadcom.com>

Hello Alan and Arnd

I wanted to follow up on this patch and ascertain what I would have to 
change. Please see below for my questions

On 15-02-18 04:46 PM, Arun Ramamurthy wrote:
>
>
> On 15-02-18 07:15 AM, Arnd Bergmann wrote:
>> On Tuesday 17 February 2015 13:05:50 Arun Ramamurthy wrote:
>>> On 15-02-17 12:53 PM, Arnd Bergmann wrote:
>>>> On Tuesday 17 February 2015 12:00:49 Arun Ramamurthy wrote:
>>>>> Arnd, I patched the ehci and ohci driver to accept multiple phys so
>>>>> they
>>>>> require different names and cannot both be "usb". That patch was
>>>>> accepted by Alen Stern but I did not update the bindings
>>>>> documentation.
>>>>> I will send out another patch for that. Could we go with the naming
>>>>> scheme of "usb" + "p" + port number or do you have other suggestions?
>>>>
>>>> I don't have a good idea, but I think it would be best if the first
>>>> phy could remain named "usb" for compatibility with the existing
>>>> binding.
>>>>
>>> The patch was written in a way that all the existing and new drivers can
>>> continue to use "usb" if they are using only one phy so that we remain
>>> compatible. The names need to be different only if more than one phy is
>>> specified. In such cases i don't think the first phy should be "usb" as
>>> it would be confusing to have
>>>          phy-names = "usb","usbp1"
>>
>> I see your patch now, as 7e7a0e67f2c ("usb: ehci-platform: add support
>> for
>> multiple phys per controller"), and I'm not too happy about the way you
>> did this.
>
>> We already concluded that there should have been a binding change
>> to go along with this, and that would have caught the fact that you
>> circumvent the API here by reading the phy names manually. That
>> part should never have made it into the kernel.
>>
>> I think we can do this either by defining specific names for the
>> phy, or by changing the generic PHY binding to allow anonymous
>> phy references (leaving out "phy-names" entirely), and adding a
>> proper API for that.
>>
> Thanks Arnd, I will wait for Alan's comments before proceeding. I am
> happy to patch the ehci-platform driver to use a new api instead of
> devm_phy_get if that is the best option.
>
>>> Should I run this by Alan Stern?
>>
>> I've added him to Cc here. He clearly didn't know the background about
>> the DT binding change, and should not need to, but he may have an opinion
>> on what names we should use.
>>
>
Arnd, should I re patch the ehci-platform driver to avoid phy-names 
entirely? Alan, if not do you have an opinion on what the usb phy names 
should be? The current patch uses "usbp" + port number such as "usbp0" , 
"usbp1" etc

>>>> What is the reason for having two phys in your case? Are these
>>>> identical phy devices connected to a single controller or do they
>>>> server different purposes?
>>>>
>>> Yes, we have three identical phys connected to a single host controller
>>> and one of the phys is also connected to the device controller
>>
>> Ok, no problem with that, let's just make sure we come up with a
>> good binding for it.
>>
Arnd do you have any other comments on the phy driver itself? Thank you
>>     Arnd
>>

  reply	other threads:[~2015-02-26  0:22 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-17 19:20 [PATCH 0/2] USB PHY driver for Broadcom's Cygnus chipset Arun Ramamurthy
2015-02-17 19:20 ` Arun Ramamurthy
2015-02-17 19:20 ` Arun Ramamurthy
2015-02-17 19:20 ` [PATCH 1/2] phy: usbphy: Add dt documentation for Broadcom Cygnus USB PHY driver Arun Ramamurthy
2015-02-17 19:20   ` Arun Ramamurthy
2015-02-17 19:20   ` Arun Ramamurthy
2015-02-17 19:41   ` Arnd Bergmann
2015-02-17 19:41     ` Arnd Bergmann
2015-02-17 20:00     ` Arun Ramamurthy
2015-02-17 20:00       ` Arun Ramamurthy
2015-02-17 20:00       ` Arun Ramamurthy
2015-02-17 20:53       ` Arnd Bergmann
2015-02-17 20:53         ` Arnd Bergmann
2015-02-17 21:05         ` Arun Ramamurthy
2015-02-17 21:05           ` Arun Ramamurthy
2015-02-17 21:05           ` Arun Ramamurthy
2015-02-18 15:15           ` Arnd Bergmann
2015-02-18 15:15             ` Arnd Bergmann
2015-02-19  0:46             ` Arun Ramamurthy
2015-02-19  0:46               ` Arun Ramamurthy
2015-02-19  0:46               ` Arun Ramamurthy
2015-02-26  0:24               ` Arun Ramamurthy [this message]
2015-02-26  0:24                 ` Arun Ramamurthy
2015-02-26  0:24                 ` Arun Ramamurthy
2015-03-10 20:27                 ` Arnd Bergmann
2015-03-10 20:27                   ` Arnd Bergmann
2015-03-11 21:37                   ` Arun Ramamurthy
2015-03-11 21:37                     ` Arun Ramamurthy
2015-03-11 21:37                     ` Arun Ramamurthy
2015-03-11 21:47                     ` Arnd Bergmann
2015-03-11 21:47                       ` Arnd Bergmann
2015-03-11 21:47                       ` Arnd Bergmann
2015-02-17 19:20 ` [PATCH 2/2] phy: usbphy: Add " Arun Ramamurthy
2015-02-17 19:20   ` Arun Ramamurthy
2015-02-17 19:20   ` Arun Ramamurthy
2015-02-18  6:10   ` Kishon Vijay Abraham I
2015-02-18  6:10     ` Kishon Vijay Abraham I
2015-02-18  6:10     ` Kishon Vijay Abraham I
2015-03-05 22:33   ` Dmitry Torokhov
2015-03-05 22:33     ` Dmitry Torokhov

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=54EE67A6.609@broadcom.com \
    --to=arun.ramamurthy@broadcom.com \
    --cc=anatol@google.com \
    --cc=arnd@arndb.de \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dtor@google.com \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jonathar@broadcom.com \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=rjui@broadcom.com \
    --cc=robh+dt@kernel.org \
    --cc=sbranden@broadcom.com \
    --cc=stern@rowland.harvard.edu \
    /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.