Re: [PATCH v3 1/2] Staging: lustre: Refactor the function interval_erase_color() in /lustre/ldlm/interval_tree.c
On Jan 13, 2014, at 11:56 PM, Dilger, Andreas andreas.dil...@intel.com wrote: Begin forwarded message: From: Greg KH gre...@linuxfoundation.org Subject: Re: [PATCH v3 1/2] Staging: lustre: Refactor the function interval_erase_color() in /lustre/ldlm/interval_tree.c Date: January 11, 2014 at 1:33:58 PM MST To: Monam Agarwal monamagarwal...@gmail.com Cc: Dan Carpenter dan.carpen...@oracle.com, de...@driverdev.osuosl.org, andreas.dil...@intel.com, peter.p.waskiewicz...@intel.com, linux-ker...@vger.kernel.org, Rashika Kheria rashika.khe...@gmail.com On Sat, Jan 11, 2014 at 05:14:35PM +0530, Monam Agarwal wrote: On Sat, Jan 11, 2014 at 5:09 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Sat, Jan 11, 2014 at 04:56:44PM +0530, Monam Agarwal wrote: I took n as a flag to decide whether parent-in_left == node is true or not in the called function. So n stands for node? Should I use some other name for the flag. Yes. Will flag be a suitable name? I’d suggest `bool is_right_child’. I’ve checked the patch and it looks good. There exists a unit test case for interval tree under lustre/tests/ named it_test.c, please compile it and verify your change. Jinshan Ick, no. You don't want a flag to have to determine what the logic is for a given function. That just causes confusion and makes things really hard to read and understand over time. This whole function looks like a red/black tree, or something like that. Shouldn't we just be using the in-kernel implementation of this? And if not, then you really need to get the feedback of the code's original authors as you might be changing the algorithm in ways that could cause big problems. thanks, greg k-h Cheers, Andreas -- Andreas Dilger Lustre Software Architect Intel Corporation ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] memstick: Add realtek USB memstick host driver
On Tue, Jan 14, 2014 at 03:47:36PM +0800, rogera...@realtek.com wrote: +static int ms_pull_ctl_disable(struct rtsx_ucr *ucr) +{ + rtsx_usb_init_cmd(ucr); + + if (CHECK_PKG(ucr, LQFP48)) { + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL1, 0xFF, 0x55); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL2, 0xFF, 0x55); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL3, 0xFF, 0x95); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL4, 0xFF, 0x55); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL5, 0xFF, 0x55); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL6, 0xFF, 0xA5); + } else { + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL1, 0xFF, 0x65); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL2, 0xFF, 0x55); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL3, 0xFF, 0x95); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL4, 0xFF, 0x55); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL5, 0xFF, 0x56); + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, + CARD_PULL_CTL6, 0xFF, 0x59); + } + I'm fine with this patch going in as-is, but I just wanted to comment on this for future reference/cleanups. These blocks where all the lines are over 80 characters are hard to look at. We could break it into separate functions. static int ms_pull_ctl_disable_lqfp48(struct rtsx_ucr *ucr) { rtsx_usb_init_cmd(ucr); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_PULL_CTL1, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_PULL_CTL2, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_PULL_CTL3, 0xFF, 0x95); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_PULL_CTL4, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_PULL_CTL5, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_PULL_CTL6, 0xFF, 0xA5); return rtsx_usb_send_cmd(ucr, MODE_C, 100); } Or another option would be to remove the CARD_ prefix. I don't think we would miss it. if (CHECK_PKG(ucr, LQFP48)) { rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL1, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL2, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL3, 0xFF, 0x95); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL4, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL5, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL6, 0xFF, 0xA5); } else { rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL1, 0xFF, 0x65); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL2, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL3, 0xFF, 0x95); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL4, 0xFF, 0x55); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL5, 0xFF, 0x56); rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, PULL_CTL6, 0xFF, 0x59); } Another option might be to remove the _usb from the rtsx_usb_add_cmd(). Just some ideas. 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 Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 04:37:32PM -0800, Insop Song wrote: On Sat, Jan 11, 2014 at 4:10 PM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 03:56:48PM -0800, Insop Song wrote: On Sat, Jan 11, 2014 at 1:05 PM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 12:51:28PM -0800, Greg KH wrote: On Thu, Jan 09, 2014 at 11:48:04AM -0800, Insop Song wrote: 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 This patch breaks the build on my machine, please be more careful: drivers/staging/gs_fpgaboot/gs_fpgaboot.c:30:28: fatal error: include/asm/io.h: No such file or directory #include include/asm/io.h Please fix this up, test it, and resend. Also, while you are fixing things up, please just remove the character device node from the driver, you aren't doing anything with it, so it's not needed. Thank you for the suggestion, makes a lot of sense and code cleaner. Could you take a quick look? If it is good, will send out a new patch, v5, shortly. Main change is +/* fake device for request_firmware */ +static struct platform_device*firmware_pdev; + firmware_pdev = platform_device_register_simple(fpgaboot, -1, + NULL, 0); Why do you need a platform device? Don't you have something on the hardware that describes the fact that this device is in the system, or not? Like a PCI id? DMI id? Device Tree entry? You must have something that you can trigger off of. This driver is only to download fpga firmware, not intended to keep managing the fpga device after the downloading. The device we use is not part of the device tree or PCI. But how does the system know to load it or not, automatically? It downloads the image and after that no need to be existing. Then why not have the module remove itself? The following (insmod to load firmware and rmmod remove the module) commands are triggered from user space application. They are wrapped as a script and triggered from an application. Once the firmware is loaded, then this driver's job is done. Here is how we use $ insmod gs_fpga.ko file=xlinx_fpga_top_bitstream.bit $ rmmod gs_fpga Linux modules should be able to be autoloaded based on hardware present in the system, it's been that way for over a decade now. Understood, however the use case for this driver is to load firmware, only that. The other way to do could have been use a user space program read file and access gpio (or other means) which could end up taking lot longer firmware programming time. I still needed device to use request_firmware() even though I don't use it. I've looked at other drivers which uses request_firmware() after your suggestion of removing char device node, and I've found platform_device can be a simple one to use. Please base your device on a real one in the system, like your FPGA device, surely there is some way it can be detected to be present or not? Do you still think a fake platform device doesn't cut? then what do you suggest? 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 Tue, Jan 14, 2014 at 1:18 AM, Insop Song insop.s...@gmail.com wrote: On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 04:37:32PM -0800, Insop Song wrote: On Sat, Jan 11, 2014 at 4:10 PM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 03:56:48PM -0800, Insop Song wrote: On Sat, Jan 11, 2014 at 1:05 PM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 12:51:28PM -0800, Greg KH wrote: On Thu, Jan 09, 2014 at 11:48:04AM -0800, Insop Song wrote: 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 This patch breaks the build on my machine, please be more careful: drivers/staging/gs_fpgaboot/gs_fpgaboot.c:30:28: fatal error: include/asm/io.h: No such file or directory #include include/asm/io.h Please fix this up, test it, and resend. Also, while you are fixing things up, please just remove the character device node from the driver, you aren't doing anything with it, so it's not needed. Thank you for the suggestion, makes a lot of sense and code cleaner. Could you take a quick look? If it is good, will send out a new patch, v5, shortly. Main change is +/* fake device for request_firmware */ +static struct platform_device*firmware_pdev; + firmware_pdev = platform_device_register_simple(fpgaboot, -1, + NULL, 0); Why do you need a platform device? Don't you have something on the hardware that describes the fact that this device is in the system, or not? Like a PCI id? DMI id? Device Tree entry? You must have something that you can trigger off of. This driver is only to download fpga firmware, not intended to keep managing the fpga device after the downloading. The device we use is not part of the device tree or PCI. But how does the system know to load it or not, automatically? It downloads the image and after that no need to be existing. Then why not have the module remove itself? The following (insmod to load firmware and rmmod remove the module) commands are triggered from user space application. They are wrapped as a script and triggered from an application. Once the firmware is loaded, then this driver's job is done. Here is how we use $ insmod gs_fpga.ko file=xlinx_fpga_top_bitstream.bit $ rmmod gs_fpga Linux modules should be able to be autoloaded based on hardware present in the system, it's been that way for over a decade now. Greg, I think it's better to add more explanation for FPGA. FPGA (Field-programmable gate array) is not functional until the firmware is downloaded. That means that it is not easy, if not possible, to auto detect the device. This is why insmod is manually called. FPGA firmware download happen ever time system boots, so this is different from occasional upgrade. To sum up, FPGA download occurs every boot, and before this download is done, the device in question is not functional. Understood, however the use case for this driver is to load firmware, only that. The other way to do could have been use a user space program read file and access gpio (or other means) which could end up taking lot longer firmware programming time. I still needed device to use request_firmware() even though I don't use it. I've looked at other drivers which uses request_firmware() after your suggestion of removing char device node, and I've found platform_device can be a simple one to use. Please base your device on a real one in the system, like your FPGA device, surely there is some way it can be detected to be present or not? Do you still think a fake platform device doesn't cut? then what do you suggest? Thank you, ISS Thank you, ISS ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: vme_tsi148 question
On 14/01/14 04:07, Michael Kenney wrote: Unfortunately, the results are the same. I'm running this from home so I'll have to confirm with the logic-analyzer tomorrow but the bridge driver error messages suggest the same D8 writes (which for some reason our A/D board does not like). The hardware probably doesn't support 8-bit reads/writes, from memory, it doesn't have to according to the spec. Here's a dmesg excerpt: [ 5631.810902] vme_tsi148 :02:02.0: VME Bus Error at address: 0x508000, attributes: 8008b900 [ 5631.810972] vme_tsi148 :02:02.0: VME Bus Exception Overflow Occurred [ 5631.810977] vme_tsi148 :02:02.0: VME Bus Error at address: 0x508001, attributes: c0087900 [ 5631.810991] vme_tsi148 :02:02.0: VME Bus Error at address: 0x508003, attributes: 80087900 [ 5631.811061] vme_tsi148 :02:02.0: VME Bus Exception Overflow Occurred The register that I am trying to write occupies 4 bytes starting at 0x508000. I have verified that the value I am trying to write to the register is 4-byte aligned. I'm essentially doing the following: uint32_t value; pwrite(window_fd, value, sizeof(value), 0x508000); How have you setup the window? The offset is relative to the window you have configured, so if that doesn't start at 0x0, then you aren't writing where you think you are. Window is configured for A24 and D32. Does the attributes value from the tsi148 provide any further clues? Not really - it gives the status of some of the VME lines when the error occurred (Address lines, data strobes), it says that there was an error, in one case it shows an exception overflow and that it happened on the VME bus. Martyn -- Martyn Welch (Lead Software Engineer) | Registered in England and Wales GE Intelligent Platforms | (3828642) at 100 Barbirolli Square T +44(0)1327322748 | Manchester, M2 3AB E martyn.we...@ge.com | VAT:GB 927559189 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/2] Staging: lustre: Refactor the function interval_erase_color() in /lustre/ldlm/interval_tree.c
On Tue, Jan 14, 2014 at 1:31 PM, Xiong, Jinshan jinshan.xi...@intel.com wrote: On Jan 13, 2014, at 11:56 PM, Dilger, Andreas andreas.dil...@intel.com wrote: Begin forwarded message: From: Greg KH gre...@linuxfoundation.org Subject: Re: [PATCH v3 1/2] Staging: lustre: Refactor the function interval_erase_color() in /lustre/ldlm/interval_tree.c Date: January 11, 2014 at 1:33:58 PM MST To: Monam Agarwal monamagarwal...@gmail.com Cc: Dan Carpenter dan.carpen...@oracle.com, de...@driverdev.osuosl.org, andreas.dil...@intel.com, peter.p.waskiewicz...@intel.com, linux-ker...@vger.kernel.org, Rashika Kheria rashika.khe...@gmail.com On Sat, Jan 11, 2014 at 05:14:35PM +0530, Monam Agarwal wrote: On Sat, Jan 11, 2014 at 5:09 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Sat, Jan 11, 2014 at 04:56:44PM +0530, Monam Agarwal wrote: I took n as a flag to decide whether parent-in_left == node is true or not in the called function. So n stands for node? Should I use some other name for the flag. Yes. Will flag be a suitable name? I’d suggest `bool is_right_child’. I’ve checked the patch and it looks good. There exists a unit test case for interval tree under lustre/tests/ named it_test.c, please compile it and verify your change. I am using tree from http://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/. There is no lustre/tests folder in my tree and no file named it_test.c. Kindly let me know the tree you are working from. Jinshan Ick, no. You don't want a flag to have to determine what the logic is for a given function. That just causes confusion and makes things really hard to read and understand over time. This whole function looks like a red/black tree, or something like that. Shouldn't we just be using the in-kernel implementation of this? And if not, then you really need to get the feedback of the code's original authors as you might be changing the algorithm in ways that could cause big problems. thanks, greg k-h Cheers, Andreas -- Andreas Dilger Lustre Software Architect Intel Corporation ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: move trailing statement to next line in ni_mio_common.c
On 2014-01-14 07:23, Dan Carpenter wrote: On Mon, Jan 13, 2014 at 07:16:14PM -0800, Joe Perches wrote: On Mon, 2014-01-13 at 21:13 -0600, Chase Southwood wrote: This patch for ni_mio_common.c silences a checkpatch error due to a trailing statement. [] diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c [] @@ -692,7 +692,8 @@ static void ni_clear_ai_fifo(struct comedi_device *dev) /* Flush the 6143 data FIFO */ ni_writel(0x10, AIFIFO_Control_6143); /* Flush fifo */ ni_writel(0x00, AIFIFO_Control_6143); /* Flush fifo */ - while (ni_readl(AIFIFO_Status_6143) 0x10) ; /* Wait for complete */ + while (ni_readl(AIFIFO_Status_6143) 0x10) + ; /* Wait for complete */ It's generally better to use timeouts too. Just to clarify what Joe is saying do: /* Wait for complete */ while (timemout TIMEOUT) { if (ni_readl(AIFIFO_Status_6143) 0x10) break; udelay(1); } I added in a delay... The problem is that you'd probably have to look at the hardware spec to know what timeout to use or if the delay is needed. Some longish timeout of, say, 1 iterations (~ 0.01 seconds) would probably do as it's not that time critical. We wouldn't expect code clean-up patches to have to deal with that sort of thing though. -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: remove unnecessary braces in pcl711.c
On 2014-01-14 03:13, Chase Southwood wrote: This patch for pcl711.c removes braces causing a checkpatch.pl warning. It also removes an empty else arm of an if-else statement. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- I removed the else arm of this statement because it was empty, save for the ignore comment. If it is instead preferred kernel style for this empty else to remain in, let me know and I will correct this patch accordingly. drivers/staging/comedi/drivers/pcl711.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/staging/comedi/drivers/pcl711.c b/drivers/staging/comedi/drivers/pcl711.c index f0fc123..6595788 100644 --- a/drivers/staging/comedi/drivers/pcl711.c +++ b/drivers/staging/comedi/drivers/pcl711.c @@ -336,11 +336,8 @@ static int pcl711_ai_cmdtest(struct comedi_device *dev, err |= cfc_check_trigger_arg_is(cmd-convert_arg, 0); err |= cfc_check_trigger_arg_is(cmd-scan_end_arg, cmd-chanlist_len); - if (cmd-stop_src == TRIG_NONE) { + if (cmd-stop_src == TRIG_NONE) err |= cfc_check_trigger_arg_is(cmd-stop_arg, 0); - } else { - /* ignore */ - } if (err) return 3; Not sure if it's worth me reviewing these minor clean-up patches or not, but FWIW I approve of this patch. Reviewed-by: Ian Abbott abbo...@mev.co.uk -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] Staging: media: Fix quoted string split across line in as102_fe.c
This patch fixes the following checkpatch.pl issues in as102/as102_fe.c WARNING: quoted string split across lines Signed-off-by: Monam Agarwal monamagarwal...@gmail.com --- drivers/staging/media/as102/as102_fe.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/media/as102/as102_fe.c b/drivers/staging/media/as102/as102_fe.c index 9ce8c9d..dc367d1 100644 --- a/drivers/staging/media/as102/as102_fe.c +++ b/drivers/staging/media/as102/as102_fe.c @@ -151,8 +151,8 @@ static int as102_fe_read_status(struct dvb_frontend *fe, fe_status_t *status) if (as10x_cmd_get_demod_stats(dev-bus_adap, (struct as10x_demod_stats *) dev-demod_stats) 0) { memset(dev-demod_stats, 0, sizeof(dev-demod_stats)); - dprintk(debug, as10x_cmd_get_demod_stats failed - (probably not tuned)\n); + dprintk(debug, + as10x_cmd_get_demod_stats failed (probably not tuned)\n); } else { dprintk(debug, demod status: fc: 0x%08x, bad fc: 0x%08x, @@ -581,8 +581,8 @@ static void as102_fe_copy_tune_parameters(struct as10x_tune_args *tune_args, as102_fe_get_code_rate(params-code_rate_LP); } - dprintk(debug, \thierarchy: 0x%02x - selected: %s code_rate_%s: 0x%02x\n, + dprintk(debug, + \thierarchy: 0x%02x selected: %s code_rate_%s: 0x%02x\n, tune_args-hierarchy, tune_args-hier_select == HIER_HIGH_PRIORITY ? HP : LP, -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] Staging: media: Fix line length exceeding 80 characters in as102_fe.c
This patch fixes the following checkpatch.pl issues in as102/as102_fe.c WARNING: line over 80 characters Signed-off-by: Monam Agarwal monamagarwal...@gmail.com --- drivers/staging/media/as102/as102_fe.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/as102/as102_fe.c b/drivers/staging/media/as102/as102_fe.c index dc367d1..bd5cd92 100644 --- a/drivers/staging/media/as102/as102_fe.c +++ b/drivers/staging/media/as102/as102_fe.c @@ -263,7 +263,8 @@ static int as102_fe_ts_bus_ctrl(struct dvb_frontend *fe, int acquire) if (acquire) { if (elna_enable) - as10x_cmd_set_context(dev-bus_adap, CONTEXT_LNA, dev-elna_cfg); + as10x_cmd_set_context(dev-bus_adap, + CONTEXT_LNA, dev-elna_cfg); ret = as10x_cmd_turn_on(dev-bus_adap); } else { -- 1.7.9.5 ___ 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
From: Roger Tseng rogera...@realtek.com Realtek USB card reader provides a channel to transfer command or data to flash memory cards. This driver exports host instances for mmc and memstick subsystems and handles basic works. Signed-off-by: Roger Tseng rogera...@realtek.com --- drivers/mfd/Kconfig | 10 + drivers/mfd/Makefile | 1 + drivers/mfd/rtsx_usb.c | 788 +++ include/linux/mfd/rtsx_usb.h | 619 + 4 files changed, 1418 insertions(+) create mode 100644 drivers/mfd/rtsx_usb.c create mode 100644 include/linux/mfd/rtsx_usb.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index b7c74a7..4c99ebd 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -507,6 +507,16 @@ config MFD_RTSX_PCI types of memory cards, such as Memory Stick, Memory Stick Pro, Secure Digital and MultiMediaCard. +config MFD_RTSX_USB + tristate Realtek USB card reader + depends on USB + select MFD_CORE + help + Select this option to get support for Realtek USB 2.0 card readers + including RTS5129, RTS5139, RTS5179 and RTS5170. + Realtek card reader supports access to many types of memory cards, + such as Memory Stick Pro, Secure Digital and MultiMediaCard. + The help section should be indented by 2 spaces. config MFD_RC5T583 bool Ricoh RC5T583 Power Management system device depends on I2C=y diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 8a28dc9..33b8de6 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o rtsx_pci-objs:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o +obj-$(CONFIG_MFD_RTSX_USB) += rtsx_usb.o obj-$(CONFIG_HTC_EGPIO) += htc-egpio.o obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c new file mode 100644 index 000..905ec8d --- /dev/null +++ b/drivers/mfd/rtsx_usb.c @@ -0,0 +1,788 @@ +/* Driver for Realtek USB card reader + * + * Copyright(c) 2009-2013 Realtek Semiconductor Corp. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2, or (at your option) any + * later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, see http://www.gnu.org/licenses/. + * + * Author: + * Roger Tseng rogera...@realtek.com + */ +#include linux/module.h +#include linux/slab.h +#include linux/mutex.h +#include linux/usb.h +#include linux/platform_device.h +#include linux/mfd/core.h +#include asm/unaligned.h +#include linux/mfd/rtsx_usb.h + +static int polling_pipe = 1; +module_param(polling_pipe, int, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(polling_pipe, polling pipe (0: ctl, 1: bulk)); '/n' here. +static struct mfd_cell rtsx_usb_cells[] = { + [RTSX_USB_SD_CARD] = { + .name = DRV_NAME_RTSX_USB_SDMMC, + }, + [RTSX_USB_MS_CARD] = { + .name = DRV_NAME_RTSX_USB_MS, + }, +}; I'm not entirely convinced that this is an MFD device? +static void rtsx_usb_sg_timed_out(unsigned long data) +{ + struct rtsx_ucr *ucr = (struct rtsx_ucr *)data; What's going to happen when your device runs 64bit? + dev_dbg(ucr-pusb_intf-dev, %s: sg transfer timed out, __func__); + usb_sg_cancel(ucr-current_sg); Are you sure this needs to live here? Why isn't this in drivers/usb? + /* we know the cancellation is caused by time-out */ + ucr-current_sg.status = -ETIMEDOUT; +} + +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
Re: [PATCH 1/3] mfd: Add realtek USB card reader driver
[ Sorry, I am coming down with the flu today so I'm doing dorky things like reviewing review comments. I'm not sure how coherent I am. ] On Tue, Jan 14, 2014 at 01:04:09PM +, Lee Jones wrote: +static void rtsx_usb_sg_timed_out(unsigned long data) +{ + struct rtsx_ucr *ucr = (struct rtsx_ucr *)data; What's going to happen when your device runs 64bit? I'm not sure I understand what you mean here. On linux sizeof(long) is always the same as sizeof(void *). + if (cmd_len IOBUF_SIZE) + return -EINVAL; + + if (cmd_len % 4) + cmd_len += (4 - cmd_len % 4); Please document in a comment. There is a kernel macro for this: cmd_len = ALIGN(cmd_len, 4); if (cmd_len IOBUF_SIZE) return -EINVAL; + + Extra '/n' It weirds me out when you mix up '\n' and /n'. +int rtsx_usb_ep0_write_register(struct rtsx_ucr *ucr, u16 addr, + u8 mask, u8 data) +{ + u16 value = 0, index = 0; + + value |= 0x03 14; + value |= addr 0x3FFF; + value = ((value 8) 0xFF00) | ((value 8) 0x00FF); + index |= (u16)mask; + index |= (u16)data 8; Lots of random numbers here, please #define for clarity and ease of reading. The only really random number is the 0x03, but yeah, it would help if that we a define. addr |= 0x03 14; value = __swab16(addr); index = mask | (data 8); + + dev_dbg(intf-dev, + : Realtek USB Card Reader found at bus %03d address %03d\n, +usb_dev-bus-busnum, usb_dev-devnum); + + ucr = kzalloc(sizeof(struct rtsx_ucr), GFP_KERNEL); s/struct rtsx_ucr/*ucr/ Any reason for not using managed resources? Roger, he means the devm_kzalloc(). regards, dan carpenter ___ 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 Tue, Jan 14, 2014 at 04:46:34PM +0300, Dan Carpenter wrote: [ Sorry, I am coming down with the flu today so I'm doing dorky things like reviewing review comments. I'm not sure how coherent I am. ] On Tue, Jan 14, 2014 at 01:04:09PM +, Lee Jones wrote: +static void rtsx_usb_sg_timed_out(unsigned long data) +{ + struct rtsx_ucr *ucr = (struct rtsx_ucr *)data; What's going to happen when your device runs 64bit? I'm not sure I understand what you mean here. On linux sizeof(long) is always the same as sizeof(void *). + if (cmd_len IOBUF_SIZE) + return -EINVAL; + + if (cmd_len % 4) + cmd_len += (4 - cmd_len % 4); Please document in a comment. There is a kernel macro for this: cmd_len = ALIGN(cmd_len, 4); Btw, what's the difference between ALIGN(cmd_len, 4) and round_up(cmd_len, 4)? Maybe round_up() is better. regards, dan carpenter ___ 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
[ Sorry, I am coming down with the flu today so I'm doing dorky things like reviewing review comments. I'm not sure how coherent I am. ] Always welcome. NB: I did this review in double-quick time, which may account for some of the weird thought processes (or lack there of). +static void rtsx_usb_sg_timed_out(unsigned long data) +{ + struct rtsx_ucr *ucr = (struct rtsx_ucr *)data; What's going to happen when your device runs 64bit? I'm not sure I understand what you mean here. On linux sizeof(long) is always the same as sizeof(void *). I had an odd moment where I thought we'd need long long for 64bit. Nevermind. + if (cmd_len IOBUF_SIZE) + return -EINVAL; + + if (cmd_len % 4) + cmd_len += (4 - cmd_len % 4); Please document in a comment. There is a kernel macro for this: cmd_len = ALIGN(cmd_len, 4); if (cmd_len IOBUF_SIZE) return -EINVAL; I had a feeling this was the intention, hence why I was asking for a comment. But yes, if all this is doing is alignment then the kernel macro is preferred. + + Extra '/n' It weirds me out when you mix up '\n' and /n'. Typos happen on occasion... +int rtsx_usb_ep0_write_register(struct rtsx_ucr *ucr, u16 addr, + u8 mask, u8 data) +{ + u16 value = 0, index = 0; + + value |= 0x03 14; + value |= addr 0x3FFF; + value = ((value 8) 0xFF00) | ((value 8) 0x00FF); + index |= (u16)mask; + index |= (u16)data 8; Lots of random numbers here, please #define for clarity and ease of reading. The only really random number is the 0x03, but yeah, it would help if that we a define. See ---^ As I say, I just glossed over the code, thus didn't spend the time to work out the arithmetic. Now I do, most of it is pretty self explanatory. I would also like to see the SHIFT #defined, although this may become superfluous once the 0x03 is clarified. addr |= 0x03 14; value = __swab16(addr); index = mask | (data 8); This is 100% better/clearer. + + dev_dbg(intf-dev, + : Realtek USB Card Reader found at bus %03d address %03d\n, + usb_dev-bus-busnum, usb_dev-devnum); + + ucr = kzalloc(sizeof(struct rtsx_ucr), GFP_KERNEL); s/struct rtsx_ucr/*ucr/ Any reason for not using managed resources? Roger, he means the devm_kzalloc(). That I do. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: media: Fix line length exceeding 80 characters in as102_drv.c
This patch fixes the following checkpatch.pl warning in as102/as102_drv.c WARNING: line over 80 characters in the file Signed-off-by: Monam Agarwal monamagarwal...@gmail.com --- drivers/staging/media/as102/as102_drv.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/as102/as102_drv.c b/drivers/staging/media/as102/as102_drv.c index 8b7bb95..7c294bf 100644 --- a/drivers/staging/media/as102/as102_drv.c +++ b/drivers/staging/media/as102/as102_drv.c @@ -133,7 +133,8 @@ static int as10x_pid_filter(struct as102_dev_t *dev, filter.pid = pid; ret = as10x_cmd_add_PID_filter(bus_adap, filter); - dprintk(debug, ADD_PID_FILTER([%02d - %02d], 0x%04x) ret = %d\n, + dprintk(debug, + ADD_PID_FILTER([%02d - %02d], 0x%04x) ret = %d\n, index, filter.idx, filter.pid, ret); break; } -- 1.7.9.5 ___ 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 Tue, Jan 14, 2014 at 03:47:34PM +0800, rogera...@realtek.com wrote: From: Roger Tseng rogera...@realtek.com Realtek USB card reader provides a channel to transfer command or data to flash memory cards. This driver exports host instances for mmc and memstick subsystems and handles basic works. Signed-off-by: Roger Tseng rogera...@realtek.com --- drivers/mfd/Kconfig | 10 + drivers/mfd/Makefile | 1 + drivers/mfd/rtsx_usb.c | 788 +++ include/linux/mfd/rtsx_usb.h | 619 + 4 files changed, 1418 insertions(+) create mode 100644 drivers/mfd/rtsx_usb.c create mode 100644 include/linux/mfd/rtsx_usb.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index b7c74a7..4c99ebd 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -507,6 +507,16 @@ config MFD_RTSX_PCI types of memory cards, such as Memory Stick, Memory Stick Pro, Secure Digital and MultiMediaCard. +config MFD_RTSX_USB + tristate Realtek USB card reader + depends on USB + select MFD_CORE + help + Select this option to get support for Realtek USB 2.0 card readers + including RTS5129, RTS5139, RTS5179 and RTS5170. + Realtek card reader supports access to many types of memory cards, + such as Memory Stick Pro, Secure Digital and MultiMediaCard. + config MFD_RC5T583 bool Ricoh RC5T583 Power Management system device depends on I2C=y diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 8a28dc9..33b8de6 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o rtsx_pci-objs:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o +obj-$(CONFIG_MFD_RTSX_USB) += rtsx_usb.o obj-$(CONFIG_HTC_EGPIO) += htc-egpio.o obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c new file mode 100644 index 000..905ec8d --- /dev/null +++ b/drivers/mfd/rtsx_usb.c @@ -0,0 +1,788 @@ +/* Driver for Realtek USB card reader + * + * Copyright(c) 2009-2013 Realtek Semiconductor Corp. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2, or (at your option) any + * later version. Do you really mean any later version? (sorry, I have to ask.) Same goes for the other files you add with this specific license wording. +#include linux/module.h +#include linux/slab.h +#include linux/mutex.h +#include linux/usb.h +#include linux/platform_device.h +#include linux/mfd/core.h +#include asm/unaligned.h Why is this file neded? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/8] staging: dgap: Fix trailing whitespace in dgap_downld.h
This patch fixed trailing whitespace found by checkpatch.pl in staging/dgap/dgap_downld.h Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/dgap/dgap_downld.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/dgap/dgap_downld.h b/drivers/staging/dgap/dgap_downld.h index 271ac19..910a45d 100644 --- a/drivers/staging/dgap/dgap_downld.h +++ b/drivers/staging/dgap/dgap_downld.h @@ -23,7 +23,7 @@ */ /* -** downld.h +** downld.h ** - describes the interface between the user level download process **and the concentrator download driver. */ @@ -57,7 +57,7 @@ struct downldio { #define DIGI_NUKE_RESET_ALL (1 31) #define DIGI_NUKE_INHIBIT_POLLER (1 30) #define DIGI_NUKE_BRD_NUMB0x0f - + #defineDLREQ_BIOS 0 -- 1.8.5.2.309.ga25014b ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/8] staging: dgap: Fix trailing whitespace in dgap_sysfs.h
This patch fixed trailing whitespace found by checkpatch.pl in staging/dgap/dgap_sysfs.h Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/dgap/dgap_sysfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/dgap/dgap_sysfs.h b/drivers/staging/dgap/dgap_sysfs.h index dde690e..151f1b3 100644 --- a/drivers/staging/dgap/dgap_sysfs.h +++ b/drivers/staging/dgap/dgap_sysfs.h @@ -32,7 +32,7 @@ struct un_t; struct pci_driver; struct class_device; -extern void dgap_create_ports_sysfiles(struct board_t *bd); +extern void dgap_create_ports_sysfiles(struct board_t *bd); extern void dgap_remove_ports_sysfiles(struct board_t *bd); extern void dgap_create_driver_sysfiles(struct pci_driver *); -- 1.8.5.2.309.ga25014b ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/8] staging: dgap: Fix trailing whitespace in dgap_fep5.h
This patch fixed trailing whitespace found by checkpatch.pl in staging/dgap/dgap_fep5.h Signed-off-by: Masanari Iida standby2...@gmail.com # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # On branch digi-fix0114 # Changes to be committed: # modified: dgap_fep5.h # --- drivers/staging/dgap/dgap_fep5.h | 46 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/staging/dgap/dgap_fep5.h b/drivers/staging/dgap/dgap_fep5.h index c9abc40..d0650ae 100644 --- a/drivers/staging/dgap/dgap_fep5.h +++ b/drivers/staging/dgap/dgap_fep5.h @@ -18,7 +18,7 @@ * * NOTE: THIS IS A SHARED HEADER. DO NOT CHANGE CODING STYLE!!! * - + *** FEP Version 5 dependent definitions / @@ -31,7 +31,7 @@ #define START 0x0004L /* Execution start address */ #define CMDBUF 0x0d10L /* Command (cm_t) structure offset */ -#define CMDSTART0x0400L /* Start of command buffer */ +#define CMDSTART0x0400L /* Start of command buffer */ #define CMDMAX 0x0800L /* End of command buffer*/ #define EVBUF 0x0d18L /* Event (ev_t) structure */ @@ -51,7 +51,7 @@ #define FEPSTAT POSTAREA/* OS here when FEP comes up*/ #define NCHAN 0x0C02L /* number of ports FEP sees */ #define PANIC 0x0C10L /* PANIC area for FEP */ -#define KMEMEM 0x0C30L /* Memory for KME use */ +#define KMEMEM 0x0C30L /* Memory for KME use */ #define CONFIG 0x0CD0L /* Concentrator configuration info */ #define CONFIGSIZE 0x0030 /* configuration info size */ #define DOWNREQ 0x0D00 /* Download request buffer pointer */ @@ -67,8 +67,8 @@ */ #define FEPCLR 0x00 -#define FEPMEM 0x02 -#define FEPRST 0x04 +#define FEPMEM 0x02 +#define FEPRST 0x04 #define FEPINT 0x08 #define FEPMASK 0x0e #define FEPWIN 0x80 @@ -79,13 +79,13 @@ #define FEPTIMEOUT 20 #define ENABLE_INTR0x0e04 /* Enable interrupts flag */ -#define FEPPOLL_MIN1 /* minimum of 1 millisecond */ +#define FEPPOLL_MIN1 /* minimum of 1 millisecond */ #define FEPPOLL_MAX20 /* maximum of 20 milliseconds */ -#define FEPPOLL0x0c26 /* Fep event poll interval */ +#define FEPPOLL0x0c26 /* Fep event poll interval */ #defineIALTPIN 0x0080 /* Input flag to swap DSR - DCD */ -/ +/ * Command structure definition. / struct cm_t { @@ -119,13 +119,13 @@ struct downld_t { uchar dl_data[1024]; /* Download data*/ }; -/ +/ * Per channel buffer structure - * Base Structure Entries Usage Meanings to Host * - * * - *W = read writeR = read only * - *C = changed by commands only * + * Base Structure Entries Usage Meanings to Host * + * * + *W = read writeR = read only * + *C = changed by commands only * *U = unknown (may be changed w/o notice) * / struct bs_t { @@ -138,7 +138,7 @@ struct bs_t { volatile unsigned short tx_head; /* W Tx buffer head offset */ volatile unsigned short tx_tail; /* R Tx buffer tail offset */ volatile unsigned short tx_max;/* W Tx buffer size - 1 */ - + volatile unsigned short rx_seg;/* W Rx segment */ volatile unsigned short rx_head; /* W Rx buffer head offset
[PATCH 4/8] staging: dgap: Fix trailing whitespace in dgap_driver.c
This patch fixed trailing whitespace found by checkpatch.pl in staging/dgap/dgap_driver.c Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/dgap/dgap_driver.c | 40 +++--- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/drivers/staging/dgap/dgap_driver.c b/drivers/staging/dgap/dgap_driver.c index 089d017..11dd6dd 100644 --- a/drivers/staging/dgap/dgap_driver.c +++ b/drivers/staging/dgap/dgap_driver.c @@ -6,12 +6,12 @@ * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2, or (at your option) * any later version. - * + * * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the - * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR + * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the + * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR * PURPOSE. See the GNU General Public License for more details. - * + * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. @@ -201,11 +201,11 @@ char *dgap_state_text[] = { Need Device Creation, Requested Device Creation, Finished Device Creation, - Need BIOS Load, - Requested BIOS, + Need BIOS Load, + Requested BIOS, Doing BIOS Load, Finished BIOS Load, - Need FEP Load, + Need FEP Load, Requested FEP, Doing FEP Load, Finished FEP Load, @@ -269,7 +269,7 @@ int dgap_init_module(void) else { dgap_create_driver_sysfiles(dgap_driver); } - + DPR_INIT((Finished init_module. Returning %d\n, rc)); return (rc); } @@ -326,7 +326,7 @@ static int dgap_start(void) if (rc 0) { APR((tty preinit - not enough memory (%d)\n, rc)); - return(rc); + return(rc); } /* Start the poller */ @@ -366,7 +366,7 @@ static int dgap_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) if (rc 0) { rc = -EIO; - } else { + } else { rc = dgap_probe1(pdev, ent-driver_data); if (rc == 0) { dgap_NumBoards++; @@ -374,15 +374,15 @@ static int dgap_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) } } return rc; -} +} static int dgap_probe1(struct pci_dev *pdev, int card_type) { return dgap_found_board(pdev, card_type); } - - + + static void dgap_remove_one(struct pci_dev *dev) { /* Do Nothing */ @@ -599,7 +599,7 @@ static int dgap_found_board(struct pci_dev *pdev, int id) pci_write_config_byte(pdev, 0x40, 0); pci_write_config_byte(pdev, 0x46, 0); - /* Limit burst length to 2 doubleword transactions */ + /* Limit burst length to 2 doubleword transactions */ pci_write_config_byte(pdev, 0x42, 1); /* @@ -718,23 +718,23 @@ static int dgap_do_remap(struct board_t *brd) /* * * Function: -* +* *dgap_poll_handler * * Author: * *Scott H Kilau -* +* * Parameters: * -*dummy -- ignored +*dummy -- ignored * * Return Values: * *none * -* Description: -* +* Description: +* *As each timer expires, it determines (a) whether the transmit *waiter needs to be woken up, and (b) whether the poller needs to *be rescheduled. @@ -910,7 +910,7 @@ static void dgap_init_globals(void) dgap_Board[i] = NULL; } - init_timer( dgap_poll_timer ); + init_timer( dgap_poll_timer ); init_waitqueue_head(dgap_dl_wait); dgap_dl_action = 0; -- 1.8.5.2.309.ga25014b ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/8] staging: dgap: Fix trailing whitespace in dgap_driver.h
This patch fixed trailing whitespace found by checkpatch.pl in staging/dgap/dgap_driver.h Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/dgap/dgap_driver.h | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/staging/dgap/dgap_driver.h b/drivers/staging/dgap/dgap_driver.h index 2f7a55a7..48d2815 100644 --- a/drivers/staging/dgap/dgap_driver.h +++ b/drivers/staging/dgap/dgap_driver.h @@ -58,7 +58,7 @@ #definePROCSTR dgap /* /proc entries */ #defineDEVSTR /dev/dg/dgap /* /dev entries */ -#defineDRVSTR dgap /* Driver name string +#defineDRVSTR dgap /* Driver name string * displayed by APR */ #defineAPR(args) do { PRINTF_TO_KMEM(args); printk(DRVSTR: ); printk args; \ } while (0) @@ -112,7 +112,7 @@ #endif #if defined TRC_TO_KMEM -#define PRINTF_TO_KMEM(args) dgap_tracef args +#define PRINTF_TO_KMEM(args) dgap_tracef args #else //!defined TRC_TO_KMEM #define PRINTF_TO_KMEM(args) #endif @@ -192,7 +192,7 @@ * Our major for the mgmt devices. * * We can use 22, because Digi was allocated 22 and 23 for the epca driver. - * 22 has now become obsolete now that the cu devices have + * 22 has now become obsolete now that the cu devices have * been removed from 2.6. * Also, this *IS* the epca driver, just PCI only now. */ @@ -290,7 +290,7 @@ extern char *dgap_state_text[]; extern char *dgap_driver_state_text[]; -/* +/* * Modem line constants are defined as macros because DSR and * DCD are swapable using the ditty altpin option. */ @@ -322,7 +322,7 @@ struct macounter }; -/ +/ * Device flag definitions for bd_flags. / #defineBD_FEP5PLUS 0x0001 /* Supports FEP5 Plus commands */ @@ -340,7 +340,7 @@ struct board_t int type; /* Type of board */ char*name; /* Product Name */ - struct pci_dev *pdev; /* Pointer to the pci_dev struct */ + struct pci_dev *pdev; /* Pointer to the pci_dev struct */ u16 vendor; /* PCI vendor ID */ u16 device; /* PCI device ID */ u16 subvendor; /* PCI subsystem vendor ID */ @@ -419,7 +419,7 @@ struct board_t -/ +/ * Unit flag definitions for un_flags. / #define UN_ISOPEN 0x0001 /* Device is open */ @@ -439,7 +439,7 @@ struct board_t struct device; / - * Structure for terminal or printer unit. + * Structure for terminal or printer unit. / struct un_t { int magic; /* Unit Magic Number. */ @@ -457,7 +457,7 @@ struct un_t { }; -/ +/ * Device flag definitions for ch_flags. / #define CH_PRON 0x0001 /* Printer on string*/ @@ -484,7 +484,7 @@ struct un_t { #define SNIFF_WAIT_SPACE 0x4 -/ +/ * Channel information structure. / struct channel_t { @@ -608,7 +608,7 @@ extern int dgap_registerttyswithsysfs; /* Should we register the */ * Global functions declared in dgap_fep5.c, but must be hidden from * user space programs. */ -extern voiddgap_poll_tasklet(unsigned long data); +extern voiddgap_poll_tasklet(unsigned long data); extern voiddgap_cmdb(struct channel_t *ch, uchar cmd, uchar byte1, uchar byte2, uint ncmds); extern voiddgap_cmdw(struct channel_t *ch, uchar cmd, u16 word, uint ncmds); extern voiddgap_wmove(struct channel_t *ch, char *buf, uint cnt); -- 1.8.5.2.309.ga25014b ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/8] staging: dgap: Fix trailing whitespace in dgap_sysfs.c
This patch fixed trailing whitespace found by checkpatch.pl in dgap/dgap_sysfs.c Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/dgap/dgap_sysfs.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/dgap/dgap_sysfs.c b/drivers/staging/dgap/dgap_sysfs.c index 7f4ec9a..aa7e36f 100644 --- a/drivers/staging/dgap/dgap_sysfs.c +++ b/drivers/staging/dgap/dgap_sysfs.c @@ -1,7 +1,7 @@ /* * Copyright 2004 Digi International (www.digi.com) * Scott H Kilau Scott_Kilau at digi dot com - * + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2, or (at your option) @@ -9,14 +9,14 @@ * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the - * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR + * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR * PURPOSE. See the GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. * - * + * * NOTE TO LINUX KERNEL HACKERS: DO NOT REFORMAT THIS CODE! * * This is shared code between Digi's CVS archive and the @@ -28,8 +28,8 @@ * Thank you. * * - * - * $Id: dgap_sysfs.c,v 1.1 2009/10/23 14:01:57 markh Exp $ + * + * $Id: dgap_sysfs.c,v 1.1 2009/10/23 14:01:57 markh Exp $ */ @@ -41,7 +41,7 @@ #include linux/device.h #include linux/pci.h #include linux/kdev_t.h - + #include dgap_driver.h #include dgap_conf.h #include dgap_parse.h @@ -130,7 +130,7 @@ void dgap_create_driver_sysfiles(struct pci_driver *dgap_driver) rc |= driver_create_file(driverfs, driver_attr_boards); rc |= driver_create_file(driverfs, driver_attr_maxboards); rc |= driver_create_file(driverfs, driver_attr_debug); - rc |= driver_create_file(driverfs, driver_attr_rawreadok); + rc |= driver_create_file(driverfs, driver_attr_rawreadok); rc |= driver_create_file(driverfs, driver_attr_pollrate); rc |= driver_create_file(driverfs, driver_attr_pollcounter); rc |= driver_create_file(driverfs, driver_attr_state); -- 1.8.5.2.309.ga25014b ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/8] staging: dgap: Fix trailing whitespace in digi.h
This patch fixed trailing whitespace error found by checkpatch.pl in dgap/digi.h Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/dgap/digi.h | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/staging/dgap/digi.h b/drivers/staging/dgap/digi.h index bcea4f7..fe87903 100644 --- a/drivers/staging/dgap/digi.h +++ b/drivers/staging/dgap/digi.h @@ -43,7 +43,7 @@ #defineTIOCMODG('d'8) | 250 /* get modem ctrl state */ #defineTIOCMODS('d'8) | 251 /* set modem ctrl state */ -#ifndef TIOCM_LE +#ifndef TIOCM_LE #defineTIOCM_LE0x01/* line enable */ #defineTIOCM_DTR 0x02/* data terminal ready */ #defineTIOCM_RTS 0x04/* request to send */ @@ -122,7 +122,7 @@ struct digiflow_t { #endif / - * Values for digi_flags + * Values for digi_flags / #define DIGI_IXON 0x0001 /* Handle IXON in the FEP */ #define DIGI_FAST 0x0002 /* Fast baud rates */ @@ -193,8 +193,8 @@ struct rw_t { #define COMXI_TYPE 5 /* Board type at the designated port is a COM/Xi */ /* Non-Zero Result codes. */ -#define RESULT_NOBDFND 1 /* A Digi product at that port is not config installed */ -#define RESULT_NODESCT 2 /* A memory descriptor was not obtainable */ +#define RESULT_NOBDFND 1 /* A Digi product at that port is not config installed */ +#define RESULT_NODESCT 2 /* A memory descriptor was not obtainable */ #define RESULT_NOOSSIG 3 /* FEP/OS signature was not detected on the board */ #define RESULT_TOOSML 4 /* Too small an area to shrink. */ #define RESULT_NOCHAN 5 /* Channel structure for the board was not found */ @@ -205,7 +205,7 @@ struct shrink_buf_struct { unsigned long shrink_buf_bseg;/* Amount of board memory */ unsigned long shrink_buf_hseg;/* '186 Beginning of Dual-Port */ - unsigned long shrink_buf_lseg;/* '186 Beginning of freed memory */ + unsigned long shrink_buf_lseg;/* '186 Beginning of freed memory */ unsigned long shrink_buf_mseg;/* Linear address from start of dual-port were freed memory begins, host viewpoint. */ @@ -220,18 +220,18 @@ struct shrink_buf_struct { unsigned char shrink_buf_result; /* Reason for call failing Zero is Good return */ - unsigned char shrink_buf_init;/* Non-Zero if it caused an + unsigned char shrink_buf_init;/* Non-Zero if it caused an xxinit call. */ unsigned char shrink_buf_anports; /* Number of async ports */ unsigned char shrink_buf_snports; /* Number of sync ports */ unsigned char shrink_buf_type;/* Board type 1 = PC/Xi, 2 = PC/Xm, - 3 = PC/Xe - 4 = MC/Xi + 3 = PC/Xe + 4 = MC/Xi 5 = COMX/i */ unsigned char shrink_buf_card;/* Card number */ - + }; / @@ -244,7 +244,7 @@ struct digi_dinfo { }; #defineDIGI_GETDD ('d'8) | 248 /* get driver info */ - + / * Structure used with ioctl commands for per-board information * @@ -264,7 +264,7 @@ struct digi_info { }; #defineDIGI_GETBD ('d'8) | 249 /* get board info */ - + struct digi_stat { unsigned intinfo_chan; /* Channel number (0 based) */ unsigned intinfo_brd; /* Board number (0 based) */ @@ -299,7 +299,7 @@ struct digi_ch { }; /* -* This structure is used with the DIGI_FEPCMD ioctl to +* This structure is used with the DIGI_FEPCMD ioctl to * tell the driver which port to send the command for. */ struct digi_cmd { -- 1.8.5.2.309.ga25014b ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/7] staging,spear_adc: Add dependency on HAS_IOMEM
On archs like S390 or um this driver cannot build nor work. Make it depend on HAS_IOMEM to bypass build failures. drivers/staging/iio/adc/spear_adc.c: In function ‘spear_adc_probe’: drivers/staging/iio/adc/spear_adc.c:393:2: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/staging/iio/adc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig index e3d6430..7d5d675 100644 --- a/drivers/staging/iio/adc/Kconfig +++ b/drivers/staging/iio/adc/Kconfig @@ -128,6 +128,7 @@ config MXS_LRADC config SPEAR_ADC tristate ST SPEAr ADC depends on PLAT_SPEAR || COMPILE_TEST + depends on HAS_IOMEM help Say yes here to build support for the integrated ADC inside the ST SPEAr SoC. Provides direct access via sysfs. -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] phy,exynos: Add dependency on HAS_IOMEM
On archs like S390 or um this driver cannot build nor work. Make it depend on HAS_IOMEM to bypass build failures. drivers/built-in.o: In function `exynos_mipi_video_phy_probe': drivers/phy/phy-exynos-mipi-video.c:130: undefined reference to `devm_ioremap_resource' Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/phy/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 330ef2d..a8b17ce 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -16,6 +16,7 @@ config GENERIC_PHY framework should select this config. config PHY_EXYNOS_MIPI_VIDEO + depends on HAS_IOMEM tristate S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver help Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] ptp_pch: Add dependency on HAS_IOMEM
On archs like S390 or um this driver cannot build nor work. Make it depend on HAS_IOMEM to bypass build failures. drivers/ptp/ptp_pch.c: In function ‘pch_remove’: drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] drivers/ptp/ptp_pch.c: In function ‘pch_probe’: drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/ptp/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig index 5be73ba..5a7910e 100644 --- a/drivers/ptp/Kconfig +++ b/drivers/ptp/Kconfig @@ -73,6 +73,7 @@ config DP83640_PHY config PTP_1588_CLOCK_PCH tristate Intel PCH EG20T as PTP clock depends on X86 || COMPILE_TEST + depends on HAS_IOMEM select PTP_1588_CLOCK help This driver adds support for using the PCH EG20T as a PTP -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/7] staging,dgap: Add dependency on HAS_IOMEM
On archs like S390 or um this driver cannot build nor work. Make it depend on HAS_IOMEM to bypass build failures. drivers/staging/dgap/dgap_driver.c: In function ‘dgap_cleanup_board’: drivers/staging/dgap/dgap_driver.c:457:3: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] drivers/staging/dgap/dgap_driver.c: In function ‘dgap_do_remap’: drivers/staging/dgap/dgap_driver.c:694:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/staging/dgap/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/dgap/Kconfig b/drivers/staging/dgap/Kconfig index 31f1d75..3bbe9e1 100644 --- a/drivers/staging/dgap/Kconfig +++ b/drivers/staging/dgap/Kconfig @@ -1,6 +1,6 @@ config DGAP tristate Digi EPCA PCI products default n - depends on TTY + depends on TTY HAS_IOMEM ---help--- Driver for the Digi International EPCA PCI based product line -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/7] net,marvell: Add dependency on HAS_IOMEM
On archs like S390 or um this driver cannot build nor work. Make it depend on HAS_IOMEM to bypass build failures. drivers/built-in.o: In function `orion_mdio_probe': drivers/net/ethernet/marvell/mvmdio.c:228: undefined reference to `devm_ioremap' Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/net/ethernet/marvell/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig index a49e81b..6300fd2 100644 --- a/drivers/net/ethernet/marvell/Kconfig +++ b/drivers/net/ethernet/marvell/Kconfig @@ -33,6 +33,7 @@ config MV643XX_ETH config MVMDIO tristate Marvell MDIO interface support + depends on HAS_IOMEM select PHYLIB ---help--- This driver supports the MDIO interface found in the network -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/8] staging: dgap: Fix trailing whitespace in dgap_parse.c
Thsi patch fixed trailing whitespace found by checkpatch.pl in dgap/dgap_parse.c Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/dgap/dgap_parse.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/dgap/dgap_parse.c b/drivers/staging/dgap/dgap_parse.c index 36fd93d..7bc5bc3 100644 --- a/drivers/staging/dgap/dgap_parse.c +++ b/drivers/staging/dgap/dgap_parse.c @@ -17,14 +17,14 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. * * - * NOTE TO LINUX KERNEL HACKERS: DO NOT REFORMAT THIS CODE! + * NOTE TO LINUX KERNEL HACKERS: DO NOT REFORMAT THIS CODE! * * This is shared code between Digi's CVS archive and the * Linux Kernel sources. * Changing the source just for reformatting needlessly breaks * our CVS diff history. * - * Send any bug fixes/changes to: Eng.Linux at digi dot com. + * Send any bug fixes/changes to: Eng.Linux at digi dot com. * Thank you. * * @@ -340,7 +340,7 @@ int dgap_parsefile(char **in, int Remove) } p-u.board.v_pcislot = 1; - DPR_INIT((Adding PCIINFO (%s %s) to config...\n, p-u.board.pcibusstr, + DPR_INIT((Adding PCIINFO (%s %s) to config...\n, p-u.board.pcibusstr, p-u.board.pcislotstr)); break; @@ -914,7 +914,7 @@ static char *dgap_sindex (char *string, char *group) if (!string || !group) return (char *) NULL; - if (*group == '^') { + if (*group == '^') { group++; for (; *string; string++) { for (ptr = group; *ptr; ptr++) { @@ -924,7 +924,7 @@ static char *dgap_sindex (char *string, char *group) if (*ptr == '\0') return string; } - } + } else { for (; *string; string++) { for (ptr = group; *ptr; ptr++) { @@ -945,14 +945,14 @@ static int dgap_gettok(char **in, struct cnode *p) { char*w; struct toklist *t; - + if (strstr(dgap_cword, boar)) { w = dgap_getword(in); snprintf(dgap_cword, MAXCWORD, %s, w); for (t = dgap_tlist; t-token != 0; t++) { if ( !strcmp(w, t-string)) { return(t-token); - } + } } dgap_err(board !!type not specified); return(1); @@ -1152,7 +1152,7 @@ uint dgap_config_get_altpin(struct board_t *bd) /* - * Given a specific type of board, if found, detached link and + * Given a specific type of board, if found, detached link and * returns the first occurrence in the list. */ struct cnode *dgap_find_config(int type, int bus, int slot) -- 1.8.5.2.309.ga25014b ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/7] power,goldfish: Add dependency on HAS_IOMEM
On archs like S390 or um this driver cannot build nor work. Make it depend on HAS_IOMEM to bypass build failures. drivers/built-in.o: In function `goldfish_battery_probe': drivers/power/goldfish_battery.c:181: undefined reference to `devm_ioremap' Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/power/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 85ad58c..32c6294 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -377,6 +377,7 @@ config AB8500_BM config BATTERY_GOLDFISH tristate Goldfish battery driver depends on GOLDFISH || COMPILE_TEST + depends on HAS_IOMEM help Say Y to enable support for the battery and AC power in the Goldfish emulator. -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/7] staging,lpc32xx_adc: Add dependency on HAS_IOMEM
On archs like S390 or um this driver cannot build nor work. Make it depend on HAS_IOMEM to bypass build failures. drivers/built-in.o: In function `lpc32xx_adc_probe': drivers/staging/iio/adc/lpc32xx_adc.c:149: undefined reference to `devm_ioremap' Signed-off-by: Richard Weinberger rich...@nod.at --- drivers/staging/iio/adc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig index 7d5d675..3633298 100644 --- a/drivers/staging/iio/adc/Kconfig +++ b/drivers/staging/iio/adc/Kconfig @@ -103,6 +103,7 @@ config AD7280 config LPC32XX_ADC tristate NXP LPC32XX ADC depends on ARCH_LPC32XX || COMPILE_TEST + depends on HAS_IOMEM help Say yes here to build support for the integrated ADC inside the LPC32XX SoC. Note that this feature uses the same hardware as the -- 1.8.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: vme_tsi148 question
On Tue, Jan 14, 2014 at 1:39 AM, Martyn Welch martyn.we...@ge.com wrote: On 14/01/14 04:07, Michael Kenney wrote: Unfortunately, the results are the same. I'm running this from home so I'll have to confirm with the logic-analyzer tomorrow but the bridge driver error messages suggest the same D8 writes (which for some reason our A/D board does not like). The hardware probably doesn't support 8-bit reads/writes, from memory, it doesn't have to according to the spec. Looks like I was wrong about where the Bus Errors are occurring. They are actually being generated on the reads from the board. Both reads and writes are being done as D8 but only the reads result in an error from the slave... Here's a dmesg excerpt: [ 5631.810902] vme_tsi148 :02:02.0: VME Bus Error at address: 0x508000, attributes: 8008b900 [ 5631.810972] vme_tsi148 :02:02.0: VME Bus Exception Overflow Occurred [ 5631.810977] vme_tsi148 :02:02.0: VME Bus Error at address: 0x508001, attributes: c0087900 [ 5631.810991] vme_tsi148 :02:02.0: VME Bus Error at address: 0x508003, attributes: 80087900 [ 5631.811061] vme_tsi148 :02:02.0: VME Bus Exception Overflow Occurred The register that I am trying to write occupies 4 bytes starting at 0x508000. I have verified that the value I am trying to write to the register is 4-byte aligned. I'm essentially doing the following: uint32_t value; pwrite(window_fd, value, sizeof(value), 0x508000); How have you setup the window? The offset is relative to the window you have configured, so if that doesn't start at 0x0, then you aren't writing where you think you are. In this case, I have setup the window starting at 0 and spanning the entire A24 address space. We have more boards than windows so we share, each board's offset is its VME bus address. Window is configured for A24 and D32. Does the attributes value from the tsi148 provide any further clues? Not really - it gives the status of some of the VME lines when the error occurred (Address lines, data strobes), it says that there was an error, in one case it shows an exception overflow and that it happened on the VME bus. That's too bad. What's interesting is the same D8 reads/writes are occurring with the other boards in the system but not causing any problems. This whole issue might be a red-herring, I just wish I knew why the reads/write were being broken up like this... It's time to find someone familiar with the A/D board hardware. The board is 10 years old (long since discontinued) and the original manufacturer (ICS) was actually purchased by GE (small world :-) some years back. --Mike ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: your mail
On Thu, Jan 09, 2014 at 06:49:39PM +, Joe Bor?? wrote: I didn't do the changes as root, I sent them from my server as it has SMTP out. Hmm, this gives me an idea. There's nothing, I believe, that makes the root user have to have the name root except for the passwd file. Maybe I'll just rename root to walley and then use root as my normal account. If anyone tries to break into root they will just gain access to a normal account and nothing more ;-) /me goes back to hacking -- Steve ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/2] Staging: lustre: Refactor the function interval_erase_color() in /lustre/ldlm/interval_tree.c
On Jan 14, 2014, at 3:09 AM, Monam Agarwal monamagarwal...@gmail.commailto:monamagarwal...@gmail.com wrote: On Tue, Jan 14, 2014 at 1:31 PM, Xiong, Jinshan jinshan.xi...@intel.commailto:jinshan.xi...@intel.com wrote: On Jan 13, 2014, at 11:56 PM, Dilger, Andreas andreas.dil...@intel.commailto:andreas.dil...@intel.com wrote: Begin forwarded message: From: Greg KH gre...@linuxfoundation.orgmailto:gre...@linuxfoundation.org Subject: Re: [PATCH v3 1/2] Staging: lustre: Refactor the function interval_erase_color() in /lustre/ldlm/interval_tree.c Date: January 11, 2014 at 1:33:58 PM MST To: Monam Agarwal monamagarwal...@gmail.commailto:monamagarwal...@gmail.com Cc: Dan Carpenter dan.carpen...@oracle.commailto:dan.carpen...@oracle.com, de...@driverdev.osuosl.orgmailto:de...@driverdev.osuosl.org, andreas.dil...@intel.commailto:andreas.dil...@intel.com, peter.p.waskiewicz...@intel.commailto:peter.p.waskiewicz...@intel.com, linux-ker...@vger.kernel.orgmailto:linux-ker...@vger.kernel.org, Rashika Kheria rashika.khe...@gmail.commailto:rashika.khe...@gmail.com On Sat, Jan 11, 2014 at 05:14:35PM +0530, Monam Agarwal wrote: On Sat, Jan 11, 2014 at 5:09 PM, Dan Carpenter dan.carpen...@oracle.commailto:dan.carpen...@oracle.com wrote: On Sat, Jan 11, 2014 at 04:56:44PM +0530, Monam Agarwal wrote: I took n as a flag to decide whether parent-in_left == node is true or not in the called function. So n stands for node? Should I use some other name for the flag. Yes. Will flag be a suitable name? I’d suggest `bool is_right_child’. I’ve checked the patch and it looks good. There exists a unit test case for interval tree under lustre/tests/ named it_test.c, please compile it and verify your change. I am using tree from http://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/. There is no lustre/tests folder in my tree and no file named it_test.c. Kindly let me know the tree you are working from. I’m using git tree git://git.hpdd.intel.com/fs/lustre-release.githttp://git.hpdd.intel.com/?p=fs/lustre-release.git there are also some useful information about compiling and running the code: https://wiki.hpdd.intel.com/display/PUB/Testing+a+Lustre+filesystem, usually I download a patched kernel rpm so that I can save a lot of time from building the kernel myself. Go to build.whamcloud.comhttp://build.whamcloud.com and look for lustre-master, click in and choose server side package based on your distro. If you’re interested in contributing code to lustre, here are some useful links about creating the patch, review the patch, pass regression test, etc: https://wiki.hpdd.intel.com/display/PUB/Lustre+Development Jinshan Jinshan Ick, no. You don't want a flag to have to determine what the logic is for a given function. That just causes confusion and makes things really hard to read and understand over time. This whole function looks like a red/black tree, or something like that. Shouldn't we just be using the in-kernel implementation of this? And if not, then you really need to get the feedback of the code's original authors as you might be changing the algorithm in ways that could cause big problems. thanks, greg k-h Cheers, Andreas -- Andreas Dilger Lustre Software Architect Intel Corporation ___ 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 Tue, Jan 14, 2014 at 01:37:50AM -0800, Insop Song wrote: On Tue, Jan 14, 2014 at 1:18 AM, Insop Song insop.s...@gmail.com wrote: On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 04:37:32PM -0800, Insop Song wrote: On Sat, Jan 11, 2014 at 4:10 PM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 03:56:48PM -0800, Insop Song wrote: On Sat, Jan 11, 2014 at 1:05 PM, Greg KH gre...@linuxfoundation.org wrote: On Sat, Jan 11, 2014 at 12:51:28PM -0800, Greg KH wrote: On Thu, Jan 09, 2014 at 11:48:04AM -0800, Insop Song wrote: 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 This patch breaks the build on my machine, please be more careful: drivers/staging/gs_fpgaboot/gs_fpgaboot.c:30:28: fatal error: include/asm/io.h: No such file or directory #include include/asm/io.h Please fix this up, test it, and resend. Also, while you are fixing things up, please just remove the character device node from the driver, you aren't doing anything with it, so it's not needed. Thank you for the suggestion, makes a lot of sense and code cleaner. Could you take a quick look? If it is good, will send out a new patch, v5, shortly. Main change is +/* fake device for request_firmware */ +static struct platform_device*firmware_pdev; + firmware_pdev = platform_device_register_simple(fpgaboot, -1, + NULL, 0); Why do you need a platform device? Don't you have something on the hardware that describes the fact that this device is in the system, or not? Like a PCI id? DMI id? Device Tree entry? You must have something that you can trigger off of. This driver is only to download fpga firmware, not intended to keep managing the fpga device after the downloading. The device we use is not part of the device tree or PCI. But how does the system know to load it or not, automatically? It downloads the image and after that no need to be existing. Then why not have the module remove itself? The following (insmod to load firmware and rmmod remove the module) commands are triggered from user space application. They are wrapped as a script and triggered from an application. Once the firmware is loaded, then this driver's job is done. Yes, but that's not the normal way kernel drivers interact with userspace. Please don't try to create something that is different than the thousands of other drivers out there, and rely on custom userspace scripts. Let's use the infrastructure we already have in place for doing this exact thing. Here is how we use $ insmod gs_fpga.ko file=xlinx_fpga_top_bitstream.bit $ rmmod gs_fpga Linux modules should be able to be autoloaded based on hardware present in the system, it's been that way for over a decade now. Greg, I think it's better to add more explanation for FPGA. I know what a FPGA is, thanks :) FPGA (Field-programmable gate array) is not functional until the firmware is downloaded. That means that it is not easy, if not possible, to auto detect the device. This is why insmod is manually called. That's not true, there has to be some way to detect the device is in the system, otherwise what's to prevent you from loading the module on a system that doesn't have the hardware and having bad things happen? There has to be some way to detect this device is present, right? What is it? Where is it located in the device tree description? FPGA firmware download happen ever time system boots, so this is different from occasional upgrade. Just like all other modules that use the firmware interface for their hardware :) To sum up, FPGA download occurs every boot, and before this download is done, the device in question is not functional. It's functional enough to get the data to it, so it must be operating somehow... Understood, however the use case for this driver is to load firmware, only that. The other way to do could have been use a user space program read file and access gpio (or other means) which could end up taking lot longer firmware programming time. I still needed device to use request_firmware() even though I don't use it. I've looked at other drivers which uses request_firmware() after your suggestion of removing char device node, and I've found platform_device can be a simple one to use. Please base your device on a real one in the system, like your FPGA device, surely there is some way it can be detected to be present or not? Do you still think a fake platform device doesn't cut? then what do you suggest? Use a real platform
[PATCH V2 1/1] Drivers: hv: Implement the file copy service
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: http://technet.microsoft.com/en-us/library/dn464282.aspx In V1 version of the patch I have addressed comments from Olaf Hering o...@aepfle.de and Dan Carpenter dan.carpen...@oracle.com In this version of the patch I have made globals static. Signed-off-by: K. Y. Srinivasan k...@microsoft.com --- drivers/hv/Makefile|2 +- drivers/hv/hv_fcopy.c | 422 drivers/hv/hv_util.c | 10 + include/linux/hyperv.h | 60 +++ tools/hv/hv_fcopy_daemon.c | 174 ++ 5 files changed, 667 insertions(+), 1 deletions(-) create mode 100644 drivers/hv/hv_fcopy.c create mode 100644 tools/hv/hv_fcopy_daemon.c diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile index 0a74b56..5e4dfa4 100644 --- a/drivers/hv/Makefile +++ b/drivers/hv/Makefile @@ -5,4 +5,4 @@ obj-$(CONFIG_HYPERV_BALLOON)+= hv_balloon.o hv_vmbus-y := vmbus_drv.o \ hv.o connection.o channel.o \ channel_mgmt.o ring_buffer.o -hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o +hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c new file mode 100644 index 000..f108a6e --- /dev/null +++ b/drivers/hv/hv_fcopy.c @@ -0,0 +1,422 @@ +/* + * An implementation of file copy service. + * + * Copyright (C) 2014, Microsoft, Inc. + * + * Author : K. Y. Srinivasan ksriniva...@novell.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or + * NON INFRINGEMENT. See the GNU General Public License for more + * details. + * + */ + +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + +#include linux/semaphore.h +#include linux/fs.h +#include linux/nls.h +#include linux/workqueue.h +#include linux/cdev.h +#include linux/hyperv.h +#include linux/sched.h +#include linux/uaccess.h + +#define WIN8_SRV_MAJOR 1 +#define WIN8_SRV_MINOR 1 +#define WIN8_SRV_VERSION (WIN8_SRV_MAJOR 16 | WIN8_SRV_MINOR) + +/* + * Global state maintained for transaction that is being processed. + * For a class of integration services, including the file copy service, + * the specified protocol is a request/response protocol which means that + * there can only be single outstanding transaction from the host at any + * given point in time. We use this to simplify memory management in this + * driver - we cache and process only one message at a time. + * + * While the request/response protocol is guaranteed by the host, we further + * ensure this by serializing packet processing in this driver - we do not + * read additional packets from the VMBUs until the current packet is fully + * handled. + * + * The transaction active state is set when we receive a request from the + * host and we cleanup this state when the transaction is completed - when we + * respond to the host with our response. When the transaction active state is + * set, we defer handling incoming packets. + */ + +static struct { + bool active; /* transaction status - active or not */ + int recv_len; /* number of bytes received. */ + struct hv_fcopy_hdr *fcopy_msg; /* current message */ + struct hv_start_fcopy message; /* sent to daemon */ + struct vmbus_channel *recv_channel; /* chn we got the request */ + u64 recv_req_id; /* request ID. */ + void *fcopy_context; /* for the channel callback */ + struct semaphore read_sema; +} fcopy_transaction; + +static dev_t fcopy_dev; +static bool daemon_died; +static bool opened; /* currently device opened */ +static struct task_struct *dtp; /* daemon task ptr */ + +/* + * Before we can accept copy messages from the host, we need + * to handshake with the user level daemon. This state tracks + * if we are in the handshake phase. + */ +static bool in_hand_shake = true; +static void fcopy_send_data(void); +static void fcopy_respond_to_host(int error); +static void fcopy_work_func(struct work_struct *dummy); +static DECLARE_DELAYED_WORK(fcopy_work, fcopy_work_func); +static u8 *recv_buffer; + +static void fcopy_work_func(struct work_struct *dummy) +{ + /* +* If the timer fires, the user-mode component has not responded; +* process the pending transaction. +*/ + fcopy_respond_to_host(HV_E_FAIL); +} + +static void fcopy_handle_handshake(void) +{ + pr_info(FCP: user-mode registering
[PATCH v2] Staging: comedi: convert while loop to timeout in ni_mio_common.c.
This patch to ni_mio_common.c changes a while loop to a timeout for loop, which is preferred. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- I know Mr. Abbott mentioned that he wouldn't expect clean-up patches to have to deal with this sort of thing, but I thought I'd at least give the timeout thing a try. Feel free to disregard this and take v1 of this patch instead if you would just like the simple clean-up. That being said, I used 1 iterations at the suggestion of Mr. Abbott, and a short delay as well. Let me know if these values seem incorrect. Also, at some (but not all) locations in this file that currently use timeouts, contain a check for i == timeout, with a call to either printk or comedi_error if the operation actually times out. Would something like that be required here? Sorry for all of the questions, but I'm sort of new around here and I'd like to help out with more than just clean-ups and this seemed like a good opportunity to at least try! 2: Changed from simple clean-up to swapping a timeout in for a while loop. drivers/staging/comedi/drivers/ni_mio_common.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 457b884..05cd5ed 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -687,12 +687,19 @@ static void ni_clear_ai_fifo(struct comedi_device *dev) { const struct ni_board_struct *board = comedi_board(dev); struct ni_private *devpriv = dev-private; + static const int timeout = 1; + int i; if (board-reg_type == ni_reg_6143) { /* Flush the 6143 data FIFO */ ni_writel(0x10, AIFIFO_Control_6143); /* Flush fifo */ ni_writel(0x00, AIFIFO_Control_6143); /* Flush fifo */ - while (ni_readl(AIFIFO_Status_6143) 0x10) ; /* Wait for complete */ + /* Wait for complete */ + for (i = 0; i timeout; i++) { + if(!(ni_readl(AIFIFO_Status_6143) 0x10)) + break; + udelay(1); + } } else { devpriv-stc_writew(dev, 1, ADC_FIFO_Clear); if (board-reg_type == ni_reg_625x) { -- 1.8.4.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] Staging: comedi: convert while loop to timeout in ni_mio_common.c.
On Tue, 2014-01-14 at 13:38 -0600, Chase Southwood wrote: This patch to ni_mio_common.c changes a while loop to a timeout for loop, which is preferred. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- I know Mr. Abbott mentioned that he wouldn't expect clean-up patches to have to deal with this sort of thing, but I thought I'd at least give the timeout thing a try. Feel free to disregard this and take v1 of this patch instead if you would just like the simple clean-up. That being said, I used 1 iterations at the suggestion of Mr. Abbott, and a short delay as well. Let me know if these values seem incorrect. Also, at some (but not all) locations in this file that currently use timeouts, contain a check for i == timeout, with a call to either printk or comedi_error if the operation actually times out. Would something like that be required here? Sorry for all of the questions, but I'm sort of new around here and I'd like to help out with more than just clean-ups and this seemed like a good opportunity to at least try! 2: Changed from simple clean-up to swapping a timeout in for a while loop. drivers/staging/comedi/drivers/ni_mio_common.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 457b884..05cd5ed 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -687,12 +687,19 @@ static void ni_clear_ai_fifo(struct comedi_device *dev) { const struct ni_board_struct *board = comedi_board(dev); struct ni_private *devpriv = dev-private; + static const int timeout = 1; + int i; if (board-reg_type == ni_reg_6143) { /* Flush the 6143 data FIFO */ ni_writel(0x10, AIFIFO_Control_6143); /* Flush fifo */ ni_writel(0x00, AIFIFO_Control_6143); /* Flush fifo */ - while (ni_readl(AIFIFO_Status_6143) 0x10) ; /* Wait for complete */ + /* Wait for complete */ + for (i = 0; i timeout; i++) { + if(!(ni_readl(AIFIFO_Status_6143) 0x10)) + break; + udelay(1); + } some sort of error message would be nice too if (i = timeout) comedi_error(dev, FIFO flush timeout); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] Staging: comedi: convert while loop to timeout in ni_mio_common.c.
On Tue, Jan 14, 2014 at 01:38:46PM -0600, Chase Southwood wrote: This patch to ni_mio_common.c changes a while loop to a timeout for loop, which is preferred. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- I know Mr. Abbott mentioned that he wouldn't expect clean-up patches to have to deal with this sort of thing, but I thought I'd at least give the timeout thing a try. Feel free to disregard this and take v1 of this patch instead if you would just like the simple clean-up. That being said, I used 1 iterations at the suggestion of Mr. Abbott, and a short delay as well. Let me know if these values seem incorrect. Also, at some (but not all) locations in this file that currently use timeouts, contain a check for i == timeout, with a call to either printk or comedi_error if the operation actually times out. Would something like that be required here? Sorry for all of the questions, but I'm sort of new around here and I'd like to help out with more than just clean-ups and this seemed like a good opportunity to at least try! 2: Changed from simple clean-up to swapping a timeout in for a while loop. drivers/staging/comedi/drivers/ni_mio_common.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 457b884..05cd5ed 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -687,12 +687,19 @@ static void ni_clear_ai_fifo(struct comedi_device *dev) { const struct ni_board_struct *board = comedi_board(dev); struct ni_private *devpriv = dev-private; + static const int timeout = 1; + int i; if (board-reg_type == ni_reg_6143) { /* Flush the 6143 data FIFO */ ni_writel(0x10, AIFIFO_Control_6143); /* Flush fifo */ ni_writel(0x00, AIFIFO_Control_6143); /* Flush fifo */ - while (ni_readl(AIFIFO_Status_6143) 0x10) ; /* Wait for complete */ + /* Wait for complete */ + for (i = 0; i timeout; i++) { + if(!(ni_readl(AIFIFO_Status_6143) 0x10)) Please always run your patches through scripts/checkpatch.pl so that we don't just point out the issues that it would have told you about :) + break; + udelay(1); This is good, but you don't need a counter, you could just look at the time that has expired so far and if it exceeds a limit, then exit and error out. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: android: fix parentheses coding style issue in alarm-dev.c
Hello, 2014/1/14 Michał Kwiatkowski michaelflower...@geekingspree.com: From: Michal Kwiatkowski michaelflower...@geekingspree.com This is a patch to the alarm-dev.c file that removes parentheses which should not appear in return statement. This error was found by the checkpatch.pl tool. Signed-off-by: Michał Kwiatkowski michaelflower...@geekingspree.com -- This is like the third patch in this two week period that does the exact same with more or less success, and I recall one getting applied (I might be wrong). Anyway, this one finally looks good! :-) -- Regards, Levente Kurusa ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: android: fix parentheses coding style issue in alarm-dev.c
On Tue, Jan 14, 2014 at 10:43:04PM +0100, Levente Kurusa wrote: Hello, 2014/1/14 Michał Kwiatkowski michaelflower...@geekingspree.com: From: Michal Kwiatkowski michaelflower...@geekingspree.com This is a patch to the alarm-dev.c file that removes parentheses which should not appear in return statement. This error was found by the checkpatch.pl tool. Signed-off-by: Michał Kwiatkowski michaelflower...@geekingspree.com -- This is like the third patch in this two week period that does the exact same with more or less success, and I recall one getting applied (I might be wrong). Anyway, this one finally looks good! :-) You are right, it doesn't apply, Michal should have received my rejection notice from my patch-bot already saying this. thanks, greg k-h ___ 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 Tue, Jan 14, K. Y. Srinivasan wrote: +static ssize_t fcopy_write(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + int error = 0; + + if (count != sizeof(int)) + return 0; + + if (copy_from_user(error, buf, sizeof(int))) + return -EFAULT; + + if (in_hand_shake) { + fcopy_handle_handshake(); + return 0; + } + /* + * Register with the kernel. + */ + write(fcopy_fd, version, sizeof(int)); Shouldnt there be some version check already even in this initial implementation? What if there will be a newer daemon running on an older kernel? 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
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. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] Staging: comedi: convert while loop to timeout in ni_mio_common.c
On Tue, Jan 14, 2014 at 03:31:01PM -0600, Chase Southwood wrote: This patch to ni_mio_common.c changes a simple while loop to a timeout, which is preferred. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- I removed the extra counter variable this time. Greg, you mentioned that I could just look at the time that has expired to far, and exit and error out if that exceeds a limit. Right now I'm just tracking how many times udelay is called, since that seems to conform more to other timeouts in this file, but did you instead want me to use some time function to keep track of actual real time that expires? Please wrap your lines of text, as that's one very long line :) The best way to do this is to do: unsigned long timeout; ... timeout = jiffies + msec_to_jiffies(500); /* Or how ever long you want to wait */ do { do something... udelay(10); } while (time_before(jiffies, timeout); or switch the loop around and do: while (something hasn't happened) { if (time_after(jjiffies, timeout)) { we timed out, handle it here; break; } udelay(10); } That's much better than having to count up a number and have to look to see how long you are sleeping for to determine just exactly how long you are going to have until the loop times out, as your code did. Make sense? greg k-h ___ 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: Tuesday, January 14, 2014 2:13 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 Tue, Jan 14, K. Y. Srinivasan wrote: +static ssize_t fcopy_write(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + int error = 0; + + if (count != sizeof(int)) + return 0; + + if (copy_from_user(error, buf, sizeof(int))) + return -EFAULT; + + if (in_hand_shake) { + fcopy_handle_handshake(); + return 0; + } + /* +* Register with the kernel. +*/ + write(fcopy_fd, version, sizeof(int)); Shouldnt there be some version check already even in this initial implementation? What if there will be a newer daemon running on an older kernel? Currently, the daemon registers with the kernel passing its version number. I will make the version check real. Thanks, K. Y Olaf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] Staging: comedi: convert while loop to timeout in ni_mio_common.c
This patch for ni_mio_common.c changes out a while loop for a timeout, which is preferred. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- OK, here's another go at it. Hopefully everything looks more correct this time. Greg, I've followed the pattern you gave me, and I really appreciate all of the tips! As always, just let me know if there are still things that need adjusting (especially length of timeout, udelay, etc.). Thanks, Chase Southwood 2: Changed from simple clean-up to swapping a timeout in for a while loop. 3: Removed extra counter variable, and added error checking. 4: No longer using counter variable, using jiffies instead. drivers/staging/comedi/drivers/ni_mio_common.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 457b884..882b249 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -687,12 +687,21 @@ static void ni_clear_ai_fifo(struct comedi_device *dev) { const struct ni_board_struct *board = comedi_board(dev); struct ni_private *devpriv = dev-private; + unsigned long timeout; if (board-reg_type == ni_reg_6143) { /* Flush the 6143 data FIFO */ ni_writel(0x10, AIFIFO_Control_6143); /* Flush fifo */ ni_writel(0x00, AIFIFO_Control_6143); /* Flush fifo */ - while (ni_readl(AIFIFO_Status_6143) 0x10) ; /* Wait for complete */ + /* Wait for complete */ + timeout = jiffies + msec_to_jiffies(100); + while (ni_readl(AIFIFO_Status_6143) 0x10) { + if (time_after(jiffies, timeout)) { + comedi_error(dev, FIFO flush timeout.); + break; + } + udelay(1); + } } else { devpriv-stc_writew(dev, 1, ADC_FIFO_Clear); if (board-reg_type == ni_reg_625x) { -- 1.8.4.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] Staging: comedi: convert while loop to timeout in ni_mio_common.c
On Tue, Jan 14, 2014 at 06:23:05PM -0600, Chase Southwood wrote: This patch for ni_mio_common.c changes out a while loop for a timeout, which is preferred. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- OK, here's another go at it. Hopefully everything looks more correct this time. Greg, I've followed the pattern you gave me, and I really appreciate all of the tips! As always, just let me know if there are still things that need adjusting (especially length of timeout, udelay, etc.). Thanks, Chase Southwood 2: Changed from simple clean-up to swapping a timeout in for a while loop. 3: Removed extra counter variable, and added error checking. 4: No longer using counter variable, using jiffies instead. drivers/staging/comedi/drivers/ni_mio_common.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 457b884..882b249 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -687,12 +687,21 @@ static void ni_clear_ai_fifo(struct comedi_device *dev) { const struct ni_board_struct *board = comedi_board(dev); struct ni_private *devpriv = dev-private; + unsigned long timeout; if (board-reg_type == ni_reg_6143) { /* Flush the 6143 data FIFO */ ni_writel(0x10, AIFIFO_Control_6143); /* Flush fifo */ ni_writel(0x00, AIFIFO_Control_6143); /* Flush fifo */ - while (ni_readl(AIFIFO_Status_6143) 0x10) ; /* Wait for complete */ + /* Wait for complete */ + timeout = jiffies + msec_to_jiffies(100); + while (ni_readl(AIFIFO_Status_6143) 0x10) { + if (time_after(jiffies, timeout)) { + comedi_error(dev, FIFO flush timeout.); + break; + } + udelay(1); Sleep for at least 10, as I think that's the smallest time delay you can sleep for anyway (meaning it will be that long no matter what number you put there less than 10, depending on the hardware used of course.) Other than that, looks much better. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5] Staging: comedi: convert while loop to timeout in ni_mio_common.c
This patch for ni_mio_common.c changes out a while loop for a timeout, which is preferred. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- All right, I think this guy's ready to go now! Thanks for all the help! Chase 2: Changed from simple clean-up to swapping a timeout in for a while loop. 3: Removed extra counter variable, and added error checking. 4: No longer using counter variable, using jiffies instead. 5: udelay for 10u, instead of 1u. drivers/staging/comedi/drivers/ni_mio_common.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 457b884..07e9777 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -687,12 +687,21 @@ static void ni_clear_ai_fifo(struct comedi_device *dev) { const struct ni_board_struct *board = comedi_board(dev); struct ni_private *devpriv = dev-private; + unsigned long timeout; if (board-reg_type == ni_reg_6143) { /* Flush the 6143 data FIFO */ ni_writel(0x10, AIFIFO_Control_6143); /* Flush fifo */ ni_writel(0x00, AIFIFO_Control_6143); /* Flush fifo */ - while (ni_readl(AIFIFO_Status_6143) 0x10) ; /* Wait for complete */ + /* Wait for complete */ + timeout = jiffies + msec_to_jiffies(500); + while (ni_readl(AIFIFO_Status_6143) 0x10) { + if (time_after(jiffies, timeout)) { + comedi_error(dev, FIFO flush timeout.); + break; + } + udelay(10); + } } else { devpriv-stc_writew(dev, 1, ADC_FIFO_Clear); if (board-reg_type == ni_reg_625x) { -- 1.8.4.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel