| Message ID | 20180828015252.28511-21-robh@kernel.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | i2c: Convert to using %pOFn instead of device_node.name | expand |
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; > } > >
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
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
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
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; }
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(-)