Messages in this thread | | | Date | Thu, 18 Dec 2014 14:50:09 -0600 | From | atull <> | Subject | Re: [PATCH v2 2/3] fpga manager: framework core |
| |
On Sat, 6 Dec 2014, Grant Likely wrote:
> >> +int fpga_mgr_write(struct fpga_manager *mgr, const char *buf, size_t count) > >> +{ > >> + int ret; > >> + > >> + if (test_and_set_bit_lock(FPGA_MGR_BUSY, &mgr->flags)) > >> + return -EBUSY; > >> + > >> + dev_info(mgr->dev, "writing buffer to %s\n", mgr->name); > >> + > >> + ret = __fpga_mgr_write(mgr, buf, count); > >> + clear_bit_unlock(FPGA_MGR_BUSY, &mgr->flags); > >> + > >> + return ret; > >> +} > >> +EXPORT_SYMBOL_GPL(fpga_mgr_write); > > > > Is the EBUSY -- userspace please try again, but you don't know when to > > try again -- right interface? I mean, normally kernel would wait, so > > that userland does not have to poll? > > Custom locking schemes are just wrong. A mutex is the right thing to > do here and then an -EBUSY isn't required. >
I've changed it to a mutex in the next version.
> > > >> +static ssize_t fpga_mgr_firmware_store(struct device *dev, > >> + struct device_attribute *attr, > >> + const char *buf, size_t count) > >> +{ > >> + struct fpga_manager *mgr = dev_get_drvdata(dev); > >> + unsigned int len; > >> + char image_name[NAME_MAX]; > > Hard coding a string length is a warning sign. That is the sort of > thing that can memdup() or strdup() can handle. >
OK, I'll fix it using kstrdup() in v6.
Alan
| |