* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 4:59 [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs Dan Carpenter
@ 2020-01-31 8:12 ` Andy Shevchenko
2020-01-31 8:48 ` Andy Shevchenko
2020-01-31 8:15 ` Andy Shevchenko
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-01-31 8:12 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
Linux Kernel Mailing List, netdev, linux-stm32, kernel-janitors
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
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 8:12 ` Andy Shevchenko
@ 2020-01-31 8:48 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2020-01-31 8:48 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
Linux Kernel Mailing List, netdev, linux-stm32, kernel-janitors
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
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 4:59 [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs Dan Carpenter
2020-01-31 8:12 ` Andy Shevchenko
@ 2020-01-31 8:15 ` Andy Shevchenko
2020-01-31 8:24 ` Dan Carpenter
2020-01-31 13:57 ` Andrew Lunn
` (3 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-01-31 8:15 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
Linux Kernel Mailing List, netdev, linux-stm32, kernel-janitors
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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 8:15 ` Andy Shevchenko
@ 2020-01-31 8:24 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-01-31 8:24 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
Linux Kernel Mailing List, netdev, linux-stm32, kernel-janitors
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 4:59 [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs Dan Carpenter
2020-01-31 8:12 ` Andy Shevchenko
2020-01-31 8:15 ` Andy Shevchenko
@ 2020-01-31 13:57 ` Andrew Lunn
2020-02-03 0:15 ` kbuild test robot
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2020-01-31 13:57 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
linux-kernel, netdev, linux-stm32, kernel-janitors
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 4:59 [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs Dan Carpenter
` (2 preceding siblings ...)
2020-01-31 13:57 ` Andrew Lunn
@ 2020-02-03 0:15 ` kbuild test robot
2020-02-03 0:16 ` kbuild test robot
2020-02-03 5:11 ` kbuild test robot
5 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2020-02-03 0:15 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild-all, Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
linux-kernel, netdev, linux-stm32, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 21988 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28989 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 4:59 [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs Dan Carpenter
` (3 preceding siblings ...)
2020-02-03 0:15 ` kbuild test robot
@ 2020-02-03 0:16 ` kbuild test robot
2020-02-03 5:11 ` kbuild test robot
5 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2020-02-03 0:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild-all, Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
linux-kernel, netdev, linux-stm32, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36397 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-01-31 4:59 [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs Dan Carpenter
` (4 preceding siblings ...)
2020-02-03 0:16 ` kbuild test robot
@ 2020-02-03 5:11 ` kbuild test robot
2020-03-11 5:53 ` Calvin Johnson
5 siblings, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2020-02-03 5:11 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild-all, Greg Kroah-Hartman, Ajay Gupta, David S. Miller,
Rafael J. Wysocki, Iyappan Subramanian, Keyur Chudgar,
Quan Nguyen, Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
linux-kernel, netdev, linux-stm32, kernel-janitors
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] device property: change device_get_phy_mode() to prevent signedess bugs
2020-02-03 5:11 ` kbuild test robot
@ 2020-03-11 5:53 ` Calvin Johnson
0 siblings, 0 replies; 10+ messages in thread
From: Calvin Johnson @ 2020-03-11 5:53 UTC (permalink / raw)
To: dan.carpenter
Cc: Dan Carpenter, kbuild-all@lists.01.org, Greg Kroah-Hartman,
Ajay Gupta, David S. Miller, Rafael J. Wysocki,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen,
Steve Glendinning, Jassi Brar, Ilias Apalodimas,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Dmitry Torokhov, Andy Shevchenko, Sakari Ailus, Heikki Krogerus,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
kernel-janitors@vger.kernel.org
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
^ permalink raw reply [flat|nested] 10+ messages in thread