LinuxLists.cc - [PATCH net] device property: change device_get_phy

2020-01-31 05:03:44

Subject: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

The device_get_phy_mode() was returning negative error codes on
failure and positive phy_interface_t values on success. The problem is
that the phy_interface_t type is an enum which GCC treats as unsigned.
This lead to recurring signedness bugs where we check "if (phy_mode < 0)"
and "phy_mode" is unsigned.

In the commit 0c65b2b90d13 ("net: of_get_phy_mode: Change API to solve
int/unit warnings") we updated of_get_phy_mode() take a pointer to
phy_mode and only return zero on success and negatives on failure. This
patch does the same thing for device_get_phy_mode(). Plus it's just
nice for the API to be the same in both places.

Fixes: b9f0b2f634c0 ("net: stmmac: platform: fix probe for ACPI devices")
Signed-off-by: Dan Carpenter <[email protected]>
---
This is a change to drivers/base/ but all the users are ethernet drivers
so probably it makes sense for Dave to take this?

Also this fixes a bug in stmmac. If you wanted I could make a one
liner fix for that and then write this change on top of that. The bug
is only in v5.6 so it doesn't affect old kernels.

drivers/base/property.c | 13 +++++++++++--
drivers/net/ethernet/apm/xgene-v2/main.c | 9 ++++-----
drivers/net/ethernet/apm/xgene-v2/main.h | 2 +-
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 6 +++---
drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 2 +-
drivers/net/ethernet/smsc/smsc911x.c | 8 +++-----
drivers/net/ethernet/socionext/netsec.c | 5 ++---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 6 +++---
include/linux/property.h | 3 ++-
9 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 511f6d7acdfe..8854cfbd213b 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -863,9 +863,18 @@ EXPORT_SYMBOL_GPL(fwnode_get_phy_mode);
* 'phy-connection-type', and return its index in phy_modes table, or errno in
* error case.
*/
-int device_get_phy_mode(struct device *dev)
+int device_get_phy_mode(struct device *dev, phy_interface_t *interface)
{
- return fwnode_get_phy_mode(dev_fwnode(dev));
+ int mode;
+
+ *interface = PHY_INTERFACE_MODE_NA;
+
+ mode = fwnode_get_phy_mode(dev_fwnode(dev));
+ if (mode < 0)
+ return mode;
+
+ *interface = mode;
+ return 0;
}
EXPORT_SYMBOL_GPL(device_get_phy_mode);

diff --git a/drivers/net/ethernet/apm/xgene-v2/main.c b/drivers/net/ethernet/apm/xgene-v2/main.c
index c48f60996761..706602918dd1 100644
--- a/drivers/net/ethernet/apm/xgene-v2/main.c
+++ b/drivers/net/ethernet/apm/xgene-v2/main.c
@@ -15,7 +15,7 @@ static int xge_get_resources(struct xge_pdata *pdata)
{
struct platform_device *pdev;
struct net_device *ndev;
- int phy_mode, ret = 0;
+ int ret = 0;
struct resource *res;
struct device *dev;

@@ -41,12 +41,11 @@ static int xge_get_resources(struct xge_pdata *pdata)

memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);

- phy_mode = device_get_phy_mode(dev);
- if (phy_mode < 0) {
+ ret = device_get_phy_mode(dev, &pdata->resources.phy_mode);
+ if (ret) {
dev_err(dev, "Unable to get phy-connection-type\n");
- return phy_mode;
+ return ret;
}
- pdata->resources.phy_mode = phy_mode;

if (pdata->resources.phy_mode != PHY_INTERFACE_MODE_RGMII) {
dev_err(dev, "Incorrect phy-connection-type specified\n");
diff --git a/drivers/net/ethernet/apm/xgene-v2/main.h b/drivers/net/ethernet/apm/xgene-v2/main.h
index d41439d2709d..d687f0185908 100644
--- a/drivers/net/ethernet/apm/xgene-v2/main.h
+++ b/drivers/net/ethernet/apm/xgene-v2/main.h
@@ -35,7 +35,7 @@

struct xge_resource {
void __iomem *base_addr;
- int phy_mode;
+ phy_interface_t phy_mode;
u32 irq;
};

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index 6aee2f0fc0db..da35e70ccceb 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -1736,10 +1736,10 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)

memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);

- pdata->phy_mode = device_get_phy_mode(dev);
- if (pdata->phy_mode < 0) {
+ ret = device_get_phy_mode(dev, &pdata->phy_mode);
+ if (ret) {
dev_err(dev, "Unable to get phy-connection-type\n");
- return pdata->phy_mode;
+ return ret;
}
if (!phy_interface_mode_is_rgmii(pdata->phy_mode) &&
pdata->phy_mode != PHY_INTERFACE_MODE_SGMII &&
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
index 18f4923b1723..600cddf1942d 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
@@ -209,7 +209,7 @@ struct xgene_enet_pdata {
void __iomem *pcs_addr;
void __iomem *ring_csr_addr;
void __iomem *ring_cmd_addr;
- int phy_mode;
+ phy_interface_t phy_mode;
enum xgene_enet_rm rm;
struct xgene_enet_cle cle;
u64 *extd_stats;
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 49a6a9167af4..2d773e5e67f8 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2361,14 +2361,12 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
static int smsc911x_probe_config(struct smsc911x_platform_config *config,
struct device *dev)
{
- int phy_interface;
u32 width = 0;
int err;

- phy_interface = device_get_phy_mode(dev);
- if (phy_interface < 0)
- phy_interface = PHY_INTERFACE_MODE_NA;
- config->phy_interface = phy_interface;
+ err = device_get_phy_mode(dev, &config->phy_interface);
+ if (err)
+ config->phy_interface = PHY_INTERFACE_MODE_NA;

device_get_mac_address(dev, config->mac, ETH_ALEN);

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index e8224b543dfc..95ff91230523 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1994,10 +1994,9 @@ static int netsec_probe(struct platform_device *pdev)
priv->msg_enable = NETIF_MSG_TX_ERR | NETIF_MSG_HW | NETIF_MSG_DRV |
NETIF_MSG_LINK | NETIF_MSG_PROBE;

- priv->phy_interface = device_get_phy_mode(&pdev->dev);
- if ((int)priv->phy_interface < 0) {
+ ret = device_get_phy_mode(&pdev->dev, &priv->phy_interface);
+ if (ret) {
dev_err(&pdev->dev, "missing required property 'phy-mode'\n");
- ret = -ENODEV;
goto free_ndev;
}

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index d10ac54bf385..aa77c332ea1d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -412,9 +412,9 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
*mac = NULL;
}

- plat->phy_interface = device_get_phy_mode(&pdev->dev);
- if (plat->phy_interface < 0)
- return ERR_PTR(plat->phy_interface);
+ rc = device_get_phy_mode(&pdev->dev, &plat->phy_interface);
+ if (rc)
+ return ERR_PTR(rc);

plat->interface = stmmac_of_get_mac_mode(np);
if (plat->interface < 0)
diff --git a/include/linux/property.h b/include/linux/property.h
index d86de017c689..2ffe9842c997 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -12,6 +12,7 @@

#include <linux/bits.h>
#include <linux/fwnode.h>
+#include <linux/phy.h>
#include <linux/types.h>

struct device;
@@ -368,7 +369,7 @@ enum dev_dma_attr device_get_dma_attr(struct device *dev);

const void *device_get_match_data(struct device *dev);

-int device_get_phy_mode(struct device *dev);
+int device_get_phy_mode(struct device *dev, phy_interface_t *interface);

void *device_get_mac_address(struct device *dev, char *addr, int alen);

--
2.11.0

2020-01-31 08:13:31

by Andy Shevchenko

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

On Fri, Jan 31, 2020 at 7:03 AM Dan Carpenter <[email protected]> wrote:
>
> The device_get_phy_mode() was returning negative error codes on
> failure and positive phy_interface_t values on success. The problem is
> that the phy_interface_t type is an enum which GCC treats as unsigned.
> This lead to recurring signedness bugs where we check "if (phy_mode < 0)"
> and "phy_mode" is unsigned.
>
> In the commit 0c65b2b90d13 ("net: of_get_phy_mode: Change API to solve
> int/unit warnings") we updated of_get_phy_mode() take a pointer to
> phy_mode and only return zero on success and negatives on failure. This
> patch does the same thing for device_get_phy_mode(). Plus it's just
> nice for the API to be the same in both places.
>

For device property API changes
Reviewed-by: Andy Shevchenko <[email protected]>

> Fixes: b9f0b2f634c0 ("net: stmmac: platform: fix probe for ACPI devices")
> Signed-off-by: Dan Carpenter <[email protected]>
> ---
> This is a change to drivers/base/ but all the users are ethernet drivers
> so probably it makes sense for Dave to take this?
>
> Also this fixes a bug in stmmac. If you wanted I could make a one
> liner fix for that and then write this change on top of that. The bug
> is only in v5.6 so it doesn't affect old kernels.
>
> drivers/base/property.c | 13 +++++++++++--
> drivers/net/ethernet/apm/xgene-v2/main.c | 9 ++++-----
> drivers/net/ethernet/apm/xgene-v2/main.h | 2 +-
> drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 6 +++---
> drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 2 +-
> drivers/net/ethernet/smsc/smsc911x.c | 8 +++-----
> drivers/net/ethernet/socionext/netsec.c | 5 ++---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 6 +++---
> include/linux/property.h | 3 ++-
> 9 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 511f6d7acdfe..8854cfbd213b 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -863,9 +863,18 @@ EXPORT_SYMBOL_GPL(fwnode_get_phy_mode);
> * 'phy-connection-type', and return its index in phy_modes table, or errno in
> * error case.
> */
> -int device_get_phy_mode(struct device *dev)
> +int device_get_phy_mode(struct device *dev, phy_interface_t *interface)
> {
> - return fwnode_get_phy_mode(dev_fwnode(dev));
> + int mode;
> +
> + *interface = PHY_INTERFACE_MODE_NA;
> +
> + mode = fwnode_get_phy_mode(dev_fwnode(dev));
> + if (mode < 0)
> + return mode;
> +
> + *interface = mode;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(device_get_phy_mode);
>
> diff --git a/drivers/net/ethernet/apm/xgene-v2/main.c b/drivers/net/ethernet/apm/xgene-v2/main.c
> index c48f60996761..706602918dd1 100644
> --- a/drivers/net/ethernet/apm/xgene-v2/main.c
> +++ b/drivers/net/ethernet/apm/xgene-v2/main.c
> @@ -15,7 +15,7 @@ static int xge_get_resources(struct xge_pdata *pdata)
> {
> struct platform_device *pdev;
> struct net_device *ndev;
> - int phy_mode, ret = 0;
> + int ret = 0;
> struct resource *res;
> struct device *dev;
>
> @@ -41,12 +41,11 @@ static int xge_get_resources(struct xge_pdata *pdata)
>
> memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);
>
> - phy_mode = device_get_phy_mode(dev);
> - if (phy_mode < 0) {
> + ret = device_get_phy_mode(dev, &pdata->resources.phy_mode);
> + if (ret) {
> dev_err(dev, "Unable to get phy-connection-type\n");
> - return phy_mode;
> + return ret;
> }
> - pdata->resources.phy_mode = phy_mode;
>
> if (pdata->resources.phy_mode != PHY_INTERFACE_MODE_RGMII) {
> dev_err(dev, "Incorrect phy-connection-type specified\n");
> diff --git a/drivers/net/ethernet/apm/xgene-v2/main.h b/drivers/net/ethernet/apm/xgene-v2/main.h
> index d41439d2709d..d687f0185908 100644
> --- a/drivers/net/ethernet/apm/xgene-v2/main.h
> +++ b/drivers/net/ethernet/apm/xgene-v2/main.h
> @@ -35,7 +35,7 @@
>
> struct xge_resource {
> void __iomem *base_addr;
> - int phy_mode;
> + phy_interface_t phy_mode;
> u32 irq;
> };
>
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> index 6aee2f0fc0db..da35e70ccceb 100644
> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> @@ -1736,10 +1736,10 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
>
> memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);
>
> - pdata->phy_mode = device_get_phy_mode(dev);
> - if (pdata->phy_mode < 0) {
> + ret = device_get_phy_mode(dev, &pdata->phy_mode);
> + if (ret) {
> dev_err(dev, "Unable to get phy-connection-type\n");
> - return pdata->phy_mode;
> + return ret;
> }
> if (!phy_interface_mode_is_rgmii(pdata->phy_mode) &&
> pdata->phy_mode != PHY_INTERFACE_MODE_SGMII &&
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> index 18f4923b1723..600cddf1942d 100644
> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> @@ -209,7 +209,7 @@ struct xgene_enet_pdata {
> void __iomem *pcs_addr;
> void __iomem *ring_csr_addr;
> void __iomem *ring_cmd_addr;
> - int phy_mode;
> + phy_interface_t phy_mode;
> enum xgene_enet_rm rm;
> struct xgene_enet_cle cle;
> u64 *extd_stats;
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index 49a6a9167af4..2d773e5e67f8 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -2361,14 +2361,12 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> static int smsc911x_probe_config(struct smsc911x_platform_config *config,
> struct device *dev)
> {
> - int phy_interface;
> u32 width = 0;
> int err;
>
> - phy_interface = device_get_phy_mode(dev);
> - if (phy_interface < 0)
> - phy_interface = PHY_INTERFACE_MODE_NA;
> - config->phy_interface = phy_interface;
> + err = device_get_phy_mode(dev, &config->phy_interface);
> + if (err)
> + config->phy_interface = PHY_INTERFACE_MODE_NA;
>
> device_get_mac_address(dev, config->mac, ETH_ALEN);
>
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index e8224b543dfc..95ff91230523 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -1994,10 +1994,9 @@ static int netsec_probe(struct platform_device *pdev)
> priv->msg_enable = NETIF_MSG_TX_ERR | NETIF_MSG_HW | NETIF_MSG_DRV |
> NETIF_MSG_LINK | NETIF_MSG_PROBE;
>
> - priv->phy_interface = device_get_phy_mode(&pdev->dev);
> - if ((int)priv->phy_interface < 0) {
> + ret = device_get_phy_mode(&pdev->dev, &priv->phy_interface);
> + if (ret) {
> dev_err(&pdev->dev, "missing required property 'phy-mode'\n");
> - ret = -ENODEV;
> goto free_ndev;
> }
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index d10ac54bf385..aa77c332ea1d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -412,9 +412,9 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> *mac = NULL;
> }
>
> - plat->phy_interface = device_get_phy_mode(&pdev->dev);
> - if (plat->phy_interface < 0)
> - return ERR_PTR(plat->phy_interface);
> + rc = device_get_phy_mode(&pdev->dev, &plat->phy_interface);
> + if (rc)
> + return ERR_PTR(rc);
>
> plat->interface = stmmac_of_get_mac_mode(np);
> if (plat->interface < 0)
> diff --git a/include/linux/property.h b/include/linux/property.h
> index d86de017c689..2ffe9842c997 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -12,6 +12,7 @@
>
> #include <linux/bits.h>
> #include <linux/fwnode.h>
> +#include <linux/phy.h>
> #include <linux/types.h>
>
> struct device;
> @@ -368,7 +369,7 @@ enum dev_dma_attr device_get_dma_attr(struct device *dev);
>
> const void *device_get_match_data(struct device *dev);
>
> -int device_get_phy_mode(struct device *dev);
> +int device_get_phy_mode(struct device *dev, phy_interface_t *interface);
>
> void *device_get_mac_address(struct device *dev, char *addr, int alen);
>
> --
> 2.11.0
>

--
With Best Regards,
Andy Shevchenko

2020-01-31 08:16:31

by Andy Shevchenko

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

2020-01-31 08:28:03

by Dan Carpenter

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

On Fri, Jan 31, 2020 at 10:15:14AM +0200, Andy Shevchenko wrote:
> On Fri, Jan 31, 2020 at 7:03 AM Dan Carpenter <[email protected]> wrote:
> >
> > The device_get_phy_mode() was returning negative error codes on
> > failure and positive phy_interface_t values on success. The problem is
> > that the phy_interface_t type is an enum which GCC treats as unsigned.
> > This lead to recurring signedness bugs where we check "if (phy_mode < 0)"
> > and "phy_mode" is unsigned.
> >
> > In the commit 0c65b2b90d13 ("net: of_get_phy_mode: Change API to solve
> > int/unit warnings") we updated of_get_phy_mode() take a pointer to
> > phy_mode and only return zero on success and negatives on failure. This
> > patch does the same thing for device_get_phy_mode(). Plus it's just
> > nice for the API to be the same in both places.
>
>
> > + err = device_get_phy_mode(dev, &config->phy_interface);
>
> > + if (err)
> > + config->phy_interface = PHY_INTERFACE_MODE_NA;
>
> Do you need these? It seems the default settings when error appears.
>

We don't need it, but I thought it made things more readable.

regards,
dan carpenter

2020-01-31 08:49:55

by Andy Shevchenko

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

On Fri, Jan 31, 2020 at 10:12 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Fri, Jan 31, 2020 at 7:03 AM Dan Carpenter <[email protected]> wrote:
> >
> > The device_get_phy_mode() was returning negative error codes on
> > failure and positive phy_interface_t values on success. The problem is
> > that the phy_interface_t type is an enum which GCC treats as unsigned.
> > This lead to recurring signedness bugs where we check "if (phy_mode < 0)"
> > and "phy_mode" is unsigned.
> >
> > In the commit 0c65b2b90d13 ("net: of_get_phy_mode: Change API to solve
> > int/unit warnings") we updated of_get_phy_mode() take a pointer to
> > phy_mode and only return zero on success and negatives on failure. This
> > patch does the same thing for device_get_phy_mode(). Plus it's just
> > nice for the API to be the same in both places.
> >
>
> For device property API changes
> Reviewed-by: Andy Shevchenko <[email protected]>

Sorry, have to withdraw my tag. See below.

> > Fixes: b9f0b2f634c0 ("net: stmmac: platform: fix probe for ACPI devices")
> > Signed-off-by: Dan Carpenter <[email protected]>
> > ---
> > This is a change to drivers/base/ but all the users are ethernet drivers
> > so probably it makes sense for Dave to take this?
> >
> > Also this fixes a bug in stmmac. If you wanted I could make a one
> > liner fix for that and then write this change on top of that. The bug
> > is only in v5.6 so it doesn't affect old kernels.
> >
> > drivers/base/property.c | 13 +++++++++++--
> > drivers/net/ethernet/apm/xgene-v2/main.c | 9 ++++-----
> > drivers/net/ethernet/apm/xgene-v2/main.h | 2 +-
> > drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 6 +++---
> > drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 2 +-
> > drivers/net/ethernet/smsc/smsc911x.c | 8 +++-----
> > drivers/net/ethernet/socionext/netsec.c | 5 ++---
> > drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 6 +++---
> > include/linux/property.h | 3 ++-
> > 9 files changed, 30 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index 511f6d7acdfe..8854cfbd213b 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -863,9 +863,18 @@ EXPORT_SYMBOL_GPL(fwnode_get_phy_mode);
> > * 'phy-connection-type', and return its index in phy_modes table, or errno in
> > * error case.
> > */

You forgot to update documentation part.
After addressing you may put my tag back.

> > -int device_get_phy_mode(struct device *dev)
> > +int device_get_phy_mode(struct device *dev, phy_interface_t *interface)
> > {
> > - return fwnode_get_phy_mode(dev_fwnode(dev));
> > + int mode;
> > +
> > + *interface = PHY_INTERFACE_MODE_NA;
> > +
> > + mode = fwnode_get_phy_mode(dev_fwnode(dev));
> > + if (mode < 0)
> > + return mode;
> > +
> > + *interface = mode;
> > + return 0;
> > }
> > EXPORT_SYMBOL_GPL(device_get_phy_mode);
> >
> > diff --git a/drivers/net/ethernet/apm/xgene-v2/main.c b/drivers/net/ethernet/apm/xgene-v2/main.c
> > index c48f60996761..706602918dd1 100644
> > --- a/drivers/net/ethernet/apm/xgene-v2/main.c
> > +++ b/drivers/net/ethernet/apm/xgene-v2/main.c
> > @@ -15,7 +15,7 @@ static int xge_get_resources(struct xge_pdata *pdata)
> > {
> > struct platform_device *pdev;
> > struct net_device *ndev;
> > - int phy_mode, ret = 0;
> > + int ret = 0;
> > struct resource *res;
> > struct device *dev;
> >
> > @@ -41,12 +41,11 @@ static int xge_get_resources(struct xge_pdata *pdata)
> >
> > memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);
> >
> > - phy_mode = device_get_phy_mode(dev);
> > - if (phy_mode < 0) {
> > + ret = device_get_phy_mode(dev, &pdata->resources.phy_mode);
> > + if (ret) {
> > dev_err(dev, "Unable to get phy-connection-type\n");
> > - return phy_mode;
> > + return ret;
> > }
> > - pdata->resources.phy_mode = phy_mode;
> >
> > if (pdata->resources.phy_mode != PHY_INTERFACE_MODE_RGMII) {
> > dev_err(dev, "Incorrect phy-connection-type specified\n");
> > diff --git a/drivers/net/ethernet/apm/xgene-v2/main.h b/drivers/net/ethernet/apm/xgene-v2/main.h
> > index d41439d2709d..d687f0185908 100644
> > --- a/drivers/net/ethernet/apm/xgene-v2/main.h
> > +++ b/drivers/net/ethernet/apm/xgene-v2/main.h
> > @@ -35,7 +35,7 @@
> >
> > struct xge_resource {
> > void __iomem *base_addr;
> > - int phy_mode;
> > + phy_interface_t phy_mode;
> > u32 irq;
> > };
> >
> > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> > index 6aee2f0fc0db..da35e70ccceb 100644
> > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> > @@ -1736,10 +1736,10 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
> >
> > memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);
> >
> > - pdata->phy_mode = device_get_phy_mode(dev);
> > - if (pdata->phy_mode < 0) {
> > + ret = device_get_phy_mode(dev, &pdata->phy_mode);
> > + if (ret) {
> > dev_err(dev, "Unable to get phy-connection-type\n");
> > - return pdata->phy_mode;
> > + return ret;
> > }
> > if (!phy_interface_mode_is_rgmii(pdata->phy_mode) &&
> > pdata->phy_mode != PHY_INTERFACE_MODE_SGMII &&
> > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> > index 18f4923b1723..600cddf1942d 100644
> > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
> > @@ -209,7 +209,7 @@ struct xgene_enet_pdata {
> > void __iomem *pcs_addr;
> > void __iomem *ring_csr_addr;
> > void __iomem *ring_cmd_addr;
> > - int phy_mode;
> > + phy_interface_t phy_mode;
> > enum xgene_enet_rm rm;
> > struct xgene_enet_cle cle;
> > u64 *extd_stats;
> > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> > index 49a6a9167af4..2d773e5e67f8 100644
> > --- a/drivers/net/ethernet/smsc/smsc911x.c
> > +++ b/drivers/net/ethernet/smsc/smsc911x.c
> > @@ -2361,14 +2361,12 @@ static const struct smsc911x_ops shifted_smsc911x_ops = {
> > static int smsc911x_probe_config(struct smsc911x_platform_config *config,
> > struct device *dev)
> > {
> > - int phy_interface;
> > u32 width = 0;
> > int err;
> >
> > - phy_interface = device_get_phy_mode(dev);
> > - if (phy_interface < 0)
> > - phy_interface = PHY_INTERFACE_MODE_NA;
> > - config->phy_interface = phy_interface;
> > + err = device_get_phy_mode(dev, &config->phy_interface);
> > + if (err)
> > + config->phy_interface = PHY_INTERFACE_MODE_NA;
> >
> > device_get_mac_address(dev, config->mac, ETH_ALEN);
> >
> > diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> > index e8224b543dfc..95ff91230523 100644
> > --- a/drivers/net/ethernet/socionext/netsec.c
> > +++ b/drivers/net/ethernet/socionext/netsec.c
> > @@ -1994,10 +1994,9 @@ static int netsec_probe(struct platform_device *pdev)
> > priv->msg_enable = NETIF_MSG_TX_ERR | NETIF_MSG_HW | NETIF_MSG_DRV |
> > NETIF_MSG_LINK | NETIF_MSG_PROBE;
> >
> > - priv->phy_interface = device_get_phy_mode(&pdev->dev);
> > - if ((int)priv->phy_interface < 0) {
> > + ret = device_get_phy_mode(&pdev->dev, &priv->phy_interface);
> > + if (ret) {
> > dev_err(&pdev->dev, "missing required property 'phy-mode'\n");
> > - ret = -ENODEV;
> > goto free_ndev;
> > }
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index d10ac54bf385..aa77c332ea1d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -412,9 +412,9 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> > *mac = NULL;
> > }
> >
> > - plat->phy_interface = device_get_phy_mode(&pdev->dev);
> > - if (plat->phy_interface < 0)
> > - return ERR_PTR(plat->phy_interface);
> > + rc = device_get_phy_mode(&pdev->dev, &plat->phy_interface);
> > + if (rc)
> > + return ERR_PTR(rc);
> >
> > plat->interface = stmmac_of_get_mac_mode(np);
> > if (plat->interface < 0)
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index d86de017c689..2ffe9842c997 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -12,6 +12,7 @@
> >
> > #include <linux/bits.h>
> > #include <linux/fwnode.h>
> > +#include <linux/phy.h>
> > #include <linux/types.h>
> >
> > struct device;
> > @@ -368,7 +369,7 @@ enum dev_dma_attr device_get_dma_attr(struct device *dev);
> >
> > const void *device_get_match_data(struct device *dev);
> >
> > -int device_get_phy_mode(struct device *dev);
> > +int device_get_phy_mode(struct device *dev, phy_interface_t *interface);
> >
> > void *device_get_mac_address(struct device *dev, char *addr, int alen);
> >
> > --
> > 2.11.0
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko

--
With Best Regards,
Andy Shevchenko

2020-01-31 13:59:19

by Andrew Lunn

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

> diff --git a/drivers/net/ethernet/apm/xgene-v2/main.c b/drivers/net/ethernet/apm/xgene-v2/main.c
> index c48f60996761..706602918dd1 100644
> --- a/drivers/net/ethernet/apm/xgene-v2/main.c
> +++ b/drivers/net/ethernet/apm/xgene-v2/main.c
> @@ -15,7 +15,7 @@ static int xge_get_resources(struct xge_pdata *pdata)
> {
> struct platform_device *pdev;
> struct net_device *ndev;
> - int phy_mode, ret = 0;
> + int ret = 0;
> struct resource *res;
> struct device *dev;

Hi Dan

DaveM likes reverse christmas tree. So you need to move ret later to
keep the tree.

Apart from that:

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2020-02-03 00:28:00

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

Hi Dan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on driver-core/driver-core-testing linus/master v5.5 next-20200131]
[cannot apply to sparc-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Dan-Carpenter/device-property-change-device_get_phy_mode-to-prevent-signedess-bugs/20200203-043126
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git b7c3a17c6062701d97a0959890a2c882bfaac537
config: x86_64-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

In file included from include/linux/ethtool.h:18:0,
from include/linux/phy.h:16,
from include/linux/property.h:15,
from include/linux/of.h:22,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from drivers/gpu/drm/i915/i915_drv.c:30:
>> include/uapi/linux/ethtool.h:1668:20: error: expected identifier before numeric constant
#define PORT_NONE 0xef
^
>> drivers/gpu/drm/i915/display/intel_display.h:190:2: note: in expansion of macro 'PORT_NONE'
PORT_NONE = -1,
^~~~~~~~~
In file included from drivers/gpu/drm/i915/display/intel_bw.h:11:0,
from drivers/gpu/drm/i915/i915_drv.c:51:
drivers/gpu/drm/i915/display/intel_display.h: In function 'port_identifier':
>> drivers/gpu/drm/i915/display/intel_display.h:214:7: error: 'PORT_A' undeclared (first use in this function); did you mean 'PORT_DA'?
case PORT_A:
^~~~~~
PORT_DA
drivers/gpu/drm/i915/display/intel_display.h:214:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/display/intel_display.h:216:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_A'?
case PORT_B:
^~~~~~
PORT_A
>> drivers/gpu/drm/i915/display/intel_display.h:218:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display.h:220:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
>> drivers/gpu/drm/i915/display/intel_display.h:222:7: error: 'PORT_E' undeclared (first use in this function); did you mean 'PORT_D'?
case PORT_E:
^~~~~~
PORT_D
>> drivers/gpu/drm/i915/display/intel_display.h:224:7: error: 'PORT_F' undeclared (first use in this function); did you mean 'PORT_E'?
case PORT_F:
^~~~~~
PORT_E
>> drivers/gpu/drm/i915/display/intel_display.h:226:7: error: 'PORT_G' undeclared (first use in this function); did you mean 'PORT_F'?
case PORT_G:
^~~~~~
PORT_F
>> drivers/gpu/drm/i915/display/intel_display.h:228:7: error: 'PORT_H' undeclared (first use in this function); did you mean 'PORT_G'?
case PORT_H:
^~~~~~
PORT_G
>> drivers/gpu/drm/i915/display/intel_display.h:230:7: error: 'PORT_I' undeclared (first use in this function); did you mean 'PORT_H'?
case PORT_I:
^~~~~~
PORT_H
In file included from drivers/gpu/drm/i915/display/intel_display_types.h:46:0,
from drivers/gpu/drm/i915/i915_drv.c:53:
drivers/gpu/drm/i915/i915_drv.h: At top level:
>> drivers/gpu/drm/i915/i915_drv.h:726:41: error: 'I915_MAX_PORTS' undeclared here (not in a function); did you mean 'I915_MAX_PHYS'?
struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
^~~~~~~~~~~~~~
I915_MAX_PHYS
In file included from drivers/gpu/drm/i915/i915_drv.c:53:0:
drivers/gpu/drm/i915/display/intel_display_types.h: In function 'vlv_dport_to_channel':
>> drivers/gpu/drm/i915/display/intel_display_types.h:1386:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_BNC'?
case PORT_B:
^~~~~~
PORT_BNC
>> drivers/gpu/drm/i915/display/intel_display_types.h:1387:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_D:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display_types.h:1389:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_D'?
case PORT_C:
^~~~~~
PORT_D
drivers/gpu/drm/i915/display/intel_display_types.h: In function 'vlv_dport_to_phy':
drivers/gpu/drm/i915/display/intel_display_types.h:1400:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_BNC'?
case PORT_B:
^~~~~~
PORT_BNC
>> drivers/gpu/drm/i915/display/intel_display_types.h:1401:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display_types.h:1403:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
--
In file included from include/linux/ethtool.h:18:0,
from include/linux/phy.h:16,
from include/linux/property.h:15,
from include/linux/of.h:22,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from include/linux/i2c.h:13,
from drivers/gpu/drm/i915/display/intel_display_types.h:30,
from drivers/gpu/drm/i915/i915_irq.c:39:
>> include/uapi/linux/ethtool.h:1668:20: error: expected identifier before numeric constant
#define PORT_NONE 0xef
^
>> drivers/gpu/drm/i915/display/intel_display.h:190:2: note: in expansion of macro 'PORT_NONE'
PORT_NONE = -1,
^~~~~~~~~
In file included from drivers/gpu/drm/i915/i915_drv.h:68:0,
from drivers/gpu/drm/i915/display/intel_display_types.h:46,
from drivers/gpu/drm/i915/i915_irq.c:39:
drivers/gpu/drm/i915/display/intel_display.h: In function 'port_identifier':
>> drivers/gpu/drm/i915/display/intel_display.h:214:7: error: 'PORT_A' undeclared (first use in this function); did you mean 'PORT_DA'?
case PORT_A:
^~~~~~
PORT_DA
drivers/gpu/drm/i915/display/intel_display.h:214:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/display/intel_display.h:216:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_A'?
case PORT_B:
^~~~~~
PORT_A
>> drivers/gpu/drm/i915/display/intel_display.h:218:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display.h:220:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
>> drivers/gpu/drm/i915/display/intel_display.h:222:7: error: 'PORT_E' undeclared (first use in this function); did you mean 'PORT_D'?
case PORT_E:
^~~~~~
PORT_D
>> drivers/gpu/drm/i915/display/intel_display.h:224:7: error: 'PORT_F' undeclared (first use in this function); did you mean 'PORT_E'?
case PORT_F:
^~~~~~
PORT_E
>> drivers/gpu/drm/i915/display/intel_display.h:226:7: error: 'PORT_G' undeclared (first use in this function); did you mean 'PORT_F'?
case PORT_G:
^~~~~~
PORT_F
>> drivers/gpu/drm/i915/display/intel_display.h:228:7: error: 'PORT_H' undeclared (first use in this function); did you mean 'PORT_G'?
case PORT_H:
^~~~~~
PORT_G
>> drivers/gpu/drm/i915/display/intel_display.h:230:7: error: 'PORT_I' undeclared (first use in this function); did you mean 'PORT_H'?
case PORT_I:
^~~~~~
PORT_H
In file included from drivers/gpu/drm/i915/display/intel_display_types.h:46:0,
from drivers/gpu/drm/i915/i915_irq.c:39:
drivers/gpu/drm/i915/i915_drv.h: At top level:
>> drivers/gpu/drm/i915/i915_drv.h:726:41: error: 'I915_MAX_PORTS' undeclared here (not in a function); did you mean 'I915_MAX_PHYS'?
struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
^~~~~~~~~~~~~~
I915_MAX_PHYS
In file included from drivers/gpu/drm/i915/i915_irq.c:39:0:
drivers/gpu/drm/i915/display/intel_display_types.h: In function 'vlv_dport_to_channel':
>> drivers/gpu/drm/i915/display/intel_display_types.h:1386:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_BNC'?
case PORT_B:
^~~~~~
PORT_BNC
>> drivers/gpu/drm/i915/display/intel_display_types.h:1387:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_D:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display_types.h:1389:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_D'?
case PORT_C:
^~~~~~
PORT_D
drivers/gpu/drm/i915/display/intel_display_types.h: In function 'vlv_dport_to_phy':
drivers/gpu/drm/i915/display/intel_display_types.h:1400:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_BNC'?
case PORT_B:
^~~~~~
PORT_BNC
>> drivers/gpu/drm/i915/display/intel_display_types.h:1401:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display_types.h:1403:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
In file included from drivers/gpu/drm/i915/i915_drv.h:64:0,
from drivers/gpu/drm/i915/display/intel_display_types.h:46,
from drivers/gpu/drm/i915/i915_irq.c:39:
drivers/gpu/drm/i915/i915_irq.c: At top level:
>> drivers/gpu/drm/i915/i915_irq.c:152:37: error: 'PORT_A' undeclared here (not in a function); did you mean 'PORT_DA'?
[HPD_PORT_A] = SDE_DDI_HOTPLUG_ICP(PORT_A),
^
drivers/gpu/drm/i915/i915_reg.h:8021:43: note: in definition of macro 'SDE_DDI_HOTPLUG_ICP'
#define SDE_DDI_HOTPLUG_ICP(port) (1 << ((port) + 16))
^~~~
>> drivers/gpu/drm/i915/i915_irq.c:153:37: error: 'PORT_B' undeclared here (not in a function); did you mean 'PORT_A'?
[HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(PORT_B),
^
drivers/gpu/drm/i915/i915_reg.h:8021:43: note: in definition of macro 'SDE_DDI_HOTPLUG_ICP'
#define SDE_DDI_HOTPLUG_ICP(port) (1 << ((port) + 16))
^~~~
>> drivers/gpu/drm/i915/i915_irq.c:163:37: error: 'PORT_C' undeclared here (not in a function); did you mean 'PORT_B'?
[HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(PORT_C),
^
drivers/gpu/drm/i915/i915_reg.h:8021:43: note: in definition of macro 'SDE_DDI_HOTPLUG_ICP'
#define SDE_DDI_HOTPLUG_ICP(port) (1 << ((port) + 16))
^~~~
--
In file included from include/linux/ethtool.h:18:0,
from include/linux/phy.h:16,
from include/linux/property.h:15,
from include/linux/of.h:22,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from include/linux/i2c.h:13,
from drivers/gpu/drm/i915/i915_drv.h:37,
from drivers/gpu/drm/i915/i915_getparam.c:8:
>> include/uapi/linux/ethtool.h:1668:20: error: expected identifier before numeric constant
#define PORT_NONE 0xef
^
>> drivers/gpu/drm/i915/display/intel_display.h:190:2: note: in expansion of macro 'PORT_NONE'
PORT_NONE = -1,
^~~~~~~~~
In file included from drivers/gpu/drm/i915/i915_drv.h:68:0,
from drivers/gpu/drm/i915/i915_getparam.c:8:
drivers/gpu/drm/i915/display/intel_display.h: In function 'port_identifier':
>> drivers/gpu/drm/i915/display/intel_display.h:214:7: error: 'PORT_A' undeclared (first use in this function); did you mean 'PORT_DA'?
case PORT_A:
^~~~~~
PORT_DA
drivers/gpu/drm/i915/display/intel_display.h:214:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/display/intel_display.h:216:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_A'?
case PORT_B:
^~~~~~
PORT_A
>> drivers/gpu/drm/i915/display/intel_display.h:218:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display.h:220:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
>> drivers/gpu/drm/i915/display/intel_display.h:222:7: error: 'PORT_E' undeclared (first use in this function); did you mean 'PORT_D'?
case PORT_E:
^~~~~~
PORT_D
>> drivers/gpu/drm/i915/display/intel_display.h:224:7: error: 'PORT_F' undeclared (first use in this function); did you mean 'PORT_E'?
case PORT_F:
^~~~~~
PORT_E
>> drivers/gpu/drm/i915/display/intel_display.h:226:7: error: 'PORT_G' undeclared (first use in this function); did you mean 'PORT_F'?
case PORT_G:
^~~~~~
PORT_F
>> drivers/gpu/drm/i915/display/intel_display.h:228:7: error: 'PORT_H' undeclared (first use in this function); did you mean 'PORT_G'?
case PORT_H:
^~~~~~
PORT_G
>> drivers/gpu/drm/i915/display/intel_display.h:230:7: error: 'PORT_I' undeclared (first use in this function); did you mean 'PORT_H'?
case PORT_I:
^~~~~~
PORT_H
In file included from drivers/gpu/drm/i915/i915_getparam.c:8:0:
drivers/gpu/drm/i915/i915_drv.h: At top level:
>> drivers/gpu/drm/i915/i915_drv.h:726:41: error: 'I915_MAX_PORTS' undeclared here (not in a function); did you mean 'I915_MAX_PHYS'?
struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
^~~~~~~~~~~~~~
I915_MAX_PHYS
--
In file included from include/linux/ethtool.h:18:0,
from include/linux/phy.h:16,
from include/linux/property.h:15,
from include/linux/of.h:22,
from include/linux/irqdomain.h:35,
from include/linux/acpi.h:13,
from include/linux/i2c.h:13,
from drivers/gpu/drm/i915/display/intel_display_types.h:30,
from drivers/gpu/drm/i915/display/intel_pipe_crc.c:33:
>> include/uapi/linux/ethtool.h:1668:20: error: expected identifier before numeric constant
#define PORT_NONE 0xef
^
>> drivers/gpu/drm/i915/display/intel_display.h:190:2: note: in expansion of macro 'PORT_NONE'
PORT_NONE = -1,
^~~~~~~~~
In file included from drivers/gpu/drm/i915/i915_drv.h:68:0,
from drivers/gpu/drm/i915/display/intel_display_types.h:46,
from drivers/gpu/drm/i915/display/intel_pipe_crc.c:33:
drivers/gpu/drm/i915/display/intel_display.h: In function 'port_identifier':
>> drivers/gpu/drm/i915/display/intel_display.h:214:7: error: 'PORT_A' undeclared (first use in this function); did you mean 'PORT_DA'?
case PORT_A:
^~~~~~
PORT_DA
drivers/gpu/drm/i915/display/intel_display.h:214:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/display/intel_display.h:216:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_A'?
case PORT_B:
^~~~~~
PORT_A
>> drivers/gpu/drm/i915/display/intel_display.h:218:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display.h:220:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
>> drivers/gpu/drm/i915/display/intel_display.h:222:7: error: 'PORT_E' undeclared (first use in this function); did you mean 'PORT_D'?
case PORT_E:
^~~~~~
PORT_D
>> drivers/gpu/drm/i915/display/intel_display.h:224:7: error: 'PORT_F' undeclared (first use in this function); did you mean 'PORT_E'?
case PORT_F:
^~~~~~
PORT_E
>> drivers/gpu/drm/i915/display/intel_display.h:226:7: error: 'PORT_G' undeclared (first use in this function); did you mean 'PORT_F'?
case PORT_G:
^~~~~~
PORT_F
>> drivers/gpu/drm/i915/display/intel_display.h:228:7: error: 'PORT_H' undeclared (first use in this function); did you mean 'PORT_G'?
case PORT_H:
^~~~~~
PORT_G
>> drivers/gpu/drm/i915/display/intel_display.h:230:7: error: 'PORT_I' undeclared (first use in this function); did you mean 'PORT_H'?
case PORT_I:
^~~~~~
PORT_H
In file included from drivers/gpu/drm/i915/display/intel_display_types.h:46:0,
from drivers/gpu/drm/i915/display/intel_pipe_crc.c:33:
drivers/gpu/drm/i915/i915_drv.h: At top level:
>> drivers/gpu/drm/i915/i915_drv.h:726:41: error: 'I915_MAX_PORTS' undeclared here (not in a function); did you mean 'I915_MAX_PHYS'?
struct ddi_vbt_port_info ddi_port_info[I915_MAX_PORTS];
^~~~~~~~~~~~~~
I915_MAX_PHYS
In file included from drivers/gpu/drm/i915/display/intel_pipe_crc.c:33:0:
drivers/gpu/drm/i915/display/intel_display_types.h: In function 'vlv_dport_to_channel':
>> drivers/gpu/drm/i915/display/intel_display_types.h:1386:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_BNC'?
case PORT_B:
^~~~~~
PORT_BNC
>> drivers/gpu/drm/i915/display/intel_display_types.h:1387:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_D:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display_types.h:1389:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_D'?
case PORT_C:
^~~~~~
PORT_D
drivers/gpu/drm/i915/display/intel_display_types.h: In function 'vlv_dport_to_phy':
drivers/gpu/drm/i915/display/intel_display_types.h:1400:7: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_BNC'?
case PORT_B:
^~~~~~
PORT_BNC
>> drivers/gpu/drm/i915/display/intel_display_types.h:1401:7: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_display_types.h:1403:7: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
drivers/gpu/drm/i915/display/intel_pipe_crc.c: In function 'i9xx_pipe_crc_auto_source':
>> drivers/gpu/drm/i915/display/intel_pipe_crc.c:103:9: error: 'PORT_B' undeclared (first use in this function); did you mean 'PORT_BNC'?
case PORT_B:
^~~~~~
PORT_BNC
>> drivers/gpu/drm/i915/display/intel_pipe_crc.c:106:9: error: 'PORT_C' undeclared (first use in this function); did you mean 'PORT_B'?
case PORT_C:
^~~~~~
PORT_B
>> drivers/gpu/drm/i915/display/intel_pipe_crc.c:109:9: error: 'PORT_D' undeclared (first use in this function); did you mean 'PORT_C'?
case PORT_D:
^~~~~~
PORT_C
..

vim +1668 include/uapi/linux/ethtool.h

103a8ad1fa3b26 Nikolay Aleksandrov 2016-02-03 1660
607ca46e97a1b6 David Howells 2012-10-13 1661 /* Which connector port. */
607ca46e97a1b6 David Howells 2012-10-13 1662 #define PORT_TP 0x00
607ca46e97a1b6 David Howells 2012-10-13 1663 #define PORT_AUI 0x01
607ca46e97a1b6 David Howells 2012-10-13 1664 #define PORT_MII 0x02
607ca46e97a1b6 David Howells 2012-10-13 1665 #define PORT_FIBRE 0x03
607ca46e97a1b6 David Howells 2012-10-13 1666 #define PORT_BNC 0x04
607ca46e97a1b6 David Howells 2012-10-13 1667 #define PORT_DA 0x05
607ca46e97a1b6 David Howells 2012-10-13 @1668 #define PORT_NONE 0xef
607ca46e97a1b6 David Howells 2012-10-13 1669 #define PORT_OTHER 0xff
607ca46e97a1b6 David Howells 2012-10-13 1670

:::::: The code at line 1668 was first introduced by commit
:::::: 607ca46e97a1b6594b29647d98a32d545c24bdff UAPI: (Scripted) Disintegrate include/linux

:::::: TO: David Howells <[email protected]>
:::::: CC: David Howells <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Attachments:

(No filename) (21.88 kB)
.config.gz (28.31 kB)
Download all attachments

2020-02-03 00:31:33

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

Attachments:

(No filename) (3.04 kB)
.config.gz (35.54 kB)
Download all attachments

2020-02-03 07:28:12

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

Hi Dan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on driver-core/driver-core-testing linus/master v5.5 next-20200131]
[cannot apply to sparc-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Dan-Carpenter/device-property-change-device_get_phy_mode-to-prevent-signedess-bugs/20200203-043126
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git b7c3a17c6062701d97a0959890a2c882bfaac537
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-154-g1dc00f87-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

sparse warnings: (new ones prefixed by >>)

arch/x86/boot/compressed/cmdline.c:5:20: sparse: sparse: multiple definitions for function 'set_fs'
>> arch/x86/include/asm/uaccess.h:29:20: sparse: the previous one is here
arch/x86/boot/compressed/../cmdline.c:28:5: sparse: sparse: symbol '__cmdline_find_option' was not declared. Should it be static?
arch/x86/boot/compressed/../cmdline.c:100:5: sparse: sparse: symbol '__cmdline_find_option_bool' was not declared. Should it be static?

vim +29 arch/x86/include/asm/uaccess.h

ca23386216b9d4 include/asm-x86/uaccess.h Glauber Costa 2008-06-13 27
13d4ea097d18b4 arch/x86/include/asm/uaccess.h Andy Lutomirski 2016-07-14 28 #define get_fs() (current->thread.addr_limit)
5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 @29 static inline void set_fs(mm_segment_t fs)
5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 30 {
5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 31 current->thread.addr_limit = fs;
5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 32 /* On user-mode return, check fs is correct */
5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 33 set_thread_flag(TIF_FSCHECK);
5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 34 }
ca23386216b9d4 include/asm-x86/uaccess.h Glauber Costa 2008-06-13 35

:::::: The code at line 29 was first introduced by commit
:::::: 5ea0727b163cb5575e36397a12eade68a1f35f24 x86/syscalls: Check address limit on user-mode return

:::::: TO: Thomas Garnier <[email protected]>
:::::: CC: Thomas Gleixner <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

2020-03-11 05:54:37

by Calvin Johnson

[permalink] [raw]

Subject: Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs

Hi Dan,

Do you plan to send v2 of this patch set?
https://lkml.org/lkml/2020/1/31/1
I'm preparing my patch set on top of this. Hence the query.

Thanks
Calvin

On Mon, Feb 03, 2020 at 05:11:49AM +0000, kbuild test robot wrote:
> Hi Dan,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on net/master]
> [also build test WARNING on driver-core/driver-core-testing linus/master v5.5 next-20200131]
> [cannot apply to sparc-next/master]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Dan-Carpenter/device-property-change-device_get_phy_mode-to-prevent-signedess-bugs/20200203-043126
> base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git b7c3a17c6062701d97a0959890a2c882bfaac537
> reproduce:
> # apt-get install sparse
> # sparse version: v0.6.1-154-g1dc00f87-dirty
> make ARCH=x86_64 allmodconfig
> make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <[email protected]>
>
>
> sparse warnings: (new ones prefixed by >>)
>
> arch/x86/boot/compressed/cmdline.c:5:20: sparse: sparse: multiple definitions for function 'set_fs'
> >> arch/x86/include/asm/uaccess.h:29:20: sparse: the previous one is here
> arch/x86/boot/compressed/../cmdline.c:28:5: sparse: sparse: symbol '__cmdline_find_option' was not declared. Should it be static?
> arch/x86/boot/compressed/../cmdline.c:100:5: sparse: sparse: symbol '__cmdline_find_option_bool' was not declared. Should it be static?
>
> vim +29 arch/x86/include/asm/uaccess.h
>
> ca23386216b9d4 include/asm-x86/uaccess.h Glauber Costa 2008-06-13 27
> 13d4ea097d18b4 arch/x86/include/asm/uaccess.h Andy Lutomirski 2016-07-14 28 #define get_fs() (current->thread.addr_limit)
> 5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 @29 static inline void set_fs(mm_segment_t fs)
> 5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 30 {
> 5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 31 current->thread.addr_limit = fs;
> 5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 32 /* On user-mode return, check fs is correct */
> 5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 33 set_thread_flag(TIF_FSCHECK);
> 5ea0727b163cb5 arch/x86/include/asm/uaccess.h Thomas Garnier 2017-06-14 34 }
> ca23386216b9d4 include/asm-x86/uaccess.h Glauber Costa 2008-06-13 35
>
> :::::: The code at line 29 was first introduced by commit
> :::::: 5ea0727b163cb5575e36397a12eade68a1f35f24 x86/syscalls: Check address limit on user-mode return
>
> :::::: TO: Thomas Garnier <[email protected]>
> :::::: CC: Thomas Gleixner <[email protected]>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation