| Message ID | 20200131045953.wbj66jkvijnmf5s2@kili.mountain |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] device property: change device_get_phy_mode() to prevent signedess bugs | expand |
On Fri, Jan 31, 2020 at 7:03 AM Dan Carpenter <dan.carpenter@oracle.com> 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 <andy.shevchenko@gmail.com> > Fixes: b9f0b2f634c0 ("net: stmmac: platform: fix probe for ACPI devices") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > 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 >
On Fri, Jan 31, 2020 at 7:03 AM Dan Carpenter <dan.carpenter@oracle.com> 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.
On Fri, Jan 31, 2020 at 10:15:14AM +0200, Andy Shevchenko wrote: > On Fri, Jan 31, 2020 at 7:03 AM Dan Carpenter <dan.carpenter@oracle.com> 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
On Fri, Jan 31, 2020 at 10:12 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Fri, Jan 31, 2020 at 7:03 AM Dan Carpenter <dan.carpenter@oracle.com> 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 <andy.shevchenko@gmail.com> Sorry, have to withdraw my tag. See below. > > Fixes: b9f0b2f634c0 ("net: stmmac: platform: fix probe for ACPI devices") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > 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
> 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 <andrew@lunn.ch> Andrew
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 <lkp@intel.com>
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 <dhowells@redhat.com>
:::::: CC: David Howells <dhowells@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
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-randconfig-s0-20200203 (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 <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:110:12: error: conflicting types for 'phy_write'
static int phy_write(struct phy *phy, u32 value, unsigned int reg)
^~~~~~~~~
In file included from include/linux/property.h:15:0,
from include/linux/of.h:22,
from include/linux/clk-provider.h:9,
from drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:8:
include/linux/phy.h:730:19: note: previous definition of 'phy_write' was here
static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
^~~~~~~~~
vim +/phy_write +110 drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c
f4c8116e294b12 Guido Günther 2019-06-20 109
f4c8116e294b12 Guido Günther 2019-06-20 @110 static int phy_write(struct phy *phy, u32 value, unsigned int reg)
f4c8116e294b12 Guido Günther 2019-06-20 111 {
f4c8116e294b12 Guido Günther 2019-06-20 112 struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
f4c8116e294b12 Guido Günther 2019-06-20 113 int ret;
f4c8116e294b12 Guido Günther 2019-06-20 114
f4c8116e294b12 Guido Günther 2019-06-20 115 ret = regmap_write(priv->regmap, reg, value);
f4c8116e294b12 Guido Günther 2019-06-20 116 if (ret < 0)
f4c8116e294b12 Guido Günther 2019-06-20 117 dev_err(&phy->dev, "Failed to write DPHY reg %d: %d\n", reg,
f4c8116e294b12 Guido Günther 2019-06-20 118 ret);
f4c8116e294b12 Guido Günther 2019-06-20 119 return ret;
f4c8116e294b12 Guido Günther 2019-06-20 120 }
f4c8116e294b12 Guido Günther 2019-06-20 121
:::::: The code at line 110 was first introduced by commit
:::::: f4c8116e294b12c360b724173f4b79f232573fb1 phy: Add driver for mixel mipi dphy found on NXP's i.MX8 SoCs
:::::: TO: Guido Günther <agx@sigxcpu.org>
:::::: CC: Kishon Vijay Abraham I <kishon@ti.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
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 <lkp@intel.com>
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 <thgarnie@google.com>
:::::: CC: Thomas Gleixner <tglx@linutronix.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
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 <lkp@intel.com> > > > 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 <thgarnie@google.com> > :::::: CC: Thomas Gleixner <tglx@linutronix.de> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
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);
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 <dan.carpenter@oracle.com> --- 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(-)