From: Andrew Lunn <andrew@lunn.ch>
To: Matthias Kaehlcke <mka@chromium.org>
Cc: "David S . Miller" <davem@davemloft.net>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Florian Fainelli <f.fainelli@gmail.com>,
Heiner Kallweit <hkallweit1@gmail.com>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
Douglas Anderson <dianders@chromium.org>
Subject: Re: [PATCH v5 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
Date: Wed, 7 Aug 2019 20:24:42 +0200 [thread overview]
Message-ID: <20190807182442.GC26047@lunn.ch> (raw)
In-Reply-To: <20190807170449.205378-5-mka@chromium.org>
> +static int rtl8211e_config_led(struct phy_device *phydev, int led,
> + struct phy_led_config *cfg)
> +{
> + u16 lacr_bits = 0, lcr_bits = 0;
> + int oldpage, ret;
> +
> + switch (cfg->trigger.t) {
> + case PHY_LED_TRIGGER_LINK:
> + lcr_bits = 7 << (led * 4);
> + break;
> +
> + case PHY_LED_TRIGGER_LINK_10M:
> + lcr_bits = 1 << (led * 4);
> + break;
> +
> + case PHY_LED_TRIGGER_LINK_100M:
> + lcr_bits = 2 << (led * 4);
> + break;
> +
> + case PHY_LED_TRIGGER_LINK_1G:
> + lcr_bits |= 4 << (led * 4);
> + break;
> +
> + case PHY_LED_TRIGGER_NONE:
> + break;
> +
> + default:
> + phydev_warn(phydev,
> + "unknown trigger for LED%d: %d\n",
> + led, cfg->trigger.t);
Lets do this once in the core, not in every driver.
> + return -EINVAL;
> + }
> +
> + if (cfg->trigger.activity)
> + lacr_bits = BIT(RLT8211E_LACR_LEDACTCTRL_SHIFT + led);
> +
> + rtl8211e_disable_eee_led_mode(phydev);
> +
> + oldpage = rtl8211x_select_ext_page(phydev, 0x2c);
> + if (oldpage < 0) {
> + phydev_err(phydev, "failed to select extended page: %d\n", oldpage);
> + return oldpage;
> + }
> +
> + ret = __phy_modify(phydev, RTL8211E_LACR,
> + LACR_MASK(led), lacr_bits);
> + if (ret) {
> + phydev_err(phydev, "failed to write LACR reg: %d\n",
> + ret);
> + goto err;
> + }
> +
> + ret = __phy_modify(phydev, RTL8211E_LCR,
> + LCR_MASK(led), lcr_bits);
> + if (ret)
> + phydev_err(phydev, "failed to write LCR reg: %d\n",
> + ret);
That is a lot of phydev_err() calls. The rest of the driver does not
use any. Also, mdio operations are very unlikely to fail. So i would
just leave it to the layer above to report there is a problem.
Andrew
prev parent reply other threads:[~2019-08-07 18:24 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-07 17:04 [PATCH v5 0/4] net: phy: Add support for DT configuration of PHY LEDs and use it for RTL8211E Matthias Kaehlcke
2019-08-07 17:04 ` [PATCH v5 1/4] dt-bindings: net: phy: Add subnode for LED configuration Matthias Kaehlcke
2019-08-07 17:04 ` [PATCH v5 2/4] net: phy: Add support for generic LED configuration through the DT Matthias Kaehlcke
2019-08-07 18:15 ` Andrew Lunn
2019-08-07 18:18 ` Andrew Lunn
2019-08-07 17:04 ` [PATCH v5 3/4] net: phy: realtek: Add helpers for accessing RTL8211x extension pages Matthias Kaehlcke
2019-08-07 17:04 ` [PATCH v5 4/4] net: phy: realtek: Add LED configuration support for RTL8211E Matthias Kaehlcke
2019-08-07 18:24 ` Andrew Lunn [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190807182442.GC26047@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=f.fainelli@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mka@chromium.org \
--cc=netdev@vger.kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).