Re: fill out more rk356x dwqe phy-mode handling
> Date: Wed, 05 Apr 2023 09:09:22 +0200
> From: Mark Kettenis
>
> > Date: Wed, 5 Apr 2023 16:09:21 +1000
> > From: David Gwynne
> >
> > On Tue, Apr 04, 2023 at 10:59:56PM +1000, David Gwynne wrote:
> > >
> > >
> > > > On 4 Apr 2023, at 20:37, Mark Kettenis wrote:
> > > >
> > > >> Date: Tue, 4 Apr 2023 09:49:40 +1000
> > > >> From: David Gwynne
> > > >>
> > > >> i did this when i was trying to figure out why TX wasn't working on the
> > > >> nanopi r5s before figuring out that problem was because we didn't have
> > > >> rkiovd.
> > > >>
> > > >> at the very least it should future proof dwqe against more phy setups,
> > > >> and provides a decent example of how to interpret those fdt properties.
> > > >>
> > > >> ok?
> > > >
> > > > Oh wow. I always thought those properties are purely about enabling
> > > > the internal delays of the PHY and came in addition to the delays
> > > > configured in the MAC. But you're right, that isn't what the device
> > > > tree bindings say and isn't what the Linux driver implements. So this
> > > > is indeed the right thing to do. And it will make the rock-3a work
> > > > once I correctly implement configuring the internal delay for
> > > > rgephy(4).
> > > >
> > > > That said, I think that once again you are doing too much DT checking.
> > > > Many of the options you're now erroring out on are optional. And
> > > > there actually is a reasonable chance that the interface will work if
> > > > they're absent, especially if the firmware already initializes the
> > > > SoC-specific GRF registers. I don't think we should even print any
> > > > warnings.
> > >
> > > having stared inside some boot loaders recently i have mixed feelings
> > > about this. i guess they can only get better from here though, right?
> > >
> > > > If you make dwqe_rockchip_setup() specific to the rk3568 (no objection
> > > > to that), you should probably rename it to dwqe_rk3568_setup().
> > >
> > > alright.
> > >
> > > > And a style issue; please don't add parentheses around trivial
> > > > function return values in code that doesn't already use that style.
> > > > We moved away from that style years ago...
> > >
> > > yeah, sorry. muscle memory.
> > >
> > > i'll fix these up tomorrow and send it around again.
> >
> > this still works for me on the r5s. e25 doesnt use onboard gmac.
>
> Just realized that this risks leaving the task uninitialized. But
> that risk is already there.
>
> ok kettenis@
BTW, I have an almost complete diff to wire up the MIIF_RXID/MIIF_TXID
flags stuff in dwqe(4) that I'll finish once this diff is in.
> > Index: if_dwqe_fdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 if_dwqe_fdt.c
> > --- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5
> > +++ if_dwqe_fdt.c 5 Apr 2023 06:06:57 -
> > @@ -63,7 +63,7 @@
> >
> > intdwqe_fdt_match(struct device *, void *, void *);
> > void dwqe_fdt_attach(struct device *, struct device *, void *);
> > -void dwqe_setup_rockchip(struct dwqe_softc *);
> > +void dwqe_setup_rk3568(struct dwqe_softc *);
> > void dwqe_mii_statchg_rk3568(struct device *);
> > void dwqe_mii_statchg_rk3588(struct device *);
> >
> > @@ -138,7 +138,7 @@ dwqe_fdt_attach(struct device *parent, s
> >
> > /* Do hardware specific initializations. */
> > if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac"))
> > - dwqe_setup_rockchip(sc);
> > + dwqe_setup_rk3568(sc);
> >
> > /* Power up PHY. */
> > phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0);
> > @@ -266,41 +266,62 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui
> > #define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8)
> > #define RK3568_GMAC_PHY_INTF_SEL_RGMII((0x7 << 4) << 16 |
> > (0x1 << 4))
> > #define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 |
> > (0x4 << 4))
> > -#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0))
> > -#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1))
> > +#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v)
> > << 0))
> > +#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v)
> > << 1))
> >
> > void dwqe_mii_statchg_rk3568_task(void *);
> >
> > void
> > -dwqe_setup_rockchip(struct dwqe_softc *sc)
> > +dwqe_setup_rk3568(struct dwqe_softc *sc)
> > {
> > + char phy_mode[32];
> > struct regmap *rm;
> > uint32_t grf;
> > int tx_delay, rx_delay;
> > + uint32_t iface;
> >
> > grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0);
> > rm = regmap_byphandle(grf);
> > if (rm == NULL)
> > return;
> >
> > + if (OF_getprop(sc->sc_node, "phy-mode",
> > + phy_mode, sizeof(phy_mode)) <= 0)
> > + return;
> > +
> > tx_delay = OF_getpropint(sc->sc_node,
Re: fill out more rk356x dwqe phy-mode handling
> Date: Wed, 5 Apr 2023 16:09:21 +1000
> From: David Gwynne
>
> On Tue, Apr 04, 2023 at 10:59:56PM +1000, David Gwynne wrote:
> >
> >
> > > On 4 Apr 2023, at 20:37, Mark Kettenis wrote:
> > >
> > >> Date: Tue, 4 Apr 2023 09:49:40 +1000
> > >> From: David Gwynne
> > >>
> > >> i did this when i was trying to figure out why TX wasn't working on the
> > >> nanopi r5s before figuring out that problem was because we didn't have
> > >> rkiovd.
> > >>
> > >> at the very least it should future proof dwqe against more phy setups,
> > >> and provides a decent example of how to interpret those fdt properties.
> > >>
> > >> ok?
> > >
> > > Oh wow. I always thought those properties are purely about enabling
> > > the internal delays of the PHY and came in addition to the delays
> > > configured in the MAC. But you're right, that isn't what the device
> > > tree bindings say and isn't what the Linux driver implements. So this
> > > is indeed the right thing to do. And it will make the rock-3a work
> > > once I correctly implement configuring the internal delay for
> > > rgephy(4).
> > >
> > > That said, I think that once again you are doing too much DT checking.
> > > Many of the options you're now erroring out on are optional. And
> > > there actually is a reasonable chance that the interface will work if
> > > they're absent, especially if the firmware already initializes the
> > > SoC-specific GRF registers. I don't think we should even print any
> > > warnings.
> >
> > having stared inside some boot loaders recently i have mixed feelings about
> > this. i guess they can only get better from here though, right?
> >
> > > If you make dwqe_rockchip_setup() specific to the rk3568 (no objection
> > > to that), you should probably rename it to dwqe_rk3568_setup().
> >
> > alright.
> >
> > > And a style issue; please don't add parentheses around trivial
> > > function return values in code that doesn't already use that style.
> > > We moved away from that style years ago...
> >
> > yeah, sorry. muscle memory.
> >
> > i'll fix these up tomorrow and send it around again.
>
> this still works for me on the r5s. e25 doesnt use onboard gmac.
Just realized that this risks leaving the task uninitialized. But
that risk is already there.
ok kettenis@
> Index: if_dwqe_fdt.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 if_dwqe_fdt.c
> --- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5
> +++ if_dwqe_fdt.c 5 Apr 2023 06:06:57 -
> @@ -63,7 +63,7 @@
>
> int dwqe_fdt_match(struct device *, void *, void *);
> void dwqe_fdt_attach(struct device *, struct device *, void *);
> -void dwqe_setup_rockchip(struct dwqe_softc *);
> +void dwqe_setup_rk3568(struct dwqe_softc *);
> void dwqe_mii_statchg_rk3568(struct device *);
> void dwqe_mii_statchg_rk3588(struct device *);
>
> @@ -138,7 +138,7 @@ dwqe_fdt_attach(struct device *parent, s
>
> /* Do hardware specific initializations. */
> if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac"))
> - dwqe_setup_rockchip(sc);
> + dwqe_setup_rk3568(sc);
>
> /* Power up PHY. */
> phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0);
> @@ -266,41 +266,62 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui
> #define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8)
> #define RK3568_GMAC_PHY_INTF_SEL_RGMII ((0x7 << 4) << 16 |
> (0x1 << 4))
> #define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 |
> (0x4 << 4))
> -#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0))
> -#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1))
> +#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v)
> << 0))
> +#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v)
> << 1))
>
> void dwqe_mii_statchg_rk3568_task(void *);
>
> void
> -dwqe_setup_rockchip(struct dwqe_softc *sc)
> +dwqe_setup_rk3568(struct dwqe_softc *sc)
> {
> + char phy_mode[32];
> struct regmap *rm;
> uint32_t grf;
> int tx_delay, rx_delay;
> + uint32_t iface;
>
> grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0);
> rm = regmap_byphandle(grf);
> if (rm == NULL)
> return;
>
> + if (OF_getprop(sc->sc_node, "phy-mode",
> + phy_mode, sizeof(phy_mode)) <= 0)
> + return;
> +
> tx_delay = OF_getpropint(sc->sc_node, "tx_delay", 0x30);
> rx_delay = OF_getpropint(sc->sc_node, "rx_delay", 0x10);
>
> - if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-gmac")) {
> - /* Program clock delay lines. */
> - regmap_write_4(rm, RK3568_GRF_GMACx_CON0(sc->sc_gmac_id),
> - RK3568_GMAC_CLK_TX_DL_CFG(tx_delay) |
> - RK3568_GMAC_CLK_RX_DL_CFG(rx_delay));
> -
> - /* Use
Re: fill out more rk356x dwqe phy-mode handling
On Tue, Apr 04, 2023 at 10:59:56PM +1000, David Gwynne wrote:
>
>
> > On 4 Apr 2023, at 20:37, Mark Kettenis wrote:
> >
> >> Date: Tue, 4 Apr 2023 09:49:40 +1000
> >> From: David Gwynne
> >>
> >> i did this when i was trying to figure out why TX wasn't working on the
> >> nanopi r5s before figuring out that problem was because we didn't have
> >> rkiovd.
> >>
> >> at the very least it should future proof dwqe against more phy setups,
> >> and provides a decent example of how to interpret those fdt properties.
> >>
> >> ok?
> >
> > Oh wow. I always thought those properties are purely about enabling
> > the internal delays of the PHY and came in addition to the delays
> > configured in the MAC. But you're right, that isn't what the device
> > tree bindings say and isn't what the Linux driver implements. So this
> > is indeed the right thing to do. And it will make the rock-3a work
> > once I correctly implement configuring the internal delay for
> > rgephy(4).
> >
> > That said, I think that once again you are doing too much DT checking.
> > Many of the options you're now erroring out on are optional. And
> > there actually is a reasonable chance that the interface will work if
> > they're absent, especially if the firmware already initializes the
> > SoC-specific GRF registers. I don't think we should even print any
> > warnings.
>
> having stared inside some boot loaders recently i have mixed feelings about
> this. i guess they can only get better from here though, right?
>
> > If you make dwqe_rockchip_setup() specific to the rk3568 (no objection
> > to that), you should probably rename it to dwqe_rk3568_setup().
>
> alright.
>
> > And a style issue; please don't add parentheses around trivial
> > function return values in code that doesn't already use that style.
> > We moved away from that style years ago...
>
> yeah, sorry. muscle memory.
>
> i'll fix these up tomorrow and send it around again.
this still works for me on the r5s. e25 doesnt use onboard gmac.
Index: if_dwqe_fdt.c
===
RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v
retrieving revision 1.5
diff -u -p -r1.5 if_dwqe_fdt.c
--- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5
+++ if_dwqe_fdt.c 5 Apr 2023 06:06:57 -
@@ -63,7 +63,7 @@
intdwqe_fdt_match(struct device *, void *, void *);
void dwqe_fdt_attach(struct device *, struct device *, void *);
-void dwqe_setup_rockchip(struct dwqe_softc *);
+void dwqe_setup_rk3568(struct dwqe_softc *);
void dwqe_mii_statchg_rk3568(struct device *);
void dwqe_mii_statchg_rk3588(struct device *);
@@ -138,7 +138,7 @@ dwqe_fdt_attach(struct device *parent, s
/* Do hardware specific initializations. */
if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac"))
- dwqe_setup_rockchip(sc);
+ dwqe_setup_rk3568(sc);
/* Power up PHY. */
phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0);
@@ -266,41 +266,62 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui
#define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8)
#define RK3568_GMAC_PHY_INTF_SEL_RGMII((0x7 << 4) << 16 |
(0x1 << 4))
#define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 | (0x4 << 4))
-#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0))
-#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1))
+#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v) << 0))
+#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v) << 1))
void dwqe_mii_statchg_rk3568_task(void *);
void
-dwqe_setup_rockchip(struct dwqe_softc *sc)
+dwqe_setup_rk3568(struct dwqe_softc *sc)
{
+ char phy_mode[32];
struct regmap *rm;
uint32_t grf;
int tx_delay, rx_delay;
+ uint32_t iface;
grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0);
rm = regmap_byphandle(grf);
if (rm == NULL)
return;
+ if (OF_getprop(sc->sc_node, "phy-mode",
+ phy_mode, sizeof(phy_mode)) <= 0)
+ return;
+
tx_delay = OF_getpropint(sc->sc_node, "tx_delay", 0x30);
rx_delay = OF_getpropint(sc->sc_node, "rx_delay", 0x10);
- if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-gmac")) {
- /* Program clock delay lines. */
- regmap_write_4(rm, RK3568_GRF_GMACx_CON0(sc->sc_gmac_id),
- RK3568_GMAC_CLK_TX_DL_CFG(tx_delay) |
- RK3568_GMAC_CLK_RX_DL_CFG(rx_delay));
-
- /* Use RGMII interface and enable clock delay. */
- regmap_write_4(rm, RK3568_GRF_GMACx_CON1(sc->sc_gmac_id),
- RK3568_GMAC_PHY_INTF_SEL_RGMII |
- RK3568_GMAC_TXCLK_DLY_ENA |
- RK3568_GMAC_RXCLK_DLY_ENA);
+ if (strcmp(phy_mode, "rgmii") == 0) {
+ iface =
Re: fill out more rk356x dwqe phy-mode handling
> On 4 Apr 2023, at 20:37, Mark Kettenis wrote:
>
>> Date: Tue, 4 Apr 2023 09:49:40 +1000
>> From: David Gwynne
>>
>> i did this when i was trying to figure out why TX wasn't working on the
>> nanopi r5s before figuring out that problem was because we didn't have
>> rkiovd.
>>
>> at the very least it should future proof dwqe against more phy setups,
>> and provides a decent example of how to interpret those fdt properties.
>>
>> ok?
>
> Oh wow. I always thought those properties are purely about enabling
> the internal delays of the PHY and came in addition to the delays
> configured in the MAC. But you're right, that isn't what the device
> tree bindings say and isn't what the Linux driver implements. So this
> is indeed the right thing to do. And it will make the rock-3a work
> once I correctly implement configuring the internal delay for
> rgephy(4).
>
> That said, I think that once again you are doing too much DT checking.
> Many of the options you're now erroring out on are optional. And
> there actually is a reasonable chance that the interface will work if
> they're absent, especially if the firmware already initializes the
> SoC-specific GRF registers. I don't think we should even print any
> warnings.
having stared inside some boot loaders recently i have mixed feelings about
this. i guess they can only get better from here though, right?
> If you make dwqe_rockchip_setup() specific to the rk3568 (no objection
> to that), you should probably rename it to dwqe_rk3568_setup().
alright.
> And a style issue; please don't add parentheses around trivial
> function return values in code that doesn't already use that style.
> We moved away from that style years ago...
yeah, sorry. muscle memory.
i'll fix these up tomorrow and send it around again.
cheers,
dlg
>
>
>> Index: if_dwqe_fdt.c
>> ===
>> RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v
>> retrieving revision 1.5
>> diff -u -p -r1.5 if_dwqe_fdt.c
>> --- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5
>> +++ if_dwqe_fdt.c 3 Apr 2023 23:45:05 -
>> @@ -63,7 +63,7 @@
>>
>> int dwqe_fdt_match(struct device *, void *, void *);
>> void dwqe_fdt_attach(struct device *, struct device *, void *);
>> -void dwqe_setup_rockchip(struct dwqe_softc *);
>> +int dwqe_setup_rockchip(struct dwqe_softc *);
>> void dwqe_mii_statchg_rk3568(struct device *);
>> void dwqe_mii_statchg_rk3588(struct device *);
>>
>> @@ -137,8 +137,12 @@ dwqe_fdt_attach(struct device *parent, s
>> delay(5000);
>>
>> /* Do hardware specific initializations. */
>> - if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac"))
>> - dwqe_setup_rockchip(sc);
>> + if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac")) {
>> + if (dwqe_setup_rockchip(sc) == -1) {
>> + /* dwqe_setup_rockchip prints the error and \n */
>> + return;
>> + }
>> + }
>>
>> /* Power up PHY. */
>> phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0);
>> @@ -266,41 +270,70 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui
>> #define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8)
>> #define RK3568_GMAC_PHY_INTF_SEL_RGMII ((0x7 << 4) << 16 | (0x1 << 4))
>> #define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 | (0x4 << 4))
>> -#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0))
>> -#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1))
>> +#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v) << 0))
>> +#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v) << 1))
>>
>> void dwqe_mii_statchg_rk3568_task(void *);
>>
>> -void
>> +int
>> dwqe_setup_rockchip(struct dwqe_softc *sc)
>> {
>> + char phy_mode[32];
>> struct regmap *rm;
>> uint32_t grf;
>> int tx_delay, rx_delay;
>> + uint32_t iface;
>>
>> grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0);
>> rm = regmap_byphandle(grf);
>> - if (rm == NULL)
>> - return;
>> + if (rm == NULL) {
>> + printf(": can't find rockchip,grf\n");
>> + return (-1);
>> + }
>> +
>> + if (OF_getprop(sc->sc_node, "phy-mode",
>> +phy_mode, sizeof(phy_mode)) <= 0) {
>> + printf(": no phy-mode\n");
>> + return (-1);
>> + }
>>
>> tx_delay = OF_getpropint(sc->sc_node, "tx_delay", 0x30);
>> rx_delay = OF_getpropint(sc->sc_node, "rx_delay", 0x10);
>>
>> - if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-gmac")) {
>> - /* Program clock delay lines. */
>> - regmap_write_4(rm, RK3568_GRF_GMACx_CON0(sc->sc_gmac_id),
>> -RK3568_GMAC_CLK_TX_DL_CFG(tx_delay) |
>> -RK3568_GMAC_CLK_RX_DL_CFG(rx_delay));
>> -
>> - /* Use RGMII interface and enable clock delay. */
>> - regmap_write_4(rm, RK3568_GRF_GMACx_CON1(sc->sc_gmac_id),
>> -RK3568_GMAC_PHY_INTF_SEL_RGMII |
>> -RK3568_GMAC_TXCLK_DLY_ENA |
>> -RK3568_GMAC_RXCLK_DLY_ENA);
>> -
>> - task_set(>sc_statchg_task,
>> -dwqe_mii_statchg_rk3568_task, sc);
>> + if (strcmp(phy_mode, "rgmii") == 0) {
>> + iface = RK3568_GMAC_PHY_INTF_SEL_RGMII;
>> + } else if (strcmp(phy_mode, "rgmii-id") == 0) {
>> + iface =
Re: fill out more rk356x dwqe phy-mode handling
> Date: Tue, 4 Apr 2023 09:49:40 +1000
> From: David Gwynne
>
> i did this when i was trying to figure out why TX wasn't working on the
> nanopi r5s before figuring out that problem was because we didn't have
> rkiovd.
>
> at the very least it should future proof dwqe against more phy setups,
> and provides a decent example of how to interpret those fdt properties.
>
> ok?
Oh wow. I always thought those properties are purely about enabling
the internal delays of the PHY and came in addition to the delays
configured in the MAC. But you're right, that isn't what the device
tree bindings say and isn't what the Linux driver implements. So this
is indeed the right thing to do. And it will make the rock-3a work
once I correctly implement configuring the internal delay for
rgephy(4).
That said, I think that once again you are doing too much DT checking.
Many of the options you're now erroring out on are optional. And
there actually is a reasonable chance that the interface will work if
they're absent, especially if the firmware already initializes the
SoC-specific GRF registers. I don't think we should even print any
warnings.
If you make dwqe_rockchip_setup() specific to the rk3568 (no objection
to that), you should probably rename it to dwqe_rk3568_setup().
And a style issue; please don't add parentheses around trivial
function return values in code that doesn't already use that style.
We moved away from that style years ago...
> Index: if_dwqe_fdt.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 if_dwqe_fdt.c
> --- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5
> +++ if_dwqe_fdt.c 3 Apr 2023 23:45:05 -
> @@ -63,7 +63,7 @@
>
> int dwqe_fdt_match(struct device *, void *, void *);
> void dwqe_fdt_attach(struct device *, struct device *, void *);
> -void dwqe_setup_rockchip(struct dwqe_softc *);
> +int dwqe_setup_rockchip(struct dwqe_softc *);
> void dwqe_mii_statchg_rk3568(struct device *);
> void dwqe_mii_statchg_rk3588(struct device *);
>
> @@ -137,8 +137,12 @@ dwqe_fdt_attach(struct device *parent, s
> delay(5000);
>
> /* Do hardware specific initializations. */
> - if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac"))
> - dwqe_setup_rockchip(sc);
> + if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac")) {
> + if (dwqe_setup_rockchip(sc) == -1) {
> + /* dwqe_setup_rockchip prints the error and \n */
> + return;
> + }
> + }
>
> /* Power up PHY. */
> phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0);
> @@ -266,41 +270,70 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui
> #define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8)
> #define RK3568_GMAC_PHY_INTF_SEL_RGMII ((0x7 << 4) << 16 |
> (0x1 << 4))
> #define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 |
> (0x4 << 4))
> -#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0))
> -#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1))
> +#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v)
> << 0))
> +#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v)
> << 1))
>
> void dwqe_mii_statchg_rk3568_task(void *);
>
> -void
> +int
> dwqe_setup_rockchip(struct dwqe_softc *sc)
> {
> + char phy_mode[32];
> struct regmap *rm;
> uint32_t grf;
> int tx_delay, rx_delay;
> + uint32_t iface;
>
> grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0);
> rm = regmap_byphandle(grf);
> - if (rm == NULL)
> - return;
> + if (rm == NULL) {
> + printf(": can't find rockchip,grf\n");
> + return (-1);
> + }
> +
> + if (OF_getprop(sc->sc_node, "phy-mode",
> + phy_mode, sizeof(phy_mode)) <= 0) {
> + printf(": no phy-mode\n");
> + return (-1);
> + }
>
> tx_delay = OF_getpropint(sc->sc_node, "tx_delay", 0x30);
> rx_delay = OF_getpropint(sc->sc_node, "rx_delay", 0x10);
>
> - if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-gmac")) {
> - /* Program clock delay lines. */
> - regmap_write_4(rm, RK3568_GRF_GMACx_CON0(sc->sc_gmac_id),
> - RK3568_GMAC_CLK_TX_DL_CFG(tx_delay) |
> - RK3568_GMAC_CLK_RX_DL_CFG(rx_delay));
> -
> - /* Use RGMII interface and enable clock delay. */
> - regmap_write_4(rm, RK3568_GRF_GMACx_CON1(sc->sc_gmac_id),
> - RK3568_GMAC_PHY_INTF_SEL_RGMII |
> - RK3568_GMAC_TXCLK_DLY_ENA |
> - RK3568_GMAC_RXCLK_DLY_ENA);
> -
> - task_set(>sc_statchg_task,
> - dwqe_mii_statchg_rk3568_task, sc);
> + if (strcmp(phy_mode, "rgmii") == 0) {
> + iface =
