On Sun, Jun 21, 2020 at 05:42:48PM +0200, Andrew Lunn wrote:
> FYI:
>
> Andrew
Thanks for forwarding - Colton, please copy me directly.
>
> ----- Forwarded message from Colton Lewis <colton.w.le...@protonmail.com>
> -----
>
> Date: Sun, 21 Jun 2020 02:23:04 +0000
> From: Colton Lewis <colton.w.le...@protonmail.com>
> To: da...@davemloft.net
> Cc: netdev@vger.kernel.org, Colton Lewis <colton.w.le...@protonmail.com>
> Subject: [PATCH 3/3] net: phylink: correct trivial kernel-doc inconsistencies
> X-Spam-Status: No, score=-1.0 required=4.0 tests=BAYES_00,DKIM_SIGNED,
> DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,
> HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS,
> SPF_PASS autolearn=no autolearn_force=no
> version=3.4.4
>
> Silence documentation build warnings by correcting kernel-doc
> comments. In the case of pcs_{config,an_restart,link_up}, change the
> declaration to a normal function since these only there for
> documentation anyway.
>
> ./include/linux/phylink.h:74: warning: Function parameter or member
> 'poll_fixed_state' not described in 'phylink_config'
> ./include/linux/phylink.h:74: warning: Function parameter or member
> 'get_fixed_state' not described in 'phylink_config'
> ./include/linux/phylink.h:336: warning: Function parameter or member
> 'pcs_config' not described in 'int'
> ./include/linux/phylink.h:336: warning: Excess function parameter 'config'
> description in 'int'
> ./include/linux/phylink.h:336: warning: Excess function parameter 'mode'
> description in 'int'
> ./include/linux/phylink.h:336: warning: Excess function parameter 'interface'
> description in 'int'
> ./include/linux/phylink.h:336: warning: Excess function parameter
> 'advertising' description in 'int'
> ./include/linux/phylink.h:345: warning: Function parameter or member
> 'pcs_an_restart' not described in 'void'
> ./include/linux/phylink.h:345: warning: Excess function parameter 'config'
> description in 'void'
> ./include/linux/phylink.h:361: warning: Function parameter or member
> 'pcs_link_up' not described in 'void'
> ./include/linux/phylink.h:361: warning: Excess function parameter 'config'
> description in 'void'
> ./include/linux/phylink.h:361: warning: Excess function parameter 'mode'
> description in 'void'
> ./include/linux/phylink.h:361: warning: Excess function parameter 'interface'
> description in 'void'
> ./include/linux/phylink.h:361: warning: Excess function parameter 'speed'
> description in 'void'
> ./include/linux/phylink.h:361: warning: Excess function parameter 'duplex'
> description in 'void'
Not all of those are valid complaints against the code.
> Signed-off-by: Colton Lewis <colton.w.le...@protonmail.com>
> ---
> include/linux/phylink.h | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index cc5b452a184e..cb3230590a1f 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -62,6 +62,8 @@ enum phylink_op_type {
> * @dev: a pointer to a struct device associated with the MAC
> * @type: operation type of PHYLINK instance
> * @pcs_poll: MAC PCS cannot provide link change interrupt
> + * @poll_fixed_state: poll link state with @get_fixed_state
> + * @get_fixed_state: read link state into struct phylink_link_state
Ack for this change.
> */
> struct phylink_config {
> struct device *dev;
> @@ -331,7 +333,7 @@ void pcs_get_state(struct phylink_config *config,
> *
> * For most 10GBASE-R, there is no advertisement.
> */
> -int (*pcs_config)(struct phylink_config *config, unsigned int mode,
> +int *pcs_config(struct phylink_config *config, unsigned int mode,
> phy_interface_t interface, const unsigned long *advertising);
*Definitely* a NAK on this and two changes below. You're changing the
function signature to be incorrect. If the documentation can't parse
a legitimate C function pointer declaration and allow it to be
documented, then that's a problem with the documentation's parsing of
C code, rather than a problem with the C code itself.
>
> /**
> @@ -341,7 +343,7 @@ int (*pcs_config)(struct phylink_config *config, unsigned
> int mode,
> * When PCS ops are present, this overrides mac_an_restart() in &struct
> * phylink_mac_ops.
> */
> -void (*pcs_an_restart)(struct phylink_config *config);
> +void *pcs_an_restart(struct phylink_config *config);
>
> /**
> * pcs_link_up() - program the PCS for the resolved link configuration
> @@ -356,7 +358,7 @@ void (*pcs_an_restart)(struct phylink_config *config);
> * mode without in-band AN needs to be manually configured for the link
> * and duplex setting. Otherwise, this should be a no-op.
> */
> -void (*pcs_link_up)(struct phylink_config *config, unsigned int mode,
> +void *pcs_link_up(struct phylink_config *config, unsigned int mode,
> phy_interface_t interface, int speed, int duplex);
> #endif
>
> --
> 2.26.2
>
>
>
> ----- End forwarded message -----
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
