Skip to content

Conversation

JiafeiPan
Copy link
Contributor

  1. added new phy driver for Motorcomm YT8521
  2. enabled ENET ethernet port on FRDM_IMX93 board.

.read = mc_ytphy_read,
.write = mc_ytphy_write,
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already said it at the former pr, please look, if that is really needed. Also if it is just the init that is a bit different (I suspect that), please try to be as close to the generic ethernet driver, that is in tree as possible. It's easier to maintain the long therm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @maass-hamburg , updated the driver to be close with generic ethernet phy driver please review again, thanks.

Copy link
Member

@decsny decsny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you clarify what is the added value of the new phy driver over the generic one

}

#if ANY_FIXED_LINK
static int mc_ytphy_initialize_fixed_link(const struct device *dev)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in fixed link, we don't communicate or control the phy. Because of that we don't need it (and want it) at the other phy drivers, that are not generic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per my understanding, fixed-link should be a fixed speed configurated by dts node property, but it doesn't mean that PHY only supports this speed, so we need to configure PHY register BMCR to to let PHY hardware to work in the speed parsed from dts nodes, otherwise, which PHY hardware knows which speed will run.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the dt prop description is This link is fixed and does not require PHY configuration. It is a way to set a fixed speed, so the ethernet driver can configure itself, if needed, basically just a dummy phy. Please don't reuse this dt prop for other functionality.

Comment on lines 257 to 286
static int mc_ytphy_get_features(const struct device *dev)
{
struct mc_ytphy_data *data = dev->data;
uint32_t val = 0;
int ret;

ret = mc_ytphy_read(dev, MII_BMSR, &val);
if (ret) {
return ret;
}

BIT_SET(val & MII_BMSR_100BASE_X_FULL, data->phy_feature, LINK_FULL_100BASE);
BIT_SET(val & MII_BMSR_100BASE_X_HALF, data->phy_feature, LINK_HALF_100BASE);
BIT_SET(val & MII_BMSR_10_FULL, data->phy_feature, LINK_FULL_10BASE);
BIT_SET(val & MII_BMSR_10_HALF, data->phy_feature, LINK_HALF_10BASE);

if ((val & MII_BMSR_EXTEND_STATUS) == 0) {
return 0;
}

ret = mc_ytphy_read(dev, MII_ESTAT, &val);
if (ret) {
return ret;
}

BIT_SET(val & MII_ESTAT_1000BASE_T_FULL, data->phy_feature, LINK_FULL_1000BASE);
BIT_SET(val & MII_ESTAT_1000BASE_T_HALF, data->phy_feature, LINK_HALF_1000BASE);

return 0;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneded, we already know, that this phy supports 10 100 and 1000

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, thanks.

Comment on lines 69 to 83
#define BIT_SET(t, v, b) \
do { \
if (t) { \
(v) |= (b); \
} \
} while (0)

#define BIT_SET_CLR(t, v, b) \
do { \
if (t) { \
(v) |= (b); \
} else { \
(v) &= ~(b); \
} \
} while (0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already have IS_BIT_SET or WRITE_BIT in include/zephyr/sys/util_macro.h, use them, don't define your own

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, thanks.

@JiafeiPan
Copy link
Contributor Author

can you clarify what is the added value of the new phy driver over the generic one

Some private features could be leveraged, such as PM and clock delay configuration.

@JiafeiPan JiafeiPan force-pushed the pr/frdm-imx93-ethernet branch from 9e82bae to 62cefba Compare October 16, 2025 03:26
Copy link

Add PHY driver for Motorcomm YT8521 which is used on FRDM_IMX93 board.

Signed-off-by: Hongbo Wang <[email protected]>
Signed-off-by: Jiafei Pan <[email protected]>
Signed-off-by: Chekhov Ma <[email protected]>
Enable ENET port on FRDM_IMX93 board.

Signed-off-by: Hongbo Wang <[email protected]>
Signed-off-by: Chekhov Ma <[email protected]>
Signed-off-by: Jiafei Pan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants