* userspace firmware load fails with current linux-next @ 2013-10-23 9:06 Lothar Waßmann 2013-10-23 10:26 ` Ming Lei 0 siblings, 1 reply; 4+ messages in thread From: Lothar Waßmann @ 2013-10-23 9:06 UTC (permalink / raw) To: linux-arm-kernel Hi, with the current linux-next loading firmware from userspace fails because when writing to /sys/class/firmware/*/data the return code is always 0 (meaning to the userspace too that no data was written). Thus the userspace tool (mdev) keeps writing the same block of data over and over again. A cursory check of the latest updates to /drivers/base/firmware* didn't reveal anything that could be the cause of this misbehaviour. Interestingly when changing the .size member of the struct bin_attribute firmware_attr_data in drivers/base/firmware_class.c (which is now '0') to e.g. PAGE_SIZE firmware loading works again. Since .size was at '0' since the beginning of the .git universe there must have been a fundamental change in the guts of the kernel handling sysfs attribute files. Is this behavioural change intended? Do all sysfs attributes that are created with zero size need to be changed? Or is it an unintended side effect of some recent change? Lothar Wa?mann -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 4+ messages in thread
* userspace firmware load fails with current linux-next 2013-10-23 9:06 userspace firmware load fails with current linux-next Lothar Waßmann @ 2013-10-23 10:26 ` Ming Lei 2013-10-23 10:43 ` Lothar Waßmann 0 siblings, 1 reply; 4+ messages in thread From: Ming Lei @ 2013-10-23 10:26 UTC (permalink / raw) To: linux-arm-kernel Hi, On Wed, 23 Oct 2013 11:06:18 +0200 Lothar Wa?mann <LW@KARO-electronics.de> wrote: > Hi, > > with the current linux-next loading firmware from userspace fails > because when writing to /sys/class/firmware/*/data the return code is > always 0 (meaning to the userspace too that no data was written). > Thus the userspace tool (mdev) keeps writing the same block of data > over and over again. > > A cursory check of the latest updates to /drivers/base/firmware* didn't > reveal anything that could be the cause of this misbehaviour. > > Interestingly when changing the .size member of the > struct bin_attribute firmware_attr_data in drivers/base/firmware_class.c > (which is now '0') to e.g. PAGE_SIZE firmware loading works again. Thank you for the report and analysis. > > Since .size was at '0' since the beginning of the .git universe there > must have been a fundamental change in the guts of the kernel handling > sysfs attribute files. It may be introduced by Tejun's recent change. > > Is this behavioural change intended? I think no. > Do all sysfs attributes that are created with zero size need to be > changed? I think no, since drivers have no idea of size of their firmwares. > Or is it an unintended side effect of some recent change? Maybe yes, could you test below patch? Tejun, looks we need to keep special attention on zero size of bin file as before, could you comment at the patch? -- diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 5d818df..366ae8d 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -277,7 +277,7 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf, ssize_t len = min_t(size_t, count, PAGE_SIZE); char *buf; - if (sysfs_is_bin(of->sd)) { + if (sysfs_is_bin(of->sd) && size) { loff_t size = file_inode(file)->i_size; if (size <= *ppos) Thanks, -- Ming Lei ^ permalink raw reply related [flat|nested] 4+ messages in thread
* userspace firmware load fails with current linux-next 2013-10-23 10:26 ` Ming Lei @ 2013-10-23 10:43 ` Lothar Waßmann 2013-10-23 10:48 ` Ming Lei 0 siblings, 1 reply; 4+ messages in thread From: Lothar Waßmann @ 2013-10-23 10:43 UTC (permalink / raw) To: linux-arm-kernel Hi, Ming Lei wrote: > On Wed, 23 Oct 2013 11:06:18 +0200 > Lothar Wa?mann <LW@KARO-electronics.de> wrote: > [...] > > Or is it an unintended side effect of some recent change? > > Maybe yes, could you test below patch? > > Tejun, looks we need to keep special attention on zero size of bin file > as before, could you comment at the patch? > > -- > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > index 5d818df..366ae8d 100644 > --- a/fs/sysfs/file.c > +++ b/fs/sysfs/file.c > @@ -277,7 +277,7 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf, > ssize_t len = min_t(size_t, count, PAGE_SIZE); > char *buf; > > - if (sysfs_is_bin(of->sd)) { > + if (sysfs_is_bin(of->sd) && size) { > loff_t size = file_inode(file)->i_size; > > if (size <= *ppos) > > > The patch as is produces a build error, because 'size' is only defined after the 'if' statement. The modified patch below works for me. diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 5d818df..709d6f5 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -277,7 +277,7 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf, ssize_t len = min_t(size_t, count, PAGE_SIZE); char *buf; - if (sysfs_is_bin(of->sd)) { + if (sysfs_is_bin(of->sd) && file_inode(file)->i_size) { loff_t size = file_inode(file)->i_size; if (size <= *ppos) Lothar Wa?mann -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info@karo-electronics.de ___________________________________________________________ ^ permalink raw reply related [flat|nested] 4+ messages in thread
* userspace firmware load fails with current linux-next 2013-10-23 10:43 ` Lothar Waßmann @ 2013-10-23 10:48 ` Ming Lei 0 siblings, 0 replies; 4+ messages in thread From: Ming Lei @ 2013-10-23 10:48 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 23, 2013 at 6:43 PM, Lothar Wa?mann <LW@karo-electronics.de> wrote: > The patch as is produces a build error, because 'size' is only defined > after the 'if' statement. Sorry for that, :-) > > The modified patch below works for me. > > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > index 5d818df..709d6f5 100644 > --- a/fs/sysfs/file.c > +++ b/fs/sysfs/file.c > @@ -277,7 +277,7 @@ static ssize_t sysfs_write_file(struct file *file, const char __user *user_buf, > ssize_t len = min_t(size_t, count, PAGE_SIZE); > char *buf; > > - if (sysfs_is_bin(of->sd)) { > + if (sysfs_is_bin(of->sd) && file_inode(file)->i_size) { > loff_t size = file_inode(file)->i_size; > > if (size <= *ppos) OK, thanks for your test, and will prepare a formal one later. Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-23 10:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-23 9:06 userspace firmware load fails with current linux-next Lothar Waßmann 2013-10-23 10:26 ` Ming Lei 2013-10-23 10:43 ` Lothar Waßmann 2013-10-23 10:48 ` Ming Lei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox