[net-next] net: qcom/emac: fix a sizeof() typo
diff mbox

Message ID 20170213110021.GA17205@mwanda
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Dan Carpenter Feb. 13, 2017, 11 a.m. UTC
We had intended to say "sizeof(u32)" but the "u" is missing.
Fortunately, sizeof(32) is also 4, so the original code still works.

Fixes: c4e7beea2192 ("net: qcom/emac: add ethtool support for reading hardware registers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Comments

walter harms Feb. 13, 2017, 11:55 a.m. UTC | #1
Am 13.02.2017 12:00, schrieb Dan Carpenter:
> We had intended to say "sizeof(u32)" but the "u" is missing.
> Fortunately, sizeof(32) is also 4, so the original code still works.
> 
> Fixes: c4e7beea2192 ("net: qcom/emac: add ethtool support for reading hardware registers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> index 0d9945fb79be..bbe24639aa5a 100644
> --- a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> +++ b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> @@ -227,7 +227,7 @@ static void emac_get_regs(struct net_device *netdev,
>  
>  static int emac_get_regs_len(struct net_device *netdev)
>  {
> -	return EMAC_MAX_REG_SIZE * sizeof(32);
> +	return EMAC_MAX_REG_SIZE * sizeof(u32);
>  }
>  


We have a function where the argument is ignored and the rest is const ?

emac_ethtool_get_regs_len seems the only user. So it would be fairly easy to
move that into that function.

@maintainer:
Is there a deeper logic behind this ?


re,
 wh



>  static const struct ethtool_ops emac_ethtool_ops = {
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Timur Tabi Feb. 13, 2017, 1:03 p.m. UTC | #2
walter harms wrote:
> We have a function where the argument is ignored and the rest is const ?
>
> emac_ethtool_get_regs_len seems the only user. So it would be fairly easy to
> move that into that function.
>
> @maintainer:
> Is there a deeper logic behind this ?

I don't understand the question.

The patch is valid (I have no idea how I made that mistake).
walter harms Feb. 13, 2017, 1:28 p.m. UTC | #3
Am 13.02.2017 14:03, schrieb Timur Tabi:
> walter harms wrote:
>> We have a function where the argument is ignored and the rest is const ?
>>
>> emac_ethtool_get_regs_len seems the only user. So it would be fairly
>> easy to
>> move that into that function.
>>
>> @maintainer:
>> Is there a deeper logic behind this ?
> 
> I don't understand the question.


The question is: why is a simple calculation const*const
separated into a function ?

re,
 wh
Dan Carpenter Feb. 13, 2017, 2 p.m. UTC | #4
On Mon, Feb 13, 2017 at 12:55:03PM +0100, walter harms wrote:
> 
> 
> Am 13.02.2017 12:00, schrieb Dan Carpenter:
> > We had intended to say "sizeof(u32)" but the "u" is missing.
> > Fortunately, sizeof(32) is also 4, so the original code still works.
> > 
> > Fixes: c4e7beea2192 ("net: qcom/emac: add ethtool support for reading hardware registers")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> > index 0d9945fb79be..bbe24639aa5a 100644
> > --- a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> > +++ b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> > @@ -227,7 +227,7 @@ static void emac_get_regs(struct net_device *netdev,
> >  
> >  static int emac_get_regs_len(struct net_device *netdev)
> >  {
> > -	return EMAC_MAX_REG_SIZE * sizeof(32);
> > +	return EMAC_MAX_REG_SIZE * sizeof(u32);
> >  }
> >  
> 
> 
> We have a function where the argument is ignored and the rest is const ?
> 
> emac_ethtool_get_regs_len seems the only user. So it would be fairly easy to
> move that into that function.

It's not the only user.  We have to pass netdev because it's used as a
function pointer.

regards,
dan carpenter
Timur Tabi Feb. 13, 2017, 3:43 p.m. UTC | #5
walter harms wrote:
> The question is: why is a simple calculation const*const
> separated into a function ?

This is a callback function.  That's just how it's defined.

It's rare, but there are drivers that use the parameter, like this one:

http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c#n591
Timur Tabi Feb. 13, 2017, 3:43 p.m. UTC | #6
Dan Carpenter wrote:
> We had intended to say "sizeof(u32)" but the "u" is missing.
> Fortunately, sizeof(32) is also 4, so the original code still works.
>
> Fixes: c4e7beea2192 ("net: qcom/emac: add ethtool support for reading hardware registers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Timur Tabi <timur@codeaurora.org>
David Miller Feb. 14, 2017, 3:21 a.m. UTC | #7
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 13 Feb 2017 14:00:22 +0300

> We had intended to say "sizeof(u32)" but the "u" is missing.
> Fortunately, sizeof(32) is also 4, so the original code still works.
> 
> Fixes: c4e7beea2192 ("net: qcom/emac: add ethtool support for reading hardware registers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied.

Patch
diff mbox

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
index 0d9945fb79be..bbe24639aa5a 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
@@ -227,7 +227,7 @@  static void emac_get_regs(struct net_device *netdev,
 
 static int emac_get_regs_len(struct net_device *netdev)
 {
-	return EMAC_MAX_REG_SIZE * sizeof(32);
+	return EMAC_MAX_REG_SIZE * sizeof(u32);
 }
 
 static const struct ethtool_ops emac_ethtool_ops = {