Re: [PATCH] staging: dwc2: set up all module params
On 27.11.2013, at 07:47, Gordon Hollingworth wrote: > In general it should be possible to schedule multiple split > transactions to a TT but they _must_ be completed on time or the hub > will drop the transactions on the floor... Unfortunately because of > the interrupt latency issues it cannot be guaranteed without the FIQ Maybe a bit of outside of the box thinking with limited knowledge of the finer details of USB: If the "delay" we are allowed to have is "somewhat" deterministic, then maybe use the ARM DMA to configure the USB registers after a certain delay. My experiments with a DMA only pipelined SPI driver shows that it is possible: * to schedule delays only in DMA (with an accuracy of about 2%) (note that this has some impact on available Memory bandwidth) * to configure VC registers via DMA - I am not sure if this also applies to the USB registers that are coming from an external IP. That way one could configure a max delay after which the DMA would schedule the pending USB-transfer itself. The only thing one would need to make sure is that there is no concurrent access to the registers between code and DMA. Another approach could be looking if we can get that "scheduling" part of interrupts into the VideoCore CPU, which runs RTOS. So essentially creating an interrupt engine in RTOS - even if it would only start a pre-configured DMA transfer. (but that would mean convincing lots of people to get this into firmware...) Ciao, Martin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote: > @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct > et131x_adapter *adapter) > rx_ring = &adapter->rx_ring; > > /* Alloc memory for the lookup table */ > - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); > + if (!rx_ring->fbr[0]) > + return -ENOMEM; > + > + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; > > /* The first thing we will do is configure the sizes of the buffer >* rings. These will change based on jumbo packet support. Larger I can't make myself review any further beyond this point... Please don't do this nonsense. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: ft1000: ft1000-usb: initialize 'status' with STATUS_SUCCESS in request_code_segment()
On Wed, Nov 27, 2013 at 11:01:18AM +0800, Chen Gang wrote: > If "!bool_case", it returns unexpected value instead of STATUS_SUCCESS, > so need fix it, the related warning (with allmodconfig under hexagon): > > CC [M] drivers/staging/ft1000/ft1000-usb/ft1000_download.o > drivers/staging/ft1000/ft1000-usb/ft1000_download.c: In function > 'request_code_segment': > drivers/staging/ft1000/ft1000-usb/ft1000_download.c:581:6: warning: > 'status' may be used uninitialized in this function [-Wuninitialized] > > > Signed-off-by: Chen Gang Reviewed-by: Josh Triplett > .../staging/ft1000/ft1000-usb/ft1000_download.c|2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c > b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c > index 68ded17..15f3062 100644 > --- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c > +++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c > @@ -578,7 +578,7 @@ static int request_code_segment(struct ft1000_usb > *ft1000dev, u16 **s_file, >u8 **c_file, const u8 *endpoint, bool boot_case) > { > long word_length; > - int status; > + int status = STATUS_SUCCESS; > > /*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/ > word_length = get_request_value(ft1000dev); > -- > 1.7.7.6 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: ft1000: ft1000-usb: initialize 'status' with STATUS_SUCCESS in request_code_segment()
On 11/27/2013 05:18 PM, Josh Triplett wrote: > On Wed, Nov 27, 2013 at 11:01:18AM +0800, Chen Gang wrote: >> If "!bool_case", it returns unexpected value instead of STATUS_SUCCESS, >> so need fix it, the related warning (with allmodconfig under hexagon): >> >> CC [M] drivers/staging/ft1000/ft1000-usb/ft1000_download.o >> drivers/staging/ft1000/ft1000-usb/ft1000_download.c: In function >> 'request_code_segment': >> drivers/staging/ft1000/ft1000-usb/ft1000_download.c:581:6: warning: >> 'status' may be used uninitialized in this function [-Wuninitialized] >> >> >> Signed-off-by: Chen Gang > > Reviewed-by: Josh Triplett > Thanks. :-) >> .../staging/ft1000/ft1000-usb/ft1000_download.c|2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c >> b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c >> index 68ded17..15f3062 100644 >> --- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c >> +++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c >> @@ -578,7 +578,7 @@ static int request_code_segment(struct ft1000_usb >> *ft1000dev, u16 **s_file, >> u8 **c_file, const u8 *endpoint, bool boot_case) >> { >> long word_length; >> -int status; >> +int status = STATUS_SUCCESS; >> >> /*DEBUG("FT1000:REQUEST_CODE_SEGMENT\n");i*/ >> word_length = get_request_value(ft1000dev); >> -- >> 1.7.7.6 -- Chen Gang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
Ok, yeah. This patch is the right thing. I had a couple minor style complaints. On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote: > The original code allocate rx dma memory in several dma_alloc_coherent calls, > which causes some problems: > 1. since dma_alloc_coherent allocate at least one page memory, it wastes some >memory when allocation size is smaller than one page. > 2. it causes et131x_rx_dma_memory_free as complex as > et131x_rx_dma_memory_alloc > > Instead, allocate all rx dma memory in one dma_alloc_coherent call makes less > code, > makes it easy to handle dma allocation error, and makes > et131x_rx_dma_memory_free > as simple as it could be. > > Signed-off-by: ZHAO Gang > --- > drivers/staging/et131x/et131x.c | 219 > +--- > 1 file changed, 72 insertions(+), 147 deletions(-) > > diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c > index 27da1db..409949f 100644 > --- a/drivers/staging/et131x/et131x.c > +++ b/drivers/staging/et131x/et131x.c > @@ -304,6 +304,7 @@ struct rx_ring { > u32 num_ready_recv; > > u32 num_rfd; > + u32 dma_size; > > bool unfinished_receives; > }; > @@ -2186,21 +2187,16 @@ static inline u32 bump_free_buff_ring(u32 > *free_buff_ring, u32 limit) > return tmp_free_buff_ring; > } > > -/* et131x_rx_dma_memory_alloc > - * @adapter: pointer to our private adapter structure > - * > - * Returns 0 on success and errno on failure (as defined in errno.h) > - * > - * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required, > - * and the Packet Status Ring. > - */ > static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) > { > u8 id; > u32 i, j; > - u32 bufsize; > - u32 pktstat_ringsize; > - u32 fbr_chunksize; > + u32 dma_size; > + u32 fbr_size; > + u32 pktstat_ring_size; > + u32 fbr_chunk_size; Get rid the fbr_chunk_size variable. > + dma_addr_t dma_addr; > + void *virt_addr; > struct rx_ring *rx_ring; > struct fbr_lookup *fbr; > > @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct > et131x_adapter *adapter) > rx_ring = &adapter->rx_ring; > > /* Alloc memory for the lookup table */ > - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); > + if (!rx_ring->fbr[0]) > + return -ENOMEM; > + > + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; > > /* The first thing we will do is configure the sizes of the buffer >* rings. These will change based on jumbo packet support. Larger Don't do this... > @@ -2246,48 +2245,53 @@ static int et131x_rx_dma_memory_alloc(struct > et131x_adapter *adapter) > rx_ring->fbr[1]->num_entries = 128; > } > > - adapter->rx_ring.psr_num_entries = > - adapter->rx_ring.fbr[0]->num_entries + > - adapter->rx_ring.fbr[1]->num_entries; > + rx_ring->psr_num_entries = > + rx_ring->fbr[0]->num_entries + rx_ring->fbr[1]->num_entries; > + > + dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries; > + > + pktstat_ring_size = > + sizeof(struct pkt_stat_desc) * rx_ring->psr_num_entries; > + > + dma_size += pktstat_ring_size; > + dma_size += sizeof(struct rx_status_block); > Calculate pktstat_ring_size before dma_size so it's not jumbled together. rx_ring->psr_num_entries = rx_ring->fbr[0]->num_entries + rx_ring->fbr[1]->num_entries; pktstat_ring_size = sizeof(struct pkt_stat_desc) * rx_ring->psr_num_entries; dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries; dma_size += pktstat_ring_size; dma_size += sizeof(struct rx_status_block); for (id = 0; id < NUM_FBRS; id++) dma_size += FBR_CHUNKS * rx_ring->fbr[id]->buffsize; rx_ring->dma_size = dma_size; > for (id = 0; id < NUM_FBRS; id++) { > - fbr = rx_ring->fbr[id]; > + fbr_chunk_size = FBR_CHUNKS * rx_ring->fbr[id]->buffsize; > + dma_size += fbr_chunk_size; > + } > > - /* Allocate an area of memory for Free Buffer Ring */ > - bufsize = sizeof(struct fbr_desc) * fbr->num_entries; > - fbr->ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev, > - bufsize, > - &fbr->ring_physaddr, > - GFP_KERNEL); > + rx_ring->dma_size = dma_size; > > - if (!fbr->ring_virtaddr) { > - dev_err(&adapter->pdev->dev, > -"Cannot a
Re: [PATCH 3/3] staging: et131x: fix build warning
On Wed, Nov 27, 2013 at 03:45:14PM +0800, ZHAO Gang wrote: > fix make -W1 warning: > drivers/staging/et131x//et131x.c: In function > ‘et1310_setup_device_for_multicast’: > drivers/staging/et131x//et131x.c:1055:6: warning: variable ‘pm_csr’ set but > not used [-Wunused-but-set-variable] > u32 pm_csr; > ^ > drivers/staging/et131x//et131x.c: In function > ‘et1310_setup_device_for_unicast’: > drivers/staging/et131x//et131x.c:1104:6: warning: variable ‘pm_csr’ set but > not used [-Wunused-but-set-variable] > u32 pm_csr; > ^ > drivers/staging/et131x//et131x.c: In function ‘et131x_isr_handler’: > drivers/staging/et131x//et131x.c:4009:8: warning: variable ‘pm_csr’ set but > not used [-Wunused-but-set-variable] > u32 pm_csr; > ^ Did you test these? Sometimes hardware requires stupid stuff like dummy reads... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: ced1401: ced_ioc.c: Removed whitespace
Line 633: Removed whitespace. Signed-off-by: Aldo Iljazi --- drivers/staging/ced1401/ced_ioc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ced1401/ced_ioc.c b/drivers/staging/ced1401/ced_ioc.c index 62efd74..043a932 100644 --- a/drivers/staging/ced1401/ced_ioc.c +++ b/drivers/staging/ced1401/ced_ioc.c @@ -630,7 +630,7 @@ int ClearArea(DEVICE_EXTENSION *pdx, int nArea) } spin_unlock_irq(&pdx->stagedLock); - if (pPages) { /* if we decided to release the memory */ + if (pPages) { /* if we decided to release the memory */ /* Now we must undo the pinning down of the pages. We will assume the worst and mark */ /* all the pages as dirty. Don't be tempted to move this up above as you must not be */ /* holding a spin lock to do this stuff as it is not atomic. */ -- 1.8.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()
On Wed, Nov 27, 2013 at 12:24:22PM +0800, Chen Gang wrote: > On 11/27/2013 12:03 PM, Greg KH wrote: > > On Wed, Nov 27, 2013 at 11:48:08AM +0800, Chen Gang wrote: > >> dev_*() assumes 'go' is already initialized, so need use pr_*() instead > >> of before 'go' initialized. Related warning (with allmodconfig under > >> hexagon): > >> > >> CC [M] drivers/staging/media/go7007/go7007-usb.o > >> drivers/staging/media/go7007/go7007-usb.c: In function > >> 'go7007_usb_probe': > >> drivers/staging/media/go7007/go7007-usb.c:1060:2: warning: 'go' may be > >> used uninitialized in this function [-Wuninitialized] > >> > >> Also remove useless code after 'return' statement. > > > > This should all be fixed in my staging-linus branch already, right? No > > need for this anymore from what I can tell, sorry. > > > > That's all right (in fact don't need sorry). :-) > > And excuse me, I am not quite familiar upstream kernel version merging > and branches. Is it still better/suitable/possible to sync some bug fix > patches from staging brach to next brach? next syncs with everyone once a day. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/60] staging: comedi: more printk debug cleanup
On 2013-11-26 23:41, H Hartley Sweeten wrote: There are a lot of debugging messages in the comedi drivers that are just added noise. Some of them messages are worth keeping but they should be converted to the appropriate dev_{level} format. This series is a first pass at cleaning this up. H Hartley Sweeten (60): staging: comedi: pcmmio.c: remove kcalloc() failure message staging: comedi: adv_pci1710: convert some printk's to dev_dbg() staging: comedi: dmm32at: remove final attach noise and fix return value staging: comedi: pcl812: convert some printk's to dev_dbg() staging: comedi: pcl816: convert some printk's to dev_dbg() staging: comedi: pcl818: convert some printk's to dev_dbg() staging: comedi: adq12b: remove commented out debug message staging: comedi: c6xdigio: remove commented out subdevice code staging: comedi: c6xdigio: remove commented out function trace messages staging: comedi: cb_pcimdas: remove commented out irq init code staging: comedi: cb_pcimdas: remove unnecessary printk noise staging: comedi: das16m1: tidy up the irq support in das16m1_attach() staging: comedi: dmm32at: remove unnecessary printk noise staging: comedi: dmm32at: remove commented out printk debug staging: comedi: dmm32at: tidy up the irq support in dmm32at_attach() staging: comedi: dmm32at: convert a printk(KERN_ERR to a dev->err() staging: comedi: dt2801: remove disabled code in dt2801_writedata staging: comedi: dt2801: remove commented out printk() staging: comedi: dt2801: convert printk() messages to dev_dbg() staging: comedi: dt2811: remove disabled dt2811_adtrig() function staging: comedi: dt2811: remove disabled interrupt support code staging: comedi: dt2814: remove unnecessary printk noise staging: comedi: dt2814: remove bad printk noise staging: comedi: dt2814: convert a printk(KERN_ERR into a dev_err() staging: comedi: dt2814: tidy up the irq support in dt2814_attach() staging: comedi: dt2815: convert printk's in dt2815_ao_insn() staging: comedi: dt2815: convert printk's in dt2815_attach() staging: comedi: dt282x: tidy up the irq support in dt282x_attach() staging: comedi: dt282x: remove attach noise in dt282x_grab_dma() staging: comedi: dt282x: convert printk() in dt282x_attach to a dev_err() staging: comedi: dt282x: remove unnecessary blank line message staging: comedi: dt282x: remove disable code that disables the irq staging: comedi: dt282x: convert dt282x_ao_dma_interrupt() messages staging: comedi: dt282x: convert dt282x_ai_dma_interrupt() messages staging: comedi: dt282x: convert dt282x_ao_inttrig() messages staging: comedi: dt282x: remove commented out printk staging: comedi: mpc624: remove commented out printk staging: comedi: mpc624: remove unnecessary test staging: comedi: mpc624: remove unnecessary printk noise staging: comedi: mpc624: convert printk messages in mpc624_ai_rinsn() staging: comedi: rtd520: remove commented out printk debug staging: comedi: plx9080.h: remove unnecessary printk noise staging: comedi: pcm3724: remove commented out printk debug staging: comedi: pcl818: remove commented out printk debug staging: comedi: pcl818: remove printk function trace messages staging: comedi: pcl818: remove board attach noise staging: comedi: pcl818: tidy up the irq support in pcl818_attach() staging: comedi: pcl818: convert printk() messages to dev_{level} staging: comedi: pcl816: remove commented out outb() macro staging: comedi: pcl816: remove TRIG_WAKE_EOS support stub staging: comedi: pcl816: tidy up the irq support in pcl816_attach() staging: comedi: pcl816: remove commented out printk debug staging: comedi: pcl816: convert printk messages in pcl816_attach() staging: comedi: pcl812: tidy up the irq support in pcl812_attach() staging: comedi: pcl812: convert printk messages in pcl812_attach() staging: comedi: ni_tio: remove commented out printk message staging: comedi: ni_pcimio: convert printk() to dev_dbg() staging: comedi: ni_atmio16d: remove printk() noise in atmio16d_ai_insn_read() staging: comedi: ni_atmio16d: tidy up the irq support in atmio16d_attach() staging: comedi: ni_atmio16d: remove an unnecessary printk drivers/staging/comedi/drivers/adq12b.c | 2 - drivers/staging/comedi/drivers/adv_pci1710.c | 27 +++--- drivers/staging/comedi/drivers/c6xdigio.c| 45 -- drivers/staging/comedi/drivers/cb_pcimdas.c | 14 +--- drivers/staging/comedi/drivers/das16m1.c | 78 ++--- drivers/staging/comedi/drivers/dmm32at.c | 62 +- drivers/staging/comedi/drivers/dt2801.c | 28 +++ drivers/staging/comedi/drivers/dt2811.c | 101 +- drivers/staging/comedi/drivers/dt2814.c | 40 - drivers/staging/comedi/drivers/dt2815.c | 19 +++-- drivers/staging/comedi/drivers/dt282x.c | 120 +-
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
On Wed, Nov 27, 2013 at 5:02 PM, Dan Carpenter wrote: > On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote: >> @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct >> et131x_adapter *adapter) >> rx_ring = &adapter->rx_ring; >> >> /* Alloc memory for the lookup table */ >> - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); >> - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); >> + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); >> + if (!rx_ring->fbr[0]) >> + return -ENOMEM; >> + >> + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; >> >> /* The first thing we will do is configure the sizes of the buffer >>* rings. These will change based on jumbo packet support. Larger > > I can't make myself review any further beyond this point... Please > don't do this nonsense. > I don't think it is nonsense. The original code doesn't have error check on this two kmalloc, I combine them to one so I only need to add one error check on it. > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
On Wed, Nov 27, 2013 at 07:37:30PM +0800, ZHAO Gang wrote: > On Wed, Nov 27, 2013 at 5:02 PM, Dan Carpenter > wrote: > > On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote: > >> @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct > >> et131x_adapter *adapter) > >> rx_ring = &adapter->rx_ring; > >> > >> /* Alloc memory for the lookup table */ > >> - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > >> - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > >> + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); > >> + if (!rx_ring->fbr[0]) > >> + return -ENOMEM; > >> + > >> + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; > >> > >> /* The first thing we will do is configure the sizes of the buffer > >>* rings. These will change based on jumbo packet support. Larger > > > > I can't make myself review any further beyond this point... Please > > don't do this nonsense. > > > > I don't think it is nonsense. The original code doesn't have error > check on this two kmalloc, I combine them to one so I only need to add > one error check on it. Just do two allocations and two checks unless you have benchmark numbers to show that it faster. Making the code so complicated for no reason is a wrong thing. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
On Wed, Nov 27, 2013 at 6:06 PM, Dan Carpenter wrote: > Ok, yeah. This patch is the right thing. I had a couple minor style > complaints. I am happy to hear this. > > On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote: >> The original code allocate rx dma memory in several dma_alloc_coherent calls, >> which causes some problems: >> 1. since dma_alloc_coherent allocate at least one page memory, it wastes some >>memory when allocation size is smaller than one page. >> 2. it causes et131x_rx_dma_memory_free as complex as >> et131x_rx_dma_memory_alloc >> >> Instead, allocate all rx dma memory in one dma_alloc_coherent call makes >> less code, >> makes it easy to handle dma allocation error, and makes >> et131x_rx_dma_memory_free >> as simple as it could be. >> >> Signed-off-by: ZHAO Gang >> --- >> drivers/staging/et131x/et131x.c | 219 >> +--- >> 1 file changed, 72 insertions(+), 147 deletions(-) >> >> diff --git a/drivers/staging/et131x/et131x.c >> b/drivers/staging/et131x/et131x.c >> index 27da1db..409949f 100644 >> --- a/drivers/staging/et131x/et131x.c >> +++ b/drivers/staging/et131x/et131x.c >> @@ -304,6 +304,7 @@ struct rx_ring { >> u32 num_ready_recv; >> >> u32 num_rfd; >> + u32 dma_size; >> >> bool unfinished_receives; >> }; >> @@ -2186,21 +2187,16 @@ static inline u32 bump_free_buff_ring(u32 >> *free_buff_ring, u32 limit) >> return tmp_free_buff_ring; >> } >> >> -/* et131x_rx_dma_memory_alloc >> - * @adapter: pointer to our private adapter structure >> - * >> - * Returns 0 on success and errno on failure (as defined in errno.h) >> - * >> - * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required, >> - * and the Packet Status Ring. >> - */ >> static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) >> { >> u8 id; >> u32 i, j; >> - u32 bufsize; >> - u32 pktstat_ringsize; >> - u32 fbr_chunksize; >> + u32 dma_size; >> + u32 fbr_size; >> + u32 pktstat_ring_size; >> + u32 fbr_chunk_size; > > Get rid the fbr_chunk_size variable. > I will do this change. >> + dma_addr_t dma_addr; >> + void *virt_addr; >> struct rx_ring *rx_ring; >> struct fbr_lookup *fbr; >> >> @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct >> et131x_adapter *adapter) >> rx_ring = &adapter->rx_ring; >> >> /* Alloc memory for the lookup table */ >> - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); >> - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); >> + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); >> + if (!rx_ring->fbr[0]) >> + return -ENOMEM; >> + >> + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; >> >> /* The first thing we will do is configure the sizes of the buffer >>* rings. These will change based on jumbo packet support. Larger > > Don't do this... The reason I have said in previous reply. > >> @@ -2246,48 +2245,53 @@ static int et131x_rx_dma_memory_alloc(struct >> et131x_adapter *adapter) >> rx_ring->fbr[1]->num_entries = 128; >> } >> >> - adapter->rx_ring.psr_num_entries = >> - adapter->rx_ring.fbr[0]->num_entries + >> - adapter->rx_ring.fbr[1]->num_entries; >> + rx_ring->psr_num_entries = >> + rx_ring->fbr[0]->num_entries + rx_ring->fbr[1]->num_entries; >> + >> + dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries; >> + >> + pktstat_ring_size = >> + sizeof(struct pkt_stat_desc) * rx_ring->psr_num_entries; >> + >> + dma_size += pktstat_ring_size; >> + dma_size += sizeof(struct rx_status_block); >> > > Calculate pktstat_ring_size before dma_size so it's not jumbled > together. > > > rx_ring->psr_num_entries = rx_ring->fbr[0]->num_entries + >rx_ring->fbr[1]->num_entries; > pktstat_ring_size = sizeof(struct pkt_stat_desc) * > rx_ring->psr_num_entries; > > dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries; > dma_size += pktstat_ring_size; > dma_size += sizeof(struct rx_status_block); > for (id = 0; id < NUM_FBRS; id++) > dma_size += FBR_CHUNKS * rx_ring->fbr[id]->buffsize; > > rx_ring->dma_size = dma_size; > > I will do these changes. >> for (id = 0; id < NUM_FBRS; id++) { >> - fbr = rx_ring->fbr[id]; >> + fbr_chunk_size = FBR_CHUNKS * rx_ring->fbr[id]->buffsize; >> + dma_size += fbr_chunk_size; >> + } >> >> - /* Allocate an area of memory for Free Buffer Ring */ >> - bufsize = sizeof(struct fbr_desc) * fbr->num_entries; >> - fbr->ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev, >> - bufsize, >> -
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
On Wed, Nov 27, 2013 at 08:43:57PM +0800, ZHAO Gang wrote: > >> + /* Update the pointer */ > >> + dma_addr += fbr->buffsize; > > > > Do the virt_addr update here as well. > > virt_addr += fbr->buffsize; > > > > The inner loop's aim is to record bus address, update virt_addr here > seems a noise. I still feel it's good to update virt_addr outside the > inner loop. If you do it here, you can remove the comment and it is more clear and you can remove the comment explaining how the two pointers stay in sync. Doing it here makes the code obvious. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
On Wed, Nov 27, 2013 at 8:04 PM, Dan Carpenter wrote: > On Wed, Nov 27, 2013 at 07:37:30PM +0800, ZHAO Gang wrote: >> On Wed, Nov 27, 2013 at 5:02 PM, Dan Carpenter >> wrote: >> > On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote: >> >> @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct >> >> et131x_adapter *adapter) >> >> rx_ring = &adapter->rx_ring; >> >> >> >> /* Alloc memory for the lookup table */ >> >> - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); >> >> - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); >> >> + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, >> >> GFP_KERNEL); >> >> + if (!rx_ring->fbr[0]) >> >> + return -ENOMEM; >> >> + >> >> + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; >> >> >> >> /* The first thing we will do is configure the sizes of the buffer >> >>* rings. These will change based on jumbo packet support. Larger >> > >> > I can't make myself review any further beyond this point... Please >> > don't do this nonsense. >> > >> >> I don't think it is nonsense. The original code doesn't have error >> check on this two kmalloc, I combine them to one so I only need to add >> one error check on it. > > Just do two allocations and two checks unless you have benchmark numbers > to show that it faster. Making the code so complicated for no reason is > a wrong thing. > I don't do this for benchmark, I do this for I just want to write less code. Say complication, you said it's nonsense, I think you did mean idiotic nonsense, not complicated nonsense :-) Anyway, if it must be changed, I will change it. > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: et131x: fix build warning
On Wed, Nov 27, 2013 at 6:13 PM, Dan Carpenter wrote: > On Wed, Nov 27, 2013 at 03:45:14PM +0800, ZHAO Gang wrote: >> fix make -W1 warning: >> drivers/staging/et131x//et131x.c: In function >> ‘et1310_setup_device_for_multicast’: >> drivers/staging/et131x//et131x.c:1055:6: warning: variable ‘pm_csr’ set but >> not used [-Wunused-but-set-variable] >> u32 pm_csr; >> ^ >> drivers/staging/et131x//et131x.c: In function >> ‘et1310_setup_device_for_unicast’: >> drivers/staging/et131x//et131x.c:1104:6: warning: variable ‘pm_csr’ set but >> not used [-Wunused-but-set-variable] >> u32 pm_csr; >> ^ >> drivers/staging/et131x//et131x.c: In function ‘et131x_isr_handler’: >> drivers/staging/et131x//et131x.c:4009:8: warning: variable ‘pm_csr’ set but >> not used [-Wunused-but-set-variable] >> u32 pm_csr; >> ^ > > Did you test these? Sometimes hardware requires stupid stuff like > dummy reads... > I don't have the hardware to test it. :( I looked the other part of the code, the device seems don't need a special read before write, if it does, we can change the code to just read and don't use a variable to store the read return value. > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: et131x: simplify rx dma code
On Wed, Nov 27, 2013 at 9:26 PM, Dan Carpenter wrote: > On Wed, Nov 27, 2013 at 08:43:57PM +0800, ZHAO Gang wrote: >> >> + /* Update the pointer */ >> >> + dma_addr += fbr->buffsize; >> > >> > Do the virt_addr update here as well. >> > virt_addr += fbr->buffsize; >> > >> >> The inner loop's aim is to record bus address, update virt_addr here >> seems a noise. I still feel it's good to update virt_addr outside the >> inner loop. > > If you do it here, you can remove the comment and it is more clear > and you can remove the comment explaining how the two pointers stay in > sync. Doing it here makes the code obvious. > I think I'm convinced, will do what you said. > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: et131x: fix build warning
On Wed, Nov 27, 2013 at 09:38:02PM +0800, ZHAO Gang wrote: > On Wed, Nov 27, 2013 at 6:13 PM, Dan Carpenter > wrote: > > On Wed, Nov 27, 2013 at 03:45:14PM +0800, ZHAO Gang wrote: > >> fix make -W1 warning: > >> drivers/staging/et131x//et131x.c: In function > >> ‘et1310_setup_device_for_multicast’: > >> drivers/staging/et131x//et131x.c:1055:6: warning: variable ‘pm_csr’ set > >> but not used [-Wunused-but-set-variable] > >> u32 pm_csr; > >> ^ > >> drivers/staging/et131x//et131x.c: In function > >> ‘et1310_setup_device_for_unicast’: > >> drivers/staging/et131x//et131x.c:1104:6: warning: variable ‘pm_csr’ set > >> but not used [-Wunused-but-set-variable] > >> u32 pm_csr; > >> ^ > >> drivers/staging/et131x//et131x.c: In function ‘et131x_isr_handler’: > >> drivers/staging/et131x//et131x.c:4009:8: warning: variable ‘pm_csr’ set > >> but not used [-Wunused-but-set-variable] > >> u32 pm_csr; > >> ^ > > > > Did you test these? Sometimes hardware requires stupid stuff like > > dummy reads... > > > > I don't have the hardware to test it. :( I looked the other part of > the code, the device seems don't need a special read before write, if > it does, we can change the code to just read and don't use a variable > to store the read return value. You are probably right. The original code seems like accidental. Let's just remove it. If someone complains we will add it back as a function. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: Android: Checkpatch cleanups.
alarm-dev.c: Lines 71,72: Removed parantheses since return is not a function. Signed-off-by: Aldo Iljazi --- drivers/staging/android/alarm-dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/alarm-dev.c b/drivers/staging/android/alarm-dev.c index 647694f..ccf74ee 100644 --- a/drivers/staging/android/alarm-dev.c +++ b/drivers/staging/android/alarm-dev.c @@ -68,8 +68,8 @@ static struct devalarm alarms[ANDROID_ALARM_TYPE_COUNT]; */ static int is_wakeup(enum android_alarm_type type) { - return (type == ANDROID_ALARM_RTC_WAKEUP || - type == ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP); + return type == ANDROID_ALARM_RTC_WAKEUP || + type == ANDROID_ALARM_ELAPSED_REALTIME_WAKEUP; } -- 1.8.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: staging: comedi: Fix boolean/logical and mix-up.
On Tuesday, November 26, 2013 7:57 PM, Dave Jones wrote: > Introduced in commit b3ff824a81e8. > > Signed-off-by: Dave Jones > > diff --git a/drivers/staging/comedi/drivers/pcl730.c > b/drivers/staging/comedi/drivers/pcl730.c > index d041b714db29..2baaf1db6fbf 100644 > --- a/drivers/staging/comedi/drivers/pcl730.c > +++ b/drivers/staging/comedi/drivers/pcl730.c > @@ -173,11 +173,11 @@ static int pcl730_do_insn_bits(struct comedi_device > *dev, > if (mask) { > if (mask & 0x00ff) > outb(s->state & 0xff, dev->iobase + reg); > - if ((mask & 0xff00) & (s->n_chan > 8)) > + if ((mask & 0xff00) && (s->n_chan > 8)) > outb((s->state >> 8) & 0xff, dev->iobase + reg + 1); > - if ((mask & 0xff) & (s->n_chan > 16)) > + if ((mask & 0xff) && (s->n_chan > 16)) > outb((s->state >> 16) & 0xff, dev->iobase + reg + 2); > - if ((mask & 0xff00) & (s->n_chan > 24)) > + if ((mask & 0xff00) && (s->n_chan > 24)) > outb((s->state >> 24) & 0xff, dev->iobase + reg + 3); > } My bad... Added Greg KH and the staging devel list to the CCs. Acked-by: H Hartley Sweeten ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/4] staging: et131x: simplify tx dma code
combine two dma_alloc_coherent to one Signed-off-by: ZHAO Gang --- drivers/staging/et131x/et131x.c | 70 +++-- 1 file changed, 19 insertions(+), 51 deletions(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 7c6031e..bafb85a 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -2702,12 +2702,7 @@ static void et131x_handle_recv_interrupt(struct et131x_adapter *adapter) adapter->rx_ring.unfinished_receives = false; } -/* et131x_tx_dma_memory_alloc - * @adapter: pointer to our private adapter structure - * - * Returns 0 on success and errno on failure (as defined in errno.h). - * - * Allocates memory that will be visible both to the device and to the CPU. +/* Allocates memory that will be visible both to the device and to the CPU. * The OS will pass us packets, pointers to which we will insert in the Tx * Descriptor queue. The device will read this queue to find the packets in * memory. The device will update the "status" in memory each time it xmits a @@ -2715,75 +2710,48 @@ static void et131x_handle_recv_interrupt(struct et131x_adapter *adapter) */ static int et131x_tx_dma_memory_alloc(struct et131x_adapter *adapter) { - int desc_size = 0; + int desc_size; struct tx_ring *tx_ring = &adapter->tx_ring; /* Allocate memory for the TCB's (Transmit Control Block) */ - adapter->tx_ring.tcb_ring = kcalloc(NUM_TCB, sizeof(struct tcb), - GFP_ATOMIC | GFP_DMA); - if (!adapter->tx_ring.tcb_ring) + tx_ring->tcb_ring = kcalloc(NUM_TCB, sizeof(struct tcb), GFP_KERNEL); + if (!tx_ring->tcb_ring) return -ENOMEM; desc_size = (sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX); - tx_ring->tx_desc_ring = - (struct tx_desc *)dma_alloc_coherent(&adapter->pdev->dev, -desc_size, -&tx_ring->tx_desc_ring_pa, -GFP_KERNEL); - if (!adapter->tx_ring.tx_desc_ring) { + /* Allocate dma memory for Tx descriptors and Tx status block */ + tx_ring->tx_desc_ring = dma_alloc_coherent(&adapter->pdev->dev, + desc_size + sizeof(u32), + &tx_ring->tx_desc_ring_pa, + GFP_KERNEL); + + if (!tx_ring->tx_desc_ring) { + kfree(tx_ring->tcb_ring); dev_err(&adapter->pdev->dev, "Cannot alloc memory for Tx Ring\n"); - return -ENOMEM; - } - /* Save physical address -* -* NOTE: dma_alloc_coherent(), used above to alloc DMA regions, -* ALWAYS returns SAC (32-bit) addresses. If DAC (64-bit) addresses -* are ever returned, make sure the high part is retrieved here before -* storing the adjusted address. -*/ - /* Allocate memory for the Tx status block */ - tx_ring->tx_status = dma_alloc_coherent(&adapter->pdev->dev, - sizeof(u32), - &tx_ring->tx_status_pa, - GFP_KERNEL); - if (!adapter->tx_ring.tx_status_pa) { - dev_err(&adapter->pdev->dev, - "Cannot alloc memory for Tx status block\n"); return -ENOMEM; } + + tx_ring->tx_status = (void *)tx_ring->tx_desc_ring + desc_size; + tx_ring->tx_status_pa = tx_ring->tx_desc_ring_pa + desc_size; + return 0; } -/* et131x_tx_dma_memory_free - Free all memory allocated within this module - * @adapter: pointer to our private adapter structure - * - * Returns 0 on success and errno on failure (as defined in errno.h). - */ static void et131x_tx_dma_memory_free(struct et131x_adapter *adapter) { - int desc_size = 0; + int desc_size; if (adapter->tx_ring.tx_desc_ring) { - /* Free memory relating to Tx rings here */ + /* Free Tx descriptors and Tx status block memory */ desc_size = (sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX); dma_free_coherent(&adapter->pdev->dev, - desc_size, + desc_size + sizeof(u32), adapter->tx_ring.tx_desc_ring, adapter->tx_ring.tx_desc_ring_pa); - adapter->tx_ring.tx_desc_ring = NULL; } - /* Free memory for the Tx status block */ - if (adapter->tx_ring.tx_status) { - dma_free_coherent(&adapter->pdev->dev, - sizeof(u32), -
[PATCH v2 0/4] staging: et131x: patches for et131x driver
This patch set is based on previously sent v5 of seven patches for et131x. The v2 version does all changes Dan suggested, except that I think combine two kmalloc to one is a good(at least not so bad) idea, it simplify error path in et131x_rx_dma_memory_alloc, and simplify et131x_rx_dma_memory_free: they only need to call one kfree instead of two, it reduces the lines of code. ZHAO Gang (4): staging: et131x: simplify rx dma code staging: et131x: simplify tx dma code staging: et131x: fix build warning staging: et131x: some code style change drivers/staging/et131x/et131x.c | 290 +++- 1 file changed, 81 insertions(+), 209 deletions(-) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/4] staging: et131x: simplify rx dma code
The original code allocate rx dma memory in several dma_alloc_coherent calls, which causes some problems: 1. since dma_alloc_coherent allocate at least one page memory, it wastes some memory when allocation size is smaller than one page. 2. it causes et131x_rx_dma_memory_free as complex as et131x_rx_dma_memory_alloc Instead, allocate all rx dma memory in one dma_alloc_coherent call makes less code, makes it easy to handle dma allocation error, and makes et131x_rx_dma_memory_free as simple as it could be. Signed-off-by: ZHAO Gang --- drivers/staging/et131x/et131x.c | 202 +++- 1 file changed, 56 insertions(+), 146 deletions(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 27da1db..7c6031e 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -304,6 +304,7 @@ struct rx_ring { u32 num_ready_recv; u32 num_rfd; + u32 dma_size; bool unfinished_receives; }; @@ -2186,21 +2187,15 @@ static inline u32 bump_free_buff_ring(u32 *free_buff_ring, u32 limit) return tmp_free_buff_ring; } -/* et131x_rx_dma_memory_alloc - * @adapter: pointer to our private adapter structure - * - * Returns 0 on success and errno on failure (as defined in errno.h) - * - * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required, - * and the Packet Status Ring. - */ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) { u8 id; u32 i, j; - u32 bufsize; - u32 pktstat_ringsize; - u32 fbr_chunksize; + u32 dma_size; + u32 fbr_size; + u32 pktstat_ring_size; + dma_addr_t dma_addr; + void *virt_addr; struct rx_ring *rx_ring; struct fbr_lookup *fbr; @@ -2208,8 +2203,11 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) rx_ring = &adapter->rx_ring; /* Alloc memory for the lookup table */ - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); + if (!rx_ring->fbr[0]) + return -ENOMEM; + + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; /* The first thing we will do is configure the sizes of the buffer * rings. These will change based on jumbo packet support. Larger @@ -2246,48 +2244,44 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) rx_ring->fbr[1]->num_entries = 128; } - adapter->rx_ring.psr_num_entries = - adapter->rx_ring.fbr[0]->num_entries + - adapter->rx_ring.fbr[1]->num_entries; + rx_ring->psr_num_entries = + rx_ring->fbr[0]->num_entries + rx_ring->fbr[1]->num_entries; + pktstat_ring_size = + sizeof(struct pkt_stat_desc) * rx_ring->psr_num_entries; + + dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries; + dma_size += pktstat_ring_size; + dma_size += sizeof(struct rx_status_block); for (id = 0; id < NUM_FBRS; id++) { - fbr = rx_ring->fbr[id]; + dma_size += FBR_CHUNKS * rx_ring->fbr[id]->buffsize; + } - /* Allocate an area of memory for Free Buffer Ring */ - bufsize = sizeof(struct fbr_desc) * fbr->num_entries; - fbr->ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev, - bufsize, - &fbr->ring_physaddr, - GFP_KERNEL); + rx_ring->dma_size = dma_size; - if (!fbr->ring_virtaddr) { - dev_err(&adapter->pdev->dev, - "Cannot alloc memory for Free Buffer Ring %d\n", id); - return -ENOMEM; - } + virt_addr = dma_alloc_coherent(&adapter->pdev->dev, + dma_size, + &dma_addr, + GFP_KERNEL); + if (!virt_addr) { + kfree(rx_ring->fbr[0]); + dev_err(&adapter->pdev->dev, + "Cannot allocate memory for Rx ring\n"); + return -ENOMEM; } + /* Now we distribute the pie */ for (id = 0; id < NUM_FBRS; id++) { fbr = rx_ring->fbr[id]; - fbr_chunksize = (FBR_CHUNKS * fbr->buffsize); + fbr->ring_virtaddr = virt_addr; + fbr->ring_physaddr = dma_addr; + fbr_size = sizeof(struct fbr_desc) * fbr->num_entries; + virt_addr += fbr_size; + dma_addr += fbr_size; for (i = 0; i < (fbr->num_entries / FBR_CHUNKS); i++)
[PATCH v2 4/4] staging: et131x: some code style change
Signed-off-by: ZHAO Gang --- drivers/staging/et131x/et131x.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 7305fb5..764f7d0 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -2308,7 +2308,7 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) rx_ring->rx_status_bus = dma_addr; pr_info("Packet Status Ring %llx\n", - (unsigned long long) rx_ring->ps_ring_physaddr); + (unsigned long long)rx_ring->ps_ring_physaddr); pr_info("Receive Status Ring %llx\n", (unsigned long long)rx_ring->rx_status_bus); @@ -2332,8 +2332,9 @@ static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter) WARN_ON(rx_ring->num_ready_recv != rx_ring->num_rfd); while (!list_empty(&rx_ring->recv_list)) { - rfd = (struct rfd *) list_entry(rx_ring->recv_list.next, - struct rfd, list_node); + rfd = list_entry(rx_ring->recv_list.next, +struct rfd, +list_node); list_del(&rfd->list_node); rfd->skb = NULL; @@ -2714,7 +2715,7 @@ static int et131x_tx_dma_memory_alloc(struct et131x_adapter *adapter) if (!tx_ring->tcb_ring) return -ENOMEM; - desc_size = (sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX); + desc_size = sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX; /* Allocate dma memory for Tx descriptors and Tx status block */ tx_ring->tx_desc_ring = dma_alloc_coherent(&adapter->pdev->dev, desc_size + sizeof(u32), @@ -2741,7 +2742,7 @@ static void et131x_tx_dma_memory_free(struct et131x_adapter *adapter) if (adapter->tx_ring.tx_desc_ring) { /* Free Tx descriptors and Tx status block memory */ - desc_size = (sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX); + desc_size = sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX; dma_free_coherent(&adapter->pdev->dev, desc_size + sizeof(u32), adapter->tx_ring.tx_desc_ring, -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/4] staging: et131x: fix build warning
fix make -W1 warning: drivers/staging/et131x//et131x.c: In function ‘et1310_setup_device_for_multicast’: drivers/staging/et131x//et131x.c:1055:6: warning: variable ‘pm_csr’ set but not used [-Wunused-but-set-variable] u32 pm_csr; ^ drivers/staging/et131x//et131x.c: In function ‘et1310_setup_device_for_unicast’: drivers/staging/et131x//et131x.c:1104:6: warning: variable ‘pm_csr’ set but not used [-Wunused-but-set-variable] u32 pm_csr; ^ drivers/staging/et131x//et131x.c: In function ‘et131x_isr_handler’: drivers/staging/et131x//et131x.c:4009:8: warning: variable ‘pm_csr’ set but not used [-Wunused-but-set-variable] u32 pm_csr; ^ drivers/staging/et131x//et131x.c: In function ‘et131x_multicast’: drivers/staging/et131x//et131x.c:4315:16: warning: unused variable ‘flags’ [-Wunused-variable] unsigned long flags; ^ Signed-off-by: ZHAO Gang --- drivers/staging/et131x/et131x.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index bafb85a..7305fb5 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -1052,7 +1052,6 @@ static void et1310_setup_device_for_multicast(struct et131x_adapter *adapter) u32 hash2 = 0; u32 hash3 = 0; u32 hash4 = 0; - u32 pm_csr; /* If ET131X_PACKET_TYPE_MULTICAST is specified, then we provision * the multi-cast LIST. If it is NOT specified, (and "ALL" is not @@ -1086,7 +1085,6 @@ static void et1310_setup_device_for_multicast(struct et131x_adapter *adapter) } /* Write out the new hash to the device */ - pm_csr = readl(&adapter->regs->global.pm_csr); if (!et1310_in_phy_coma(adapter)) { writel(hash1, &rxmac->multi_hash1); writel(hash2, &rxmac->multi_hash2); @@ -1101,7 +1099,6 @@ static void et1310_setup_device_for_unicast(struct et131x_adapter *adapter) u32 uni_pf1; u32 uni_pf2; u32 uni_pf3; - u32 pm_csr; /* Set up unicast packet filter reg 3 to be the first two octets of * the MAC address for both address @@ -1127,7 +1124,6 @@ static void et1310_setup_device_for_unicast(struct et131x_adapter *adapter) (adapter->addr[4] << ET_RX_UNI_PF_ADDR1_5_SHIFT) | adapter->addr[5]; - pm_csr = readl(&adapter->regs->global.pm_csr); if (!et1310_in_phy_coma(adapter)) { writel(uni_pf1, &rxmac->uni_pf_addr1); writel(uni_pf2, &rxmac->uni_pf_addr2); @@ -3991,12 +3987,10 @@ static void et131x_isr_handler(struct work_struct *work) */ if (adapter->flowcontrol == FLOW_TXONLY || adapter->flowcontrol == FLOW_BOTH) { - u32 pm_csr; /* Tell the device to send a pause packet via the back * pressure register (bp req and bp xon/xoff) */ - pm_csr = readl(&iomem->global.pm_csr); if (!et1310_in_phy_coma(adapter)) writel(3, &iomem->txmac.bp_ctrl); } @@ -4297,7 +4291,6 @@ static void et131x_multicast(struct net_device *netdev) { struct et131x_adapter *adapter = netdev_priv(netdev); int packet_filter; - unsigned long flags; struct netdev_hw_addr *ha; int i; -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: staging: comedi: Fix boolean/logical and mix-up.
On Wednesday, November 27, 2013 9:39 AM, Hartley Sweeten wrote: > On Tuesday, November 26, 2013 7:57 PM, Dave Jones wrote: >> Introduced in commit b3ff824a81e8. >> >> Signed-off-by: Dave Jones >> >> diff --git a/drivers/staging/comedi/drivers/pcl730.c >> b/drivers/staging/comedi/drivers/pcl730.c >> index d041b714db29..2baaf1db6fbf 100644 >> --- a/drivers/staging/comedi/drivers/pcl730.c >> +++ b/drivers/staging/comedi/drivers/pcl730.c >> @@ -173,11 +173,11 @@ static int pcl730_do_insn_bits(struct comedi_device >> *dev, >> if (mask) { >> if (mask & 0x00ff) >> outb(s->state & 0xff, dev->iobase + reg); >> -if ((mask & 0xff00) & (s->n_chan > 8)) >> +if ((mask & 0xff00) && (s->n_chan > 8)) >> outb((s->state >> 8) & 0xff, dev->iobase + reg + 1); >> -if ((mask & 0xff) & (s->n_chan > 16)) >> +if ((mask & 0xff) && (s->n_chan > 16)) >> outb((s->state >> 16) & 0xff, dev->iobase + reg + 2); >> -if ((mask & 0xff00) & (s->n_chan > 24)) >> +if ((mask & 0xff00) && (s->n_chan > 24)) >> outb((s->state >> 24) & 0xff, dev->iobase + reg + 3); >> } > > My bad... > > Added Greg KH and the staging devel list to the CCs. > > Acked-by: H Hartley Sweeten Dave, I just noticed that Dan Carpenter already submitted a patch that fixed this. It was merged as commit 9382c06e2d192adec090fb09ff0b699e951f88e1. Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: Fix boolean/logical and mix-up.
On Wed, Nov 27, 2013 at 04:59:17PM +, Hartley Sweeten wrote: > > Added Greg KH and the staging devel list to the CCs. > > I just noticed that Dan Carpenter already submitted a patch that fixed this. > It was merged as commit 9382c06e2d192adec090fb09ff0b699e951f88e1. Ah, I didn't look at -next. thanks, Dave ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 51/60] staging: comedi: pcl816: tidy up the irq support in pcl816_attach()
On Tuesday, November 26, 2013 4:42 PM, H Hartley Sweeten wrote: > Tidy up the code that does the request_irq(). > > Signed-off-by: H Hartley Sweeten > Cc: Ian Abbott > Cc: Greg Kroah-Hartman > --- > drivers/staging/comedi/drivers/pcl816.c | 33 > ++--- > 1 file changed, 6 insertions(+), 27 deletions(-) > > diff --git a/drivers/staging/comedi/drivers/pcl816.c > b/drivers/staging/comedi/drivers/pcl816.c > index 149a28b..ad75bb6 100644 > --- a/drivers/staging/comedi/drivers/pcl816.c > +++ b/drivers/staging/comedi/drivers/pcl816.c > @@ -872,36 +872,15 @@ static int pcl816_attach(struct comedi_device *dev, > struct comedi_devconfig *it) > if (!devpriv) > return -ENOMEM; > > - /* grab our IRQ */ > - irq = 0; > - if (board->IRQbits != 0) { /* board support IRQ */ > - irq = it->options[1]; > - if (irq) { /* we want to use IRQ */ > - if (((1 << irq) & board->IRQbits) == 0) { > - printk > - (", IRQ %u is out of allowed range, " > - "DISABLING IT", irq); > - irq = 0;/* Bad IRQ */ > - } else { > - if (request_irq(irq, interrupt_pcl816, 0, > - dev->board_name, dev)) { > - printk > - (", unable to allocate IRQ %u, " > - "DISABLING IT", irq); > - irq = 0;/* Can't use IRQ */ > - } else { > - printk(KERN_INFO ", irq=%u", irq); > - } > - } > + if ((1 << it->options[1]) & board->IRQbits) { > + ret = request_irq(board->IRQbits, interrupt_pcl816, 0, > + dev->board_name, dev); > + if (ret == 0) { > + dev->irq = irq; I just noticed a bug in this patch. The code above should be: + ret = request_irq(it->options[1], interrupt_pcl816, 0, + dev->board_name, dev); + if (ret == 0) { + dev->irq = it->options[1]; And the local variable 'irq' should have been removed. The sparse output has been a bit jacked up lately and I missed this. Greg, Can I just fix this patch (51/60) and repost it or would you prefer to take patches 1-50 and have me repost 51-60 after fixing this one? Thanks, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 51/60] staging: comedi: pcl816: tidy up the irq support in pcl816_attach()
Tidy up the code that does the request_irq(). Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- v2: fix a bug in the original patch drivers/staging/comedi/drivers/pcl816.c | 35 +++-- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/drivers/staging/comedi/drivers/pcl816.c b/drivers/staging/comedi/drivers/pcl816.c index 149a28b..5717215 100644 --- a/drivers/staging/comedi/drivers/pcl816.c +++ b/drivers/staging/comedi/drivers/pcl816.c @@ -854,7 +854,7 @@ static int pcl816_attach(struct comedi_device *dev, struct comedi_devconfig *it) const struct pcl816_board *board = comedi_board(dev); struct pcl816_private *devpriv; int ret; - unsigned int irq, dma; + unsigned int dma; unsigned long pages; /* int i; */ struct comedi_subdevice *s; @@ -872,36 +872,15 @@ static int pcl816_attach(struct comedi_device *dev, struct comedi_devconfig *it) if (!devpriv) return -ENOMEM; - /* grab our IRQ */ - irq = 0; - if (board->IRQbits != 0) { /* board support IRQ */ - irq = it->options[1]; - if (irq) { /* we want to use IRQ */ - if (((1 << irq) & board->IRQbits) == 0) { - printk - (", IRQ %u is out of allowed range, " -"DISABLING IT", irq); - irq = 0;/* Bad IRQ */ - } else { - if (request_irq(irq, interrupt_pcl816, 0, - dev->board_name, dev)) { - printk - (", unable to allocate IRQ %u, " -"DISABLING IT", irq); - irq = 0;/* Can't use IRQ */ - } else { - printk(KERN_INFO ", irq=%u", irq); - } - } + if ((1 << it->options[1]) & board->IRQbits) { + ret = request_irq(it->options[1], interrupt_pcl816, 0, + dev->board_name, dev); + if (ret == 0) { + dev->irq = it->options[1]; + devpriv->irq_free = 1; } } - dev->irq = irq; - if (irq)/* 1=we have allocated irq */ - devpriv->irq_free = 1; - else - devpriv->irq_free = 0; - devpriv->irq_blocked = 0; /* number of subdevice which use IRQ */ devpriv->int816_mode = 0; /* mode of irq */ -- 1.8.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: dwc2: set up all module params
Adding Jonathan. > From: Paul Zimmerman > Sent: Wednesday, November 27, 2013 11:34 AM > > > From: gordon.hollingwo...@gmail.com [mailto:gordon.hollingwo...@gmail.com] > > On Behalf Of Gordon > Hollingworth > > Sent: Tuesday, November 26, 2013 10:48 PM > > To: Stephen Warren > > Cc: Paul Zimmerman; Greg Kroah-Hartman; de...@driverdev.osuosl.org; > > linux-rpi- > > ker...@lists.infradead.org > > Subject: Re: [PATCH] staging: dwc2: set up all module params > > > > This looks like it is the same problem we saw in the dwc_otg driver > > with incorrect scheduling of split USB transactions to downstream > > ports behind single TT hubs (this just makes the problem worse rather > > than be the cause of anything) > > > > In general it should be possible to schedule multiple split > > transactions to a TT but they _must_ be completed on time or the hub > > will drop the transactions on the floor... Unfortunately because of > > the interrupt latency issues it cannot be guaranteed without the FIQ > > Hi Gordon, > > Any chance one of your guys could look at adding FIQ support to the > mainline kernel for the BCM2835 (Raspberry Pi) platform and the dwc2 > driver? Support for FIQs already exists in the arm tree, and is used on > other platforms. Some time back you volunteered Jonathan Bell to help > work on the dwc2 driver, perhaps he could look into this? I'm probably > not going to have the bandwidth to work on this entirely on my own, but > I could certainly try to help with any questions or issues you might run > into. > > -- > Paul ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: dwc2: set up all module params
> From: gordon.hollingwo...@gmail.com [mailto:gordon.hollingwo...@gmail.com] On > Behalf Of Gordon Hollingworth > Sent: Tuesday, November 26, 2013 10:48 PM > To: Stephen Warren > Cc: Paul Zimmerman; Greg Kroah-Hartman; de...@driverdev.osuosl.org; linux-rpi- > ker...@lists.infradead.org > Subject: Re: [PATCH] staging: dwc2: set up all module params > > This looks like it is the same problem we saw in the dwc_otg driver > with incorrect scheduling of split USB transactions to downstream > ports behind single TT hubs (this just makes the problem worse rather > than be the cause of anything) > > In general it should be possible to schedule multiple split > transactions to a TT but they _must_ be completed on time or the hub > will drop the transactions on the floor... Unfortunately because of > the interrupt latency issues it cannot be guaranteed without the FIQ Hi Gordon, Any chance one of your guys could look at adding FIQ support to the mainline kernel for the BCM2835 (Raspberry Pi) platform and the dwc2 driver? Support for FIQs already exists in the arm tree, and is used on other platforms. Some time back you volunteered Jonathan Bell to help work on the dwc2 driver, perhaps he could look into this? I'm probably not going to have the bandwidth to work on this entirely on my own, but I could certainly try to help with any questions or issues you might run into. -- Paul ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/4] staging: et131x: some code style change
On Thu, Nov 28, 2013 at 12:53:42AM +0800, ZHAO Gang wrote: > Signed-off-by: ZHAO Gang > --- > drivers/staging/et131x/et131x.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c > index 7305fb5..764f7d0 100644 > --- a/drivers/staging/et131x/et131x.c > +++ b/drivers/staging/et131x/et131x.c > @@ -2308,7 +2308,7 @@ static int et131x_rx_dma_memory_alloc(struct > et131x_adapter *adapter) > rx_ring->rx_status_bus = dma_addr; > > pr_info("Packet Status Ring %llx\n", > - (unsigned long long) rx_ring->ps_ring_physaddr); > + (unsigned long long)rx_ring->ps_ring_physaddr); > pr_info("Receive Status Ring %llx\n", > (unsigned long long)rx_ring->rx_status_bus); > Just delete these lines I meant. It's just spammy /var/log/messages for no reason. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging driver fixes for 3.13-rc2
The following changes since commit 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae: Linux 3.13-rc1 (2013-11-22 11:30:55 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ tags/staging-3.13-rc2 for you to fetch changes up to 930ba4a374b96560ef9fde2145cdc454a164ddcc: Staging: tidspbridge: disable driver (2013-11-27 09:32:49 -0800) Staging fixes for 3.13-rc2 Here are a number of staging, and IIO driver, fixes for 3.13-rc2 that resolve issues that have been reported for 3.13-rc1. All of these have been in linux-next for a bit this week. Signed-off-by: Greg Kroah-Hartman Dan Carpenter (3): Staging: comedi: pcl730: fix some bitwise vs logical AND bugs Staging: vt6655-6: potential NULL dereference in hostap_disable_hostapd() staging: nvec: potential NULL dereference on error path Frank Zago (1): iio:accel:kxsd9 fix missing mutex unlock Geert Uytterhoeven (1): Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback Greg Kroah-Hartman (4): Revert "staging:media: Use dev_dbg() instead of pr_debug()" Staging: go7007: fix up some remaining go->dev issues Merge tag 'fixes-for-3.13a' of git://git.kernel.org/.../jic23/iio into staging-linus Staging: tidspbridge: disable driver Ian Abbott (1): staging: comedi: s626: fix value written by s626_set_dac() Josh Boyer (1): staging: imx-drm: Fix modular build of DRM_IMX_IPUV3 Larry Finger (1): staging: r8188eu: Fix AP mode Lars-Peter Clausen (1): iio: adc: ti_am335x_adc: avoid double free of buffer. Malcolm Priestley (1): staging: vt6656: [BUG] Fix for TX USB resets from vendors driver. Michal Nazarewicz (2): staging: comedi: fix potentially uninitialised variable staging: ft1000: fix use of potentially uninitialized variable Olav Haugan (1): staging: zsmalloc: Ensure handle is never 0 on success Peng Tao (1): staging/lustre/ptlrpc: fix ptlrpc_stop_pinger logic Peter Meerwald (5): iio: Fix sign extension table in mcp3422 driver iio: Fix mag3110 scan_type iio: Fix mag3110 Kconfig dependencies iio: Fix tcs3472 Kconfig dependencies staging:iio: Fix hmc5843 Kconfig dependencies Rashika Kheria (1): Staging: zram: Fix memory leak by refcount mismatch Srinivas Pandruvada (1): iio: hid_Sensors: fix crash during trigger unregister Wei Yongjun (1): iio: at91: fix error return code in at91_adc_probe() drivers/iio/accel/hid-sensor-accel-3d.c| 5 ++-- drivers/iio/accel/kxsd9.c | 7 +++--- drivers/iio/adc/at91_adc.c | 1 + drivers/iio/adc/mcp3422.c | 8 +++ drivers/iio/adc/ti_am335x_adc.c| 7 -- .../iio/common/hid-sensors/hid-sensor-trigger.c| 9 --- .../iio/common/hid-sensors/hid-sensor-trigger.h| 2 +- drivers/iio/gyro/hid-sensor-gyro-3d.c | 5 ++-- drivers/iio/light/Kconfig | 2 ++ drivers/iio/light/hid-sensor-als.c | 5 ++-- drivers/iio/magnetometer/Kconfig | 2 ++ drivers/iio/magnetometer/hid-sensor-magn-3d.c | 5 ++-- drivers/iio/magnetometer/mag3110.c | 7 +- drivers/staging/btmtk_usb/btmtk_usb.c | 3 +-- drivers/staging/comedi/drivers/pcl730.c| 6 ++--- drivers/staging/comedi/drivers/s626.c | 2 +- drivers/staging/comedi/drivers/vmk80xx.c | 2 +- .../staging/ft1000/ft1000-usb/ft1000_download.c| 3 +-- drivers/staging/iio/magnetometer/Kconfig | 2 ++ drivers/staging/imx-drm/Makefile | 4 +++- drivers/staging/imx-drm/imx-drm-core.c | 1 + drivers/staging/lustre/lustre/ptlrpc/pinger.c | 4 ++-- drivers/staging/media/go7007/go7007-usb.c | 28 -- drivers/staging/nvec/nvec.c| 3 ++- drivers/staging/rtl8188eu/core/rtw_ap.c| 3 +++ drivers/staging/tidspbridge/Kconfig| 2 +- drivers/staging/vt6655/hostap.c| 3 ++- drivers/staging/vt6656/baseband.c | 11 + drivers/staging/vt6656/hostap.c| 3 ++- drivers/staging/vt6656/rndis.h | 2 ++ drivers/staging/zram/zram_drv.c| 19 +++ drivers/staging/zsmalloc/zsmalloc-main.c | 17 + include/linux/hid-sensor-hub.h | 3 +++ 33 files changed, 124 insertions(+), 62 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/4 resent] staging: et131x: some code style change
Signed-off-by: ZHAO Gang --- I now see the good point of separating code style changes from others. If your code is right, just edit code style change patch until reviewers are satisfied :-) drivers/staging/et131x/et131x.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 7305fb5..881a8df 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -2307,11 +2307,6 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) rx_ring->rx_status_block = virt_addr; rx_ring->rx_status_bus = dma_addr; - pr_info("Packet Status Ring %llx\n", - (unsigned long long) rx_ring->ps_ring_physaddr); - pr_info("Receive Status Ring %llx\n", - (unsigned long long)rx_ring->rx_status_bus); - rx_ring->num_rfd = NIC_DEFAULT_NUM_RFD; /* The RFDs are going to be put on lists later on, so initialize the * lists now. @@ -2332,8 +2327,9 @@ static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter) WARN_ON(rx_ring->num_ready_recv != rx_ring->num_rfd); while (!list_empty(&rx_ring->recv_list)) { - rfd = (struct rfd *) list_entry(rx_ring->recv_list.next, - struct rfd, list_node); + rfd = list_entry(rx_ring->recv_list.next, +struct rfd, +list_node); list_del(&rfd->list_node); rfd->skb = NULL; @@ -2714,7 +2710,7 @@ static int et131x_tx_dma_memory_alloc(struct et131x_adapter *adapter) if (!tx_ring->tcb_ring) return -ENOMEM; - desc_size = (sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX); + desc_size = sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX; /* Allocate dma memory for Tx descriptors and Tx status block */ tx_ring->tx_desc_ring = dma_alloc_coherent(&adapter->pdev->dev, desc_size + sizeof(u32), @@ -2741,7 +2737,7 @@ static void et131x_tx_dma_memory_free(struct et131x_adapter *adapter) if (adapter->tx_ring.tx_desc_ring) { /* Free Tx descriptors and Tx status block memory */ - desc_size = (sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX); + desc_size = sizeof(struct tx_desc) * NUM_DESC_PER_RING_TX; dma_free_coherent(&adapter->pdev->dev, desc_size + sizeof(u32), adapter->tx_ring.tx_desc_ring, -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] drivers: staging: media: go7007: go7007-usb.c use pr_*() instead of dev_*() before 'go' initialized in go7007_usb_probe()
On 11/27/2013 06:43 PM, Dan Carpenter wrote: > On Wed, Nov 27, 2013 at 12:24:22PM +0800, Chen Gang wrote: >> On 11/27/2013 12:03 PM, Greg KH wrote: >>> On Wed, Nov 27, 2013 at 11:48:08AM +0800, Chen Gang wrote: dev_*() assumes 'go' is already initialized, so need use pr_*() instead of before 'go' initialized. Related warning (with allmodconfig under hexagon): CC [M] drivers/staging/media/go7007/go7007-usb.o drivers/staging/media/go7007/go7007-usb.c: In function 'go7007_usb_probe': drivers/staging/media/go7007/go7007-usb.c:1060:2: warning: 'go' may be used uninitialized in this function [-Wuninitialized] Also remove useless code after 'return' statement. >>> >>> This should all be fixed in my staging-linus branch already, right? No >>> need for this anymore from what I can tell, sorry. >>> >> >> That's all right (in fact don't need sorry). :-) >> >> And excuse me, I am not quite familiar upstream kernel version merging >> and branches. Is it still better/suitable/possible to sync some bug fix >> patches from staging brach to next brach? > > next syncs with everyone once a day. > OK, thanks. :-) -- Chen Gang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH -next] staging: dwc2: fix sparse non static symbol warning
From: Wei Yongjun Fixes the following sparse warning: drivers/staging/dwc2/core.c:2672:6: warning: symbol 'dwc2_set_param_uframe_sched' was not declared. Should it be static? Signed-off-by: Wei Yongjun --- drivers/staging/dwc2/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c index c0b122a..ffc3fa7 100644 --- a/drivers/staging/dwc2/core.c +++ b/drivers/staging/dwc2/core.c @@ -2669,7 +2669,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) return 0; } -void dwc2_set_param_uframe_sched(struct dwc2_hsotg *hsotg, int val) +static void dwc2_set_param_uframe_sched(struct dwc2_hsotg *hsotg, int val) { if (DWC2_OUT_OF_BOUNDS(val, 0, 1)) { if (val >= 0) { ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/4] staging: et131x: simplify rx dma code
On Thu, Nov 28, 2013 at 12:53:39AM +0800, ZHAO Gang wrote: > @@ -2208,8 +2203,11 @@ static int et131x_rx_dma_memory_alloc(struct > et131x_adapter *adapter) > rx_ring = &adapter->rx_ring; > > /* Alloc memory for the lookup table */ > - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); > + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); > + if (!rx_ring->fbr[0]) > + return -ENOMEM; > + > + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; > This kind of double allocation is not very common in the kernel, but the ones that exist make my life more complicated. My static checker complains because it can't figure out the real "size" of fbr1. We allocate a large buffer but really we want a small buffer. That's confusing to static checkers and to people. Doing confusing things makes sense if you have benchmarks to back it up. Once you get your benchmarks then you put the confusing things in a separate function with a comment. int allocate_fbr_lookups(...) { /* Just do one allocation on the fast path */ } int free_fbr_lookups(...) { } That's how you do confusing things in the kernel. Otherwise, if you don't have the benchmarks then just do two allocations. It already annoys me when I see people do this and I don't want to encourage more people to start "simplifying" code this way. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel