On 2022/12/16 19:58, Heiner Kallweit wrote:
> On 16.12.2022 08:06, Yanhong Wang wrote:
>> This adds basic support for the Motorcomm YT8531
>> Gigabit Ethernet PHY.
>>
>> Signed-off-by: Yanhong Wang <yanhong.wang@xxxxxxxxxxxxxxxx>
>> ---
>> drivers/net/phy/Kconfig | 3 +-
>> drivers/net/phy/motorcomm.c | 202 ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 204 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>> index c57a0262fb64..86399254d9ff 100644
>> --- a/drivers/net/phy/Kconfig
>> +++ b/drivers/net/phy/Kconfig
>> @@ -258,9 +258,10 @@ config MICROSEMI_PHY
>>
>> config MOTORCOMM_PHY
>> tristate "Motorcomm PHYs"
>> + default SOC_STARFIVE
>
> Both are completely independent. This default should be removed.
>
>> help
>> Enables support for Motorcomm network PHYs.
>> - Currently supports the YT8511 gigabit PHY.
>> + Currently supports the YT8511 and YT8531 gigabit PHYs.
>>
>
> This doesn't apply. Parts of your patch exist already in net-next.
> Support for YT8531S has been added in the meantime. Please rebase
> your patch on net-next and annotate your patch as net-next.
>
Thanks. Parts of this patch exist already, after discussion unanimity was achieved,
i will remove the parts of YT8531 in the next version.
>> config NATIONAL_PHY
>> tristate "National Semiconductor PHYs"
>> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
>> index 7e6ac2c5e27e..bca03185b338 100644
>> --- a/drivers/net/phy/motorcomm.c
>> +++ b/drivers/net/phy/motorcomm.c
>> @@ -3,13 +3,17 @@
>> * Driver for Motorcomm PHYs
>> *
>> * Author: Peter Geis <pgwipeout@xxxxxxxxx>
>> + *
>> */
>>
>> +#include <linux/bitops.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> +#include <linux/of.h>
>> #include <linux/phy.h>
>>
>> #define PHY_ID_YT8511 0x0000010a
>> +#define PHY_ID_YT8531 0x4f51e91b
>>
>> #define YT8511_PAGE_SELECT 0x1e
>> #define YT8511_PAGE 0x1f
>> @@ -17,6 +21,10 @@
>> #define YT8511_EXT_DELAY_DRIVE 0x0d
>> #define YT8511_EXT_SLEEP_CTRL 0x27
>>
>> +#define YTPHY_EXT_SMI_SDS_PHY 0xa000
>> +#define YTPHY_EXT_CHIP_CONFIG 0xa001
>> +#define YTPHY_EXT_RGMII_CONFIG1 0xa003
>> +
>> /* 2b00 25m from pll
>> * 2b01 25m from xtl *default*
>> * 2b10 62.m from pll
>> @@ -38,6 +46,51 @@
>> #define YT8511_DELAY_FE_TX_EN (0xf << 12)
>> #define YT8511_DELAY_FE_TX_DIS (0x2 << 12)
>>
>> +struct ytphy_reg_field {
>> + char *name;
>> + u32 mask;
>> + u8 dflt; /* Default value */
>> +};
>> +
>> +struct ytphy_priv_t {
>> + u32 tx_inverted_1000;
>> + u32 tx_inverted_100;
>> + u32 tx_inverted_10;
>> +};
>> +
>> +/* rx_delay_sel: RGMII rx clock delay train configuration, about 150ps per
>> + * step. Delay = 150ps * N
>> + *
>> + * tx_delay_sel_fe: RGMII tx clock delay train configuration when speed is
>> + * 100Mbps or 10Mbps, it's 150ps per step. Delay = 150ps * N
>> + *
>> + * tx_delay_sel: RGMII tx clock delay train configuration when speed is
>> + * 1000Mbps, it's 150ps per step. Delay = 150ps * N
>> + */
>> +static const struct ytphy_reg_field ytphy_rxtxd_grp[] = {
>> + { "rx_delay_sel", GENMASK(13, 10), 0x0 },
>> + { "tx_delay_sel_fe", GENMASK(7, 4), 0xf },
>> + { "tx_delay_sel", GENMASK(3, 0), 0x1 }
>> +};
>> +
>> +/* tx_inverted_x: Use original or inverted RGMII TX_CLK to drive the RGMII
>> + * TX_CLK delay train configuration when speed is
>> + * xMbps(10/100/1000Mbps).
>> + * 0: original, 1: inverted
>> + */
>> +static const struct ytphy_reg_field ytphy_txinver_grp[] = {
>> + { "tx_inverted_1000", BIT(14), 0x0 },
>> + { "tx_inverted_100", BIT(14), 0x0 },
>> + { "tx_inverted_10", BIT(14), 0x0 }
>
> Copy & Paste error that mask is the same for all entries?
>
>> +};
>> +
>> +/* rxc_dly_en: RGMII clk 2ns delay control bit.
>> + * 0: disable 1: enable
>> + */
>> +static const struct ytphy_reg_field ytphy_rxden_grp[] = {
>> + { "rxc_dly_en", BIT(8), 0x1 }
>> +};
>> +
>> static int yt8511_read_page(struct phy_device *phydev)
>> {
>> return __phy_read(phydev, YT8511_PAGE_SELECT);
>> @@ -48,6 +101,33 @@ static int yt8511_write_page(struct phy_device *phydev, int page)
>> return __phy_write(phydev, YT8511_PAGE_SELECT, page);
>> };
>>
>> +static int ytphy_read_ext(struct phy_device *phydev, u32 regnum)
>> +{
>> + int ret;
>> + int val;
>> +
>> + ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
>> + if (ret < 0)
>> + return ret;
>> +
>> + val = __phy_read(phydev, YT8511_PAGE);
>> +
>> + return val;
>> +}
>> +
>> +static int ytphy_write_ext(struct phy_device *phydev, u32 regnum, u16 val)
>> +{
>> + int ret;
>> +
>> + ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = __phy_write(phydev, YT8511_PAGE, val);
>> +
>> + return ret;
>> +}
>> +
>> static int yt8511_config_init(struct phy_device *phydev)
>> {
>> int oldpage, ret = 0;
>> @@ -111,6 +191,116 @@ static int yt8511_config_init(struct phy_device *phydev)
>> return phy_restore_page(phydev, oldpage, ret);
>> }
>>
>> +static int ytphy_config_init(struct phy_device *phydev)
>> +{
>> + struct device_node *of_node;
>> + u32 val;
>> + u32 mask;
>> + u32 cfg;
>> + int ret;
>> + int i = 0;
>> +
>> + of_node = phydev->mdio.dev.of_node;
>> + if (of_node) {
>> + ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);
>> + if (!ret) {
>> + mask = ytphy_rxden_grp[0].mask;
>> + val = ytphy_read_ext(phydev, YTPHY_EXT_CHIP_CONFIG);
>> +
>> + /* check the cfg overflow or not */
>> + cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
>> +
>> + val &= ~mask;
>> + val |= FIELD_PREP(mask, cfg);
>> + ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG, val);
>
> This is the unlocked version. MDIO bus locking is missing.
>
>> + }
>> +
>> + val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
>> + for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) {
>> + ret = of_property_read_u32(of_node, ytphy_rxtxd_grp[i].name, &cfg);
>> + if (!ret) {
>> + mask = ytphy_rxtxd_grp[i].mask;
>> +
>> + /* check the cfg overflow or not */
>> + cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
>> +
>> + val &= ~mask;
>> + val |= cfg << (ffs(mask) - 1);
>> + }
>> + }
>> + return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
>> + }
>> +
>> + phydev_err(phydev, "Get of node fail\n");
>> +
>
> Please consider that the PHY may be used on non-DT systems.
>
>> + return -EINVAL;
>> +}
>> +
>> +static void ytphy_link_change_notify(struct phy_device *phydev)
>> +{
>> + u32 val;
>> + struct ytphy_priv_t *ytphy_priv = phydev->priv;
>> +
>> + if (phydev->speed < 0)
>> + return;
>> +
>> + val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
>> + switch (phydev->speed) {
>> + case SPEED_1000:
>> + val &= ~ytphy_txinver_grp[0].mask;
>> + val |= FIELD_PREP(ytphy_txinver_grp[0].mask,
>> + ytphy_priv->tx_inverted_1000);
>> + break;
>> +
>> + case SPEED_100:
>> + val &= ~ytphy_txinver_grp[1].mask;
>> + val |= FIELD_PREP(ytphy_txinver_grp[1].mask,
>> + ytphy_priv->tx_inverted_100);
>> + break;
>> +
>> + case SPEED_10:
>> + val &= ~ytphy_txinver_grp[2].mask;
>> + val |= FIELD_PREP(ytphy_txinver_grp[2].mask,
>> + ytphy_priv->tx_inverted_10);
>> + break;
>> +
>> + default:
>> + break;
>> + }
>> +
>> + ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
>> +}
>> +
>> +static int yt8531_probe(struct phy_device *phydev)
>> +{
>> + struct ytphy_priv_t *priv;
>> + const struct device_node *of_node;
>> + u32 val;
>> + int ret;
>> +
>> + priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + of_node = phydev->mdio.dev.of_node;
>> + if (of_node) {
>> + ret = of_property_read_u32(of_node, ytphy_txinver_grp[0].name, &val);
>> + if (!ret)
>> + priv->tx_inverted_1000 = val;
>> +
>> + ret = of_property_read_u32(of_node, ytphy_txinver_grp[1].name, &val);
>> + if (!ret)
>> + priv->tx_inverted_100 = val;
>> +
>> + ret = of_property_read_u32(of_node, ytphy_txinver_grp[2].name, &val);
>> + if (!ret)
>> + priv->tx_inverted_10 = val;
>> + }
>> + phydev->priv = priv;
>> +
>> + return 0;
>> +}
>> +
>> static struct phy_driver motorcomm_phy_drvs[] = {
>> {
>> PHY_ID_MATCH_EXACT(PHY_ID_YT8511),
>> @@ -120,6 +310,17 @@ static struct phy_driver motorcomm_phy_drvs[] = {
>> .resume = genphy_resume,
>> .read_page = yt8511_read_page,
>> .write_page = yt8511_write_page,
>> + }, {
>> + PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
>> + .name = "YT8531 Gigabit Ethernet",
>> + .probe = yt8531_probe,
>> + .config_init = ytphy_config_init,
>> + .read_status = genphy_read_status,
>> + .suspend = genphy_suspend,
>> + .resume = genphy_resume,
>> + .read_page = yt8511_read_page,
>> + .write_page = yt8511_write_page,
>> + .link_change_notify = ytphy_link_change_notify,
>> },
>> };
>>
>> @@ -131,6 +332,7 @@ MODULE_LICENSE("GPL");
>>
>> static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
>> { PHY_ID_MATCH_EXACT(PHY_ID_YT8511) },
>> + { PHY_ID_MATCH_EXACT(PHY_ID_YT8531) },
>> { /* sentinal */ }
>> };
>>
>
