All of lore.kernel.org
 help / color / mirror / Atom feed
* Fixed PHY Device Tree usage?
@ 2013-07-09 16:33 Thomas Petazzoni
  2013-07-09 16:44 ` Florian Fainelli
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2013-07-09 16:33 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem

Hello,

We have a case of an hardware platform that uses the mvneta network
driver, but instead of the SoC being connected to a PHY, it's connected
directly to a switch, so my understanding is that there's no MDIO bus,
and we should have a kind of a "fake PHY" to make the mvneta driver
believe that the link is up, at a given speed.

Looking at this problem, I stumbled across the "fixed PHY" driver in
drivers/net/phy/fixed.c, which registers a fake "Fixed MDIO bus", and
then provides a fixed_phy_add() API to add one "fake" PHY. This seems
to fit my need, except that my ARM platform is obviously Device Tree
based, so I'm wondering what I should do. One option is to implement a
Device Tree binding for the fixed PHY driver (the exact DT binding
would have to be discussed), but I'm wondering whether describing a
fixed PHY in the DT is actually correct, because describing a fixed PHy
is not really describing the hardware, the hardware is actually a
switch.

Do you have some thoughts about this situation? Maybe there's already
some solutions that I'm not aware of?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-09 16:33 Fixed PHY Device Tree usage? Thomas Petazzoni
@ 2013-07-09 16:44 ` Florian Fainelli
  2013-07-09 18:02   ` Florian Fainelli
  0 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2013-07-09 16:44 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem

Hello Thomas,

2013/7/9 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
> Hello,
>
> We have a case of an hardware platform that uses the mvneta network
> driver, but instead of the SoC being connected to a PHY, it's connected
> directly to a switch, so my understanding is that there's no MDIO bus,
> and we should have a kind of a "fake PHY" to make the mvneta driver
> believe that the link is up, at a given speed.

Good timing, I was about to post questions/suggestions about how we
should represent fixed PHYs in device tree.

>
> Looking at this problem, I stumbled across the "fixed PHY" driver in
> drivers/net/phy/fixed.c, which registers a fake "Fixed MDIO bus", and
> then provides a fixed_phy_add() API to add one "fake" PHY. This seems
> to fit my need, except that my ARM platform is obviously Device Tree
> based, so I'm wondering what I should do. One option is to implement a
> Device Tree binding for the fixed PHY driver (the exact DT binding
> would have to be discussed), but I'm wondering whether describing a
> fixed PHY in the DT is actually correct, because describing a fixed PHy
> is not really describing the hardware, the hardware is actually a
> switch.

Well, it does not seem to be too far from the "hardware" reality to
describe a link between a switch CPU port and an Ethernet MAC as a
fixed PHY because that is what it really is in fact. Once you have a
drivers for your switch you can start using this PHY along with its
corresponding driver.

There is a helper: of_phy_connect_fixed_link() in drivers/of/of_mdio.c
is flagged as being a
temporary solution for Freescale Ethernet drivers to move to something else,
so I would like to discuss what the "something else should be".

Here what I would like to see the new "fixed-link" phy node look like:

ethernet-phy@0 {
     reg = <0>;
     id = "deadbeef";
     speed = <1000>;
     full-duplex;
     pause;
     asym-pause;
};

It has the same properties as the binding described in:
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt but expressed in a more
explicit way instead of using an array of integers.

>
> Do you have some thoughts about this situation? Maybe there's already
> some solutions that I'm not aware of?
>
> Thanks,
--
Florian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-09 16:44 ` Florian Fainelli
@ 2013-07-09 18:02   ` Florian Fainelli
  2013-07-10 16:22     ` Thomas Petazzoni
  0 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2013-07-09 18:02 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Widening audience

FlorianLe mardi 9 juillet 2013 17:44:55  Fainelli a écrit :
> Hello Thomas,
> 
> 2013/7/9 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
> > Hello,
> > 
> > We have a case of an hardware platform that uses the mvneta network
> > driver, but instead of the SoC being connected to a PHY, it's connected
> > directly to a switch, so my understanding is that there's no MDIO bus,
> > and we should have a kind of a "fake PHY" to make the mvneta driver
> > believe that the link is up, at a given speed.
> 
> Good timing, I was about to post questions/suggestions about how we
> should represent fixed PHYs in device tree.
> 
> > Looking at this problem, I stumbled across the "fixed PHY" driver in
> > drivers/net/phy/fixed.c, which registers a fake "Fixed MDIO bus", and
> > then provides a fixed_phy_add() API to add one "fake" PHY. This seems
> > to fit my need, except that my ARM platform is obviously Device Tree
> > based, so I'm wondering what I should do. One option is to implement a
> > Device Tree binding for the fixed PHY driver (the exact DT binding
> > would have to be discussed), but I'm wondering whether describing a
> > fixed PHY in the DT is actually correct, because describing a fixed PHy
> > is not really describing the hardware, the hardware is actually a
> > switch.
> 
> Well, it does not seem to be too far from the "hardware" reality to
> describe a link between a switch CPU port and an Ethernet MAC as a
> fixed PHY because that is what it really is in fact. Once you have a
> drivers for your switch you can start using this PHY along with its
> corresponding driver.
> 
> There is a helper: of_phy_connect_fixed_link() in drivers/of/of_mdio.c
> is flagged as being a
> temporary solution for Freescale Ethernet drivers to move to something else,
> so I would like to discuss what the "something else should be".
> 
> Here what I would like to see the new "fixed-link" phy node look like:
> 
> ethernet-phy@0 {
>      reg = <0>;
>      id = "deadbeef";
>      speed = <1000>;
>      full-duplex;
>      pause;
>      asym-pause;
> };
> 
> It has the same properties as the binding described in:
> Documentation/devicetree/bindings/net/fsl-tsec-phy.txt but expressed in a
> more explicit way instead of using an array of integers.
> 
> > Do you have some thoughts about this situation? Maybe there's already
> > some solutions that I'm not aware of?
> > 
> > Thanks,
> 
> --
> Florian
-- 
Florian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-09 18:02   ` Florian Fainelli
@ 2013-07-10 16:22     ` Thomas Petazzoni
  2013-07-10 16:29       ` Florian Fainelli
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2013-07-10 16:22 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Dear Florian Fainelli,

On Tue, 09 Jul 2013 19:02:05 +0100, Florian Fainelli wrote:

> > > We have a case of an hardware platform that uses the mvneta network
> > > driver, but instead of the SoC being connected to a PHY, it's connected
> > > directly to a switch, so my understanding is that there's no MDIO bus,
> > > and we should have a kind of a "fake PHY" to make the mvneta driver
> > > believe that the link is up, at a given speed.
> > 
> > Good timing, I was about to post questions/suggestions about how we
> > should represent fixed PHYs in device tree.

Great.

> > Well, it does not seem to be too far from the "hardware" reality to
> > describe a link between a switch CPU port and an Ethernet MAC as a
> > fixed PHY because that is what it really is in fact. Once you have a
> > drivers for your switch you can start using this PHY along with its
> > corresponding driver.

Ok.

> > There is a helper: of_phy_connect_fixed_link() in drivers/of/of_mdio.c
> > is flagged as being a
> > temporary solution for Freescale Ethernet drivers to move to something else,
> > so I would like to discuss what the "something else should be".

Yeah, I saw this helper function as well, and the comment you spotted.

> > Here what I would like to see the new "fixed-link" phy node look like:
> > 
> > ethernet-phy@0 {
> >      reg = <0>;
> >      id = "deadbeef";
> >      speed = <1000>;
> >      full-duplex;
> >      pause;
> >      asym-pause;
> > };
> > 
> > It has the same properties as the binding described in:
> > Documentation/devicetree/bindings/net/fsl-tsec-phy.txt but expressed in a
> > more explicit way instead of using an array of integers.

And so the fixed-phy driver would look for what exactly in the Device
Tree to find which fixed PHYs to create?

Should we have something like:

	mdio-fixed {
		compatible = "generic,mdio-fixed";
		phy0: ethernet-phy@0 {
			... all the properties you listed ...
			... maybe the "id" property is not needed
			    because of the phandle ...
		};

		phy1: ethernet-phy@1 {
			... all the properties you listed ...
			... maybe the "id" property is not needed
			    because of the phandle ...
		};
	};

	soc {
		ethernet@0 {
			phy = <&phy0>;
			...
		};

		ethernet@1 {
			phy = <&phy1>;
			...
		};
	};

or do you have in mind another representation?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-10 16:22     ` Thomas Petazzoni
@ 2013-07-10 16:29       ` Florian Fainelli
  2013-07-10 16:39         ` Thomas Petazzoni
  0 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2013-07-10 16:29 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Hello Thomas,

2013/7/10 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:

[snip]

>> >
>> > It has the same properties as the binding described in:
>> > Documentation/devicetree/bindings/net/fsl-tsec-phy.txt but expressed in a
>> > more explicit way instead of using an array of integers.
>
> And so the fixed-phy driver would look for what exactly in the Device
> Tree to find which fixed PHYs to create?
>
> Should we have something like:
>
>         mdio-fixed {
>                 compatible = "generic,mdio-fixed";
>                 phy0: ethernet-phy@0 {
>                         ... all the properties you listed ...
>                         ... maybe the "id" property is not needed
>                             because of the phandle ...

In the "fixed-phy" terminology "id" is unfortunately ambiguous, the
driver internally uses "phy_id" which is nothing more than a PHY
address, but it also supports being assigned an "id" as in
Identification register 2 & 3. I was refering to the identification
register by "id".

>                 };
>
>                 phy1: ethernet-phy@1 {
>                         ... all the properties you listed ...
>                         ... maybe the "id" property is not needed
>                             because of the phandle ...
>                 };
>         };
>
>         soc {
>                 ethernet@0 {
>                         phy = <&phy0>;
>                         ...
>                 };
>
>                 ethernet@1 {
>                         phy = <&phy1>;
>                         ...
>                 };
>         };
>
> or do you have in mind another representation?

Not really this is more or less what I had in mind. I am wondering
whether we should really declare the "mdio-fixed" node, or if we
should not rather make the following:

- declare all PHY nodes in the system as sub nodes of their belonging
real hardware MDIO bus node
- flag specific PHY nodes as "fixed" with a "fixed-link" boolean for instance
- if we see that flag, make that specific PHY node bind to the
fixed-phy driver instead

What do you think? I suspect someone might rightfully say that the
"fixed-mdio" is not a real piece of hardware and is just a software
concept. A PHY in the real world may very well have a fixed link
speed/duplex/pause settings on the other end.
--
Florian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-10 16:29       ` Florian Fainelli
@ 2013-07-10 16:39         ` Thomas Petazzoni
  2013-07-10 17:23           ` Florian Fainelli
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2013-07-10 16:39 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Dear Florian Fainelli,

On Wed, 10 Jul 2013 17:29:44 +0100, Florian Fainelli wrote:

> > Should we have something like:
> >
> >         mdio-fixed {
> >                 compatible = "generic,mdio-fixed";
> >                 phy0: ethernet-phy@0 {
> >                         ... all the properties you listed ...
> >                         ... maybe the "id" property is not needed
> >                             because of the phandle ...
> 
> In the "fixed-phy" terminology "id" is unfortunately ambiguous, the
> driver internally uses "phy_id" which is nothing more than a PHY
> address, but it also supports being assigned an "id" as in
> Identification register 2 & 3. I was refering to the identification
> register by "id".

Hum, but your "id" property contained a string, so I'm not sure how it
would fit in Identification register 2 and 3. Am I missing something
obvious here? Maybe you wanted to have:

	id = <0xdeadbeef>;

which would make the "emulated" PHY return 0xdeadbeef as its PHY ID
when reading those identification registers.

> >                 };
> >
> >                 phy1: ethernet-phy@1 {
> >                         ... all the properties you listed ...
> >                         ... maybe the "id" property is not needed
> >                             because of the phandle ...
> >                 };
> >         };
> >
> >         soc {
> >                 ethernet@0 {
> >                         phy = <&phy0>;
> >                         ...
> >                 };
> >
> >                 ethernet@1 {
> >                         phy = <&phy1>;
> >                         ...
> >                 };
> >         };
> >
> > or do you have in mind another representation?
> 
> Not really this is more or less what I had in mind. I am wondering
> whether we should really declare the "mdio-fixed" node, or if we
> should not rather make the following:
> 
> - declare all PHY nodes in the system as sub nodes of their belonging
> real hardware MDIO bus node
> - flag specific PHY nodes as "fixed" with a "fixed-link" boolean for instance
> - if we see that flag, make that specific PHY node bind to the
> fixed-phy driver instead

So the fixed PHY driver is going to travel through *all* nodes of the
DT, and whenever some random node has a "fixed" property, it's going to
say it corresponds to a fixed PHY? That doesn't seem like a good idea.

So that's really what I was asking: how is the fixed PHY driver going
to know which DT nodes to look at. Is it a platform_driver, where the
corresponding DT node has sub-nodes? Is it something else? Or a
specific compatible string?

> What do you think? I suspect someone might rightfully say that the
> "fixed-mdio" is not a real piece of hardware and is just a software
> concept. A PHY in the real world may very well have a fixed link
> speed/duplex/pause settings on the other end.

I agree that the mdio-fixed idea is clearly moving away from the
hardware representation. But see my question above: we need a way of
letting the fixed PHY driver know which DT nodes it should have a look
at. And just saying "those nodes will have property 'foo' is not
sufficient".

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-10 16:39         ` Thomas Petazzoni
@ 2013-07-10 17:23           ` Florian Fainelli
  2013-07-12 11:56             ` Thomas Petazzoni
  2013-07-12 22:44             ` Grant Likely
  0 siblings, 2 replies; 13+ messages in thread
From: Florian Fainelli @ 2013-07-10 17:23 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Hello Thomas,

2013/7/10 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
> Dear Florian Fainelli,
>
> On Wed, 10 Jul 2013 17:29:44 +0100, Florian Fainelli wrote:
[snip]

>
>> >                 };
>> >
>> >                 phy1: ethernet-phy@1 {
>> >                         ... all the properties you listed ...
>> >                         ... maybe the "id" property is not needed
>> >                             because of the phandle ...
>> >                 };
>> >         };
>> >
>> >         soc {
>> >                 ethernet@0 {
>> >                         phy = <&phy0>;
>> >                         ...
>> >                 };
>> >
>> >                 ethernet@1 {
>> >                         phy = <&phy1>;
>> >                         ...
>> >                 };
>> >         };
>> >
>> > or do you have in mind another representation?
>>
>> Not really this is more or less what I had in mind. I am wondering
>> whether we should really declare the "mdio-fixed" node, or if we
>> should not rather make the following:
>>
>> - declare all PHY nodes in the system as sub nodes of their belonging
>> real hardware MDIO bus node
>> - flag specific PHY nodes as "fixed" with a "fixed-link" boolean for instance
>> - if we see that flag, make that specific PHY node bind to the
>> fixed-phy driver instead
>
> So the fixed PHY driver is going to travel through *all* nodes of the
> DT, and whenever some random node has a "fixed" property, it's going to
> say it corresponds to a fixed PHY? That doesn't seem like a good idea.

Why not? Since we are already have to scan the entire MDIO bus we are
attached to, when we encounter such a PHY node with the special
"fixed" properties, we just call fixed_phy_add() with the right
parameters and voila. Which is also the reason why I was suggesting to
put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
we have this logic only in one place.

>
> So that's really what I was asking: how is the fixed PHY driver going
> to know which DT nodes to look at. Is it a platform_driver, where the
> corresponding DT node has sub-nodes? Is it something else? Or a
> specific compatible string?

Without DT at play here, the usual way to register a fixed PHY is:

1) make your arch code or whatever runs before the fixed MDIO bus
probing to call fixed_phy_add() with the expected parameters
2) when your ethernet driver probes its PHY devices, format the phy
name to be bound to the fixed bus with the expected address by then
the fixed MDIO bus would have already been probed and would find the
fixed PHY nodes because of the first step
3) call of_phy_connect() from your driver to attach to the fixed PHY

>
>> What do you think? I suspect someone might rightfully say that the
>> "fixed-mdio" is not a real piece of hardware and is just a software
>> concept. A PHY in the real world may very well have a fixed link
>> speed/duplex/pause settings on the other end.
>
> I agree that the mdio-fixed idea is clearly moving away from the
> hardware representation. But see my question above: we need a way of
> letting the fixed PHY driver know which DT nodes it should have a look
> at. And just saying "those nodes will have property 'foo' is not
> sufficient".
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com



--
Florian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-10 17:23           ` Florian Fainelli
@ 2013-07-12 11:56             ` Thomas Petazzoni
  2013-07-12 12:05               ` Florian Fainelli
  2013-07-12 22:44             ` Grant Likely
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2013-07-12 11:56 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Dear Florian Fainelli,

On Wed, 10 Jul 2013 18:23:30 +0100, Florian Fainelli wrote:

> >> - declare all PHY nodes in the system as sub nodes of their belonging
> >> real hardware MDIO bus node
> >> - flag specific PHY nodes as "fixed" with a "fixed-link" boolean for instance
> >> - if we see that flag, make that specific PHY node bind to the
> >> fixed-phy driver instead
> >
> > So the fixed PHY driver is going to travel through *all* nodes of the
> > DT, and whenever some random node has a "fixed" property, it's going to
> > say it corresponds to a fixed PHY? That doesn't seem like a good idea.
> 
> Why not? Since we are already have to scan the entire MDIO bus we are
> attached to, when we encounter such a PHY node with the special
> "fixed" properties, we just call fixed_phy_add() with the right
> parameters and voila. Which is also the reason why I was suggesting to
> put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
> we have this logic only in one place.

I'm still not sure to understand you here. Scanning the *entire* DT
tree and consider all nodes having a property named "fixed" as fixed
PHYs is definitely not acceptable. So I suppose you have a different
idea, but I'm still not getting it. Where in the DT would the fixed PHY
driver start looking for fixed PHYs ?

> > So that's really what I was asking: how is the fixed PHY driver going
> > to know which DT nodes to look at. Is it a platform_driver, where the
> > corresponding DT node has sub-nodes? Is it something else? Or a
> > specific compatible string?
> 
> Without DT at play here, the usual way to register a fixed PHY is:
> 
> 1) make your arch code or whatever runs before the fixed MDIO bus
> probing to call fixed_phy_add() with the expected parameters
> 2) when your ethernet driver probes its PHY devices, format the phy
> name to be bound to the fixed bus with the expected address by then
> the fixed MDIO bus would have already been probed and would find the
> fixed PHY nodes because of the first step
> 3) call of_phy_connect() from your driver to attach to the fixed PHY

Right, but that's still doesn't answer the question of how the fixed
PHY driver discovers from the DT which PHYs to instantiate.

For example, we would probably have something:

	phys {
		phy0: phy@0 {
			... PHY properties ...
		};
		phy1: phy@1 {
			... PHY properties ...
		};
	};

	soc {
		ethernet@0 {
			compatible = "...";
			...
			phy = <&phy0>;
		};
		ethernet@1 {
			compatible = "...";
			...
			phy = <&phy1>;
		};
	};

How will the fixed PHY driver know that it should instantiate
phy@0 and phy@1 as PHY devices?
	
Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-12 11:56             ` Thomas Petazzoni
@ 2013-07-12 12:05               ` Florian Fainelli
  2013-07-12 13:04                 ` Thomas Petazzoni
  0 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2013-07-12 12:05 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Hello Thomas,

2013/7/12 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:

>> Why not? Since we are already have to scan the entire MDIO bus we are
>> attached to, when we encounter such a PHY node with the special
>> "fixed" properties, we just call fixed_phy_add() with the right
>> parameters and voila. Which is also the reason why I was suggesting to
>> put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
>> we have this logic only in one place.
>
> I'm still not sure to understand you here. Scanning the *entire* DT
> tree and consider all nodes having a property named "fixed" as fixed
> PHYs is definitely not acceptable. So I suppose you have a different
> idea, but I'm still not getting it. Where in the DT would the fixed PHY
> driver start looking for fixed PHYs ?

I am talking about scanning the MDIO bus DT nodes, not the entire DT.
That job is already done by of_mdiobus_probe() to register PHY
devices, so having a central point where the knowledge of how to treat
PHY deivces could be here I guess.

>
>> > So that's really what I was asking: how is the fixed PHY driver going
>> > to know which DT nodes to look at. Is it a platform_driver, where the
>> > corresponding DT node has sub-nodes? Is it something else? Or a
>> > specific compatible string?
>>
>> Without DT at play here, the usual way to register a fixed PHY is:
>>
>> 1) make your arch code or whatever runs before the fixed MDIO bus
>> probing to call fixed_phy_add() with the expected parameters
>> 2) when your ethernet driver probes its PHY devices, format the phy
>> name to be bound to the fixed bus with the expected address by then
>> the fixed MDIO bus would have already been probed and would find the
>> fixed PHY nodes because of the first step
>> 3) call of_phy_connect() from your driver to attach to the fixed PHY
>
> Right, but that's still doesn't answer the question of how the fixed
> PHY driver discovers from the DT which PHYs to instantiate.
>
> For example, we would probably have something:
>
>         phys {
>                 phy0: phy@0 {
>                         ... PHY properties ...
>                 };
>                 phy1: phy@1 {
>                         ... PHY properties ...
>                 };
>         };
>
>         soc {
>                 ethernet@0 {
>                         compatible = "...";
>                         ...
>                         phy = <&phy0>;
>                 };
>                 ethernet@1 {
>                         compatible = "...";
>                         ...
>                         phy = <&phy1>;
>                 };
>         };
>
> How will the fixed PHY driver know that it should instantiate
> phy@0 and phy@1 as PHY devices?

Well either we go with some specific compatible property like
"ethernet-phy-fixed" for instance, or we simply add a boolean property
to the node, so a fixed PHY would either look like this:

phy {
         compatible = "linux,ethernet-phy-fixed";
         speed = <1000>;
         duplex = <1>;
         pause;
         asym-pause;
};

or respectively, something like this:

phy {
         fixed;
         speed = <1000>;
         duplex = <1>;
         pause;
         asym-pause;
};
--
Florian

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-12 12:05               ` Florian Fainelli
@ 2013-07-12 13:04                 ` Thomas Petazzoni
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2013-07-12 13:04 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sebastian Hesselbarth, Gregory Clément,
	Ezequiel Garcia, Lior Amsalem, devicetree-discuss, grant.likely,
	afleming

Dear Florian Fainelli,

On Fri, 12 Jul 2013 13:05:59 +0100, Florian Fainelli wrote:

> I am talking about scanning the MDIO bus DT nodes, not the entire DT.
> That job is already done by of_mdiobus_probe() to register PHY
> devices, so having a central point where the knowledge of how to treat
> PHY deivces could be here I guess.

So, I guess your idea is to call of_mdiobus_register() from
drivers/net/phy/fixed.c:fixed_mdio_bus_init(). But then, what DT node
will you be passing to of_mdiobus_register() ? As a reminder, this
function takes as a second argument the DT node that contains the
various PHYs as sub-nodes.

In all the other PHY drivers, the MDIO bus node as a compatible string,
so the usual platform_driver/platform_device mechanism kicks in, and
calls the ->probe() function, passing the DT node of the MDIO bus,
which is then used by the PHY driver ->probe() function as the second
argument of of_mdiobus_register().

But the fixed.c PHY driver is not a platform_driver, and in our
discussion, we mentioned that it wouldn't make sense to have a
compatible string for the fixed MDIO bus DT node.

So I'm still unsure *which* DT node you'll pass as the second argument
of of_mdiobus_register() :-)


> Well either we go with some specific compatible property like
> "ethernet-phy-fixed" for instance, or we simply add a boolean property
> to the node, so a fixed PHY would either look like this:
> 
> phy {
>          compatible = "linux,ethernet-phy-fixed";
>          speed = <1000>;
>          duplex = <1>;
>          pause;
>          asym-pause;
> };
> 
> or respectively, something like this:
> 
> phy {
>          fixed;
>          speed = <1000>;
>          duplex = <1>;
>          pause;
>          asym-pause;
> };

Yeah, that's fine, I have no problem with the internal properties of
the PHY nodes themselves. My question is really the one described above.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-10 17:23           ` Florian Fainelli
  2013-07-12 11:56             ` Thomas Petazzoni
@ 2013-07-12 22:44             ` Grant Likely
  2013-07-12 23:29               ` Florian Fainelli
  2013-07-13 17:02               ` Thomas Petazzoni
  1 sibling, 2 replies; 13+ messages in thread
From: Grant Likely @ 2013-07-12 22:44 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Thomas Petazzoni, netdev, Sebastian Hesselbarth,
	Gregory Clément, Ezequiel Garcia, Lior Amsalem,
	devicetree-discuss, Andy Fleming

On Wed, Jul 10, 2013 at 6:23 PM, Florian Fainelli <florian@openwrt.org> wrote:
> Hello Thomas,
>
> 2013/7/10 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
>> Dear Florian Fainelli,
>>
>> On Wed, 10 Jul 2013 17:29:44 +0100, Florian Fainelli wrote:
> [snip]
>
>>
>>> >                 };
>>> >
>>> >                 phy1: ethernet-phy@1 {
>>> >                         ... all the properties you listed ...
>>> >                         ... maybe the "id" property is not needed
>>> >                             because of the phandle ...
>>> >                 };
>>> >         };
>>> >
>>> >         soc {
>>> >                 ethernet@0 {
>>> >                         phy = <&phy0>;
>>> >                         ...
>>> >                 };
>>> >
>>> >                 ethernet@1 {
>>> >                         phy = <&phy1>;
>>> >                         ...
>>> >                 };
>>> >         };
>>> >
>>> > or do you have in mind another representation?
>>>
>>> Not really this is more or less what I had in mind. I am wondering
>>> whether we should really declare the "mdio-fixed" node, or if we
>>> should not rather make the following:
>>>
>>> - declare all PHY nodes in the system as sub nodes of their belonging
>>> real hardware MDIO bus node
>>> - flag specific PHY nodes as "fixed" with a "fixed-link" boolean for instance
>>> - if we see that flag, make that specific PHY node bind to the
>>> fixed-phy driver instead
>>
>> So the fixed PHY driver is going to travel through *all* nodes of the
>> DT, and whenever some random node has a "fixed" property, it's going to
>> say it corresponds to a fixed PHY? That doesn't seem like a good idea.
>
> Why not? Since we are already have to scan the entire MDIO bus we are
> attached to, when we encounter such a PHY node with the special
> "fixed" properties, we just call fixed_phy_add() with the right
> parameters and voila. Which is also the reason why I was suggesting to
> put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
> we have this logic only in one place.

Hi Florian,

I think this discussion is going in the wrong direction. The concept
of a dummy phy is really a Linux kernel internal detail. Creating some
kind of dummy MDIO bus node does not describe the hardware. There is
already support in the kernel for Ethernet MACs connected directly to
a switch or other device. It is far better to describe how the MAC
needs to be configured than to invent a non-existent phy. Search for
"fixed-link" in the kernel tree to see how it is used.

g.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-12 22:44             ` Grant Likely
@ 2013-07-12 23:29               ` Florian Fainelli
  2013-07-13 17:02               ` Thomas Petazzoni
  1 sibling, 0 replies; 13+ messages in thread
From: Florian Fainelli @ 2013-07-12 23:29 UTC (permalink / raw)
  To: Grant Likely
  Cc: Thomas Petazzoni, netdev, Sebastian Hesselbarth,
	Gregory Clément, Ezequiel Garcia, Lior Amsalem,
	devicetree-discuss, Andy Fleming

Le 13/07/2013 00:44, Grant Likely a écrit :
> On Wed, Jul 10, 2013 at 6:23 PM, Florian Fainelli <florian@openwrt.org> wrote:
>> Hello Thomas,
>>
>> 2013/7/10 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
>>> Dear Florian Fainelli,
>>>
>>> On Wed, 10 Jul 2013 17:29:44 +0100, Florian Fainelli wrote:
>> [snip]
>>
>>>
>>>>>                  };
>>>>>
>>>>>                  phy1: ethernet-phy@1 {
>>>>>                          ... all the properties you listed ...
>>>>>                          ... maybe the "id" property is not needed
>>>>>                              because of the phandle ...
>>>>>                  };
>>>>>          };
>>>>>
>>>>>          soc {
>>>>>                  ethernet@0 {
>>>>>                          phy = <&phy0>;
>>>>>                          ...
>>>>>                  };
>>>>>
>>>>>                  ethernet@1 {
>>>>>                          phy = <&phy1>;
>>>>>                          ...
>>>>>                  };
>>>>>          };
>>>>>
>>>>> or do you have in mind another representation?
>>>>
>>>> Not really this is more or less what I had in mind. I am wondering
>>>> whether we should really declare the "mdio-fixed" node, or if we
>>>> should not rather make the following:
>>>>
>>>> - declare all PHY nodes in the system as sub nodes of their belonging
>>>> real hardware MDIO bus node
>>>> - flag specific PHY nodes as "fixed" with a "fixed-link" boolean for instance
>>>> - if we see that flag, make that specific PHY node bind to the
>>>> fixed-phy driver instead
>>>
>>> So the fixed PHY driver is going to travel through *all* nodes of the
>>> DT, and whenever some random node has a "fixed" property, it's going to
>>> say it corresponds to a fixed PHY? That doesn't seem like a good idea.
>>
>> Why not? Since we are already have to scan the entire MDIO bus we are
>> attached to, when we encounter such a PHY node with the special
>> "fixed" properties, we just call fixed_phy_add() with the right
>> parameters and voila. Which is also the reason why I was suggesting to
>> put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
>> we have this logic only in one place.
>
> Hi Florian,
>
> I think this discussion is going in the wrong direction. The concept
> of a dummy phy is really a Linux kernel internal detail. Creating some
> kind of dummy MDIO bus node does not describe the hardware. There is
> already support in the kernel for Ethernet MACs connected directly to
> a switch or other device. It is far better to describe how the MAC
> needs to be configured than to invent a non-existent phy. Search for
> "fixed-link" in the kernel tree to see how it is used.

Errm, fixed-link is deprecated according to the comment which parses it. 
In fact, the code parsing this special property does not parse all the 
integers representing the fixed-link. But fair enough, if that is the 
way to go, then let's stick with it.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixed PHY Device Tree usage?
  2013-07-12 22:44             ` Grant Likely
  2013-07-12 23:29               ` Florian Fainelli
@ 2013-07-13 17:02               ` Thomas Petazzoni
  1 sibling, 0 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2013-07-13 17:02 UTC (permalink / raw)
  To: Grant Likely
  Cc: Florian Fainelli, netdev, Sebastian Hesselbarth,
	Gregory Clément, Ezequiel Garcia, Lior Amsalem,
	devicetree-discuss, Andy Fleming

Dear Grant Likely,

On Fri, 12 Jul 2013 23:44:21 +0100, Grant Likely wrote:

> I think this discussion is going in the wrong direction. The concept
> of a dummy phy is really a Linux kernel internal detail. Creating some
> kind of dummy MDIO bus node does not describe the hardware.

This is exactly what I was suggesting in my original e-mail of this
thread, see http://marc.info/?l=linux-netdev&m=137338762627063&w=2 :

"""
One option is to implement a Device Tree binding for the fixed PHY
driver (the exact DT binding would have to be discussed), but I'm
wondering whether describing a fixed PHY in the DT is actually correct,
because describing a fixed PHy is not really describing the hardware,
the hardware is actually a switch.
"""

> There is
> already support in the kernel for Ethernet MACs connected directly to
> a switch or other device. It is far better to describe how the MAC
> needs to be configured than to invent a non-existent phy. Search for
> "fixed-link" in the kernel tree to see how it is used.

As Florian pointed out, the of_phy_connect_fixed_link() comment
indicates:

 * This function is a temporary stop-gap and will be removed soon.  It is
 * only to support the fs_enet, ucc_geth and gianfar Ethernet drivers. Do
 * not call this function from new drivers.

Also, it would probably be good to have a few more helpers to make
parsing the "phy" and "fixed-link" property easier for network drivers.

Thanks for your feedback,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-07-13 17:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-09 16:33 Fixed PHY Device Tree usage? Thomas Petazzoni
2013-07-09 16:44 ` Florian Fainelli
2013-07-09 18:02   ` Florian Fainelli
2013-07-10 16:22     ` Thomas Petazzoni
2013-07-10 16:29       ` Florian Fainelli
2013-07-10 16:39         ` Thomas Petazzoni
2013-07-10 17:23           ` Florian Fainelli
2013-07-12 11:56             ` Thomas Petazzoni
2013-07-12 12:05               ` Florian Fainelli
2013-07-12 13:04                 ` Thomas Petazzoni
2013-07-12 22:44             ` Grant Likely
2013-07-12 23:29               ` Florian Fainelli
2013-07-13 17:02               ` Thomas Petazzoni

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.