[PATCH] Staging : comedi : comedidev.h Fixed warning space coding style issue
Fixed a coding style issue,removed space after the function pointer name
Signed-off-by: Surendra Patil surendra@gmail.com
---
drivers/staging/comedi/comedidev.h | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index f82bd42..588e9e7 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -61,29 +61,29 @@ struct comedi_subdevice {
unsigned int *chanlist; /* driver-owned chanlist (not used) */
- int (*insn_read) (struct comedi_device *, struct comedi_subdevice *,
+ int (*insn_read)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *);
- int (*insn_write) (struct comedi_device *, struct comedi_subdevice *,
+ int (*insn_write)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *);
- int (*insn_bits) (struct comedi_device *, struct comedi_subdevice *,
+ int (*insn_bits)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *);
- int (*insn_config) (struct comedi_device *, struct comedi_subdevice *,
+ int (*insn_config)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *);
- int (*do_cmd) (struct comedi_device *, struct comedi_subdevice *);
- int (*do_cmdtest) (struct comedi_device *, struct comedi_subdevice *,
+ int (*do_cmd)(struct comedi_device *, struct comedi_subdevice *);
+ int (*do_cmdtest)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_cmd *);
- int (*poll) (struct comedi_device *, struct comedi_subdevice *);
- int (*cancel) (struct comedi_device *, struct comedi_subdevice *);
+ int (*poll)(struct comedi_device *, struct comedi_subdevice *);
+ int (*cancel)(struct comedi_device *, struct comedi_subdevice *);
/* int (*do_lock)(struct comedi_device *, struct comedi_subdevice *); */
/* int (*do_unlock)(struct comedi_device *, \
struct comedi_subdevice *); */
/* called when the buffer changes */
- int (*buf_change) (struct comedi_device *dev,
+ int (*buf_change)(struct comedi_device *dev,
struct comedi_subdevice *s, unsigned long new_size);
- void (*munge) (struct comedi_device *dev, struct comedi_subdevice *s,
+ void (*munge)(struct comedi_device *dev, struct comedi_subdevice *s,
void *data, unsigned int num_bytes,
unsigned int start_chan_index);
enum dma_data_direction async_dma_dir;
@@ -146,7 +146,7 @@ struct comedi_async {
unsigned int cb_mask;
- int (*inttrig) (struct comedi_device *dev, struct comedi_subdevice *s,
+ int (*inttrig)(struct comedi_device *dev, struct comedi_subdevice *s,
unsigned int x);
};
@@ -155,9 +155,9 @@ struct comedi_driver {
const char *driver_name;
struct module *module;
- int (*attach) (struct comedi_device *, struct comedi_devconfig *);
- void (*detach) (struct comedi_device *);
- int (*auto_attach) (struct comedi_device *, unsigned long);
+ int (*attach)(struct comedi_device *, struct comedi_devconfig *);
+ void (*detach)(struct comedi_device *);
+ int (*auto_attach)(struct comedi_device *, unsigned long);
/* number of elements in board_name and board_id arrays */
unsigned int num_names;
@@ -202,8 +202,8 @@ struct comedi_device {
struct fasync_struct *async_queue;
- int (*open) (struct comedi_device *dev);
- void (*close) (struct comedi_device *dev);
+ int (*open)(struct comedi_device *dev);
+ void (*close)(struct comedi_device *dev);
};
static inline const void *comedi_board(const struct comedi_device *dev)
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: fix GFP_ATOMIC macro usage
Hello,
On 2014-01-17 15:33, Greg Kroah-Hartman wrote:
On Fri, Jan 17, 2014 at 09:46:56AM +0100, Marek Szyprowski wrote:
GFP_ATOMIC is not a single gfp flag, but a macro which expands to the other
flags and LACK of __GFP_WAIT flag. To check if caller wanted to perform an
atomic allocation, the code must test __GFP_WAIT flag presence.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
.../lustre/include/linux/libcfs/libcfs_private.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
index d0d942c..dddccca1 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
@@ -120,7 +120,7 @@ do { \
do {
\
LASSERT(!in_interrupt() || \
((size) = LIBCFS_VMALLOC_SIZE \
- ((mask) GFP_ATOMIC)) != 0); \
+ ((mask) __GFP_WAIT) == 0)); \
} while (0)
What a horrible assert, can't we just remove this entirely?
in_interrupt() usually should never be checked, if so, the code is doing
something wrong. And __GFP flags shouldn't be used on their own.
Well, I've prepared this patch when I was checking kernel sources for
incorrect
GFP_ATOMIC usage. I agree that drivers should use generic memory allocation
methods instead of inventing their own stuff. Feel free to ignore my
patch in
favor of removing this custom allocator at all.
Best regards
--
Marek Szyprowski, PhD
Samsung RD Institute Poland
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] mfd: Add realtek USB card reader driver
On 01/16/2014 05:35 PM, Lee Jones wrote:
+static int rtsx_usb_bulk_transfer_sglist(struct rtsx_ucr *ucr,
+ unsigned int pipe, struct scatterlist *sg, int num_sg,
+ unsigned int length, unsigned int *act_len, int timeout)
+{
+ int ret;
+
+ dev_dbg(ucr-pusb_intf-dev, %s: xfer %u bytes, %d entries\n,
+ __func__, length, num_sg);
+ ret = usb_sg_init(ucr-current_sg, ucr-pusb_dev, pipe, 0,
+ sg, num_sg, length, GFP_NOIO);
+ if (ret)
+ return ret;
+
+ ucr-sg_timer.expires = jiffies + msecs_to_jiffies(timeout);
+ add_timer(ucr-sg_timer);
+ usb_sg_wait(ucr-current_sg);
+ del_timer(ucr-sg_timer);
+
+ if (act_len)
+ *act_len = ucr-current_sg.bytes;
+
+ return ucr-current_sg.status;
+}
Code looks fine, but shouldn't this live in a USB driver?
We have studied drivers in usb/storage, the place that most likely
to put the driver. All of them are for STANDARD usb mass storage
class(something like USB_CLASS_MASS_STORAGE). But this is not the
same case. This driver is for our vendor-class device with
completely different protocol. It is really an USB interfaced flash
card command converter(or channel) device rather than a real
storage. It also has two derived modules(rtsx_usb_sdmmc,
rtsx_usb_memstick) for other two subsystems.
We also have another driver: rtsx_pcr.c resident in mfd/ for similar
devices but of PCIe interface. It is nature for us to choose the
same place for this variant.
Very well, as long as it truly does not fit in drivers/usb. It would
be good to have one of the USB folk give the nod though.
I found Greg K-H is exactly one of the maintainers of USB subsystem as I
read the MAINTAINERS document.
Greg, would you please comment this subsystem concern or introduce
other members? Thanks.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging : comedi : comedidev.h Fixed warning space coding style issue
On Mon, Jan 20, 2014 at 12:19:09AM -0800, Surendra Patil wrote:
Fixed a coding style issue,removed space after the function pointer name
Signed-off-by: Surendra Patil surendra@gmail.com
---
drivers/staging/comedi/comedidev.h | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index f82bd42..588e9e7 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -61,29 +61,29 @@ struct comedi_subdevice {
unsigned int *chanlist; /* driver-owned chanlist (not used) */
- int (*insn_read) (struct comedi_device *, struct comedi_subdevice *,
+ int (*insn_read)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *);
Now the the second line has one extra space. The 's' characters in
struct comedi_device * and struct comedi_insn * should be on the
same column.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch] staging: android: ion: dummy: fix an error code
We should be returning -ENOMEM here instead of zero. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/staging/android/ion/ion_dummy_driver.c b/drivers/staging/android/ion/ion_dummy_driver.c index 55b2002753f2..f0d786c539ea 100644 --- a/drivers/staging/android/ion/ion_dummy_driver.c +++ b/drivers/staging/android/ion/ion_dummy_driver.c @@ -69,7 +69,7 @@ static int __init ion_dummy_init(void) heaps = kzalloc(sizeof(struct ion_heap *) * dummy_ion_pdata.nr, GFP_KERNEL); if (!heaps) - return PTR_ERR(heaps); + return -ENOMEM; /* Allocate a dummy carveout heap */ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
Hi Chen,
On 19/01/14 10:07, Chen Gang wrote:
BTW: this patch is related with another patch which is discussing (so I
have cc that patch to you and Greg too): if we could sure that it is a
compiler's feature issue, we will skip this patch.
If you're referring to the #pragma pack portability issue then this
issue is unrelated since it doesn't use #pragma pack.
The issue is *not* that the compiler is defectively failing to pack
nested structs. Doing that would be utterly broken since it would change
the layout of the same struct depending on where it is placed in the
program, Consider this example (which uses a nested struct rather than
union to demonstrate the point):
struct a {
struct b {
unsigned int x;
unsigned short y;
} x;
unsigned short y;
} __packed;
Both ABIs behave the same here:
Archsizeof(struct b)sizeof(struct a)
x86_64 8 10
metag 8 10
If struct b is made __packed, again both ABIs behave differently in the
same way:
Archsizeof(struct b)sizeof(struct a)
x86_64 6 8
metag 6 8
The issue is that C compiler ABIs may (and unfortunately metag ABI does)
pack structs and unions to at least 4 bytes, even if no members of the
struct or union are that large, which means that the nested struct/union
should be __packed too to portably ensure it's the expected size.
Cheers
James
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
Ah. From so metag is a new arch and not a compiler like the changelog
says.
On Mon, Jan 20, 2014 at 11:56:47AM +, James Hogan wrote:
struct a {
struct b {
unsigned int x;
unsigned short y;
} x;
unsigned short y;
} __packed;
This is not the code we are discussing. It should look like:
struct a {
union {
short x;
short y;
}
short z;
};
Any normal person would assume that sizeof(struct a) would be 4 but
apparently on metag it is 8. That totally defeats the point of using
a union in the first place. It's easy enough to add a __packed to the
lustre declaration but I expect this to cause an endless stream of bugs.
It it is really stupid.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
On 20/01/14 12:30, Dan Carpenter wrote:
Ah. From so metag is a new arch and not a compiler like the changelog
says.
On Mon, Jan 20, 2014 at 11:56:47AM +, James Hogan wrote:
struct a {
struct b {
unsigned int x;
unsigned short y;
} x;
unsigned short y;
} __packed;
This is not the code we are discussing. It should look like:
struct a {
union {
short x;
short y;
}
short z;
};
Any normal person would assume that sizeof(struct a) would be 4 but
apparently on metag it is 8. That totally defeats the point of using
a union in the first place. It's easy enough to add a __packed to the
lustre declaration but I expect this to cause an endless stream of bugs.
It it is really stupid.
I agree completely (and did request this be changed when I first found
out about it, but since it's an ABI issue it was really too late).
That's why I'm not actively pushing for every case to be fixed unless
it's in generic code that actually affects metag.
Cheers
James
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
On Mon, Jan 20, 2014 at 12:37:57PM +, James Hogan wrote:
On 20/01/14 12:30, Dan Carpenter wrote:
Ah. From so metag is a new arch and not a compiler like the changelog
says.
On Mon, Jan 20, 2014 at 11:56:47AM +, James Hogan wrote:
struct a {
struct b {
unsigned int x;
unsigned short y;
} x;
unsigned short y;
} __packed;
This is not the code we are discussing. It should look like:
struct a {
union {
short x;
short y;
}
short z;
};
Any normal person would assume that sizeof(struct a) would be 4 but
apparently on metag it is 8. That totally defeats the point of using
a union in the first place. It's easy enough to add a __packed to the
lustre declaration but I expect this to cause an endless stream of bugs.
It it is really stupid.
I agree completely (and did request this be changed when I first found
out about it, but since it's an ABI issue it was really too late).
That's why I'm not actively pushing for every case to be fixed unless
it's in generic code that actually affects metag.
It would be easy enough to make the compiler complain about any union
which would normally have size which is not a multiple of 4.
Warning: union will be padded with 2 bytes unless __attribute__((packed)).
Otherwise you will be fighting this for ever.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
Are you sure it's padding the unions, and not just treating the unions
as structs? What is the size of this union?
union a {
int x;
short y;
};
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/1] staging: fpgaboot: Xilinx FPGA firmware download driver
On Thu, Jan 16, 2014 at 1:41 PM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Jan 16, 2014 at 11:47:41AM -0800, Insop Song wrote: There is no way to detect FPGA until it is programmed. This is a reason and the only reason of this driver to download the program to the FPGA so that it can function. So how do you get the memory locations of where the FPGA is in the system in order to be able to send data to it? Surely that's in the device tree file somewhere, right? On the FPGA side, there are dedicated pins for programming, and through these you cannot get meaningful information (again unless you are JTAG capable) Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE. On a process side, we use gpio pin to connect to the above pins. It's GPIO pins that we do the bit banging as defined for programming guide from Xilinx. Yes, but where do you learn about how those pins are hooked up to the CPU so that the driver can control them? This is hard coded. Anyway, I have no objection to your driver, it does good things, and we want it, I think we are talking past each other at the moment, sorry. How about you repost the code, just using a platform device for the moment, and we can take it from there. After all, code in the staging tree is meant to be cleaned up :) Thank you for your suggestions and feedback. I am sure I will have to clean up more :) Will clean up and send a d new patch shortly. Thank you, ISS ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/1] staging: fpgaboot: Xilinx FPGA firmware download driver
On Mon, Jan 20, 2014 at 09:16:08AM -0800, Insop Song wrote: On Thu, Jan 16, 2014 at 1:41 PM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Jan 16, 2014 at 11:47:41AM -0800, Insop Song wrote: There is no way to detect FPGA until it is programmed. This is a reason and the only reason of this driver to download the program to the FPGA so that it can function. So how do you get the memory locations of where the FPGA is in the system in order to be able to send data to it? Surely that's in the device tree file somewhere, right? On the FPGA side, there are dedicated pins for programming, and through these you cannot get meaningful information (again unless you are JTAG capable) Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE. On a process side, we use gpio pin to connect to the above pins. It's GPIO pins that we do the bit banging as defined for programming guide from Xilinx. Yes, but where do you learn about how those pins are hooked up to the CPU so that the driver can control them? This is hard coded. Really? Shouldn't they be in a board file or device tree attribute somewhere? What happens in the next board that is created with this chip and the memory locations are in a different place? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4 1/1] staging: fpgaboot: Xilinx FPGA firmware download driver
On Monday, January 20, 2014 11:06 AM, Greg KH wrote: On Mon, Jan 20, 2014 at 09:16:08AM -0800, Insop Song wrote: On Thu, Jan 16, 2014 at 1:41 PM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Jan 16, 2014 at 11:47:41AM -0800, Insop Song wrote: There is no way to detect FPGA until it is programmed. This is a reason and the only reason of this driver to download the program to the FPGA so that it can function. So how do you get the memory locations of where the FPGA is in the system in order to be able to send data to it? Surely that's in the device tree file somewhere, right? On the FPGA side, there are dedicated pins for programming, and through these you cannot get meaningful information (again unless you are JTAG capable) Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE. On a process side, we use gpio pin to connect to the above pins. It's GPIO pins that we do the bit banging as defined for programming guide from Xilinx. Yes, but where do you learn about how those pins are hooked up to the CPU so that the driver can control them? This is hard coded. Really? Shouldn't they be in a board file or device tree attribute somewhere? What happens in the next board that is created with this chip and the memory locations are in a different place? I have not looked at this driver but... Couldn't this be done from user space using urjtag? http://urjtag.org/ Regards, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
I made a quick and dirty sparse patch to check for this. I don't think
I will bother trying to send it to sparse upstream, but you can if you
want to.
It found 289 unions which might need a __packed added. The lustre
unions were not in my allmodconfig so they're not listed.
Perhaps there could be a command line option or a pragma so that unions
will work in the kernel. We don't care about linking to outside
libraries.
regards,
dan carpenter
diff --git a/symbol.c b/symbol.c
index ebba56deaf94..596e47883aad 100644
--- a/symbol.c
+++ b/symbol.c
@@ -187,6 +187,12 @@ static struct symbol * examine_struct_union_type(struct
symbol *sym, int advance
bit_size = (bit_size + bit_align) ~bit_align;
}
sym-bit_size = bit_size;
+
+ if (!advance (info.bit_size / 8) % 4) {
+ int pad = 4 - (info.bit_size / 8) % 4;
+ warning(sym-pos, '%s' union will be padded with %d bytes
unless __attribute__((packed))., sym-ident ? sym-ident-name: null,
pad);
+ }
+
return sym;
}
drivers/acpi/sbshc.c:55:7: warning: 'acpi_smb_status' union will be padded with
3 bytes unless __attribute__((packed)).
drivers/atm/horizon.h:317:9: warning: 'null' union will be padded with 3
bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2173:15: warning: 'DAC960_V1_StatusMailbox' union will
be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2580:15: warning: 'DAC960_GEM_OutboundDoorBellRegister'
union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2618:15: warning: 'DAC960_GEM_ErrorStatusRegister' union
will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2867:15: warning: 'DAC960_BA_InboundDoorBellRegister'
union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2891:15: warning: 'DAC960_BA_OutboundDoorBellRegister'
union will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2912:15: warning: 'DAC960_BA_InterruptMaskRegister'
union will be padded with 1 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2929:15: warning: 'DAC960_BA_ErrorStatusRegister' union
will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3173:15: warning: 'DAC960_LP_InboundDoorBellRegister'
union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3197:15: warning: 'DAC960_LP_OutboundDoorBellRegister'
union will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3218:15: warning: 'DAC960_LP_InterruptMaskRegister'
union will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3234:15: warning: 'DAC960_LP_ErrorStatusRegister' union
will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3487:15: warning: 'DAC960_LA_InboundDoorBellRegister'
union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3511:15: warning: 'DAC960_LA_OutboundDoorBellRegister'
union will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3532:15: warning: 'DAC960_LA_InterruptMaskRegister'
union will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3548:15: warning: 'DAC960_LA_ErrorStatusRegister' union
will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:3869:15: warning: 'DAC960_PG_ErrorStatusRegister' union
will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:4155:15: warning: 'DAC960_PD_OutboundDoorBellRegister'
union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:4174:15: warning: 'DAC960_PD_InterruptEnableRegister'
union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:4189:15: warning: 'DAC960_PD_ErrorStatusRegister' union
will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/mtip32xx/mtip32xx.h:221:15: warning: 'null' union will be
padded with 3 bytes unless __attribute__((packed)).
drivers/block/mtip32xx/mtip32xx.h:225:15: warning: 'null' union will be
padded with 3 bytes unless __attribute__((packed)).
drivers/block/mtip32xx/mtip32xx.h:229:15: warning: 'null' union will be
padded with 3 bytes unless __attribute__((packed)).
drivers/block/mtip32xx/mtip32xx.h:233:15: warning: 'null' union will be
padded with 3 bytes unless __attribute__((packed)).
drivers/block/mtip32xx/mtip32xx.h:238:15: warning: 'null' union will be
padded with 3 bytes unless __attribute__((packed)).
drivers/block/mtip32xx/mtip32xx.h:242:15: warning: 'null' union will be
padded with 3 bytes unless __attribute__((packed)).
drivers/block/mtip32xx/mtip32xx.h:246:15: warning: 'null' union will be
padded with 3 bytes unless __attribute__((packed)).
drivers/block/paride/paride.h:120:15: warning: 'null' union will be padded
with 2 bytes unless __attribute__((packed)).
Re: [PATCH 1/3] mfd: Add realtek USB card reader driver
On Mon, Jan 20, 2014 at 04:55:52PM +0800, Roger wrote:
On 01/16/2014 05:35 PM, Lee Jones wrote:
+static int rtsx_usb_bulk_transfer_sglist(struct rtsx_ucr *ucr,
+ unsigned int pipe, struct scatterlist *sg, int num_sg,
+ unsigned int length, unsigned int *act_len, int timeout)
+{
+ int ret;
+
+ dev_dbg(ucr-pusb_intf-dev, %s: xfer %u bytes, %d entries\n,
+ __func__, length, num_sg);
+ ret = usb_sg_init(ucr-current_sg, ucr-pusb_dev, pipe, 0,
+ sg, num_sg, length, GFP_NOIO);
+ if (ret)
+ return ret;
+
+ ucr-sg_timer.expires = jiffies + msecs_to_jiffies(timeout);
+ add_timer(ucr-sg_timer);
+ usb_sg_wait(ucr-current_sg);
+ del_timer(ucr-sg_timer);
+
+ if (act_len)
+ *act_len = ucr-current_sg.bytes;
+
+ return ucr-current_sg.status;
+}
Code looks fine, but shouldn't this live in a USB driver?
We have studied drivers in usb/storage, the place that most likely
to put the driver. All of them are for STANDARD usb mass storage
class(something like USB_CLASS_MASS_STORAGE). But this is not the
same case. This driver is for our vendor-class device with
completely different protocol. It is really an USB interfaced flash
card command converter(or channel) device rather than a real
storage. It also has two derived modules(rtsx_usb_sdmmc,
rtsx_usb_memstick) for other two subsystems.
We also have another driver: rtsx_pcr.c resident in mfd/ for similar
devices but of PCIe interface. It is nature for us to choose the
same place for this variant.
Very well, as long as it truly does not fit in drivers/usb. It would
be good to have one of the USB folk give the nod though.
I found Greg K-H is exactly one of the maintainers of USB subsystem as I
read the MAINTAINERS document.
Greg, would you please comment this subsystem concern or introduce other
members? Thanks.
It's the middle of the merge window, nothing can happen until after
3.14-rc1 is out. So how about resending all of this in two weeks after
that happens so we can all properly discuss it?
As for where the driver should be located, if it shares logic with the
usb storage driver, then it should be in drivers/usb/storage, otherwise
put it wherever makes most sense, there's no need to put all USB drivers
under drivers/usb/ at all.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
-Original Message- From: Haiyang Zhang Sent: Monday, January 20, 2014 2:06 PM To: David Miller Cc: net...@vger.kernel.org; KY Srinivasan; o...@aepfle.de; jasow...@redhat.com; linux-ker...@vger.kernel.org; driverdev- de...@linuxdriverproject.org Subject: RE: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer -Original Message- From: David Miller [mailto:da...@davemloft.net] Sent: Tuesday, January 14, 2014 5:32 PM To: Haiyang Zhang Cc: net...@vger.kernel.org; KY Srinivasan; o...@aepfle.de; jasow...@redhat.com; linux-ker...@vger.kernel.org; driverdev- de...@linuxdriverproject.org Subject: Re: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer From: Haiyang Zhang haiya...@microsoft.com Date: Thu, 9 Jan 2014 14:24:47 -0800 This will allow us to use bigger receive buffer, and prevent allocation failure due to fragmented memory. Signed-off-by: Haiyang Zhang haiya...@microsoft.com Reviewed-by: K. Y. Srinivasan k...@microsoft.com Not until you start using paged SKBs in netvsc_recv_callback. Whatever fragmention you think you're avoiding in the hyperv layer, you're still going to get from the: skb = netdev_alloc_skb_ip_align(net, packet-total_data_buflen); call there. This change makes no sense in isolation, therefore I'm not applying it until you also include the appropriate changes to avoid the same exact fragmentation issue in netvsc_drv.c as stated above. The receive buffer currently requires multiple MB of physically continuous memory, and may fail to be allocated when memory is fragmented. The patch is created for this issue. The SKB buffer is usually less than 1500 bytes or up to several KB with jumbo frame, so it's much less sensitive to fragmented memory. I will work on another patch to use SKB buffer with discontinuous pages. Could you accept this patch separately? Today, if we try to unload and load the network driver, the load may fail because we may not be able to allocate the receive buffers if memory is fragmented. This patch specifically addresses this problem. Regards, K. Y Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
Olaf,
I am cleaning up the code based on your feedback. By the time I am done with my
cleanup, I doubt if
this patch would apply. Do you mind if I were to include your changes here as
part of my cleanup?
Thank you,
K. Y
-Original Message-
From: Olaf Hering [mailto:o...@aepfle.de]
Sent: Thursday, January 16, 2014 2:49 AM
To: KY Srinivasan
Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com
Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
On Tue, Jan 14, K. Y. Srinivasan wrote:
Implement the file copy service for Linux guests on Hyper-V. This permits
the
host to copy a file (over VMBUS) into the guest. This facility is part of
guest integration services supported on the Windows platform.
Here is a link that provides additional details on this functionality:
The change below fixes some warnings in the daemon code.
Compile tested only.
I also think the newlines in some of the syslog calls should be removed.
Olaf
hv_fcopy_daemon.c: In function 'hv_start_fcopy':
hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
*', but argument 3 has type '__u16 *' [-Wformat=]
smsg-file_name);
^
hv_fcopy_daemon.c:44:3: warning: format '%s' expects argument of type 'char
*', but argument 5 has type '__u16 *' [-Wformat=]
hv_fcopy_daemon.c:57:6: warning: format '%s' expects argument of type 'char
*', but argument 3 has type '__u16 *' [-Wformat=]
errno, strerror(errno));
^
hv_fcopy_daemon.c:61:4: warning: format '%s' expects argument of type 'char
*', but argument 3 has type '__u16 *' [-Wformat=]
syslog(LOG_ERR, Invalid path: %s\n, smsg-path_name);
^
hv_fcopy_daemon.c: In function 'main':
hv_fcopy_daemon.c:117:8: warning: ignoring return value of 'daemon', declared
with attribute warn_unused_result [-Wunused-result]
daemon(1, 0);
^
hv_fcopy_daemon.c:132:7: warning: ignoring return value of 'write', declared
with attribute warn_unused_result [-Wunused-result]
write(fcopy_fd, version, sizeof(int));
^
hv_fcopy_daemon.c:171:9: warning: ignoring return value of 'pwrite', declared
with attribute warn_unused_result [-Wunused-result]
pwrite(fcopy_fd, error, sizeof(int), 0);
^
Signed-off-by: Olaf Hering o...@aepfle.de
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index c0e5c90..d1fadb7 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -35,14 +35,14 @@
#include dirent.h
static int target_fd;
-char target_fname[W_MAX_PATH];
+static char target_fname[W_MAX_PATH];
static int hv_start_fcopy(struct hv_start_fcopy *smsg)
{
int error = HV_E_FAIL;
- sprintf(target_fname, %s%s%s, smsg-path_name, /,
- smsg-file_name);
+ snprintf(target_fname, sizeof(target_fname), %s/%s,
+ (char *)smsg-path_name, (char*)smsg-file_name);
syslog(LOG_INFO, Target file name: %s\n, target_fname);
/*
@@ -54,12 +54,12 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
if (mkdir((char *)smsg-path_name, 0755)) {
syslog(LOG_ERR,
Failed to create '%s'; error: %d %s\n,
- smsg-path_name,
+ (char *)smsg-path_name,
errno, strerror(errno));
goto done;
}
} else {
- syslog(LOG_ERR, Invalid path: %s\n, smsg-
path_name);
+ syslog(LOG_ERR, Invalid path: %s, (char *)smsg-
path_name);
goto done;
}
}
@@ -115,7 +115,8 @@ int main(void)
char *buffer[4096 * 2];
struct hv_fcopy_hdr *in_msg;
- daemon(1, 0);
+ if (daemon(1, 0))
+ return 1;
openlog(HV_FCOPY, 0, LOG_USER);
syslog(LOG_INFO, HV_FCOPY starting; pid is:%d, getpid());
@@ -130,7 +131,10 @@ int main(void)
/*
* Register with the kernel.
*/
- write(fcopy_fd, version, sizeof(int));
+ if (write(fcopy_fd, version, sizeof(int)) != sizeof(int)) {
+ syslog(LOG_ERR, write failed: %s,strerror(errno));
+ exit(EXIT_FAILURE);
+ }
while (1) {
/*
@@ -169,6 +173,9 @@ int main(void)
}
- pwrite(fcopy_fd, error, sizeof(int), 0);
+ if (pwrite(fcopy_fd, error, sizeof(int), 0) != sizeof(int)) {
+ syslog(LOG_ERR, pwrite failed: %s,strerror(errno));
+ exit(EXIT_FAILURE);
+ }
}
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
On Mon, Jan 20, KY Srinivasan wrote: I am cleaning up the code based on your feedback. By the time I am done with my cleanup, I doubt if this patch would apply. Do you mind if I were to include your changes here as part of my cleanup? Yes, thats fine. Olaf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH V2 1/1] Drivers: hv: Implement the file copy service
-Original Message- From: Olaf Hering [mailto:o...@aepfle.de] Sent: Monday, January 20, 2014 2:16 PM To: KY Srinivasan Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; a...@canonical.com; jasow...@redhat.com Subject: Re: [PATCH V2 1/1] Drivers: hv: Implement the file copy service On Mon, Jan 20, KY Srinivasan wrote: I am cleaning up the code based on your feedback. By the time I am done with my cleanup, I doubt if this patch would apply. Do you mind if I were to include your changes here as part of my cleanup? Yes, thats fine. Thanks Olaf. K. Y Olaf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
-Original Message- From: David Miller [mailto:da...@davemloft.net] Sent: Tuesday, January 14, 2014 5:32 PM To: Haiyang Zhang Cc: net...@vger.kernel.org; KY Srinivasan; o...@aepfle.de; jasow...@redhat.com; linux-ker...@vger.kernel.org; driverdev- de...@linuxdriverproject.org Subject: Re: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer From: Haiyang Zhang haiya...@microsoft.com Date: Thu, 9 Jan 2014 14:24:47 -0800 This will allow us to use bigger receive buffer, and prevent allocation failure due to fragmented memory. Signed-off-by: Haiyang Zhang haiya...@microsoft.com Reviewed-by: K. Y. Srinivasan k...@microsoft.com Not until you start using paged SKBs in netvsc_recv_callback. Whatever fragmention you think you're avoiding in the hyperv layer, you're still going to get from the: skb = netdev_alloc_skb_ip_align(net, packet-total_data_buflen); call there. This change makes no sense in isolation, therefore I'm not applying it until you also include the appropriate changes to avoid the same exact fragmentation issue in netvsc_drv.c as stated above. The receive buffer currently requires multiple MB of physically continuous memory, and may fail to be allocated when memory is fragmented. The patch is created for this issue. The SKB buffer is usually less than 1500 bytes or up to several KB with jumbo frame, so it's much less sensitive to fragmented memory. I will work on another patch to use SKB buffer with discontinuous pages. Could you accept this patch separately? Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/1] staging: fpgaboot: Xilinx FPGA firmware download driver
On Mon, Jan 20, 2014 at 10:51 AM, Hartley Sweeten hartl...@visionengravers.com wrote: I have not looked at this driver but... Couldn't this be done from user space using urjtag? http://urjtag.org/ Thank you for letting me know urjtag. This can be useful to replace existing jtag dongles. However, the use case for my fpgaboot driver and urjtag are different. The fpgaboot driver downloads Xilinx FPGA firmware during run time on the target without any h/w device support (i.e jtag dongle). This can be used in development as well as deployment scenario. And it does _one_ thing very well, this can download fpga firmware (10s of MByte) in less then half a seconds within our system. What I see urjtag does exactly what jtag devices are doing, which means urjtag normally runs on development host (not on target), and need a jatag dongle. It supports many different types of dongles and devices, which itself is awesome in many ways, and could be cheaper. Regards, ISS ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 0/1] staging: fpgaboot: Xilinx FPGA firmware download drive
Xilinx FPGA firmware download driver v5 - update based on Greg's feedback - updated README to reflect the use case of the driver Insop Song (1): staging: fpgaboot: Xilinx FPGA firmware download driver drivers/staging/Kconfig |2 + drivers/staging/Makefile |1 + drivers/staging/gs_fpgaboot/Kconfig |8 + drivers/staging/gs_fpgaboot/Makefile |4 + drivers/staging/gs_fpgaboot/README| 71 + drivers/staging/gs_fpgaboot/TODO |7 + drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 425 + drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 56 drivers/staging/gs_fpgaboot/io.c | 294 drivers/staging/gs_fpgaboot/io.h | 90 ++ 10 files changed, 958 insertions(+) create mode 100644 drivers/staging/gs_fpgaboot/Kconfig create mode 100644 drivers/staging/gs_fpgaboot/Makefile create mode 100644 drivers/staging/gs_fpgaboot/README create mode 100644 drivers/staging/gs_fpgaboot/TODO create mode 100644 drivers/staging/gs_fpgaboot/gs_fpgaboot.c create mode 100644 drivers/staging/gs_fpgaboot/gs_fpgaboot.h create mode 100644 drivers/staging/gs_fpgaboot/io.c create mode 100644 drivers/staging/gs_fpgaboot/io.h -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 1/1] staging: fpgaboot: Xilinx FPGA firmware download driver
This driver downloads Xilinx FPGA firmware using gpio pins. It loads Xilinx FPGA bitstream format firmware image and program the Xilinx FPGA using SelectMAP (parallel) mode. Signed-off-by: Insop Song insop.s...@gainspeed.com --- drivers/staging/Kconfig |2 + drivers/staging/Makefile |1 + drivers/staging/gs_fpgaboot/Kconfig |8 + drivers/staging/gs_fpgaboot/Makefile |4 + drivers/staging/gs_fpgaboot/README| 71 + drivers/staging/gs_fpgaboot/TODO |7 + drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 425 + drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 56 drivers/staging/gs_fpgaboot/io.c | 294 drivers/staging/gs_fpgaboot/io.h | 90 ++ 10 files changed, 958 insertions(+) create mode 100644 drivers/staging/gs_fpgaboot/Kconfig create mode 100644 drivers/staging/gs_fpgaboot/Makefile create mode 100644 drivers/staging/gs_fpgaboot/README create mode 100644 drivers/staging/gs_fpgaboot/TODO create mode 100644 drivers/staging/gs_fpgaboot/gs_fpgaboot.c create mode 100644 drivers/staging/gs_fpgaboot/gs_fpgaboot.h create mode 100644 drivers/staging/gs_fpgaboot/io.c create mode 100644 drivers/staging/gs_fpgaboot/io.h diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig index 4bb6b11..d08b14f 100644 --- a/drivers/staging/Kconfig +++ b/drivers/staging/Kconfig @@ -148,4 +148,6 @@ source drivers/staging/dgnc/Kconfig source drivers/staging/dgap/Kconfig +source drivers/staging/gs_fpgaboot/Kconfig + endif # STAGING diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile index 9f07e5e..ecba69b 100644 --- a/drivers/staging/Makefile +++ b/drivers/staging/Makefile @@ -66,3 +66,4 @@ obj-$(CONFIG_XILLYBUS)+= xillybus/ obj-$(CONFIG_DGNC) += dgnc/ obj-$(CONFIG_DGAP) += dgap/ obj-$(CONFIG_MTD_SPINAND_MT29F)+= mt29f_spinand/ +obj-$(CONFIG_GS_FPGABOOT) += gs_fpgaboot/ diff --git a/drivers/staging/gs_fpgaboot/Kconfig b/drivers/staging/gs_fpgaboot/Kconfig new file mode 100644 index 000..5506452 --- /dev/null +++ b/drivers/staging/gs_fpgaboot/Kconfig @@ -0,0 +1,8 @@ +# +# xilinx FPGA firmware download, fpgaboot +# +config GS_FPGABOOT + tristate Xilinx FPGA firmware download module + default n + help + Xilinx FPGA firmware download module diff --git a/drivers/staging/gs_fpgaboot/Makefile b/drivers/staging/gs_fpgaboot/Makefile new file mode 100644 index 000..34cb606 --- /dev/null +++ b/drivers/staging/gs_fpgaboot/Makefile @@ -0,0 +1,4 @@ +gs_fpga-y += gs_fpgaboot.o io.o +obj-$(CONFIG_GS_FPGABOOT) += gs_fpga.o + +ccflags-$(CONFIG_GS_FPGA_DEBUG):= -DDEBUG diff --git a/drivers/staging/gs_fpgaboot/README b/drivers/staging/gs_fpgaboot/README new file mode 100644 index 000..cfa8624 --- /dev/null +++ b/drivers/staging/gs_fpgaboot/README @@ -0,0 +1,71 @@ +== +Linux Driver Source for Xilinx FPGA firmware download +== + + +TABLE OF CONTENTS. + +1. SUMMARY +2. BACKGROUND +3. DESIGN +4. HOW TO USE +5. REFERENCE + +1. SUMMARY + + - Download Xilinx FPGA firmware + - This module downloads Xilinx FPGA firmware using gpio pins. + +2. BACKGROUND + + An FPGA (Field Programmable Gate Array) is a programmable hardware that is + used in various applications. Hardware design needs to programmed through + a dedicated device or CPU assisted way (serial or parallel). + This driver provides a way to download FPGA firmware. + +3. DESIGN + + - load Xilinx FPGA bitstream format[1] firmware image file using + kernel firmware framework, request_firmware() + - program the Xilinx FPGA using SelectMAP (parallel) mode [2] + - FPGA prgram is done by gpio based bit-banging, as an example + - platform independent file: gs_fpgaboot.c + - platform dependent file: io.c + + +4. HOW TO USE + + $ insmod gs_fpga.ko file=xlinx_fpga_top_bitstream.bit + $ rmmod gs_fpga + +5. USE CASE (from a mailing list discussion with Greg) + + a. As a FPGA development support tool, + During FPGA firmware development, you need to download a new FPGA + image frequently. + You would do that with a dedicated JTAG, which usually a limited + resource in the lab. + However, if you use my driver, you don't have to have a dedicated JTAG. + This is a real gain :) + + b. For the FPGA that runs without config after the download, which + doesn't talk to any of Linux interfaces (such as PCIE). + + We download FPGA firmware from user triggered or some other way, and that's it. + Since that FPGA runs on its own, it doesn't require a linux driver + after the download. + +
