Re: [PATCH phy-next 10/13] phy: lynx-28g: add support for big endian register maps

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Thu, May 28, 2026 at 05:47:57PM +0000, sashiko-bot@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@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.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy