| Message ID | 20240415161220.8347-1-jain.abhinav177@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: mpc: Removal of of_node_put with __free for auto cleanup | expand |
On 16/04/24 04:12, Abhinav Jain wrote: > Remove of_node_put from node_ctrl and node struct device_nodes. > Move the declaration to initialization for ensuring scope sanity. > > Suggested-by: Julia Lawall <julia.lawall@inria.fr> > Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > drivers/i2c/busses/i2c-mpc.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c > index 8d73c0f405ed..c4223556b3b8 100644 > --- a/drivers/i2c/busses/i2c-mpc.c > +++ b/drivers/i2c/busses/i2c-mpc.c > @@ -304,13 +304,12 @@ static void mpc_i2c_setup_512x(struct device_node *node, > struct mpc_i2c *i2c, > u32 clock) > { > - struct device_node *node_ctrl; > void __iomem *ctrl; > u32 idx; > > /* Enable I2C interrupts for mpc5121 */ > - node_ctrl = of_find_compatible_node(NULL, NULL, > - "fsl,mpc5121-i2c-ctrl"); > + struct device_node *node_ctrl __free(device_node) = > + of_find_compatible_node(NULL, NULL, "fsl,mpc5121-i2c-ctrl"); > if (node_ctrl) { > ctrl = of_iomap(node_ctrl, 0); > if (ctrl) { > @@ -321,7 +320,6 @@ static void mpc_i2c_setup_512x(struct device_node *node, > setbits32(ctrl, 1 << (24 + idx * 2)); > iounmap(ctrl); > } > - of_node_put(node_ctrl); > } > > /* The clock setup for the 52xx works also fine for the 512x */ > @@ -358,11 +356,11 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = { > > static u32 mpc_i2c_get_sec_cfg_8xxx(void) > { > - struct device_node *node; > u32 __iomem *reg; > u32 val = 0; > > - node = of_find_node_by_name(NULL, "global-utilities"); > + struct device_node *node __free(device_node) = > + of_find_node_by_name(NULL, "global-utilities"); > if (node) { > const u32 *prop = of_get_property(node, "reg", NULL); > if (prop) { > @@ -383,7 +381,6 @@ static u32 mpc_i2c_get_sec_cfg_8xxx(void) > iounmap(reg); > } > } > - of_node_put(node); > > return val; > }
Hi Chris, On Tue, Apr 16, 2024 at 03:59:13AM +0000, Chris Packham wrote: > On 16/04/24 08:54, Andi Shyti wrote: > >> /* Enable I2C interrupts for mpc5121 */ > >> - node_ctrl = of_find_compatible_node(NULL, NULL, > >> - "fsl,mpc5121-i2c-ctrl"); > >> + struct device_node *node_ctrl __free(device_node) = > > How have you tested this? > > I'm not sure I know anyone that still has a mpc5121. Maybe someone on > linuxppc-dev? that's why I was asking, this would be the first driver in i2c using Rob's __free(device_node). > I did try to take the patch for a spin on my T2081RDB but I'm having > some userland issues on it for some reason (unrelated to this change). > The kernel boot does discover a few peripherals hanging of the I2C > interface but I'm not in a position to offer up a Tested-by and I've run > out of time to debug why my board is unhappy. Thanks for giving it a try. Andi
Hi On Mon, 15 Apr 2024 16:12:20 +0000, Abhinav Jain wrote: > Remove of_node_put from node_ctrl and node struct device_nodes. > Move the declaration to initialization for ensuring scope sanity. > > Applied to i2c/i2c-host on git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git Thank you, Andi Patches applied =============== [1/1] i2c: mpc: Removal of of_node_put with __free for auto cleanup commit: 24e4a0d7c6b7a6491cc44781f3900129f56f447f
Chris Packham <Chris.Packham@alliedtelesis.co.nz> writes: > On 16/04/24 08:54, Andi Shyti wrote: >> Hi Abhinav, >> >>> /* Enable I2C interrupts for mpc5121 */ >>> - node_ctrl = of_find_compatible_node(NULL, NULL, >>> - "fsl,mpc5121-i2c-ctrl"); >>> + struct device_node *node_ctrl __free(device_node) = >> How have you tested this? > > I'm not sure I know anyone that still has a mpc5121. Maybe someone on > linuxppc-dev? Not that I know of. The driver loads on my T4240, but doesn't hit the right paths to test this patch. No objection to it being merged though. cheers
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 8d73c0f405ed..c4223556b3b8 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -304,13 +304,12 @@ static void mpc_i2c_setup_512x(struct device_node *node, struct mpc_i2c *i2c, u32 clock) { - struct device_node *node_ctrl; void __iomem *ctrl; u32 idx; /* Enable I2C interrupts for mpc5121 */ - node_ctrl = of_find_compatible_node(NULL, NULL, - "fsl,mpc5121-i2c-ctrl"); + struct device_node *node_ctrl __free(device_node) = + of_find_compatible_node(NULL, NULL, "fsl,mpc5121-i2c-ctrl"); if (node_ctrl) { ctrl = of_iomap(node_ctrl, 0); if (ctrl) { @@ -321,7 +320,6 @@ static void mpc_i2c_setup_512x(struct device_node *node, setbits32(ctrl, 1 << (24 + idx * 2)); iounmap(ctrl); } - of_node_put(node_ctrl); } /* The clock setup for the 52xx works also fine for the 512x */ @@ -358,11 +356,11 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = { static u32 mpc_i2c_get_sec_cfg_8xxx(void) { - struct device_node *node; u32 __iomem *reg; u32 val = 0; - node = of_find_node_by_name(NULL, "global-utilities"); + struct device_node *node __free(device_node) = + of_find_node_by_name(NULL, "global-utilities"); if (node) { const u32 *prop = of_get_property(node, "reg", NULL); if (prop) { @@ -383,7 +381,6 @@ static u32 mpc_i2c_get_sec_cfg_8xxx(void) iounmap(reg); } } - of_node_put(node); return val; }
Remove of_node_put from node_ctrl and node struct device_nodes. Move the declaration to initialization for ensuring scope sanity. Suggested-by: Julia Lawall <julia.lawall@inria.fr> Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com> --- drivers/i2c/busses/i2c-mpc.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)