| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZNYmHFkC5ZxySq1h@vergenet.net>
Date: Fri, 11 Aug 2023 14:14:20 +0200
From: Simon Horman <horms@...nel.org>
To: Daniel Golle <daniel@...rotopia.org>
Cc: Qingfang Deng <dqfext@...il.com>,
SkyLake Huang <SkyLake.Huang@...iatek.com>,
Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH net-next] net: phy: mediatek-ge-soc: support PHY LEDs
On Fri, Aug 11, 2023 at 04:35:38AM +0100, Daniel Golle wrote:
> Implement netdev trigger and primitive bliking offloading as well as
> simple set_brigthness function for both PHY LEDs of the in-SoC PHYs
> found in MT7981 and MT7988.
>
> On MT7988 it is necessary to read the boottrap register and apply LED
> polarities accordingly to get uniform behavior from all LEDs.
>
> Signed-off-by: Daniel Golle <daniel@...rotopia.org>
...
> +static int mt798x_phy_hw_led_on_set(struct phy_device *phydev, u8 index,
> + bool on)
> +{
> + struct mtk_socphy_priv *priv = phydev->priv;
> + u32 mask = MTK_PHY_LED_STATE_FORCE_ON << (index ? 16 : 0);
> + bool changed;
> +
> + if (on)
> + changed = (test_and_set_bit(mask, &priv->led_state) != mask);
> + else
> + changed = !!test_and_clear_bit(mask, &priv->led_state);
Hi Daniel,
are you sure the first parameter to test_and_set_bit() and
test_and_clear_bit() is correct here and below?
Smatch warns that: test_and_clear_bit() takes a bit number
I.e. the first argument should be a bit number, not a mask.
> +
> + changed |= !!test_and_clear_bit(MTK_PHY_LED_STATE_NETDEV <<
> + (index ? 16 : 0), &priv->led_state);
> + if (changed)
> + return phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
> + MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL,
> + MTK_PHY_LED_ON_MASK,
> + on ? MTK_PHY_LED_ON_FORCE_ON : 0);
> + else
> + return 0;
> +}
> +
> +static int mt798x_phy_hw_led_blink_set(struct phy_device *phydev, u8 index,
> + bool blinking)
> +{
> + struct mtk_socphy_priv *priv = phydev->priv;
> + u32 mask = MTK_PHY_LED_STATE_FORCE_BLINK << (index ? 16 : 0);
> + bool changed;
> +
> + if (blinking)
> + changed = (test_and_set_bit(mask, &priv->led_state) != mask);
> + else
> + changed = !!test_and_clear_bit(mask, &priv->led_state);
> +
> + changed |= !!test_bit(MTK_PHY_LED_STATE_NETDEV << (index ? 16 : 0), &priv->led_state);
> + if (changed)
> + return phy_write_mmd(phydev, MDIO_MMD_VEND2, index ?
> + MTK_PHY_LED1_BLINK_CTRL : MTK_PHY_LED0_BLINK_CTRL,
> + blinking ? MTK_PHY_LED_BLINK_FORCE_BLINK : 0);
> + else
> + return 0;
> +}
...
> +static int mt798x_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
> + unsigned long *rules)
> +{
> + u32 blink_mask = MTK_PHY_LED_STATE_FORCE_BLINK << (index ? 16 : 0);
> + u32 netdev_mask = MTK_PHY_LED_STATE_NETDEV << (index ? 16 : 0);
> + u32 on_mask = MTK_PHY_LED_STATE_FORCE_ON << (index ? 16 : 0);
> + struct mtk_socphy_priv *priv = phydev->priv;
> + int on, blink;
> +
> + if (index > 1)
> + return -EINVAL;
> +
> + on = phy_read_mmd(phydev, MDIO_MMD_VEND2,
> + index ? MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL);
> +
> + if (on < 0)
> + return -EIO;
> +
> + blink = phy_read_mmd(phydev, MDIO_MMD_VEND2,
> + index ? MTK_PHY_LED1_BLINK_CTRL :
> + MTK_PHY_LED0_BLINK_CTRL);
> + if (blink < 0)
> + return -EIO;
> +
> + if ((on & (MTK_PHY_LED_ON_LINK1000 | MTK_PHY_LED_ON_LINK100 |
> + MTK_PHY_LED_ON_LINK10)) ||
> + (blink & (MTK_PHY_LED_BLINK_1000RX | MTK_PHY_LED_BLINK_100RX |
> + MTK_PHY_LED_BLINK_10RX | MTK_PHY_LED_BLINK_1000TX |
> + MTK_PHY_LED_BLINK_100TX | MTK_PHY_LED_BLINK_10TX)))
> + set_bit(netdev_mask, &priv->led_state);
> + else
> + clear_bit(netdev_mask, &priv->led_state);
> +
> + if (on & MTK_PHY_LED_ON_FORCE_ON)
> + set_bit(on_mask, &priv->led_state);
> + else
> + clear_bit(on_mask, &priv->led_state);
> +
> + if (blink & MTK_PHY_LED_BLINK_FORCE_BLINK)
> + set_bit(blink_mask, &priv->led_state);
> + else
> + clear_bit(blink_mask, &priv->led_state);
> +
> + if (!rules)
> + return 0;
> +
> + if (on & (MTK_PHY_LED_ON_LINK1000 | MTK_PHY_LED_ON_LINK100 | MTK_PHY_LED_ON_LINK10))
> + *rules |= BIT(TRIGGER_NETDEV_LINK);
> +
> + if (on & MTK_PHY_LED_ON_LINK10)
> + *rules |= BIT(TRIGGER_NETDEV_LINK_10);
> +
> + if (on & MTK_PHY_LED_ON_LINK100)
> + *rules |= BIT(TRIGGER_NETDEV_LINK_100);
> +
> + if (on & MTK_PHY_LED_ON_LINK1000)
> + *rules |= BIT(TRIGGER_NETDEV_LINK_1000);
> +
> + if (on & MTK_PHY_LED_ON_FDX)
> + *rules |= BIT(TRIGGER_NETDEV_FULL_DUPLEX);
> +
> + if (on & MTK_PHY_LED_ON_HDX)
> + *rules |= BIT(TRIGGER_NETDEV_HALF_DUPLEX);
> +
> + if (blink & (MTK_PHY_LED_BLINK_1000RX | MTK_PHY_LED_BLINK_100RX | MTK_PHY_LED_BLINK_10RX))
> + *rules |= BIT(TRIGGER_NETDEV_RX);
> +
> + if (blink & (MTK_PHY_LED_BLINK_1000TX | MTK_PHY_LED_BLINK_100TX | MTK_PHY_LED_BLINK_10TX))
> + *rules |= BIT(TRIGGER_NETDEV_TX);
> +
> + return 0;
> +};
> +
> +static int mt798x_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
> + unsigned long rules)
> +{
> + u32 mask = MTK_PHY_LED_STATE_NETDEV << (index ? 16 : 0);
> + struct mtk_socphy_priv *priv = phydev->priv;
> + u16 on = 0, blink = 0;
> + int ret;
> +
> + if (index > 1)
> + return -EINVAL;
> +
> + if (rules & BIT(TRIGGER_NETDEV_FULL_DUPLEX))
> + on |= MTK_PHY_LED_ON_FDX;
> +
> + if (rules & BIT(TRIGGER_NETDEV_HALF_DUPLEX))
> + on |= MTK_PHY_LED_ON_HDX;
> +
> + if (rules & (BIT(TRIGGER_NETDEV_LINK_10) | BIT(TRIGGER_NETDEV_LINK)))
> + on |= MTK_PHY_LED_ON_LINK10;
> +
> + if (rules & (BIT(TRIGGER_NETDEV_LINK_100) | BIT(TRIGGER_NETDEV_LINK)))
> + on |= MTK_PHY_LED_ON_LINK100;
> +
> + if (rules & (BIT(TRIGGER_NETDEV_LINK_1000) | BIT(TRIGGER_NETDEV_LINK)))
> + on |= MTK_PHY_LED_ON_LINK1000;
> +
> + if (rules & BIT(TRIGGER_NETDEV_RX)) {
> + blink |= MTK_PHY_LED_BLINK_10RX |
> + MTK_PHY_LED_BLINK_100RX |
> + MTK_PHY_LED_BLINK_1000RX;
> + }
> +
> + if (rules & BIT(TRIGGER_NETDEV_TX)) {
> + blink |= MTK_PHY_LED_BLINK_10TX |
> + MTK_PHY_LED_BLINK_100TX |
> + MTK_PHY_LED_BLINK_1000TX;
> + }
> +
> + if (blink || on)
> + set_bit(mask, &priv->led_state);
> + else
> + clear_bit(mask, &priv->led_state);
> +
> + ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
> + MTK_PHY_LED1_ON_CTRL :
> + MTK_PHY_LED0_ON_CTRL,
> + MTK_PHY_LED_ON_FDX |
> + MTK_PHY_LED_ON_HDX |
> + MTK_PHY_LED_ON_LINK10 |
> + MTK_PHY_LED_ON_LINK100 |
> + MTK_PHY_LED_ON_LINK1000,
> + on);
> +
> + if (ret)
> + return ret;
> +
> + return phy_write_mmd(phydev, MDIO_MMD_VEND2, index ?
> + MTK_PHY_LED1_BLINK_CTRL :
> + MTK_PHY_LED0_BLINK_CTRL, blink);
> +};
...
Powered by blists - more mailing lists