[PATCH phy-next 10/13] phy: lynx-28g: add support for big endian register maps
Vladimir Oltean
vladimir.oltean at nxp.com
Fri May 29 05:00:05 PDT 2026
On Thu, May 28, 2026 at 05:47:57PM +0000, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Returning an expression in a function with a 'void' return type is a constraint violation in standard C.
> --
>
> commit f3cbe0a1122ada7c6725f37e9dc47932febfd4ad
> Author: Vladimir Oltean <vladimir.oltean at nxp.com>
>
> phy: lynx-28g: add support for big endian register maps
>
> This commit adds support for big-endian 10G Lynx SerDes blocks. It reads
> the "big-endian" device tree property and updates the lynx_read() and
> lynx_write() accessors to use the appropriate ioread32be/iowrite32be or
> ioread32/iowrite32 variants based on the endianness.
>
> > diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
> > index e8b280cc9b38c..d82e529fa65a6 100644
> > --- a/drivers/phy/freescale/phy-fsl-lynx-core.h
> > +++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
>
> [ ... ]
>
> > @@ -58,36 +58,52 @@ struct lynx_priv {
> > * like PCCn
> > */
> > spinlock_t pcc_lock;
> > + bool big_endian;
> > struct lynx_pll pll[LYNX_NUM_PLL];
> > struct lynx_lane *lane;
> >
> > struct delayed_work cdr_check;
> > };
> >
> > +static inline u32 lynx_read(struct lynx_priv *priv, unsigned long off)
> > +{
> > + void __iomem *reg = priv->base + off;
> > +
> > + if (priv->big_endian)
> > + return ioread32be(reg);
> > +
> > + return ioread32(reg);
> > +}
> > +
> > +static inline void lynx_write(struct lynx_priv *priv, unsigned long off, u32 val)
> > +{
> > + void __iomem *reg = priv->base + off;
> > +
> > + if (priv->big_endian)
> > + return iowrite32be(val, reg);
>
> [Severity: Low]
> Does this code unintentionally return a void expression?
>
> The lynx_write() function is declared with a void return type, but it uses
> the return keyword before calling iowrite32be() and iowrite32() which also
> evaluate to void.
>
> While GNU C allows returning a void expression as an extension, standard C
> does not permit it, and it could trigger warnings in strict static analysis
> tools. Would it be better to just drop the return keywords here?
>
> > +
> > + return iowrite32(val, reg);
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260528172404.733196-1-vladimir.oltean@nxp.com?part=10
This was intentional, because "return iowrite32be()" also stops the control flow
and is more succinct than "{ iowrite32be(); return; }".
I thought the pattern was widely accepted, see include/linux/skbuff.h:
static inline void consume_skb(struct sk_buff *skb)
{
return kfree_skb(skb);
}
Unless a human has a valid objection, I will disregard this feedback.
More information about the linux-phy
mailing list