| 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: <27b6701b-f3f1-4277-8720-b436dc8cc75a@lunn.ch>
Date: Tue, 30 Jan 2024 01:53:32 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Andy Gross <agross@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
Robert Marko <robert.marko@...tura.hr>,
linux-arm-msm@...r.kernel.org, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Jie Luo <quic_luoj@...cinc.com>
Subject: Re: [net-next PATCH v2 2/3] net: mdio: ipq4019: add support for
clock-frequency property
On Tue, Jan 30, 2024 at 01:35:21AM +0100, Christian Marangi wrote:
> The IPQ4019 MDIO internally divide the clock feed by AHB based on the
> MDIO_MODE reg. On reset or power up, the default value for the
> divider is 0xff that reflect the divider set to /256.
>
> This makes the MDC run at a very low rate, that is, considering AHB is
> always fixed to 100Mhz, a value of 390KHz.
>
> This hasn't have been a problem as MDIO wasn't used for time sensitive
> operation, it is now that on IPQ807x is usually mounted with PHY that
> requires MDIO to load their firmware (example Aquantia PHY).
>
> To handle this problem and permit to set the correct designed MDC
> frequency for the SoC add support for the standard "clock-frequency"
> property for the MDIO node.
>
> The divider supports value from /1 to /256 and the common value are to
> set it to /16 to reflect 6.25Mhz or to /8 on newer platform to reflect
> 12.5Mhz.
>
> To scan if the requested rate is supported by the divider, loop with
> each supported divider and stop when the requested rate match the final
> rate with the current divider. An error is returned if the rate doesn't
> match any value.
>
> On MDIO reset, the divider is restored to the requested value to prevent
> any kind of downclocking caused by the divider reverting to a default
> value.
>
> To follow 802.3 spec of 2.5MHz of default value, if divider is set at
> /256 and "clock-frequency" is not set in DT, assume nobody set the
> divider and try to find the closest MDC rate to 2.5MHz. (in the case of
> AHB set to 100MHz, it's 1.5625MHz)
>
> While at is also document other bits of the MDIO_MODE reg to have a
> clear idea of what is actually applied there.
>
> Documentation of some BITs is skipped as they are marked as reserved and
> their usage is not clear (RES 11:9 GENPHY 16:13 RES1 19:17)
>
> Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> ---
> drivers/net/mdio/mdio-ipq4019.c | 109 ++++++++++++++++++++++++++++++--
> 1 file changed, 103 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
> index abd8b508ec16..61638f25c184 100644
> --- a/drivers/net/mdio/mdio-ipq4019.c
> +++ b/drivers/net/mdio/mdio-ipq4019.c
> @@ -14,6 +14,20 @@
> #include <linux/clk.h>
>
> #define MDIO_MODE_REG 0x40
> +#define MDIO_MODE_MDC_MODE BIT(12)
> +/* 0 = Clause 22, 1 = Clause 45 */
> +#define MDIO_MODE_C45 BIT(8)
> +#define MDIO_MODE_DIV_MASK GENMASK(7, 0)
> +#define MDIO_MODE_DIV(x) FIELD_PREP(MDIO_MODE_DIV_MASK, (x) - 1)
> +#define MDIO_MODE_DIV_1 0x0
> +#define MDIO_MODE_DIV_2 0x1
> +#define MDIO_MODE_DIV_4 0x3
> +#define MDIO_MODE_DIV_8 0x7
> +#define MDIO_MODE_DIV_16 0xf
> +#define MDIO_MODE_DIV_32 0x1f
> +#define MDIO_MODE_DIV_64 0x3f
> +#define MDIO_MODE_DIV_128 0x7f
> +#define MDIO_MODE_DIV_256 0xff
> #define MDIO_ADDR_REG 0x44
> #define MDIO_DATA_WRITE_REG 0x48
> #define MDIO_DATA_READ_REG 0x4c
> @@ -26,9 +40,6 @@
> #define MDIO_CMD_ACCESS_CODE_C45_WRITE 1
> #define MDIO_CMD_ACCESS_CODE_C45_READ 2
>
> -/* 0 = Clause 22, 1 = Clause 45 */
> -#define MDIO_MODE_C45 BIT(8)
> -
> #define IPQ4019_MDIO_TIMEOUT 10000
> #define IPQ4019_MDIO_SLEEP 10
>
> @@ -41,6 +52,7 @@ struct ipq4019_mdio_data {
> void __iomem *membase;
> void __iomem *eth_ldo_rdy;
> struct clk *mdio_clk;
> + unsigned int mdc_rate;
> };
>
> static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
> @@ -203,6 +215,38 @@ static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
> return 0;
> }
>
> +static int ipq4019_mdio_set_div(struct ipq4019_mdio_data *priv)
> +{
> + unsigned long ahb_rate;
> + int div;
> + u32 val;
> +
> + /* If we don't have a clock for AHB use the fixed value */
> + ahb_rate = IPQ_MDIO_CLK_RATE;
> + if (priv->mdio_clk)
> + ahb_rate = clk_get_rate(priv->mdio_clk);
> +
> + /* MDC rate is ahb_rate/(MDIO_MODE_DIV + 1)
> + * While supported, internal documentation doesn't
> + * assure correct functionality of the MDIO bus
> + * with divider of 1, 2 or 4.
> + */
> + for (div = 8; div <= 256; div *= 2) {
> + /* The requested rate is supported by the div */
> + if (priv->mdc_rate == DIV_ROUND_UP(ahb_rate, div)) {
> + val = readl(priv->membase + MDIO_MODE_REG);
> + val &= ~MDIO_MODE_DIV_MASK;
> + val |= MDIO_MODE_DIV(div);
> + writel(val, priv->membase + MDIO_MODE_REG);
> +
> + return 0;
> + }
> + }
> +
> + /* The requested rate is not supported */
> + return -EINVAL;
> +}
> +
> static int ipq_mdio_reset(struct mii_bus *bus)
> {
> struct ipq4019_mdio_data *priv = bus->priv;
> @@ -225,10 +269,58 @@ static int ipq_mdio_reset(struct mii_bus *bus)
> return ret;
>
> ret = clk_prepare_enable(priv->mdio_clk);
> - if (ret == 0)
> - mdelay(10);
> + if (ret)
> + return ret;
>
> - return ret;
> + mdelay(10);
> +
> + /* Restore MDC rate */
> + return ipq4019_mdio_set_div(priv);
> +}
> +
> +static void ipq4019_mdio_select_mdc_rate(struct platform_device *pdev,
> + struct ipq4019_mdio_data *priv)
> +{
> + unsigned long ahb_rate;
> + int div;
> + u32 val;
> +
> + /* MDC rate defined in DT, we don't have to decide a default value */
> + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> + &priv->mdc_rate))
> + return;
> +
> + /* If we don't have a clock for AHB use the fixed value */
> + ahb_rate = IPQ_MDIO_CLK_RATE;
> + if (priv->mdio_clk)
> + ahb_rate = clk_get_rate(priv->mdio_clk);
> +
> + /* Check what is the current div set */
> + val = readl(priv->membase + MDIO_MODE_REG);
> + div = FIELD_GET(MDIO_MODE_DIV_MASK, val);
> +
> + /* div is not set to the default value of /256
> + * Probably someone changed that (bootloader, other drivers)
> + * Keep this and doesn't overwrite it.
doesn't -> don't
Otherwise please add my Reviewed-by on the next version.
Andrew
Powered by blists - more mailing lists