Re: [PATCH] v2 Staging : Add RIFFA PCIe staging driver
> For further details, please refer to
> https://github.com/KastnerRG/riffa/pull/31
> That is not permanent, please provide the details here.
This patch helps to enable bi-directional PCIe communication at PCIe gen2 speed
grade
Major change in this patch is the enabling of chnl_recv() scatter-gather list
first in the case of loopback,
as you can see in
https://github.com/promach/riffa/blob/full_duplex/driver/linux/riffa_driver.c#L832-L836
Some other changes include splitting TX and RX into two separate FSMs in two
always blocks as you can see in chnl_tester.v
Also, I need to give credit to @marzoul for maintaining the kernel functions
which I have already included in this patch altogether.
However, this patch degrades bandwidth measurement result with respect to
Xillybus
and riffa original driver (slower with an approximate factor of 3 at the same
given data batch length)
> Please fix this all up and submit a v3, after at least commenting on the
> things asked previously.
Please allows time to clear up my confusion first regarding the following
questions about subvendor ID and subdevice ID before v3 patch submission
> Also, you still have this line which prevents me from being able to
> accept this patch, as I talked about previously:
> + {PCI_DEVICE(VENDOR_ID1, PCI_ANY_ID)},
> Do NOT just bind to all PCI devices from a vendor, that will break other
> drivers in the system.
This line is for subvendor ID and subdevice ID. I do not think lspci helps here.
Could you suggest how to get around PCI_ANY_ID for subdevice ID ?
Same question for subvendor ID.
phung@UbuntuHW15:~/riffa_backup/driver/linux$ sudo locate -b "pci.ids"
/usr/share/hwdata/pci.ids
/usr/share/misc/pci.ids
phung@UbuntuHW15:~/riffa_backup/driver/linux$ cat /usr/share/hwdata/pci.ids |
grep pci
# the PCI ID Project at http://pci-ids.ucw.cz/.
a000 Psycho UPA-PCI Bus Module [pcipsy]
a001 Psycho UPA-PCI Bus Module [pcipsy]
a801 Schizo Fireplane-PCI bus bridge module [pcisch]
1113 8201 T-Com T-Sinus 154pcicard Wireless PCI Adapter
phung@UbuntuHW15:~/riffa_backup/driver/linux$ cat /usr/share/misc/pci.ids |
grep pci
# the PCI ID Project at http://pci-ids.ucw.cz/.
a000 Psycho UPA-PCI Bus Module [pcipsy]
a001 Psycho UPA-PCI Bus Module [pcipsy]
a801 Schizo Fireplane-PCI bus bridge module [pcisch]
1113 8201 T-Com T-Sinus 154pcicard Wireless PCI Adapter
phung@UbuntuHW15:~/riffa_backup/driver/linux$
> Also, do not use a random MAJOR number that you just picked out of no
> where, that too will break working systems and needs to be fixed. Use
> the dynamic allocation method, or better yet, just use a misc device.
What do you exactly mean by misc device ?
> How do you plan to proceed ?
please keep the driver in the staging state until it can really match
Xillybus or Riffa original driver's bandwidth benchmark.
I am actually a bit worried with circ_queue structure and its corresponding
push() and pull() functions which systemtap complains to have some
significant timing overhead, but to be frank, this is not the root cause of
why the full duplex version of the driver is slower.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] v2 Staging : Add RIFFA PCIe staging driver
On Tue, Dec 04, 2018 at 08:13:10AM +, Cheng Fei Phung wrote:
>
> > For further details, please refer to
> > https://github.com/KastnerRG/riffa/pull/31
> > That is not permanent, please provide the details here.
>
> This patch helps to enable bi-directional PCIe communication at PCIe gen2
> speed grade
> Major change in this patch is the enabling of chnl_recv() scatter-gather list
> first in the case of loopback,
> as you can see in
> https://github.com/promach/riffa/blob/full_duplex/driver/linux/riffa_driver.c#L832-L836
>
> Some other changes include splitting TX and RX into two separate FSMs in two
> always blocks as you can see in chnl_tester.v
> Also, I need to give credit to @marzoul for maintaining the kernel functions
> which I have already included in this patch altogether.
>
> However, this patch degrades bandwidth measurement result with respect to
> Xillybus
> and riffa original driver (slower with an approximate factor of 3 at the same
> given data batch length)
Put all of this in your patch description, and again, properly line-wrap
your lines at 72 columns. I'm getting tired of asking this...
> > Please fix this all up and submit a v3, after at least commenting on the
> > things asked previously.
>
> Please allows time to clear up my confusion first regarding the following
> questions about subvendor ID and subdevice ID before v3 patch submission
>
>
> > Also, you still have this line which prevents me from being able to
> > accept this patch, as I talked about previously:
>
> > + {PCI_DEVICE(VENDOR_ID1, PCI_ANY_ID)},
>
> > Do NOT just bind to all PCI devices from a vendor, that will break other
> > drivers in the system.
>
> This line is for subvendor ID and subdevice ID. I do not think lspci helps
> here.
What do you mean by this?
You just told the kernel that ALL devices with PCI vendor id of
VENDOR_ID1 (horrible description) are handled properly by this driver.
I do not think that is what you really want, correct?
> Could you suggest how to get around PCI_ANY_ID for subdevice ID ?
> Same question for subvendor ID.
Look at include/linux/pci.h. Use the PCI_DEVICE_SUB() macro here or one
of the other macros defined in that file to properly describe your
device as being unique and not "ALL DEVICES FROM A VENDOR".
> > Also, do not use a random MAJOR number that you just picked out of no
> > where, that too will break working systems and needs to be fixed. Use
> > the dynamic allocation method, or better yet, just use a misc device.
>
> What do you exactly mean by misc device ?
Look at include/linux/miscdevice.h for the details.
> > How do you plan to proceed ?
>
> please keep the driver in the staging state until it can really match
> Xillybus or Riffa original driver's bandwidth benchmark.
>
> I am actually a bit worried with circ_queue structure and its corresponding
> push() and pull() functions which systemtap complains to have some
> significant timing overhead, but to be frank, this is not the root cause of
> why the full duplex version of the driver is slower.
Yes, that's kind of obvious by looking at that code :)
Again, fix up the driver to use the in-kernel data structures for this
type of thing and you will see a much better improvement in performance.
good luck!
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFCv2 3/4] mm/memory_hotplug: Introduce and use more memory types
On Fri, 30 Nov 2018 18:59:21 +0100 David Hildenbrand wrote: > Let's introduce new types for different kinds of memory blocks and use > them in existing code. As I don't see an easy way to split this up, > do it in one hunk for now. > > acpi: > Use DIMM or DIMM_UNREMOVABLE depending on hotremove support in the kernel. > Properly change the type when trying to add memory that was already > detected and used during boot (so this memory will correctly end up as > "acpi" in user space). > > pseries: > Use DIMM or DIMM_UNREMOVABLE depending on hotremove support in the kernel. > As far as I see, handling like in the acpi case for existing blocks is > not required. > > probed memory from user space: > Use DIMM_UNREMOVABLE as there is no interface to get rid of this code > again. > > hv_balloon,xen/balloon: > Use BALLOON. As simple as that :) > > s390x/sclp: > Use a dedicated type S390X_STANDBY as this type of memory and it's > semantics are very s390x specific. > > powernv/memtrace: > Only allow to use BOOT memory for memtrace. I consider this code in > general dangerous, but we have to keep it working ... most probably just > a debug feature. I don't think it should be arbitrarily restricted like that. Thanks Michal ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFCv2 3/4] mm/memory_hotplug: Introduce and use more memory types
On 04.12.18 10:44, Michal Suchánek wrote: > On Fri, 30 Nov 2018 18:59:21 +0100 > David Hildenbrand wrote: > >> Let's introduce new types for different kinds of memory blocks and use >> them in existing code. As I don't see an easy way to split this up, >> do it in one hunk for now. >> >> acpi: >> Use DIMM or DIMM_UNREMOVABLE depending on hotremove support in the kernel. >> Properly change the type when trying to add memory that was already >> detected and used during boot (so this memory will correctly end up as >> "acpi" in user space). >> >> pseries: >> Use DIMM or DIMM_UNREMOVABLE depending on hotremove support in the kernel. >> As far as I see, handling like in the acpi case for existing blocks is >> not required. >> >> probed memory from user space: >> Use DIMM_UNREMOVABLE as there is no interface to get rid of this code >> again. >> >> hv_balloon,xen/balloon: >> Use BALLOON. As simple as that :) >> >> s390x/sclp: >> Use a dedicated type S390X_STANDBY as this type of memory and it's >> semantics are very s390x specific. >> >> powernv/memtrace: >> Only allow to use BOOT memory for memtrace. I consider this code in >> general dangerous, but we have to keep it working ... most probably just >> a debug feature. > > I don't think it should be arbitrarily restricted like that. > Well code that "randomly" offlines/onlines/removes/adds memory blocks that it does not own (hint: nobody else in the kernel does that), should be restricted to types we can guarantee to work. > Thanks > > Michal > -- Thanks, David / dhildenb ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] binder: implement binderfs
As discussed at Linux Plumbers Conference 2018 in Vancouver [1] this is the
implementation of binderfs. If you want to skip reading and just see how it
works, please go to [2].
binderfs is a backwards-compatible filesystem for Android's binder ipc
mechanism. Each ipc namespace will mount a new binderfs instance. Mounting
binderfs multiple times at different locations in the same ipc namespace
will not cause a new super block to be allocated and hence it will be the
same filesystem instance.
Each new binderfs mount will have its own set of binder devices only
visible in the ipc namespace it has been mounted in. All devices in a new
binderfs mount will follow the scheme binder%d and numbering will always
start at 0.
/* Backwards compatibility */
Devices requested in the Kconfig via CONFIG_ANDROID_BINDER_DEVICES for the
initial ipc namespace will work as before. They will be registered via
misc_register() and appear in the devtmpfs mount. Specifically, the
standard devices binder, hwbinder, and vndbinder will all appear in their
standard locations in /dev. Mounting or unmounting the binderfs mount in
the initial ipc namespace will have no effect on these devices, i.e. they
will neither show up in the binderfs mount nor will they disappear when the
binderfs mount is gone.
/* binder-control */
Each new binderfs instance comes with a binder-control device. No other
devices will be present at first. The binder-control device can be used to
dynamically allocate binder devices. All requests operate on the binderfs
mount the binder-control device resides in:
- BINDER_CTL_ADD
Allocate a new binder device.
Assuming a new instance of binderfs has been mounted at /dev/binderfs via
mount -t binderfs binderfs /dev/binderfs. Then a request to create a new
binder device can be made via:
struct binderfs_device device = {0};
int fd = open("/dev/binderfs/binder-control", O_RDWR);
ioctl(fd, BINDER_CTL_ADD, &device);
The struct binderfs_device will be used to return the major and minor
number, as well as the index used as the new name for the device.
Binderfs devices can simply be removed via unlink().
/* Implementation details */
- When binderfs is registered as a new filesystem it will dynamically
allocate a new major number. The allocated major number will be returned
in struct binderfs_device when a new binder device is allocated.
Minor numbers that have been given out are tracked in a global idr struct
that is capped at BINDERFS_MAX_MINOR. The minor number tracker is
protected by a global mutex. This is the only point of contention between
binderfs mounts.
- The naming scheme for binder devices is binder%d. Each binderfs mount
starts numbering of new binder devices at 0 up to n. The indeces used in
constructing the name are tracked in a struct idr that is per-binderfs
super block.
- Each binderfs super block has its own struct binderfs_info that tracks
specific details about a binderfs instance: the ipc namespace, the idr
tracker for new names, the dentry of the binder-control device, the
root uid and gid of the user namespace the binderfs instance was mounted
in, and a mutex to protect the binder-control device and idr tracker from
concurrent access.
- binderfs can be mounted by user namespace root in a non-initial user
namespace. The devices will be owned by user namespace root.
- New binder devices associated with a binderfs mount do not use the
full misc_register() infrastructure. The misc_register() infrastructure
can only create new devices in the host's devtmpfs mount. binderfs does
however only make devices appear under its own mountpoint and thus
allocates new character devices nodes from the inode of the root dentry
of the super block. This will have the side-effect that binderfs specific
device nodes do not appear in sysfs. This behavior is similar to devpts
allocated pts devices and has no effect on the functionality of the ipc
mechanism itself.
/* Create a new binder device in a binderfs mount */
sudo mkdir /dev/binderfs
sudo mount -t binder binder /dev/binderfs
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
int main(int argc, char *argv[])
{
int fd, ret, saved_errno;
struct binderfs_device device = { 0 };
fd = open("/dev/binderfs/binder-control", O_RDONLY | O_CLOEXEC);
if (fd < 0) {
printf("%s - Failed to open binder-control device\n",
strerror(errno));
exit(EXIT_FAILURE);
}
ret = ioctl(fd, BINDER_CTL_ADD, &device);
saved_errno = errno;
close(fd);
errno = saved_errno;
if (ret < 0) {
printf("%s - Failed to allocate new binder device\n",
strerror(errno));
exit(EXIT_FAILURE);
}
printf("Allocated new binder device with major %d, minor %d, and "
Re: [PATCH] staging: comedi: change do_insn*_ioctl to allow more samples
On Wed, Oct 31, 2018 at 4:49 AM Ian Abbott wrote:
>
> On 25/10/18 02:36, Spencer E. Olson wrote:
> > Changes do_insn*_ioctl functions to allow for data lengths for each
> > comedi_insn of up to 2^16. This patch also changes these functions to only
> > allocate as much memory as is necessary for each comedi_insn, rather than
> > allocating a fixed-sized scratch space.
> >
> > In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES
> > facility with some newer hardware, I discovered that do_insn_ioctl and
> > do_insnlist_ioctl limited the amount of data that can be passed into the
> > kernel for insn's to a length of 256. For some newer hardware, the number
> > of routes can be greater than 1000. Working around the old limits (256)
> > would complicate the user-space/kernel interaction.
> >
> > The new upper limit is reasonable with current memory available and does
> > not otherwise impact the memory footprint for any current or otherwise
> > typical configuration.
> >
> > Signed-off-by: Spencer E. Olson
> > ---
> > drivers/staging/comedi/comedi_fops.c | 37 +---
> > 1 file changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/comedi_fops.c
> > b/drivers/staging/comedi/comedi_fops.c
> > index c1c6b2b4ab91..a163ec2df872 100644
> > --- a/drivers/staging/comedi/comedi_fops.c
> > +++ b/drivers/staging/comedi/comedi_fops.c
> > @@ -1500,7 +1500,7 @@ static int parse_insn(struct comedi_device *dev,
> > struct comedi_insn *insn,
> >* data (for reads) to insns[].data pointers
> >*/
> > /* arbitrary limits */
> > -#define MAX_SAMPLES 256
> > +#define MAX_SAMPLES 65536
> > static int do_insnlist_ioctl(struct comedi_device *dev,
> >struct comedi_insnlist __user *arg, void *file)
> > {
> > @@ -1513,12 +1513,6 @@ static int do_insnlist_ioctl(struct comedi_device
> > *dev,
> > if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
> > return -EFAULT;
> >
> > - data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
> > - if (!data) {
> > - ret = -ENOMEM;
> > - goto error;
> > - }
> > -
> > insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
> > if (!insns) {
> > ret = -ENOMEM;
> > @@ -1539,6 +1533,14 @@ static int do_insnlist_ioctl(struct comedi_device
> > *dev,
> > ret = -EINVAL;
> > goto error;
> > }
> > +
> > + data = kmalloc_array(insns[i].n, sizeof(unsigned int),
> > + GFP_KERNEL);
> > + if (!data) {
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > +
>
> Something you could do here is work out which insn->n in the instruction
> list is the largest, and allocate the 'data' once outside the
> instruction list handling loop instead of allocating it inside the loop.
I just realized that I left this unfinished. How much headway did you
already make with fixing the various other drivers that had previously
not checked length of insn->n?
(I'll try your last suggestion and get that submitted in the next day or so.)
>
> --
> -=( Ian Abbott || Web: www.mev.co.uk )=-
> -=( MEV Ltd. is a company registered in England & Wales. )=-
> -=( Registered number: 02862268. Registered address:)=-
> -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/26] Staging: fbtft: flexfb: Switch to the gpio descriptor interface
On Mon, Nov 26, 2018 at 01:13:08PM +0300, Dan Carpenter wrote:
> On Sun, Nov 25, 2018 at 04:56:29PM +0530, Nishad Kamdar wrote:
> > This switches the flexfb.c to use GPIO descriptors
> > rather than numerical gpios.
> >
> > Signed-off-by: Nishad Kamdar
> > ---
> > drivers/staging/fbtft/flexfb.c | 12 ++--
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
> > index 2af474469e7d..c5fa59105a43 100644
> > --- a/drivers/staging/fbtft/flexfb.c
> > +++ b/drivers/staging/fbtft/flexfb.c
> > @@ -9,7 +9,7 @@
> > #include
> > #include
> > #include
> > -#include
> > +#include
> > #include
> > #include
> >
> > @@ -521,7 +521,7 @@ static int flexfb_verify_gpios_dc(struct fbtft_par *par)
> > {
> > fbtft_par_dbg(DEBUG_VERIFY_GPIOS, par, "%s()\n", __func__);
> >
> > - if (par->gpio.dc < 0) {
> > + if (!par->gpio.dc) {
> > dev_err(par->info->device,
> > "Missing info about 'dc' gpio. Aborting.\n");
> > return -EINVAL;
>
> We changed par->gpio.c from an int to a pointer in patch 1 so we have
> to update all the checks as well in the same patch. Otherwise it breaks
> `git bisect`.
>
> (I don't know this code well. But it just feels like it has to be
> breaking git bisect just from from glancing at the patches. Perhaps I
> have misunderstood).
>
> regards,
> dan carpenter
>
Ok. I'll merge them into one patch.
Thanks for the review and the pointer.
Regards,
Nishad
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] Staging: fbtft: Switch to the gpio descriptor interface
This switches the fbtft driver to use GPIO descriptors
rather than numerical gpios:
Utilize the GPIO library's intrinsic handling of OF GPIOs
and polarity. If the line is flagged active low, gpiolib
will deal with this.
Remove gpios from platform device structure. Neither assign
statically numbers to gpios in platform device nor allow
gpios to be parsed as module parameters.
Signed-off-by: Nishad Kamdar
Changes in v2:
- Merge all patches in a single patch. This is because the
first patch changed par->gpio from an int to a pointer
so all the checks have to be updated in the same patch.
Otherwie it breaks 'git bisect'.
---
drivers/staging/fbtft/fb_agm1264k-fl.c | 52 ++--
drivers/staging/fbtft/fb_bd663474.c| 6 +-
drivers/staging/fbtft/fb_ili9163.c | 6 +-
drivers/staging/fbtft/fb_ili9320.c | 2 +-
drivers/staging/fbtft/fb_ili9325.c | 6 +-
drivers/staging/fbtft/fb_ili9340.c | 2 +-
drivers/staging/fbtft/fb_pcd8544.c | 4 +-
drivers/staging/fbtft/fb_ra8875.c | 4 +-
drivers/staging/fbtft/fb_s6d1121.c | 6 +-
drivers/staging/fbtft/fb_sh1106.c | 2 +-
drivers/staging/fbtft/fb_ssd1289.c | 6 +-
drivers/staging/fbtft/fb_ssd1305.c | 4 +-
drivers/staging/fbtft/fb_ssd1306.c | 4 +-
drivers/staging/fbtft/fb_ssd1325.c | 6 +-
drivers/staging/fbtft/fb_ssd1331.c | 10 +-
drivers/staging/fbtft/fb_ssd1351.c | 2 +-
drivers/staging/fbtft/fb_tls8204.c | 6 +-
drivers/staging/fbtft/fb_uc1611.c | 4 +-
drivers/staging/fbtft/fb_uc1701.c | 6 +-
drivers/staging/fbtft/fb_upd161704.c | 6 +-
drivers/staging/fbtft/fb_watterott.c | 4 +-
drivers/staging/fbtft/fbtft-bus.c | 6 +-
drivers/staging/fbtft/fbtft-core.c | 173 +++--
drivers/staging/fbtft/fbtft-io.c | 26 +-
drivers/staging/fbtft/fbtft.h | 21 +-
drivers/staging/fbtft/fbtft_device.c | 344 +
drivers/staging/fbtft/flexfb.c | 12 +-
27 files changed, 143 insertions(+), 587 deletions(-)
diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c
b/drivers/staging/fbtft/fb_agm1264k-fl.c
index f6f30f5bf15a..8f27bd8da17d 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -8,7 +8,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -79,14 +79,14 @@ static int init_display(struct fbtft_par *par)
static void reset(struct fbtft_par *par)
{
- if (par->gpio.reset == -1)
+ if (!par->gpio.reset)
return;
dev_dbg(par->info->device, "%s()\n", __func__);
- gpio_set_value(par->gpio.reset, 0);
+ gpiod_set_value(par->gpio.reset, 0);
udelay(20);
- gpio_set_value(par->gpio.reset, 1);
+ gpiod_set_value(par->gpio.reset, 1);
mdelay(120);
}
@@ -98,30 +98,30 @@ static int verify_gpios(struct fbtft_par *par)
dev_dbg(par->info->device,
"%s()\n", __func__);
- if (par->EPIN < 0) {
+ if (!par->EPIN) {
dev_err(par->info->device,
"Missing info about 'wr' (aka E) gpio. Aborting.\n");
return -EINVAL;
}
for (i = 0; i < 8; ++i) {
- if (par->gpio.db[i] < 0) {
+ if (!par->gpio.db[i]) {
dev_err(par->info->device,
"Missing info about 'db[%i]' gpio. Aborting.\n",
i);
return -EINVAL;
}
}
- if (par->CS0 < 0) {
+ if (!par->CS0) {
dev_err(par->info->device,
"Missing info about 'cs0' gpio. Aborting.\n");
return -EINVAL;
}
- if (par->CS1 < 0) {
+ if (!par->CS1) {
dev_err(par->info->device,
"Missing info about 'cs1' gpio. Aborting.\n");
return -EINVAL;
}
- if (par->RW < 0) {
+ if (!par->RW) {
dev_err(par->info->device,
"Missing info about 'rw' gpio. Aborting.\n");
return -EINVAL;
@@ -139,22 +139,22 @@ request_gpios_match(struct fbtft_par *par, const struct
fbtft_gpio *gpio)
if (strcasecmp(gpio->name, "wr") == 0) {
/* left ks0108 E pin */
par->EPIN = gpio->gpio;
- return GPIOF_OUT_INIT_LOW;
+ return GPIOD_OUT_LOW;
} else if (strcasecmp(gpio->name, "cs0") == 0) {
/* left ks0108 controller pin */
par->CS0 = gpio->gpio;
- return GPIOF_OUT_INIT_HIGH;
+ return GPIOD_OUT_HIGH;
} else if (strcasecmp(gpio->name, "cs1") == 0) {
/* right ks0108 controller pin */
par->CS1 = gpio->gpio;
- return GPIOF_OUT_INIT_HIGH;
+ return GPIOD_OUT_HIGH;
}
/* if write (rw = 0) e(1-
[PATCH v2] staging: comedi: change do_insn*_ioctl to allow more samples
Changes do_insn*_ioctl functions to allow for data lengths for each
comedi_insn of up to 2^16. This patch also changes these functions to only
allocate as much memory as is necessary for each comedi_insn, rather than
allocating a fixed-sized scratch space.
In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES
facility with some newer hardware, I discovered that do_insn_ioctl and
do_insnlist_ioctl limited the amount of data that can be passed into the
kernel for insn's to a length of 256. For some newer hardware, the number
of routes can be greater than 1000. Working around the old limits (256)
would complicate the user-space/kernel interaction.
The new upper limit is reasonable with current memory available and does
not otherwise impact the memory footprint for any current or otherwise
typical configuration.
Signed-off-by: Spencer E. Olson
---
Implements Ian's suggestions to:
1) Provide a minimum amount of space to allocate (16*sizeof(uint)) to protect
drivers that do not (yet) check the size of the instruction data (Ian has
submitted several patches fixing this for other drivers already). In case
insn.n does not get set, this minimum amount also avoids trying to allocate
zero-length data.
2) Allocate the maximum required space needed for any of the instructions in an
instruction list before executing the list of instructions (this, rather than
allocating and freeing memory separately while iterating through the
instruction list and executing each instruction).
drivers/staging/comedi/comedi_fops.c | 48 ++--
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index ceb6ba5dd57c..5d2fcbfe02af 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1501,25 +1501,21 @@ static int parse_insn(struct comedi_device *dev, struct
comedi_insn *insn,
* data (for reads) to insns[].data pointers
*/
/* arbitrary limits */
-#define MAX_SAMPLES 256
+#define MIN_SAMPLES 16
+#define MAX_SAMPLES 65536
static int do_insnlist_ioctl(struct comedi_device *dev,
struct comedi_insnlist __user *arg, void *file)
{
struct comedi_insnlist insnlist;
struct comedi_insn *insns = NULL;
unsigned int *data = NULL;
+ unsigned int max_n_data_required = MIN_SAMPLES;
int i = 0;
int ret = 0;
if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
return -EFAULT;
- data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto error;
- }
-
insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
if (!insns) {
ret = -ENOMEM;
@@ -1533,13 +1529,26 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
goto error;
}
- for (i = 0; i < insnlist.n_insns; i++) {
+ /* Determine maximum memory needed for all instructions. */
+ for (i = 0; i < insnlist.n_insns; ++i) {
if (insns[i].n > MAX_SAMPLES) {
dev_dbg(dev->class_dev,
"number of samples too large\n");
ret = -EINVAL;
goto error;
}
+ max_n_data_required = max(max_n_data_required, insns[i].n);
+ }
+
+ /* Allocate scratch space for all instruction data. */
+ data = kmalloc_array(max_n_data_required, sizeof(unsigned int),
+GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ for (i = 0; i < insnlist.n_insns; ++i) {
if (insns[i].insn & INSN_MASK_WRITE) {
if (copy_from_user(data, insns[i].data,
insns[i].n * sizeof(unsigned int))) {
@@ -1593,22 +1602,27 @@ static int do_insn_ioctl(struct comedi_device *dev,
{
struct comedi_insn insn;
unsigned int *data = NULL;
+ unsigned int n_data = MIN_SAMPLES;
int ret = 0;
- data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto error;
- }
-
if (copy_from_user(&insn, arg, sizeof(insn))) {
- ret = -EFAULT;
- goto error;
+ return -EFAULT;
}
+ n_data = max(n_data, insn.n);
+
/* This is where the behavior of insn and insnlist deviate. */
- if (insn.n > MAX_SAMPLES)
+ if (insn.n > MAX_SAMPLES) {
insn.n = MAX_SAMPLES;
+ n_data = MAX_SAMPLES;
+ }
+
+ data = kmalloc_array(n_data, sizeof(unsigned int), GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
Re: [PATCH v2] staging: comedi: change do_insn*_ioctl to allow more samples
On Tue, Dec 4, 2018 at 12:08 PM Spencer E. Olson wrote:
>
> Changes do_insn*_ioctl functions to allow for data lengths for each
> comedi_insn of up to 2^16. This patch also changes these functions to only
> allocate as much memory as is necessary for each comedi_insn, rather than
> allocating a fixed-sized scratch space.
>
> In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES
> facility with some newer hardware, I discovered that do_insn_ioctl and
> do_insnlist_ioctl limited the amount of data that can be passed into the
> kernel for insn's to a length of 256. For some newer hardware, the number
> of routes can be greater than 1000. Working around the old limits (256)
> would complicate the user-space/kernel interaction.
>
> The new upper limit is reasonable with current memory available and does
> not otherwise impact the memory footprint for any current or otherwise
> typical configuration.
>
> Signed-off-by: Spencer E. Olson
> ---
> Implements Ian's suggestions to:
> 1) Provide a minimum amount of space to allocate (16*sizeof(uint)) to protect
>drivers that do not (yet) check the size of the instruction data (Ian has
>submitted several patches fixing this for other drivers already). In case
>insn.n does not get set, this minimum amount also avoids trying to allocate
>zero-length data.
> 2) Allocate the maximum required space needed for any of the instructions in
> an
>instruction list before executing the list of instructions (this, rather
> than
>allocating and freeing memory separately while iterating through the
>instruction list and executing each instruction).
>
> drivers/staging/comedi/comedi_fops.c | 48 ++--
> 1 file changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c
> b/drivers/staging/comedi/comedi_fops.c
> index ceb6ba5dd57c..5d2fcbfe02af 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -1501,25 +1501,21 @@ static int parse_insn(struct comedi_device *dev,
> struct comedi_insn *insn,
> * data (for reads) to insns[].data pointers
> */
> /* arbitrary limits */
> -#define MAX_SAMPLES 256
> +#define MIN_SAMPLES 16
> +#define MAX_SAMPLES 65536
> static int do_insnlist_ioctl(struct comedi_device *dev,
> struct comedi_insnlist __user *arg, void *file)
> {
> struct comedi_insnlist insnlist;
> struct comedi_insn *insns = NULL;
> unsigned int *data = NULL;
> + unsigned int max_n_data_required = MIN_SAMPLES;
> int i = 0;
> int ret = 0;
>
> if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
> return -EFAULT;
>
> - data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
> - if (!data) {
> - ret = -ENOMEM;
> - goto error;
> - }
> -
> insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
> if (!insns) {
> ret = -ENOMEM;
> @@ -1533,13 +1529,26 @@ static int do_insnlist_ioctl(struct comedi_device
> *dev,
> goto error;
> }
>
> - for (i = 0; i < insnlist.n_insns; i++) {
> + /* Determine maximum memory needed for all instructions. */
> + for (i = 0; i < insnlist.n_insns; ++i) {
> if (insns[i].n > MAX_SAMPLES) {
> dev_dbg(dev->class_dev,
> "number of samples too large\n");
> ret = -EINVAL;
> goto error;
> }
> + max_n_data_required = max(max_n_data_required, insns[i].n);
> + }
I realized just now that this patch does change behavior just bit:
the complaint, error, and exit are done _before_ any instruction is
executed, rather than after the prior instructions in the list are
executed. I argue this is actually a better behavior than partially
executing an instruction list, especially since this pre-inspection
could have already easily been done before.
> +
> + /* Allocate scratch space for all instruction data. */
> + data = kmalloc_array(max_n_data_required, sizeof(unsigned int),
> +GFP_KERNEL);
> + if (!data) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + for (i = 0; i < insnlist.n_insns; ++i) {
> if (insns[i].insn & INSN_MASK_WRITE) {
> if (copy_from_user(data, insns[i].data,
>insns[i].n * sizeof(unsigned
> int))) {
> @@ -1593,22 +1602,27 @@ static int do_insn_ioctl(struct comedi_device *dev,
> {
> struct comedi_insn insn;
> unsigned int *data = NULL;
> + unsigned int n_data = MIN_SAMPLES;
> int ret = 0;
>
> - data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
> - if (!data) {
> -
[PATCH 0/2] staging: greybus: Fix parameters alignment and strings concatenation
First patch align some parameters with parenthesis. Second patch will add some spaces between string. Cristian Sicilia (2): staging: greybus: Align function call parameters to parenthesis staging: greybus: Added space between string concatenated drivers/staging/greybus/loopback.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) -- 2.7.4 signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: greybus: Added space between string concatenated
Some concatenated strings are now spaced.
Signed-off-by: Cristian Sicilia
---
drivers/staging/greybus/loopback.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c
b/drivers/staging/greybus/loopback.c
index 1085e06..acfa392 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -141,7 +141,7 @@ static ssize_t name##_##field##_show(struct device *dev,
\
/* Report 0 for min and max if no transfer successed */ \
if (!gb->requests_completed)\
return sprintf(buf, "0\n"); \
- return sprintf(buf, "%"#type"\n", gb->name.field); \
+ return sprintf(buf, "%" #type "\n", gb->name.field);\
} \
static DEVICE_ATTR_RO(name##_##field)
@@ -176,7 +176,7 @@ static ssize_t field##_show(struct device *dev,
\
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
@@ -212,7 +212,7 @@ static ssize_t field##_show(struct device *dev,
\
char *buf) \
{ \
struct gb_loopback *gb = dev_get_drvdata(dev); \
- return sprintf(buf, "%"#type"\n", gb->field); \
+ return sprintf(buf, "%" #type "\n", gb->field); \
} \
static ssize_t field##_store(struct device *dev, \
struct device_attribute *attr, \
--
2.7.4
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: greybus: Align function call parameters to parenthesis
Aligned some parameters to the previous parenthesis.
Signed-off-by: Cristian Sicilia
---
drivers/staging/greybus/loopback.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/loopback.c
b/drivers/staging/greybus/loopback.c
index e4d42c1..1085e06 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -638,7 +638,8 @@ static int gb_loopback_async_transfer(struct gb_loopback
*gb, u32 len)
retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_TRANSFER,
request, len + sizeof(*request),
len + response_len,
-
gb_loopback_async_transfer_complete);
+gb_loopback_async_transfer_complete
+);
if (retval)
goto gb_error;
@@ -682,7 +683,8 @@ static int gb_loopback_request_handler(struct gb_operation
*operation)
}
if (!gb_operation_response_alloc(operation,
- len + sizeof(*response), GFP_KERNEL)) {
+len + sizeof(*response),
+GFP_KERNEL)) {
dev_err(dev, "error allocating response\n");
return -ENOMEM;
}
@@ -882,7 +884,7 @@ static int gb_loopback_fn(void *data)
gb->type = 0;
gb->send_count = 0;
sysfs_notify(&gb->dev->kobj, NULL,
- "iteration_count");
+"iteration_count");
dev_dbg(&bundle->dev, "load test complete\n");
} else {
dev_dbg(&bundle->dev,
@@ -1066,7 +1068,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
/* Allocate kfifo */
if (kfifo_alloc(&gb->kfifo_lat, kfifo_depth * sizeof(u32),
- GFP_KERNEL)) {
+ GFP_KERNEL)) {
retval = -ENOMEM;
goto out_conn;
}
--
2.7.4
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 09/15] dt-bindings: sram: sunxi: Add compatible for the A64 SRAM C1
On Thu, 15 Nov 2018 15:50:07 +0100, Paul Kocialkowski wrote: > This introduces a new compatible for the A64 SRAM C1 section, that is > compatible with the SRAM C1 section as found on the A10. > > Signed-off-by: Paul Kocialkowski > --- > Documentation/devicetree/bindings/sram/sunxi-sram.txt | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Rob Herring ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/15] dt-bindings: sram: sunxi: Add bindings for the H5 with SRAM C1
On Thu, 15 Nov 2018 15:50:03 +0100, Paul Kocialkowski wrote: > This introduces new bindings for the H5 SoC in the SRAM controller. > Because the SRAM layout is different from other SoCs, no backward > compatibility is assumed with any of them. > > However, the C1 SRAM section alone looks similar to previous SoCs, > so it is compatible with the initial A10 binding. > > Signed-off-by: Paul Kocialkowski > --- > Documentation/devicetree/bindings/sram/sunxi-sram.txt | 4 > 1 file changed, 4 insertions(+) > Reviewed-by: Rob Herring ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 11/15] dt-bindings: media: cedrus: Add compatibles for the A64 and H5
On Thu, 15 Nov 2018 15:50:09 +0100, Paul Kocialkowski wrote: > This introduces two new compatibles for the cedrus driver, for the > A64 and H5 platforms. > > Signed-off-by: Paul Kocialkowski > --- > Documentation/devicetree/bindings/media/cedrus.txt | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Rob Herring ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: octeon-usb: use a helper function to set the DMA mask
Use a helper function to set the DMA mask. Signed-off-by: Aaro Koskinen --- drivers/staging/octeon-usb/octeon-hcd.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index 9c766f5b812f..14982b6472a0 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #include @@ -3606,8 +3607,9 @@ static int octeon_usb_probe(struct platform_device *pdev) * Set the DMA mask to 64bits so we get buffers already translated for * DMA. */ - dev->coherent_dma_mask = ~0; - dev->dma_mask = &dev->coherent_dma_mask; + i = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); + if (i) + return i; /* * Only cn52XX and cn56XX have DWC_OTG USB hardware and the -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: mt7621-spi: drop the broken full-duplex mode
On Tue, Dec 04 2018, Chuanhong Guo wrote:
> Hi!
> NeilBrown 于2018年12月4日周二 上午5:55写道：
>>
>> On Mon, Dec 03 2018, Chuanhong Guo wrote:
>>
>> > Under MORE_BUF_MODE the controller will always shift one bit out of
>> > spi_opcode if (mosi_bit_cnt > 0) && (cmd_bit_cnt == 0) so the full-
>> > duplex mode is broken since we can't read anything from MISO during
>> > writing spi_opcode.
>> > This piece of code also make CS1 unavailable since it forces the
>> > broken full-duplex mode to be enabled on CS1.
>>
>> Hi,
>> How do you know these details of the hardware? Do you have detailed
>> specs for the hardware or have you experimented with it or are you one
>> of the designers or ?
>> It would be nice to have the source of this information documented.
> Those info are provided by John Crispin on IRC #openwrt-devel. Here is
> the quoted reply:
> ---
> Nov 26 17:00:15 gch981213[w]: so basically i made cs1 work
> for MTK/labs when i built the linkit smart for them. the req-sheet
> said that cs1 should be proper duplex spi. however
> Nov 26 17:00:34 1) the core will always send 1 byte before
> any transfer, this is the m25p80 command.
> Nov 26 17:00:49 2) mode 3 is broken and bit reversed (?)
> Nov 26 17:01:01 3) some bit are incorrectly wired in hw for mode2/3
> Nov 26 17:01:50 we wrote a test script and test for
> [0-0x] on all modes and certain bits are swizzled under certain
> conditions and it was not possible to fix this even using a hack.
> Nov 26 17:02:06 we then decided to use spi-gpio and i never
> removed the errornous code
> Nov 26 17:02:38 basically the spi is fecked for anything but
> half duplex spi mode0 running a sflash on it
I think this is useful information that is worth preserving in the
commit message. I would strip the time stamps, and make it something
like this:
According to John Crispin (aka blogic) on IRC on Nov 26 2018:
so basically i made cs1 work for MTK/labs when i built
the linkit smart for them. the req-sheet said that cs1 should be proper
duplex spi. however
1) the core will always send 1 byte before any transfer, this is the
m25p80 command.
2) mode 3 is broken and bit reversed (?)
3) some bit are incorrectly wired in hw for mode2/3
we wrote a test script and test for [0-0x] on all modes and certain
bits are swizzled under certain conditions and it was not possible to
fix this even using a hack.
we then decided to use spi-gpio and i never removed the errornous code
basically the spi is fecked for anything but half duplex spi mode0
running a sflash on it
If you add that to the commit message, I would be quite happy to Ack it.
Thanks,
NeilBrown
> ---
> And another guy told me that his experiment shows there will be one
> extra bit if we set cmd_bit_cnt to 0 and mosi_bit_cnt > 0.
> I guess it doesn't matter whether it's an extra "byte" or "bit" since
> there are always some data that have to be sent under half-duplex.
>
> Do I need to add "some experiment shows that" at the beginning of my
> commit message?
>>
>> Thanks,
>> NeilBrown
>>
>>
>> >
>> > Signed-off-by: Chuanhong Guo
>> > ---
>> > drivers/staging/mt7621-spi/spi-mt7621.c | 120 +++-
>> > 1 file changed, 15 insertions(+), 105 deletions(-)
>> >
>> > diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
>> > b/drivers/staging/mt7621-spi/spi-mt7621.c
>> > index d045b5568e0f..8af6f307e176 100644
>> > --- a/drivers/staging/mt7621-spi/spi-mt7621.c
>> > +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
>> > @@ -86,16 +86,13 @@ static inline void mt7621_spi_write(struct mt7621_spi
>> > *rs, u32 reg, u32 val)
>> > iowrite32(val, rs->base + reg);
>> > }
>> >
>> > -static void mt7621_spi_reset(struct mt7621_spi *rs, int duplex)
>> > +static void mt7621_spi_reset(struct mt7621_spi *rs)
>> > {
>> > u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
>> >
>> > master |= 7 << 29;
>> > master |= 1 << 2;
>> > - if (duplex)
>> > - master |= 1 << 10;
>> > - else
>> > - master &= ~(1 << 10);
>> > + master &= ~(1 << 10);
>> >
>> > mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
>> > rs->pending_write = 0;
>> > @@ -107,7 +104,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi,
>> > int enable)
>> > int cs = spi->chip_select;
>> > u32 polar = 0;
>> >
>> > - mt7621_spi_reset(rs, cs);
>> > + mt7621_spi_reset(rs);
>> > if (enable)
>> > polar = BIT(cs);
>> > mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
>> > @@ -261,7 +258,7 @@ static void mt7621_spi_write_half_duplex(struct
>> > mt7621_spi *rs,
>> > rs->pending_write = len;
>> > }
>> >
>> > -static int mt7621_spi_transfer_half_duplex(struct spi_master *master,
>> > +static int mt7621_spi_transfer_one_message(struct spi_master *master,
>> > struct spi_message *m)
>> > {
>> > struct mt7621_spi *rs = spi_master_get_
[PATCH] Revert "Staging: iio: adt7316: Add an extra check for 'ret' equals to 0"
This reverts commit 00426e99789357dbff7e719a092ce36a3ce49d94.
i2c_smbus_read_byte() returns 0 when a byte with the value 0 is read from
the device. This is a valid read so revert the check for 0.
Signed-off-by: Jeremy Fertic
---
drivers/staging/iio/addac/adt7316-i2c.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316-i2c.c
b/drivers/staging/iio/addac/adt7316-i2c.c
index ac91163656b5..2d51bd425662 100644
--- a/drivers/staging/iio/addac/adt7316-i2c.c
+++ b/drivers/staging/iio/addac/adt7316-i2c.c
@@ -30,10 +30,6 @@ static int adt7316_i2c_read(void *client, u8 reg, u8 *data)
}
ret = i2c_smbus_read_byte(client);
-
- if (!ret)
- return -EIO;
-
if (ret < 0) {
dev_err(&cl->dev, "I2C read error\n");
return ret;
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] v2 Staging : Add RIFFA PCIe staging driver
> > Also, do not use a random MAJOR number that you just picked out of no > > where, that too will break working systems and needs to be fixed. Use > > the dynamic allocation method, or better yet, just use a misc device. > > What do you exactly mean by misc device ? > Look at include/linux/miscdevice.h for the details. I will study http://www.embeddedlinux.org.cn/essentiallinuxdevicedrivers/final/ch05lev1sec7.html on my own. I am still working on the hardware verilog side now. > I am actually a bit worried with circ_queue structure and its corresponding > push() and pull() functions which systemtap complains to have some > significant timing overhead, but to be frank, this is not the root cause of > why the full duplex version of the driver is slower. > Yes, that's kind of obvious by looking at that code :) > Again, fix up the driver to use the in-kernel data structures for this > type of thing and you will see a much better improvement in performance. To be frank, the 3x slowdown is actually due to "one extra wake_up() call" in the full-duplex version of riffa_driver.c line 685 if I am not wrong. https://github.com/promach/riffa/blob/full_duplex/driver/linux/riffa_driver.c#L685 If I use https://www.kernel.org/doc/html/v4.19/core-api/circular-buffers.html#the-producer instead of https://github.com/promach/riffa/blob/full_duplex/driver/linux/circ_queue.c , I actually worry that the in-kernel data structure with memory barrier can actually have more timing delay or overhead than atomic functions used in circ_queue.c Please advise regarding wake_up(consumer). ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
