init dynamic bin_attribute structures

init dynamic bin_attribute structures

[Wolfram Sang]

Hello,

I was pointed to the fact that commit 6992f5334995af474c2b58d010d08bc597f0f2fe
requires adjustments to dynamically created sysfs-attributes when Albrecht Dreß
tried to compile the eeprom-driver at24 with lockdep enabled. After fixing this
for at24 and at25, I came up with this coccinelle-script to find further
candidates:

@ init @
identifier struct_name, bin;
@@

	struct struct_name {
		...
		struct bin_attribute bin;
		...
	};

@ main extends init @
expression E;
statement S;
identifier name, err;
@@

(
	struct struct_name *name;
|
-	struct struct_name *name = NULL;
+	struct struct_name *name;
)
	...
(
	sysfs_bin_attr_init(&name->bin);
|
+	sysfs_bin_attr_init(&name->bin);
	if (sysfs_create_bin_file(E, &name->bin))
		S
|
+	sysfs_bin_attr_init(&name->bin);
	err = sysfs_create_bin_file(E, &name->bin);
)

(Note: I did a similar script for non-binary attributes, too. That one didn't
find a match)

The resulting series is the edited outcome of that. "Edited" because the
patches were manually reviewed afterwards to find the best place to add the
fix. The patches are not more than compile-tested on x86 where possible due to
no hardware. Please also note that my semantic patch is probably not perfect
and may have missed code still needing the fix.

Comments welcome, of course.

Regards,

   Wolfram Sang

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] arch/mips/txx9/generic: init dynamic bin_attribute structures

[Wolfram Sang]

Commit 6992f5334995af474c2b58d010d08bc597f0f2fe introduced this requirement.
Found with coccinelle, but fixed manually. Compile tested on X86 where
possible.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
 arch/mips/txx9/generic/setup.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index 7174d83..95184a0 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -956,6 +956,7 @@ void __init txx9_sramc_init(struct resource *r)
 	if (!dev->base)
 		goto exit;
 	dev->dev.cls = &txx9_sramc_sysdev_class;
+	sysfs_bin_attr_init(&dev->bindata_attr);
 	dev->bindata_attr.attr.name = "bindata";
 	dev->bindata_attr.attr.mode = S_IRUSR | S_IWUSR;
 	dev->bindata_attr.read = txx9_sram_read;
-- 
1.7.0

Re: [PATCH 1/3] arch/mips/txx9/generic: init dynamic bin_attribute

[Dmitry Torokhov]

On Fri, Mar 12, 2010 at 08:03:49AM +0100, Wolfram Sang wrote:
> Commit 6992f5334995af474c2b58d010d08bc597f0f2fe introduced this requirement.
> Found with coccinelle, but fixed manually. Compile tested on X86 where
> possible.
> 

Regarding all 3 - it looks like these dynamically alocated attributes
could be converted to statically allocated ones. I'd recommend doing
that instead (in fact, I posted patch for the firmware_class couple days
ago).

> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  arch/mips/txx9/generic/setup.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> index 7174d83..95184a0 100644
> --- a/arch/mips/txx9/generic/setup.c
> +++ b/arch/mips/txx9/generic/setup.c
> @@ -956,6 +956,7 @@ void __init txx9_sramc_init(struct resource *r)
>  	if (!dev->base)
>  		goto exit;
>  	dev->dev.cls = &txx9_sramc_sysdev_class;
> +	sysfs_bin_attr_init(&dev->bindata_attr);
>  	dev->bindata_attr.attr.name = "bindata";
>  	dev->bindata_attr.attr.mode = S_IRUSR | S_IWUSR;
>  	dev->bindata_attr.read = txx9_sram_read;

-- 
Dmitry

Re: [PATCH 1/3] arch/mips/txx9/generic: init dynamic bin_attribute

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]

On Fri, Mar 12, 2010 at 10:34:51AM -0800, Dmitry Torokhov wrote:
> On Fri, Mar 12, 2010 at 08:03:49AM +0100, Wolfram Sang wrote:
> > Commit 6992f5334995af474c2b58d010d08bc597f0f2fe introduced this requirement.
> > Found with coccinelle, but fixed manually. Compile tested on X86 where
> > possible.
> > 
> 
> Regarding all 3 - it looks like these dynamically alocated attributes
> could be converted to statically allocated ones. I'd recommend doing
> that instead (in fact, I posted patch for the firmware_class couple days
> ago).

I agree for the firmware-patch. Regarding the MIPS one, 'size' might differ and
'private' will differ per instance. Regarding the RTC driver, 'size' might also
differ. I don't know if somebody really wants two RTCs or the SRAM for MIPS can
be instantiated more than once. Unless somebody with actual hardware jumps in,
I'd say better safe than sorry.

Thanks for the comment!

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/3] arch/mips/txx9/generic: init dynamic bin_attribute

[Dmitry Torokhov]

On Sat, Mar 13, 2010 at 03:28:55AM +0100, Wolfram Sang wrote:
> On Fri, Mar 12, 2010 at 10:34:51AM -0800, Dmitry Torokhov wrote:
> > On Fri, Mar 12, 2010 at 08:03:49AM +0100, Wolfram Sang wrote:
> > > Commit 6992f5334995af474c2b58d010d08bc597f0f2fe introduced this requirement.
> > > Found with coccinelle, but fixed manually. Compile tested on X86 where
> > > possible.
> > > 
> > 
> > Regarding all 3 - it looks like these dynamically alocated attributes
> > could be converted to statically allocated ones. I'd recommend doing
> > that instead (in fact, I posted patch for the firmware_class couple days
> > ago).
> 
> I agree for the firmware-patch. Regarding the MIPS one, 'size' might differ and
> 'private' will differ per instance. Regarding the RTC driver, 'size' might also
> differ. I don't know if somebody really wants two RTCs or the SRAM for MIPS can
> be instantiated more than once. Unless somebody with actual hardware jumps in,
> I'd say better safe than sorry.
> 

Ah, right, size... I forgot about it. You are right, making the other 2
static is not an option.

-- 
Dmitry

Re: [PATCH 1/3] arch/mips/txx9/generic: init dynamic bin_attribute

[Ralf Baechle]

On Sat, Mar 13, 2010 at 03:28:55AM +0100, Wolfram Sang wrote:

> > Regarding all 3 - it looks like these dynamically alocated attributes
> > could be converted to statically allocated ones. I'd recommend doing
> > that instead (in fact, I posted patch for the firmware_class couple days
> > ago).
> 
> I agree for the firmware-patch. Regarding the MIPS one, 'size' might differ and
> 'private' will differ per instance. Regarding the RTC driver, 'size' might also
> differ. I don't know if somebody really wants two RTCs or the SRAM for MIPS can
> be instantiated more than once. Unless somebody with actual hardware jumps in,
> I'd say better safe than sorry.

On the txx9 platform you've posted the patch for additional RTCs or SRAMs
would not normally be expected.  On other platforms such as IP27 there
would be one per node that is potencially very many.

  Ralf

[PATCH 2/3] drivers/base: init dynamic bin_attribute structures

[Wolfram Sang]

Commit 6992f5334995af474c2b58d010d08bc597f0f2fe introduced this requirement.
Found with coccinelle, but fixed manually. Compile tested on X86 where
possible.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
 drivers/base/firmware_class.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index d0dc26a..443231f 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -391,6 +391,7 @@ static int fw_register_device(struct device **dev_p, const char *fw_name,
 
 	init_completion(&fw_priv->completion);
 	fw_priv->attr_data = firmware_attr_data_tmpl;
+	sysfs_bin_attr_init(&fw_priv->attr_data);
 	fw_priv->fw_id = kstrdup(fw_name, GFP_KERNEL);
 	if (!fw_priv->fw_id) {
 		dev_err(device, "%s: Firmware name allocation failed\n",
-- 
1.7.0

Re: [PATCH 2/3] drivers/base: init dynamic bin_attribute structures

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Fri, Mar 12, 2010 at 08:03:50AM +0100, Wolfram Sang wrote:
> Commit 6992f5334995af474c2b58d010d08bc597f0f2fe introduced this requirement.
> Found with coccinelle, but fixed manually. Compile tested on X86 where
> possible.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Eric W. Biederman <ebiederm@xmission.com>

Please drop in favor of http://lkml.org/lkml/2010/3/11/57

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 3/3] drivers/rtc: init dynamic bin_attribute structures

[Wolfram Sang]

Commit 6992f5334995af474c2b58d010d08bc597f0f2fe introduced this requirement.
Found with coccinelle, but fixed manually. Compile tested on X86 where
possible.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Paul Gortmaker <p_gortmaker@yahoo.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
 drivers/rtc/rtc-ds1742.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/rtc-ds1742.c b/drivers/rtc/rtc-ds1742.c
index a127336..cad9ceb 100644
--- a/drivers/rtc/rtc-ds1742.c
+++ b/drivers/rtc/rtc-ds1742.c
@@ -184,6 +184,7 @@ static int __devinit ds1742_rtc_probe(struct platform_device *pdev)
 	pdata->size_nvram = pdata->size - RTC_SIZE;
 	pdata->ioaddr_rtc = ioaddr + pdata->size_nvram;
 
+	sysfs_bin_attr_init(&pdata->nvram_attr);
 	pdata->nvram_attr.attr.name = "nvram";
 	pdata->nvram_attr.attr.mode = S_IRUGO | S_IWUSR;
 	pdata->nvram_attr.read = ds1742_nvram_read;
-- 
1.7.0