* [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
@ 2023-06-14 21:44 Maxim Kiselev
2023-07-22 8:20 ` Ramon Fried
2024-01-16 0:17 ` Andre Przywara
0 siblings, 2 replies; 8+ messages in thread
From: Maxim Kiselev @ 2023-06-14 21:44 UTC (permalink / raw)
To: u-boot; +Cc: Maksim Kiselev, Joe Hershberger, Ramon Fried
From: Maksim Kiselev <bigunclemax@gmail.com>
Based on dt-specs fixed-link doesn't require phy-handle to be used.
Fix driver to only read phy related setting when phy-handle is found.
Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
---
drivers/net/sun8i_emac.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 04c3274fbe..0e339d69e0 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
priv->use_internal_phy = false;
offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
- if (offset < 0) {
- debug("%s: Cannot find PHY address\n", __func__);
- return -EINVAL;
- }
- priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
+ if (offset >= 0)
+ priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
pdata->phy_interface = dev_read_phy_mode(dev);
debug("phy interface %d\n", pdata->phy_interface);
--
2.39.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
2023-06-14 21:44 [PATCH v1] net: sun8i-emac: Add support for fixed-link phy Maxim Kiselev
@ 2023-07-22 8:20 ` Ramon Fried
2024-01-16 0:17 ` Andre Przywara
1 sibling, 0 replies; 8+ messages in thread
From: Ramon Fried @ 2023-07-22 8:20 UTC (permalink / raw)
To: Maxim Kiselev; +Cc: u-boot, Joe Hershberger
On Thu, Jun 15, 2023 at 12:51 AM Maxim Kiselev <bigunclemax@gmail.com> wrote:
>
> From: Maksim Kiselev <bigunclemax@gmail.com>
>
> Based on dt-specs fixed-link doesn't require phy-handle to be used.
> Fix driver to only read phy related setting when phy-handle is found.
>
> Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> ---
> drivers/net/sun8i_emac.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> index 04c3274fbe..0e339d69e0 100644
> --- a/drivers/net/sun8i_emac.c
> +++ b/drivers/net/sun8i_emac.c
> @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> priv->use_internal_phy = false;
>
> offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> - if (offset < 0) {
> - debug("%s: Cannot find PHY address\n", __func__);
> - return -EINVAL;
> - }
> - priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> + if (offset >= 0)
> + priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
>
> pdata->phy_interface = dev_read_phy_mode(dev);
> debug("phy interface %d\n", pdata->phy_interface);
> --
> 2.39.2
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
2023-06-14 21:44 [PATCH v1] net: sun8i-emac: Add support for fixed-link phy Maxim Kiselev
2023-07-22 8:20 ` Ramon Fried
@ 2024-01-16 0:17 ` Andre Przywara
2024-01-16 16:58 ` Maxim Kiselev
1 sibling, 1 reply; 8+ messages in thread
From: Andre Przywara @ 2024-01-16 0:17 UTC (permalink / raw)
To: Maxim Kiselev; +Cc: u-boot, Joe Hershberger, Ramon Fried
On Thu, 15 Jun 2023 00:44:06 +0300
Maxim Kiselev <bigunclemax@gmail.com> wrote:
Hi Maxim,
> From: Maksim Kiselev <bigunclemax@gmail.com>
>
> Based on dt-specs fixed-link doesn't require phy-handle to be used.
Do you have such a board? And where is that written down? I don't see
it explicitly mentioned as optional in ethernet-controller.yaml or in
the DT spec PDF. The sun8i EMAC binding lists phy-handle as required,
so that would need to be relaxed there then.
> Fix driver to only read phy related setting when phy-handle is found.
The patch itself looks fine, we already specify -1 as the default when
the PHY DT node does not contain a reg property, so that looks like it
would work.
Cheers,
Andre
> Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> ---
> drivers/net/sun8i_emac.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> index 04c3274fbe..0e339d69e0 100644
> --- a/drivers/net/sun8i_emac.c
> +++ b/drivers/net/sun8i_emac.c
> @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> priv->use_internal_phy = false;
>
> offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> - if (offset < 0) {
> - debug("%s: Cannot find PHY address\n", __func__);
> - return -EINVAL;
> - }
> - priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> + if (offset >= 0)
> + priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
>
> pdata->phy_interface = dev_read_phy_mode(dev);
> debug("phy interface %d\n", pdata->phy_interface);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
2024-01-16 0:17 ` Andre Przywara
@ 2024-01-16 16:58 ` Maxim Kiselev
2024-01-17 0:13 ` Andre Przywara
2024-01-19 17:35 ` Andre Przywara
0 siblings, 2 replies; 8+ messages in thread
From: Maxim Kiselev @ 2024-01-16 16:58 UTC (permalink / raw)
To: Andre Przywara; +Cc: u-boot, Joe Hershberger, Ramon Fried
вт, 16 янв. 2024 г. в 03:18, Andre Przywara <andre.przywara@arm.com>:
>
> On Thu, 15 Jun 2023 00:44:06 +0300
> Maxim Kiselev <bigunclemax@gmail.com> wrote:
>
> Hi Maxim,
>
> > From: Maksim Kiselev <bigunclemax@gmail.com>
> >
> > Based on dt-specs fixed-link doesn't require phy-handle to be used.
>
> Do you have such a board?
Yes, I had a custom board with T113 SoC which has fixed phy eth.
> And where is that written down? I don't see
> it explicitly mentioned as optional in ethernet-controller.yaml or in
> the DT spec PDF.
Sorry for that commit description. I just found the similar commit, that
fixes the same problem for zynq_gem
3888c8d1979289efe685fe29276aed4d4b685975 ("net: zynq_gem: Add support for
fixed-link phy") and copied the description from there.
> The sun8i EMAC binding lists phy-handle as required,
> so that would need to be relaxed there then.
Oh, indeed. So it will require to send dt-binding changes to Linux first...
Therefore, I think we can live without fixed-phy support for sun8i EMAC.
At least until some device with such a configuration appears on the
market :)
> > Fix driver to only read phy related setting when phy-handle is found.
>
> The patch itself looks fine, we already specify -1 as the default when
> the PHY DT node does not contain a reg property, so that looks like it
> would work.
>
> Cheers,
> Andre
>
> > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> > ---
> > drivers/net/sun8i_emac.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > index 04c3274fbe..0e339d69e0 100644
> > --- a/drivers/net/sun8i_emac.c
> > +++ b/drivers/net/sun8i_emac.c
> > @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> > priv->use_internal_phy = false;
> >
> > offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> > - if (offset < 0) {
> > - debug("%s: Cannot find PHY address\n", __func__);
> > - return -EINVAL;
> > - }
> > - priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > + if (offset >= 0)
> > + priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> >
> > pdata->phy_interface = dev_read_phy_mode(dev);
> > debug("phy interface %d\n", pdata->phy_interface);
>
Best wishes,
Maksim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
2024-01-16 16:58 ` Maxim Kiselev
@ 2024-01-17 0:13 ` Andre Przywara
2024-01-19 17:35 ` Andre Przywara
1 sibling, 0 replies; 8+ messages in thread
From: Andre Przywara @ 2024-01-17 0:13 UTC (permalink / raw)
To: Maxim Kiselev; +Cc: u-boot, Joe Hershberger, Ramon Fried
On Tue, 16 Jan 2024 19:58:56 +0300
Maxim Kiselev <bigunclemax@gmail.com> wrote:
Hi Maxim,
> вт, 16 янв. 2024 г. в 03:18, Andre Przywara <andre.przywara@arm.com>:
> >
> > On Thu, 15 Jun 2023 00:44:06 +0300
> > Maxim Kiselev <bigunclemax@gmail.com> wrote:
> >
> > Hi Maxim,
> >
> > > From: Maksim Kiselev <bigunclemax@gmail.com>
> > >
> > > Based on dt-specs fixed-link doesn't require phy-handle to be used.
> >
> > Do you have such a board?
>
> Yes, I had a custom board with T113 SoC which has fixed phy eth.
>
> > And where is that written down? I don't see
> > it explicitly mentioned as optional in ethernet-controller.yaml or in
> > the DT spec PDF.
>
> Sorry for that commit description. I just found the similar commit, that
> fixes the same problem for zynq_gem
> 3888c8d1979289efe685fe29276aed4d4b685975 ("net: zynq_gem: Add support for
> fixed-link phy") and copied the description from there.
>
> > The sun8i EMAC binding lists phy-handle as required,
> > so that would need to be relaxed there then.
>
> Oh, indeed. So it will require to send dt-binding changes to Linux first...
Well, in the long run yes, but this doesn't mean that this patch needs
to wait - if that fixes a problem for you. I think in general that
rationale is probably true and phy-handle should be optional,
especially since the code can already cope with it.
I would just like to have this documented in the commit message.
Cheers,
Andre
> Therefore, I think we can live without fixed-phy support for sun8i EMAC.
> At least until some device with such a configuration appears on the
> market :)
>
> > > Fix driver to only read phy related setting when phy-handle is found.
> >
> > The patch itself looks fine, we already specify -1 as the default when
> > the PHY DT node does not contain a reg property, so that looks like it
> > would work.
> >
> > Cheers,
> > Andre
> >
> > > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> > > ---
> > > drivers/net/sun8i_emac.c | 7 ++-----
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > index 04c3274fbe..0e339d69e0 100644
> > > --- a/drivers/net/sun8i_emac.c
> > > +++ b/drivers/net/sun8i_emac.c
> > > @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> > > priv->use_internal_phy = false;
> > >
> > > offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> > > - if (offset < 0) {
> > > - debug("%s: Cannot find PHY address\n", __func__);
> > > - return -EINVAL;
> > > - }
> > > - priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > > + if (offset >= 0)
> > > + priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > >
> > > pdata->phy_interface = dev_read_phy_mode(dev);
> > > debug("phy interface %d\n", pdata->phy_interface);
> >
>
> Best wishes,
> Maksim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
2024-01-16 16:58 ` Maxim Kiselev
2024-01-17 0:13 ` Andre Przywara
@ 2024-01-19 17:35 ` Andre Przywara
2024-01-19 18:11 ` Maxim Kiselev
1 sibling, 1 reply; 8+ messages in thread
From: Andre Przywara @ 2024-01-19 17:35 UTC (permalink / raw)
To: Maxim Kiselev; +Cc: u-boot, Joe Hershberger, Ramon Fried
On Tue, 16 Jan 2024 19:58:56 +0300
Maxim Kiselev <bigunclemax@gmail.com> wrote:
Hi Maxim,
> вт, 16 янв. 2024 г. в 03:18, Andre Przywara <andre.przywara@arm.com>:
> >
> > On Thu, 15 Jun 2023 00:44:06 +0300
> > Maxim Kiselev <bigunclemax@gmail.com> wrote:
> >
> > Hi Maxim,
> >
> > > From: Maksim Kiselev <bigunclemax@gmail.com>
> > >
> > > Based on dt-specs fixed-link doesn't require phy-handle to be used.
> >
> > Do you have such a board?
>
> Yes, I had a custom board with T113 SoC which has fixed phy eth.
So just to clarify: this board connected something like a switch directly
to the MAC, just using RMII/RGMII? And then you have a fixed-phy node in
the DT, detailing the speed and duplex parameters? Matching the fixed-phy
node description in ethernet-controller.yaml? And drivers/net/phy/fixed.c
takes care of parsing that?
> > And where is that written down? I don't see
> > it explicitly mentioned as optional in ethernet-controller.yaml or in
> > the DT spec PDF.
>
> Sorry for that commit description. I just found the similar commit, that
> fixes the same problem for zynq_gem
> 3888c8d1979289efe685fe29276aed4d4b685975 ("net: zynq_gem: Add support for
> fixed-link phy") and copied the description from there.
>
> > The sun8i EMAC binding lists phy-handle as required,
> > so that would need to be relaxed there then.
>
> Oh, indeed. So it will require to send dt-binding changes to Linux first...
>
> Therefore, I think we can live without fixed-phy support for sun8i EMAC.
> At least until some device with such a configuration appears on the
> market :)
Well, I am fine with that patch if it fixes a problem for you. It looks
like requiring the phy-handle property is too strict, and just needs to be
relaxed in the binding. If I see this correctly, the driver would still
accept every valid DT, by today's and future bindings?
If you could just confirm my above assumptions, and maybe send a v2 with an
amended commit message, I would be happy to merge that patch.
Cheers,
Andre
> > > Fix driver to only read phy related setting when phy-handle is found.
> >
> > The patch itself looks fine, we already specify -1 as the default when
> > the PHY DT node does not contain a reg property, so that looks like it
> > would work.
> >
> > Cheers,
> > Andre
> >
> > > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> > > ---
> > > drivers/net/sun8i_emac.c | 7 ++-----
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > index 04c3274fbe..0e339d69e0 100644
> > > --- a/drivers/net/sun8i_emac.c
> > > +++ b/drivers/net/sun8i_emac.c
> > > @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> > > priv->use_internal_phy = false;
> > >
> > > offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> > > - if (offset < 0) {
> > > - debug("%s: Cannot find PHY address\n", __func__);
> > > - return -EINVAL;
> > > - }
> > > - priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > > + if (offset >= 0)
> > > + priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > >
> > > pdata->phy_interface = dev_read_phy_mode(dev);
> > > debug("phy interface %d\n", pdata->phy_interface);
> >
>
> Best wishes,
> Maksim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
2024-01-19 17:35 ` Andre Przywara
@ 2024-01-19 18:11 ` Maxim Kiselev
2024-01-19 21:56 ` Andre Przywara
0 siblings, 1 reply; 8+ messages in thread
From: Maxim Kiselev @ 2024-01-19 18:11 UTC (permalink / raw)
To: Andre Przywara; +Cc: u-boot, Joe Hershberger, Ramon Fried
Hi Andre,
пт, 19 янв. 2024 г. в 20:35, Andre Przywara <andre.przywara@arm.com>:
>
> On Tue, 16 Jan 2024 19:58:56 +0300
> Maxim Kiselev <bigunclemax@gmail.com> wrote:
>
> Hi Maxim,
>
> > вт, 16 янв. 2024 г. в 03:18, Andre Przywara <andre.przywara@arm.com>:
> > >
> > > On Thu, 15 Jun 2023 00:44:06 +0300
> > > Maxim Kiselev <bigunclemax@gmail.com> wrote:
> > >
> > > Hi Maxim,
> > >
> > > > From: Maksim Kiselev <bigunclemax@gmail.com>
> > > >
> > > > Based on dt-specs fixed-link doesn't require phy-handle to be used.
> > >
> > > Do you have such a board?
> >
> > Yes, I had a custom board with T113 SoC which has fixed phy eth.
>
> So just to clarify: this board connected something like a switch directly
> to the MAC, just using RMII/RGMII? And then you have a fixed-phy node in
> the DT, detailing the speed and duplex parameters? Matching the fixed-phy
> node description in ethernet-controller.yaml? And drivers/net/phy/fixed.c
> takes care of parsing that?
Yes, you are absolutely right. The T113s connected directly to the switch port
via RMII interface. Here is the part of DT, that describes that configuration
&emac {
pinctrl-names = "default";
pinctrl-0 = <&rmii_pg_pins>;
phy-mode = "rmii";
snps,reset-gpio = <&pio 4 12 GPIO_ACTIVE_LOW>; /* PE12 */
allwinner,tx-delay-ps = <700>;
allwinner,rx-delay-ps = <3100>;
fixed-link {
speed = <100>;
full-duplex;
};
};
>
> > > And where is that written down? I don't see
> > > it explicitly mentioned as optional in ethernet-controller.yaml or in
> > > the DT spec PDF.
> >
> > Sorry for that commit description. I just found the similar commit, that
> > fixes the same problem for zynq_gem
> > 3888c8d1979289efe685fe29276aed4d4b685975 ("net: zynq_gem: Add support for
> > fixed-link phy") and copied the description from there.
> >
> > > The sun8i EMAC binding lists phy-handle as required,
> > > so that would need to be relaxed there then.
> >
> > Oh, indeed. So it will require to send dt-binding changes to Linux first...
> >
> > Therefore, I think we can live without fixed-phy support for sun8i EMAC.
> > At least until some device with such a configuration appears on the
> > market :)
>
> Well, I am fine with that patch if it fixes a problem for you. It looks
> like requiring the phy-handle property is too strict, and just needs to be
> relaxed in the binding. If I see this correctly, the driver would still
> accept every valid DT, by today's and future bindings?
>
> If you could just confirm my above assumptions, and maybe send a v2 with an
> amended commit message, I would be happy to merge that patch.
Sure, I'll fix the commit description and send v2 asap.
> Cheers,
> Andre
>
> > > > Fix driver to only read phy related setting when phy-handle is found.
> > >
> > > The patch itself looks fine, we already specify -1 as the default when
> > > the PHY DT node does not contain a reg property, so that looks like it
> > > would work.
> > >
> > > Cheers,
> > > Andre
> > >
> > > > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> > > > ---
> > > > drivers/net/sun8i_emac.c | 7 ++-----
> > > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > > index 04c3274fbe..0e339d69e0 100644
> > > > --- a/drivers/net/sun8i_emac.c
> > > > +++ b/drivers/net/sun8i_emac.c
> > > > @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> > > > priv->use_internal_phy = false;
> > > >
> > > > offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> > > > - if (offset < 0) {
> > > > - debug("%s: Cannot find PHY address\n", __func__);
> > > > - return -EINVAL;
> > > > - }
> > > > - priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > > > + if (offset >= 0)
> > > > + priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > > >
> > > > pdata->phy_interface = dev_read_phy_mode(dev);
> > > > debug("phy interface %d\n", pdata->phy_interface);
> > >
> >
> > Best wishes,
> > Maksim
>
Best wishes,
Maksim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] net: sun8i-emac: Add support for fixed-link phy
2024-01-19 18:11 ` Maxim Kiselev
@ 2024-01-19 21:56 ` Andre Przywara
0 siblings, 0 replies; 8+ messages in thread
From: Andre Przywara @ 2024-01-19 21:56 UTC (permalink / raw)
To: Maxim Kiselev; +Cc: u-boot, Joe Hershberger, Ramon Fried
On Fri, 19 Jan 2024 21:11:07 +0300
Maxim Kiselev <bigunclemax@gmail.com> wrote:
Hi Maxim,
> пт, 19 янв. 2024 г. в 20:35, Andre Przywara <andre.przywara@arm.com>:
> >
> > On Tue, 16 Jan 2024 19:58:56 +0300
> > Maxim Kiselev <bigunclemax@gmail.com> wrote:
> >
> > Hi Maxim,
> >
> > > вт, 16 янв. 2024 г. в 03:18, Andre Przywara <andre.przywara@arm.com>:
> > > >
> > > > On Thu, 15 Jun 2023 00:44:06 +0300
> > > > Maxim Kiselev <bigunclemax@gmail.com> wrote:
> > > >
> > > > Hi Maxim,
> > > >
> > > > > From: Maksim Kiselev <bigunclemax@gmail.com>
> > > > >
> > > > > Based on dt-specs fixed-link doesn't require phy-handle to be used.
> > > >
> > > > Do you have such a board?
> > >
> > > Yes, I had a custom board with T113 SoC which has fixed phy eth.
> >
> > So just to clarify: this board connected something like a switch directly
> > to the MAC, just using RMII/RGMII? And then you have a fixed-phy node in
> > the DT, detailing the speed and duplex parameters? Matching the fixed-phy
> > node description in ethernet-controller.yaml? And drivers/net/phy/fixed.c
> > takes care of parsing that?
>
> Yes, you are absolutely right. The T113s connected directly to the switch port
> via RMII interface. Here is the part of DT, that describes that configuration
Excellent, thanks for the confirmation!
Looking forward to v2!
Cheers,
Andre
> &emac {
> pinctrl-names = "default";
> pinctrl-0 = <&rmii_pg_pins>;
> phy-mode = "rmii";
> snps,reset-gpio = <&pio 4 12 GPIO_ACTIVE_LOW>; /* PE12 */
> allwinner,tx-delay-ps = <700>;
> allwinner,rx-delay-ps = <3100>;
>
> fixed-link {
> speed = <100>;
> full-duplex;
> };
> };
>
> >
> > > > And where is that written down? I don't see
> > > > it explicitly mentioned as optional in ethernet-controller.yaml or in
> > > > the DT spec PDF.
> > >
> > > Sorry for that commit description. I just found the similar commit, that
> > > fixes the same problem for zynq_gem
> > > 3888c8d1979289efe685fe29276aed4d4b685975 ("net: zynq_gem: Add support for
> > > fixed-link phy") and copied the description from there.
> > >
> > > > The sun8i EMAC binding lists phy-handle as required,
> > > > so that would need to be relaxed there then.
> > >
> > > Oh, indeed. So it will require to send dt-binding changes to Linux first...
> > >
> > > Therefore, I think we can live without fixed-phy support for sun8i EMAC.
> > > At least until some device with such a configuration appears on the
> > > market :)
> >
> > Well, I am fine with that patch if it fixes a problem for you. It looks
> > like requiring the phy-handle property is too strict, and just needs to be
> > relaxed in the binding. If I see this correctly, the driver would still
> > accept every valid DT, by today's and future bindings?
> >
> > If you could just confirm my above assumptions, and maybe send a v2 with an
> > amended commit message, I would be happy to merge that patch.
>
> Sure, I'll fix the commit description and send v2 asap.
>
> > Cheers,
> > Andre
> >
> > > > > Fix driver to only read phy related setting when phy-handle is found.
> > > >
> > > > The patch itself looks fine, we already specify -1 as the default when
> > > > the PHY DT node does not contain a reg property, so that looks like it
> > > > would work.
> > > >
> > > > Cheers,
> > > > Andre
> > > >
> > > > > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> > > > > ---
> > > > > drivers/net/sun8i_emac.c | 7 ++-----
> > > > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > > > index 04c3274fbe..0e339d69e0 100644
> > > > > --- a/drivers/net/sun8i_emac.c
> > > > > +++ b/drivers/net/sun8i_emac.c
> > > > > @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> > > > > priv->use_internal_phy = false;
> > > > >
> > > > > offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> > > > > - if (offset < 0) {
> > > > > - debug("%s: Cannot find PHY address\n", __func__);
> > > > > - return -EINVAL;
> > > > > - }
> > > > > - priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > > > > + if (offset >= 0)
> > > > > + priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > > > >
> > > > > pdata->phy_interface = dev_read_phy_mode(dev);
> > > > > debug("phy interface %d\n", pdata->phy_interface);
> > > >
> > >
> > > Best wishes,
> > > Maksim
> >
>
> Best wishes,
> Maksim
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-01-19 21:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-14 21:44 [PATCH v1] net: sun8i-emac: Add support for fixed-link phy Maxim Kiselev
2023-07-22 8:20 ` Ramon Fried
2024-01-16 0:17 ` Andre Przywara
2024-01-16 16:58 ` Maxim Kiselev
2024-01-17 0:13 ` Andre Przywara
2024-01-19 17:35 ` Andre Przywara
2024-01-19 18:11 ` Maxim Kiselev
2024-01-19 21:56 ` Andre Przywara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox