Re: [PATCH net-next 09/12] net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands

2017-03-14 Thread Doug Berger 
On 03/14/2017 04:04 AM, David Laight wrote:
> From: Doug Berger
>> Sent: 14 March 2017 00:42
>> This commit changes the ioctl handling behavior to return the
>> EOPNOTSUPP error code instead of the EINVAL error code when an
>> unknown ioctl command value is detected.
>>
>> It also removes some redundant parsing of the ioctl command value
>> and allows the SIOCSHWTSTAMP value to be handled.
> 
> A better description would seem to be:
> Remove checks on ioctl command and just forward all ioctl requests
> to phy_mii_ioctl().
That is a good description of the code change, but I felt that was
clearly conveyed by the patch content.  I thought it would be a better
use of the comment to describe the more subtle functional change that
might be less clear.

> 
> I also thought the 'generic' response to an unknown ioctl command
> was ENOTTY.
and I think it probably helped solicit this feedback :).  I would have
thought that error makes more sense if there is no ioctl handler, but I
will definitely look into it.

Thanks for the feedback,
Doug

RE: [PATCH net-next 09/12] net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands

2017-03-14 Thread David Laight 
From: Doug Berger
> Sent: 14 March 2017 00:42
> This commit changes the ioctl handling behavior to return the
> EOPNOTSUPP error code instead of the EINVAL error code when an
> unknown ioctl command value is detected.
> 
> It also removes some redundant parsing of the ioctl command value
> and allows the SIOCSHWTSTAMP value to be handled.

A better description would seem to be:
Remove checks on ioctl command and just forward all ioctl requests
to phy_mii_ioctl().

I also thought the 'generic' response to an unknown ioctl command
was ENOTTY.

David

Re: [PATCH net-next 09/12] net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands

2017-03-13 Thread Florian Fainelli 
On 03/13/2017 05:41 PM, Doug Berger wrote:
> This commit changes the ioctl handling behavior to return the
> EOPNOTSUPP error code instead of the EINVAL error code when an
> unknown ioctl command value is detected.
> 
> It also removes some redundant parsing of the ioctl command value
> and allows the SIOCSHWTSTAMP value to be handled.
> 
> Signed-off-by: Doug Berger 

Reviewed-by: Florian Fainelli 
-- 
Florian

[PATCH net-next 09/12] net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands

2017-03-13 Thread Doug Berger 
This commit changes the ioctl handling behavior to return the
EOPNOTSUPP error code instead of the EINVAL error code when an
unknown ioctl command value is detected.

It also removes some redundant parsing of the ioctl command value
and allows the SIOCSHWTSTAMP value to be handled.

Signed-off-by: Doug Berger 
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index d90d366b286f..3b49c14128e2 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1062,27 +1062,14 @@ static void bcmgenet_power_up(struct bcmgenet_priv 
*priv,
 static int bcmgenet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
struct bcmgenet_priv *priv = netdev_priv(dev);
-   int val = 0;
 
if (!netif_running(dev))
return -EINVAL;
 
-   switch (cmd) {
-   case SIOCGMIIPHY:
-   case SIOCGMIIREG:
-   case SIOCSMIIREG:
-   if (!priv->phydev)
-   val = -ENODEV;
-   else
-   val = phy_mii_ioctl(priv->phydev, rq, cmd);
-   break;
-
-   default:
-   val = -EINVAL;
-   break;
-   }
+   if (!priv->phydev)
+   return -ENODEV;
 
-   return val;
+   return phy_mii_ioctl(priv->phydev, rq, cmd);
 }
 
 static struct enet_cb *bcmgenet_get_txcb(struct bcmgenet_priv *priv,
-- 
2.11.1