Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Thu, Oct 24, 2013 at 10:51:43AM +0530, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 23 October 2013 08:12 PM, Matt Porter wrote:
> > On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
> >> On 10/22/2013 06:25 AM, Matt Porter wrote:
> >>> On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
> Hi Kishon,
>
> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
> > I think it makes sense to keep the data width property in the dwc2 node
> > itself.
> > I mean it describes how the dwc2 IP is configured in that particular
> > SoC (given
> > that it can be either <8> or <16>).
> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8
> and 16 supported" would make sense as a property of dwc2 (though this
> value should be autodetectable through GHWCFG4), while the actual 8 or
> 16 supported by the PHY would make sense as property of a phy.
> >>>
> >>> There would be no value in adding a property for an already detectable
> >>> value to dwc2's binding. To be honest, it's pretty much useless
> >>> information due to the existence of the "8 and 16" option.
> >>>
> Note sure if this is really useful in practice as well, or if just
> setting the actual width to use on dwc2 makes more sense...
> >>>
> >>> The GHWCFG4 information itself is not useful in practice, as described
> >>> in the original thread: https://lkml.org/lkml/2013/10/10/477
> >>>
> >>> It's certainly useful in practice to have this width property in either
> >>> the dwc2 or the phy binding. One can make a case for either. As I
> >>> mentioned in the original post, if we put it in the phy binding we'll be
> >>> updating the generic phy binding. We'll then need an api added into the
> >>> generic phy framework to fetch the width of a phy.
> >>>
> >>> Both cases are doable and trivial, we just need the canonical decision
> >>> from a DT maintainer as to where the property belongs. Given that they
> >>> are in ARM ksummit, I'm not expecting to hear anything right this
> >>> moment. :)
> >>
> >> The host can support both, so it is not a property of the host and is a
> >> property of the phy. It is no different than what mode a SPI slave
> >> requires or whether an i2c slave supports 8 or 10-bit addressing. Those
> >> examples are all 1 to many rather than 1 to 1 where it doesn't really
> >> matter, but the same logic applies.
> >
> > Makes good sense, thanks.
> >
> > In this case, given the PHY ownership of width, we can completely avoid
> > any DT properties. The generic phy compliant BCM Kona phy driver can
> > report via the generic phy framework that it is 8-bit wide. There's no
> > support for this type of thing now but it's pretty trivial to add.
> >
> > I went ahead and did a quick proof-of-concept that adds a free-form
> > phy attributes struct for the generic phy. Given that generic phys can
> > be for any transmission technology this could be filled with a jumble
> > unrelated and often unpopulated attributes over time. In any case, the
> > below patch allows the phy provider to choose to specify utmi_width and
> > a controller driver that cares can use phy_get_attrs() to fetch the
> > optional phy attributes and use the utmi_width field if applicable.
> >
> > Kishon: I'll start a separate thread to discuss what approach you'd like
> > to see in the generic phy framework to manage this.
> >
> > -Matt
> >
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index 6d72269..b763d7b 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -38,6 +38,14 @@ struct phy_ops {
> > };
> >
> > /**
> > + * struct phy_attrs - represents phy attributes
> > + * @utmi_width: Data path width implemented by UTMI PHY
> > + */
> > +struct phy_attrs {
> > + int utmi_width;
> > +};
> > +
> > +/**
> > * struct phy - represents the phy device
> > * @dev: phy device
> > * @id: id of the phy device
> > @@ -51,6 +59,7 @@ struct phy {
> > struct device dev;
> > int id;
> > const struct phy_ops*ops;
> > + struct phy_attrs*attrs;
> > struct phy_init_data*init_data;
> > struct mutexmutex;
> > int init_count;
> > @@ -127,6 +136,9 @@ int phy_init(struct phy *phy);
> > int phy_exit(struct phy *phy);
> > int phy_power_on(struct phy *phy);
> > int phy_power_off(struct phy *phy);
> > +static inline struct phy_attrs *phy_get_attrs(struct phy *phy) {
> > + return phy->attrs;
> > +};
>
> I'd prefer to have phy_set_bus_width and phy_get_bus_width instead.
Ok, will incorporate this. Thanks.
-Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Wed, Oct 23, 2013 at 09:29:22PM +, Paul Zimmerman wrote: > > From: Matt Porter [mailto:matt.por...@linaro.org] > > Sent: Wednesday, October 23, 2013 7:43 AM > > > > On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote: > > > On 10/22/2013 06:25 AM, Matt Porter wrote: > > > > On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: > > > >> Hi Kishon, > > > >> > > > >> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: > > > >>> I think it makes sense to keep the data width property in the dwc2 > > > >>> node itself. > > > >>> I mean it describes how the dwc2 IP is configured in that particular > > > >>> SoC (given > > > >>> that it can be either <8> or <16>). > > > >> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the > > > >> IP > > > >> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 > > > >> and 16 supported" would make sense as a property of dwc2 (though this > > > >> value should be autodetectable through GHWCFG4), while the actual 8 or > > > >> 16 supported by the PHY would make sense as property of a phy. > > > > > > > > There would be no value in adding a property for an already detectable > > > > value to dwc2's binding. To be honest, it's pretty much useless > > > > information due to the existence of the "8 and 16" option. > > > > > > > >> Note sure if this is really useful in practice as well, or if just > > > >> setting the actual width to use on dwc2 makes more sense... > > > > > > > > The GHWCFG4 information itself is not useful in practice, as described > > > > in the original thread: https://lkml.org/lkml/2013/10/10/477 > > > > > > > > It's certainly useful in practice to have this width property in either > > > > the dwc2 or the phy binding. One can make a case for either. As I > > > > mentioned in the original post, if we put it in the phy binding we'll be > > > > updating the generic phy binding. We'll then need an api added into the > > > > generic phy framework to fetch the width of a phy. > > > > > > > > Both cases are doable and trivial, we just need the canonical decision > > > > from a DT maintainer as to where the property belongs. Given that they > > > > are in ARM ksummit, I'm not expecting to hear anything right this > > > > moment. :) > > > > > > The host can support both, so it is not a property of the host and is a > > > property of the phy. It is no different than what mode a SPI slave > > > requires or whether an i2c slave supports 8 or 10-bit addressing. Those > > > examples are all 1 to many rather than 1 to 1 where it doesn't really > > > matter, but the same logic applies. > > > > Makes good sense, thanks. > > > > In this case, given the PHY ownership of width, we can completely avoid > > any DT properties. The generic phy compliant BCM Kona phy driver can > > report via the generic phy framework that it is 8-bit wide. There's no > > support for this type of thing now but it's pretty trivial to add. > > > I went ahead and did a quick proof-of-concept that adds a free-form > > phy attributes struct for the generic phy. Given that generic phys can > > be for any transmission technology this could be filled with a jumble > > unrelated and often unpopulated attributes over time. In any case, the > > below patch allows the phy provider to choose to specify utmi_width and > > a controller driver that cares can use phy_get_attrs() to fetch the > > optional phy attributes and use the utmi_width field if applicable. > > As an alternate approach, you could add a 'utmi_width' property to the > PHY's DT node, and have the dwc2 driver scan the tree until it finds its > PHY, and then check it for that property. That would avoid the need to > add anything new to the PHY framework. I don't know if that would be > considered good practice by the DT guys, though. It's a possibility, however that only solves the issues for DT driven systems. By addressing this in the generic phy layer itself, we solve the problem for another system that doesn't boot using DT. Perhaps a DWC2 core in PCI with the same HWCFG4 8/16 setting and the same phy situation as I have. It's also one less property to get wrong as we attempt to stabilize bindings. -Matt -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Wed, Oct 23, 2013 at 09:29:22PM +, Paul Zimmerman wrote: From: Matt Porter [mailto:matt.por...@linaro.org] Sent: Wednesday, October 23, 2013 7:43 AM On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote: On 10/22/2013 06:25 AM, Matt Porter wrote: On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: Hi Kishon, On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: I think it makes sense to keep the data width property in the dwc2 node itself. I mean it describes how the dwc2 IP is configured in that particular SoC (given that it can be either 8 or 16). If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8 and 16 supported would make sense as a property of dwc2 (though this value should be autodetectable through GHWCFG4), while the actual 8 or 16 supported by the PHY would make sense as property of a phy. There would be no value in adding a property for an already detectable value to dwc2's binding. To be honest, it's pretty much useless information due to the existence of the 8 and 16 option. Note sure if this is really useful in practice as well, or if just setting the actual width to use on dwc2 makes more sense... The GHWCFG4 information itself is not useful in practice, as described in the original thread: https://lkml.org/lkml/2013/10/10/477 It's certainly useful in practice to have this width property in either the dwc2 or the phy binding. One can make a case for either. As I mentioned in the original post, if we put it in the phy binding we'll be updating the generic phy binding. We'll then need an api added into the generic phy framework to fetch the width of a phy. Both cases are doable and trivial, we just need the canonical decision from a DT maintainer as to where the property belongs. Given that they are in ARM ksummit, I'm not expecting to hear anything right this moment. :) The host can support both, so it is not a property of the host and is a property of the phy. It is no different than what mode a SPI slave requires or whether an i2c slave supports 8 or 10-bit addressing. Those examples are all 1 to many rather than 1 to 1 where it doesn't really matter, but the same logic applies. Makes good sense, thanks. In this case, given the PHY ownership of width, we can completely avoid any DT properties. The generic phy compliant BCM Kona phy driver can report via the generic phy framework that it is 8-bit wide. There's no support for this type of thing now but it's pretty trivial to add. I went ahead and did a quick proof-of-concept that adds a free-form phy attributes struct for the generic phy. Given that generic phys can be for any transmission technology this could be filled with a jumble unrelated and often unpopulated attributes over time. In any case, the below patch allows the phy provider to choose to specify utmi_width and a controller driver that cares can use phy_get_attrs() to fetch the optional phy attributes and use the utmi_width field if applicable. As an alternate approach, you could add a 'utmi_width' property to the PHY's DT node, and have the dwc2 driver scan the tree until it finds its PHY, and then check it for that property. That would avoid the need to add anything new to the PHY framework. I don't know if that would be considered good practice by the DT guys, though. It's a possibility, however that only solves the issues for DT driven systems. By addressing this in the generic phy layer itself, we solve the problem for another system that doesn't boot using DT. Perhaps a DWC2 core in PCI with the same HWCFG4 8/16 setting and the same phy situation as I have. It's also one less property to get wrong as we attempt to stabilize bindings. -Matt -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Thu, Oct 24, 2013 at 10:51:43AM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Wednesday 23 October 2013 08:12 PM, Matt Porter wrote:
On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
On 10/22/2013 06:25 AM, Matt Porter wrote:
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
Hi Kishon,
On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
I think it makes sense to keep the data width property in the dwc2 node
itself.
I mean it describes how the dwc2 IP is configured in that particular
SoC (given
that it can be either 8 or 16).
If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8
and 16 supported would make sense as a property of dwc2 (though this
value should be autodetectable through GHWCFG4), while the actual 8 or
16 supported by the PHY would make sense as property of a phy.
There would be no value in adding a property for an already detectable
value to dwc2's binding. To be honest, it's pretty much useless
information due to the existence of the 8 and 16 option.
Note sure if this is really useful in practice as well, or if just
setting the actual width to use on dwc2 makes more sense...
The GHWCFG4 information itself is not useful in practice, as described
in the original thread: https://lkml.org/lkml/2013/10/10/477
It's certainly useful in practice to have this width property in either
the dwc2 or the phy binding. One can make a case for either. As I
mentioned in the original post, if we put it in the phy binding we'll be
updating the generic phy binding. We'll then need an api added into the
generic phy framework to fetch the width of a phy.
Both cases are doable and trivial, we just need the canonical decision
from a DT maintainer as to where the property belongs. Given that they
are in ARM ksummit, I'm not expecting to hear anything right this
moment. :)
The host can support both, so it is not a property of the host and is a
property of the phy. It is no different than what mode a SPI slave
requires or whether an i2c slave supports 8 or 10-bit addressing. Those
examples are all 1 to many rather than 1 to 1 where it doesn't really
matter, but the same logic applies.
Makes good sense, thanks.
In this case, given the PHY ownership of width, we can completely avoid
any DT properties. The generic phy compliant BCM Kona phy driver can
report via the generic phy framework that it is 8-bit wide. There's no
support for this type of thing now but it's pretty trivial to add.
I went ahead and did a quick proof-of-concept that adds a free-form
phy attributes struct for the generic phy. Given that generic phys can
be for any transmission technology this could be filled with a jumble
unrelated and often unpopulated attributes over time. In any case, the
below patch allows the phy provider to choose to specify utmi_width and
a controller driver that cares can use phy_get_attrs() to fetch the
optional phy attributes and use the utmi_width field if applicable.
Kishon: I'll start a separate thread to discuss what approach you'd like
to see in the generic phy framework to manage this.
-Matt
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..b763d7b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -38,6 +38,14 @@ struct phy_ops {
};
/**
+ * struct phy_attrs - represents phy attributes
+ * @utmi_width: Data path width implemented by UTMI PHY
+ */
+struct phy_attrs {
+ int utmi_width;
+};
+
+/**
* struct phy - represents the phy device
* @dev: phy device
* @id: id of the phy device
@@ -51,6 +59,7 @@ struct phy {
struct device dev;
int id;
const struct phy_ops*ops;
+ struct phy_attrs*attrs;
struct phy_init_data*init_data;
struct mutexmutex;
int init_count;
@@ -127,6 +136,9 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
+static inline struct phy_attrs *phy_get_attrs(struct phy *phy) {
+ return phy-attrs;
+};
I'd prefer to have phy_set_bus_width and phy_get_bus_width instead.
Ok, will incorporate this. Thanks.
-Matt
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
Hi,
On Wednesday 23 October 2013 08:12 PM, Matt Porter wrote:
> On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
>> On 10/22/2013 06:25 AM, Matt Porter wrote:
>>> On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
Hi Kishon,
On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
> I think it makes sense to keep the data width property in the dwc2 node
> itself.
> I mean it describes how the dwc2 IP is configured in that particular SoC
> (given
> that it can be either <8> or <16>).
If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8
and 16 supported" would make sense as a property of dwc2 (though this
value should be autodetectable through GHWCFG4), while the actual 8 or
16 supported by the PHY would make sense as property of a phy.
>>>
>>> There would be no value in adding a property for an already detectable
>>> value to dwc2's binding. To be honest, it's pretty much useless
>>> information due to the existence of the "8 and 16" option.
>>>
Note sure if this is really useful in practice as well, or if just
setting the actual width to use on dwc2 makes more sense...
>>>
>>> The GHWCFG4 information itself is not useful in practice, as described
>>> in the original thread: https://lkml.org/lkml/2013/10/10/477
>>>
>>> It's certainly useful in practice to have this width property in either
>>> the dwc2 or the phy binding. One can make a case for either. As I
>>> mentioned in the original post, if we put it in the phy binding we'll be
>>> updating the generic phy binding. We'll then need an api added into the
>>> generic phy framework to fetch the width of a phy.
>>>
>>> Both cases are doable and trivial, we just need the canonical decision
>>> from a DT maintainer as to where the property belongs. Given that they
>>> are in ARM ksummit, I'm not expecting to hear anything right this
>>> moment. :)
>>
>> The host can support both, so it is not a property of the host and is a
>> property of the phy. It is no different than what mode a SPI slave
>> requires or whether an i2c slave supports 8 or 10-bit addressing. Those
>> examples are all 1 to many rather than 1 to 1 where it doesn't really
>> matter, but the same logic applies.
>
> Makes good sense, thanks.
>
> In this case, given the PHY ownership of width, we can completely avoid
> any DT properties. The generic phy compliant BCM Kona phy driver can
> report via the generic phy framework that it is 8-bit wide. There's no
> support for this type of thing now but it's pretty trivial to add.
>
> I went ahead and did a quick proof-of-concept that adds a free-form
> phy attributes struct for the generic phy. Given that generic phys can
> be for any transmission technology this could be filled with a jumble
> unrelated and often unpopulated attributes over time. In any case, the
> below patch allows the phy provider to choose to specify utmi_width and
> a controller driver that cares can use phy_get_attrs() to fetch the
> optional phy attributes and use the utmi_width field if applicable.
>
> Kishon: I'll start a separate thread to discuss what approach you'd like
> to see in the generic phy framework to manage this.
>
> -Matt
>
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 6d72269..b763d7b 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -38,6 +38,14 @@ struct phy_ops {
> };
>
> /**
> + * struct phy_attrs - represents phy attributes
> + * @utmi_width: Data path width implemented by UTMI PHY
> + */
> +struct phy_attrs {
> + int utmi_width;
> +};
> +
> +/**
> * struct phy - represents the phy device
> * @dev: phy device
> * @id: id of the phy device
> @@ -51,6 +59,7 @@ struct phy {
> struct device dev;
> int id;
> const struct phy_ops*ops;
> + struct phy_attrs*attrs;
> struct phy_init_data*init_data;
> struct mutexmutex;
> int init_count;
> @@ -127,6 +136,9 @@ int phy_init(struct phy *phy);
> int phy_exit(struct phy *phy);
> int phy_power_on(struct phy *phy);
> int phy_power_off(struct phy *phy);
> +static inline struct phy_attrs *phy_get_attrs(struct phy *phy) {
> + return phy->attrs;
> +};
I'd prefer to have phy_set_bus_width and phy_get_bus_width instead.
Thanks
Kishon
> struct phy *phy_get(struct device *dev, const char *string);
> struct phy *devm_phy_get(struct device *dev, const char *string);
> void phy_put(struct phy *phy);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
> From: Matt Porter [mailto:matt.por...@linaro.org] > Sent: Wednesday, October 23, 2013 7:43 AM > > On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote: > > On 10/22/2013 06:25 AM, Matt Porter wrote: > > > On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: > > >> Hi Kishon, > > >> > > >> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: > > >>> I think it makes sense to keep the data width property in the dwc2 node > > >>> itself. > > >>> I mean it describes how the dwc2 IP is configured in that particular > > >>> SoC (given > > >>> that it can be either <8> or <16>). > > >> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP > > >> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 > > >> and 16 supported" would make sense as a property of dwc2 (though this > > >> value should be autodetectable through GHWCFG4), while the actual 8 or > > >> 16 supported by the PHY would make sense as property of a phy. > > > > > > There would be no value in adding a property for an already detectable > > > value to dwc2's binding. To be honest, it's pretty much useless > > > information due to the existence of the "8 and 16" option. > > > > > >> Note sure if this is really useful in practice as well, or if just > > >> setting the actual width to use on dwc2 makes more sense... > > > > > > The GHWCFG4 information itself is not useful in practice, as described > > > in the original thread: https://lkml.org/lkml/2013/10/10/477 > > > > > > It's certainly useful in practice to have this width property in either > > > the dwc2 or the phy binding. One can make a case for either. As I > > > mentioned in the original post, if we put it in the phy binding we'll be > > > updating the generic phy binding. We'll then need an api added into the > > > generic phy framework to fetch the width of a phy. > > > > > > Both cases are doable and trivial, we just need the canonical decision > > > from a DT maintainer as to where the property belongs. Given that they > > > are in ARM ksummit, I'm not expecting to hear anything right this > > > moment. :) > > > > The host can support both, so it is not a property of the host and is a > > property of the phy. It is no different than what mode a SPI slave > > requires or whether an i2c slave supports 8 or 10-bit addressing. Those > > examples are all 1 to many rather than 1 to 1 where it doesn't really > > matter, but the same logic applies. > > Makes good sense, thanks. > > In this case, given the PHY ownership of width, we can completely avoid > any DT properties. The generic phy compliant BCM Kona phy driver can > report via the generic phy framework that it is 8-bit wide. There's no > support for this type of thing now but it's pretty trivial to add. > I went ahead and did a quick proof-of-concept that adds a free-form > phy attributes struct for the generic phy. Given that generic phys can > be for any transmission technology this could be filled with a jumble > unrelated and often unpopulated attributes over time. In any case, the > below patch allows the phy provider to choose to specify utmi_width and > a controller driver that cares can use phy_get_attrs() to fetch the > optional phy attributes and use the utmi_width field if applicable. As an alternate approach, you could add a 'utmi_width' property to the PHY's DT node, and have the dwc2 driver scan the tree until it finds its PHY, and then check it for that property. That would avoid the need to add anything new to the PHY framework. I don't know if that would be considered good practice by the DT guys, though. -- Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Wed, Oct 23, 2013 at 01:11:15PM -0500, Felipe Balbi wrote:
> Hi,
>
> On Wed, Oct 23, 2013 at 10:42:42AM -0400, Matt Porter wrote:
> > On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
> > > On 10/22/2013 06:25 AM, Matt Porter wrote:
> > > > On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
> > > >> Hi Kishon,
> > > >>
> > > >> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
> > > >>> I think it makes sense to keep the data width property in the dwc2
> > > >>> node itself.
> > > >>> I mean it describes how the dwc2 IP is configured in that particular
> > > >>> SoC (given
> > > >>> that it can be either <8> or <16>).
> > > >> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the
> > > >> IP
> > > >> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8
> > > >> and 16 supported" would make sense as a property of dwc2 (though this
> > > >> value should be autodetectable through GHWCFG4), while the actual 8 or
> > > >> 16 supported by the PHY would make sense as property of a phy.
> > > >
> > > > There would be no value in adding a property for an already detectable
> > > > value to dwc2's binding. To be honest, it's pretty much useless
> > > > information due to the existence of the "8 and 16" option.
> > > >
> > > >> Note sure if this is really useful in practice as well, or if just
> > > >> setting the actual width to use on dwc2 makes more sense...
> > > >
> > > > The GHWCFG4 information itself is not useful in practice, as described
> > > > in the original thread: https://lkml.org/lkml/2013/10/10/477
> > > >
> > > > It's certainly useful in practice to have this width property in either
> > > > the dwc2 or the phy binding. One can make a case for either. As I
> > > > mentioned in the original post, if we put it in the phy binding we'll be
> > > > updating the generic phy binding. We'll then need an api added into the
> > > > generic phy framework to fetch the width of a phy.
> > > >
> > > > Both cases are doable and trivial, we just need the canonical decision
> > > > from a DT maintainer as to where the property belongs. Given that they
> > > > are in ARM ksummit, I'm not expecting to hear anything right this
> > > > moment. :)
> > >
> > > The host can support both, so it is not a property of the host and is a
> > > property of the phy. It is no different than what mode a SPI slave
> > > requires or whether an i2c slave supports 8 or 10-bit addressing. Those
> > > examples are all 1 to many rather than 1 to 1 where it doesn't really
> > > matter, but the same logic applies.
> >
> > Makes good sense, thanks.
> >
> > In this case, given the PHY ownership of width, we can completely avoid
> > any DT properties. The generic phy compliant BCM Kona phy driver can
> > report via the generic phy framework that it is 8-bit wide. There's no
> > support for this type of thing now but it's pretty trivial to add.
> >
> > I went ahead and did a quick proof-of-concept that adds a free-form
> > phy attributes struct for the generic phy. Given that generic phys can
> > be for any transmission technology this could be filled with a jumble
> > unrelated and often unpopulated attributes over time. In any case, the
> > below patch allows the phy provider to choose to specify utmi_width and
> > a controller driver that cares can use phy_get_attrs() to fetch the
> > optional phy attributes and use the utmi_width field if applicable.
> >
> > Kishon: I'll start a separate thread to discuss what approach you'd like
> > to see in the generic phy framework to manage this.
> >
> > -Matt
> >
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index 6d72269..b763d7b 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -38,6 +38,14 @@ struct phy_ops {
> > };
> >
> > /**
> > + * struct phy_attrs - represents phy attributes
> > + * @utmi_width: Data path width implemented by UTMI PHY
> > + */
> > +struct phy_attrs {
> > + int utmi_width;
>
> this is supposed to be a generic PHY layer and as such, it shouldn't
> know about USB details such as the UTMI bus. How about calling bus_width
> just to make it more generic ? Then it would work for UTMI, PIPE3, ULPI,
> SLPI (did that even fly ?) or any other PHY <-> link interconnect.
That sounds much better. Yeah, I was also thinking about embedding a
per-phy-class attribute struct and that just looked ugly. I like the simple
generic bus_width.
I'll update and post a real patch.
Thanks,
Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
Hi,
On Wed, Oct 23, 2013 at 10:42:42AM -0400, Matt Porter wrote:
> On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
> > On 10/22/2013 06:25 AM, Matt Porter wrote:
> > > On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
> > >> Hi Kishon,
> > >>
> > >> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
> > >>> I think it makes sense to keep the data width property in the dwc2 node
> > >>> itself.
> > >>> I mean it describes how the dwc2 IP is configured in that particular
> > >>> SoC (given
> > >>> that it can be either <8> or <16>).
> > >> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
> > >> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8
> > >> and 16 supported" would make sense as a property of dwc2 (though this
> > >> value should be autodetectable through GHWCFG4), while the actual 8 or
> > >> 16 supported by the PHY would make sense as property of a phy.
> > >
> > > There would be no value in adding a property for an already detectable
> > > value to dwc2's binding. To be honest, it's pretty much useless
> > > information due to the existence of the "8 and 16" option.
> > >
> > >> Note sure if this is really useful in practice as well, or if just
> > >> setting the actual width to use on dwc2 makes more sense...
> > >
> > > The GHWCFG4 information itself is not useful in practice, as described
> > > in the original thread: https://lkml.org/lkml/2013/10/10/477
> > >
> > > It's certainly useful in practice to have this width property in either
> > > the dwc2 or the phy binding. One can make a case for either. As I
> > > mentioned in the original post, if we put it in the phy binding we'll be
> > > updating the generic phy binding. We'll then need an api added into the
> > > generic phy framework to fetch the width of a phy.
> > >
> > > Both cases are doable and trivial, we just need the canonical decision
> > > from a DT maintainer as to where the property belongs. Given that they
> > > are in ARM ksummit, I'm not expecting to hear anything right this
> > > moment. :)
> >
> > The host can support both, so it is not a property of the host and is a
> > property of the phy. It is no different than what mode a SPI slave
> > requires or whether an i2c slave supports 8 or 10-bit addressing. Those
> > examples are all 1 to many rather than 1 to 1 where it doesn't really
> > matter, but the same logic applies.
>
> Makes good sense, thanks.
>
> In this case, given the PHY ownership of width, we can completely avoid
> any DT properties. The generic phy compliant BCM Kona phy driver can
> report via the generic phy framework that it is 8-bit wide. There's no
> support for this type of thing now but it's pretty trivial to add.
>
> I went ahead and did a quick proof-of-concept that adds a free-form
> phy attributes struct for the generic phy. Given that generic phys can
> be for any transmission technology this could be filled with a jumble
> unrelated and often unpopulated attributes over time. In any case, the
> below patch allows the phy provider to choose to specify utmi_width and
> a controller driver that cares can use phy_get_attrs() to fetch the
> optional phy attributes and use the utmi_width field if applicable.
>
> Kishon: I'll start a separate thread to discuss what approach you'd like
> to see in the generic phy framework to manage this.
>
> -Matt
>
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 6d72269..b763d7b 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -38,6 +38,14 @@ struct phy_ops {
> };
>
> /**
> + * struct phy_attrs - represents phy attributes
> + * @utmi_width: Data path width implemented by UTMI PHY
> + */
> +struct phy_attrs {
> + int utmi_width;
this is supposed to be a generic PHY layer and as such, it shouldn't
know about USB details such as the UTMI bus. How about calling bus_width
just to make it more generic ? Then it would work for UTMI, PIPE3, ULPI,
SLPI (did that even fly ?) or any other PHY <-> link interconnect.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
> On 10/22/2013 06:25 AM, Matt Porter wrote:
> > On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
> >> Hi Kishon,
> >>
> >> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
> >>> I think it makes sense to keep the data width property in the dwc2 node
> >>> itself.
> >>> I mean it describes how the dwc2 IP is configured in that particular SoC
> >>> (given
> >>> that it can be either <8> or <16>).
> >> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
> >> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8
> >> and 16 supported" would make sense as a property of dwc2 (though this
> >> value should be autodetectable through GHWCFG4), while the actual 8 or
> >> 16 supported by the PHY would make sense as property of a phy.
> >
> > There would be no value in adding a property for an already detectable
> > value to dwc2's binding. To be honest, it's pretty much useless
> > information due to the existence of the "8 and 16" option.
> >
> >> Note sure if this is really useful in practice as well, or if just
> >> setting the actual width to use on dwc2 makes more sense...
> >
> > The GHWCFG4 information itself is not useful in practice, as described
> > in the original thread: https://lkml.org/lkml/2013/10/10/477
> >
> > It's certainly useful in practice to have this width property in either
> > the dwc2 or the phy binding. One can make a case for either. As I
> > mentioned in the original post, if we put it in the phy binding we'll be
> > updating the generic phy binding. We'll then need an api added into the
> > generic phy framework to fetch the width of a phy.
> >
> > Both cases are doable and trivial, we just need the canonical decision
> > from a DT maintainer as to where the property belongs. Given that they
> > are in ARM ksummit, I'm not expecting to hear anything right this
> > moment. :)
>
> The host can support both, so it is not a property of the host and is a
> property of the phy. It is no different than what mode a SPI slave
> requires or whether an i2c slave supports 8 or 10-bit addressing. Those
> examples are all 1 to many rather than 1 to 1 where it doesn't really
> matter, but the same logic applies.
Makes good sense, thanks.
In this case, given the PHY ownership of width, we can completely avoid
any DT properties. The generic phy compliant BCM Kona phy driver can
report via the generic phy framework that it is 8-bit wide. There's no
support for this type of thing now but it's pretty trivial to add.
I went ahead and did a quick proof-of-concept that adds a free-form
phy attributes struct for the generic phy. Given that generic phys can
be for any transmission technology this could be filled with a jumble
unrelated and often unpopulated attributes over time. In any case, the
below patch allows the phy provider to choose to specify utmi_width and
a controller driver that cares can use phy_get_attrs() to fetch the
optional phy attributes and use the utmi_width field if applicable.
Kishon: I'll start a separate thread to discuss what approach you'd like
to see in the generic phy framework to manage this.
-Matt
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..b763d7b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -38,6 +38,14 @@ struct phy_ops {
};
/**
+ * struct phy_attrs - represents phy attributes
+ * @utmi_width: Data path width implemented by UTMI PHY
+ */
+struct phy_attrs {
+ int utmi_width;
+};
+
+/**
* struct phy - represents the phy device
* @dev: phy device
* @id: id of the phy device
@@ -51,6 +59,7 @@ struct phy {
struct device dev;
int id;
const struct phy_ops*ops;
+ struct phy_attrs*attrs;
struct phy_init_data*init_data;
struct mutexmutex;
int init_count;
@@ -127,6 +136,9 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
+static inline struct phy_attrs *phy_get_attrs(struct phy *phy) {
+ return phy->attrs;
+};
struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
void phy_put(struct phy *phy);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
On 10/22/2013 06:25 AM, Matt Porter wrote:
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
Hi Kishon,
On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
I think it makes sense to keep the data width property in the dwc2 node
itself.
I mean it describes how the dwc2 IP is configured in that particular SoC
(given
that it can be either 8 or 16).
If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8
and 16 supported would make sense as a property of dwc2 (though this
value should be autodetectable through GHWCFG4), while the actual 8 or
16 supported by the PHY would make sense as property of a phy.
There would be no value in adding a property for an already detectable
value to dwc2's binding. To be honest, it's pretty much useless
information due to the existence of the 8 and 16 option.
Note sure if this is really useful in practice as well, or if just
setting the actual width to use on dwc2 makes more sense...
The GHWCFG4 information itself is not useful in practice, as described
in the original thread: https://lkml.org/lkml/2013/10/10/477
It's certainly useful in practice to have this width property in either
the dwc2 or the phy binding. One can make a case for either. As I
mentioned in the original post, if we put it in the phy binding we'll be
updating the generic phy binding. We'll then need an api added into the
generic phy framework to fetch the width of a phy.
Both cases are doable and trivial, we just need the canonical decision
from a DT maintainer as to where the property belongs. Given that they
are in ARM ksummit, I'm not expecting to hear anything right this
moment. :)
The host can support both, so it is not a property of the host and is a
property of the phy. It is no different than what mode a SPI slave
requires or whether an i2c slave supports 8 or 10-bit addressing. Those
examples are all 1 to many rather than 1 to 1 where it doesn't really
matter, but the same logic applies.
Makes good sense, thanks.
In this case, given the PHY ownership of width, we can completely avoid
any DT properties. The generic phy compliant BCM Kona phy driver can
report via the generic phy framework that it is 8-bit wide. There's no
support for this type of thing now but it's pretty trivial to add.
I went ahead and did a quick proof-of-concept that adds a free-form
phy attributes struct for the generic phy. Given that generic phys can
be for any transmission technology this could be filled with a jumble
unrelated and often unpopulated attributes over time. In any case, the
below patch allows the phy provider to choose to specify utmi_width and
a controller driver that cares can use phy_get_attrs() to fetch the
optional phy attributes and use the utmi_width field if applicable.
Kishon: I'll start a separate thread to discuss what approach you'd like
to see in the generic phy framework to manage this.
-Matt
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..b763d7b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -38,6 +38,14 @@ struct phy_ops {
};
/**
+ * struct phy_attrs - represents phy attributes
+ * @utmi_width: Data path width implemented by UTMI PHY
+ */
+struct phy_attrs {
+ int utmi_width;
+};
+
+/**
* struct phy - represents the phy device
* @dev: phy device
* @id: id of the phy device
@@ -51,6 +59,7 @@ struct phy {
struct device dev;
int id;
const struct phy_ops*ops;
+ struct phy_attrs*attrs;
struct phy_init_data*init_data;
struct mutexmutex;
int init_count;
@@ -127,6 +136,9 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
+static inline struct phy_attrs *phy_get_attrs(struct phy *phy) {
+ return phy-attrs;
+};
struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
void phy_put(struct phy *phy);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
Hi,
On Wed, Oct 23, 2013 at 10:42:42AM -0400, Matt Porter wrote:
On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
On 10/22/2013 06:25 AM, Matt Porter wrote:
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
Hi Kishon,
On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
I think it makes sense to keep the data width property in the dwc2 node
itself.
I mean it describes how the dwc2 IP is configured in that particular
SoC (given
that it can be either 8 or 16).
If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8
and 16 supported would make sense as a property of dwc2 (though this
value should be autodetectable through GHWCFG4), while the actual 8 or
16 supported by the PHY would make sense as property of a phy.
There would be no value in adding a property for an already detectable
value to dwc2's binding. To be honest, it's pretty much useless
information due to the existence of the 8 and 16 option.
Note sure if this is really useful in practice as well, or if just
setting the actual width to use on dwc2 makes more sense...
The GHWCFG4 information itself is not useful in practice, as described
in the original thread: https://lkml.org/lkml/2013/10/10/477
It's certainly useful in practice to have this width property in either
the dwc2 or the phy binding. One can make a case for either. As I
mentioned in the original post, if we put it in the phy binding we'll be
updating the generic phy binding. We'll then need an api added into the
generic phy framework to fetch the width of a phy.
Both cases are doable and trivial, we just need the canonical decision
from a DT maintainer as to where the property belongs. Given that they
are in ARM ksummit, I'm not expecting to hear anything right this
moment. :)
The host can support both, so it is not a property of the host and is a
property of the phy. It is no different than what mode a SPI slave
requires or whether an i2c slave supports 8 or 10-bit addressing. Those
examples are all 1 to many rather than 1 to 1 where it doesn't really
matter, but the same logic applies.
Makes good sense, thanks.
In this case, given the PHY ownership of width, we can completely avoid
any DT properties. The generic phy compliant BCM Kona phy driver can
report via the generic phy framework that it is 8-bit wide. There's no
support for this type of thing now but it's pretty trivial to add.
I went ahead and did a quick proof-of-concept that adds a free-form
phy attributes struct for the generic phy. Given that generic phys can
be for any transmission technology this could be filled with a jumble
unrelated and often unpopulated attributes over time. In any case, the
below patch allows the phy provider to choose to specify utmi_width and
a controller driver that cares can use phy_get_attrs() to fetch the
optional phy attributes and use the utmi_width field if applicable.
Kishon: I'll start a separate thread to discuss what approach you'd like
to see in the generic phy framework to manage this.
-Matt
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..b763d7b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -38,6 +38,14 @@ struct phy_ops {
};
/**
+ * struct phy_attrs - represents phy attributes
+ * @utmi_width: Data path width implemented by UTMI PHY
+ */
+struct phy_attrs {
+ int utmi_width;
this is supposed to be a generic PHY layer and as such, it shouldn't
know about USB details such as the UTMI bus. How about calling bus_width
just to make it more generic ? Then it would work for UTMI, PIPE3, ULPI,
SLPI (did that even fly ?) or any other PHY - link interconnect.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Wed, Oct 23, 2013 at 01:11:15PM -0500, Felipe Balbi wrote:
Hi,
On Wed, Oct 23, 2013 at 10:42:42AM -0400, Matt Porter wrote:
On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
On 10/22/2013 06:25 AM, Matt Porter wrote:
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
Hi Kishon,
On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
I think it makes sense to keep the data width property in the dwc2
node itself.
I mean it describes how the dwc2 IP is configured in that particular
SoC (given
that it can be either 8 or 16).
If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the
IP
can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8
and 16 supported would make sense as a property of dwc2 (though this
value should be autodetectable through GHWCFG4), while the actual 8 or
16 supported by the PHY would make sense as property of a phy.
There would be no value in adding a property for an already detectable
value to dwc2's binding. To be honest, it's pretty much useless
information due to the existence of the 8 and 16 option.
Note sure if this is really useful in practice as well, or if just
setting the actual width to use on dwc2 makes more sense...
The GHWCFG4 information itself is not useful in practice, as described
in the original thread: https://lkml.org/lkml/2013/10/10/477
It's certainly useful in practice to have this width property in either
the dwc2 or the phy binding. One can make a case for either. As I
mentioned in the original post, if we put it in the phy binding we'll be
updating the generic phy binding. We'll then need an api added into the
generic phy framework to fetch the width of a phy.
Both cases are doable and trivial, we just need the canonical decision
from a DT maintainer as to where the property belongs. Given that they
are in ARM ksummit, I'm not expecting to hear anything right this
moment. :)
The host can support both, so it is not a property of the host and is a
property of the phy. It is no different than what mode a SPI slave
requires or whether an i2c slave supports 8 or 10-bit addressing. Those
examples are all 1 to many rather than 1 to 1 where it doesn't really
matter, but the same logic applies.
Makes good sense, thanks.
In this case, given the PHY ownership of width, we can completely avoid
any DT properties. The generic phy compliant BCM Kona phy driver can
report via the generic phy framework that it is 8-bit wide. There's no
support for this type of thing now but it's pretty trivial to add.
I went ahead and did a quick proof-of-concept that adds a free-form
phy attributes struct for the generic phy. Given that generic phys can
be for any transmission technology this could be filled with a jumble
unrelated and often unpopulated attributes over time. In any case, the
below patch allows the phy provider to choose to specify utmi_width and
a controller driver that cares can use phy_get_attrs() to fetch the
optional phy attributes and use the utmi_width field if applicable.
Kishon: I'll start a separate thread to discuss what approach you'd like
to see in the generic phy framework to manage this.
-Matt
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..b763d7b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -38,6 +38,14 @@ struct phy_ops {
};
/**
+ * struct phy_attrs - represents phy attributes
+ * @utmi_width: Data path width implemented by UTMI PHY
+ */
+struct phy_attrs {
+ int utmi_width;
this is supposed to be a generic PHY layer and as such, it shouldn't
know about USB details such as the UTMI bus. How about calling bus_width
just to make it more generic ? Then it would work for UTMI, PIPE3, ULPI,
SLPI (did that even fly ?) or any other PHY - link interconnect.
That sounds much better. Yeah, I was also thinking about embedding a
per-phy-class attribute struct and that just looked ugly. I like the simple
generic bus_width.
I'll update and post a real patch.
Thanks,
Matt
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
From: Matt Porter [mailto:matt.por...@linaro.org] Sent: Wednesday, October 23, 2013 7:43 AM On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote: On 10/22/2013 06:25 AM, Matt Porter wrote: On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: Hi Kishon, On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: I think it makes sense to keep the data width property in the dwc2 node itself. I mean it describes how the dwc2 IP is configured in that particular SoC (given that it can be either 8 or 16). If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8 and 16 supported would make sense as a property of dwc2 (though this value should be autodetectable through GHWCFG4), while the actual 8 or 16 supported by the PHY would make sense as property of a phy. There would be no value in adding a property for an already detectable value to dwc2's binding. To be honest, it's pretty much useless information due to the existence of the 8 and 16 option. Note sure if this is really useful in practice as well, or if just setting the actual width to use on dwc2 makes more sense... The GHWCFG4 information itself is not useful in practice, as described in the original thread: https://lkml.org/lkml/2013/10/10/477 It's certainly useful in practice to have this width property in either the dwc2 or the phy binding. One can make a case for either. As I mentioned in the original post, if we put it in the phy binding we'll be updating the generic phy binding. We'll then need an api added into the generic phy framework to fetch the width of a phy. Both cases are doable and trivial, we just need the canonical decision from a DT maintainer as to where the property belongs. Given that they are in ARM ksummit, I'm not expecting to hear anything right this moment. :) The host can support both, so it is not a property of the host and is a property of the phy. It is no different than what mode a SPI slave requires or whether an i2c slave supports 8 or 10-bit addressing. Those examples are all 1 to many rather than 1 to 1 where it doesn't really matter, but the same logic applies. Makes good sense, thanks. In this case, given the PHY ownership of width, we can completely avoid any DT properties. The generic phy compliant BCM Kona phy driver can report via the generic phy framework that it is 8-bit wide. There's no support for this type of thing now but it's pretty trivial to add. I went ahead and did a quick proof-of-concept that adds a free-form phy attributes struct for the generic phy. Given that generic phys can be for any transmission technology this could be filled with a jumble unrelated and often unpopulated attributes over time. In any case, the below patch allows the phy provider to choose to specify utmi_width and a controller driver that cares can use phy_get_attrs() to fetch the optional phy attributes and use the utmi_width field if applicable. As an alternate approach, you could add a 'utmi_width' property to the PHY's DT node, and have the dwc2 driver scan the tree until it finds its PHY, and then check it for that property. That would avoid the need to add anything new to the PHY framework. I don't know if that would be considered good practice by the DT guys, though. -- Paul -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
Hi,
On Wednesday 23 October 2013 08:12 PM, Matt Porter wrote:
On Tue, Oct 22, 2013 at 04:38:52PM -0500, Rob Herring wrote:
On 10/22/2013 06:25 AM, Matt Porter wrote:
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote:
Hi Kishon,
On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote:
I think it makes sense to keep the data width property in the dwc2 node
itself.
I mean it describes how the dwc2 IP is configured in that particular SoC
(given
that it can be either 8 or 16).
If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP
can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8
and 16 supported would make sense as a property of dwc2 (though this
value should be autodetectable through GHWCFG4), while the actual 8 or
16 supported by the PHY would make sense as property of a phy.
There would be no value in adding a property for an already detectable
value to dwc2's binding. To be honest, it's pretty much useless
information due to the existence of the 8 and 16 option.
Note sure if this is really useful in practice as well, or if just
setting the actual width to use on dwc2 makes more sense...
The GHWCFG4 information itself is not useful in practice, as described
in the original thread: https://lkml.org/lkml/2013/10/10/477
It's certainly useful in practice to have this width property in either
the dwc2 or the phy binding. One can make a case for either. As I
mentioned in the original post, if we put it in the phy binding we'll be
updating the generic phy binding. We'll then need an api added into the
generic phy framework to fetch the width of a phy.
Both cases are doable and trivial, we just need the canonical decision
from a DT maintainer as to where the property belongs. Given that they
are in ARM ksummit, I'm not expecting to hear anything right this
moment. :)
The host can support both, so it is not a property of the host and is a
property of the phy. It is no different than what mode a SPI slave
requires or whether an i2c slave supports 8 or 10-bit addressing. Those
examples are all 1 to many rather than 1 to 1 where it doesn't really
matter, but the same logic applies.
Makes good sense, thanks.
In this case, given the PHY ownership of width, we can completely avoid
any DT properties. The generic phy compliant BCM Kona phy driver can
report via the generic phy framework that it is 8-bit wide. There's no
support for this type of thing now but it's pretty trivial to add.
I went ahead and did a quick proof-of-concept that adds a free-form
phy attributes struct for the generic phy. Given that generic phys can
be for any transmission technology this could be filled with a jumble
unrelated and often unpopulated attributes over time. In any case, the
below patch allows the phy provider to choose to specify utmi_width and
a controller driver that cares can use phy_get_attrs() to fetch the
optional phy attributes and use the utmi_width field if applicable.
Kishon: I'll start a separate thread to discuss what approach you'd like
to see in the generic phy framework to manage this.
-Matt
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..b763d7b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -38,6 +38,14 @@ struct phy_ops {
};
/**
+ * struct phy_attrs - represents phy attributes
+ * @utmi_width: Data path width implemented by UTMI PHY
+ */
+struct phy_attrs {
+ int utmi_width;
+};
+
+/**
* struct phy - represents the phy device
* @dev: phy device
* @id: id of the phy device
@@ -51,6 +59,7 @@ struct phy {
struct device dev;
int id;
const struct phy_ops*ops;
+ struct phy_attrs*attrs;
struct phy_init_data*init_data;
struct mutexmutex;
int init_count;
@@ -127,6 +136,9 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
+static inline struct phy_attrs *phy_get_attrs(struct phy *phy) {
+ return phy-attrs;
+};
I'd prefer to have phy_set_bus_width and phy_get_bus_width instead.
Thanks
Kishon
struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
void phy_put(struct phy *phy);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On 10/22/2013 06:25 AM, Matt Porter wrote: > On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: >> Hi Kishon, >> >> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: >>> I think it makes sense to keep the data width property in the dwc2 node >>> itself. >>> I mean it describes how the dwc2 IP is configured in that particular SoC >>> (given >>> that it can be either <8> or <16>). >> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP >> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 >> and 16 supported" would make sense as a property of dwc2 (though this >> value should be autodetectable through GHWCFG4), while the actual 8 or >> 16 supported by the PHY would make sense as property of a phy. > > There would be no value in adding a property for an already detectable > value to dwc2's binding. To be honest, it's pretty much useless > information due to the existence of the "8 and 16" option. > >> Note sure if this is really useful in practice as well, or if just >> setting the actual width to use on dwc2 makes more sense... > > The GHWCFG4 information itself is not useful in practice, as described > in the original thread: https://lkml.org/lkml/2013/10/10/477 > > It's certainly useful in practice to have this width property in either > the dwc2 or the phy binding. One can make a case for either. As I > mentioned in the original post, if we put it in the phy binding we'll be > updating the generic phy binding. We'll then need an api added into the > generic phy framework to fetch the width of a phy. > > Both cases are doable and trivial, we just need the canonical decision > from a DT maintainer as to where the property belongs. Given that they > are in ARM ksummit, I'm not expecting to hear anything right this > moment. :) The host can support both, so it is not a property of the host and is a property of the phy. It is no different than what mode a SPI slave requires or whether an i2c slave supports 8 or 10-bit addressing. Those examples are all 1 to many rather than 1 to 1 where it doesn't really matter, but the same logic applies. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: > Hi Kishon, > > On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: > > I think it makes sense to keep the data width property in the dwc2 node > > itself. > > I mean it describes how the dwc2 IP is configured in that particular SoC > > (given > > that it can be either <8> or <16>). > If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP > can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 > and 16 supported" would make sense as a property of dwc2 (though this > value should be autodetectable through GHWCFG4), while the actual 8 or > 16 supported by the PHY would make sense as property of a phy. There would be no value in adding a property for an already detectable value to dwc2's binding. To be honest, it's pretty much useless information due to the existence of the "8 and 16" option. > Note sure if this is really useful in practice as well, or if just > setting the actual width to use on dwc2 makes more sense... The GHWCFG4 information itself is not useful in practice, as described in the original thread: https://lkml.org/lkml/2013/10/10/477 It's certainly useful in practice to have this width property in either the dwc2 or the phy binding. One can make a case for either. As I mentioned in the original post, if we put it in the phy binding we'll be updating the generic phy binding. We'll then need an api added into the generic phy framework to fetch the width of a phy. Both cases are doable and trivial, we just need the canonical decision from a DT maintainer as to where the property belongs. Given that they are in ARM ksummit, I'm not expecting to hear anything right this moment. :) -Matt -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
Hi Kishon, On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: > I think it makes sense to keep the data width property in the dwc2 node > itself. > I mean it describes how the dwc2 IP is configured in that particular SoC > (given > that it can be either <8> or <16>). If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 and 16 supported" would make sense as a property of dwc2 (though this value should be autodetectable through GHWCFG4), while the actual 8 or 16 supported by the PHY would make sense as property of a phy. Note sure if this is really useful in practice as well, or if just setting the actual width to use on dwc2 makes more sense... Gr. Matthijs signature.asc Description: Digital signature
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
Hi Kishon, On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: I think it makes sense to keep the data width property in the dwc2 node itself. I mean it describes how the dwc2 IP is configured in that particular SoC (given that it can be either 8 or 16). If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8 and 16 supported would make sense as a property of dwc2 (though this value should be autodetectable through GHWCFG4), while the actual 8 or 16 supported by the PHY would make sense as property of a phy. Note sure if this is really useful in practice as well, or if just setting the actual width to use on dwc2 makes more sense... Gr. Matthijs signature.asc Description: Digital signature
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: Hi Kishon, On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: I think it makes sense to keep the data width property in the dwc2 node itself. I mean it describes how the dwc2 IP is configured in that particular SoC (given that it can be either 8 or 16). If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8 and 16 supported would make sense as a property of dwc2 (though this value should be autodetectable through GHWCFG4), while the actual 8 or 16 supported by the PHY would make sense as property of a phy. There would be no value in adding a property for an already detectable value to dwc2's binding. To be honest, it's pretty much useless information due to the existence of the 8 and 16 option. Note sure if this is really useful in practice as well, or if just setting the actual width to use on dwc2 makes more sense... The GHWCFG4 information itself is not useful in practice, as described in the original thread: https://lkml.org/lkml/2013/10/10/477 It's certainly useful in practice to have this width property in either the dwc2 or the phy binding. One can make a case for either. As I mentioned in the original post, if we put it in the phy binding we'll be updating the generic phy binding. We'll then need an api added into the generic phy framework to fetch the width of a phy. Both cases are doable and trivial, we just need the canonical decision from a DT maintainer as to where the property belongs. Given that they are in ARM ksummit, I'm not expecting to hear anything right this moment. :) -Matt -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On 10/22/2013 06:25 AM, Matt Porter wrote: On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: Hi Kishon, On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: I think it makes sense to keep the data width property in the dwc2 node itself. I mean it describes how the dwc2 IP is configured in that particular SoC (given that it can be either 8 or 16). If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP can be configured for 8, 16 or 8 _and_ 16. In the latter case, the 8 and 16 supported would make sense as a property of dwc2 (though this value should be autodetectable through GHWCFG4), while the actual 8 or 16 supported by the PHY would make sense as property of a phy. There would be no value in adding a property for an already detectable value to dwc2's binding. To be honest, it's pretty much useless information due to the existence of the 8 and 16 option. Note sure if this is really useful in practice as well, or if just setting the actual width to use on dwc2 makes more sense... The GHWCFG4 information itself is not useful in practice, as described in the original thread: https://lkml.org/lkml/2013/10/10/477 It's certainly useful in practice to have this width property in either the dwc2 or the phy binding. One can make a case for either. As I mentioned in the original post, if we put it in the phy binding we'll be updating the generic phy binding. We'll then need an api added into the generic phy framework to fetch the width of a phy. Both cases are doable and trivial, we just need the canonical decision from a DT maintainer as to where the property belongs. Given that they are in ARM ksummit, I'm not expecting to hear anything right this moment. :) The host can support both, so it is not a property of the host and is a property of the phy. It is no different than what mode a SPI slave requires or whether an i2c slave supports 8 or 10-bit addressing. Those examples are all 1 to many rather than 1 to 1 where it doesn't really matter, but the same logic applies. Rob -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Friday 18 October 2013 07:42 PM, Matt Porter wrote:
> This is a summary of an unresolved issue resulting from this thread:
> http://www.spinics.net/lists/arm-kernel/msg277700.html
>
> The BCM281xx family of SoCs contain an OTG subsystem consisting of a
> DWC2 HSOTG controller and an internal UTMI PHY. This is appears as
> follows (monospace font requirement ahead):
>
> ++ +-+
> || | |
> || 8 | |
> | DWC2 |<--/->| BCM Kona|
> || UTMI | UTMI PHY|
> || | |
> ++ +-+
>
> The internal UTMI phy is connected via an 8-bit data path. There is
> no way to autodetect whether the data path is 8-bit or 16-bit. As such,
> it was determined that a DT property is necessary to reflect this.
>
> In the original patch submitted this property was offered as an
> additional optional dwc2 property:
>
> --- a/Documentation/devicetree/bindings/staging/dwc2.txt
> +++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> @@ -6,10 +6,14 @@ Required properties:
> - reg : Should contain 1 register range (address and length)
> - interrupts : Should contain 1 interrupt
>
> +Optional properties:
> +- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
> +
> Example:
>
> usb@101c {
> compatible = "ralink,rt3050-usb, snps,dwc2";
> reg = <0x101c 4>;
> interrupts = <18>;
> + snps,phy-utmi-width = <8>;
> };
>
> The open question is whether this required hardware property belongs to
> the DWC2 controller or the PHY itself.
I think it makes sense to keep the data width property in the dwc2 node itself.
I mean it describes how the dwc2 IP is configured in that particular SoC (given
that it can be either <8> or <16>).
Thanks
Kishon
>
> If the UTMI data path width is considered to be a property of the PHY
> then this will impact both the generic PHY framework and the PHY device
> node (producer) binding. The binding would need to be extended to carry
> the data path width property. In addition, the generic PHY framework
> would need to allow for this information to be gathered in some manner
> for use by the controller driver (PHY consumer). In the case of DWC2,
> the driver needs to know whether to program the phy interface for 8 or
> 16 bit UTMI communication.
>
> Thanks,
> Matt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Does PHY UTMI data width belong to DWC2 or PHY binding?
On Friday 18 October 2013 07:42 PM, Matt Porter wrote:
This is a summary of an unresolved issue resulting from this thread:
http://www.spinics.net/lists/arm-kernel/msg277700.html
The BCM281xx family of SoCs contain an OTG subsystem consisting of a
DWC2 HSOTG controller and an internal UTMI PHY. This is appears as
follows (monospace font requirement ahead):
++ +-+
|| | |
|| 8 | |
| DWC2 |--/-| BCM Kona|
|| UTMI | UTMI PHY|
|| | |
++ +-+
The internal UTMI phy is connected via an 8-bit data path. There is
no way to autodetect whether the data path is 8-bit or 16-bit. As such,
it was determined that a DT property is necessary to reflect this.
In the original patch submitted this property was offered as an
additional optional dwc2 property:
--- a/Documentation/devicetree/bindings/staging/dwc2.txt
+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
@@ -6,10 +6,14 @@ Required properties:
- reg : Should contain 1 register range (address and length)
- interrupts : Should contain 1 interrupt
+Optional properties:
+- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
+
Example:
usb@101c {
compatible = ralink,rt3050-usb, snps,dwc2;
reg = 0x101c 4;
interrupts = 18;
+ snps,phy-utmi-width = 8;
};
The open question is whether this required hardware property belongs to
the DWC2 controller or the PHY itself.
I think it makes sense to keep the data width property in the dwc2 node itself.
I mean it describes how the dwc2 IP is configured in that particular SoC (given
that it can be either 8 or 16).
Thanks
Kishon
If the UTMI data path width is considered to be a property of the PHY
then this will impact both the generic PHY framework and the PHY device
node (producer) binding. The binding would need to be extended to carry
the data path width property. In addition, the generic PHY framework
would need to allow for this information to be gathered in some manner
for use by the controller driver (PHY consumer). In the case of DWC2,
the driver needs to know whether to program the phy interface for 8 or
16 bit UTMI communication.
Thanks,
Matt
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
