* [PATCH net-next v1 0/2] net: dsa: realtek: enable RTL8365MB power supplies @ 2026-08-06 14:02 Oleksij Rempel 2026-08-06 14:02 ` [PATCH net-next v1 1/2] dt-bindings: net: dsa: realtek: add " Oleksij Rempel 2026-08-06 14:02 ` [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies Oleksij Rempel 0 siblings, 2 replies; 16+ messages in thread From: Oleksij Rempel @ 2026-08-06 14:02 UTC (permalink / raw) To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Oleksij Rempel, kernel, linux-kernel, Ahmad Fatoum, Alvin Šipraga, netdev, devicetree The RTL8365MB family has several external power rails (analog, digital I/O and PLL supplies) that on some boards are gated by regulators. Until now the driver assumed they were always on, so it could not be used on designs that keep these rails software-controlled. This series adds a device-tree description for those supplies and enables them in the driver before the chip is accessed: The supplies are optional: boards that hard-wire the rails work unchanged Ahmad Fatoum (1): net: dsa: realtek: rtl83xx: add support for enabling supplies Oleksij Rempel (1): dt-bindings: net: dsa: realtek: add RTL8365MB power supplies .../devicetree/bindings/net/dsa/realtek.yaml | 25 ++++++++++++ drivers/net/dsa/realtek/realtek.h | 2 + drivers/net/dsa/realtek/rtl8365mb_main.c | 5 +++ drivers/net/dsa/realtek/rtl83xx.c | 40 ++++++++++++++++--- 4 files changed, 66 insertions(+), 6 deletions(-) -- 2.47.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v1 1/2] dt-bindings: net: dsa: realtek: add RTL8365MB power supplies 2026-08-06 14:02 [PATCH net-next v1 0/2] net: dsa: realtek: enable RTL8365MB power supplies Oleksij Rempel @ 2026-08-06 14:02 ` Oleksij Rempel 2026-08-06 14:02 ` [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies Oleksij Rempel 1 sibling, 0 replies; 16+ messages in thread From: Oleksij Rempel @ 2026-08-06 14:02 UTC (permalink / raw) To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Oleksij Rempel, kernel, linux-kernel, Ahmad Fatoum, Alvin Šipraga, netdev, devicetree Document the RTL8365MB power rails as optional supplies. They are optional because the rails are often always-on; some designs break them out for granular hwmon monitoring and regulator control. They are scoped to realtek,rtl8365mb: different Realtek switch families use different power-rail naming schemes, so these rails do not apply to other families such as realtek,rtl8366rb (VDDA/VDD_CORE/VDDIO/...). Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- .../devicetree/bindings/net/dsa/realtek.yaml | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml index 473facd87a62..98ea6072bd96 100644 --- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml +++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml @@ -8,6 +8,30 @@ title: Realtek switches for unmanaged switches allOf: - $ref: dsa.yaml#/$defs/ethernet-ports + - if: + properties: + compatible: + contains: + const: realtek,rtl8365mb + then: + properties: + avddh-supply: + description: AVDDH high-voltage analog supply. + + avddl-supply: + description: AVDDL low-voltage analog supply. + + dvddio-supply: + description: DVDDIO digital I/O supply. + + dvddio1-supply: + description: DVDDIO_1 secondary digital I/O supply for the RGMII bank. + + dvddl-supply: + description: DVDDL digital core supply. + + pllvddl-supply: + description: PLLVDDL PLL analog supply. maintainers: - Linus Walleij <linusw@kernel.org> @@ -243,6 +267,7 @@ examples: platform { ethernet-switch { compatible = "realtek,rtl8365mb"; + avddh-supply = <&avddh>; mdc-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>; mdio-gpios = <&gpio1 17 GPIO_ACTIVE_HIGH>; reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; -- 2.47.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-06 14:02 [PATCH net-next v1 0/2] net: dsa: realtek: enable RTL8365MB power supplies Oleksij Rempel 2026-08-06 14:02 ` [PATCH net-next v1 1/2] dt-bindings: net: dsa: realtek: add " Oleksij Rempel @ 2026-08-06 14:02 ` Oleksij Rempel 2026-08-06 14:12 ` Andrew Lunn 1 sibling, 1 reply; 16+ messages in thread From: Oleksij Rempel @ 2026-08-06 14:02 UTC (permalink / raw) To: Linus Walleij, Luiz Angelo Daros de Luca, Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Ahmad Fatoum, Alvin Šipraga, Oleksij Rempel, kernel, linux-kernel, Alvin Šipraga, netdev, devicetree From: Ahmad Fatoum <a.fatoum@pengutronix.de> The power supplies powering the IC may not necessarily be enabled by the time the driver probes. The binding has been extended to describe the different power rails, so have the driver enable them in case they are specified. Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/realtek/realtek.h | 2 ++ drivers/net/dsa/realtek/rtl8365mb_main.c | 5 +++ drivers/net/dsa/realtek/rtl83xx.c | 40 ++++++++++++++++++++---- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h index 6e0148cee8d8..ba31605423b1 100644 --- a/drivers/net/dsa/realtek/realtek.h +++ b/drivers/net/dsa/realtek/realtek.h @@ -166,6 +166,8 @@ struct realtek_variant { u8 cmd_read; u8 cmd_write; size_t chip_data_sz; + /* NULL-terminated list of regulator supply names, or NULL */ + const char *const *supplies; }; /* RTL8366 library helpers */ diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c index 728231d8f94c..52ec4e91306c 100644 --- a/drivers/net/dsa/realtek/rtl8365mb_main.c +++ b/drivers/net/dsa/realtek/rtl8365mb_main.c @@ -3334,6 +3334,10 @@ static const struct realtek_ops rtl8365mb_ops = { .phy_write = rtl8365mb_phy_write, }; +static const char *const rtl8365mb_supplies[] = { + "avddh", "avddl", "dvddio", "dvddio1", "dvddl", "pllvddl", NULL, +}; + const struct realtek_variant rtl8365mb_variant = { .ds_ops = &rtl8365mb_switch_ops, .ops = &rtl8365mb_ops, @@ -3342,6 +3346,7 @@ const struct realtek_variant rtl8365mb_variant = { .cmd_read = 0xb9, .cmd_write = 0xb8, .chip_data_sz = sizeof(struct rtl8365mb), + .supplies = rtl8365mb_supplies, }; static const struct of_device_id rtl8365mb_of_match[] = { diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c index 35df809a5951..fb6aaaa60aa3 100644 --- a/drivers/net/dsa/realtek/rtl83xx.c +++ b/drivers/net/dsa/realtek/rtl83xx.c @@ -2,6 +2,7 @@ #include <linux/module.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h> #include <linux/of_mdio.h> #include <linux/if_bridge.h> #include <linux/etherdevice.h> @@ -113,6 +114,24 @@ int rtl83xx_setup_user_mdio(struct dsa_switch *ds) } EXPORT_SYMBOL_NS_GPL(rtl83xx_setup_user_mdio, "REALTEK_DSA"); +static int rtl83xx_enable_supplies(struct device *dev, + const char *const *supplies) +{ + int i, ret, count = 0; + + for (i = 0; supplies && supplies[i]; i++) { + ret = devm_regulator_get_enable_optional(dev, supplies[i]); + if (ret == -ENODEV) + continue; + if (ret) + return dev_err_probe(dev, ret, "failed to enable %s supply\n", + supplies[i]); + count++; + } + + return count; +} + /** * rtl83xx_probe() - probe a Realtek switch * @dev: the device being probed @@ -145,6 +164,7 @@ rtl83xx_probe(struct device *dev, .lock = rtl83xx_lock, .unlock = rtl83xx_unlock, }; + int num_supplies; int ret; var = of_device_get_match_data(dev); @@ -195,17 +215,22 @@ rtl83xx_probe(struct device *dev, priv->leds_disabled = of_property_read_bool(dev->of_node, "realtek,disable-leds"); - /* TODO: if power is software controlled, set up any regulators here */ + /* Enable the supplies before the RESET line is requested and driven, + * so the chip is powered before current is forced into its pins. + */ + num_supplies = rtl83xx_enable_supplies(dev, var->supplies); + if (num_supplies < 0) + return ERR_PTR(num_supplies); + priv->reset_ctl = devm_reset_control_get_optional(dev, NULL); if (IS_ERR(priv->reset_ctl)) return dev_err_cast_probe(dev, priv->reset_ctl, "failed to get reset control\n"); priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); - if (IS_ERR(priv->reset)) { - dev_err(dev, "failed to get RESET GPIO\n"); - return ERR_CAST(priv->reset); - } + if (IS_ERR(priv->reset)) + return dev_err_cast_probe(dev, priv->reset, + "failed to get RESET GPIO\n"); dev_set_drvdata(dev, priv); @@ -214,10 +239,13 @@ rtl83xx_probe(struct device *dev, dev_dbg(dev, "asserted RESET\n"); msleep(REALTEK_HW_STOP_DELAY); rtl83xx_reset_deassert(priv); - msleep(REALTEK_HW_START_DELAY); dev_dbg(dev, "deasserted RESET\n"); } + /* Wait for the chip to come up only if we powered and/or reset it. */ + if (priv->reset_ctl || priv->reset || num_supplies) + msleep(REALTEK_HW_START_DELAY); + return priv; } EXPORT_SYMBOL_NS_GPL(rtl83xx_probe, "REALTEK_DSA"); -- 2.47.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-06 14:02 ` [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies Oleksij Rempel @ 2026-08-06 14:12 ` Andrew Lunn 2026-08-06 15:10 ` Oleksij Rempel 0 siblings, 1 reply; 16+ messages in thread From: Andrew Lunn @ 2026-08-06 14:12 UTC (permalink / raw) To: Oleksij Rempel Cc: Linus Walleij, Luiz Angelo Daros de Luca, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ahmad Fatoum, Alvin Šipraga, kernel, linux-kernel, Alvin Šipraga, netdev, devicetree > +static const char *const rtl8365mb_supplies[] = { > + "avddh", "avddl", "dvddio", "dvddio1", "dvddl", "pllvddl", NULL, > +}; > + > +static int rtl83xx_enable_supplies(struct device *dev, > + const char *const *supplies) > +{ > + int i, ret, count = 0; > + > + for (i = 0; supplies && supplies[i]; i++) { > + ret = devm_regulator_get_enable_optional(dev, supplies[i]); > + if (ret == -ENODEV) > + continue; > + if (ret) > + return dev_err_probe(dev, ret, "failed to enable %s supply\n", > + supplies[i]); > + count++; > + } > + I've never used it, i've no idea if it is applicable, but there does appear to be _bulk_ calls in the regulator API, maybe which is similar to the clk _bulk_ API? Andrew ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-06 14:12 ` Andrew Lunn @ 2026-08-06 15:10 ` Oleksij Rempel 2026-08-06 15:35 ` Andrew Lunn 0 siblings, 1 reply; 16+ messages in thread From: Oleksij Rempel @ 2026-08-06 15:10 UTC (permalink / raw) To: Andrew Lunn Cc: Linus Walleij, Luiz Angelo Daros de Luca, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ahmad Fatoum, Alvin Šipraga, kernel, linux-kernel, Alvin Šipraga, netdev, devicetree On Thu, Aug 06, 2026 at 04:12:44PM +0200, Andrew Lunn wrote: > > +static const char *const rtl8365mb_supplies[] = { > > + "avddh", "avddl", "dvddio", "dvddio1", "dvddl", "pllvddl", NULL, > > +}; > > + > > > +static int rtl83xx_enable_supplies(struct device *dev, > > + const char *const *supplies) > > +{ > > + int i, ret, count = 0; > > + > > + for (i = 0; supplies && supplies[i]; i++) { > > + ret = devm_regulator_get_enable_optional(dev, supplies[i]); > > + if (ret == -ENODEV) > > + continue; > > + if (ret) > > + return dev_err_probe(dev, ret, "failed to enable %s supply\n", > > + supplies[i]); > > + count++; > > + } > > + > > I've never used it, i've no idea if it is applicable, but there does > appear to be _bulk_ calls in the regulator API, maybe which is similar > to the clk _bulk_ API? Ack, in the first iteration of this patch we was using devm_regulator_bulk_get_enable(). But it would print warnings on existing boards without regulators in devicetree. Currently there are no bulk helpers with _optional regulators, so I reworked it to hand made optional bulk support. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-06 15:10 ` Oleksij Rempel @ 2026-08-06 15:35 ` Andrew Lunn 2026-08-06 15:56 ` Ahmad Fatoum 0 siblings, 1 reply; 16+ messages in thread From: Andrew Lunn @ 2026-08-06 15:35 UTC (permalink / raw) To: Oleksij Rempel Cc: Linus Walleij, Luiz Angelo Daros de Luca, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ahmad Fatoum, Alvin Šipraga, kernel, linux-kernel, Alvin Šipraga, netdev, devicetree On Thu, Aug 06, 2026 at 05:10:23PM +0200, Oleksij Rempel wrote: > On Thu, Aug 06, 2026 at 04:12:44PM +0200, Andrew Lunn wrote: > > > +static const char *const rtl8365mb_supplies[] = { > > > + "avddh", "avddl", "dvddio", "dvddio1", "dvddl", "pllvddl", NULL, > > > +}; > > > + > > > > > +static int rtl83xx_enable_supplies(struct device *dev, > > > + const char *const *supplies) > > > +{ > > > + int i, ret, count = 0; > > > + > > > + for (i = 0; supplies && supplies[i]; i++) { > > > + ret = devm_regulator_get_enable_optional(dev, supplies[i]); > > > + if (ret == -ENODEV) > > > + continue; > > > + if (ret) > > > + return dev_err_probe(dev, ret, "failed to enable %s supply\n", > > > + supplies[i]); > > > + count++; > > > + } > > > + > > > > I've never used it, i've no idea if it is applicable, but there does > > appear to be _bulk_ calls in the regulator API, maybe which is similar > > to the clk _bulk_ API? > > Ack, in the first iteration of this patch we was using > devm_regulator_bulk_get_enable(). But it would print warnings on > existing boards without regulators in devicetree. Currently there are no > bulk helpers with _optional regulators, so I reworked it to hand made > optional bulk support. static inline int __must_check clk_bulk_get_optional(struct device *dev, static inline int __must_check devm_clk_bulk_get_optional(struct device *dev, static inline int __must_check devm_clk_bulk_get_optional_enable(struct device *dev, If clk has them, maybe regulators should as well? Andrew ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-06 15:35 ` Andrew Lunn @ 2026-08-06 15:56 ` Ahmad Fatoum 2026-08-06 17:40 ` Linus Walleij 0 siblings, 1 reply; 16+ messages in thread From: Ahmad Fatoum @ 2026-08-06 15:56 UTC (permalink / raw) To: Andrew Lunn, Oleksij Rempel Cc: Linus Walleij, Luiz Angelo Daros de Luca, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alvin Šipraga, kernel, linux-kernel, Alvin Šipraga, netdev, devicetree, Liam Girdwood, Mark Brown Hello Andrew, [Cc += regulator maintainers] On 8/6/26 17:35, Andrew Lunn wrote: > On Thu, Aug 06, 2026 at 05:10:23PM +0200, Oleksij Rempel wrote: >> On Thu, Aug 06, 2026 at 04:12:44PM +0200, Andrew Lunn wrote: >>>> +static const char *const rtl8365mb_supplies[] = { >>>> + "avddh", "avddl", "dvddio", "dvddio1", "dvddl", "pllvddl", NULL, >>>> +}; >>>> + >>> >>>> +static int rtl83xx_enable_supplies(struct device *dev, >>>> + const char *const *supplies) >>>> +{ >>>> + int i, ret, count = 0; >>>> + >>>> + for (i = 0; supplies && supplies[i]; i++) { >>>> + ret = devm_regulator_get_enable_optional(dev, supplies[i]); >>>> + if (ret == -ENODEV) >>>> + continue; >>>> + if (ret) >>>> + return dev_err_probe(dev, ret, "failed to enable %s supply\n", >>>> + supplies[i]); >>>> + count++; >>>> + } >>>> + >>> >>> I've never used it, i've no idea if it is applicable, but there does >>> appear to be _bulk_ calls in the regulator API, maybe which is similar >>> to the clk _bulk_ API? >> >> Ack, in the first iteration of this patch we was using >> devm_regulator_bulk_get_enable(). But it would print warnings on >> existing boards without regulators in devicetree. Currently there are no >> bulk helpers with _optional regulators, so I reworked it to hand made >> optional bulk support. > > static inline int __must_check clk_bulk_get_optional(struct device *dev, > static inline int __must_check devm_clk_bulk_get_optional(struct device *dev, > static inline int __must_check devm_clk_bulk_get_optional_enable(struct device *dev, > > If clk has them, maybe regulators should as well? clk and regulator have different interpretations of _optional when encountering a non-existent resource: clk_get return error clk_get_optional return NULL regulator_get return handle dummy regulator regulator_get_optional return error code So devm_regulator_bulk_get() is already the "optional" variant, but it causes all existing users to get dummy supply warnings. I don't know if there's a strong case for a devm_regulator_bulk_get_nowarn(), maybe Mark or Liam have an opinion on that. Cheers, Ahmad > > Andrew > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-06 15:56 ` Ahmad Fatoum @ 2026-08-06 17:40 ` Linus Walleij 2026-08-07 4:26 ` Oleksij Rempel 0 siblings, 1 reply; 16+ messages in thread From: Linus Walleij @ 2026-08-06 17:40 UTC (permalink / raw) To: Ahmad Fatoum, Mark Brown Cc: Andrew Lunn, Oleksij Rempel, Luiz Angelo Daros de Luca, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alvin Šipraga, kernel, linux-kernel, Alvin Šipraga, netdev, devicetree, Liam Girdwood On Thu, Aug 6, 2026 at 5:56 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: [Andrew says] > > static inline int __must_check clk_bulk_get_optional(struct device *dev, > > static inline int __must_check devm_clk_bulk_get_optional(struct device *dev, > > static inline int __must_check devm_clk_bulk_get_optional_enable(struct device *dev, > > > > If clk has them, maybe regulators should as well? > > clk and regulator have different interpretations of _optional > when encountering a non-existent resource: > > clk_get return error > clk_get_optional return NULL > regulator_get return handle dummy regulator > regulator_get_optional return error code > > So devm_regulator_bulk_get() is already the "optional" variant, but it causes > all existing users to get dummy supply warnings. So is this a big problem? Does it cause any other issues than dmesg noise? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-06 17:40 ` Linus Walleij @ 2026-08-07 4:26 ` Oleksij Rempel 2026-08-07 6:51 ` Linus Walleij 0 siblings, 1 reply; 16+ messages in thread From: Oleksij Rempel @ 2026-08-07 4:26 UTC (permalink / raw) To: Linus Walleij Cc: Ahmad Fatoum, Mark Brown, Andrew Lunn, Conor Dooley, Paolo Abeni, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Alvin Šipraga, Jakub Kicinski, Krzysztof Kozlowski, Vladimir Oltean, Alvin Šipraga, David S. Miller, devicetree On Thu, Aug 06, 2026 at 07:40:29PM +0200, Linus Walleij wrote: > On Thu, Aug 6, 2026 at 5:56 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > [Andrew says] > > > static inline int __must_check clk_bulk_get_optional(struct device *dev, > > > static inline int __must_check devm_clk_bulk_get_optional(struct device *dev, > > > static inline int __must_check devm_clk_bulk_get_optional_enable(struct device *dev, > > > > > > If clk has them, maybe regulators should as well? > > > > clk and regulator have different interpretations of _optional > > when encountering a non-existent resource: > > > > clk_get return error > > clk_get_optional return NULL > > regulator_get return handle dummy regulator > > regulator_get_optional return error code > > > > So devm_regulator_bulk_get() is already the "optional" variant, but it causes > > all existing users to get dummy supply warnings. > > So is this a big problem? Does it cause any other issues than > dmesg noise? As tested with local sashiko setup, new warnings for existing system without good reason will be reported as regression, so it will be blocker for net-next inclusion. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-07 4:26 ` Oleksij Rempel @ 2026-08-07 6:51 ` Linus Walleij 2026-08-07 7:10 ` Oleksij Rempel 0 siblings, 1 reply; 16+ messages in thread From: Linus Walleij @ 2026-08-07 6:51 UTC (permalink / raw) To: Oleksij Rempel, Mark Brown Cc: Ahmad Fatoum, Andrew Lunn, Conor Dooley, Paolo Abeni, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Alvin Šipraga, Jakub Kicinski, Krzysztof Kozlowski, Vladimir Oltean, Alvin Šipraga, David S. Miller, devicetree On Fri, Aug 7, 2026 at 6:26 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Thu, Aug 06, 2026 at 07:40:29PM +0200, Linus Walleij wrote: > > On Thu, Aug 6, 2026 at 5:56 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > > > [Andrew says] > > > > static inline int __must_check clk_bulk_get_optional(struct device *dev, > > > > static inline int __must_check devm_clk_bulk_get_optional(struct device *dev, > > > > static inline int __must_check devm_clk_bulk_get_optional_enable(struct device *dev, > > > > > > > > If clk has them, maybe regulators should as well? > > > > > > clk and regulator have different interpretations of _optional > > > when encountering a non-existent resource: > > > > > > clk_get return error > > > clk_get_optional return NULL > > > regulator_get return handle dummy regulator > > > regulator_get_optional return error code > > > > > > So devm_regulator_bulk_get() is already the "optional" variant, but it causes > > > all existing users to get dummy supply warnings. > > > > So is this a big problem? Does it cause any other issues than > > dmesg noise? > > As tested with local sashiko setup, new warnings for existing system > without good reason will be reported as regression, so it will be > blocker for net-next inclusion. Strong reverse centaur (*) warning on this one. I'm not sure this is how Mark want such "regressions" to be treated. I think a few dummy supplies is just fine and it's rather that Sashiko need to be taught not to treat that as bugs/regressions. Yours, Linus Walleij (*) Reverse centaur = humans touted to please AI, as opposed to AI helping humans, sarcastic term coined by author Cory Doctorow. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-07 6:51 ` Linus Walleij @ 2026-08-07 7:10 ` Oleksij Rempel 2026-08-07 7:49 ` Ahmad Fatoum 0 siblings, 1 reply; 16+ messages in thread From: Oleksij Rempel @ 2026-08-07 7:10 UTC (permalink / raw) To: Linus Walleij Cc: Mark Brown, Andrew Lunn, Conor Dooley, Vladimir Oltean, Ahmad Fatoum, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Alvin Šipraga, Jakub Kicinski, Krzysztof Kozlowski, Paolo Abeni, Alvin Šipraga, David S. Miller, devicetree On Fri, Aug 07, 2026 at 08:51:53AM +0200, Linus Walleij wrote: > On Fri, Aug 7, 2026 at 6:26 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > On Thu, Aug 06, 2026 at 07:40:29PM +0200, Linus Walleij wrote: > > > On Thu, Aug 6, 2026 at 5:56 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > > > > > [Andrew says] > > > > > static inline int __must_check clk_bulk_get_optional(struct device *dev, > > > > > static inline int __must_check devm_clk_bulk_get_optional(struct device *dev, > > > > > static inline int __must_check devm_clk_bulk_get_optional_enable(struct device *dev, > > > > > > > > > > If clk has them, maybe regulators should as well? > > > > > > > > clk and regulator have different interpretations of _optional > > > > when encountering a non-existent resource: > > > > > > > > clk_get return error > > > > clk_get_optional return NULL > > > > regulator_get return handle dummy regulator > > > > regulator_get_optional return error code > > > > > > > > So devm_regulator_bulk_get() is already the "optional" variant, but it causes > > > > all existing users to get dummy supply warnings. > > > > > > So is this a big problem? Does it cause any other issues than > > > dmesg noise? > > > > As tested with local sashiko setup, new warnings for existing system > > without good reason will be reported as regression, so it will be > > blocker for net-next inclusion. > > Strong reverse centaur (*) warning on this one. > > I'm not sure this is how Mark want such "regressions" to be treated. > > I think a few dummy supplies is just fine and it's rather that Sashiko > need to be taught not to treat that as bugs/regressions. > > Yours, > Linus Walleij > > (*) Reverse centaur = humans touted to please AI, as opposed > to AI helping humans, sarcastic term coined by author Cory Doctorow. Without context, I would fully agree with this statement. But as some one who need to adders new warnings on every kernel update for our customers, I see the point of sashiko - new unnecessary warning is extra unnecessary work for some one else == regression. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-07 7:10 ` Oleksij Rempel @ 2026-08-07 7:49 ` Ahmad Fatoum 2026-08-07 8:20 ` Linus Walleij 0 siblings, 1 reply; 16+ messages in thread From: Ahmad Fatoum @ 2026-08-07 7:49 UTC (permalink / raw) To: Oleksij Rempel, Linus Walleij Cc: Mark Brown, Andrew Lunn, Conor Dooley, Vladimir Oltean, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Alvin Šipraga, Jakub Kicinski, Krzysztof Kozlowski, Paolo Abeni, Alvin Šipraga, David S. Miller, devicetree Hello, On 8/7/26 9:10 AM, Oleksij Rempel wrote: > On Fri, Aug 07, 2026 at 08:51:53AM +0200, Linus Walleij wrote: >> On Fri, Aug 7, 2026 at 6:26 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote: >>> On Thu, Aug 06, 2026 at 07:40:29PM +0200, Linus Walleij wrote: >>>> On Thu, Aug 6, 2026 at 5:56 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: >>>> >>>> [Andrew says] >>>>> So devm_regulator_bulk_get() is already the "optional" variant, but it causes >>>>> all existing users to get dummy supply warnings. >>>> >>>> So is this a big problem? Does it cause any other issues than >>>> dmesg noise? >>> >>> As tested with local sashiko setup, new warnings for existing system >>> without good reason will be reported as regression, so it will be >>> blocker for net-next inclusion. >> >> Strong reverse centaur (*) warning on this one. >> >> I'm not sure this is how Mark want such "regressions" to be treated. >> >> I think a few dummy supplies is just fine and it's rather that Sashiko >> need to be taught not to treat that as bugs/regressions. >> >> Yours, >> Linus Walleij >> >> (*) Reverse centaur = humans touted to please AI, as opposed >> to AI helping humans, sarcastic term coined by author Cory Doctorow. > > Without context, I would fully agree with this statement. But as some > one who need to adders new warnings on every kernel update for our > customers, I see the point of sashiko - new unnecessary warning > is extra unnecessary work for some one else == regression. I also think we shouldn't dump 6 new warnings on other users if their setup is working ok. Cheers, Ahmad > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-07 7:49 ` Ahmad Fatoum @ 2026-08-07 8:20 ` Linus Walleij 2026-08-07 8:27 ` Ahmad Fatoum 0 siblings, 1 reply; 16+ messages in thread From: Linus Walleij @ 2026-08-07 8:20 UTC (permalink / raw) To: Ahmad Fatoum Cc: Oleksij Rempel, Mark Brown, Andrew Lunn, Conor Dooley, Vladimir Oltean, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Alvin Šipraga, Jakub Kicinski, Krzysztof Kozlowski, Paolo Abeni, Alvin Šipraga, David S. Miller, devicetree On Fri, Aug 7, 2026 at 9:49 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > >> (*) Reverse centaur = humans touted to please AI, as opposed > >> to AI helping humans, sarcastic term coined by author Cory Doctorow. > > > > Without context, I would fully agree with this statement. But as some > > one who need to adders new warnings on every kernel update for our > > customers, I see the point of sashiko - new unnecessary warning > > is extra unnecessary work for some one else == regression. > > I also think we shouldn't dump 6 new warnings on other users if their > setup is working ok. If you think the regulator subsystem is too talkative that is one thing. But there are millions of systems out there booting every day with dummy regulator prints that have been there since forever. New ones have been added over the years. It doesn't mean dummy regulator prints are regressions. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-07 8:20 ` Linus Walleij @ 2026-08-07 8:27 ` Ahmad Fatoum 2026-08-07 12:19 ` Mark Brown 0 siblings, 1 reply; 16+ messages in thread From: Ahmad Fatoum @ 2026-08-07 8:27 UTC (permalink / raw) To: Linus Walleij Cc: Oleksij Rempel, Mark Brown, Andrew Lunn, Conor Dooley, Vladimir Oltean, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Alvin Šipraga, Jakub Kicinski, Krzysztof Kozlowski, Paolo Abeni, Alvin Šipraga, David S. Miller, devicetree Hi, On 8/7/26 10:20 AM, Linus Walleij wrote: > On Fri, Aug 7, 2026 at 9:49 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > >>>> (*) Reverse centaur = humans touted to please AI, as opposed >>>> to AI helping humans, sarcastic term coined by author Cory Doctorow. >>> >>> Without context, I would fully agree with this statement. But as some >>> one who need to adders new warnings on every kernel update for our >>> customers, I see the point of sashiko - new unnecessary warning >>> is extra unnecessary work for some one else == regression. >> >> I also think we shouldn't dump 6 new warnings on other users if their >> setup is working ok. > > If you think the regulator subsystem is too talkative that is one thing. > > But there are millions of systems out there booting every day > with dummy regulator prints that have been there since forever. > New ones have been added over the years. > > It doesn't mean dummy regulator prints are regressions. I think it's a regression in user experience to have warnings pile up over the years when nothing is broken. Anyways, circling back to the patch: Any objections to leave it as-is and suppress the warnings in this particular driver? Thanks, Ahmad > > Yours, > Linus Walleij > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-07 8:27 ` Ahmad Fatoum @ 2026-08-07 12:19 ` Mark Brown 2026-08-07 14:05 ` Ahmad Fatoum 0 siblings, 1 reply; 16+ messages in thread From: Mark Brown @ 2026-08-07 12:19 UTC (permalink / raw) To: Ahmad Fatoum Cc: Linus Walleij, Oleksij Rempel, Andrew Lunn, Conor Dooley, Vladimir Oltean, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Alvin Šipraga, Jakub Kicinski, Krzysztof Kozlowski, Paolo Abeni, Alvin Šipraga, David S. Miller, devicetree [-- Attachment #1: Type: text/plain, Size: 577 bytes --] On Fri, Aug 07, 2026 at 10:27:31AM +0200, Ahmad Fatoum wrote: > Anyways, circling back to the patch: Any objections to leave it as-is > and suppress the warnings in this particular driver? Yes, that is a buggy use of regulator_get_optional() so you should not do that. Unless the supply may be physically absent it should not be used. If you have ideas for how to safely improve the implementation of the normal regulator_get() then it'd be better to do that, bearing in mind that it's an API where getting things wrong can easily result in physical damange to the system. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies 2026-08-07 12:19 ` Mark Brown @ 2026-08-07 14:05 ` Ahmad Fatoum 0 siblings, 0 replies; 16+ messages in thread From: Ahmad Fatoum @ 2026-08-07 14:05 UTC (permalink / raw) To: Linus Walleij, Oleksij Rempel, Andrew Lunn, Conor Dooley, Vladimir Oltean, Rob Herring, netdev, Luiz Angelo Daros de Luca, linux-kernel, Liam Girdwood, Eric Dumazet, kernel, Jakub Kicinski, Krzysztof Kozlowski, Paolo Abeni, Alvin Šipraga, David S. Miller, devicetree Hello Mark, On 8/7/26 2:19 PM, Mark Brown wrote: > On Fri, Aug 07, 2026 at 10:27:31AM +0200, Ahmad Fatoum wrote: > >> Anyways, circling back to the patch: Any objections to leave it as-is >> and suppress the warnings in this particular driver? > > Yes, that is a buggy use of regulator_get_optional() so you should not > do that. If the DT binding documents a regulator as optional, it makes no sense to me that the driver implementing the binding triggers a warning when the optional regulator is omitted. > Unless the supply may be physically absent it should not be > used. If you have ideas for how to safely improve the implementation of > the normal regulator_get() then it'd be better to do that, bearing in > mind that it's an API where getting things wrong can easily result in > physical damange to the system. How about demoting the dummy supply warning to notice? I think this better reflects the severity of the message. Cheers, Ahmad -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-08-07 14:05 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-06 14:02 [PATCH net-next v1 0/2] net: dsa: realtek: enable RTL8365MB power supplies Oleksij Rempel 2026-08-06 14:02 ` [PATCH net-next v1 1/2] dt-bindings: net: dsa: realtek: add " Oleksij Rempel 2026-08-06 14:02 ` [PATCH net-next v1 2/2] net: dsa: realtek: rtl83xx: add support for enabling supplies Oleksij Rempel 2026-08-06 14:12 ` Andrew Lunn 2026-08-06 15:10 ` Oleksij Rempel 2026-08-06 15:35 ` Andrew Lunn 2026-08-06 15:56 ` Ahmad Fatoum 2026-08-06 17:40 ` Linus Walleij 2026-08-07 4:26 ` Oleksij Rempel 2026-08-07 6:51 ` Linus Walleij 2026-08-07 7:10 ` Oleksij Rempel 2026-08-07 7:49 ` Ahmad Fatoum 2026-08-07 8:20 ` Linus Walleij 2026-08-07 8:27 ` Ahmad Fatoum 2026-08-07 12:19 ` Mark Brown 2026-08-07 14:05 ` Ahmad Fatoum
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox