i2c: Convert to using %pOFn instead of device_node.name
diff mbox series

Message ID 20180828015252.28511-21-robh@kernel.org
State Changes Requested
Headers show
Series
  • i2c: Convert to using %pOFn instead of device_node.name
Related show

Commit Message

Rob Herring Aug. 28, 2018, 1:52 a.m. UTC
In preparation to remove the node name pointer from struct device_node,
convert printf users to use the %pOFn format specifier.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Peter Rosin <peda@axentia.se>
Cc: linux-i2c@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/i2c/busses/i2c-powermac.c | 15 ++++++++-------
 drivers/i2c/muxes/i2c-mux-gpmux.c |  4 ++--
 2 files changed, 10 insertions(+), 9 deletions(-)

Comments

Peter Rosin Aug. 28, 2018, 6:33 a.m. UTC | #1
On 2018-08-28 03:52, Rob Herring wrote:
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
> 
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Peter Rosin <peda@axentia.se>
> Cc: linux-i2c@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/i2c/busses/i2c-powermac.c | 15 ++++++++-------
>  drivers/i2c/muxes/i2c-mux-gpmux.c |  4 ++--
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
> index f2a2067525ef..b706fd136ca5 100644
> --- a/drivers/i2c/busses/i2c-powermac.c
> +++ b/drivers/i2c/busses/i2c-powermac.c
> @@ -390,7 +390,6 @@ static int i2c_powermac_probe(struct platform_device *dev)
>  	struct pmac_i2c_bus *bus = dev_get_platdata(&dev->dev);
>  	struct device_node *parent = NULL;

Lose the initializer...

>  	struct i2c_adapter *adapter;
> -	const char *basename;
>  	int rc;
>  
>  	if (bus == NULL)
> @@ -407,23 +406,25 @@ static int i2c_powermac_probe(struct platform_device *dev)
>  		parent = of_get_parent(pmac_i2c_get_controller(bus));
>  		if (parent == NULL)
>  			return -EINVAL;
> -		basename = parent->name;
> +		snprintf(adapter->name, sizeof(adapter->name), "%pOFn %d", 
> +			 parent,
> +			 pmac_i2c_get_channel(bus));

...and I would have written "parent, pmac_i2c_get_channel(bus));" on one line,
but maybe that's just me.

Anyway, with the initializer fix,

Reviewed-by: Peter Rosin <peda@axentia.se>

(Wolfram, no need to split this, just take it in full.)

Cheers,
Peter

> +		of_node_put(parent);
>  		break;
>  	case pmac_i2c_bus_pmu:
> -		basename = "pmu";
> +		snprintf(adapter->name, sizeof(adapter->name), "pmu %d",
> +			 pmac_i2c_get_channel(bus));
>  		break;
>  	case pmac_i2c_bus_smu:
>  		/* This is not what we used to do but I'm fixing drivers at
>  		 * the same time as this change
>  		 */
> -		basename = "smu";
> +		snprintf(adapter->name, sizeof(adapter->name), "smu %d",
> +			 pmac_i2c_get_channel(bus));
>  		break;
>  	default:
>  		return -EINVAL;
>  	}
> -	snprintf(adapter->name, sizeof(adapter->name), "%s %d", basename,
> -		 pmac_i2c_get_channel(bus));
> -	of_node_put(parent);
>  
>  	platform_set_drvdata(dev, adapter);
>  	adapter->algo = &i2c_powermac_algorithm;
> diff --git a/drivers/i2c/muxes/i2c-mux-gpmux.c b/drivers/i2c/muxes/i2c-mux-gpmux.c
> index 92cf5f48afe6..f60b670deff7 100644
> --- a/drivers/i2c/muxes/i2c-mux-gpmux.c
> +++ b/drivers/i2c/muxes/i2c-mux-gpmux.c
> @@ -120,8 +120,8 @@ static int i2c_mux_probe(struct platform_device *pdev)
>  
>  		ret = of_property_read_u32(child, "reg", &chan);
>  		if (ret < 0) {
> -			dev_err(dev, "no reg property for node '%s'\n",
> -				child->name);
> +			dev_err(dev, "no reg property for node '%pOFn'\n",
> +				child);
>  			goto err_children;
>  		}
>  
>
Rob Herring Aug. 29, 2018, 6:42 p.m. UTC | #2
On Wed, Aug 29, 2018 at 1:03 PM Peter Rosin <peda@axentia.se> wrote:
>
> On 2018-08-28 03:52, Rob Herring wrote:
> > In preparation to remove the node name pointer from struct device_node,
> > convert printf users to use the %pOFn format specifier.
> >
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Peter Rosin <peda@axentia.se>
> > Cc: linux-i2c@vger.kernel.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/i2c/busses/i2c-powermac.c | 15 ++++++++-------
> >  drivers/i2c/muxes/i2c-mux-gpmux.c |  4 ++--
> >  2 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
> > index f2a2067525ef..b706fd136ca5 100644
> > --- a/drivers/i2c/busses/i2c-powermac.c
> > +++ b/drivers/i2c/busses/i2c-powermac.c
> > @@ -390,7 +390,6 @@ static int i2c_powermac_probe(struct platform_device *dev)
> >       struct pmac_i2c_bus *bus = dev_get_platdata(&dev->dev);
> >       struct device_node *parent = NULL;
>
> Lose the initializer...

That's pretty much unrelated though. I'd have to write "Also, remove
the unnecessary parent pointer init" in the commit message and we all
know "Also" is a clue for belongs in a separate patch.

Rob
Peter Rosin Aug. 29, 2018, 7:52 p.m. UTC | #3
On 2018-08-29 20:42, Rob Herring wrote:
> On Wed, Aug 29, 2018 at 1:03 PM Peter Rosin <peda@axentia.se> wrote:
>>
>> On 2018-08-28 03:52, Rob Herring wrote:
>>> In preparation to remove the node name pointer from struct device_node,
>>> convert printf users to use the %pOFn format specifier.
>>>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Peter Rosin <peda@axentia.se>
>>> Cc: linux-i2c@vger.kernel.org
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> ---
>>>  drivers/i2c/busses/i2c-powermac.c | 15 ++++++++-------
>>>  drivers/i2c/muxes/i2c-mux-gpmux.c |  4 ++--
>>>  2 files changed, 10 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
>>> index f2a2067525ef..b706fd136ca5 100644
>>> --- a/drivers/i2c/busses/i2c-powermac.c
>>> +++ b/drivers/i2c/busses/i2c-powermac.c
>>> @@ -390,7 +390,6 @@ static int i2c_powermac_probe(struct platform_device *dev)
>>>       struct pmac_i2c_bus *bus = dev_get_platdata(&dev->dev);
>>>       struct device_node *parent = NULL;
>>
>> Lose the initializer...
> 
> That's pretty much unrelated though.

I disagree. If you remove the need for the initializer, it's very much
related to also remove the initializer.

> I'd have to write "Also, remove
> the unnecessary parent pointer init" in the commit message and we all
> know "Also" is a clue for belongs in a separate patch.

How about: "This makes the parent pointer initializer redundant, lose it."

See, no "Also" in there, and no separate patch needed. Or don't mention it
at all.

Cheers,
Peter
Rob Herring Aug. 30, 2018, 12:49 a.m. UTC | #4
On Wed, Aug 29, 2018 at 7:25 PM Peter Rosin <peda@axentia.se> wrote:
>
> On 2018-08-29 20:42, Rob Herring wrote:
> > On Wed, Aug 29, 2018 at 1:03 PM Peter Rosin <peda@axentia.se> wrote:
> >>
> >> On 2018-08-28 03:52, Rob Herring wrote:
> >>> In preparation to remove the node name pointer from struct device_node,
> >>> convert printf users to use the %pOFn format specifier.
> >>>
> >>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >>> Cc: Paul Mackerras <paulus@samba.org>
> >>> Cc: Michael Ellerman <mpe@ellerman.id.au>
> >>> Cc: Peter Rosin <peda@axentia.se>
> >>> Cc: linux-i2c@vger.kernel.org
> >>> Cc: linuxppc-dev@lists.ozlabs.org
> >>> Signed-off-by: Rob Herring <robh@kernel.org>
> >>> ---
> >>>  drivers/i2c/busses/i2c-powermac.c | 15 ++++++++-------
> >>>  drivers/i2c/muxes/i2c-mux-gpmux.c |  4 ++--
> >>>  2 files changed, 10 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
> >>> index f2a2067525ef..b706fd136ca5 100644
> >>> --- a/drivers/i2c/busses/i2c-powermac.c
> >>> +++ b/drivers/i2c/busses/i2c-powermac.c
> >>> @@ -390,7 +390,6 @@ static int i2c_powermac_probe(struct platform_device *dev)
> >>>       struct pmac_i2c_bus *bus = dev_get_platdata(&dev->dev);
> >>>       struct device_node *parent = NULL;
> >>
> >> Lose the initializer...
> >
> > That's pretty much unrelated though.
>
> I disagree. If you remove the need for the initializer, it's very much
> related to also remove the initializer.

You're right. I'd stared at it a bit and missed that in fact parent is
only accessed within the case statement.

Rob

Patch
diff mbox series

diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
index f2a2067525ef..b706fd136ca5 100644
--- a/drivers/i2c/busses/i2c-powermac.c
+++ b/drivers/i2c/busses/i2c-powermac.c
@@ -390,7 +390,6 @@  static int i2c_powermac_probe(struct platform_device *dev)
 	struct pmac_i2c_bus *bus = dev_get_platdata(&dev->dev);
 	struct device_node *parent = NULL;
 	struct i2c_adapter *adapter;
-	const char *basename;
 	int rc;
 
 	if (bus == NULL)
@@ -407,23 +406,25 @@  static int i2c_powermac_probe(struct platform_device *dev)
 		parent = of_get_parent(pmac_i2c_get_controller(bus));
 		if (parent == NULL)
 			return -EINVAL;
-		basename = parent->name;
+		snprintf(adapter->name, sizeof(adapter->name), "%pOFn %d", 
+			 parent,
+			 pmac_i2c_get_channel(bus));
+		of_node_put(parent);
 		break;
 	case pmac_i2c_bus_pmu:
-		basename = "pmu";
+		snprintf(adapter->name, sizeof(adapter->name), "pmu %d",
+			 pmac_i2c_get_channel(bus));
 		break;
 	case pmac_i2c_bus_smu:
 		/* This is not what we used to do but I'm fixing drivers at
 		 * the same time as this change
 		 */
-		basename = "smu";
+		snprintf(adapter->name, sizeof(adapter->name), "smu %d",
+			 pmac_i2c_get_channel(bus));
 		break;
 	default:
 		return -EINVAL;
 	}
-	snprintf(adapter->name, sizeof(adapter->name), "%s %d", basename,
-		 pmac_i2c_get_channel(bus));
-	of_node_put(parent);
 
 	platform_set_drvdata(dev, adapter);
 	adapter->algo = &i2c_powermac_algorithm;
diff --git a/drivers/i2c/muxes/i2c-mux-gpmux.c b/drivers/i2c/muxes/i2c-mux-gpmux.c
index 92cf5f48afe6..f60b670deff7 100644
--- a/drivers/i2c/muxes/i2c-mux-gpmux.c
+++ b/drivers/i2c/muxes/i2c-mux-gpmux.c
@@ -120,8 +120,8 @@  static int i2c_mux_probe(struct platform_device *pdev)
 
 		ret = of_property_read_u32(child, "reg", &chan);
 		if (ret < 0) {
-			dev_err(dev, "no reg property for node '%s'\n",
-				child->name);
+			dev_err(dev, "no reg property for node '%pOFn'\n",
+				child);
 			goto err_children;
 		}