Hello,
This patch removes a board specific hook for sama5d3xek boards from the
sama5d3 generic DT board file.
This hook (which register a phy fixup configuring board specific delays
in the ksz9021 ethernet phy) is now replaced by the appropriate DT
properties definitions in the sama5d3xcm.dtsi file.
Best Regards,
Boris
Changes since v1:
- fix txc-skew-ps and rxc-skew-ps delays
- remove phy address info to handle Ronetix and Embest HW designs
Boris BREZILLON (2):
ARM: at91/dt: describe rgmii ethernet phy connected to sama5d3xek
boards
ARM: at91: remove phy fixup for sama5d3xek boards
arch/arm/boot/dts/sama5d3xcm.dtsi | 15 +++++++++++++++
arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
2 files changed, 15 insertions(+), 22 deletions(-)
--
1.8.3.2
Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
The PHY address is not specified here because atmel have 2 different
designs
for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
(Embest design) and the other one is connection PHYAD0 to a pull up
resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
As a result, Ronetix design will have its PHY available at address 0x1 and
Embest design at 0x7.
Let the net PHY core automatically detect the PHY address by scanning the
MDIO bus.
Define board specific delays to apply to RGMII signals.
Signed-off-by: Boris BREZILLON <[email protected]>
---
arch/arm/boot/dts/sama5d3xcm.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/sama5d3xcm.dtsi b/arch/arm/boot/dts/sama5d3xcm.dtsi
index b0b1331..fc68bae 100644
--- a/arch/arm/boot/dts/sama5d3xcm.dtsi
+++ b/arch/arm/boot/dts/sama5d3xcm.dtsi
@@ -34,6 +34,21 @@
macb0: ethernet@f0028000 {
phy-mode = "rgmii";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-phy {
+ interrupt-parent = <&pioB>;
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ txen-skew-ps = <800>;
+ txc-skew-ps = <3000>;
+ rxdv-skew-ps = <400>;
+ rxc-skew-ps = <3000>;
+ rxd0-skew-ps = <400>;
+ rxd1-skew-ps = <400>;
+ rxd2-skew-ps = <400>;
+ rxd3-skew-ps = <400>;
+ };
};
pmc: pmc@fffffc00 {
--
1.8.3.2
These board specific delays are now configured through micrel's specific
DT bindings (see Documentation/devicetree/bindings/net/micrel-ksz9021.txt).
Remove this phy fixup registration from sama5 DT machine file to keep it
as generic as possible.
Signed-off-by: Boris BREZILLON <[email protected]>
---
arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
1 file changed, 22 deletions(-)
diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
index d6fe04b..8c5814f 100644
--- a/arch/arm/mach-at91/board-dt-sama5.c
+++ b/arch/arm/mach-at91/board-dt-sama5.c
@@ -35,30 +35,8 @@ static void __init sama5_dt_timer_init(void)
at91sam926x_pit_init();
}
-static int ksz9021rn_phy_fixup(struct phy_device *phy)
-{
- int value;
-
- /* Set delay values */
- value = MICREL_KSZ9021_RGMII_CLK_CTRL_PAD_SCEW | 0x8000;
- phy_write(phy, MICREL_KSZ9021_EXTREG_CTRL, value);
- value = 0xF2F4;
- phy_write(phy, MICREL_KSZ9021_EXTREG_DATA_WRITE, value);
- value = MICREL_KSZ9021_RGMII_RX_DATA_PAD_SCEW | 0x8000;
- phy_write(phy, MICREL_KSZ9021_EXTREG_CTRL, value);
- value = 0x2222;
- phy_write(phy, MICREL_KSZ9021_EXTREG_DATA_WRITE, value);
-
- return 0;
-}
-
static void __init sama5_dt_device_init(void)
{
- if (of_machine_is_compatible("atmel,sama5d3xcm") &&
- IS_ENABLED(CONFIG_PHYLIB))
- phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
- ksz9021rn_phy_fixup);
-
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}
--
1.8.3.2
Hi Boris,
2014-06-26 3:13 GMT-07:00 Boris BREZILLON <[email protected]>:
> Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
>
> The PHY address is not specified here because atmel have 2 different
> designs
> for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
> (Embest design) and the other one is connection PHYAD0 to a pull up
> resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
> As a result, Ronetix design will have its PHY available at address 0x1 and
> Embest design at 0x7.
> Let the net PHY core automatically detect the PHY address by scanning the
> MDIO bus.
I though the compatible string was listed as a required property, but
it is not. The 'reg' property however is listed as required, although
the of_miodbus_register() works just fine without it, although that is
a Linux-specific implementation detail.
>
> Define board specific delays to apply to RGMII signals.
>
> Signed-off-by: Boris BREZILLON <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
> ---
> arch/arm/boot/dts/sama5d3xcm.dtsi | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sama5d3xcm.dtsi b/arch/arm/boot/dts/sama5d3xcm.dtsi
> index b0b1331..fc68bae 100644
> --- a/arch/arm/boot/dts/sama5d3xcm.dtsi
> +++ b/arch/arm/boot/dts/sama5d3xcm.dtsi
> @@ -34,6 +34,21 @@
>
> macb0: ethernet@f0028000 {
> phy-mode = "rgmii";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy {
> + interrupt-parent = <&pioB>;
> + interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> + txen-skew-ps = <800>;
> + txc-skew-ps = <3000>;
> + rxdv-skew-ps = <400>;
> + rxc-skew-ps = <3000>;
> + rxd0-skew-ps = <400>;
> + rxd1-skew-ps = <400>;
> + rxd2-skew-ps = <400>;
> + rxd3-skew-ps = <400>;
> + };
> };
>
> pmc: pmc@fffffc00 {
> --
> 1.8.3.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Florian
Hi Florian,
On 26/06/2014 20:15, Florian Fainelli wrote:
> Hi Boris,
>
> 2014-06-26 3:13 GMT-07:00 Boris BREZILLON <[email protected]>:
>> Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
>>
>> The PHY address is not specified here because atmel have 2 different
>> designs
>> for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
>> (Embest design) and the other one is connection PHYAD0 to a pull up
>> resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
>> As a result, Ronetix design will have its PHY available at address 0x1 and
>> Embest design at 0x7.
>> Let the net PHY core automatically detect the PHY address by scanning the
>> MDIO bus.
> I though the compatible string was listed as a required property, but
> it is not. The 'reg' property however is listed as required, although
> the of_miodbus_register() works just fine without it, although that is
> a Linux-specific implementation detail.
Indeed, it's listed in the required property list of the DT binding doc,
but the code implement auto detection if reg is missing.
However this line [1] clearly shows that specifying the reg property is
the preferred way of doing things.
I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
sama5d3xcm-embest.dtsi) to avoid this dirty hack,
but then we would have 2 more dtb and the user would have to determine
which CPU module he owns to choose the appropriate dtb.
If at91, arm-soc and DT maintainers agree with this approach I can
definitely propose something.
>
>> Define board specific delays to apply to RGMII signals.
>>
>> Signed-off-by: Boris BREZILLON <[email protected]>
> Reviewed-by: Florian Fainelli <[email protected]>
Thanks for your review.
Best Regards,
Boris
[1] http://lxr.free-electrons.com/source/drivers/of/of_mdio.c#L187
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On 26/06/2014 22:01, Boris BREZILLON :
> Hi Florian,
>
> On 26/06/2014 20:15, Florian Fainelli wrote:
>> Hi Boris,
>>
>> 2014-06-26 3:13 GMT-07:00 Boris BREZILLON <[email protected]>:
>>> Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
>>>
>>> The PHY address is not specified here because atmel have 2 different
>>> designs
>>> for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
>>> (Embest design) and the other one is connection PHYAD0 to a pull up
>>> resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
>>> As a result, Ronetix design will have its PHY available at address 0x1 and
>>> Embest design at 0x7.
>>> Let the net PHY core automatically detect the PHY address by scanning the
>>> MDIO bus.
>> I though the compatible string was listed as a required property, but
>> it is not. The 'reg' property however is listed as required, although
>> the of_miodbus_register() works just fine without it, although that is
>> a Linux-specific implementation detail.
>
> Indeed, it's listed in the required property list of the DT binding doc,
> but the code implement auto detection if reg is missing.
> However this line [1] clearly shows that specifying the reg property is
> the preferred way of doing things.
>
> I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
> sama5d3xcm-embest.dtsi) to avoid this dirty hack,
> but then we would have 2 more dtb and the user would have to determine
> which CPU module he owns to choose the appropriate dtb.
> If at91, arm-soc and DT maintainers agree with this approach I can
> definitely propose something.
Yes Boris, I definitively prefer not to add another .dtsi file for this
series if we can avoid it.
So, I would push for this "reg-less" solution. If it is chosen, you can
add my:
Acked-by: Nicolas Ferre <[email protected]>
Thanks, bye.
>>> Define board specific delays to apply to RGMII signals.
>>>
>>> Signed-off-by: Boris BREZILLON <[email protected]>
>> Reviewed-by: Florian Fainelli <[email protected]>
>
> Thanks for your review.
>
> Best Regards,
>
> Boris
>
> [1] http://lxr.free-electrons.com/source/drivers/of/of_mdio.c#L187
>
--
Nicolas Ferre
From: Boris BREZILLON <[email protected]>
Date: Thu, 26 Jun 2014 12:13:33 +0200
> This patch removes a board specific hook for sama5d3xek boards from the
> sama5d3 generic DT board file.
>
> This hook (which register a phy fixup configuring board specific delays
> in the ksz9021 ethernet phy) is now replaced by the appropriate DT
> properties definitions in the sama5d3xcm.dtsi file.
>
> Changes since v1:
> - fix txc-skew-ps and rxc-skew-ps delays
> - remove phy address info to handle Ronetix and Embest HW designs
These patches do not apply cleanly to 'net' nor 'net-next', in fact
you did not even say which tree these changes are targetting.
Please respin these patches and explicitly say what tree you wish
them to be applied to.
Thanks.
Hello David,
On Tue, 01 Jul 2014 15:38:06 -0700 (PDT)
David Miller <[email protected]> wrote:
> From: Boris BREZILLON <[email protected]>
> Date: Thu, 26 Jun 2014 12:13:33 +0200
>
> > This patch removes a board specific hook for sama5d3xek boards from
> > the sama5d3 generic DT board file.
> >
> > This hook (which register a phy fixup configuring board specific
> > delays in the ksz9021 ethernet phy) is now replaced by the
> > appropriate DT properties definitions in the sama5d3xcm.dtsi file.
> >
> > Changes since v1:
> > - fix txc-skew-ps and rxc-skew-ps delays
> > - remove phy address info to handle Ronetix and Embest HW designs
>
> These patches do not apply cleanly to 'net' nor 'net-next', in fact
> you did not even say which tree these changes are targetting.
Actually, these patches were intended to be taken through at91 (and then
arm-soc) tree.
The reason I added netdev in Cc is because I wanted to get feedback on
the DT phy node definition (which I got from Florian).
I guess patch 2 does not apply cleanly because I based this work on top
of other changes which are not merged yet ([1]).
Nicolas, I'll take care to rebase this series on top of linus/master
branch (or whatever branch you want me to base it onto).
Thanks,
Boris
[1]http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265216.html
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hello Bo,
I know you're quite busy, but if you have some time could you test this
series on both CPU Modules (Embest and Ronetix): I only own the Embest
one.
Best Regards,
Boris
On Thu, 26 Jun 2014 12:13:33 +0200
Boris BREZILLON <[email protected]> wrote:
> Hello,
>
> This patch removes a board specific hook for sama5d3xek boards from the
> sama5d3 generic DT board file.
>
> This hook (which register a phy fixup configuring board specific delays
> in the ksz9021 ethernet phy) is now replaced by the appropriate DT
> properties definitions in the sama5d3xcm.dtsi file.
>
> Best Regards,
>
> Boris
>
> Changes since v1:
> - fix txc-skew-ps and rxc-skew-ps delays
> - remove phy address info to handle Ronetix and Embest HW designs
>
> Boris BREZILLON (2):
> ARM: at91/dt: describe rgmii ethernet phy connected to sama5d3xek
> boards
> ARM: at91: remove phy fixup for sama5d3xek boards
>
> arch/arm/boot/dts/sama5d3xcm.dtsi | 15 +++++++++++++++
> arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
> 2 files changed, 15 insertions(+), 22 deletions(-)
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi Boris,
On 07/10/2014 12:34 AM, Boris BREZILLON wrote:
> Hello Bo,
>
> I know you're quite busy, but if you have some time could you test this
> series on both CPU Modules (Embest and Ronetix): I only own the Embest
> one.
>
> Best Regards,
>
> Boris
>
> On Thu, 26 Jun 2014 12:13:33 +0200
> Boris BREZILLON <[email protected]> wrote:
>
>> Hello,
>>
>> This patch removes a board specific hook for sama5d3xek boards from the
>> sama5d3 generic DT board file.
>>
>> This hook (which register a phy fixup configuring board specific delays
>> in the ksz9021 ethernet phy) is now replaced by the appropriate DT
>> properties definitions in the sama5d3xcm.dtsi file.
>>
>> Best Regards,
>>
>> Boris
>>
>> Changes since v1:
>> - fix txc-skew-ps and rxc-skew-ps delays
>> - remove phy address info to handle Ronetix and Embest HW designs
>>
>> Boris BREZILLON (2):
>> ARM: at91/dt: describe rgmii ethernet phy connected to sama5d3xek
>> boards
>> ARM: at91: remove phy fixup for sama5d3xek boards
>>
>> arch/arm/boot/dts/sama5d3xcm.dtsi | 15 +++++++++++++++
>> arch/arm/mach-at91/board-dt-sama5.c | 22 ----------------------
>> 2 files changed, 15 insertions(+), 22 deletions(-)
>>
For this series, test OK on sama5d33ek (Ronetix) and sama5d34ek (Embest).
Tested-by: Bo Shen <[email protected]>
Best Regards,
Bo Shen
2014-07-10 2:07 GMT-07:00 Boris BREZILLON <[email protected]>:
> On Fri, 27 Jun 2014 09:52:56 +0200
> Nicolas Ferre <[email protected]> wrote:
>
>> On 26/06/2014 22:01, Boris BREZILLON :
>> > Hi Florian,
>> >
>> > On 26/06/2014 20:15, Florian Fainelli wrote:
>> >> Hi Boris,
>> >>
>> >> 2014-06-26 3:13 GMT-07:00 Boris BREZILLON <[email protected]>:
>> >>> Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
>> >>>
>> >>> The PHY address is not specified here because atmel have 2 different
>> >>> designs
>> >>> for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
>> >>> (Embest design) and the other one is connection PHYAD0 to a pull up
>> >>> resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
>> >>> As a result, Ronetix design will have its PHY available at address 0x1 and
>> >>> Embest design at 0x7.
>> >>> Let the net PHY core automatically detect the PHY address by scanning the
>> >>> MDIO bus.
>> >> I though the compatible string was listed as a required property, but
>> >> it is not. The 'reg' property however is listed as required, although
>> >> the of_miodbus_register() works just fine without it, although that is
>> >> a Linux-specific implementation detail.
>> >
>> > Indeed, it's listed in the required property list of the DT binding doc,
>> > but the code implement auto detection if reg is missing.
>> > However this line [1] clearly shows that specifying the reg property is
>> > the preferred way of doing things.
>> >
>> > I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
>> > sama5d3xcm-embest.dtsi) to avoid this dirty hack,
>> > but then we would have 2 more dtb and the user would have to determine
>> > which CPU module he owns to choose the appropriate dtb.
>> > If at91, arm-soc and DT maintainers agree with this approach I can
>> > definitely propose something.
>>
>> Yes Boris, I definitively prefer not to add another .dtsi file for this
>> series if we can avoid it.
>>
>
> Okay, now that I don't specify the reg property I have a bunch of
> noisy logs (which is exactly what the developer of of_mdio.c wanted in
> order to force people to specify the reg property).
>
> It seems to be a problem for atmel users (all these logs make them
> think there is something wrong with the net device).
>
> Apart from the dts/dtsi split solution, which Nicolas wants to avoid, I
> see two solutions here:
>
> 1) remove the logs (or use dev_dbg instead of dev_info) from of_mdio.c.
> But I'm pretty sure this solution won't be accepted :-).
I am fine with using dev_dbg() instead of dev_info() for that sort of
messages, provided that you state the rationale of this change
(spewing the log console with probing messages) and specify tha the
'reg' property is optional.
>
> 2) define 2 ethernet phys (one for each possible solution). I tested it
> and it works fine (only the available PHY is registered and there is no
> noisy logs anymore).
One advantage of that solution is that you'll get slightly faster boot
times since you won't have to auto-probe for the PHYs on the MDIO bus,
the time savings get bigger as you start using higher PHY addresses.
>
> Could net and DT maintainers help us choose the best solution (or
> propose a new one) ?
>
> Best Regards,
>
> Boris
>
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Florian
On Thu, 10 Jul 2014 08:35:15 -0700
Florian Fainelli <[email protected]> wrote:
> 2014-07-10 2:07 GMT-07:00 Boris BREZILLON <[email protected]>:
> > On Fri, 27 Jun 2014 09:52:56 +0200
> > Nicolas Ferre <[email protected]> wrote:
> >
> >> On 26/06/2014 22:01, Boris BREZILLON :
> >> > Hi Florian,
> >> >
> >> > On 26/06/2014 20:15, Florian Fainelli wrote:
> >> >> Hi Boris,
> >> >>
> >> >> 2014-06-26 3:13 GMT-07:00 Boris BREZILLON <[email protected]>:
> >> >>> Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
> >> >>>
> >> >>> The PHY address is not specified here because atmel have 2 different
> >> >>> designs
> >> >>> for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
> >> >>> (Embest design) and the other one is connection PHYAD0 to a pull up
> >> >>> resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
> >> >>> As a result, Ronetix design will have its PHY available at address 0x1 and
> >> >>> Embest design at 0x7.
> >> >>> Let the net PHY core automatically detect the PHY address by scanning the
> >> >>> MDIO bus.
> >> >> I though the compatible string was listed as a required property, but
> >> >> it is not. The 'reg' property however is listed as required, although
> >> >> the of_miodbus_register() works just fine without it, although that is
> >> >> a Linux-specific implementation detail.
> >> >
> >> > Indeed, it's listed in the required property list of the DT binding doc,
> >> > but the code implement auto detection if reg is missing.
> >> > However this line [1] clearly shows that specifying the reg property is
> >> > the preferred way of doing things.
> >> >
> >> > I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
> >> > sama5d3xcm-embest.dtsi) to avoid this dirty hack,
> >> > but then we would have 2 more dtb and the user would have to determine
> >> > which CPU module he owns to choose the appropriate dtb.
> >> > If at91, arm-soc and DT maintainers agree with this approach I can
> >> > definitely propose something.
> >>
> >> Yes Boris, I definitively prefer not to add another .dtsi file for this
> >> series if we can avoid it.
> >>
> >
> > Okay, now that I don't specify the reg property I have a bunch of
> > noisy logs (which is exactly what the developer of of_mdio.c wanted in
> > order to force people to specify the reg property).
> >
> > It seems to be a problem for atmel users (all these logs make them
> > think there is something wrong with the net device).
> >
> > Apart from the dts/dtsi split solution, which Nicolas wants to avoid, I
> > see two solutions here:
> >
> > 1) remove the logs (or use dev_dbg instead of dev_info) from of_mdio.c.
> > But I'm pretty sure this solution won't be accepted :-).
>
> I am fine with using dev_dbg() instead of dev_info() for that sort of
> messages, provided that you state the rationale of this change
> (spewing the log console with probing messages) and specify tha the
> 'reg' property is optional.
>
> >
> > 2) define 2 ethernet phys (one for each possible solution). I tested it
> > and it works fine (only the available PHY is registered and there is no
> > noisy logs anymore).
>
> One advantage of that solution is that you'll get slightly faster boot
> times since you won't have to auto-probe for the PHYs on the MDIO bus,
> the time savings get bigger as you start using higher PHY addresses.
Yes I prefer this solution too, but is it acceptable to define 2 phy
nodes even if only one is really available ?
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
2014-07-10 10:19 GMT-07:00 Boris BREZILLON <[email protected]>:
> On Thu, 10 Jul 2014 08:35:15 -0700
> Florian Fainelli <[email protected]> wrote:
>
>> 2014-07-10 2:07 GMT-07:00 Boris BREZILLON <[email protected]>:
>> > On Fri, 27 Jun 2014 09:52:56 +0200
>> > Nicolas Ferre <[email protected]> wrote:
>> >
>> >> On 26/06/2014 22:01, Boris BREZILLON :
>> >> > Hi Florian,
>> >> >
>> >> > On 26/06/2014 20:15, Florian Fainelli wrote:
>> >> >> Hi Boris,
>> >> >>
>> >> >> 2014-06-26 3:13 GMT-07:00 Boris BREZILLON <[email protected]>:
>> >> >>> Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
>> >> >>>
>> >> >>> The PHY address is not specified here because atmel have 2 different
>> >> >>> designs
>> >> >>> for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
>> >> >>> (Embest design) and the other one is connection PHYAD0 to a pull up
>> >> >>> resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
>> >> >>> As a result, Ronetix design will have its PHY available at address 0x1 and
>> >> >>> Embest design at 0x7.
>> >> >>> Let the net PHY core automatically detect the PHY address by scanning the
>> >> >>> MDIO bus.
>> >> >> I though the compatible string was listed as a required property, but
>> >> >> it is not. The 'reg' property however is listed as required, although
>> >> >> the of_miodbus_register() works just fine without it, although that is
>> >> >> a Linux-specific implementation detail.
>> >> >
>> >> > Indeed, it's listed in the required property list of the DT binding doc,
>> >> > but the code implement auto detection if reg is missing.
>> >> > However this line [1] clearly shows that specifying the reg property is
>> >> > the preferred way of doing things.
>> >> >
>> >> > I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
>> >> > sama5d3xcm-embest.dtsi) to avoid this dirty hack,
>> >> > but then we would have 2 more dtb and the user would have to determine
>> >> > which CPU module he owns to choose the appropriate dtb.
>> >> > If at91, arm-soc and DT maintainers agree with this approach I can
>> >> > definitely propose something.
>> >>
>> >> Yes Boris, I definitively prefer not to add another .dtsi file for this
>> >> series if we can avoid it.
>> >>
>> >
>> > Okay, now that I don't specify the reg property I have a bunch of
>> > noisy logs (which is exactly what the developer of of_mdio.c wanted in
>> > order to force people to specify the reg property).
>> >
>> > It seems to be a problem for atmel users (all these logs make them
>> > think there is something wrong with the net device).
>> >
>> > Apart from the dts/dtsi split solution, which Nicolas wants to avoid, I
>> > see two solutions here:
>> >
>> > 1) remove the logs (or use dev_dbg instead of dev_info) from of_mdio.c.
>> > But I'm pretty sure this solution won't be accepted :-).
>>
>> I am fine with using dev_dbg() instead of dev_info() for that sort of
>> messages, provided that you state the rationale of this change
>> (spewing the log console with probing messages) and specify tha the
>> 'reg' property is optional.
>>
>> >
>> > 2) define 2 ethernet phys (one for each possible solution). I tested it
>> > and it works fine (only the available PHY is registered and there is no
>> > noisy logs anymore).
>>
>> One advantage of that solution is that you'll get slightly faster boot
>> times since you won't have to auto-probe for the PHYs on the MDIO bus,
>> the time savings get bigger as you start using higher PHY addresses.
>
> Yes I prefer this solution too, but is it acceptable to define 2 phy
> nodes even if only one is really available ?
One or the other machine .dts should have to set the status property
accordingly so there is only effectively one PHY declared, assuming
this is possible based on your .dtsi layout? If not, provided by the
phy-handle properties are correct, I can't see any problem with having
an unused PHY specified in DT.
--
Florian
On Thu, 10 Jul 2014 10:46:21 -0700
Florian Fainelli <[email protected]> wrote:
> 2014-07-10 10:19 GMT-07:00 Boris BREZILLON <[email protected]>:
> > On Thu, 10 Jul 2014 08:35:15 -0700
> > Florian Fainelli <[email protected]> wrote:
> >
> >> 2014-07-10 2:07 GMT-07:00 Boris BREZILLON <[email protected]>:
> >> > On Fri, 27 Jun 2014 09:52:56 +0200
> >> > Nicolas Ferre <[email protected]> wrote:
> >> >
> >> >> On 26/06/2014 22:01, Boris BREZILLON :
> >> >> > Hi Florian,
> >> >> >
> >> >> > On 26/06/2014 20:15, Florian Fainelli wrote:
> >> >> >> Hi Boris,
> >> >> >>
> >> >> >> 2014-06-26 3:13 GMT-07:00 Boris BREZILLON <[email protected]>:
> >> >> >>> Add ethernet-phy node and specify phy interrupt (connected to pin PB25).
> >> >> >>>
> >> >> >>> The PHY address is not specified here because atmel have 2 different
> >> >> >>> designs
> >> >> >>> for its CPU modules: one is connecting PHYAD[0-2] pins to pull up resistors
> >> >> >>> (Embest design) and the other one is connection PHYAD0 to a pull up
> >> >> >>> resistor and PHYAD[1-2] to pull down resistors (Ronetix design).
> >> >> >>> As a result, Ronetix design will have its PHY available at address 0x1 and
> >> >> >>> Embest design at 0x7.
> >> >> >>> Let the net PHY core automatically detect the PHY address by scanning the
> >> >> >>> MDIO bus.
> >> >> >> I though the compatible string was listed as a required property, but
> >> >> >> it is not. The 'reg' property however is listed as required, although
> >> >> >> the of_miodbus_register() works just fine without it, although that is
> >> >> >> a Linux-specific implementation detail.
> >> >> >
> >> >> > Indeed, it's listed in the required property list of the DT binding doc,
> >> >> > but the code implement auto detection if reg is missing.
> >> >> > However this line [1] clearly shows that specifying the reg property is
> >> >> > the preferred way of doing things.
> >> >> >
> >> >> > I could define 2 different sama5d3xcm.dtsi (sama5d3xcm-ronetix.dtsi and
> >> >> > sama5d3xcm-embest.dtsi) to avoid this dirty hack,
> >> >> > but then we would have 2 more dtb and the user would have to determine
> >> >> > which CPU module he owns to choose the appropriate dtb.
> >> >> > If at91, arm-soc and DT maintainers agree with this approach I can
> >> >> > definitely propose something.
> >> >>
> >> >> Yes Boris, I definitively prefer not to add another .dtsi file for this
> >> >> series if we can avoid it.
> >> >>
> >> >
> >> > Okay, now that I don't specify the reg property I have a bunch of
> >> > noisy logs (which is exactly what the developer of of_mdio.c wanted in
> >> > order to force people to specify the reg property).
> >> >
> >> > It seems to be a problem for atmel users (all these logs make them
> >> > think there is something wrong with the net device).
> >> >
> >> > Apart from the dts/dtsi split solution, which Nicolas wants to avoid, I
> >> > see two solutions here:
> >> >
> >> > 1) remove the logs (or use dev_dbg instead of dev_info) from of_mdio.c.
> >> > But I'm pretty sure this solution won't be accepted :-).
> >>
> >> I am fine with using dev_dbg() instead of dev_info() for that sort of
> >> messages, provided that you state the rationale of this change
> >> (spewing the log console with probing messages) and specify tha the
> >> 'reg' property is optional.
> >>
> >> >
> >> > 2) define 2 ethernet phys (one for each possible solution). I tested it
> >> > and it works fine (only the available PHY is registered and there is no
> >> > noisy logs anymore).
> >>
> >> One advantage of that solution is that you'll get slightly faster boot
> >> times since you won't have to auto-probe for the PHYs on the MDIO bus,
> >> the time savings get bigger as you start using higher PHY addresses.
> >
> > Yes I prefer this solution too, but is it acceptable to define 2 phy
> > nodes even if only one is really available ?
>
> One or the other machine .dts should have to set the status property
> accordingly so there is only effectively one PHY declared, assuming
> this is possible based on your .dtsi layout?
This is exactly what Nicolas wants to avoid (duplication of dts files
to handle Embest and Ronetix designs).
> If not, provided by the
> phy-handle properties are correct, I can't see any problem with having
> an unused PHY specified in DT.
Okay, then I'll go for that solution.
Thanks,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com