mbox series

[0/5] arm64: dts: meson: g12a: add network support

Message ID 20190510164940.13496-1-jbrunet@baylibre.com
Headers show
Series arm64: dts: meson: g12a: add network support | expand

Message

Jerome Brunet May 10, 2019, 4:49 p.m. UTC
Add network support for the g12a

Jerome Brunet (5):
  arm64: dts: meson: g12a: add ethernet mac controller
  arm64: dts: meson: g12a: add ethernet pinctrl definitions
  arm64: dts: meson: g12a: add mdio multiplexer
  arm64: dts: meson: u200: add internal network
  arm64: dts: meson: sei510: add network support

 .../boot/dts/amlogic/meson-g12a-sei510.dts    |  7 ++
 .../boot/dts/amlogic/meson-g12a-u200.dts      |  7 ++
 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi   | 90 +++++++++++++++++++
 3 files changed, 104 insertions(+)

-- 
2.20.1

Comments

Jerome Brunet May 10, 2019, 4:52 p.m. UTC | #1
On Fri, 2019-05-10 at 18:49 +0200, Jerome Brunet wrote:
> Add network support for the g12a


I forgot to mention that series applies on top of the node
re-order series [0]

[0]: https://lkml.kernel.org/r/20190510155327.5759-1-jbrunet@baylibre.com

> 

> Jerome Brunet (5):

>   arm64: dts: meson: g12a: add ethernet mac controller

>   arm64: dts: meson: g12a: add ethernet pinctrl definitions

>   arm64: dts: meson: g12a: add mdio multiplexer

>   arm64: dts: meson: u200: add internal network

>   arm64: dts: meson: sei510: add network support

> 

>  .../boot/dts/amlogic/meson-g12a-sei510.dts    |  7 ++

>  .../boot/dts/amlogic/meson-g12a-u200.dts      |  7 ++

>  arch/arm64/boot/dts/amlogic/meson-g12a.dtsi   | 90 +++++++++++++++++++

>  3 files changed, 104 insertions(+)

>
Kevin Hilman May 10, 2019, 10:45 p.m. UTC | #2
Jerome Brunet <jbrunet@baylibre.com> writes:

> Enable the network interface of the SEI510 which use the internal PHY.

>

> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>


I tried testing this series on SEI510, but I must still be missing some
defconfig options, as the default defconfig doesn't lead to a working
interface.


I tried adding this kconfig fragment[1], and the dwmac probes/inits but
I must still be missing something, as the dwmac is still failing to find
a PHY.  Boot log: https://termbin.com/ivf3

I have the same result testing on the u200.

Kevin

[1] amlogic network kconfig fragment
CONFIG_STMMAC_ETH=y

# following are needed, but automatically enabled if above is set
#CONFIG_STMMAC_PLATFORM=m
#CONFIG_DWMAC_MESON=m

CONFIG_PHYLIB=y
CONFIG_MICREL_PHY=y
CONFIG_REALTEK_PHY=y

CONFIG_MDIO_BUS_MUX_MESON_G12A=y
CONFIG_MESON_GXL_PHY=y
Jerome Brunet May 11, 2019, 5:27 p.m. UTC | #3
On Sat, 2019-05-11 at 19:06 +0200, Martin Blumenstingl wrote:
> Hi Jerome,

> 

> On Fri, May 10, 2019 at 6:49 PM Jerome Brunet <jbrunet@baylibre.com> wrote:

> > Add the ethernet pinctrl settings for RMII, RGMII and internal phy leds

> > 

> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

> > ---

> >  arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 37 +++++++++++++++++++++

> >  1 file changed, 37 insertions(+)

> > 

> > diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi

> > index a32db09809f7..fe0f73730525 100644

> > --- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi

> > +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi

> > @@ -206,6 +206,43 @@

> >                                                 };

> >                                         };

> > 

> > +                                       eth_leds_pins: eth-leds {

> > +                                               mux {

> > +                                                       groups = "eth_link_led",

> > +                                                                "eth_act_led";

> > +                                                       function = "eth";

> > +                                                       bias-disable;

> > +                                               };

> > +                                       };

> > +

> > +                                       eth_rmii_pins: eth-rmii {

> > +                                               mux {

> > +                                                       groups = "eth_mdio",

> > +                                                                "eth_mdc",

> > +                                                                "eth_rgmii_rx_clk",

> > +                                                                "eth_rx_dv",

> > +                                                                "eth_rxd0",

> > +                                                                "eth_rxd1",

> > +                                                                "eth_txen",

> > +                                                                "eth_txd0",

> > +                                                                "eth_txd1";

> > +                                                       function = "eth";

> > +                                                       bias-disable;

> > +                                               };

> > +                                       };

> > +

> > +                                       eth_rgmii_pins: eth-rgmii {

> > +                                               mux {

> > +                                                       groups = "eth_rxd2_rgmii",

> > +                                                                "eth_rxd3_rgmii",

> > +                                                                "eth_rgmii_tx_clk",

> > +                                                                "eth_txd2_rgmii",

> > +                                                                "eth_txd3_rgmii";

> > +                                                       function = "eth";

> > +                                                       bias-disable;

> > +                                               };

> > +                                       };

> it seems that the group definition is incomplete (missing things like

> eth_mdc, eth_rx_dv, ...)

> 

> we could also mix the eth_rmii_pins and eth_rgmii_pins in a board.dts

> (maybe that was your idea in the first place?):


yes that's the idea

>   phy-mode = "rgmii";

>   pinctrl-0 = <&eth_rmii_pins>, <&eth_rgmii_pins>;

>   pinctrl-names = "default";

> however, in this case I would prefer if "eth_rmii_pins" was named only

> "eth_pins" (following mostly what Amlogic does with the pin group

> naming: eth_* for pins that are valid in both, rmii and rgmii mode and

> eth*rgmii* for pins that are only valid in rgmii mode)


I can't say I share your preference. I let Kevin decide what he wants.

> 

> 

> Regards

> Martin