[PATCH 2/2] misc: ocxl: use put_device() instead of device_unregister()

[Arvind Yadav]

if device_register() returned an error! Always use put_device()
to give up the reference initialized.

Signed-off-by: Arvind Yadav 
---
 drivers/misc/ocxl/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ocxl/pci.c b/drivers/misc/ocxl/pci.c
index 0051d9e..21f4254 100644
--- a/drivers/misc/ocxl/pci.c
+++ b/drivers/misc/ocxl/pci.c
@@ -519,7 +519,7 @@ static struct ocxl_fn *init_function(struct pci_dev *dev)
rc = device_register(&fn->dev);
if (rc) {
deconfigure_function(fn);
-   device_unregister(&fn->dev);
+   put_device(&fn->dev);
return ERR_PTR(rc);
}
return fn;
-- 
1.9.1

[PATCH 1/2] misc: mic: Release reference count and memory for VOP device

[Arvind Yadav]

Never directly free @dev after calling device_register(),
even if it returned an error! Always use put_device() to
give up the reference initialized.
Release allocated memory for vop device in vop_release_dev().

Signed-off-by: Arvind Yadav 
---
 drivers/misc/mic/bus/vop_bus.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mic/bus/vop_bus.c b/drivers/misc/mic/bus/vop_bus.c
index fd7f2a6..e5bb9c7 100644
--- a/drivers/misc/mic/bus/vop_bus.c
+++ b/drivers/misc/mic/bus/vop_bus.c
@@ -135,7 +135,9 @@ void vop_unregister_driver(struct vop_driver *driver)
 
 static void vop_release_dev(struct device *d)
 {
-   put_device(d);
+   struct vop_device *dev = dev_to_vop(d);
+
+   kfree(dev);
 }
 
 struct vop_device *
@@ -174,7 +176,7 @@ struct vop_device *
goto free_vdev;
return vdev;
 free_vdev:
-   kfree(vdev);
+   put_device(&vdev->dev);
return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(vop_register_device);
-- 
1.9.1

[PATCH 0/2] misc: use put_device() instead of kfree()

[Arvind Yadav]

Never directly free @dev after calling device_register(), even
if it returned an error! Always use put_device() to give up the
reference initialized.

Arvind Yadav (2):
  [PATCH 1/2] misc: mic: Release reference count and memory for VOP device
  [PATCH 2/2] misc: ocxl: use put_device() instead of device_unregister()

 drivers/misc/mic/bus/vop_bus.c | 6 --
 drivers/misc/ocxl/pci.c| 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH v2 2/3] ASoC: fsl-asoc-card: Handle return value of devm_kasprintf

[Arvind Yadav]

devm_kasprintf() can fail here and we must check its return value.

Signed-off-by: Arvind Yadav 
---
changes in v2:
   Set return 'ret' to -ENOMEM.

 sound/soc/fsl/fsl-asoc-card.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 2db4d0c..a389885 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -639,6 +639,10 @@ static int fsl_asoc_card_probe(struct platform_device 
*pdev)
devm_kasprintf(&pdev->dev, GFP_KERNEL,
   "ac97-codec.%u",
   (unsigned int)idx);
+   if (!priv->dai_link[0].codec_name) {
+   ret = -ENOMEM;
+   goto asrc_fail;
+   }
}
 
priv->dai_link[0].platform_of_node = cpu_np;
-- 
1.9.1

[PATCH 3/3] ASoC: omap-hdmi-audio: Handle return value of devm_kasprintf

[Arvind Yadav]

devm_kasprintf() can fail here and we must check its return value.

Signed-off-by: Arvind Yadav 
---
 sound/soc/omap/omap-hdmi-audio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/omap/omap-hdmi-audio.c b/sound/soc/omap/omap-hdmi-audio.c
index 3e9cc48..8eeac7c 100644
--- a/sound/soc/omap/omap-hdmi-audio.c
+++ b/sound/soc/omap/omap-hdmi-audio.c
@@ -362,6 +362,9 @@ static int omap_hdmi_audio_probe(struct platform_device 
*pdev)
 
card->name = devm_kasprintf(dev, GFP_KERNEL,
"HDMI %s", dev_name(ad->dssdev));
+   if (!card->name)
+   return -ENOMEM;
+
card->owner = THIS_MODULE;
card->dai_link =
devm_kzalloc(dev, sizeof(*(card->dai_link)), GFP_KERNEL);
-- 
1.9.1

[PATCH 2/3] ASoC: fsl-asoc-card: Handle return value of devm_kasprintf

[Arvind Yadav]

devm_kasprintf() can fail here and we must check its return value.

Signed-off-by: Arvind Yadav 
---
 sound/soc/fsl/fsl-asoc-card.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 2db4d0c..86f240b 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -639,6 +639,8 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
devm_kasprintf(&pdev->dev, GFP_KERNEL,
   "ac97-codec.%u",
   (unsigned int)idx);
+   if (!priv->dai_link[0].codec_name)
+   goto asrc_fail;
}
 
priv->dai_link[0].platform_of_node = cpu_np;
-- 
1.9.1

[PATCH 1/3] ASoC: davinci-mcasp: Handle return value of devm_kasprintf

[Arvind Yadav]

devm_kasprintf() can fail here and we must check its return value.

Signed-off-by: Arvind Yadav 
---
 sound/soc/davinci/davinci-mcasp.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/sound/soc/davinci/davinci-mcasp.c 
b/sound/soc/davinci/davinci-mcasp.c
index f395bbc..d1a4aa2 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -1867,6 +1867,10 @@ static int davinci_mcasp_probe(struct platform_device 
*pdev)
if (irq >= 0) {
irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_common",
  dev_name(&pdev->dev));
+   if (!irq_name) {
+   ret = -ENOMEM;
+   goto err;
+   }
ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,

davinci_mcasp_common_irq_handler,
IRQF_ONESHOT | IRQF_SHARED,
@@ -1884,6 +1888,10 @@ static int davinci_mcasp_probe(struct platform_device 
*pdev)
if (irq >= 0) {
irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_rx",
  dev_name(&pdev->dev));
+   if (!irq_name) {
+   ret = -ENOMEM;
+   goto err;
+   }
ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
davinci_mcasp_rx_irq_handler,
IRQF_ONESHOT, irq_name, mcasp);
@@ -1899,6 +1907,10 @@ static int davinci_mcasp_probe(struct platform_device 
*pdev)
if (irq >= 0) {
irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_tx",
  dev_name(&pdev->dev));
+   if (!irq_name) {
+   ret = -ENOMEM;
+   goto err;
+   }
ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
davinci_mcasp_tx_irq_handler,
IRQF_ONESHOT, irq_name, mcasp);
-- 
1.9.1

[PATCH 0/3] ASoC: Handle return value of devm_kasprintf

[Arvind Yadav]

devm_kasprintf() can fail here and we must check its return value.

Arvind Yadav (3):
  [PATCH 1/3] ASoC: davinci-mcasp: Handle return value of devm_kasprintf
  [PATCH 2/3] ASoC: fsl-asoc-card: Handle return value of devm_kasprintf
  [PATCH 3/3] ASoC: omap-hdmi-audio: Handle return value of devm_kasprintf

 sound/soc/davinci/davinci-mcasp.c | 12 
 sound/soc/fsl/fsl-asoc-card.c |  2 ++
 sound/soc/omap/omap-hdmi-audio.c  |  3 +++
 3 files changed, 17 insertions(+)

-- 
1.9.1

[PATCH] powerpc: 4xx: constify platform_suspend_ops

[Arvind Yadav]

platform_suspend_ops are not supposed to change at runtime.
Functions suspend_set_ops working with const platform_suspend_ops.
So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 arch/powerpc/sysdev/ppc4xx_cpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/ppc4xx_cpm.c b/arch/powerpc/sysdev/ppc4xx_cpm.c
index ba95adf..6483675 100644
--- a/arch/powerpc/sysdev/ppc4xx_cpm.c
+++ b/arch/powerpc/sysdev/ppc4xx_cpm.c
@@ -240,7 +240,7 @@ static int cpm_suspend_enter(suspend_state_t state)
return 0;
 }
 
-static struct platform_suspend_ops cpm_suspend_ops = {
+static const struct platform_suspend_ops cpm_suspend_ops = {
.valid  = cpm_suspend_valid,
.enter  = cpm_suspend_enter,
 };
-- 
2.7.4

[PATCH] powerpc/512x: clk: constify clk_div_table

[Arvind Yadav]

clk_div_table are not supposed to change at runtime.
mpc512x_clk_divtable function working with const
clk_div_table. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 arch/powerpc/platforms/512x/clock-commonclk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/512x/clock-commonclk.c 
b/arch/powerpc/platforms/512x/clock-commonclk.c
index add5a53..b3097fe 100644
--- a/arch/powerpc/platforms/512x/clock-commonclk.c
+++ b/arch/powerpc/platforms/512x/clock-commonclk.c
@@ -363,7 +363,7 @@ static int get_cpmf_mult_x2(void)
  */
 
 /* applies to the IPS_DIV, and PCI_DIV values */
-static struct clk_div_table divtab_2346[] = {
+static const struct clk_div_table divtab_2346[] = {
{ .val = 2, .div = 2, },
{ .val = 3, .div = 3, },
{ .val = 4, .div = 4, },
@@ -372,7 +372,7 @@ static int get_cpmf_mult_x2(void)
 };
 
 /* applies to the MBX_DIV, LPC_DIV, and NFC_DIV values */
-static struct clk_div_table divtab_1234[] = {
+static const struct clk_div_table divtab_1234[] = {
{ .val = 1, .div = 1, },
{ .val = 2, .div = 2, },
{ .val = 3, .div = 3, },
-- 
1.9.1

[PATCH] hwrng: pseries: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/char/hw_random/pseries-rng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/pseries-rng.c 
b/drivers/char/hw_random/pseries-rng.c
index d9f46b4..4e2a3f6 100644
--- a/drivers/char/hw_random/pseries-rng.c
+++ b/drivers/char/hw_random/pseries-rng.c
@@ -72,7 +72,7 @@ static int pseries_rng_remove(struct vio_dev *dev)
return 0;
 }
 
-static struct vio_device_id pseries_rng_driver_ids[] = {
+static const struct vio_device_id pseries_rng_driver_ids[] = {
{ "ibm,random-v1", "ibm,random"},
{ "", "" }
 };
-- 
2.7.4

[PATCH] tpm: vtpm: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/char/tpm/tpm_ibmvtpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index f01d083..d2ce46b 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -32,7 +32,7 @@
 
 static const char tpm_ibmvtpm_driver_name[] = "tpm_ibmvtpm";
 
-static struct vio_device_id tpm_ibmvtpm_device_table[] = {
+static const struct vio_device_id tpm_ibmvtpm_device_table[] = {
{ "IBM,vtpm", "IBM,vtpm"},
{ "", "" }
 };
-- 
2.7.4

[PATCH 0/3] constify scsi vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Arvind Yadav (3):
  [PATCH 1/3] scsi: ibmvfc: constify vio_device_id
  [PATCH 2/3] scsi: ibmvscsi: constify vio_device_id
  [PATCH 3/3] scsi: ibmvscsi_tgt: constify vio_device_id

 drivers/scsi/ibmvscsi/ibmvfc.c   | 2 +-
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +-
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 3/3] scsi: ibmvscsi_tgt: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 659ab48..7575276 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3956,7 +3956,7 @@ static struct class ibmvscsis_class = {
.dev_groups = ibmvscsis_dev_groups,
 };
 
-static struct vio_device_id ibmvscsis_device_table[] = {
+static const struct vio_device_id ibmvscsis_device_table[] = {
{ "v-scsi-host", "IBM,v-scsi-host" },
{ "", "" }
 };
-- 
2.7.4

[PATCH 2/3] scsi: ibmvscsi: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index da22b36..7d156b161 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -2330,7 +2330,7 @@ static int ibmvscsi_resume(struct device *dev)
  * ibmvscsi_device_table: Used by vio.c to match devices in the device tree we 
  * support.
  */
-static struct vio_device_id ibmvscsi_device_table[] = {
+static const struct vio_device_id ibmvscsi_device_table[] = {
{"vscsi", "IBM,v-scsi"},
{ "", "" }
 };
-- 
2.7.4

[PATCH 1/3] scsi: ibmvfc: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index cc4e05b..87d83b1 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -4929,7 +4929,7 @@ static unsigned long ibmvfc_get_desired_dma(struct 
vio_dev *vdev)
return pool_dma + ((512 * 1024) * driver_template.cmd_per_lun);
 }
 
-static struct vio_device_id ibmvfc_device_table[] = {
+static const struct vio_device_id ibmvfc_device_table[] = {
{"fcp", "IBM,vfc-client"},
{ "", "" }
 };
-- 
2.7.4

[PATCH] tty: hvcs: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/tty/hvc/hvcs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 79cc5be..7320190 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -675,7 +675,7 @@ static int khvcsd(void *unused)
return 0;
 }
 
-static struct vio_device_id hvcs_driver_table[] = {
+static const struct vio_device_id hvcs_driver_table[] = {
{"serial-server", "hvterm2"},
{ "", "" }
 };
-- 
2.7.4

[PATCH] tty: hvc_vio: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/tty/hvc/hvc_vio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c
index b05dc50..39002a1 100644
--- a/drivers/tty/hvc/hvc_vio.c
+++ b/drivers/tty/hvc/hvc_vio.c
@@ -53,7 +53,7 @@
 
 static const char hvc_driver_name[] = "hvc_console";
 
-static struct vio_device_id hvc_driver_table[] = {
+static const struct vio_device_id hvc_driver_table[] = {
{"serial", "hvterm1"},
 #ifndef HVC_OLD_HVSI
{"serial", "hvterm-protocol"},
-- 
2.7.4

[PATCH] net: ibm: ibmvnic: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index a3e6946..d5372b5 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3859,7 +3859,7 @@ static int ibmvnic_resume(struct device *dev)
return 0;
 }
 
-static struct vio_device_id ibmvnic_device_table[] = {
+static const struct vio_device_id ibmvnic_device_table[] = {
{"network", "IBM,vnic"},
{"", "" }
 };
-- 
2.7.4

[PATCH] net: ibm: ibmveth: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/ibm/ibmveth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index d17c2b0..f210398 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1897,7 +1897,7 @@ static int ibmveth_resume(struct device *dev)
return 0;
 }
 
-static struct vio_device_id ibmveth_device_table[] = {
+static const struct vio_device_id ibmveth_device_table[] = {
{ "network", "IBM,l-lan"},
{ "", "" }
 };
-- 
2.7.4

[PATCH] crypto: nx: 842: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/crypto/nx/nx-842-pseries.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/nx/nx-842-pseries.c 
b/drivers/crypto/nx/nx-842-pseries.c
index cddc6d8..bf52cd1 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -1082,7 +1082,7 @@ static int nx842_remove(struct vio_dev *viodev)
return 0;
 }
 
-static struct vio_device_id nx842_vio_driver_ids[] = {
+static const struct vio_device_id nx842_vio_driver_ids[] = {
{"ibm,compression-v1", "ibm,compression"},
{"", ""},
 };
-- 
2.7.4

[PATCH] crypto: nx: constify vio_device_id

[Arvind Yadav]

vio_device_id are not supposed to change at runtime. All functions
working with vio_device_id provided by  work with
const vio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/crypto/nx/nx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index 036057a..3a5e31b 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -833,7 +833,7 @@ static void __exit nx_fini(void)
vio_unregister_driver(&nx_driver.viodriver);
 }
 
-static struct vio_device_id nx_crypto_driver_ids[] = {
+static const struct vio_device_id nx_crypto_driver_ids[] = {
{ "ibm,sym-encryption-v1", "ibm,sym-encryption" },
{ "", "" }
 };
-- 
2.7.4

[PATCH 3/3] PCI: hotplug: constify attribute_group structures.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work
with const attribute_group. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
418 160   8 586 24a drivers/pci/hotplug/rpadlpar_sysfs.o

File size After adding 'const':
   textdata bss dec hex filename
482  96   8 586 232 drivers/pci/hotplug/rpadlpar_sysfs.o

Signed-off-by: Arvind Yadav 
---
 drivers/pci/hotplug/rpadlpar_sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c 
b/drivers/pci/hotplug/rpadlpar_sysfs.c
index a796301..edb5d8a 100644
--- a/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -102,7 +102,7 @@ static ssize_t remove_slot_show(struct kobject *kobj,
NULL,
 };
 
-static struct attribute_group dlpar_attr_group = {
+static const struct attribute_group dlpar_attr_group = {
.attrs = default_attrs,
 };
 
-- 
1.9.1

[PATCH 0/3] constify pci attribute_group structures.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work
with const attribute_group. So mark the non-const structs as const.

Arvind Yadav (3):
  [PATCH 1/3] PCI: pci-sysfs: constify attribute_group structures.
  [PATCH 2/3] PCI: pci-label: constify attribute_group structures.
  [PATCH 3/3] PCI: hotplug: constify attribute_group structures.

 drivers/pci/hotplug/rpadlpar_sysfs.c |  2 +-
 drivers/pci/pci-label.c  |  4 ++--
 drivers/pci/pci-sysfs.c  | 10 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

-- 
1.9.1

[PATCH] scsi: ibmvfc: constify dev_pm_ops structures.

[Arvind Yadav]

dev_pm_ops are not supposed to change at runtime. All functions
working with dev_pm_ops provided by  work with const
dev_pm_ops. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
  419371296  20   43253a8f5 drivers/scsi/ibmvscsi/ibmvfc.o

File size After adding 'const':
   textdata bss dec hex filename
  421291104  20   43253a8f5 drivers/scsi/ibmvscsi/ibmvfc.o

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 26cd3c2..cc4e05b 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -4935,7 +4935,7 @@ static unsigned long ibmvfc_get_desired_dma(struct 
vio_dev *vdev)
 };
 MODULE_DEVICE_TABLE(vio, ibmvfc_device_table);
 
-static struct dev_pm_ops ibmvfc_pm_ops = {
+static const struct dev_pm_ops ibmvfc_pm_ops = {
.resume = ibmvfc_resume
 };
 
-- 
1.9.1

[PATCH] scsi: ibmvscsi: constify dev_pm_ops structures.

[Arvind Yadav]

dev_pm_ops are not supposed to change at runtime. All functions
working with dev_pm_ops provided by  work with const
dev_pm_ops. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
  179561456   8   194204bdc drivers/scsi/ibmvscsi/ibmvscsi.o

File size After adding 'const':
   textdata bss dec hex filename
  181641264   8   194364bec drivers/scsi/ibmvscsi/ibmvscsi.o

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 1deb0a9..da22b36 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -2336,7 +2336,7 @@ static int ibmvscsi_resume(struct device *dev)
 };
 MODULE_DEVICE_TABLE(vio, ibmvscsi_device_table);
 
-static struct dev_pm_ops ibmvscsi_pm_ops = {
+static const struct dev_pm_ops ibmvscsi_pm_ops = {
.resume = ibmvscsi_resume
 };
 
-- 
1.9.1

[PATCH] net: ibm: ibmveth: constify dev_pm_ops structures.

[Arvind Yadav]

dev_pm_ops are not supposed to change at runtime. All functions
working with dev_pm_ops provided by  work with const
dev_pm_ops. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
  154261256   0   16682412a drivers/net/ethernet/ibm/ibmveth.o

File size After adding 'const':
   textdata bss dec hex filename
  156181064   0   16682412a drivers/net/ethernet/ibm/ibmveth.o

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/ibm/ibmveth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 72ab7b6..02b26bf 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1843,7 +1843,7 @@ static int ibmveth_resume(struct device *dev)
 };
 MODULE_DEVICE_TABLE(vio, ibmveth_device_table);
 
-static struct dev_pm_ops ibmveth_pm_ops = {
+static const struct dev_pm_ops ibmveth_pm_ops = {
.resume = ibmveth_resume
 };
 
-- 
1.9.1

[PATCH v2] macintosh: rack-meter: make of_device_ids const.

[Arvind Yadav]

of_device_ids are not supposed to change at runtime. All functions
working with of_device_ids provided by  work with const
of_device_ids. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
407 576   0 983 3d7 drivers/macintosh/rack-meter.o

File size after constify rackmeter_match.
   textdata bss dec hex filename
807 176   0 983 3d7 drivers/macintosh/rack-meter.o

Signed-off-by: Arvind Yadav 
---
Changes in v1:
  Subject was wrong. It should be constify rackmeter_match.

 drivers/macintosh/rack-meter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index e199fd6..35582eb 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -579,7 +579,7 @@ static int rackmeter_shutdown(struct macio_dev* mdev)
return 0;
 }
 
-static struct of_device_id rackmeter_match[] = {
+static const struct of_device_id rackmeter_match[] = {
{ .name = "i2s" },
{ }
 };
-- 
1.9.1

[PATCH] macintosh: rack-meter: make of_device_ids const.

[Arvind Yadav]

of_device_ids are not supposed to change at runtime. All functions
working with of_device_ids provided by  work with const
of_device_ids. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
407 576   0 983 3d7 drivers/macintosh/rack-meter.o

File size after constify keystone_qmss_of_match.
   textdata bss dec hex filename
807 176   0 983 3d7 drivers/macintosh/rack-meter.o

Signed-off-by: Arvind Yadav 
---
 drivers/macintosh/rack-meter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index e199fd6..35582eb 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -579,7 +579,7 @@ static int rackmeter_shutdown(struct macio_dev* mdev)
return 0;
 }
 
-static struct of_device_id rackmeter_match[] = {
+static const struct of_device_id rackmeter_match[] = {
{ .name = "i2s" },
{ }
 };
-- 
1.9.1

[PATCH] dma/fsldma : Unmap region obtained by of_iomap

[Arvind Yadav]

Free memory mapping, if probe is not successful.

Signed-off-by: Arvind Yadav 
---
 drivers/dma/fsldma.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 911b717..7ba8944 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1351,7 +1351,7 @@ static int fsldma_of_probe(struct platform_device *op)
if (!fdev->regs) {
dev_err(&op->dev, "unable to ioremap registers\n");
err = -ENOMEM;
-   goto out_free_fdev;
+   goto out_free;
}
 
/* map the channel IRQ if it exists, but don't hookup the handler yet */
@@ -1416,6 +1416,8 @@ static int fsldma_of_probe(struct platform_device *op)
 
 out_free_fdev:
irq_dispose_mapping(fdev->irq);
+   iounmap(fdev->regs);
+out_free:
kfree(fdev);
 out_return:
return err;
-- 
1.7.9.5

[5.3] ucc_geth: Fix to avoid IS_ERR_VALUE abuses and dead code on 64bit systems.

[Arvind Yadav]

   UCC_GETH_RX_STATISTICS_ALIGNMENT);
   if (IS_ERR_VALUE(ugeth->rx_fw_statistics_pram_offset)) {
   if (netif_msg_ifup(ugeth))
pr_err(
"Can not allocate DPRAM memory for p_rx_fw_statistics_pram\n");
   return -ENOMEM;
}
   /* Size varies with number of Rx queues */
ugeth->rx_irq_coalescing_tbl_offset =
qe_muram_alloc(ug_info->numQueuesRx *
   sizeof(struct ucc_geth_rx_interrupt_coalescing_entry)
   + 4, UCC_GETH_RX_INTERRUPT_COALESCING_ALIGNMENT);
if (IS_ERR_VALUE(ugeth->rx_irq_coalescing_tbl_offset)) {
   if (netif_msg_ifup(ugeth))
pr_err(
"Can not allocate DPRAM memory for p_rx_irq_coalescing_tbl\n");
   return -ENOMEM;
}
/* Size varies with number of Rx queues */
ugeth->rx_bd_qs_tbl_offset =
   qe_muram_alloc(ug_info->numQueuesRx *
   (sizeof(struct ucc_geth_rx_bd_queues_entry) +
sizeof(struct ucc_geth_rx_prefetched_bds)),
   UCC_GETH_RX_BD_QUEUES_ALIGNMENT);
if (IS_ERR_VALUE(ugeth->rx_bd_qs_tbl_offset)) {
   if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for p_rx_bd_qs_tbl\n");
   return -ENOMEM;
}
ugeth->exf_glbl_param_offset =
   qe_muram_alloc(sizeof(struct ucc_geth_exf_global_pram),
 UCC_GETH_RX_EXTENDED_FILTERING_GLOBAL_PARAMETERS_ALIGNMENT);
if (IS_ERR_VALUE(ugeth->exf_glbl_param_offset)) {
   if (netif_msg_ifup(ugeth))
 pr_err(
"Can not allocate DPRAM memory for p_exf_glbl_param\n");
   return -ENOMEM;
}

/* Allocate InitEnet command parameter structure */
init_enet_pram_offset =
  qe_muram_alloc(sizeof(struct ucc_geth_init_pram), 4);
if (IS_ERR_VALUE(init_enet_pram_offset)) {
  if (netif_msg_ifup(ugeth))
pr_err(
   "Can not allocate DPRAM memory for p_init_enet_pram\n");
  return -ENOMEM;
}
p_init_enet_pram =
 (struct ucc_geth_init_pram __iomem *)
 qe_muram_addr(init_enet_pram_offset);

qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.
Return value store in a u32 (init_enet_offset, exf_glbl_param_offset,
rx_glbl_pram_offset, tx_glbl_pram_offset, send_q_mem_reg_offset,
thread_dat_tx_offset, thread_dat_rx_offset, scheduler_offset,
tx_fw_statistics_pram_offset, rx_fw_statistics_pram_offset,
rx_irq_coalescing_tbl_offset, rx_bd_qs_tbl_offset, tx_bd_ring_offset,
init_enet_pram_offset and rx_bd_ring_offset).
If qe_muram_alloc will return any error, Then IS_ERR_VALUE will always
return 0. it'll not call ucc_fast_free for any failure. Inside 'if code'
will be a dead code on 64bit. Even qe_muram_addr will return wrong
virtual address. Which can cause an error.

 kfree((void *)ugeth->tx_bd_ring_offset[i]);

which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
that also holds the return value of qe_muram_alloc.
This patch is to avoid these problem on 64bit machine.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c |  7 ---
 drivers/net/ethernet/freescale/ucc_geth.h | 26 +-
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..eb1f4e2 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -273,7 +273,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
  unsigned int risc,
  int skip_page_for_first_entry)
 {
-   u32 init_enet_offset;
+   unsigned long init_enet_offset;
u8 i;
int snum;
 
@@ -1871,7 +1871,7 @@ static void ucc_geth_free_rx(struct ucc_geth_private 
*ugeth)
 
if (ugeth->ug_info->uf_info.bd_mem_part ==
MEM_PART_SYSTEM)
-   kfree((void *)ugeth->rx_bd_ring_offset[i]);
+   kfree(ugeth->rx_bd_ring_offset[i]);
else if (ugeth->ug_info->uf_info.bd_mem_part ==
 MEM_PART_MURAM)
qe_muram_free(ugeth->rx_bd_ring_offset[i]);
@@ -2367,7 +2367,8 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
struct ucc_geth __iomem *ug_regs;
int ret_val = -EINVAL;
u32 remoder = UCC_GETH_REMODER_INIT;
-   u32 init_enet_pram_offset, cecr_subblock, command;
+   unsigned long init_enet_pram_offset;
+   u32 cecr_subblock, command;
u32 ifstat, i, j, size, l2qt, l3qt;
u16 temoder 

[v5.2] ucc_slow: Fix to avoid IS_ERR_VALUE abuses and dead code on 64bit systems.

[Arvind Yadav]

IS_ERR_VALUE() assumes that parameter is an unsigned long.
It can not be used to check if 'unsigned int' is passed insted.
Which tends to reflect an error.
In 64bit architectures sizeof (int) == 4 && sizeof (long) == 8.
IS_ERR_VALUE(x) is ((x) >= (unsigned long)-4095).
IS_ERR_VALUE() of 'unsigned int' is always false because the 32bit
value is zero extended to 64 bits.

Now Problem In UCC slow protocols -: drivers/soc/fsl/qe/ucc_slow.c

/* Get PRAM base */
uccs->us_pram_offset =
   qe_muram_alloc(UCC_SLOW_PRAM_SIZE, ALIGNMENT_OF_UCC_SLOW_PRAM);
if (IS_ERR_VALUE(uccs->us_pram_offset)) {
   printk(KERN_ERR "%s: cannot allocate MURAM for PRAM", __func__);
   ucc_slow_free(uccs);
   return -ENOMEM;
}
id = ucc_slow_get_qe_cr_subblock(us_info->ucc_num);
qe_issue_cmd(QE_ASSIGN_PAGE_TO_DEVICE, id, us_info->protocol,
 uccs->us_pram_offset);

uccs->us_pram = qe_muram_addr(uccs->us_pram_offset);

/* Allocate BDs. */
uccs->rx_base_offset =
qe_muram_alloc(us_info->rx_bd_ring_len * sizeof(struct qe_bd),
QE_ALIGNMENT_OF_BD);
if (IS_ERR_VALUE(uccs->rx_base_offset)) {
printk(KERN_ERR "%s: cannot allocate %u RX BDs\n", __func__,
us_info->rx_bd_ring_len);
uccs->rx_base_offset = 0;
ucc_slow_free(uccs);
return -ENOMEM;
}

uccs->tx_base_offset =
 qe_muram_alloc(us_info->tx_bd_ring_len * sizeof(struct qe_bd),
QE_ALIGNMENT_OF_BD);
if (IS_ERR_VALUE(uccs->tx_base_offset)) {
 printk(KERN_ERR "%s: cannot allocate TX BDs", __func__);
 uccs->tx_base_offset = 0;
 ucc_slow_free(uccs);
 return -ENOMEM;
}

qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.
Return value store in a u32 (us_pram_offset, rx_base_offset
and tx_base_offset).If qe_muram_alloc will return any error,
Then IS_ERR_VALUE will always return 0. it'll not call ucc_fast_free
for any failure. Inside 'if code' will be a dead code on 64bit.
Even  qe_muram_addr will return wrong virtual address. Which
can cause an error.
This patch is to avoid this problem on 64bit machine.

Signed-off-by: Arvind Yadav 
---
 include/soc/fsl/qe/ucc_slow.h | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/soc/fsl/qe/ucc_slow.h b/include/soc/fsl/qe/ucc_slow.h
index 6c0573a..fca30a1 100644
--- a/include/soc/fsl/qe/ucc_slow.h
+++ b/include/soc/fsl/qe/ucc_slow.h
@@ -189,7 +189,7 @@ struct ucc_slow_private {
struct ucc_slow_info *us_info;
struct ucc_slow __iomem *us_regs; /* Ptr to memory map of UCC regs */
struct ucc_slow_pram *us_pram;  /* a pointer to the parameter RAM */
-   u32 us_pram_offset;
+   unsigned long us_pram_offset;
int enabled_tx; /* Whether channel is enabled for Tx (ENT) */
int enabled_rx; /* Whether channel is enabled for Rx (ENR) */
int stopped_tx; /* Whether channel has been stopped for Tx
@@ -198,8 +198,12 @@ struct ucc_slow_private {
struct list_head confQ; /* frames passed to chip waiting for tx */
u32 first_tx_bd_mask;   /* mask is used in Tx routine to save status
   and length for first BD in a frame */
-   u32 tx_base_offset; /* first BD in Tx BD table offset (In MURAM) */
-   u32 rx_base_offset; /* first BD in Rx BD table offset (In MURAM) */
+   unsigned long tx_base_offset;   /* first BD in Tx BD table offset
+* (In MURAM)
+*/
+   unsigned long rx_base_offset;   /* first BD in Rx BD table offset
+* (In MURAM)
+*/
struct qe_bd *confBd;   /* next BD for confirm after Tx */
struct qe_bd *tx_bd;/* next BD for new Tx request */
struct qe_bd *rx_bd;/* next BD to collect after Rx */
-- 
1.9.1

Re: [v5.1] ucc_fast: Fix to avoid IS_ERR_VALUE abuses and dead code on 64bit systems.

[arvind Yadav]

On Friday 05 August 2016 02:01 AM, Arnd Bergmann wrote:

On Thursday, August 4, 2016 10:22:43 PM CEST Arvind Yadav wrote:

index df8ea79..ada9070 100644
--- a/include/soc/fsl/qe/ucc_fast.h
+++ b/include/soc/fsl/qe/ucc_fast.h
@@ -165,10 +165,12 @@ struct ucc_fast_private {
 int stopped_tx; /* Whether channel has been stopped for Tx
(STOP_TX, etc.) */
 int stopped_rx; /* Whether channel has been stopped for Rx */
-   u32 ucc_fast_tx_virtual_fifo_base_offset;/* pointer to base of Tx
-   virtual fifo */
-   u32 ucc_fast_rx_virtual_fifo_base_offset;/* pointer to base of Rx
-   virtual fifo */
+   unsigned long ucc_fast_tx_virtual_fifo_base_offset;/* pointer to base of
+   * Tx virtual fifo
+   */
+   unsigned long ucc_fast_rx_virtual_fifo_base_offset;/* pointer to base of
+   * Rx virtual fifo
+   */
  #ifdef STATISTICS
 u32 tx_frames;  /* Transmitted frames counter. */
 u32 rx_frames;  /* Received frames counter (only frames


This change seems ok, but what about the other u32 variables in ucc_geth.c
that get checked for IS_ERR_VALUE?

Arnd
I have send separate patch for ucc_geth ans ucc_slow.
-Arvind

[v5.1] ucc_fast: Fix to avoid IS_ERR_VALUE abuses and dead code on 64bit systems.

[Arvind Yadav]

IS_ERR_VALUE() assumes that parameter is an unsigned long.
It can not be used to check if 'unsigned int' is passed insted.
Which tends to reflect an error.
In 64bit architectures sizeof (int) == 4 && sizeof (long) == 8.
IS_ERR_VALUE(x) is ((x) >= (unsigned long)-4095).
IS_ERR_VALUE() of 'unsigned int' is always false because the 32bit
value is zero extended to 64 bits.

Now Problem In UCC fast protocols -: drivers/soc/fsl/qe/ucc_fast.c

/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
  qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
}

/* Allocate memory for Rx Virtual Fifo */
uccf->ucc_fast_rx_virtual_fifo_base_offset =
   qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
}

qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.
Return value store in a u32 (ucc_fast_tx_virtual_fifo_base_offset
and ucc_fast_rx_virtual_fifo_base_offset).If qe_muram_alloc will
return any error, Then IS_ERR_VALUE will always return 0. it'll not
call ucc_fast_free for any failure. Inside 'if code' will be a dead
code on 64bit.
This patch is to avoid this problem on 64bit machine.

Signed-off-by: Arvind Yadav 
---
 include/soc/fsl/qe/ucc_fast.h | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/soc/fsl/qe/ucc_fast.h b/include/soc/fsl/qe/ucc_fast.h
index df8ea79..ada9070 100644
--- a/include/soc/fsl/qe/ucc_fast.h
+++ b/include/soc/fsl/qe/ucc_fast.h
@@ -165,10 +165,12 @@ struct ucc_fast_private {
int stopped_tx; /* Whether channel has been stopped for Tx
   (STOP_TX, etc.) */
int stopped_rx; /* Whether channel has been stopped for Rx */
-   u32 ucc_fast_tx_virtual_fifo_base_offset;/* pointer to base of Tx
-   virtual fifo */
-   u32 ucc_fast_rx_virtual_fifo_base_offset;/* pointer to base of Rx
-   virtual fifo */
+   unsigned long ucc_fast_tx_virtual_fifo_base_offset;/* pointer to base of
+   * Tx virtual fifo
+   */
+   unsigned long ucc_fast_rx_virtual_fifo_base_offset;/* pointer to base of
+   * Rx virtual fifo
+   */
 #ifdef STATISTICS
u32 tx_frames;  /* Transmitted frames counter. */
u32 rx_frames;  /* Received frames counter (only frames
-- 
1.9.1

Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.

[arvind Yadav]

On Wednesday 03 August 2016 01:27 AM, Scott Wood wrote:

On 08/02/2016 10:34 AM, arvind Yadav wrote:


On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote:

On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote:

On 08/01/2016 02:02 AM, Arnd Bergmann wrote:

diff --git a/include/linux/err.h b/include/linux/err.h
index 1e35588..c2a2789 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,17 @@
  
  #ifndef __ASSEMBLY__
  
-#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)

+#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
+
+static inline int is_error_check(unsigned long error)

Please leave the existing macro alone. I think you were looking for
something specific to the return code of qe_muram_alloc() function,
so please add a helper in that subsystem if you need it, not in
the generic header files.

qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.  The
problem is certain callers that store the return value in a u32.  Why
not just fix those callers to store it in unsigned long (at least until
error checking is done)?


Yes, that would also address another problem with code like

  kfree((void *)ugeth->tx_bd_ring_offset[i]);

which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
that also holds the return value of qe_muram_alloc.

Well, hopefully it doesn't hold a return of qe_muram_alloc() when it's
being passed to kfree()...

There's also the code that casts kmalloc()'s return to u32, etc.
ucc_geth is not 64-bit clean in general.


Arnd

Yes, we will fix caller. Caller api is not safe on 64bit.

The API is fine (or at least, I haven't seen a valid issue pointed out
yet).  The problem is the ucc_geth driver.


Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int,
but it should be unsigned long.

cpm_muram_addr takes unsigned long as a parameter, not that it matters
since you can't pass errors into it and a muram offset should never
exceed 32 bits.

-Scott

Yes, It will work for 32bit machine. But will not safe for 64bit.

Example :
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length  UCC_GETH_TX_BD_RING_ALIGNMENT);
if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
   ugeth->p_tx_bd_ring[j] =
   (u8 __iomem *) qe_muram_addr(ugeth-> tx_bd_ring_offset[j]);

If qe_muram_alloc will return any error,  IS_ERR_VALUE will
always return 0 (IS_ERR_VALUE will always pass for 'unsigned int').
Now qe_muram_addr will return wrong virtual address. Which
can cause an error.

-Arvind

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.

[arvind Yadav]

On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote:

On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote:

On 08/01/2016 02:02 AM, Arnd Bergmann wrote:

diff --git a/include/linux/err.h b/include/linux/err.h
index 1e35588..c2a2789 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,17 @@
  
  #ifndef __ASSEMBLY__
  
-#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)

+#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
+
+static inline int is_error_check(unsigned long error)

Please leave the existing macro alone. I think you were looking for
something specific to the return code of qe_muram_alloc() function,
so please add a helper in that subsystem if you need it, not in
the generic header files.

qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.  The
problem is certain callers that store the return value in a u32.  Why
not just fix those callers to store it in unsigned long (at least until
error checking is done)?


Yes, that would also address another problem with code like

  kfree((void *)ugeth->tx_bd_ring_offset[i]);

which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
that also holds the return value of qe_muram_alloc.

Arnd

Yes, we will fix caller. Caller api is not safe on 64bit.
Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int,
but it should be unsigned long.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.

[arvind Yadav]

On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote:

On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote:

On 08/01/2016 02:02 AM, Arnd Bergmann wrote:

diff --git a/include/linux/err.h b/include/linux/err.h
index 1e35588..c2a2789 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,17 @@
  
  #ifndef __ASSEMBLY__
  
-#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)

+#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
+
+static inline int is_error_check(unsigned long error)

Please leave the existing macro alone. I think you were looking for
something specific to the return code of qe_muram_alloc() function,
so please add a helper in that subsystem if you need it, not in
the generic header files.

qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.  The
problem is certain callers that store the return value in a u32.  Why
not just fix those callers to store it in unsigned long (at least until
error checking is done)?


Yes, that would also address another problem with code like

  kfree((void *)ugeth->tx_bd_ring_offset[i]);

which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
that also holds the return value of qe_muram_alloc.

Arnd

Yes, we will fix caller. Caller api is not safe on 64bit.
Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int,
but it should be unsigned long. Need to work on it.

Arvind
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.

[Arvind Yadav]

IS_ERR_VALUE() assumes that parameter is an unsigned long.
It can not be used to check if 'unsigned int' is passed insted.
Which tends to reflect an error.

In 64bit architectures sizeof (int) == 4 && sizeof (long) == 8.
IS_ERR_VALUE(x) is ((x) >= (unsigned long)-4095).

IS_ERR_VALUE() of 'unsigned int' is always false because the 32bit
value is zero extended to 64 bits.

Value of (unsigned int)-4095 is always less than value of
(unsigned long)-4095.

Now We are taking only first 32 bit for error checking rest of the 32 bit
we ignore such that we get appropriate comparison on 64bit system as well.

First 32bit of Value of (unsigned int)-4095 and (unsigned long)-4095 will
be equal.

Signed-off-by: Arvind Yadav 
---
 include/linux/err.h | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/err.h b/include/linux/err.h
index 1e35588..c2a2789 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -18,7 +18,17 @@
 
 #ifndef __ASSEMBLY__
 
-#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned 
long)-MAX_ERRNO)
+#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
+
+static inline int is_error_check(unsigned long error)
+{
+   unsigned int first32bit = (error & 0x);
+
+   if (first32bit >= (unsigned int)-MAX_ERRNO)
+   return 1;
+   else
+   return 0;
+}
 
 static inline void * __must_check ERR_PTR(long error)
 {
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [v3] UCC_GETH/UCC_FAST: Use IS_ERR_VALUE_U32 API to avoid IS_ERR_VALUE abuses.

[arvind Yadav]

I am also agree with Arnd Bergmann. We should use 'static inline function'
instead of macro to deal with error check.

On Tuesday 26 July 2016 05:09 PM, Arnd Bergmann wrote:

On Saturday, July 23, 2016 11:35:51 PM CEST Arvind Yadav wrote:

diff --git a/include/linux/err.h b/include/linux/err.h
index 1e35588..a42f942 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -19,6 +19,7 @@
  #ifndef __ASSEMBLY__
  
  #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)

+#define IS_ERR_VALUE_U32(x) unlikely((unsigned int)(x) >= (unsigned 
int)-MAX_ERRNO)
  
  static inline void * __must_check ERR_PTR(long error)

  {

This doesn't really look like something we want to have as a generic
interface. The IS_ERR_VALUE() API is rather awkward already, and your
use seems specific to the cpu_muram_alloc() function.

How about something like

int cpm_muram_error(unsigned long addr)
{
if (addr >= (unsigned long)-MAX_ERRNO)
return addr;
else
return 0;
}

and then use that to check the value returned by the allocation
that is still an 'unsigned long', before assigning it to a 'u32'.

Arnd


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[v3] UCC_GETH/UCC_FAST: Use IS_ERR_VALUE_U32 API to avoid IS_ERR_VALUE abuses.

[Arvind Yadav]

IS_ERR_VALUE() assumes that its parameter is an unsigned long.
It can not be used to check if an 'unsigned int' reflects an error.
As they pass an 'unsigned int' into a function that takes an
'unsigned long' argument. This happens to work because the type
is sign-extended on 64-bit architectures before it gets converted
into an unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

It would be nice to any users that are not passing 'unsigned int'
arguments.

Passing value in IS_ERR_VALUE() is wrong, as they pass an
'unsigned int' into a function that takes an 'unsigned long'
argument.This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

Passing an 'unsigned short' or 'unsigned int'argument into
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/bcma/scan.c   |  2 --
 drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
 drivers/soc/fsl/qe/ucc_fast.c |  4 ++--
 include/linux/err.h   |  1 +
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/bcma/scan.c b/drivers/bcma/scan.c
index 4a2d1b2..319d78e 100644
--- a/drivers/bcma/scan.c
+++ b/drivers/bcma/scan.c
@@ -272,8 +272,6 @@ static struct bcma_device *bcma_find_core_reverse(struct 
bcma_bus *bus, u16 core
return NULL;
 }
 
-#define IS_ERR_VALUE_U32(x) ((x) >= (u32)-MAX_ERRNO)
-
 static int bcma_get_next_core(struct bcma_bus *bus, u32 __iomem **eromptr,
  struct bcma_device_id *match, int core_num,
  struct bcma_device *core)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..d290dea 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (IS_ERR_VALUE_U32(init_enet_offset)) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!IS_ERR_VALUE_U32(ugeth->tx_bd_ring_offset[j]))
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!IS_ERR_VALUE_U32(ugeth->rx_bd_ring_offset[j]))
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (IS_ERR_VALUE_U32(ugeth->tx_glbl_pram_offset)) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   

[PATCH v1] Specific requirement of type casting for 64-bit architectures.

[Arvind Yadav]

-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. It will work on 32-bit architectures
But data can be loss on 64-bit architectures if 'qe_muram_alloc' will
return greater then MAX value of 'unsigned int'.

-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'unsigned int'
into a function, It will through this compilation warning.

"
 include/linux/err.h:21:49: warning: cast to pointer from integer of different 
size [-Wint-to-pointer-cast]
 #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned 
long)-MAX_ERRNO)
 ^
 include/linux/compiler.h:170:42: note: in definition of macro ‘unlikely’
 # define unlikely(x) __builtin_expect(!!(x), 0)
"

-Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'unsigned int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..208b198 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -141,6 +141,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
struct ucc_fast __iomem *uf_regs;
u32 gumr;
int ret;
+   unsigned long ret_muram;
 
if (!uf_info)
return -EINVAL;
@@ -265,28 +266,34 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
gumr |= uf_info->mode;
out_be32(&uf_regs->gumr, gumr);
 
-   /* Allocate memory for Tx Virtual Fifo */
-   uccf->ucc_fast_tx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   ret_muram =
+   qe_muram_alloc(uf_info->utfs,
+   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
+
+   if (IS_ERR_VALUE(ret_muram)) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
+   } else {
+   /* Allocate memory for Tx Virtual Fifo */
+   uccf->ucc_fast_tx_virtual_fifo_base_offset = (u32)ret_muram;
}
 
-   /* Allocate memory for Rx Virtual Fifo */
-   uccf->ucc_fast_rx_virtual_fifo_base_offset =
+   ret_muram =
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (IS_ERR_VALUE(ret_muram)) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
+   } else {
+   /* Allocate memory for Rx Virtual Fifo */
+   uccf->ucc_fast_rx_virtual_fifo_base_offset = (u32)ret_muram;
}
 
/* Set Virtual Fifo registers */
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: Need proper type casting before assignment, Remove compilation Warning.

[arvind Yadav]

As per you concern, I have submitted one more patch with some changes. 
Please review it.


Thanks,

On Friday 08 July 2016 09:03 PM, Guenter Roeck wrote:

On Thu, Jul 07, 2016 at 10:31:11PM +0530, Arvind Yadav wrote:

-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. These variable are 'unsigned int'.
So before assginment need a proper type casting.

Are they ? In the upstream kernel, they seem to be "u32".

Yes, I have changed as per you suggestion.



-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'
into a function that takes an 'unsigned long' argument.This happens
to work because the type is sign-extended on 64-bit architectures
before it gets converted into an unsigned type.


Not really sure I understand if/how this applies to the patch in question.
I don't see an int passed to IS_ERR_VALUE(), I only see u32.

-Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'unsigned int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

-Passing an 'unsigned short' or 'unsigned int'argument into
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.


What does this have to do with this patch ?

Specific requirement of type casting for 64-bit architectures.

-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. It will work on 32-bit architectures
But data can be loss on 64-bit architectures if 'qe_muram_alloc' will return
greater then MAX value of 'unsigned int'.


-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'unsigned int'
into a function, It will through this compilation warning.
"
 include/linux/err.h:21:49: warning: cast to pointer from integer of 
different size [-Wint-to-pointer-cast]
 #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= 
(unsigned long)-MAX_ERRNO)

 ^
 include/linux/compiler.h:170:42: note: in definition of macro ‘unlikely’
 # define unlikely(x) __builtin_expect(!!(x), 0)
"

-Any user will get compilation warning for that do not pass an
unsigned long' argument.


Sure, but that doesn't mean that typecasting the parameter to unsigned long
does any good (other than hiding the real bug).

Your subject line still does not list the affected subsystem and/or driver.
Documentation/SubmittingPatches might give some hints about proper subject
lines, and looking at other patches applied to the same file(s) might help
as well.

Also, if you want someone to review your patches, it helps to Cc: that
someone.

Thanks, For your suggestion.

Signed-off-by: Arvind Yadav 
---
  drivers/soc/fsl/qe/ucc_fast.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..98eed25 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -267,8 +267,10 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
  
  	/* Allocate memory for Tx Virtual Fifo */

uccf->ucc_fast_tx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   (unsigned int)qe_muram_alloc(uf_info->utfs,

I don't see the point of this typecast.


+   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
+   if (IS_ERR_VALUE(
+   (unsigned long)uccf->ucc_fast_tx_virtual_fifo_base_offset)) {

If sizeof(u32) == sizeof(unsigned long), this patch does not have an effect.
If sizeof(u32) < sizeof(unsigned long), it does not change anything, and the
resulting code is as wrong as it was before.


printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -278,10 +280,11 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
  
  	/* Allocate memory for Rx Virtual Fifo */

uccf->ucc_fast_rx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->urfs +
+   (unsigned int)qe_muram_alloc(uf_info->urfs +

Re: Need proper type casting before assignment, Remove compilation Warning.

[arvind Yadav]

As per your concern, I have changed and submitted one more patch.

This answer of your all questions,
-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. It will work on 32-bit architectures
But data can be loss on 64-bit architectures if 'qe_muram_alloc' will return
greater then MAX value of 'unsigned int'.


-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'unsigned int'
into a function, It will through this compilation warning.
"
 include/linux/err.h:21:49: warning: cast to pointer from integer of 
different size [-Wint-to-pointer-cast]
 #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= 
(unsigned long)-MAX_ERRNO)

 ^
 include/linux/compiler.h:170:42: note: in definition of macro ‘unlikely’
 # define unlikely(x) __builtin_expect(!!(x), 0)
"

-Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'unsigned int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.


Thanks,
Arvind Yadav

On Friday 08 July 2016 09:03 PM, Guenter Roeck wrote:

On Thu, Jul 07, 2016 at 10:31:11PM +0530, Arvind Yadav wrote:

-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. These variable are 'unsigned int'.
So before assginment need a proper type casting.

Are they ? In the upstream kernel, they seem to be "u32".
-Yes, I have changed as per you suggestion.

-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'
into a function that takes an 'unsigned long' argument.This happens
to work because the type is sign-extended on 64-bit architectures
before it gets converted into an unsigned type.


Not really sure I understand if/how this applies to the patch in question.
I don't see an int passed to IS_ERR_VALUE(), I only see u32.


-Passing an 'unsigned short' or 'unsigned int'argument into
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.


What does this have to do with this patch ?


-Any user will get compilation warning for that do not pass an
unsigned long' argument.


Sure, but that doesn't mean that typecasting the parameter to unsigned long
does any good (other than hiding the real bug).

Your subject line still does not list the affected subsystem and/or driver.
Documentation/SubmittingPatches might give some hints about proper subject
lines, and looking at other patches applied to the same file(s) might help
as well.

Also, if you want someone to review your patches, it helps to Cc: that
someone.


Signed-off-by: Arvind Yadav 
---
  drivers/soc/fsl/qe/ucc_fast.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..98eed25 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -267,8 +267,10 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
  
  	/* Allocate memory for Tx Virtual Fifo */

uccf->ucc_fast_tx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   (unsigned int)qe_muram_alloc(uf_info->utfs,

I don't see the point of this typecast.


+   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
+   if (IS_ERR_VALUE(
+   (unsigned long)uccf->ucc_fast_tx_virtual_fifo_base_offset)) {

If sizeof(u32) == sizeof(unsigned long), this patch does not have an effect.
If sizeof(u32) < sizeof(unsigned long), it does not change anything, and the
resulting code is as wrong as it was before.


printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -278,10 +280,11 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
  
  	/* Allocate memory for Rx Virtual Fifo */

uccf->ucc_fast_rx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->urfs +
+   (unsigned int)qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIR

[PATCH v1] Specific requirement of type casting for 64-bit architectures.

[Arvind Yadav]

-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. It will work on 32-bit architectures
But data can be loss on 64-bit architectures if 'qe_muram_alloc' will
return greater then MAX value of 'unsigned int'.

-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'unsigned int'
into a function, It will through this compilation warning.

"
 include/linux/err.h:21:49: warning: cast to pointer from integer of different 
size [-Wint-to-pointer-cast]
 #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned 
long)-MAX_ERRNO)
 ^
 include/linux/compiler.h:170:42: note: in definition of macro ‘unlikely’
 # define unlikely(x) __builtin_expect(!!(x), 0)
"

-Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'unsigned int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..208b198 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -141,6 +141,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
struct ucc_fast __iomem *uf_regs;
u32 gumr;
int ret;
+   unsigned long ret_muram;
 
if (!uf_info)
return -EINVAL;
@@ -265,28 +266,34 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
gumr |= uf_info->mode;
out_be32(&uf_regs->gumr, gumr);
 
-   /* Allocate memory for Tx Virtual Fifo */
-   uccf->ucc_fast_tx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   ret_muram =
+   qe_muram_alloc(uf_info->utfs,
+   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
+
+   if (IS_ERR_VALUE(ret_muram)) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
+   } else {
+   /* Allocate memory for Tx Virtual Fifo */
+   uccf->ucc_fast_tx_virtual_fifo_base_offset = (u32)ret_muram;
}
 
-   /* Allocate memory for Rx Virtual Fifo */
-   uccf->ucc_fast_rx_virtual_fifo_base_offset =
+   ret_muram =
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (IS_ERR_VALUE(ret_muram)) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
+   } else {
+   /* Allocate memory for Rx Virtual Fifo */
+   uccf->ucc_fast_rx_virtual_fifo_base_offset = (u32)ret_muram;
}
 
/* Set Virtual Fifo registers */
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: Removing lots of IS_ERR_VALUE abuses and compilation warning.

[arvind Yadav]

Yes, You are right,

-Now Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying 
to assigned in member of this structure 'ucc_geth_private' . This 
structure variable are 'unsigned int'. So before assignment need a 
proper type casting.


-Passing value in IS_ERR_VALUE() is wrong. So this is also need a proper 
type casting before passing an argument.


 ugeth->tx_bd_ring_offset[j] =
-qe_muram_alloc(length,
+(u32)qe_muram_alloc(length, UCC_GETH_RX_BD_RING_ALIGNMENT);

-if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!IS_ERR_VALUE((unsigned 
long)ugeth->rx_bd_ring_offset[j]))




I have done the changes and re-submitted anther patch, Please review It.

Thanks,
Arvind

On Thursday 07 July 2016 10:47 PM, Guenter Roeck wrote:

On Thu, Jul 07, 2016 at 09:30:14PM +0530, Arvind Yadav wrote:

Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'
into a function that takes an 'unsigned long' argument.This happens
to work because the type is sign-extended on 64-bit architectures
before it gets converted into an unsigned type.

Passing an 'unsigned short' or 'unsigned int'argument into
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
unsigned long' argument.

Commit '287980e49f; - This change is alreday fixes lots of other
Worst abusers

Signed-off-by: Arvind Yadav 
---
  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {

init_enet_offset is defined as u32 and thus never < 0.


if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])

qe_muram_alloc() returns a pointer or offset, not 0, if there is no error,
meaning this change breaks the driver.


ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])

Same here.


ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {

tx_glbl_pram_offset is u32 and thus never < 0.


if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
+   if (ugeth->thread_dat_tx_offset < 0) {

thread_dat_tx_offset is u32 and thus never < 0.


if (netif_ms

[PATCH] Need proper type casting before assignment, Remove compilation Warning.

[Arvind Yadav]

-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. These variable are 'unsigned int'.
So before assginment need a proper type casting.

-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'
into a function that takes an 'unsigned long' argument.This happens
to work because the type is sign-extended on 64-bit architectures
before it gets converted into an unsigned type.

-Passing an 'unsigned short' or 'unsigned int'argument into
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.

-Any user will get compilation warning for that do not pass an
unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c | 71 +--
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..a5086b2 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -273,7 +273,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
  unsigned int risc,
  int skip_page_for_first_entry)
 {
-   u32 init_enet_offset;
+   unsigned long init_enet_offset;
u8 i;
int snum;
 
@@ -297,8 +297,8 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
}
}
*(p_start++) =
-   ((u8) snum << ENET_INIT_PARAM_SNUM_SHIFT) | init_enet_offset
-   | risc;
+   ((u8)snum << ENET_INIT_PARAM_SNUM_SHIFT)
+   | (u32)init_enet_offset | risc;
}
 
return 0;
@@ -2232,9 +2232,10 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
align) & ~(align - 1));
} else if (uf_info->bd_mem_part == MEM_PART_MURAM) {
ugeth->tx_bd_ring_offset[j] =
-   qe_muram_alloc(length,
+   (u32)qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!IS_ERR_VALUE(
+   (unsigned long)ugeth->tx_bd_ring_offset[j]))
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2309,9 +2310,10 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
align) & ~(align - 1));
} else if (uf_info->bd_mem_part == MEM_PART_MURAM) {
ugeth->rx_bd_ring_offset[j] =
-   qe_muram_alloc(length,
+   (u32)qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!IS_ERR_VALUE(
+   (unsigned long)ugeth->rx_bd_ring_offset[j]))
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2367,7 +2369,8 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
struct ucc_geth __iomem *ug_regs;
int ret_val = -EINVAL;
u32 remoder = UCC_GETH_REMODER_INIT;
-   u32 init_enet_pram_offset, cecr_subblock, command;
+   u32 cecr_subblock, command;
+   unsigned long init_enet_pram_offset;
u32 ifstat, i, j, size, l2qt, l3qt;
u16 temoder = UCC_GETH_TEMODER_INIT;
u16 test;
@@ -2519,9 +2522,9 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
/* Tx global PRAM */
/* Allocate global tx parameter RAM page */
ugeth->tx_glbl_pram_offset =
-   qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
+   (u32)qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (IS_ERR_VALUE((unsigned long)ugeth->tx_glbl_pram_offset)) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2537,11 +2540,11 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
/* TQPTR

Re: Remove lots of IS_ERR_VALUE abuses and compilation warning.

[arvind Yadav]

Yes, You are right,
-Now Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying 
to assigned in ucc_fast_tx_virtual_fifo_base_offset and 
ucc_fast_rx_virtual_fifo_base_offset. These variable are 'unsigned int'. 
So before assignment need a proper type casting.


-Passing value in IS_ERR_VALUE() is wrong. So this is also need a proper 
type casting before passing an argument.


I have done the changes and re-submitted anther patch, Please review It.

Thanks,
Arvind

On Thursday 07 July 2016 09:21 PM, Guenter Roeck wrote:

On Thu, Jul 07, 2016 at 08:57:29PM +0530, Arvind Yadav wrote:

Passing value in IS_ERR_VALUE() is wrong, as they
 pass an 'int' into a function that takes an 'unsigned long'
 argument. This happens to work because the type is sign-extended
 on 64-bit architectures before it gets converted into an
 unsigned type.

 Passing an 'unsigned short' or 'unsigned int'
 argument into IS_ERR_VALUE() is guaranteed to be broken, as are
 8-bit integers and types that are wider than 'unsigned long'.

 Any user will get compilation warning for that do not pass an
 'unsigned long' argument.

 Commit '287980e49f; - This change is alreday fixes lots of other
 worst abusers.


Couple of generic comments:

- Your patch subject lines don't include the affected drivers/modules.
   As such, most of them will be ignored because maintainers won't realize
   that you are talking with them. Some may ask you to resubmit with proper
   subject lines.
   Commit 287980e49f is different; it addresses the problem in several
   drivers in a single commit.
- If you patch a single file, I think it would be better to adjust the
   description accordingly. In this patch, the offending variable type is
   u32. The patch description is therefore misleading; the code here simply does
   not work.
- When you resubmit a patch, you don't include a version, not a change log.
   This means additional work for maintainers, who have to figure out which
   patch to apply.

Specific comment:

The allocator in question returns -ENOMEM in an unsigned long. This is assigned
to u32. A proper fix would be to assign the return value to an unsigned
long and to use IS_ER_VALUE() to check if it reports an error, and to only
assign it to ucc_fast_rx_virtual_fifo_base_offset if there was no error.

Also, unless I am missing something - since ucc_fast_rx_virtual_fifo_base_offset
is defined as u32, it is somewhat unlikely that it is ever < 0.

Guenter


Signed-off-by: Arvind Yadav 
---
  drivers/soc/fsl/qe/ucc_fast.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..7cc783c 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -268,7 +268,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -281,7 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] Need proper type casting before assignment, Remove compilation Warning.

[Arvind Yadav]

-Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
assigned in ucc_fast_tx_virtual_fifo_base_offset and
ucc_fast_rx_virtual_fifo_base_offset. These variable are 'unsigned int'.
So before assginment need a proper type casting.

-Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'
into a function that takes an 'unsigned long' argument.This happens
to work because the type is sign-extended on 64-bit architectures
before it gets converted into an unsigned type.

-Passing an 'unsigned short' or 'unsigned int'argument into
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.

-Any user will get compilation warning for that do not pass an
unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..98eed25 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -267,8 +267,10 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
 
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   (unsigned int)qe_muram_alloc(uf_info->utfs,
+   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
+   if (IS_ERR_VALUE(
+   (unsigned long)uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -278,10 +280,11 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
 
/* Allocate memory for Rx Virtual Fifo */
uccf->ucc_fast_rx_virtual_fifo_base_offset =
-   qe_muram_alloc(uf_info->urfs +
+   (unsigned int)qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (IS_ERR_VALUE(
+   (unsigned long)uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] Removing lots of IS_ERR_VALUE abuses and compilation warning.

[Arvind Yadav]

Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'
into a function that takes an 'unsigned long' argument.This happens
to work because the type is sign-extended on 64-bit architectures
before it gets converted into an unsigned type.

Passing an 'unsigned short' or 'unsigned int'argument into
IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
unsigned long' argument.

Commit '287980e49f; - This change is alreday fixes lots of other
Worst abusers

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
+   if (ugeth->thread_dat_tx_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_thread_data_tx\n");
return -ENOMEM;
@@ -2568,7 +2568,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
qe_muram_alloc(ug_info->numQueuesTx *
   sizeof(struct ucc_geth_send_queue_qd),
   UCC_GETH_SEND_QUEUE_QUEUE_DESCRIPTOR_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->send_q_mem_reg_offset)) {
+   if (ugeth->send_q_mem_reg_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_send_q_mem_reg\n");
return -ENOMEM;
@@ -2609,7 +2609,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->scheduler_offset =
qe_muram_alloc(sizeof(struct ucc_geth_scheduler),
   UCC_GETH_SCHEDULER_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->scheduler_offset)) {
+   if (ugeth->scheduler_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_

[PATCH] Removing lots of IS_ERR_VALUE abuses and compilation warning.

[Arvind Yadav]

Passing value in IS_ERR_VALUE() is wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

Passing an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
'unsigned long' argument.

Commit '287980e49f; - This change is alreday fixes lots of other
worst abusers.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
+   if (ugeth->thread_dat_tx_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_thread_data_tx\n");
return -ENOMEM;
@@ -2568,7 +2568,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
qe_muram_alloc(ug_info->numQueuesTx *
   sizeof(struct ucc_geth_send_queue_qd),
   UCC_GETH_SEND_QUEUE_QUEUE_DESCRIPTOR_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->send_q_mem_reg_offset)) {
+   if (ugeth->send_q_mem_reg_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_send_q_mem_reg\n");
return -ENOMEM;
@@ -2609,7 +2609,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->scheduler_offset =
qe_muram_alloc(sizeof(struct ucc_geth_scheduler),
   UCC_GETH_SCHEDULER_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->scheduler_offset)) {
+   if (ugeth->scheduler_offset < 0) {
if (netif_msg_ifup(ugeth))
 

[PATCH] Remove lots of IS_ERR_VALUE abuses and compilation warning.

[Arvind Yadav]

Passing value in IS_ERR_VALUE() is wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

Passing an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
'unsigned long' argument.

Commit '287980e49f; - This change is alreday fixes lots of other
worst abusers.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..7cc783c 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -268,7 +268,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -281,7 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] Removing lots of IS_ERR_VALUE abuses and compilation warning.

[Arvind Yadav]

Passing value in IS_ERR_VALUE() is wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

Passing an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
'unsigned long' argument.

Commit '287980e49f; - This change is alreday fixes lots of other
worst abusers.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
+   if (ugeth->thread_dat_tx_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_thread_data_tx\n");
return -ENOMEM;
@@ -2568,7 +2568,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
qe_muram_alloc(ug_info->numQueuesTx *
   sizeof(struct ucc_geth_send_queue_qd),
   UCC_GETH_SEND_QUEUE_QUEUE_DESCRIPTOR_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->send_q_mem_reg_offset)) {
+   if (ugeth->send_q_mem_reg_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_send_q_mem_reg\n");
return -ENOMEM;
@@ -2609,7 +2609,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->scheduler_offset =
qe_muram_alloc(sizeof(struct ucc_geth_scheduler),
   UCC_GETH_SCHEDULER_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->scheduler_offset)) {
+   if (ugeth->scheduler_offset < 0) {
if (netif_msg_ifup(ugeth))
 

Re: [PATCH] Remove lots of IS_ERR_VALUE abuses.

[arvind Yadav]

As per your concern, I have change commit message. Submit other patch.

Thanks,
Arvind Yadav

On Thursday 07 July 2016 01:40 PM, Arnd Bergmann wrote:

On Thursday, July 7, 2016 12:47:43 AM CEST Arvind Yadav wrote:

 Most users of IS_ERR_VALUE() in the kernel are wrong, as they
 pass an 'int' into a function that takes an 'unsigned long'
 argument. This happens to work because the type is sign-extended
 on 64-bit architectures before it gets converted into an
 unsigned type.

 However, anything that passes an 'unsigned short' or 'unsigned int'
 argument into IS_ERR_VALUE() is guaranteed to be broken, as are
 8-bit integers and types that are wider than 'unsigned long'.

 Andrzej Hajda has already fixed a lot of the worst abusers that
 were causing actual bugs, but it would be nice to prevent any
 users that are not passing 'unsigned long' arguments.

 This patch changes all users of IS_ERR_VALUE() that I could find
 on 32-bit ARM randconfig builds and x86 allmodconfig. For the
 moment, this doesn't change the definition of IS_ERR_VALUE()
 because there are probably still architecture specific users
 elsewhere.

 Almost all the warnings I got are for files that are better off
 using 'if (err)' or 'if (err < 0)'.
 The only legitimate user I could find that we get a warning for
 is the (32-bit only) freescale gigabit ethernet driver.

 I was using this definition for testing:

  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
   unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

 which ends up making all 16-bit or wider types work correctly with
 the most plausible interpretation of what IS_ERR_VALUE() was supposed
 to return according to its users, but also causes a compile-time
     warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 

The above is my changelog text for commit 287980e49f, but it's not my patch.

Your changes look fine, but reusing my description there seems wrong.
Can you add a new description and just point to my commit instead?

Arnd


  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");

[PATCH] Remove lots of IS_ERR_VALUE abuses.

[Arvind Yadav]

Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale QE UCC Fast API.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
  unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
    warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..7cc783c 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -268,7 +268,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -281,7 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] Remove lots of IS_ERR_VALUE abuses.

[Arvind Yadav]

Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale gigabit ethernet driver.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
  unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
    warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
+   if (ugeth->thread_dat_tx_offset < 0) {
if (netif_msg_ifup(ugeth))