[PATCH net-next] net: phylink: constify fwnode arguments

[PATCH net-next] net: phylink: constify fwnode arguments

[Russell King (Oracle)]

Both phylink_create() and phylink_fwnode_phy_connect() do not modify
the fwnode argument that they are passed, so lets constify these.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 11 ++++++-----
 include/linux/phylink.h   |  5 +++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a4111f1be375..cf53096047e6 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -708,7 +708,7 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
 }
 
 static int phylink_parse_fixedlink(struct phylink *pl,
-				   struct fwnode_handle *fwnode)
+				   const struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *fixed_node;
 	bool pause, asym_pause, autoneg;
@@ -819,7 +819,8 @@ static int phylink_parse_fixedlink(struct phylink *pl,
 	return 0;
 }
 
-static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
+static int phylink_parse_mode(struct phylink *pl,
+			      const struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *dn;
 	const char *managed;
@@ -1441,7 +1442,7 @@ static void phylink_fixed_poll(struct timer_list *t)
 static const struct sfp_upstream_ops sfp_phylink_ops;
 
 static int phylink_register_sfp(struct phylink *pl,
-				struct fwnode_handle *fwnode)
+				const struct fwnode_handle *fwnode)
 {
 	struct sfp_bus *bus;
 	int ret;
@@ -1480,7 +1481,7 @@ static int phylink_register_sfp(struct phylink *pl,
  * must use IS_ERR() to check for errors from this function.
  */
 struct phylink *phylink_create(struct phylink_config *config,
-			       struct fwnode_handle *fwnode,
+			       const struct fwnode_handle *fwnode,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *mac_ops)
 {
@@ -1809,7 +1810,7 @@ EXPORT_SYMBOL_GPL(phylink_of_phy_connect);
  * Returns 0 on success or a negative errno.
  */
 int phylink_fwnode_phy_connect(struct phylink *pl,
-			       struct fwnode_handle *fwnode,
+			       const struct fwnode_handle *fwnode,
 			       u32 flags)
 {
 	struct fwnode_handle *phy_fwnode;
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 71755c66c162..02c777ad18f2 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -568,7 +568,8 @@ void phylink_generic_validate(struct phylink_config *config,
 			      unsigned long *supported,
 			      struct phylink_link_state *state);
 
-struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
+struct phylink *phylink_create(struct phylink_config *,
+			       const struct fwnode_handle *,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *mac_ops);
 void phylink_destroy(struct phylink *);
@@ -577,7 +578,7 @@ bool phylink_expects_phy(struct phylink *pl);
 int phylink_connect_phy(struct phylink *, struct phy_device *);
 int phylink_of_phy_connect(struct phylink *, struct device_node *, u32 flags);
 int phylink_fwnode_phy_connect(struct phylink *pl,
-			       struct fwnode_handle *fwnode,
+			       const struct fwnode_handle *fwnode,
 			       u32 flags);
 void phylink_disconnect_phy(struct phylink *);
 
-- 
2.30.2

Re: [PATCH net-next] net: phylink: constify fwnode arguments

[Simon Horman]

On Wed, May 10, 2023 at 12:03:37PM +0100, Russell King (Oracle) wrote:
> Both phylink_create() and phylink_fwnode_phy_connect() do not modify
> the fwnode argument that they are passed, so lets constify these.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next] net: phylink: constify fwnode arguments

[Andy Shevchenko]

On Wed, May 10, 2023 at 2:03 PM Russell King (Oracle)
<rmk+kernel@armlinux.org.uk> wrote:
>
> Both phylink_create() and phylink_fwnode_phy_connect() do not modify
> the fwnode argument that they are passed, so lets constify these.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you!

> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/phy/phylink.c | 11 ++++++-----
>  include/linux/phylink.h   |  5 +++--
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index a4111f1be375..cf53096047e6 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -708,7 +708,7 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
>  }
>
>  static int phylink_parse_fixedlink(struct phylink *pl,
> -                                  struct fwnode_handle *fwnode)
> +                                  const struct fwnode_handle *fwnode)
>  {
>         struct fwnode_handle *fixed_node;
>         bool pause, asym_pause, autoneg;
> @@ -819,7 +819,8 @@ static int phylink_parse_fixedlink(struct phylink *pl,
>         return 0;
>  }
>
> -static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
> +static int phylink_parse_mode(struct phylink *pl,
> +                             const struct fwnode_handle *fwnode)
>  {
>         struct fwnode_handle *dn;
>         const char *managed;
> @@ -1441,7 +1442,7 @@ static void phylink_fixed_poll(struct timer_list *t)
>  static const struct sfp_upstream_ops sfp_phylink_ops;
>
>  static int phylink_register_sfp(struct phylink *pl,
> -                               struct fwnode_handle *fwnode)
> +                               const struct fwnode_handle *fwnode)
>  {
>         struct sfp_bus *bus;
>         int ret;
> @@ -1480,7 +1481,7 @@ static int phylink_register_sfp(struct phylink *pl,
>   * must use IS_ERR() to check for errors from this function.
>   */
>  struct phylink *phylink_create(struct phylink_config *config,
> -                              struct fwnode_handle *fwnode,
> +                              const struct fwnode_handle *fwnode,
>                                phy_interface_t iface,
>                                const struct phylink_mac_ops *mac_ops)
>  {
> @@ -1809,7 +1810,7 @@ EXPORT_SYMBOL_GPL(phylink_of_phy_connect);
>   * Returns 0 on success or a negative errno.
>   */
>  int phylink_fwnode_phy_connect(struct phylink *pl,
> -                              struct fwnode_handle *fwnode,
> +                              const struct fwnode_handle *fwnode,
>                                u32 flags)
>  {
>         struct fwnode_handle *phy_fwnode;
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index 71755c66c162..02c777ad18f2 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -568,7 +568,8 @@ void phylink_generic_validate(struct phylink_config *config,
>                               unsigned long *supported,
>                               struct phylink_link_state *state);
>
> -struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
> +struct phylink *phylink_create(struct phylink_config *,
> +                              const struct fwnode_handle *,
>                                phy_interface_t iface,
>                                const struct phylink_mac_ops *mac_ops);
>  void phylink_destroy(struct phylink *);
> @@ -577,7 +578,7 @@ bool phylink_expects_phy(struct phylink *pl);
>  int phylink_connect_phy(struct phylink *, struct phy_device *);
>  int phylink_of_phy_connect(struct phylink *, struct device_node *, u32 flags);
>  int phylink_fwnode_phy_connect(struct phylink *pl,
> -                              struct fwnode_handle *fwnode,
> +                              const struct fwnode_handle *fwnode,
>                                u32 flags);
>  void phylink_disconnect_phy(struct phylink *);
>
> --
> 2.30.2
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next] net: phylink: constify fwnode arguments

[Paolo Abeni]

On Wed, 2023-05-10 at 12:03 +0100, Russell King (Oracle) wrote:
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index 71755c66c162..02c777ad18f2 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -568,7 +568,8 @@ void phylink_generic_validate(struct phylink_config *config,
>  			      unsigned long *supported,
>  			      struct phylink_link_state *state);
>  
> -struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
> +struct phylink *phylink_create(struct phylink_config *,
> +			       const struct fwnode_handle *,

While touching the above, could you please also add the missing params
name, to keep checkpatch happy and be consistent with the others
arguments?

Thanks!

Paolo

Re: [PATCH net-next] net: phylink: constify fwnode arguments

[Russell King (Oracle)]

On Thu, May 11, 2023 at 01:29:50PM +0200, Paolo Abeni wrote:
> On Wed, 2023-05-10 at 12:03 +0100, Russell King (Oracle) wrote:
> > diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> > index 71755c66c162..02c777ad18f2 100644
> > --- a/include/linux/phylink.h
> > +++ b/include/linux/phylink.h
> > @@ -568,7 +568,8 @@ void phylink_generic_validate(struct phylink_config *config,
> >  			      unsigned long *supported,
> >  			      struct phylink_link_state *state);
> >  
> > -struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
> > +struct phylink *phylink_create(struct phylink_config *,
> > +			       const struct fwnode_handle *,
> 
> While touching the above, could you please also add the missing params
> name, to keep checkpatch happy and be consistent with the others
> arguments?

For interest, when did naming parameters in a prototype become a
requirement?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: phylink: constify fwnode arguments

[Paolo Abeni]

On Thu, 2023-05-11 at 12:32 +0100, Russell King (Oracle) wrote:
> On Thu, May 11, 2023 at 01:29:50PM +0200, Paolo Abeni wrote:
> > On Wed, 2023-05-10 at 12:03 +0100, Russell King (Oracle) wrote:
> > > diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> > > index 71755c66c162..02c777ad18f2 100644
> > > --- a/include/linux/phylink.h
> > > +++ b/include/linux/phylink.h
> > > @@ -568,7 +568,8 @@ void phylink_generic_validate(struct phylink_config *config,
> > >  			      unsigned long *supported,
> > >  			      struct phylink_link_state *state);
> > >  
> > > -struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
> > > +struct phylink *phylink_create(struct phylink_config *,
> > > +			       const struct fwnode_handle *,
> > 
> > While touching the above, could you please also add the missing params
> > name, to keep checkpatch happy and be consistent with the others
> > arguments?
> 
> For interest, when did naming parameters in a prototype become a
> requirement?

I would not call it a general requirement, but in this specific case we
have 2 named params and 2 unnamed ones for the same function, which
looks not good to me. Since you are touching that function definition
and checkpatch is complaining about the above, I think it would be
better to make the function declaration self-consistent.

Looking again at the checkpatch warning, that is possibly a false
positive - git history hints such check should apply only to function
definition, not declaration. 

I still think it would be better removing the mixed unnamed/named
params usage.

Thanks,

Paolo

Re: [PATCH net-next] net: phylink: constify fwnode arguments

[Russell King (Oracle)]

On Thu, May 11, 2023 at 03:47:13PM +0200, Paolo Abeni wrote:
> On Thu, 2023-05-11 at 12:32 +0100, Russell King (Oracle) wrote:
> > On Thu, May 11, 2023 at 01:29:50PM +0200, Paolo Abeni wrote:
> > > On Wed, 2023-05-10 at 12:03 +0100, Russell King (Oracle) wrote:
> > > > diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> > > > index 71755c66c162..02c777ad18f2 100644
> > > > --- a/include/linux/phylink.h
> > > > +++ b/include/linux/phylink.h
> > > > @@ -568,7 +568,8 @@ void phylink_generic_validate(struct phylink_config *config,
> > > >  			      unsigned long *supported,
> > > >  			      struct phylink_link_state *state);
> > > >  
> > > > -struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
> > > > +struct phylink *phylink_create(struct phylink_config *,
> > > > +			       const struct fwnode_handle *,
> > > 
> > > While touching the above, could you please also add the missing params
> > > name, to keep checkpatch happy and be consistent with the others
> > > arguments?
> > 
> > For interest, when did naming parameters in a prototype become a
> > requirement?
> 
> I would not call it a general requirement, but in this specific case we
> have 2 named params and 2 unnamed ones for the same function, which
> looks not good to me. Since you are touching that function definition
> and checkpatch is complaining about the above, I think it would be
> better to make the function declaration self-consistent.
> 
> Looking again at the checkpatch warning, that is possibly a false
> positive - git history hints such check should apply only to function
> definition, not declaration. 
> 
> I still think it would be better removing the mixed unnamed/named
> params usage.

In this particular instance, I think removing them is appropriate,
since giving names for them doesn't contribute anything useful
(since the types give allt he information necessary.)

However, for something like a function that takes e.g. a u32 and a
bunch of other unique structs, I think it is entirely appropriate
to use mixed named/unnamed parameters - since a "u32" can't describe
on its own what it is. Hence, I think, e.g.:

int phylink_of_phy_connect(struct phylink *, struct device_node *, u32 flags);

is entirely reasonable, since "flags" describes what the u32 is.
Adding "pl" and "dn" to the other two arguments doesn't add any
useful value.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH net-next] net: phylink: constify fwnode arguments

[Russell King (Oracle)]

Both phylink_create() and phylink_fwnode_phy_connect() do not modify
the fwnode argument that they are passed, so lets constify these.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 11 ++++++-----
 include/linux/phylink.h   |  9 +++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a4111f1be375..cf53096047e6 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -708,7 +708,7 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
 }
 
 static int phylink_parse_fixedlink(struct phylink *pl,
-				   struct fwnode_handle *fwnode)
+				   const struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *fixed_node;
 	bool pause, asym_pause, autoneg;
@@ -819,7 +819,8 @@ static int phylink_parse_fixedlink(struct phylink *pl,
 	return 0;
 }
 
-static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
+static int phylink_parse_mode(struct phylink *pl,
+			      const struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *dn;
 	const char *managed;
@@ -1441,7 +1442,7 @@ static void phylink_fixed_poll(struct timer_list *t)
 static const struct sfp_upstream_ops sfp_phylink_ops;
 
 static int phylink_register_sfp(struct phylink *pl,
-				struct fwnode_handle *fwnode)
+				const struct fwnode_handle *fwnode)
 {
 	struct sfp_bus *bus;
 	int ret;
@@ -1480,7 +1481,7 @@ static int phylink_register_sfp(struct phylink *pl,
  * must use IS_ERR() to check for errors from this function.
  */
 struct phylink *phylink_create(struct phylink_config *config,
-			       struct fwnode_handle *fwnode,
+			       const struct fwnode_handle *fwnode,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *mac_ops)
 {
@@ -1809,7 +1810,7 @@ EXPORT_SYMBOL_GPL(phylink_of_phy_connect);
  * Returns 0 on success or a negative errno.
  */
 int phylink_fwnode_phy_connect(struct phylink *pl,
-			       struct fwnode_handle *fwnode,
+			       const struct fwnode_handle *fwnode,
 			       u32 flags)
 {
 	struct fwnode_handle *phy_fwnode;
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 71755c66c162..bb782f05ad08 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -568,16 +568,17 @@ void phylink_generic_validate(struct phylink_config *config,
 			      unsigned long *supported,
 			      struct phylink_link_state *state);
 
-struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
-			       phy_interface_t iface,
-			       const struct phylink_mac_ops *mac_ops);
+struct phylink *phylink_create(struct phylink_config *,
+			       const struct fwnode_handle *,
+			       phy_interface_t,
+			       const struct phylink_mac_ops *);
 void phylink_destroy(struct phylink *);
 bool phylink_expects_phy(struct phylink *pl);
 
 int phylink_connect_phy(struct phylink *, struct phy_device *);
 int phylink_of_phy_connect(struct phylink *, struct device_node *, u32 flags);
 int phylink_fwnode_phy_connect(struct phylink *pl,
-			       struct fwnode_handle *fwnode,
+			       const struct fwnode_handle *fwnode,
 			       u32 flags);
 void phylink_disconnect_phy(struct phylink *);
 
-- 
2.30.2

Re: [PATCH net-next] net: phylink: constify fwnode arguments

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 12 May 2023 17:58:37 +0100 you wrote:
> Both phylink_create() and phylink_fwnode_phy_connect() do not modify
> the fwnode argument that they are passed, so lets constify these.
> 
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/phy/phylink.c | 11 ++++++-----
>  include/linux/phylink.h   |  9 +++++----
>  2 files changed, 11 insertions(+), 9 deletions(-)

Here is the summary with links:
  - [net-next] net: phylink: constify fwnode arguments
    https://git.kernel.org/netdev/net-next/c/a0b7955310a4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html