diff mbox

[08/11] net/fsl: use of_property_read_bool

Message ID 1470387411-15879-9-git-send-email-Julia.Lawall@lip6.fr
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Julia Lawall Aug. 5, 2016, 8:56 a.m. UTC
Use of_property_read_bool to check for the existence of a property.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e1,e2;
statement S2,S1;
@@
-       if (of_get_property(e1,e2,NULL))
+       if (of_property_read_bool(e1,e2))
        S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/net/ethernet/freescale/xgmac_mdio.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Johannes Berg Aug. 5, 2016, 9:30 a.m. UTC | #1
> -	if (of_get_property(pdev->dev.of_node,
> -			    "little-endian", NULL))
> +	if (of_property_read_bool(pdev->dev.of_node, "little-
> endian"))
>  		priv->is_little_endian = true;
>  	else
>  		priv->is_little_endian = false;
> 

Perhaps, while changing this, that'd be better as

	priv->is_little_endian = of_property_read_bool(...);

Obviously that would've worked before, but now it'd be even easier to
understand, it seems.

johannes
Julia Lawall Aug. 5, 2016, 9:47 a.m. UTC | #2
On Fri, 5 Aug 2016, Johannes Berg wrote:

>
> > -	if (of_get_property(pdev->dev.of_node,
> > -			    "little-endian", NULL))
> > +	if (of_property_read_bool(pdev->dev.of_node, "little-
> > endian"))
> >  		priv->is_little_endian = true;
> >  	else
> >  		priv->is_little_endian = false;
> >
>
> Perhaps, while changing this, that'd be better as
>
> 	priv->is_little_endian = of_property_read_bool(...);
>
> Obviously that would've worked before, but now it'd be even easier to
> understand, it seems.

Thanks for the suggestion.  Will send shortly.

julia
Julia Lawall Aug. 5, 2016, 10:08 a.m. UTC | #3
On Fri, 5 Aug 2016, Johannes Berg wrote:

>
> > -	if (of_get_property(pdev->dev.of_node,
> > -			    "little-endian", NULL))
> > +	if (of_property_read_bool(pdev->dev.of_node, "little-
> > endian"))
> >  		priv->is_little_endian = true;
> >  	else
> >  		priv->is_little_endian = false;
> >
>
> Perhaps, while changing this, that'd be better as
>
> 	priv->is_little_endian = of_property_read_bool(...);
>
> Obviously that would've worked before, but now it'd be even easier to
> understand, it seems.

Can I do the same for:

	if (of_property_read_bool(np, "phy-clk-valid"))
                pdata->check_phy_clk_valid = 1;
	else
                pdata->check_phy_clk_valid = 0;

The type is not bool, but:

include/linux/fsl_devices.h:    unsigned        check_phy_clk_valid:1;

thanks,
julia
Johannes Berg Aug. 5, 2016, 10:12 a.m. UTC | #4
On Fri, 2016-08-05 at 12:08 +0200, Julia Lawall wrote:

> Can I do the same for:
> 
>         if (of_property_read_bool(np, "phy-clk-valid"))
>                 pdata->check_phy_clk_valid = 1;
>         else
>                 pdata->check_phy_clk_valid = 0;
> 
> The type is not bool, but:
> 
> include/linux/fsl_devices.h:    unsigned        check_phy_clk_valid:1;
> 
The type doesn't even matter, does it? Any bool->int conversion should
result in 0/1.

johannes
Julia Lawall Aug. 5, 2016, 10:14 a.m. UTC | #5
On Fri, 5 Aug 2016, Johannes Berg wrote:

> On Fri, 2016-08-05 at 12:08 +0200, Julia Lawall wrote:
> > 
> > Can I do the same for:
> >
> >         if (of_property_read_bool(np, "phy-clk-valid"))
> >                 pdata->check_phy_clk_valid = 1;
> >         else
> >                 pdata->check_phy_clk_valid = 0;
> >
> > The type is not bool, but:
> >
> > include/linux/fsl_devices.h:    unsigned        check_phy_clk_valid:1;
> >
> The type doesn't even matter, does it? Any bool->int conversion should
> result in 0/1.

Just paranoid.  Thanks.

julia
Sergei Shtylyov Aug. 5, 2016, 11:35 a.m. UTC | #6
Hello.

On 8/5/2016 11:56 AM, Julia Lawall wrote:

> Use of_property_read_bool to check for the existence of a property.
>
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression e1,e2;
> statement S2,S1;
> @@
> -       if (of_get_property(e1,e2,NULL))
> +       if (of_property_read_bool(e1,e2))
>         S1 else S2
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
>  drivers/net/ethernet/freescale/xgmac_mdio.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> index 7b8fe86..a77ba98 100644
> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> @@ -271,8 +271,7 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
>  		goto err_ioremap;
>  	}
>
> -	if (of_get_property(pdev->dev.of_node,
> -			    "little-endian", NULL))
> +	if (of_property_read_bool(pdev->dev.of_node, "little-endian"))
>  		priv->is_little_endian = true;
>  	else
>  		priv->is_little_endian = false;

    priv->is_little_endian = of_property_read_bool(pdev->dev.of_node,
						  "little-endian"));

MBR, Sergei
Julia Lawall Aug. 5, 2016, 11:38 a.m. UTC | #7
On Fri, 5 Aug 2016, Sergei Shtylyov wrote:

> Hello.
>
> On 8/5/2016 11:56 AM, Julia Lawall wrote:
>
> > Use of_property_read_bool to check for the existence of a property.
> >
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression e1,e2;
> > statement S2,S1;
> > @@
> > -       if (of_get_property(e1,e2,NULL))
> > +       if (of_property_read_bool(e1,e2))
> >         S1 else S2
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> >  drivers/net/ethernet/freescale/xgmac_mdio.c |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c
> > b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > index 7b8fe86..a77ba98 100644
> > --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> > +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > @@ -271,8 +271,7 @@ static int xgmac_mdio_probe(struct platform_device
> > *pdev)
> >  		goto err_ioremap;
> >  	}
> >
> > -	if (of_get_property(pdev->dev.of_node,
> > -			    "little-endian", NULL))
> > +	if (of_property_read_bool(pdev->dev.of_node, "little-endian"))
> >  		priv->is_little_endian = true;
> >  	else
> >  		priv->is_little_endian = false;
>
>    priv->is_little_endian = of_property_read_bool(pdev->dev.of_node,
> 						  "little-endian"));

Thanks,

I just sent a v2 with this change.

julia
Sergei Shtylyov Aug. 5, 2016, 11:40 a.m. UTC | #8
On 8/5/2016 2:38 PM, Julia Lawall wrote:

>>> Use of_property_read_bool to check for the existence of a property.
>>>
>>> The semantic patch that makes this change is as follows:
>>> (http://coccinelle.lip6.fr/)
>>>
>>> // <smpl>
>>> @@
>>> expression e1,e2;
>>> statement S2,S1;
>>> @@
>>> -       if (of_get_property(e1,e2,NULL))
>>> +       if (of_property_read_bool(e1,e2))
>>>         S1 else S2
>>> // </smpl>
>>>
>>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>>
>>> ---
>>>  drivers/net/ethernet/freescale/xgmac_mdio.c |    3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> b/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> index 7b8fe86..a77ba98 100644
>>> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> @@ -271,8 +271,7 @@ static int xgmac_mdio_probe(struct platform_device
>>> *pdev)
>>>  		goto err_ioremap;
>>>  	}
>>>
>>> -	if (of_get_property(pdev->dev.of_node,
>>> -			    "little-endian", NULL))
>>> +	if (of_property_read_bool(pdev->dev.of_node, "little-endian"))
>>>  		priv->is_little_endian = true;
>>>  	else
>>>  		priv->is_little_endian = false;
>>
>>    priv->is_little_endian = of_property_read_bool(pdev->dev.of_node,
>> 						  "little-endian"));
>
> Thanks,
>
> I just sent a v2 with this change.

    I've seen. Sorry, forgot to look at the followups before commenting...

> julia

MBR, Sergei
diff mbox

Patch

diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 7b8fe86..a77ba98 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -271,8 +271,7 @@  static int xgmac_mdio_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
-	if (of_get_property(pdev->dev.of_node,
-			    "little-endian", NULL))
+	if (of_property_read_bool(pdev->dev.of_node, "little-endian"))
 		priv->is_little_endian = true;
 	else
 		priv->is_little_endian = false;