[PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)

[PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)

[Anton Vorontsov]

commit 1d2397d742b7a2b39b2f09dd9da3b9d1463f55e9 ("fsl_pq_mdio: Add
Suport for etsec2.0 devices") introduced the following warnings:

  CHECK   fsl_pq_mdio.c
fsl_pq_mdio.c:287:22: warning: incorrect type in initializer (different base types)
fsl_pq_mdio.c:287:22:    expected unknown type 11 const *__mptr
fsl_pq_mdio.c:287:22:    got unsigned long long [unsigned] [assigned] [usertype] addr
fsl_pq_mdio.c:287:19: warning: incorrect type in assignment (different base types)
fsl_pq_mdio.c:287:19:    expected unsigned long long [unsigned] [usertype] ioremap_miimcfg
fsl_pq_mdio.c:287:19:    got struct fsl_pq_mdio *<noident>
  CC      fsl_pq_mdio.o
fsl_pq_mdio.c: In function 'fsl_pq_mdio_probe':
fsl_pq_mdio.c:287: warning: initialization makes pointer from integer without a cast
fsl_pq_mdio.c:287: warning: assignment makes integer from pointer without a cast

These warnings are not easy to fix without ugly __force casts. So,
instead of introducing the casts, rework the code to substitute an
offset from an already mapped area. This makes the code a lot simpler
and less duplicated.

Plus, from now on we don't actually map reserved registers on
non-etsec2.0 devices, so we have more chances to catch programming
errors.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/fsl_pq_mdio.c |   31 ++++++++++++-------------------
 1 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/fsl_pq_mdio.c b/drivers/net/fsl_pq_mdio.c
index 4065b7c..fb8c8d9 100644
--- a/drivers/net/fsl_pq_mdio.c
+++ b/drivers/net/fsl_pq_mdio.c
@@ -262,10 +262,11 @@ static int fsl_pq_mdio_probe(struct of_device *ofdev,
 	struct device_node *np = ofdev->node;
 	struct device_node *tbi;
 	struct fsl_pq_mdio __iomem *regs = NULL;
+	void __iomem *map;
 	u32 __iomem *tbipa;
 	struct mii_bus *new_bus;
 	int tbiaddr = -1;
-	u64 addr = 0, size = 0, ioremap_miimcfg = 0;
+	u64 addr = 0, size = 0;
 	int err = 0;
 
 	new_bus = mdiobus_alloc();
@@ -279,28 +280,20 @@ static int fsl_pq_mdio_probe(struct of_device *ofdev,
 	fsl_pq_mdio_bus_name(new_bus->id, np);
 
 	/* Set the PHY base address */
-	if (of_device_is_compatible(np,"fsl,gianfar-mdio") ||
-		of_device_is_compatible(np, "fsl,gianfar-tbi") ||
-		of_device_is_compatible(np, "fsl,ucc-mdio") ||
-		of_device_is_compatible(np,"ucc_geth_phy" )) {
-		addr = of_translate_address(np, of_get_address(np, 0, &size, NULL));
-		ioremap_miimcfg =  container_of(addr, struct fsl_pq_mdio, miimcfg);
-		regs = ioremap(ioremap_miimcfg, size +
-				offsetof(struct fsl_pq_mdio, miimcfg));
-	} else if (of_device_is_compatible(np,"fsl,etsec2-mdio") ||
-			of_device_is_compatible(np, "fsl,etsec2-tbi")) {
-		addr = of_translate_address(np, of_get_address(np, 0, &size, NULL));
-		regs = ioremap(addr, size);
-	} else {
-		err = -EINVAL;
-		goto err_free_bus;
-	}
-
-	if (NULL == regs) {
+	addr = of_translate_address(np, of_get_address(np, 0, &size, NULL));
+	map = ioremap(addr, size);
+	if (!map) {
 		err = -ENOMEM;
 		goto err_free_bus;
 	}
 
+	if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
+			of_device_is_compatible(np, "fsl,gianfar-tbi") ||
+			of_device_is_compatible(np, "fsl,ucc-mdio") ||
+			of_device_is_compatible(np, "ucc_geth_phy"))
+		map -= offsetof(struct fsl_pq_mdio, miimcfg);
+	regs = map;
+
 	new_bus->priv = (void __force *)regs;
 
 	new_bus->irq = kcalloc(PHY_MAX_ADDR, sizeof(int), GFP_KERNEL);
-- 
1.6.3.3

RE: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)

[Kumar Gopalpet-B05799]

=20

>-----Original Message-----
>From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]=20
>Sent: Thursday, November 05, 2009 4:23 AM
>To: David Miller
>Cc: Fleming Andy-AFLEMING; Kumar Gopalpet-B05799;=20
>netdev@vger.kernel.org; linuxppc-dev@ozlabs.org
>Subject: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)
>
>commit 1d2397d742b7a2b39b2f09dd9da3b9d1463f55e9 ("fsl_pq_mdio:=20
>Add Suport for etsec2.0 devices") introduced the following warnings:
>
>  CHECK   fsl_pq_mdio.c
>fsl_pq_mdio.c:287:22: warning: incorrect type in initializer=20
>(different base types)
>fsl_pq_mdio.c:287:22:    expected unknown type 11 const *__mptr
>fsl_pq_mdio.c:287:22:    got unsigned long long [unsigned]=20
>[assigned] [usertype] addr
>fsl_pq_mdio.c:287:19: warning: incorrect type in assignment=20
>(different base types)
>fsl_pq_mdio.c:287:19:    expected unsigned long long=20
>[unsigned] [usertype] ioremap_miimcfg
>fsl_pq_mdio.c:287:19:    got struct fsl_pq_mdio *<noident>
>  CC      fsl_pq_mdio.o
>fsl_pq_mdio.c: In function 'fsl_pq_mdio_probe':
>fsl_pq_mdio.c:287: warning: initialization makes pointer from=20
>integer without a cast
>fsl_pq_mdio.c:287: warning: assignment makes integer from=20
>pointer without a cast
>
>These warnings are not easy to fix without ugly __force casts.=20
>So, instead of introducing the casts, rework the code to=20
>substitute an offset from an already mapped area. This makes=20
>the code a lot simpler and less duplicated.
>
>Plus, from now on we don't actually map reserved registers on=20
>non-etsec2.0 devices, so we have more chances to catch=20
>programming errors.
>
>Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>---
> drivers/net/fsl_pq_mdio.c |   31 ++++++++++++-------------------
> 1 files changed, 12 insertions(+), 19 deletions(-)
>
>diff --git a/drivers/net/fsl_pq_mdio.c=20
>b/drivers/net/fsl_pq_mdio.c index 4065b7c..fb8c8d9 100644
>--- a/drivers/net/fsl_pq_mdio.c
>+++ b/drivers/net/fsl_pq_mdio.c
>@@ -262,10 +262,11 @@ static int fsl_pq_mdio_probe(struct=20
>of_device *ofdev,
> 	struct device_node *np =3D ofdev->node;
> 	struct device_node *tbi;
> 	struct fsl_pq_mdio __iomem *regs =3D NULL;
>+	void __iomem *map;
> 	u32 __iomem *tbipa;
> 	struct mii_bus *new_bus;
> 	int tbiaddr =3D -1;
>-	u64 addr =3D 0, size =3D 0, ioremap_miimcfg =3D 0;
>+	u64 addr =3D 0, size =3D 0;
> 	int err =3D 0;
>=20
> 	new_bus =3D mdiobus_alloc();
>@@ -279,28 +280,20 @@ static int fsl_pq_mdio_probe(struct=20
>of_device *ofdev,
> 	fsl_pq_mdio_bus_name(new_bus->id, np);
>=20
> 	/* Set the PHY base address */
>-	if (of_device_is_compatible(np,"fsl,gianfar-mdio") ||
>-		of_device_is_compatible(np, "fsl,gianfar-tbi") ||
>-		of_device_is_compatible(np, "fsl,ucc-mdio") ||
>-		of_device_is_compatible(np,"ucc_geth_phy" )) {
>-		addr =3D of_translate_address(np,=20
>of_get_address(np, 0, &size, NULL));
>-		ioremap_miimcfg =3D  container_of(addr, struct=20
>fsl_pq_mdio, miimcfg);
>-		regs =3D ioremap(ioremap_miimcfg, size +
>-				offsetof(struct fsl_pq_mdio, miimcfg));
>-	} else if (of_device_is_compatible(np,"fsl,etsec2-mdio") ||
>-			of_device_is_compatible(np, "fsl,etsec2-tbi")) {
>-		addr =3D of_translate_address(np,=20
>of_get_address(np, 0, &size, NULL));
>-		regs =3D ioremap(addr, size);
>-	} else {
>-		err =3D -EINVAL;
>-		goto err_free_bus;
>-	}
>-
>-	if (NULL =3D=3D regs) {
>+	addr =3D of_translate_address(np, of_get_address(np, 0,=20
>&size, NULL));
>+	map =3D ioremap(addr, size);
>+	if (!map) {
> 		err =3D -ENOMEM;
> 		goto err_free_bus;
> 	}
>=20
>+	if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
>+			of_device_is_compatible(np,=20
>"fsl,gianfar-tbi") ||
>+			of_device_is_compatible(np, "fsl,ucc-mdio") ||
>+			of_device_is_compatible(np, "ucc_geth_phy"))
>+		map -=3D offsetof(struct fsl_pq_mdio, miimcfg);
>+	regs =3D map;
>+
> 	new_bus->priv =3D (void __force *)regs;
>=20
> 	new_bus->irq =3D kcalloc(PHY_MAX_ADDR, sizeof(int), GFP_KERNEL);
>--

HI Anton, thanks for the changes. I have only one concern, has this code
been tried for ucc_geth ? I remember I had some issues with getting the
ucc_geth mdio also working. I will take in these changes and try at my
end for ucc_geth.


-Thanks
Sandeep=20

RE: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)

[Kumar Gopalpet-B05799]

=20

>-----Original Message-----
>From: Kumar Gopalpet-B05799=20
>Sent: Thursday, November 05, 2009 10:31 AM
>To: 'Anton Vorontsov'
>Cc: Fleming Andy-AFLEMING; netdev@vger.kernel.org;=20
>linuxppc-dev@ozlabs.org; David Miller
>Subject: RE: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse=20
>warnings (part 1)
>
>=20
>
>>-----Original Message-----
>>From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]
>>Sent: Thursday, November 05, 2009 4:23 AM
>>To: David Miller
>>Cc: Fleming Andy-AFLEMING; Kumar Gopalpet-B05799;=20
>>netdev@vger.kernel.org; linuxppc-dev@ozlabs.org
>>Subject: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse=20
>warnings (part 1)
>>
>>commit 1d2397d742b7a2b39b2f09dd9da3b9d1463f55e9 ("fsl_pq_mdio:=20
>>Add Suport for etsec2.0 devices") introduced the following warnings:
>>
>>  CHECK   fsl_pq_mdio.c
>>fsl_pq_mdio.c:287:22: warning: incorrect type in initializer=20
>(different=20
>>base types)
>>fsl_pq_mdio.c:287:22:    expected unknown type 11 const *__mptr
>>fsl_pq_mdio.c:287:22:    got unsigned long long [unsigned]=20
>>[assigned] [usertype] addr
>>fsl_pq_mdio.c:287:19: warning: incorrect type in assignment=20
>(different=20
>>base types)
>>fsl_pq_mdio.c:287:19:    expected unsigned long long=20
>>[unsigned] [usertype] ioremap_miimcfg
>>fsl_pq_mdio.c:287:19:    got struct fsl_pq_mdio *<noident>
>>  CC      fsl_pq_mdio.o
>>fsl_pq_mdio.c: In function 'fsl_pq_mdio_probe':
>>fsl_pq_mdio.c:287: warning: initialization makes pointer from integer=20
>>without a cast
>>fsl_pq_mdio.c:287: warning: assignment makes integer from pointer=20
>>without a cast
>>
>>These warnings are not easy to fix without ugly __force casts.=20
>>So, instead of introducing the casts, rework the code to=20
>substitute an=20
>>offset from an already mapped area. This makes the code a lot simpler=20
>>and less duplicated.
>>
>>Plus, from now on we don't actually map reserved registers on=20
>>non-etsec2.0 devices, so we have more chances to catch programming=20
>>errors.
>>
>>Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>>---
>> drivers/net/fsl_pq_mdio.c |   31 ++++++++++++-------------------
>> 1 files changed, 12 insertions(+), 19 deletions(-)
>>
>>diff --git a/drivers/net/fsl_pq_mdio.c b/drivers/net/fsl_pq_mdio.c=20
>>index 4065b7c..fb8c8d9 100644
>>--- a/drivers/net/fsl_pq_mdio.c
>>+++ b/drivers/net/fsl_pq_mdio.c
>>@@ -262,10 +262,11 @@ static int fsl_pq_mdio_probe(struct of_device=20
>>*ofdev,
>> 	struct device_node *np =3D ofdev->node;
>> 	struct device_node *tbi;
>> 	struct fsl_pq_mdio __iomem *regs =3D NULL;
>>+	void __iomem *map;
>> 	u32 __iomem *tbipa;
>> 	struct mii_bus *new_bus;
>> 	int tbiaddr =3D -1;
>>-	u64 addr =3D 0, size =3D 0, ioremap_miimcfg =3D 0;
>>+	u64 addr =3D 0, size =3D 0;
>> 	int err =3D 0;
>>=20
>> 	new_bus =3D mdiobus_alloc();
>>@@ -279,28 +280,20 @@ static int fsl_pq_mdio_probe(struct of_device=20
>>*ofdev,
>> 	fsl_pq_mdio_bus_name(new_bus->id, np);
>>=20
>> 	/* Set the PHY base address */
>>-	if (of_device_is_compatible(np,"fsl,gianfar-mdio") ||
>>-		of_device_is_compatible(np, "fsl,gianfar-tbi") ||
>>-		of_device_is_compatible(np, "fsl,ucc-mdio") ||
>>-		of_device_is_compatible(np,"ucc_geth_phy" )) {
>>-		addr =3D of_translate_address(np,=20
>>of_get_address(np, 0, &size, NULL));
>>-		ioremap_miimcfg =3D  container_of(addr, struct=20
>>fsl_pq_mdio, miimcfg);
>>-		regs =3D ioremap(ioremap_miimcfg, size +
>>-				offsetof(struct fsl_pq_mdio, miimcfg));
>>-	} else if (of_device_is_compatible(np,"fsl,etsec2-mdio") ||
>>-			of_device_is_compatible(np, "fsl,etsec2-tbi")) {
>>-		addr =3D of_translate_address(np,=20
>>of_get_address(np, 0, &size, NULL));
>>-		regs =3D ioremap(addr, size);
>>-	} else {
>>-		err =3D -EINVAL;
>>-		goto err_free_bus;
>>-	}
>>-
>>-	if (NULL =3D=3D regs) {
>>+	addr =3D of_translate_address(np, of_get_address(np, 0,
>>&size, NULL));
>>+	map =3D ioremap(addr, size);
>>+	if (!map) {
>> 		err =3D -ENOMEM;
>> 		goto err_free_bus;
>> 	}
>>=20
>>+	if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
>>+			of_device_is_compatible(np,
>>"fsl,gianfar-tbi") ||
>>+			of_device_is_compatible(np, "fsl,ucc-mdio") ||
>>+			of_device_is_compatible(np, "ucc_geth_phy"))
>>+		map -=3D offsetof(struct fsl_pq_mdio, miimcfg);
>>+	regs =3D map;
>>+
>> 	new_bus->priv =3D (void __force *)regs;
>>=20
>> 	new_bus->irq =3D kcalloc(PHY_MAX_ADDR, sizeof(int), GFP_KERNEL);
>>--
>
>HI Anton, thanks for the changes. I have only one concern, has=20
>this code been tried for ucc_geth ? I remember I had some=20
>issues with getting the ucc_geth mdio also working. I will=20
>take in these changes and try at my end for ucc_geth.

Sorry, if my earlier statement was confusing. What I meant was with
similar changes as Anton's changes
I had some issues with ucc_geth mdio ( may be Anton's changes don't have
that issue).


--

Thanks
Sandeep

Re: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)

[Anton Vorontsov]

On Thu, Nov 05, 2009 at 11:11:56AM +0530, Kumar Gopalpet-B05799 wrote:
[...]
> >HI Anton, thanks for the changes. I have only one concern, has 
> >this code been tried for ucc_geth ? I remember I had some 
> >issues with getting the ucc_geth mdio also working. I will 
> >take in these changes and try at my end for ucc_geth.
> 
> Sorry, if my earlier statement was confusing. What I meant was with
> similar changes as Anton's changes
> I had some issues with ucc_geth mdio ( may be Anton's changes don't have
> that issue).

Nope, I see no issues on MPC8360E-MDS using ucc_geth and fsl_pq_mdio
drivers.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)

[David Miller]

From: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Thu, 5 Nov 2009 01:52:56 +0300

> commit 1d2397d742b7a2b39b2f09dd9da3b9d1463f55e9 ("fsl_pq_mdio: Add
> Suport for etsec2.0 devices") introduced the following warnings:
 ...
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Applied.