All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>, David Miller <davem@davemloft.net>,
	Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Doug Berger <opendmb@gmail.com>,
	Pantelis Antoniou <pantelis.antoniou@gmail.com>,
	Yisen Zhuang <yisen.zhuang@huawei.com>,
	Salil Mehta <salil.mehta@huawei.com>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Sylvain Lemieux <slemieux.tyco@gmail.com>,
	Timur Tabi <timur@kernel.org>,
	Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	Steve Glendinning <steve.glendinning@shawell.net>,
	Michal Simek <michal.simek@xilinx.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	bcm-kernel-feedback-list@broadcom.com,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	linux-renesas-soc@vger.kernel.org,
	Linux USB Mailing List <linux-usb@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running
Date: Wed, 22 Jan 2020 08:28:06 +0100	[thread overview]
Message-ID: <ec2a401d-e504-da38-8bc7-1826f5de7941@gmail.com> (raw)
In-Reply-To: <9d2dbcc0-7e22-601a-35f6-135f2a9e6f99@gmail.com>

On 22.01.2020 05:04, Florian Fainelli wrote:
> 
> 
> On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
>> Convert suitable drivers to use new helper phy_do_ioctl_running.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> The vast majority of drivers that you are converting use the following
> convention:
> 
> - !netif_running -> return -EINVAL
> - !dev->phydev -> return -ENODEV
> 
> so it may make sense to change the helper to accommodate the majority
> here, not that I believe this is going to make much practical
> difference, but if there were test cases that were specifically looking
> for such an error code, they could be failing after this changeset.
> 
Right, I also stumbled across the different error codes, mainly as you
say -EINVAL. However there is no "wrong value", if netdev isn't running,
then typically the PHY is not attached, and from a netdev point of view
it's not there. So ENODEV seems to be best suited.
In kernel code the changed return code doesn't make a difference,
but yes, in theory there could be userspace programs checking for
-EINVAL. However such userspace programs should check for ENODEV too
anyway to cover the second check that already returns -ENODEV.

> For bgmac.c, bcmgenet.c and cpmac.c:
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Whether you decide to spin another version or not.
> 
Heiner

WARNING: multiple messages have this Message-ID (diff)
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>, David Miller <davem@davemloft.net>,
	Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Doug Berger <opendmb@gmail.com>,
	Pantelis Antoniou <pantelis.antoniou@gmail.com>,
	Yisen Zhuang <yisen.zhuang@huawei.com>,
	Salil Mehta <salil.mehta@huawei.com>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Sylvain Lemieux <slemieux.tyco@gmail.com>,
	Timur Tabi <timur@kernel.org>,
	Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	Steve Glendinning <steve.glendinning@shawell.net>,
	Michal Simek <michal.simek@xilinx.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Linux USB Mailing List <linux-usb@vger.kernel.org>,
	linux-renesas-soc@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running
Date: Wed, 22 Jan 2020 08:28:06 +0100	[thread overview]
Message-ID: <ec2a401d-e504-da38-8bc7-1826f5de7941@gmail.com> (raw)
In-Reply-To: <9d2dbcc0-7e22-601a-35f6-135f2a9e6f99@gmail.com>

On 22.01.2020 05:04, Florian Fainelli wrote:
> 
> 
> On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
>> Convert suitable drivers to use new helper phy_do_ioctl_running.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> The vast majority of drivers that you are converting use the following
> convention:
> 
> - !netif_running -> return -EINVAL
> - !dev->phydev -> return -ENODEV
> 
> so it may make sense to change the helper to accommodate the majority
> here, not that I believe this is going to make much practical
> difference, but if there were test cases that were specifically looking
> for such an error code, they could be failing after this changeset.
> 
Right, I also stumbled across the different error codes, mainly as you
say -EINVAL. However there is no "wrong value", if netdev isn't running,
then typically the PHY is not attached, and from a netdev point of view
it's not there. So ENODEV seems to be best suited.
In kernel code the changed return code doesn't make a difference,
but yes, in theory there could be userspace programs checking for
-EINVAL. However such userspace programs should check for ENODEV too
anyway to cover the second check that already returns -ENODEV.

> For bgmac.c, bcmgenet.c and cpmac.c:
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Whether you decide to spin another version or not.
> 
Heiner

WARNING: multiple messages have this Message-ID (diff)
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>, David Miller <davem@davemloft.net>,
	Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Doug Berger <opendmb@gmail.com>,
	Pantelis Antoniou <pantelis.antoniou@gmail.com>,
	Yisen Zhuang <yisen.zhuang@huawei.com>,
	Salil Mehta <salil.mehta@huawei.com>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Sylvain Lemieux <slemieux.tyco@gmail.com>,
	Timur Tabi <timur@kernel.org>,
	Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	Steve Glendinning <steve.glendinning@shawell.net>,
	Michal Simek <michal.simek@xilinx.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Linux USB Mailing List <linux-usb@vger.kernel.org>,
	linux-renesas-soc@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running
Date: Wed, 22 Jan 2020 08:28:06 +0100	[thread overview]
Message-ID: <ec2a401d-e504-da38-8bc7-1826f5de7941@gmail.com> (raw)
In-Reply-To: <9d2dbcc0-7e22-601a-35f6-135f2a9e6f99@gmail.com>

On 22.01.2020 05:04, Florian Fainelli wrote:
> 
> 
> On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
>> Convert suitable drivers to use new helper phy_do_ioctl_running.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> The vast majority of drivers that you are converting use the following
> convention:
> 
> - !netif_running -> return -EINVAL
> - !dev->phydev -> return -ENODEV
> 
> so it may make sense to change the helper to accommodate the majority
> here, not that I believe this is going to make much practical
> difference, but if there were test cases that were specifically looking
> for such an error code, they could be failing after this changeset.
> 
Right, I also stumbled across the different error codes, mainly as you
say -EINVAL. However there is no "wrong value", if netdev isn't running,
then typically the PHY is not attached, and from a netdev point of view
it's not there. So ENODEV seems to be best suited.
In kernel code the changed return code doesn't make a difference,
but yes, in theory there could be userspace programs checking for
-EINVAL. However such userspace programs should check for ENODEV too
anyway to cover the second check that already returns -ENODEV.

> For bgmac.c, bcmgenet.c and cpmac.c:
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Whether you decide to spin another version or not.
> 
Heiner

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-01-22  7:32 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-21 21:09 [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running Heiner Kallweit
2020-01-21 21:09 ` Heiner Kallweit
2020-01-21 21:09 ` Heiner Kallweit
2020-01-22  1:44 ` Timur Tabi
2020-01-22  1:44   ` Timur Tabi
2020-01-22  1:44   ` Timur Tabi
2020-01-22  4:04 ` Florian Fainelli
2020-01-22  4:04   ` Florian Fainelli
2020-01-22  4:04   ` Florian Fainelli
2020-01-22  7:28   ` Heiner Kallweit [this message]
2020-01-22  7:28     ` Heiner Kallweit
2020-01-22  7:28     ` Heiner Kallweit
2020-01-23  9:49 ` David Miller
2020-01-23  9:49   ` David Miller
2020-01-23  9:49   ` David Miller
2020-01-24  9:15 ` kbuild test robot
2020-01-24 19:36   ` Heiner Kallweit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ec2a401d-e504-da38-8bc7-1826f5de7941@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=michal.simek@xilinx.com \
    --cc=mripard@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=opendmb@gmail.com \
    --cc=pantelis.antoniou@gmail.com \
    --cc=salil.mehta@huawei.com \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=slemieux.tyco@gmail.com \
    --cc=steve.glendinning@shawell.net \
    --cc=timur@kernel.org \
    --cc=vz@mleia.com \
    --cc=wens@csie.org \
    --cc=woojung.huh@microchip.com \
    --cc=yisen.zhuang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.