I tested the new code on my orange pi one (H3) and it works for the most
part.  The communications works but the yellow LED that indicates line speed
(I think) does not light up.  I compared the code to my working version and
put back the following, which makes the LED work.

# diff -u if_dwxe_new.c  if_dwxe.c
--- if_dwxe_new.c       Tue Dec 12 10:53:42 2017
+++ if_dwxe.c   Tue Dec 12 10:57:04 2017
@@ -475,9 +475,8 @@
                        syscon |= SYSCON_H3_EPHY_LED_POL;
                else
                        syscon &= ~SYSCON_H3_EPHY_LED_POL;
-               syscon &= ~SYSCON_H3_EPHY_ADDR_MASK;
+               syscon &= ~(SYSCON_H3_EPHY_ADDR_MASK <<
SYSCON_H3_EPHY_ADDR_SHIFT);
                syscon |= (sc->sc_phyloc << SYSCON_H3_EPHY_ADDR_SHIFT);
-               return;
        }
        free(phy_mode, M_TEMP, len);


But let's not get carried away with assigning credits.  Patrick Wildt
provided the changes to make it work on the H3.  I tested and did find some
problems, particularly the one about noisolate.  Even then Jonathan Gray
showed me how to correct it properly.

-----Original Message-----
From: owner-...@openbsd.org [mailto:owner-...@openbsd.org] On Behalf Of Mark
Kettenis
Sent: Sunday, December 10, 2017 4:30 AM
To: s_g...@telus.net
Cc: j...@jsg.id.au; mark.kette...@xs4all.nl; arm@openbsd.org
Subject: Re: FW: help with setting up dwxe on orange pi one (h3)

> From: "Stephen Graf" <s_g...@telus.net>
> Date: Thu, 30 Nov 2017 17:00:28 -0800
> 
> I tested with the new official dtb and the following diff on the Nov 
> 24th snapshot and it works fine.
> 
> I have been using this code on my orange pi one (Allwinner h3) for a 
> couple of weeks now without any problems. The new dtb seems to have 
> only required a name change for the clock and reset, back to what was in
the original code.
> (It seems there is a bit of formatting that caused the diff to pick up 
> the name lines that did not change.)

I've committed this now.  I've made some stylistic fixes and verified that
it still works on my Orange Pi PC2 (H5) with external PHY.

Probably good to verify that my changes didn't break things ;).

Thanks,

Mark

> # diff -u if_dwxe.c_orig if_dwxe.c
> --- if_dwxe.c_orig      Tue Nov 28 13:04:10 2017
> +++ if_dwxe.c   Thu Nov 30 16:19:25 2017
> @@ -146,11 +146,11 @@
>  #define  DWXE_MDIO_CMD_PHY_REG_SHIFT           4
>  #define  DWXE_MDIO_CMD_PHY_ADDR_SHIFT          12
>  #define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_SHIFT   20
> -#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_MASK    (0x7 << 20)
> -#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_16      (0 << 20)
> -#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_32      (1 << 20)
> -#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_64      (2 << 20)
> -#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_128     (3 << 20)
> +#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_MASK    0x7
> +#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_16      0
> +#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_32      1
> +#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_64      2
> +#define  DWXE_MDIO_CMD_MDC_DIV_RATIO_M_128     3
>  #define DWXE_MDIO_DATA         0x4C
>  #define DWXE_MACADDR_HI                0x50
>  #define DWXE_MACADDR_LO                0x54
> @@ -236,6 +236,7 @@
>  #define SYSCON_H3_EPHY_LED_POL         (1 << 17) /* 1: active low, 0:
> active high */
>  #define SYSCON_H3_EPHY_CLK_SEL         (1 << 18) /* 1: 24MHz, 0: 25MHz */
>  #define SYSCON_H3_EPHY_ADDR_SHIFT      20
> +#define SYSCON_H3_EPHY_ADDR_MASK        0x1f
> 
>  struct dwxe_buf {
>         bus_dmamap_t    tb_map;
> @@ -270,6 +271,7 @@
>         struct mii_data         sc_mii;
>  #define sc_media       sc_mii.mii_media
>         int                     sc_link;
> +       int                     sc_phyloc;
> 
>         struct dwxe_dmamem      *sc_txring;
>         struct dwxe_buf         *sc_txbuf;
> @@ -357,7 +359,6 @@
>         struct fdt_attach_args *faa = aux;
>         struct ifnet *ifp;
>         int phy, phy_supply, node;
> -       int phyloc = MII_PHY_ANY;
> 
>         sc->sc_node = faa->fa_node;
>         sc->sc_iot = faa->fa_iot;
> @@ -372,13 +373,12 @@
>         phy = OF_getpropint(faa->fa_node, "phy-handle", 0);
>         node = OF_getnodebyphandle(phy);
>         if (node)
> -               phyloc = OF_getpropint(node, "reg", phyloc);
> -
> +                sc->sc_phyloc = OF_getpropint(node, "reg", 
> + MII_PHY_ANY);
>         pinctrl_byname(faa->fa_node, "default");
> 
>         /* Enable clock. */
> -       clock_enable(faa->fa_node, "stmmaceth");
> -       reset_deassert(faa->fa_node, "stmmaceth");
> +        clock_enable(faa->fa_node, "stmmaceth");
> +        reset_deassert(faa->fa_node, "stmmaceth");
>         delay(5000);
> 
>         /* Power up PHY. */
> @@ -386,7 +386,7 @@
>         if (phy_supply)
>                 regulator_enable(phy_supply);
> 
> -       sc->sc_clk = clock_get_frequency(faa->fa_node, "stmmaceth");
> +        sc->sc_clk = clock_get_frequency(faa->fa_node, "stmmaceth");
>         if (sc->sc_clk > 160000000)
>                 sc->sc_clk = DWXE_MDIO_CMD_MDC_DIV_RATIO_M_128;
>         else if (sc->sc_clk > 80000000) @@ -424,8 +424,8 @@
> 
>         dwxe_reset(sc);
> 
> -       mii_attach(self, &sc->sc_mii, 0xffffffff, phyloc,
> -           MII_OFFSET_ANY, 0);
> +        mii_attach(self, &sc->sc_mii, 0xffffffff, sc->sc_phyloc,
> +           MII_OFFSET_ANY, MIIF_NOISOLATE);
>         if (LIST_FIRST(&sc->sc_mii.mii_phys) == NULL) {
>                 printf("%s: no PHY found!\n", sc->sc_dev.dv_xname);
>                 ifmedia_add(&sc->sc_media, IFM_ETHER|IFM_MANUAL, 0, 
> NULL); @@ -466,8 +466,14 @@
>                 syscon |= SYSCON_EPIT | SYSCON_ETCS_EXT_GMII;
>         else if (!strncmp(phy_mode, "mii", strlen("mii")) &&
>             OF_is_compatible(sc->sc_node, "allwinner,sun8i-h3-emac")) {
> -               panic("%s: setup internal phy", DEVNAME(sc));
> -               return;
> +                syscon &= ~SYSCON_H3_EPHY_SHUTDOWN;
> +                syscon |= SYSCON_H3_EPHY_SELECT|SYSCON_H3_EPHY_CLK_SEL;
> +                if (OF_getproplen(sc->sc_node, 
> + "allwinner,leds-active-low")
> >= 0)
> +                        syscon |= SYSCON_H3_EPHY_LED_POL;
> +                else
> +               syscon &= ~SYSCON_H3_EPHY_LED_POL;
> +               syscon &= ~(SYSCON_H3_EPHY_ADDR_MASK <<
> SYSCON_H3_EPHY_ADDR_SHIFT);
> +               syscon |= sc->sc_phyloc << SYSCON_H3_EPHY_ADDR_SHIFT;
>         }
>         free(phy_mode, M_TEMP, len);
> 
> -----Original Message-----
> From: owner-...@openbsd.org [mailto:owner-...@openbsd.org] On Behalf 
> Of Jonathan Gray
> Sent: Monday, November 27, 2017 12:46 AM
> To: Mark Kettenis <mark.kette...@xs4all.nl>
> Cc: arm@openbsd.org
> Subject: Re: FW: help with setting up dwxe on orange pi one (h3)
> 
> On Thu, Sep 28, 2017 at 04:09:36PM +0200, Mark Kettenis wrote:
> > Jonathan Gray schreef op 2017-09-28 09:51:
> > > On Thu, Sep 28, 2017 at 09:34:49AM +0200, Mark Kettenis wrote:
> > > > Jonathan Gray schreef op 2017-09-28 05:02:
> > > > > On Wed, Sep 27, 2017 at 10:56:44PM +0200, Patrick Wildt wrote:
> > > > > > So, first of all copying the dtb entries is not a good idea.  
> > > > > > The reason is that the phandles are gonna be all wrong and 
> > > > > > overriden, because those are _generated_ on compile time.  
> > > > > > As you can see, the ethernet controller references phy 
> > > > > > handle 0x7, but the phy has a phandle of 0x47.
> > > > > > Something is wrong there.
> > > > > >
> > > > > > You should try to apply this "revert" that was committed in
linux.
> > > > > > Hope
> > > > > > they'll soon make up their minds and commit a newer version.
> > > > >
> > > > > The device tree that is included with 
> > > > > /usr/local/share/u-boot/orangepi_one/u-boot-sunxi-with-spl.bin
> > > > > should still have emac nodes.
> > > > 
> > > > But they use a different binding from the Linux kernel, which 
> > > > are the ones Patrick used for the implementation.  And some of 
> > > > the regulator stuff is missing so things don't actually work 
> > > > even if we would change the driver to use the bindings currently 
> > > > used by U-Boot.  It's a mess.
> > > > 
> > > > So using the dtb from Linux with the revert applied is the way 
> > > > to go for now.
> > > > 
> > > 
> > > So we patch the dtb port to revert the revert commits for now?
> > > Doesn't appear to have been added back in 3.14 rcs yet.
> > 
> > We could do that.
> 
> The port now builds 4.15-rc1 which has EMAC for A64/H5/H3 added back.
> The mdio child nodes change but it looks like it should already be 
> compatible with what is in the tree as that is not used.
> 
> 
> 


Reply via email to