Re: [PATCH net-next v1 2/3] net: phy: phy_device: Call into the PHY driver to set LED offload

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 21 Jun 2023 15:24:15 -0700 Jakub Kicinski wrote:
> On Mon, 19 Jun 2023 23:57:02 +0200 Andrew Lunn wrote:
> > +	/**
> > +	 * Can the HW support the given rules. Return 0 if yes,
> > +	 * -EOPNOTSUPP if not, or an error code.
> > +	 */
> > +	int (*led_hw_is_supported)(struct phy_device *dev, u8 index,
> > +				   unsigned long rules);
> > +	/**
> > +	 * Set the HW to control the LED as described by rules.
> > +	 */
> > +	int (*led_hw_control_set)(struct phy_device *dev, u8 index,
> > +				  unsigned long rules);
> > +	/**
> > +	 * Get the rules used to describe how the HW is currently
> > +	 * configure.
> > +	 */
> > +	int (*led_hw_control_get)(struct phy_device *dev, u8 index,
> > +				  unsigned long *rules);  
> 
> Why not include @led_hw_control_get in the kernel doc?
> IIUC the problem is that the value doesn't get rendered when building
> documentation correctly, but that should get resolved sooner or later.
> 
> OTOH what this patch adds is not valid kdoc at all, and it will never 
> be valid, right?

I can't repro the issue with per-member docs, BTW:

/**
 * struct abc - a grocery struct
 * @a: give me an A
 */
struct abc {
	/** @c: C is problematic? */
	void (*c)(void);
	/**
	 * @d: maybe D then?
	 */
	void (*d)(int arg, struct device *s);
	int a;
	/** @b: give me a B */
	int b;
};

/**
 * struct zabka_ops - another grocery struct
 */
struct zabka_ops {
	/** @c: C is problematic? */
	void (*c)(void);
	/**
	 * @d: maybe D then?
	 */
	void (*d)(int arg, struct device *s);
};


$ ./scripts/kernel-doc test.h

.. c:struct:: abc

  a grocery struct

.. container:: kernelindent

  **Definition**::

    struct abc {
        void (*c)(void);
        void (*d)(int arg, struct device *s);
        int a;
        int b;
    };

  **Members**

  ``c``
    C is problematic? 

  ``d``
    maybe D then?

  ``a``
    give me an A

  ``b``
    give me a B 


.. c:struct:: zabka_ops

  another grocery struct

.. container:: kernelindent

  **Definition**::

    struct zabka_ops {
        void (*c)(void);
        void (*d)(int arg, struct device *s);
    };

  **Members**

  ``c``
    C is problematic? 

  ``d``
    maybe D then?