LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v1 0/2] of: property: Support for few more properties
@ 2021-08-14  2:31 Saravana Kannan
  2021-08-14  2:31 ` [PATCH v1 1/2] of: property: fw_devlink: Add support for "leds" and "backlight" Saravana Kannan
  2021-08-14  2:31 ` [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property Saravana Kannan
  0 siblings, 2 replies; 10+ messages in thread
From: Saravana Kannan @ 2021-08-14  2:31 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: Saravana Kannan, kernel-team, devicetree, linux-kernel

This is a series only to avoid merge conflicts. No real dependency
between the patches.

Saravana Kannan (2):
  of: property: fw_devlink: Add support for "leds" and "backlight"
  of: property: fw_devlink: Add support for "phy-handle" property

 drivers/of/property.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

-- 
2.33.0.rc1.237.g0d66db33f3-goog


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

* [PATCH v1 1/2] of: property: fw_devlink: Add support for "leds" and "backlight"
  2021-08-14  2:31 [PATCH v1 0/2] of: property: Support for few more properties Saravana Kannan
@ 2021-08-14  2:31 ` Saravana Kannan
  2021-08-17 22:02   ` Rob Herring
  2021-08-14  2:31 ` [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property Saravana Kannan
  1 sibling, 1 reply; 10+ messages in thread
From: Saravana Kannan @ 2021-08-14  2:31 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: Saravana Kannan, kernel-team, devicetree, linux-kernel

Allows tracking dependencies between leds/backlights devices and their
consumers.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/of/property.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index a81559c5ee2b..931340329414 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1289,6 +1289,8 @@ DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
 DEFINE_SIMPLE_PROP(remote_endpoint, "remote-endpoint", NULL)
 DEFINE_SIMPLE_PROP(pwms, "pwms", "#pwm-cells")
 DEFINE_SIMPLE_PROP(resets, "resets", "#reset-cells")
+DEFINE_SIMPLE_PROP(leds, "leds", NULL)
+DEFINE_SIMPLE_PROP(backlight, "backlight", NULL)
 DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
 DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
 
@@ -1375,6 +1377,8 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_remote_endpoint, .node_not_dev = true, },
 	{ .parse_prop = parse_pwms, },
 	{ .parse_prop = parse_resets, },
+	{ .parse_prop = parse_leds, },
+	{ .parse_prop = parse_backlight, },
 	{ .parse_prop = parse_gpio_compat, },
 	{ .parse_prop = parse_interrupts, },
 	{ .parse_prop = parse_regulators, },
-- 
2.33.0.rc1.237.g0d66db33f3-goog


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

* [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property
  2021-08-14  2:31 [PATCH v1 0/2] of: property: Support for few more properties Saravana Kannan
  2021-08-14  2:31 ` [PATCH v1 1/2] of: property: fw_devlink: Add support for "leds" and "backlight" Saravana Kannan
@ 2021-08-14  2:31 ` Saravana Kannan
  2021-08-14 15:22   ` Andrew Lunn
  1 sibling, 1 reply; 10+ messages in thread
From: Saravana Kannan @ 2021-08-14  2:31 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: Saravana Kannan, kernel-team, devicetree, linux-kernel,
	Andrew Lunn, netdev

Allows tracking dependencies between consumers of an Ethernet PHY and
the parent devices that own the PHY.

Cc: Andrew Lunn <andrew@lunn.ch>
Cc: netdev@vger.kernel.org
Signed-off-by: Saravana Kannan <saravanak@google.com>
---

Hi Andrew,

I spent a few hours looking at most/all uses of phy-handle and my
comment in the code seems valid. Can you confirm that please? Also there
are so many phy related properties that my head is spinning. Is there a
"phy" property (which is different from "phys") that treated exactly as
"phy-handle"?

-Saravana

 drivers/of/property.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 931340329414..70d9843fd4bf 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1350,6 +1350,20 @@ static struct device_node *parse_interrupts(struct device_node *np,
 	return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np;
 }
 
+static struct device_node *parse_phy_handle(struct device_node *np,
+					    const char *prop_name, int index)
+{
+	if (strcmp(prop_name, "phy-handle"))
+		return NULL;
+
+	/*
+	 * Device tree nodes pointed to by phy-handle never have struct devices
+	 * created for them even if they have a "compatible" property. So
+	 * return the parent node pointer.
+	 */
+	return of_get_next_parent(of_parse_phandle(np, prop_name, index));
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_clocks, },
 	{ .parse_prop = parse_interconnects, },
@@ -1379,6 +1393,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_resets, },
 	{ .parse_prop = parse_leds, },
 	{ .parse_prop = parse_backlight, },
+	{ .parse_prop = parse_phy_handle, },
 	{ .parse_prop = parse_gpio_compat, },
 	{ .parse_prop = parse_interrupts, },
 	{ .parse_prop = parse_regulators, },
-- 
2.33.0.rc1.237.g0d66db33f3-goog


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

* Re: [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property
  2021-08-14  2:31 ` [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property Saravana Kannan
@ 2021-08-14 15:22   ` Andrew Lunn
  2021-08-16 20:43     ` Saravana Kannan
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2021-08-14 15:22 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, kernel-team, devicetree, linux-kernel, netdev

Hi Saravana

> Hi Andrew,
> 

> Also there
> are so many phy related properties that my head is spinning. Is there a
> "phy" property (which is different from "phys") that treated exactly as
> "phy-handle"?

Sorry, i don't understand your question.

> +	/*
> +	 * Device tree nodes pointed to by phy-handle never have struct devices
> +	 * created for them even if they have a "compatible" property. So
> +	 * return the parent node pointer.
> +	 */

We have a classic bus with devices on it. The bus master is registers
with linux using one of the mdiobus_register() calls. That then
enumerates the bus, looking at the 32 possible address on the bus,
using mdiobus_scan. It then gets a little complex, due to
history.

Originally, the only thing you could have on an MDIO bus was a
PHY. But devices on MDIO busses are more generic, and Linux gained
support for Ethernet switches on an MDIO bus, and there are a few
other sort device. So to keep the PHY API untouched, but to add these
extra devices, we added the generic struct mdio_device which
represents any sort of device on an MDIO bus. This has a struct device
embedded in it.

When we scan the bus and find a PHY, a struct phy_device is created,
which has an embedded struct mdio_device. The struct device in that is
then registered with the driver core.

So a phy-handle does point to a device, but you need to do an object
orientated style look at the base class to find it.

	   Andrew

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

* Re: [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property
  2021-08-14 15:22   ` Andrew Lunn
@ 2021-08-16 20:43     ` Saravana Kannan
  2021-08-16 21:11       ` Rob Herring
  2021-08-16 21:15       ` Andrew Lunn
  0 siblings, 2 replies; 10+ messages in thread
From: Saravana Kannan @ 2021-08-16 20:43 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Rob Herring, Frank Rowand, kernel-team, devicetree, linux-kernel, netdev

On Sat, Aug 14, 2021 at 8:22 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> Hi Saravana
>
> > Hi Andrew,
> >
>
> > Also there
> > are so many phy related properties that my head is spinning. Is there a
> > "phy" property (which is different from "phys") that treated exactly as
> > "phy-handle"?
>
> Sorry, i don't understand your question.

Sorry. I was just saying I understand the "phy-handle" DT property
(seems specific to ethernet PHY) and "phys" DT property (seems to be
for generic PHYs -- used mostly by display and USB?). But I noticed
there's yet another "phy" DT property which I'm not sure I understand.
It seems to be used by display and ethernet and seems to be a
deprecated property. If you can explain that DT property in the
context of networking and how to interpret it as a human, that'd be
nice.

>
> > +     /*
> > +      * Device tree nodes pointed to by phy-handle never have struct devices
> > +      * created for them even if they have a "compatible" property. So
> > +      * return the parent node pointer.
> > +      */
>
> We have a classic bus with devices on it. The bus master is registers
> with linux using one of the mdiobus_register() calls. That then
> enumerates the bus, looking at the 32 possible address on the bus,
> using mdiobus_scan. It then gets a little complex, due to
> history.
>
> Originally, the only thing you could have on an MDIO bus was a
> PHY. But devices on MDIO busses are more generic, and Linux gained
> support for Ethernet switches on an MDIO bus, and there are a few
> other sort device. So to keep the PHY API untouched, but to add these
> extra devices, we added the generic struct mdio_device which
> represents any sort of device on an MDIO bus. This has a struct device
> embedded in it.
>
> When we scan the bus and find a PHY, a struct phy_device is created,
> which has an embedded struct mdio_device. The struct device in that is
> then registered with the driver core.
>
> So a phy-handle does point to a device, but you need to do an object
> orientated style look at the base class to find it.

Thanks for the detailed explanation. I didn't notice a phy_device had
an mdio_device inside it. Makes sense. I think my comment is not
worded accurately and it really should be:

Device tree nodes pointed to by phy-handle (even if they have a
"compatible" property) will never have struct devices probed and bound
to a driver through the driver framework. It's the parent node/device
that gets bound to a driver and initializes the PHY. So return the
parent node pointer instead.

Does this sound right? As opposed to PHYs the other generic mdio
devices seem to actually have drivers that'll bind to them through the
driver framework.

-Saravana

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

* Re: [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property
  2021-08-16 20:43     ` Saravana Kannan
@ 2021-08-16 21:11       ` Rob Herring
  2021-08-17 18:18         ` Saravana Kannan
  2021-08-16 21:15       ` Andrew Lunn
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2021-08-16 21:11 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Andrew Lunn, Frank Rowand, Android Kernel Team, devicetree,
	linux-kernel, netdev

On Mon, Aug 16, 2021 at 3:43 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Sat, Aug 14, 2021 at 8:22 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > Hi Saravana
> >
> > > Hi Andrew,
> > >
> >
> > > Also there
> > > are so many phy related properties that my head is spinning. Is there a
> > > "phy" property (which is different from "phys") that treated exactly as
> > > "phy-handle"?
> >
> > Sorry, i don't understand your question.
>
> Sorry. I was just saying I understand the "phy-handle" DT property
> (seems specific to ethernet PHY) and "phys" DT property (seems to be
> for generic PHYs -- used mostly by display and USB?). But I noticed
> there's yet another "phy" DT property which I'm not sure I understand.
> It seems to be used by display and ethernet and seems to be a
> deprecated property. If you can explain that DT property in the
> context of networking and how to interpret it as a human, that'd be
> nice.

For net devices, you can have 2 PHYs. 'phys' is the serdes phy and
'phy-handle' is the ethernet (typically) phy. On some chips, a serdes
phy can do PCS (ethernet), SATA, PCIe.

'phy' is deprecated, so ignore it. The one case for displays I see in
display/exynos/exynos_hdmi.txt should be deprecated as well.

There's also 'usb-phy' which should be deprecated.

Rob

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

* Re: [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property
  2021-08-16 20:43     ` Saravana Kannan
  2021-08-16 21:11       ` Rob Herring
@ 2021-08-16 21:15       ` Andrew Lunn
  2021-08-18  2:13         ` Saravana Kannan
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2021-08-16 21:15 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, kernel-team, devicetree, linux-kernel, netdev

On Mon, Aug 16, 2021 at 01:43:19PM -0700, Saravana Kannan wrote:
> On Sat, Aug 14, 2021 at 8:22 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > Hi Saravana
> >
> > > Hi Andrew,
> > >
> >
> > > Also there
> > > are so many phy related properties that my head is spinning. Is there a
> > > "phy" property (which is different from "phys") that treated exactly as
> > > "phy-handle"?
> >
> > Sorry, i don't understand your question.
> 
> Sorry. I was just saying I understand the "phy-handle" DT property
> (seems specific to ethernet PHY) and "phys" DT property (seems to be
> for generic PHYs -- used mostly by display and USB?). But I noticed
> there's yet another "phy" DT property which I'm not sure I understand.
> It seems to be used by display and ethernet and seems to be a
> deprecated property. If you can explain that DT property in the
> context of networking and how to interpret it as a human, that'd be
> nice.

Ah, i think i understand:

Documentation/devicetree/bindings/net/ethernet-controller.yaml

  phy:
    $ref: "#/properties/phy-handle"
    deprecated: true

So it is used the same as phy-handle. I doubt there are many examples
of it, it has been deprecated a long time. Maybe look in the powerpc
dts files?

> > > +     /*
> > > +      * Device tree nodes pointed to by phy-handle never have struct devices
> > > +      * created for them even if they have a "compatible" property. So
> > > +      * return the parent node pointer.
> > > +      */
> >
> > We have a classic bus with devices on it. The bus master is registers
> > with linux using one of the mdiobus_register() calls. That then
> > enumerates the bus, looking at the 32 possible address on the bus,
> > using mdiobus_scan. It then gets a little complex, due to
> > history.
> >
> > Originally, the only thing you could have on an MDIO bus was a
> > PHY. But devices on MDIO busses are more generic, and Linux gained
> > support for Ethernet switches on an MDIO bus, and there are a few
> > other sort device. So to keep the PHY API untouched, but to add these
> > extra devices, we added the generic struct mdio_device which
> > represents any sort of device on an MDIO bus. This has a struct device
> > embedded in it.
> >
> > When we scan the bus and find a PHY, a struct phy_device is created,
> > which has an embedded struct mdio_device. The struct device in that is
> > then registered with the driver core.
> >
> > So a phy-handle does point to a device, but you need to do an object
> > orientated style look at the base class to find it.
> 
> Thanks for the detailed explanation. I didn't notice a phy_device had
> an mdio_device inside it. Makes sense. I think my comment is not
> worded accurately and it really should be:
> 
> Device tree nodes pointed to by phy-handle (even if they have a
> "compatible" property) will never have struct devices probed and bound
> to a driver through the driver framework. It's the parent node/device
> that gets bound to a driver and initializes the PHY. So return the
> parent node pointer instead.
> 
> Does this sound right? As opposed to PHYs the other generic mdio
> devices seem to actually have drivers that'll bind to them through the
> driver framework.

That sounds wrong. The MDIO bus master is a linux device and has a
driver. Same as an I2C bus master, or an SPI bus master, or a USB
host. All these busses have devices on them, same as an MDIO bus. The
devices on the bus are found and registered with the driver
framework. The driver framework, with some help from the mdio bus
class, with then find the correct driver of the device, and probe
it. During probe, it gets initialized by the PHY driver.

So for me, the parent of a PHY would be the MDIO bus master, and the
bus master is not driving the PHY, in the same way an I2C bus master
does not drive the tmp100 temperature sensor on an i2c bus.

But maybe i don't understand your terminology here?

Maybe this will help:

root@370rd:/sys/class/mdio_bus# ls -l
total 0
lrwxrwxrwx 1 root root 0 Jan  2  2021 '!soc!internal-regs!mdio@72004!switch@10!mdio' -> '../../devices/platform/soc/soc:internal-regs/f1072004.mdio/mdio_bus/f1072004.mdio-mii/f1072004.mdio-mii:10/mdio_bus/!soc!internal-regs!mdio@72004!switch@10!mdio'
lrwxrwxrwx 1 root root 0 Jan  2  2021  f1072004.mdio-mii -> ../../devices/platform/soc/soc:internal-regs/f1072004.mdio/mdio_bus/f1072004.mdio-mii
lrwxrwxrwx 1 root root 0 Jan  2  2021  fixed-0 -> '../../devices/platform/Fixed MDIO bus.0/mdio_bus/fixed-0'

So there are three MDIO bus masters.

Going into f1072004.mdio-mii, we see there are two PHYs on this bus:

root@370rd:/sys/class/mdio_bus/f1072004.mdio-mii# ls -l
total 0
lrwxrwxrwx 1 root root    0 Aug 16 21:03 device -> ../../../f1072004.mdio
drwxr-xr-x 5 root root    0 Jan  2  2021 f1072004.mdio-mii:00
drwxr-xr-x 6 root root    0 Jan  2  2021 f1072004.mdio-mii:10
lrwxrwxrwx 1 root root    0 Aug 16 21:03 of_node -> ../../../../../../../firmware/devicetree/base/soc/internal-regs/mdio@72004
drwxr-xr-x 2 root root    0 Aug 16 21:03 power
drwxr-xr-x 2 root root    0 Aug 16 21:03 statistics
lrwxrwxrwx 1 root root    0 Jan  2  2021 subsystem -> ../../../../../../../class/mdio_bus
-rw-r--r-- 1 root root 4096 Jan  2  2021 uevent
-r--r--r-- 1 root root 4096 Aug 16 21:03 waiting_for_supplier

and going into one of the PHYs f1072004.mdio-mii:00

lrwxrwxrwx 1 root root    0 Aug 16 20:54 attached_dev -> ../../../../f1070000.ethernet/net/eth0
lrwxrwxrwx 1 root root    0 Aug 16 20:54 driver -> '../../../../../../../../bus/mdio_bus/drivers/Marvell 88E1510'
drwxr-xr-x 3 root root    0 Jan  2  2021 hwmon
lrwxrwxrwx 1 root root    0 Aug 16 20:54 of_node -> ../../../../../../../../firmware/devicetree/base/soc/internal-regs/mdio@72004/ethernet-phy@0
-r--r--r-- 1 root root 4096 Aug 16 20:54 phy_dev_flags
-r--r--r-- 1 root root 4096 Aug 16 20:54 phy_has_fixups
-r--r--r-- 1 root root 4096 Aug 16 20:54 phy_id
-r--r--r-- 1 root root 4096 Aug 16 20:54 phy_interface
drwxr-xr-x 2 root root    0 Aug 16 20:54 power
drwxr-xr-x 2 root root    0 Aug 16 20:54 statistics
lrwxrwxrwx 1 root root    0 Jan  2  2021 subsystem -> ../../../../../../../../bus/mdio_bus
-rw-r--r-- 1 root root 4096 Jan  2  2021 uevent

The phy-handle in the MAC node points to ethernet-phy@0.

    Andrew

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

* Re: [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property
  2021-08-16 21:11       ` Rob Herring
@ 2021-08-17 18:18         ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2021-08-17 18:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: Andrew Lunn, Frank Rowand, Android Kernel Team, devicetree,
	linux-kernel, netdev

On Mon, Aug 16, 2021 at 2:11 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Mon, Aug 16, 2021 at 3:43 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Sat, Aug 14, 2021 at 8:22 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > Hi Saravana
> > >
> > > > Hi Andrew,
> > > >
> > >
> > > > Also there
> > > > are so many phy related properties that my head is spinning. Is there a
> > > > "phy" property (which is different from "phys") that treated exactly as
> > > > "phy-handle"?
> > >
> > > Sorry, i don't understand your question.
> >
> > Sorry. I was just saying I understand the "phy-handle" DT property
> > (seems specific to ethernet PHY) and "phys" DT property (seems to be
> > for generic PHYs -- used mostly by display and USB?). But I noticed
> > there's yet another "phy" DT property which I'm not sure I understand.
> > It seems to be used by display and ethernet and seems to be a
> > deprecated property. If you can explain that DT property in the
> > context of networking and how to interpret it as a human, that'd be
> > nice.
>
> For net devices, you can have 2 PHYs. 'phys' is the serdes phy and
> 'phy-handle' is the ethernet (typically) phy. On some chips, a serdes
> phy can do PCS (ethernet), SATA, PCIe.
>
> 'phy' is deprecated, so ignore it. The one case for displays I see in
> display/exynos/exynos_hdmi.txt should be deprecated as well.
>
> There's also 'usb-phy' which should be deprecated.

Thanks for the explanation Rob. I'll ignore phy and usb-phy unless it
becomes an issue for any future changes/improvements.

-Saravana


-Saravana

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

* Re: [PATCH v1 1/2] of: property: fw_devlink: Add support for "leds" and "backlight"
  2021-08-14  2:31 ` [PATCH v1 1/2] of: property: fw_devlink: Add support for "leds" and "backlight" Saravana Kannan
@ 2021-08-17 22:02   ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2021-08-17 22:02 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, devicetree, linux-kernel, kernel-team, Frank Rowand

On Fri, 13 Aug 2021 19:31:30 -0700, Saravana Kannan wrote:
> Allows tracking dependencies between leds/backlights devices and their
> consumers.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  drivers/of/property.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Applied, thanks!

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

* Re: [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property
  2021-08-16 21:15       ` Andrew Lunn
@ 2021-08-18  2:13         ` Saravana Kannan
  0 siblings, 0 replies; 10+ messages in thread
From: Saravana Kannan @ 2021-08-18  2:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Rob Herring, Frank Rowand, kernel-team, devicetree, linux-kernel, netdev

On Mon, Aug 16, 2021 at 2:15 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Aug 16, 2021 at 01:43:19PM -0700, Saravana Kannan wrote:
> > On Sat, Aug 14, 2021 at 8:22 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > Hi Saravana
> > >
> > > > Hi Andrew,
> > > >
> > >
> > > > Also there
> > > > are so many phy related properties that my head is spinning. Is there a
> > > > "phy" property (which is different from "phys") that treated exactly as
> > > > "phy-handle"?
> > >
> > > Sorry, i don't understand your question.
> >
> > Sorry. I was just saying I understand the "phy-handle" DT property
> > (seems specific to ethernet PHY) and "phys" DT property (seems to be
> > for generic PHYs -- used mostly by display and USB?). But I noticed
> > there's yet another "phy" DT property which I'm not sure I understand.
> > It seems to be used by display and ethernet and seems to be a
> > deprecated property. If you can explain that DT property in the
> > context of networking and how to interpret it as a human, that'd be
> > nice.
>
> Ah, i think i understand:
>
> Documentation/devicetree/bindings/net/ethernet-controller.yaml
>
>   phy:
>     $ref: "#/properties/phy-handle"
>     deprecated: true
>
> So it is used the same as phy-handle. I doubt there are many examples
> of it, it has been deprecated a long time. Maybe look in the powerpc
> dts files?
>
> > > > +     /*
> > > > +      * Device tree nodes pointed to by phy-handle never have struct devices
> > > > +      * created for them even if they have a "compatible" property. So
> > > > +      * return the parent node pointer.
> > > > +      */
> > >
> > > We have a classic bus with devices on it. The bus master is registers
> > > with linux using one of the mdiobus_register() calls. That then
> > > enumerates the bus, looking at the 32 possible address on the bus,
> > > using mdiobus_scan. It then gets a little complex, due to
> > > history.
> > >
> > > Originally, the only thing you could have on an MDIO bus was a
> > > PHY. But devices on MDIO busses are more generic, and Linux gained
> > > support for Ethernet switches on an MDIO bus, and there are a few
> > > other sort device. So to keep the PHY API untouched, but to add these
> > > extra devices, we added the generic struct mdio_device which
> > > represents any sort of device on an MDIO bus. This has a struct device
> > > embedded in it.
> > >
> > > When we scan the bus and find a PHY, a struct phy_device is created,
> > > which has an embedded struct mdio_device. The struct device in that is
> > > then registered with the driver core.
> > >
> > > So a phy-handle does point to a device, but you need to do an object
> > > orientated style look at the base class to find it.
> >
> > Thanks for the detailed explanation. I didn't notice a phy_device had
> > an mdio_device inside it. Makes sense. I think my comment is not
> > worded accurately and it really should be:
> >
> > Device tree nodes pointed to by phy-handle (even if they have a
> > "compatible" property) will never have struct devices probed and bound
> > to a driver through the driver framework. It's the parent node/device
> > that gets bound to a driver and initializes the PHY. So return the
> > parent node pointer instead.
> >
> > Does this sound right? As opposed to PHYs the other generic mdio
> > devices seem to actually have drivers that'll bind to them through the
> > driver framework.
>
> That sounds wrong. The MDIO bus master is a linux device and has a
> driver. Same as an I2C bus master, or an SPI bus master, or a USB
> host. All these busses have devices on them, same as an MDIO bus. The
> devices on the bus are found and registered with the driver
> framework. The driver framework, with some help from the mdio bus
> class, with then find the correct driver of the device, and probe
> it. During probe, it gets initialized by the PHY driver.
>
> So for me, the parent of a PHY would be the MDIO bus master, and the
> bus master is not driving the PHY, in the same way an I2C bus master
> does not drive the tmp100 temperature sensor on an i2c bus.
>
> But maybe i don't understand your terminology here?
>
> Maybe this will help:
>
> root@370rd:/sys/class/mdio_bus# ls -l
> total 0
> lrwxrwxrwx 1 root root 0 Jan  2  2021 '!soc!internal-regs!mdio@72004!switch@10!mdio' -> '../../devices/platform/soc/soc:internal-regs/f1072004.mdio/mdio_bus/f1072004.mdio-mii/f1072004.mdio-mii:10/mdio_bus/!soc!internal-regs!mdio@72004!switch@10!mdio'
> lrwxrwxrwx 1 root root 0 Jan  2  2021  f1072004.mdio-mii -> ../../devices/platform/soc/soc:internal-regs/f1072004.mdio/mdio_bus/f1072004.mdio-mii
> lrwxrwxrwx 1 root root 0 Jan  2  2021  fixed-0 -> '../../devices/platform/Fixed MDIO bus.0/mdio_bus/fixed-0'
>
> So there are three MDIO bus masters.
>
> Going into f1072004.mdio-mii, we see there are two PHYs on this bus:
>
> root@370rd:/sys/class/mdio_bus/f1072004.mdio-mii# ls -l
> total 0
> lrwxrwxrwx 1 root root    0 Aug 16 21:03 device -> ../../../f1072004.mdio
> drwxr-xr-x 5 root root    0 Jan  2  2021 f1072004.mdio-mii:00
> drwxr-xr-x 6 root root    0 Jan  2  2021 f1072004.mdio-mii:10
> lrwxrwxrwx 1 root root    0 Aug 16 21:03 of_node -> ../../../../../../../firmware/devicetree/base/soc/internal-regs/mdio@72004
> drwxr-xr-x 2 root root    0 Aug 16 21:03 power
> drwxr-xr-x 2 root root    0 Aug 16 21:03 statistics
> lrwxrwxrwx 1 root root    0 Jan  2  2021 subsystem -> ../../../../../../../class/mdio_bus
> -rw-r--r-- 1 root root 4096 Jan  2  2021 uevent
> -r--r--r-- 1 root root 4096 Aug 16 21:03 waiting_for_supplier
>
> and going into one of the PHYs f1072004.mdio-mii:00
>
> lrwxrwxrwx 1 root root    0 Aug 16 20:54 attached_dev -> ../../../../f1070000.ethernet/net/eth0
> lrwxrwxrwx 1 root root    0 Aug 16 20:54 driver -> '../../../../../../../../bus/mdio_bus/drivers/Marvell 88E1510'
> drwxr-xr-x 3 root root    0 Jan  2  2021 hwmon
> lrwxrwxrwx 1 root root    0 Aug 16 20:54 of_node -> ../../../../../../../../firmware/devicetree/base/soc/internal-regs/mdio@72004/ethernet-phy@0
> -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_dev_flags
> -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_has_fixups
> -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_id
> -r--r--r-- 1 root root 4096 Aug 16 20:54 phy_interface
> drwxr-xr-x 2 root root    0 Aug 16 20:54 power
> drwxr-xr-x 2 root root    0 Aug 16 20:54 statistics
> lrwxrwxrwx 1 root root    0 Jan  2  2021 subsystem -> ../../../../../../../../bus/mdio_bus
> -rw-r--r-- 1 root root 4096 Jan  2  2021 uevent
>
> The phy-handle in the MAC node points to ethernet-phy@0.

Thanks! Looks like I got confused a bit based on some misconceptions I
got when working on unrelated patches. I read through the code and now
understand how this works. I'll fix up this patch and send out a v2
(it actually makes the patch simpler).

On an unrelated note, I'm still a bit confused on what's going on in
get_phy_device() when one PHY C45 device has multiple IDs found by
get_phy_c45_ids(). Based on the comments, it looks like each of those
IDs might be separate devices? Or am I misunderstanding the comment?
If they are separate devices, how is one phy_device representing them
all?

-Saravana

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

end of thread, other threads:[~2021-08-18  2:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-14  2:31 [PATCH v1 0/2] of: property: Support for few more properties Saravana Kannan
2021-08-14  2:31 ` [PATCH v1 1/2] of: property: fw_devlink: Add support for "leds" and "backlight" Saravana Kannan
2021-08-17 22:02   ` Rob Herring
2021-08-14  2:31 ` [PATCH v1 2/2] of: property: fw_devlink: Add support for "phy-handle" property Saravana Kannan
2021-08-14 15:22   ` Andrew Lunn
2021-08-16 20:43     ` Saravana Kannan
2021-08-16 21:11       ` Rob Herring
2021-08-17 18:18         ` Saravana Kannan
2021-08-16 21:15       ` Andrew Lunn
2021-08-18  2:13         ` Saravana Kannan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).