Consumer drivers shouldn't dereference struct phy, not even to get to
its attributes.
We have phy_get_bus_width() as a precedent for getting the bus_width
attribute, so let's add phy_get_max_link_rate() and use it in DRM and
CAN drivers.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Vincent Mailhol <mailhol@kernel.org>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
Cc: Markus Schneider-Pargmann <msp@baylibre.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Magnus Damm <magnus.damm@gmail.com>
---
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++--
drivers/gpu/drm/bridge/synopsys/dw-dp.c | 2 +-
drivers/net/can/at91_can.c | 2 +-
drivers/net/can/flexcan/flexcan-core.c | 2 +-
drivers/net/can/m_can/m_can_platform.c | 2 +-
drivers/net/can/rcar/rcar_canfd.c | 2 +-
drivers/phy/phy-core.c | 6 ++++++
include/linux/phy/phy.h | 6 ++++++
8 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index a8b6ae58cb0a..ed7ed82ddb64 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1300,7 +1300,7 @@ static u32 cdns_mhdp_get_training_interval_us(struct cdns_mhdp_device *mhdp,
static void cdns_mhdp_fill_host_caps(struct cdns_mhdp_device *mhdp)
{
- unsigned int link_rate;
+ u32 link_rate;
/* Get source capabilities based on PHY attributes */
@@ -1308,7 +1308,7 @@ static void cdns_mhdp_fill_host_caps(struct cdns_mhdp_device *mhdp)
if (!mhdp->host.lanes_cnt)
mhdp->host.lanes_cnt = 4;
- link_rate = mhdp->phy->attrs.max_link_rate;
+ link_rate = phy_get_max_link_rate(mhdp->phy);
if (!link_rate)
link_rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_8_1);
else
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
index 4ab6922dd79c..79c72ee8e263 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
@@ -536,7 +536,7 @@ static int dw_dp_link_parse(struct dw_dp *dp, struct drm_connector *connector)
link->revision = link->dpcd[DP_DPCD_REV];
link->rate = min_t(u32, min(dp->plat_data.max_link_rate,
- dp->phy->attrs.max_link_rate * 100),
+ phy_get_max_link_rate(dp->phy) * 100),
drm_dp_max_link_rate(link->dpcd));
link->lanes = min_t(u8, phy_get_bus_width(dp->phy),
drm_dp_max_lane_count(link->dpcd));
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index 58da323f14d7..b56db253f02d 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -1126,7 +1126,7 @@ static int at91_can_probe(struct platform_device *pdev)
can_rx_offload_add_timestamp(dev, &priv->offload);
if (transceiver)
- priv->can.bitrate_max = transceiver->attrs.max_link_rate;
+ priv->can.bitrate_max = phy_get_max_link_rate(transceiver);
if (at91_is_sam9263(priv))
dev->sysfs_groups[0] = &at91_sysfs_attr_group;
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index f5d22c61503f..3a4307bc1d61 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -2211,7 +2211,7 @@ static int flexcan_probe(struct platform_device *pdev)
priv->transceiver = transceiver;
if (transceiver)
- priv->can.bitrate_max = transceiver->attrs.max_link_rate;
+ priv->can.bitrate_max = phy_get_max_link_rate(transceiver);
if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
priv->irq_boff = platform_get_irq(pdev, 1);
diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index 56da411878af..73525be6566b 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -132,7 +132,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
}
if (transceiver)
- mcan_class->can.bitrate_max = transceiver->attrs.max_link_rate;
+ mcan_class->can.bitrate_max = phy_get_max_link_rate(transceiver);
priv->base = addr;
priv->mram_base = mram_addr;
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index eaf8cac78038..645d5671705d 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1885,7 +1885,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
priv->channel = ch;
priv->gpriv = gpriv;
if (transceiver)
- priv->can.bitrate_max = transceiver->attrs.max_link_rate;
+ priv->can.bitrate_max = phy_get_max_link_rate(transceiver);
priv->can.clock.freq = fcan_freq;
dev_info(dev, "can_clk rate is %u\n", priv->can.clock.freq);
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index a1aff00fba7c..89f7410241aa 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -640,6 +640,12 @@ void phy_set_bus_width(struct phy *phy, int bus_width)
}
EXPORT_SYMBOL_GPL(phy_set_bus_width);
+u32 phy_get_max_link_rate(struct phy *phy)
+{
+ return phy->attrs.max_link_rate;
+}
+EXPORT_SYMBOL_GPL(phy_get_max_link_rate);
+
/**
* _of_phy_get() - lookup and obtain a reference to a phy by phandle
* @np: device_node for which to get the phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 36307e47760d..af9c3e795786 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -57,6 +57,7 @@ int phy_notify_disconnect(struct phy *phy, int port);
int phy_notify_state(struct phy *phy, union phy_notify state);
int phy_get_bus_width(struct phy *phy);
void phy_set_bus_width(struct phy *phy, int bus_width);
+u32 phy_get_max_link_rate(struct phy *phy);
#else
static inline struct phy *phy_get(struct device *dev, const char *string)
{
@@ -256,6 +257,11 @@ static inline int phy_get_bus_width(struct phy *phy)
static inline void phy_set_bus_width(struct phy *phy, int bus_width)
{
}
+
+static inline u32 phy_get_max_link_rate(struct phy *phy)
+{
+ return 0;
+}
#endif /* IS_ENABLED(CONFIG_GENERIC_PHY) */
#endif /* __PHY_CONSUMER_H */
--
2.43.0Hi,
On Wed Mar 4, 2026 at 6:57 PM CET, Vladimir Oltean wrote:
> Consumer drivers shouldn't dereference struct phy, not even to get to
> its attributes.
>
> We have phy_get_bus_width() as a precedent for getting the bus_width
> attribute, so let's add phy_get_max_link_rate() and use it in DRM and
> CAN drivers.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Andy Yan <andy.yan@rock-chips.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Vincent Mailhol <mailhol@kernel.org>
> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Cc: Markus Schneider-Pargmann <msp@baylibre.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> ---
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++--
> drivers/gpu/drm/bridge/synopsys/dw-dp.c | 2 +-
> drivers/net/can/at91_can.c | 2 +-
> drivers/net/can/flexcan/flexcan-core.c | 2 +-
> drivers/net/can/m_can/m_can_platform.c | 2 +-
> drivers/net/can/rcar/rcar_canfd.c | 2 +-
> drivers/phy/phy-core.c | 6 ++++++
> include/linux/phy/phy.h | 6 ++++++
> 8 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index a8b6ae58cb0a..ed7ed82ddb64 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -1300,7 +1300,7 @@ static u32 cdns_mhdp_get_training_interval_us(struct cdns_mhdp_device *mhdp,
>
> static void cdns_mhdp_fill_host_caps(struct cdns_mhdp_device *mhdp)
> {
> - unsigned int link_rate;
> + u32 link_rate;
>
> /* Get source capabilities based on PHY attributes */
>
> @@ -1308,7 +1308,7 @@ static void cdns_mhdp_fill_host_caps(struct cdns_mhdp_device *mhdp)
> if (!mhdp->host.lanes_cnt)
> mhdp->host.lanes_cnt = 4;
>
> - link_rate = mhdp->phy->attrs.max_link_rate;
> + link_rate = phy_get_max_link_rate(mhdp->phy);
> if (!link_rate)
> link_rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_8_1);
> else
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 4ab6922dd79c..79c72ee8e263 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -536,7 +536,7 @@ static int dw_dp_link_parse(struct dw_dp *dp, struct drm_connector *connector)
>
> link->revision = link->dpcd[DP_DPCD_REV];
> link->rate = min_t(u32, min(dp->plat_data.max_link_rate,
> - dp->phy->attrs.max_link_rate * 100),
> + phy_get_max_link_rate(dp->phy) * 100),
> drm_dp_max_link_rate(link->dpcd));
> link->lanes = min_t(u8, phy_get_bus_width(dp->phy),
> drm_dp_max_lane_count(link->dpcd));
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index 58da323f14d7..b56db253f02d 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -1126,7 +1126,7 @@ static int at91_can_probe(struct platform_device *pdev)
> can_rx_offload_add_timestamp(dev, &priv->offload);
>
> if (transceiver)
> - priv->can.bitrate_max = transceiver->attrs.max_link_rate;
> + priv->can.bitrate_max = phy_get_max_link_rate(transceiver);
>
> if (at91_is_sam9263(priv))
> dev->sysfs_groups[0] = &at91_sysfs_attr_group;
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f5d22c61503f..3a4307bc1d61 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -2211,7 +2211,7 @@ static int flexcan_probe(struct platform_device *pdev)
> priv->transceiver = transceiver;
>
> if (transceiver)
> - priv->can.bitrate_max = transceiver->attrs.max_link_rate;
> + priv->can.bitrate_max = phy_get_max_link_rate(transceiver);
>
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
> priv->irq_boff = platform_get_irq(pdev, 1);
> diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
> index 56da411878af..73525be6566b 100644
> --- a/drivers/net/can/m_can/m_can_platform.c
> +++ b/drivers/net/can/m_can/m_can_platform.c
> @@ -132,7 +132,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
> }
>
> if (transceiver)
> - mcan_class->can.bitrate_max = transceiver->attrs.max_link_rate;
> + mcan_class->can.bitrate_max = phy_get_max_link_rate(transceiver);
>
> priv->base = addr;
> priv->mram_base = mram_addr;
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index eaf8cac78038..645d5671705d 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1885,7 +1885,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
> priv->channel = ch;
> priv->gpriv = gpriv;
> if (transceiver)
> - priv->can.bitrate_max = transceiver->attrs.max_link_rate;
> + priv->can.bitrate_max = phy_get_max_link_rate(transceiver);
> priv->can.clock.freq = fcan_freq;
> dev_info(dev, "can_clk rate is %u\n", priv->can.clock.freq);
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index a1aff00fba7c..89f7410241aa 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -640,6 +640,12 @@ void phy_set_bus_width(struct phy *phy, int bus_width)
> }
> EXPORT_SYMBOL_GPL(phy_set_bus_width);
>
> +u32 phy_get_max_link_rate(struct phy *phy)
> +{
All of the can drivers that would use this function are checking phy
before assigning the max_link_rate:
if (transceiver)
priv->can.bitrate_max = transceiver->attrs.max_link_rate;
Would it be reasonable to have
if (!phy)
return 0;
in this function to be able to drop these individual checks of the
drivers? This would be similar to clk_get_rate() which does the same
check and return 0 for convenience.
Best
Markus
> + return phy->attrs.max_link_rate;
> +}
> +EXPORT_SYMBOL_GPL(phy_get_max_link_rate);
> +
> /**
> * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> * @np: device_node for which to get the phy
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 36307e47760d..af9c3e795786 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -57,6 +57,7 @@ int phy_notify_disconnect(struct phy *phy, int port);
> int phy_notify_state(struct phy *phy, union phy_notify state);
> int phy_get_bus_width(struct phy *phy);
> void phy_set_bus_width(struct phy *phy, int bus_width);
> +u32 phy_get_max_link_rate(struct phy *phy);
> #else
> static inline struct phy *phy_get(struct device *dev, const char *string)
> {
> @@ -256,6 +257,11 @@ static inline int phy_get_bus_width(struct phy *phy)
> static inline void phy_set_bus_width(struct phy *phy, int bus_width)
> {
> }
> +
> +static inline u32 phy_get_max_link_rate(struct phy *phy)
> +{
> + return 0;
> +}
> #endif /* IS_ENABLED(CONFIG_GENERIC_PHY) */
>
> #endif /* __PHY_CONSUMER_H */
On Thu, Mar 05, 2026 at 10:36:14AM +0100, Markus Schneider-Pargmann wrote: > All of the can drivers that would use this function are checking phy > before assigning the max_link_rate: > > if (transceiver) > priv->can.bitrate_max = transceiver->attrs.max_link_rate; > > Would it be reasonable to have > > if (!phy) > return 0; > > in this function to be able to drop these individual checks of the > drivers? This would be similar to clk_get_rate() which does the same > check and return 0 for convenience. > > Best > Markus Thanks, that's a good point. The transceiver is acquired through devm_phy_optional_get() and NULL is given by the API as a non-error case, so I guess it means it should also tolerate NULL coming back to it. This just leaves an inconsistency with phy_(get|set)_bus_width() which are not NULL-tolerant but should also be. I'll leave those as is for now, since I don't want to make the series any larger than it is, but I'll update the new API with your suggestion.
Hi Vladimir,
On Wed, 4 Mar 2026 at 19:00, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> Consumer drivers shouldn't dereference struct phy, not even to get to
> its attributes.
>
> We have phy_get_bus_width() as a precedent for getting the bus_width
> attribute, so let's add phy_get_max_link_rate() and use it in DRM and
> CAN drivers.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Thanks for your patch!
> drivers/net/can/rcar/rcar_canfd.c | 2 +-
For the Renesas part:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -640,6 +640,12 @@ void phy_set_bus_width(struct phy *phy, int bus_width)
> }
> EXPORT_SYMBOL_GPL(phy_set_bus_width);
>
> +u32 phy_get_max_link_rate(struct phy *phy)
> +{
> + return phy->attrs.max_link_rate;
> +}
> +EXPORT_SYMBOL_GPL(phy_get_max_link_rate);
Any specific reason you are not making this a simple static inline
function, like phy_get_bus_width()?
> +
> /**
> * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> * @np: device_node for which to get the phy
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Thu, Mar 05, 2026 at 08:47:47AM +0100, Geert Uytterhoeven wrote:
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -640,6 +640,12 @@ void phy_set_bus_width(struct phy *phy, int bus_width)
> > }
> > EXPORT_SYMBOL_GPL(phy_set_bus_width);
> >
> > +u32 phy_get_max_link_rate(struct phy *phy)
> > +{
> > + return phy->attrs.max_link_rate;
> > +}
> > +EXPORT_SYMBOL_GPL(phy_get_max_link_rate);
>
> Any specific reason you are not making this a simple static inline
> function, like phy_get_bus_width()?
For a consumer function to be static inline and to dereference struct
phy fields, it would mean that the struct phy contents need to be
visible to the consumer directly. Against my stated purpose.
phy_get_bus_width() was also changed to be non-inline.
© 2016 - 2026 Red Hat, Inc.