RE: [PATCH] PCI: imx6:don't sleep in atomic context
-Original Message- From: Bjorn Helgaas [mailto:helg...@kernel.org] Sent: Thursday, February 18, 2016 8:39 PM To: Sharma, Sanjeev Cc: richard@freescale.com; l.st...@pengutronix.de; bhelg...@google.com; linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; David Mueller Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context On Thu, Feb 18, 2016 at 07:17:41AM +0000, Sharma, Sanjeev wrote: > -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Thursday, January 07, 2016 3:35 AM > To: Sharma, Sanjeev > Cc: richard@freescale.com; l.st...@pengutronix.de; > bhelg...@google.com; linux-...@vger.kernel.org; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > David Mueller > Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context > > Hi Sanjeev, > > On Mon, Nov 09, 2015 at 04:18:00PM +0530, Sanjeev Sharma wrote: > > If additional PCIe switch get connected between the host and the > > NIC,the kernel crashes with "BUG: > > scheduling while atomic". To handle this we need to call mdelay() > > instead of usleep_range(). > > > > For more detail please refer bugzilla.kernel.org, Bug > > 100031 > > > > Signed-off-by: Sanjeev Sharma > > Signed-off-by: David Mueller > > I'm dropping this for now because we've been kicking around the same solution > (with tweaks to the mdelay amount) since June, but no progress on the *real* > issue, which is that imx6_pcie_link_up() should never wait; it should simply > return the link status. > > I'm pretty sure the amount of time I've spent looking into this would have > been enough to make some progress on that underlying issue. > > Bjorn > > Ok ! please share the change you are planning to implement. I didn't mean I personally was going to do this. But I think Lucas stepped up and did it: https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-imx6&id=4d107d3b5a686b5834e533a00b73bf7b1cf59df7 Thanks Bjorn code look ok ! > > --- > > drivers/pci/host/pci-imx6.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/host/pci-imx6.c > > b/drivers/pci/host/pci-imx6.c index 233a196..9769b13 100644 > > --- a/drivers/pci/host/pci-imx6.c > > +++ b/drivers/pci/host/pci-imx6.c > > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp) > > * Wait a little bit, then re-check if the link finished > > * the training. > > */ > > - usleep_range(1000, 2000); > > + mdelay(1000); > > } > > /* > > * From L0, initiate MAC entry to gen2 if EP/RC supports gen2. > > -- > > 1.7.11.7 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-pci" > > in the body of a message to majord...@vger.kernel.org More majordomo > > info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" > in the body of a message to majord...@vger.kernel.org More majordomo > info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] PCI: imx6:don't sleep in atomic context
-Original Message- From: Bjorn Helgaas [mailto:helg...@kernel.org] Sent: Thursday, January 07, 2016 3:35 AM To: Sharma, Sanjeev Cc: richard@freescale.com; l.st...@pengutronix.de; bhelg...@google.com; linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; David Mueller Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context Hi Sanjeev, On Mon, Nov 09, 2015 at 04:18:00PM +0530, Sanjeev Sharma wrote: > If additional PCIe switch get connected between the host and the > NIC,the kernel crashes with "BUG: > scheduling while atomic". To handle this we need to call mdelay() > instead of usleep_range(). > > For more detail please refer bugzilla.kernel.org, Bug > 100031 > > Signed-off-by: Sanjeev Sharma > Signed-off-by: David Mueller I'm dropping this for now because we've been kicking around the same solution (with tweaks to the mdelay amount) since June, but no progress on the *real* issue, which is that imx6_pcie_link_up() should never wait; it should simply return the link status. I'm pretty sure the amount of time I've spent looking into this would have been enough to make some progress on that underlying issue. Bjorn Ok ! please share the change you are planning to implement. Sanjeev Sharma > --- > drivers/pci/host/pci-imx6.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index 233a196..9769b13 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp) >* Wait a little bit, then re-check if the link finished >* the training. >*/ > - usleep_range(1000, 2000); > + mdelay(1000); > } > /* >* From L0, initiate MAC entry to gen2 if EP/RC supports gen2. > -- > 1.7.11.7 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" > in the body of a message to majord...@vger.kernel.org More majordomo > info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] PCI: imx6:don't sleep in atomic context
On Tuesday 10 November 2015 10:35:10 Lucas Stach wrote: > Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann: > > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote: > > > > diff --git a/drivers/pci/host/pci-imx6.c > > > > b/drivers/pci/host/pci-imx6.c index 233a196..9769b13 100644 > > > > --- a/drivers/pci/host/pci-imx6.c > > > > +++ b/drivers/pci/host/pci-imx6.c > > > > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp) > > > >* Wait a little bit, then re-check if the link finished > > > >* the training. > > > >*/ > > > > - usleep_range(1000, 2000); > > > > + mdelay(1000); > > > > > > A mdelay(1000) is a whole different timescale than a usleep(1000). > > > If this patch works for you with mdelay(1) or maybe mdelay(2) I > > > would be fine with it. > > > > mdelay(1) is still a really long time to block the CPU for, on > > potentially every config space access. > > > > Everybody else just returns the link status here, which seems to be > > the better alternative. If you need to delay the startup, better > > have a msleep(1) loop in the initial probe function where you are > > allowed to sleep. > > > Yes, it's somewhere on my TODO list to rework the link-up handling > here, but as there are quite a few timing and ordering implications in > that code, this needs a good thought and a good deal of testing. So > I'm inclined to ACK the current patch to get rid of the obvious bug > and sort things out properly in a follow on patchset. Maybe use that patch with some modifications then: * add a comment to explain that this is currently called from possibly atomic context through pci_config_{read,write} and that the link state handling never belonged here. * instead of looping five times for up to 2ms each, loop 100 times around a udelay(20) to hopefully be done earlier. I was going to suggest using time_before(timeout, jiffies) as the condition to wait for, but that doesn't work if called with interrupts disabled. Arnd Shall I go ahead by changing only current patch to mdelay(1). I will also Incorporate comment #1 given by Arnd above. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM:dma-mapping: Handle DMA_BIDIRECTIONAL in _dma_page_cpu_to_dev()
On Mon, Nov 09, 2015 at 11:29:17AM +0530, sanjeev sharma wrote: > On Wed, Nov 04, 2015 at 10:39:13AM +, Will Deacon wrote: > > On Wed, Nov 04, 2015 at 03:26:48PM +0530, Sanjeev Sharma wrote: > > > _dma_page_cpu_to_dev() treat DMA_BIDIRECTIONAL similar to > > > DMA_TO_DEVICE which means that destination buffer is device > > > memory,means cpu may have written some data to source buffer and > > > data may be in cache line.For cleaner operation we need to call > > > outer_flush_range() which will clean and invalidate outer cache lines. > > > > Why isn't the clean sufficient in this case? We're mapping the buffer > > to the device, so we clean the dirty lines in the CPU caches and make > > them visible to the device. If the CPU later wants to read the buffer > > (i.e. after the device has DMA'd into it), you'll need to map the > > buffer to the CPU, which will perform the invalidation of the CPU > caches. > > Indeed. bidirectional mode is already handled prefectly well by this > code. No patches are required. > > Thanks Russell & Will for providing input. > > Let's assume , CPU don't read the buffer then there could be the problem > correct ? IMO, to handle every use case outer_flush_range can be used ? > If still it doesn't make sense to use flush on bidirectional mappings, > then > FIXME comment should be removed from the function to avoid any > Confusion. > > > > Please let me know what you think on above comment ? I still don't understand the problem that you're trying to fix. It may cause the following issue. 1.we create the buffer with cache, and in some cases, the cache may be dirty. 2.then we call the sync_for_device function with flag DMA_BIDIRECTIONAL to avoid some cache problems. 3. however __dma_page_cpu_to_dev() just see DMA_BIDIRECTIONAL the same as DMA_TO_DEVICE, which means the kernel will not invalid the cache if we use the flag DMA_BIDIRECTIONAL. 4.since the dirty cache is not invalid, the dirty content may be showed on the buffer in the future rendering. Sorry, Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM:dma-mapping: Handle DMA_BIDIRECTIONAL in _dma_page_cpu_to_dev()
-Original Message- From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk] Sent: Wednesday, November 04, 2015 4:24 PM To: Will Deacon Cc: Sharma, Sanjeev; m.szyprow...@samsung.com; linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] ARM:dma-mapping: Handle DMA_BIDIRECTIONAL in _dma_page_cpu_to_dev() On Wed, Nov 04, 2015 at 10:39:13AM +, Will Deacon wrote: > On Wed, Nov 04, 2015 at 03:26:48PM +0530, Sanjeev Sharma wrote: > > _dma_page_cpu_to_dev() treat DMA_BIDIRECTIONAL similar to > > DMA_TO_DEVICE which means that destination buffer is device > > memory,means cpu may have written some data to source buffer and > > data may be in cache line.For cleaner operation we need to call > > outer_flush_range() which will clean and invalidate outer cache lines. > > Why isn't the clean sufficient in this case? We're mapping the buffer > to the device, so we clean the dirty lines in the CPU caches and make > them visible to the device. If the CPU later wants to read the buffer > (i.e. after the device has DMA'd into it), you'll need to map the > buffer to the CPU, which will perform the invalidation of the CPU caches. Indeed. bidirectional mode is already handled prefectly well by this code. No patches are required. Thanks Russell & Will for providing input. Let's assume , CPU don't read the buffer then there could be the problem correct ? IMO, to handle every use case outer_flush_range can be used ? If still it doesn't make sense to use flush on bidirectional mappings, then FIXME comment should be removed from the function to avoid any Confusion. (I never received the original email.) -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] mtd: remove .owner field for driver using module_platform_driver
Hi Sanjeev Sharma, On 11/11/2014 03:57 PM, Sanjeev Sharma wrote: > This patch removes the .owner field for drivers which use the > platform_driver_register api because this is overriden in > _platform_driver_register. > > Signed-off-by: Sanjeev Sharma > --- > drivers/mtd/nand/ams-delta.c | 1 - > drivers/mtd/nand/atmel_nand.c| 2 -- > drivers/mtd/nand/au1550nd.c | 1 - > drivers/mtd/nand/bf5xx_nand.c| 1 - > drivers/mtd/nand/davinci_nand.c | 1 - > drivers/mtd/nand/denali_dt.c | 1 - > drivers/mtd/nand/docg4.c | 1 - > drivers/mtd/nand/fsl_elbc_nand.c | 1 - > drivers/mtd/nand/fsl_ifc_nand.c | 1 - > drivers/mtd/nand/fsl_upm.c | 1 - > drivers/mtd/nand/fsmc_nand.c | 1 - > drivers/mtd/nand/gpio.c | 1 - > drivers/mtd/nand/jz4740_nand.c | 1 - > drivers/mtd/nand/lpc32xx_mlc.c | 1 - > drivers/mtd/nand/lpc32xx_slc.c | 1 - > drivers/mtd/nand/mpc5121_nfc.c | 1 - > drivers/mtd/nand/mxc_nand.c | 1 - > drivers/mtd/nand/ndfc.c | 1 - > drivers/mtd/nand/nuc900_nand.c | 1 - > drivers/mtd/nand/omap2.c | 1 - > drivers/mtd/nand/omap_elm.c | 1 - > drivers/mtd/nand/orion_nand.c| 1 - > drivers/mtd/nand/pasemi_nand.c | 1 - > drivers/mtd/nand/plat_nand.c | 1 - > drivers/mtd/nand/s3c2410.c | 1 - > drivers/mtd/nand/sh_flctl.c | 1 - > drivers/mtd/nand/sharpsl.c | 1 - > drivers/mtd/nand/socrates_nand.c | 1 - > drivers/mtd/nand/tmio_nand.c | 1 - > drivers/mtd/nand/txx9ndfmc.c | 1 - > 30 files changed, 31 deletions(-) This removal of .owner field should be done for all the platform drivers which are using module_platform_driver(). Wolfram Sang working on this change on the all the drivers. Please see [1]. [1]:https://git.kernel.org/cgit/linux/kernel/git/wsa/linux.git/log/?h=platform/remove_owner Thanks for the information and it surely make sense to take this up in all the subsystem. Sanjeev -- Thanks and Regards, Varka Bhadram. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] staging:rtl8723au: core: Fix Warning reported by checkpatch.
-Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Thursday, November 06, 2014 9:13 PM To: Sharma, Sanjeev Cc: larry.fin...@lwfinger.net; jes.soren...@redhat.com; de...@driverdev.osuosl.org; linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:rtl8723au: core: Fix Warning reported by checkpatch. On Thu, Nov 06, 2014 at 12:06:36PM +0530, Sanjeev Sharma wrote: > This is a patch to the rtw_cmd.c file that fixes following Warning by > introducing temporary structure. > > WARNING: line over 80 characters > > Signed-off-by: Sanjeev Sharma > --- > drivers/staging/rtl8723au/core/rtw_cmd.c | 123 > +++ > 1 file changed, 60 insertions(+), 63 deletions(-) Same as the other patch, give us a hint as to the warning in the subject. This patch is Fix of Warning introduced in Previous patch while fixing " ERROR: spaces required around that '>' (ctx:WxV)".Can I mentioned dependency or hint in subject line or do we have another way to described these type of fix.(One patch introduced another Warning/Error) > > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c > b/drivers/staging/rtl8723au/core/rtw_cmd.c > index 4eaa502..6186575 100644 > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c > @@ -919,34 +919,34 @@ static void traffic_status_watchdog(struct rtw_adapter > *padapter) > u8 bHigherBusyTxTraffic = false; > struct mlme_priv *pmlmepriv = &padapter->mlmepriv; > int BusyThreshold = 100; > + struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo; > + > /* */ > /* Determine if our traffic is busy now */ > /* */ > if (check_fwstate(pmlmepriv, _FW_LINKED)) { > if (rtl8723a_BT_coexist(padapter)) > BusyThreshold = 50; > - else if (pmlmepriv->LinkDetectInfo.bBusyTraffic) > + else if (ldi->bBusyTraffic) > BusyThreshold = 75; > /* if we raise bBusyTraffic in last watchdog, using > lower threshold. */ > - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > BusyThreshold || > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > BusyThreshold) { > + if (ldi->NumRxOkInPeriod > BusyThreshold || > + ldi->NumTxOkInPeriod > BusyThreshold) { > bBusyTraffic = true; > > - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > + if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod) > bRxBusyTraffic = true; > else > bTxBusyTraffic = true; > } > > /* Higher Tx/Rx data. */ > - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > 4000 || > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > 4000) { > + if (ldi->NumRxOkInPeriod > 4000 || > + ldi->NumTxOkInPeriod > 4000) { > bHigherBusyTraffic = true; > > - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > + if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod) > bHigherBusyRxTraffic = true; > else > bHigherBusyTxTraffic = true; > @@ -955,9 +955,9 @@ static void traffic_status_watchdog(struct rtw_adapter > *padapter) > if (!rtl8723a_BT_coexist(padapter) || > !rtl8723a_BT_using_antenna_1(padapter)) { > /* check traffic for powersaving. */ > - if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod + > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) || > - pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2) > + if (((ldi->NumRxUnicastOkInPeriod + > + ldi->NumTxOkInPeriod) > 8) || > + ldi->NumRxUnicastOkInPeriod > 2) > bEnterPS = false; > else > bEnterPS = true; > @@ -971,15 +971,15 @@ static void traffic_status_watchdog(struct rtw_adapter > *padapter) > } else > LPS_Leave23a(padapter); > > - pmlmepriv->LinkDetectInfo.NumRxOkInPeriod = 0; >
RE: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.
-Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Thursday, November 06, 2014 9:11 PM To: Sharma, Sanjeev Cc: larry.fin...@lwfinger.net; jes.soren...@redhat.com; de...@driverdev.osuosl.org; linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch. Please specify the "error" in the subject in some way. On Thu, Nov 06, 2014 at 11:46:13AM +0530, Sanjeev Sharma wrote: > This is a patch to the rtw_cmd.c file that fixes following error. > > ERROR: spaces required around that '>' (ctx:WxV) > ERROR: that open brace { should be on the previous line > > Signed-off-by: Sanjeev Sharma > --- > drivers/staging/rtl8723au/core/rtw_cmd.c | 83 > +++- > 1 file changed, 40 insertions(+), 43 deletions(-) This does two different things, please make it different patches. For individual Error, is Separate patch needed. If Error has been mentioned in description. then it should be OK IMO. And you sent 2 patches, with no hint as to which one comes before which. Please resend all of them, correctly numbered in a series, so that I have a chance to get the order correct when applying them. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] staging:rtl8723au: core: Fix error reported by checkpatch.
-Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Wednesday, November 05, 2014 9:58 PM To: Sharma, Sanjeev Cc: larry.fin...@lwfinger.net; jes.soren...@redhat.com; de...@driverdev.osuosl.org; linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] staging:rtl8723au: core: Fix error reported by checkpatch. On Wed, Nov 05, 2014 at 05:05:03PM +0530, Sanjeev Sharma wrote: > This is a patch to the rtw_cmd.c file that fixes Error reported by > checkpatch. What error are you "fixing"? Please be specific. Submitted individual patch with detail of fix. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] staging:rtl8723au: core: Fix error reported by checkpatch.
-Original Message- From: Joe Perches [mailto:j...@perches.com] Sent: Wednesday, November 05, 2014 10:46 PM To: Sharma, Sanjeev Cc: larry.fin...@lwfinger.net; jes.soren...@redhat.com; gre...@linuxfoundation.org; linux-wirel...@vger.kernel.org; de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] staging:rtl8723au: core: Fix error reported by checkpatch. On Wed, 2014-11-05 at 17:05 +0530, Sanjeev Sharma wrote: > This is a patch to the rtw_cmd.c file that fixes Error reported by > checkpatch. Please run your patches through checkpatch before sending them. In this patch I am fixing error reported by check patch since Error has higher priority. WARNING: suspect code indent for conditional statements (24, 24) #178: FILE: drivers/staging/rtl8723au/core/rtw_cmd.c:1025: + if (check_fwstate(pmlmepriv, _FW_LINKED)) LPS_Leave23a(padapter); Also, there are a couple of different things you changing here. This should be 2 separate patches. Ok I will come up with 2 patch separately. One to use the temporary for: + struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo; (and that bit has a defect here: + } else { LPS_Leave23a(padapter); + } where the indentation for the last close brace is wrong) and another for the whitespace only changes -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.
-Original Message- From: Jes Sorensen [mailto:jes.soren...@redhat.com] Sent: Thursday, October 30, 2014 8:21 PM To: Sharma, Sanjeev Cc: Joe Perches; larry.fin...@lwfinger.net; gre...@linuxfoundation.org; linux-wirel...@vger.kernel.org; de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch. "Sharma, Sanjeev" writes: > -Original Message- > From: Joe Perches [mailto:j...@perches.com] > Sent: Monday, October 27, 2014 8:23 PM > To: Jes Sorensen > Cc: Sharma, Sanjeev; larry.fin...@lwfinger.net; > gre...@linuxfoundation.org; linux-wirel...@vger.kernel.org; > de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by > checkpatch. > > On Mon, 2014-10-27 at 09:43 +0100, Jes Sorensen wrote: >> Sanjeev Sharma writes: >> > This is a patch to the rtw_cmd.c file that fixes Error reported by >> > checkpatch. > [] >> > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c >> > b/drivers/staging/rtl8723au/core/rtw_cmd.c > [] >> > @@ -957,7 +957,7 @@ static void traffic_status_watchdog(struct rtw_adapter >> > *padapter) >> >/* check traffic for powersaving. */ >> >if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod + >> > pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) || >> > - pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2) >> > + pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod > >> > 2) >> >bEnterPS = false; >> >else >> >bEnterPS = true; >> >> This makes the line longer than 80 characters, that is worse than the >> 'problem' you are fixing. > > The code already has dozens of long lines already. > > This is generally a problem because the variable names are pretty long so > strict 80 column adherence generally isn't possible. > > A possible way to shorten these relatively long variable name/line > lengths is to use a temporary for > > pmlmeprv->LinkDetectInfo > > struct rt_link_detect *ldi = &pmlmeprv->LinkDetectInfo; > > so: > > I am agree on this approach but Let's wait for Jes opinion on it. > > Sanjeev Sharma > > drivers/staging/rtl8723au/core/rtw_cmd.c | 46 > > 1 file changed, 23 insertions(+), 23 deletions(-) This is fine with me. Jes Summited another patch after incorporating the change. Sanjeev Sharma > > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c > b/drivers/staging/rtl8723au/core/rtw_cmd.c > index d2d7edf..1b24945 100644 > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c > @@ -922,34 +922,33 @@ static void traffic_status_watchdog(struct rtw_adapter > *padapter) > u8 bHigherBusyTxTraffic = false; > struct mlme_priv *pmlmepriv = &padapter->mlmepriv; > int BusyThreshold = 100; > + struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo; > + > /* */ > /* Determine if our traffic is busy now */ > /* */ > if (check_fwstate(pmlmepriv, _FW_LINKED)) { > if (rtl8723a_BT_coexist(padapter)) > BusyThreshold = 50; > - else if (pmlmepriv->LinkDetectInfo.bBusyTraffic) > + else if (ldi->bBusyTraffic) > BusyThreshold = 75; > /* if we raise bBusyTraffic in last watchdog, using > lower threshold. */ > - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > BusyThreshold || > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > BusyThreshold) { > + if (ldi->NumRxOkInPeriod > BusyThreshold || > + ldi->NumTxOkInPeriod > BusyThreshold) { > bBusyTraffic = true; > > - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > + if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod) > bRxBusyTraffic = true; > else > bTxBusyTraffic = true; > } > > /* Higher Tx/Rx data. */ > - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > 4000 || > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > 4000) {
RE: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.
-Original Message- From: Joe Perches [mailto:j...@perches.com] Sent: Monday, October 27, 2014 8:23 PM To: Jes Sorensen Cc: Sharma, Sanjeev; larry.fin...@lwfinger.net; gre...@linuxfoundation.org; linux-wirel...@vger.kernel.org; de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch. On Mon, 2014-10-27 at 09:43 +0100, Jes Sorensen wrote: > Sanjeev Sharma writes: > > This is a patch to the rtw_cmd.c file that fixes Error reported by > > checkpatch. [] > > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c > > b/drivers/staging/rtl8723au/core/rtw_cmd.c [] > > @@ -957,7 +957,7 @@ static void traffic_status_watchdog(struct rtw_adapter > > *padapter) > > /* check traffic for powersaving. */ > > if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod + > > pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) || > > - pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2) > > + pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod > > > 2) > > bEnterPS = false; > > else > > bEnterPS = true; > > This makes the line longer than 80 characters, that is worse than the > 'problem' you are fixing. The code already has dozens of long lines already. This is generally a problem because the variable names are pretty long so strict 80 column adherence generally isn't possible. A possible way to shorten these relatively long variable name/line lengths is to use a temporary for pmlmeprv->LinkDetectInfo struct rt_link_detect *ldi = &pmlmeprv->LinkDetectInfo; so: I am agree on this approach but Let's wait for Jes opinion on it. Sanjeev Sharma drivers/staging/rtl8723au/core/rtw_cmd.c | 46 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c index d2d7edf..1b24945 100644 --- a/drivers/staging/rtl8723au/core/rtw_cmd.c +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c @@ -922,34 +922,33 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter) u8 bHigherBusyTxTraffic = false; struct mlme_priv *pmlmepriv = &padapter->mlmepriv; int BusyThreshold = 100; + struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo; + /* */ /* Determine if our traffic is busy now */ /* */ if (check_fwstate(pmlmepriv, _FW_LINKED)) { if (rtl8723a_BT_coexist(padapter)) BusyThreshold = 50; - else if (pmlmepriv->LinkDetectInfo.bBusyTraffic) + else if (ldi->bBusyTraffic) BusyThreshold = 75; /* if we raise bBusyTraffic in last watchdog, using lower threshold. */ - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > BusyThreshold || - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > BusyThreshold) { + if (ldi->NumRxOkInPeriod > BusyThreshold || + ldi->NumTxOkInPeriod > BusyThreshold) { bBusyTraffic = true; - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) + if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod) bRxBusyTraffic = true; else bTxBusyTraffic = true; } /* Higher Tx/Rx data. */ - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > 4000 || - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > 4000) { + if (ldi->NumRxOkInPeriod > 4000 || + ldi->NumTxOkInPeriod > 4000) { bHigherBusyTraffic = true; - - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) + if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod) bHigherBusyRxTraffic = true; else bHigherBusyTxTraffic = true; @@ -958,9 +957,9 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter) if (!rtl8723a_BT_coexist(padapter) || !rtl8723a_BT_using_antenna_1(padapter)) { /* check traffic for powersaving. */ - if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod + -
RE: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.
-Original Message- From: Jes Sorensen [mailto:jes.soren...@redhat.com] Sent: Monday, October 27, 2014 2:13 PM To: Sharma, Sanjeev Cc: larry.fin...@lwfinger.net; gre...@linuxfoundation.org; linux-wirel...@vger.kernel.org; de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch. Sanjeev Sharma writes: > This is a patch to the rtw_cmd.c file that fixes Error reported by > checkpatch. > > Signed-off-by: Sanjeev Sharma > --- > drivers/staging/rtl8723au/core/rtw_cmd.c | 83 > +++- > 1 file changed, 40 insertions(+), 43 deletions(-) > > diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c > b/drivers/staging/rtl8723au/core/rtw_cmd.c > index 4eaa502..c1f6254 100644 > --- a/drivers/staging/rtl8723au/core/rtw_cmd.c > +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c > @@ -957,7 +957,7 @@ static void traffic_status_watchdog(struct rtw_adapter > *padapter) > /* check traffic for powersaving. */ > if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod + > pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) || > - pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2) > + pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod > > 2) > bEnterPS = false; > else > bEnterPS = true; This makes the line longer than 80 characters, that is worse than the 'problem' you are fixing. Jes Hello jes, How it can be worse because checkpatch treating this as an Error and line longer than 80 character is warning reported by checkpatch and I could see that in entire staging directory, every maintainer most of the time ignore the 80 column limit and give priority to Error. Please let me know your comment . Sanjeev Sharma -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] drivers: staging: rtl8192u: Fix "space prohibited after that open parenthesis '('" errors
Look ok. Regards Sanjeev Sharma -Original Message- From: Greg Donald [mailto:gdon...@gmail.com] Sent: Monday, October 27, 2014 1:49 AM To: Greg Kroah-Hartman; Ana Rey; Peter P Waskiewicz Jr; Chaitanya Hazarey; Sharma, Sanjeev; Roxana Blaj; Antoine Schweitzer-Chaput; de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org Cc: Greg Donald Subject: [PATCH] drivers: staging: rtl8192u: Fix "space prohibited after that open parenthesis '('" errors Fix checkpatch.pl "space prohibited after that open parenthesis '('" errors Signed-off-by: Greg Donald --- drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 22 +- .../staging/rtl8192u/ieee80211/ieee80211_softmac.c | 11 --- .../rtl8192u/ieee80211/ieee80211_softmac_wx.c | 2 +- drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c | 3 +-- .../staging/rtl8192u/ieee80211/rtl819x_BAProc.c| 16 ++-- .../staging/rtl8192u/ieee80211/rtl819x_HTProc.c| 11 --- 6 files changed, 25 insertions(+), 40 deletions(-) diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c index 73de9e9..d401dbf 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c @@ -713,8 +713,8 @@ static void RxReorderIndicatePacket(struct ieee80211_device *ieee, while(!list_empty(&pTS->RxPendingPktList)) { IEEE80211_DEBUG(IEEE80211_DL_REORDER,"%s(): start RREORDER indicate\n",__func__); pReorderEntry = (PRX_REORDER_ENTRY)list_entry(pTS->RxPendingPktList.prev,RX_REORDER_ENTRY,List); - if( SN_LESS(pReorderEntry->SeqNum, pTS->RxIndicateSeq) || - SN_EQUAL(pReorderEntry->SeqNum, pTS->RxIndicateSeq)) + if (SN_LESS(pReorderEntry->SeqNum, pTS->RxIndicateSeq) || + SN_EQUAL(pReorderEntry->SeqNum, pTS->RxIndicateSeq)) { /* This protect buffer from overflow. */ if(index >= REORDER_WIN_SIZE) { @@ -800,9 +800,8 @@ static u8 parse_subframe(struct sk_buff *skb, // Null packet, don't indicate it to upper layer ChkLength = LLCOffset;/* + (Frame_WEP(frame)!=0 ?Adapter->MgntInfo.SecurityInfo.EncryptionHeadOverhead:0);*/ - if( skb->len <= ChkLength ) { + if (skb->len <= ChkLength) return 0; - } skb_pull(skb, LLCOffset); @@ -1035,10 +1034,9 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, { // IEEE80211_DEBUG(IEEE80211_DL_REORDER,"%s(): pRxTS->RxLastFragNum is %d,frag is %d,pRxTS->RxLastSeqNum is %d,seq is %d\n",__func__,pRxTS->RxLastFragNum,frag,pRxTS->RxLastSeqNum,WLAN_GET_SEQ_SEQ(sc)); - if( (fc & (1<<11)) && - (frag == pRxTS->RxLastFragNum) && - (WLAN_GET_SEQ_SEQ(sc) == pRxTS->RxLastSeqNum) ) - { + if ((fc & (1<<11)) && + (frag == pRxTS->RxLastFragNum) && + (WLAN_GET_SEQ_SEQ(sc) == pRxTS->RxLastSeqNum)) { goto rx_dropped; } else @@ -2456,7 +2454,7 @@ static inline void ieee80211_process_probe_response( // then wireless adapter should do active scan from ch1~11 and // passive scan from ch12~14 - if( !IsLegalChannel(ieee, network.channel) ) + if (!IsLegalChannel(ieee, network.channel)) return; if(ieee->bGlobalDomain) { @@ -2465,8 +2463,7 @@ static inline void ieee80211_process_probe_response( // Case 1: Country code if(IS_COUNTRY_IE_VALID(ieee) ) { - if( !IsLegalChannel(ieee, network.channel) ) - { + if (!IsLegalChannel(ieee, network.channel)) { printk("GetScanInfo(): For Country code, filter probe response at channel(%d).\n", network.channel); return; } @@ -2487,8 +2484,7 @@ static inline void ieee80211_process_probe_response( // Case 1: Country code if(IS_COUNTRY_IE_VALID(ieee) ) { - if( !IsLegalChannel(ieee, network.channel) ) - { + if (!IsLegalChannel(ieee, network.channel)) { printk("Get
RE: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
-Original Message- From: Julian Calaby [mailto:julian.cal...@gmail.com] Sent: Monday, September 15, 2014 11:32 AM To: Sharma, Sanjeev Cc: Johannes Berg; d...@gentoo.org; k...@deine-taler.de; linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; net...@vger.kernel.org Subject: Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held() Hi Sanjeev, On Thu, Sep 11, 2014 at 8:36 PM, Sharma, Sanjeev wrote: > -Original Message- > From: Johannes Berg [mailto:johan...@sipsolutions.net] > Sent: Thursday, September 11, 2014 3:42 PM > To: Sharma, Sanjeev > Cc: d...@gentoo.org; k...@deine-taler.de; > linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; > net...@vger.kernel.org > Subject: Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with > lockdep_assert_held() > > On Thu, 2014-09-11 at 15:39 +0530, Sanjeev Sharma wrote: >> on some architecture spin_is_locked() always return false in >> uniprocessor configuration and therefore it would be advise to >> replace with lockdep_assert_held(). >> >> Signed-off-by: Sanjeev Sharma >> --- >> Changes in v2: >> - corrected the typo > >> Now it compiles, but you got the logic wrong. > >> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c >> @@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac) { >> flush_workqueue(zd_workqueue); >> zd_chip_clear(&mac->chip); >> - ZD_ASSERT(!spin_is_locked(&mac->lock)); >> + lockdep_assert_held(&mac->lock); >> ZD_MEMCLEAR(mac, sizeof(struct zd_mac)); } > >>Look closely at this again. > > I didn't understand where I put wrong logic ? I find it helps to spell out what code is doing in words. E.g. the line you're removing is: ZD_ASSERT(!spin_is_locked(&mac->lock)); So, we'll assert when spin_is_locked(&mac->lock) is false, i.e. when mac->lock is not spin locked. This isn't the same as what you're replacing it with. I feel logic is absolutely correct and if you expand it will looks like .. +#define lockdep_assert_held(l) do {\ + WARN_ON(debug_locks && !lockdep_is_held(l));\ + } while (0) Please refer http://lkml.iu.edu/hypermail/linux/kernel/1203.2/00369.html and also see include/linux/lockdep.h for more detail. Thanks Sanjeev Sharma
RE: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
-Original Message- From: Johannes Berg [mailto:johan...@sipsolutions.net] Sent: Thursday, September 11, 2014 3:42 PM To: Sharma, Sanjeev Cc: d...@gentoo.org; k...@deine-taler.de; linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; net...@vger.kernel.org Subject: Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held() On Thu, 2014-09-11 at 15:39 +0530, Sanjeev Sharma wrote: > on some architecture spin_is_locked() always return false in > uniprocessor configuration and therefore it would be advise to replace > with lockdep_assert_held(). > > Signed-off-by: Sanjeev Sharma > --- > Changes in v2: > - corrected the typo > Now it compiles, but you got the logic wrong. > +++ b/drivers/net/wireless/zd1211rw/zd_mac.c > @@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac) { > flush_workqueue(zd_workqueue); > zd_chip_clear(&mac->chip); > - ZD_ASSERT(!spin_is_locked(&mac->lock)); > + lockdep_assert_held(&mac->lock); > ZD_MEMCLEAR(mac, sizeof(struct zd_mac)); } >Look closely at this again. I didn't understand where I put wrong logic ? Regards Sanjeev Sharma
RE: [PATCH v2] sgi-xp: Do not use BUG_ON(!spin_is_locked())
-Original Message- From: Sanjeev Sharma [mailto:sanjeev_sha...@mentor.com] Sent: Wednesday, August 20, 2014 11:06 AM To: c...@sgi.com; robinmh...@gmail.com Cc: linux-kernel@vger.kernel.org; Sharma, Sanjeev; Sharma, Sanjeev Subject: [PATCH v2] sgi-xp: Do not use BUG_ON(!spin_is_locked()) on some architecture spin_is_locked() always return false in uniprocessor configuration and can therefore not be used with BUG_ON.it would be advise to replace with lockdep_assert_held(). Signed-off-by: Sanjeev Sharma --- Changes in v2: - corrected the typo drivers/misc/sgi-xp/xpc_channel.c | 6 +++--- drivers/misc/sgi-xp/xpc_sn2.c | 2 +- drivers/misc/sgi-xp/xpc_uv.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/misc/sgi-xp/xpc_channel.c b/drivers/misc/sgi-xp/xpc_channel.c index 128d561..611d34f 100644 --- a/drivers/misc/sgi-xp/xpc_channel.c +++ b/drivers/misc/sgi-xp/xpc_channel.c @@ -28,7 +28,7 @@ xpc_process_connect(struct xpc_channel *ch, unsigned long *irq_flags) { enum xp_retval ret; - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(&ch->lock); if (!(ch->flags & XPC_C_OPENREQUEST) || !(ch->flags & XPC_C_ROPENREQUEST)) { @@ -82,7 +82,7 @@ xpc_process_disconnect(struct xpc_channel *ch, unsigned long *irq_flags) struct xpc_partition *part = &xpc_partitions[ch->partid]; u32 channel_was_connected = (ch->flags & XPC_C_WASCONNECTED); - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(&ch->lock); if (!(ch->flags & XPC_C_DISCONNECTING)) return; @@ -758,7 +758,7 @@ xpc_disconnect_channel(const int line, struct xpc_channel *ch, { u32 channel_was_connected = (ch->flags & XPC_C_CONNECTED); - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(&ch->lock); if (ch->flags & (XPC_C_DISCONNECTING | XPC_C_DISCONNECTED)) return; diff --git a/drivers/misc/sgi-xp/xpc_sn2.c b/drivers/misc/sgi-xp/xpc_sn2.c index 7d71c04..9d3b113 100644 --- a/drivers/misc/sgi-xp/xpc_sn2.c +++ b/drivers/misc/sgi-xp/xpc_sn2.c @@ -1674,7 +1674,7 @@ xpc_teardown_msg_structures_sn2(struct xpc_channel *ch) { struct xpc_channel_sn2 *ch_sn2 = &ch->sn.sn2; - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(&ch->lock); ch_sn2->remote_msgqueue_pa = 0; diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c index 95c8944..828733c 100644 --- a/drivers/misc/sgi-xp/xpc_uv.c +++ b/drivers/misc/sgi-xp/xpc_uv.c @@ -1183,7 +1183,7 @@ xpc_teardown_msg_structures_uv(struct xpc_channel *ch) { struct xpc_channel_uv *ch_uv = &ch->sn.uv; - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(&ch->lock); kfree(ch_uv->cached_notify_gru_mq_desc); ch_uv->cached_notify_gru_mq_desc = NULL; -- 1.7.11.7 Any update on this patch because it's been quite long time . Regards Sanjeev Sharma -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
-Original Message- From: Johannes Berg [mailto:johan...@sipsolutions.net] Sent: Friday, August 22, 2014 5:01 PM To: Sharma, Sanjeev Cc: d...@gentoo.org; k...@deine-taler.de; linvi...@tuxdriver.com; linux-wirel...@vger.kernel.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held() On Tue, 2014-08-19 at 06:39 +, Sharma, Sanjeev wrote: > Ping for review the patch. > Make sure it compiles ... Any update on this patch ? Regards Sanjeev Sharma
RE: [PATCH] staging:r8190_rtl8256: coding style: Fixed commenting style
-Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Sunday, August 31, 2014 2:28 AM To: Sharma, Sanjeev Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:r8190_rtl8256: coding style: Fixed commenting style On Mon, Aug 25, 2014 at 12:55:27PM +0530, Sanjeev Sharma wrote: > This is a patch to the r8190_rtl8256.c file that fixes commenting > style Error > > Signed-off-by: Sanjeev Sharma > --- > drivers/staging/rtl8192u/r8190_rtl8256.c | 59 > +--- > 1 file changed, 31 insertions(+), 28 deletions(-) > > diff --git a/drivers/staging/rtl8192u/r8190_rtl8256.c > b/drivers/staging/rtl8192u/r8190_rtl8256.c > index 43ed768..8fe35ad 100644 > --- a/drivers/staging/rtl8192u/r8190_rtl8256.c > +++ b/drivers/staging/rtl8192u/r8190_rtl8256.c > @@ -61,13 +61,15 @@ void PHY_SetRF8256Bandwidth(struct net_device *dev , > HT_CHANNEL_WIDTH Bandwidth) > break; > case HT_CHANNEL_WIDTH_20_40: > if (priv->card_8192_version == VERSION_819xU_A > || priv->card_8192_version == VERSION_819xU_B) { /* 8256 D-cut, E-cut, xiong: > consider it later! */ > - rtl8192_phy_SetRFReg(dev, > (RF90_RADIO_PATH_E)eRFPath, 0x0b, bMask12Bits, 0x300); //phy para:3ba > + rtl8192_phy_SetRFReg(dev, > (RF90_RADIO_PATH_E)eRFPath, 0x0b, > +bMask12Bits, 0x300); /* phy para:3ba */ > rtl8192_phy_SetRFReg(dev, > (RF90_RADIO_PATH_E)eRFPath, 0x2c, bMask12Bits, 0x3df); > rtl8192_phy_SetRFReg(dev, > (RF90_RADIO_PATH_E)eRFPath, 0x0e, > bMask12Bits, 0x0a1); > > - //cosa add for sd3's request 01/23/2008 > + /* cosa add for sd3's request 01/23/2008 > + * > + */ > if (priv->chan == 3 || priv->chan == 9) > - //I need to set priv->chan > whenever current channel changes > + /* I need to set priv->chan > whenever current channel changes */ > rtl8192_phy_SetRFReg(dev, > (RF90_RADIO_PATH_E)eRFPath, 0x14, bMask12Bits, 0x59b); > else > rtl8192_phy_SetRFReg(dev, > (RF90_RADIO_PATH_E)eRFPath, 0x14, > bMask12Bits, 0x5ab); @@ -91,11 +93,12 @@ void > PHY_SetRF8256Bandwidth(struct net_device *dev , HT_CHANNEL_WIDTH > Bandwidth) void PHY_RF8256_Config(struct net_device *dev) { > struct r8192_priv *priv = ieee80211_priv(dev); > - // Initialize general global value > - // > - // TODO: Extend RF_PATH_C and RF_PATH_D in the future > + /* Initialize general global value > + * > + * TODO: Extend RF_PATH_C and RF_PATH_D in the future > + */ > priv->NumTotalRFPath = RTL819X_TOTAL_RF_PATH; > - // Config BB and RF > + /* Config BB and RF */ > phy_RF8256_Config_ParaFile(dev); > } > > /* > -- @@ -107,10 +110,11 @@ void PHY_RF8256_Config(struct net_device > *dev) void phy_RF8256_Config_ParaFile(struct net_device *dev) { > u32 u4RegValue = 0; > - //static s1Byte szRadioAFile[] = > RTL819X_PHY_RADIO_A; > - //static s1Byte szRadioBFile[] = > RTL819X_PHY_RADIO_B; > - //static s1Byte szRadioCFile[] = > RTL819X_PHY_RADIO_C; > - //static s1Byte szRadioDFile[] = > RTL819X_PHY_RADIO_D; > + /* static s1ByteszRadioAFile[] = > RTL819X_PHY_RADIO_A; > + * static s1ByteszRadioBFile[] = > RTL819X_PHY_RADIO_B; > + * static s1ByteszRadioCFile[] = > RTL819X_PHY_RADIO_C; > + * static s1ByteszRadioDFile[] = > RTL819X_PHY_RADIO_D; > + */ Why not just remove stuff like this if it's not being used? Same goes for other commented out code lines. thanks, greg k-h Hi greg submitted patch after removal of unwanted code thanks, Sanjeev sharma -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
-Original Message- From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org] Sent: Tuesday, August 19, 2014 3:01 PM To: Sharma, Sanjeev Cc: kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-s...@vger.kernel.org; Hans de Goede Subject: Re: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held() On Tue, Aug 19, 2014 at 06:33:04AM +, Sharma, Sanjeev wrote: > Hi Greg, > > Any feedback on this patch ? The merge window ended 2 days ago, _and_ I'm at the kernel summit this week, _and_ my queue is currently sitting at: $ mdfrm -c ~/mail/todo/ 1317 messages in /home/gregkh/mail/todo/ So please be patient. I'll get to it in a few weeks. Please let me know when this is going to be merged ? Thanks Sanjeev greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] staging:iio: moved platform_data into include/linux/iio
-Original Message- From: Lars-Peter Clausen [mailto:l...@metafoo.de] Sent: Monday, August 25, 2014 7:03 PM To: Sharma, Sanjeev; sanjeev sharma Cc: ji...@kernel.org; gregkh; linux-...@vger.kernel.org; devel; linux-kernel Subject: Re: [PATCH] staging:iio: moved platform_data into include/linux/iio On 08/25/2014 01:49 PM, Sharma, Sanjeev wrote: > -Original Message- > From: Lars-Peter Clausen [mailto:l...@metafoo.de] > Sent: Monday, August 25, 2014 1:39 PM > To: Sharma, Sanjeev; sanjeev sharma > Cc: ji...@kernel.org; gregkh; linux-...@vger.kernel.org; devel; > linux-kernel > Subject: Re: [PATCH] staging:iio: moved platform_data into > include/linux/iio > > On 08/25/2014 09:17 AM, Sharma, Sanjeev wrote: >> Hello Lars, >> >> As per your suggestion Can I move complete Driver out of staging specially >> SPI ADC Driver. > > Only if they are cleaned up first. All of the drivers that are still in > staging do have issues, otherwise we'd already had moved them. A few of them > are OK codestyle wise, but do have ABI issues which need to be resolved > before they can be moved. > > - Lars > > Where I can find ABI issues which need to be resolved so that these can be > looked upon. Compare the sysfs files and their behavior that the driver registers with what is documented in the IIO ABI spec[1]. If you can't find it in the documentation it either needs to be documented or updated to use the existing ABI correctly. Note that this is not necessarily trivial and may require in-depth knowledge and understanding of the IIO ABI. You mean to say I shouldn't spent much time here because this is not much important and if this is the case then I think I should look into some area which make more sense. Thanks Sanjeev Thanks, - Lars [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing/sysfs-bus-iio > > -Sanjeev >> >> Regards >> Sanjeev Sharma >> >> -Original Message- >> From: Lars-Peter Clausen [mailto:l...@metafoo.de] >> Sent: Wednesday, August 20, 2014 12:20 PM >> To: sanjeev sharma >> Cc: Sharma, Sanjeev; ji...@kernel.org; gregkh; >> linux-...@vger.kernel.org; devel; linux-kernel >> Subject: Re: [PATCH] staging:iio: moved platform_data into >> include/linux/iio >> >> On 08/20/2014 08:44 AM, sanjeev sharma wrote: >>> Hi, >>> >>> This was the action item(TO-DO). IMO, it make sense to move into >>> include/linux/iio because IIO complete subsystem may take some time. >> >> The code that is in staging is not supposed to 'leak' outside of staging. So >> either you move the driver as a whole out of staging or leave it there, but >> do not move individual files of the driver out of staging. The action item >> is for when the driver is moved out of staging. >> > > N r y b X ǧv ^ )Þº{.n +{ *" ^n r z h& G h > ( 階 Ý¢j" m z Þ– f h ~ mml== > N�r��yb�X��ǧv�^�)Þº{.n�+{zX����ܨ}ï¿½ï¿½ï¿½Æ z�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jÇ«y�m��@A�a��� 0��h���i
RE: [PATCH] staging:iio: moved platform_data into include/linux/iio
-Original Message- From: Lars-Peter Clausen [mailto:l...@metafoo.de] Sent: Monday, August 25, 2014 1:39 PM To: Sharma, Sanjeev; sanjeev sharma Cc: ji...@kernel.org; gregkh; linux-...@vger.kernel.org; devel; linux-kernel Subject: Re: [PATCH] staging:iio: moved platform_data into include/linux/iio On 08/25/2014 09:17 AM, Sharma, Sanjeev wrote: > Hello Lars, > > As per your suggestion Can I move complete Driver out of staging specially > SPI ADC Driver. Only if they are cleaned up first. All of the drivers that are still in staging do have issues, otherwise we'd already had moved them. A few of them are OK codestyle wise, but do have ABI issues which need to be resolved before they can be moved. - Lars Where I can find ABI issues which need to be resolved so that these can be looked upon. -Sanjeev > > Regards > Sanjeev Sharma > > -Original Message- > From: Lars-Peter Clausen [mailto:l...@metafoo.de] > Sent: Wednesday, August 20, 2014 12:20 PM > To: sanjeev sharma > Cc: Sharma, Sanjeev; ji...@kernel.org; gregkh; > linux-...@vger.kernel.org; devel; linux-kernel > Subject: Re: [PATCH] staging:iio: moved platform_data into > include/linux/iio > > On 08/20/2014 08:44 AM, sanjeev sharma wrote: >> Hi, >> >> This was the action item(TO-DO). IMO, it make sense to move into >> include/linux/iio because IIO complete subsystem may take some time. > > The code that is in staging is not supposed to 'leak' outside of staging. So > either you move the driver as a whole out of staging or leave it there, but > do not move individual files of the driver out of staging. The action item is > for when the driver is moved out of staging. >
RE: [PATCH] staging:iio: moved platform_data into include/linux/iio
Hello Lars, As per your suggestion Can I move complete Driver out of staging specially SPI ADC Driver. Regards Sanjeev Sharma -Original Message- From: Lars-Peter Clausen [mailto:l...@metafoo.de] Sent: Wednesday, August 20, 2014 12:20 PM To: sanjeev sharma Cc: Sharma, Sanjeev; ji...@kernel.org; gregkh; linux-...@vger.kernel.org; devel; linux-kernel Subject: Re: [PATCH] staging:iio: moved platform_data into include/linux/iio On 08/20/2014 08:44 AM, sanjeev sharma wrote: > Hi, > > This was the action item(TO-DO). IMO, it make sense to move into > include/linux/iio because IIO complete subsystem may take some time. The code that is in staging is not supposed to 'leak' outside of staging. So either you move the driver as a whole out of staging or leave it there, but do not move individual files of the driver out of staging. The action item is for when the driver is moved out of staging.
RE: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
Yes, it compiling successfully. -Original Message- From: Johannes Berg [mailto:johan...@sipsolutions.net] Sent: Friday, August 22, 2014 5:01 PM To: Sharma, Sanjeev Cc: d...@gentoo.org; k...@deine-taler.de; linvi...@tuxdriver.com; linux-wirel...@vger.kernel.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held() On Tue, 2014-08-19 at 06:39 +, Sharma, Sanjeev wrote: > Ping for review the patch. Make sure it compiles ...
RE: [PATCH] staging:iio: moved platform_data into include/linux/iio
Thanks greg TODO was very confusing. @Lars: These drivers are looks stable enough to move out of staging tree and Can we move to outside staging tree as per your 1st Comment. Regards Sanjeev Sharma -Original Message- From: gregkh [mailto:gre...@linuxfoundation.org] Sent: Wednesday, August 20, 2014 4:46 PM To: sanjeev sharma Cc: Lars-Peter Clausen; Sharma, Sanjeev; ji...@kernel.org; linux-...@vger.kernel.org; devel; linux-kernel Subject: Re: [PATCH] staging:iio: moved platform_data into include/linux/iio A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Wed, Aug 20, 2014 at 12:32:52PM +0530, sanjeev sharma wrote: > Hi, > > I have only moved some header files because other header files are > already present there and these are only pending files which is not > moved outside the staging directory. > > @Greg:What is your though on this ? Lars-Peter is correct, don't "leak" files from staging. Take the time to work on fixing up these drivers correctly so they can be moved out of staging instead of just doing simple file moves which do nothing. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] iio: remove .owner field for driver using module_platform_driver
Adding Greg into CC list. Sanjeev Sharma -Original Message- From: Sanjeev Sharma [mailto:sanjeev_sha...@mentor.com] Sent: Wednesday, August 20, 2014 3:03 PM To: ji...@kernel.org; kgene@samsung.com Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; de...@driverdev.osuosl.org; Sharma, Sanjeev; Sharma, Sanjeev Subject: [PATCH] iio: remove .owner field for driver using module_platform_driver This patch removes the .owner field for drivers which use the platform_driver_register api because this is overriden in _platform_driver_register. Signed-off-by: Sanjeev Sharma --- drivers/iio/accel/hid-sensor-accel-3d.c | 1 - drivers/iio/adc/exynos_adc.c | 1 - drivers/iio/adc/lp8788_adc.c | 1 - drivers/iio/adc/ti_am335x_adc.c | 1 - drivers/iio/adc/twl4030-madc.c| 1 - drivers/iio/adc/twl6030-gpadc.c | 1 - drivers/iio/adc/vf610_adc.c | 1 - drivers/iio/adc/viperboard_adc.c | 1 - drivers/iio/gyro/hid-sensor-gyro-3d.c | 1 - drivers/iio/humidity/dht11.c | 1 - drivers/iio/light/hid-sensor-als.c| 1 - drivers/iio/light/hid-sensor-prox.c | 1 - drivers/iio/light/lm3533-als.c| 1 - drivers/iio/magnetometer/hid-sensor-magn-3d.c | 1 - drivers/iio/orientation/hid-sensor-incl-3d.c | 1 - drivers/iio/orientation/hid-sensor-rotation.c | 1 - drivers/iio/pressure/hid-sensor-press.c | 1 - drivers/iio/trigger/iio-trig-interrupt.c | 1 - 18 files changed, 18 deletions(-) diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c index 54e464e..d5d9531 100644 --- a/drivers/iio/accel/hid-sensor-accel-3d.c +++ b/drivers/iio/accel/hid-sensor-accel-3d.c @@ -419,7 +419,6 @@ static struct platform_driver hid_accel_3d_platform_driver = { .id_table = hid_accel_3d_ids, .driver = { .name = KBUILD_MODNAME, - .owner = THIS_MODULE, }, .probe = hid_accel_3d_probe, .remove = hid_accel_3d_remove, diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c index fc9dfc2..b4373f7 100644 --- a/drivers/iio/adc/exynos_adc.c +++ b/drivers/iio/adc/exynos_adc.c @@ -606,7 +606,6 @@ static struct platform_driver exynos_adc_driver = { .remove = exynos_adc_remove, .driver = { .name = "exynos-adc", - .owner = THIS_MODULE, .of_match_table = exynos_adc_match, .pm = &exynos_adc_pm_ops, }, diff --git a/drivers/iio/adc/lp8788_adc.c b/drivers/iio/adc/lp8788_adc.c index 5c8c915..152cfc8 100644 --- a/drivers/iio/adc/lp8788_adc.c +++ b/drivers/iio/adc/lp8788_adc.c @@ -244,7 +244,6 @@ static struct platform_driver lp8788_adc_driver = { .remove = lp8788_adc_remove, .driver = { .name = LP8788_DEV_ADC, - .owner = THIS_MODULE, }, }; module_platform_driver(lp8788_adc_driver); diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index d5dc4c6..b730864 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -545,7 +545,6 @@ MODULE_DEVICE_TABLE(of, ti_adc_dt_ids); static struct platform_driver tiadc_driver = { .driver = { .name = "TI-am335x-adc", - .owner = THIS_MODULE, .pm = TIADC_PM_OPS, .of_match_table = ti_adc_dt_ids, }, diff --git a/drivers/iio/adc/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c index eb86786..94c5f05 100644 --- a/drivers/iio/adc/twl4030-madc.c +++ b/drivers/iio/adc/twl4030-madc.c @@ -883,7 +883,6 @@ static struct platform_driver twl4030_madc_driver = { .remove = twl4030_madc_remove, .driver = { .name = "twl4030_madc", - .owner = THIS_MODULE, .of_match_table = of_match_ptr(twl_madc_of_match), }, }; diff --git a/drivers/iio/adc/twl6030-gpadc.c b/drivers/iio/adc/twl6030-gpadc.c index 15282f1..89d8aa1 100644 --- a/drivers/iio/adc/twl6030-gpadc.c +++ b/drivers/iio/adc/twl6030-gpadc.c @@ -994,7 +994,6 @@ static struct platform_driver twl6030_gpadc_driver = { .remove = twl6030_gpadc_remove, .driver = { .name = DRIVER_NAME, - .owner = THIS_MODULE, .pm = &twl6030_gpadc_pm_ops, .of_match_table = of_twl6030_match_tbl, }, diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c index 44799eb5..4a10ae9 100644 --- a/drivers/iio/adc/vf610_adc.c +++ b/drivers/iio/adc/vf610_adc.c @@ -698,7 +698,6 @@ static struct platform_driver vf610_adc_driver = { .remove = vf610_adc_remove, .driver = { .name = DRIVER_NAME,
RE: [PATCH] sgi-xp: Do not use BUG_ON(!spin_is_locked())
Opps ! Thanks for review comment. Just sent V2 updated patch. Sanjeev Sharma -Original Message- From: Ryan Mallon [mailto:rmal...@gmail.com] Sent: Wednesday, August 20, 2014 4:34 AM To: Sharma, Sanjeev; c...@sgi.com; robinmh...@gmail.com Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] sgi-xp: Do not use BUG_ON(!spin_is_locked()) On 12/08/14 17:35, Sanjeev Sharma wrote: > on some architecture spin_is_locked() always return false in > uniprocessor configuration and can therefore not be used with > BUG_ON.it would be advise to replace with lockdep_assert_held(). > > Signed-off-by: Sanjeev Sharma > --- > drivers/misc/sgi-xp/xpc_channel.c | 6 +++--- > drivers/misc/sgi-xp/xpc_sn2.c | 2 +- > drivers/misc/sgi-xp/xpc_uv.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/misc/sgi-xp/xpc_channel.c > b/drivers/misc/sgi-xp/xpc_channel.c > index 128d561..240fe5a 100644 > --- a/drivers/misc/sgi-xp/xpc_channel.c > +++ b/drivers/misc/sgi-xp/xpc_channel.c > @@ -28,7 +28,7 @@ xpc_process_connect(struct xpc_channel *ch, unsigned > long *irq_flags) { > enum xp_retval ret; > > - DBUG_ON(!spin_is_locked(&ch->lock)); > + lockdep_assert_held(!spin_is_locked(&ch->lock)); This is incorrect, and will break the build with CONFIG_LOCKDEP enabled. You actually want: lockdep_assert_held(&ch->lock); Same for the other instances. ~Ryan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
Thanks for letting me know. Sanjeev Sharma -Original Message- From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org] Sent: Tuesday, August 19, 2014 3:01 PM To: Sharma, Sanjeev Cc: kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-s...@vger.kernel.org; Hans de Goede Subject: Re: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held() On Tue, Aug 19, 2014 at 06:33:04AM +, Sharma, Sanjeev wrote: > Hi Greg, > > Any feedback on this patch ? The merge window ended 2 days ago, _and_ I'm at the kernel summit this week, _and_ my queue is currently sitting at: $ mdfrm -c ~/mail/todo/ 1317 messages in /home/gregkh/mail/todo/ So please be patient. I'll get to it in a few weeks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
Ping for review the patch. thanks, Sanjeev Sharma -Original Message- From: Sanjeev Sharma [mailto:sanjeev_sha...@mentor.com] Sent: Tuesday, August 12, 2014 12:36 PM To: d...@gentoo.org; k...@deine-taler.de Cc: linvi...@tuxdriver.com; linux-wirel...@vger.kernel.org; net...@vger.kernel.org; linux-kernel@vger.kernel.org; Sharma, Sanjeev; Sharma, Sanjeev Subject: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held() on some architecture spin_is_locked() always return false in uniprocessor configuration and therefore it would be advise to replace with lockdep_assert_held(). Signed-off-by: Sanjeev Sharma --- drivers/net/wireless/zd1211rw/zd_mac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c index e7af261..72c0bc2 100644 --- a/drivers/net/wireless/zd1211rw/zd_mac.c +++ b/drivers/net/wireless/zd1211rw/zd_mac.c @@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac) { flush_workqueue(zd_workqueue); zd_chip_clear(&mac->chip); - ZD_ASSERT(!spin_is_locked(&mac->lock)); + lockdep_assert_held(!spin_is_locked(&mac->lock)); ZD_MEMCLEAR(mac, sizeof(struct zd_mac)); } -- 1.7.11.7 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sgi-xp: Do not use BUG_ON(!spin_is_locked())
Hello Robin, Who will merge the change in linux-next tree. Regards Sanjeev Sharma -Original Message- From: Sanjeev Sharma [mailto:sanjeev_sha...@mentor.com] Sent: Tuesday, August 12, 2014 1:05 PM To: c...@sgi.com; robinmh...@gmail.com Cc: linux-kernel@vger.kernel.org; Sharma, Sanjeev; Sharma, Sanjeev Subject: [PATCH] sgi-xp: Do not use BUG_ON(!spin_is_locked()) on some architecture spin_is_locked() always return false in uniprocessor configuration and can therefore not be used with BUG_ON.it would be advise to replace with lockdep_assert_held(). Signed-off-by: Sanjeev Sharma --- drivers/misc/sgi-xp/xpc_channel.c | 6 +++--- drivers/misc/sgi-xp/xpc_sn2.c | 2 +- drivers/misc/sgi-xp/xpc_uv.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/misc/sgi-xp/xpc_channel.c b/drivers/misc/sgi-xp/xpc_channel.c index 128d561..240fe5a 100644 --- a/drivers/misc/sgi-xp/xpc_channel.c +++ b/drivers/misc/sgi-xp/xpc_channel.c @@ -28,7 +28,7 @@ xpc_process_connect(struct xpc_channel *ch, unsigned long *irq_flags) { enum xp_retval ret; - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(!spin_is_locked(&ch->lock)); if (!(ch->flags & XPC_C_OPENREQUEST) || !(ch->flags & XPC_C_ROPENREQUEST)) { @@ -82,7 +82,7 @@ xpc_process_disconnect(struct xpc_channel *ch, unsigned long *irq_flags) struct xpc_partition *part = &xpc_partitions[ch->partid]; u32 channel_was_connected = (ch->flags & XPC_C_WASCONNECTED); - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(!spin_is_locked(&ch->lock)); if (!(ch->flags & XPC_C_DISCONNECTING)) return; @@ -758,7 +758,7 @@ xpc_disconnect_channel(const int line, struct xpc_channel *ch, { u32 channel_was_connected = (ch->flags & XPC_C_CONNECTED); - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(!spin_is_locked(&ch->lock)); if (ch->flags & (XPC_C_DISCONNECTING | XPC_C_DISCONNECTED)) return; diff --git a/drivers/misc/sgi-xp/xpc_sn2.c b/drivers/misc/sgi-xp/xpc_sn2.c index 7d71c04..f90ee26 100644 --- a/drivers/misc/sgi-xp/xpc_sn2.c +++ b/drivers/misc/sgi-xp/xpc_sn2.c @@ -1674,7 +1674,7 @@ xpc_teardown_msg_structures_sn2(struct xpc_channel *ch) { struct xpc_channel_sn2 *ch_sn2 = &ch->sn.sn2; - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(!spin_is_locked(&ch->lock)); ch_sn2->remote_msgqueue_pa = 0; diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c index 95c8944..a349940 100644 --- a/drivers/misc/sgi-xp/xpc_uv.c +++ b/drivers/misc/sgi-xp/xpc_uv.c @@ -1183,7 +1183,7 @@ xpc_teardown_msg_structures_uv(struct xpc_channel *ch) { struct xpc_channel_uv *ch_uv = &ch->sn.uv; - DBUG_ON(!spin_is_locked(&ch->lock)); + lockdep_assert_held(!spin_is_locked(&ch->lock)); kfree(ch_uv->cached_notify_gru_mq_desc); ch_uv->cached_notify_gru_mq_desc = NULL; -- 1.7.11.7 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
Hi Greg, Any feedback on this patch ? Regards Sanjeev Sharma -Original Message- From: Sharma, Sanjeev Sent: Tuesday, August 12, 2014 12:07 PM To: 'Hans de Goede' Cc: gre...@linuxfoundation.org; kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-s...@vger.kernel.org Subject: RE: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held() Yes I have incorporated review comment from Greg. Regards Sanjeev Sharma -Original Message- From: Hans de Goede [mailto:hdego...@redhat.com] Sent: Tuesday, August 12, 2014 11:59 AM To: Sharma, Sanjeev Cc: gre...@linuxfoundation.org; kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-s...@vger.kernel.org Subject: Re: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held() Hi, On 08/12/2014 08:40 AM, Sanjeev Sharma wrote: > on some architecture spin_is_locked() always return false in > uniprocessor configuration and therefore it would be advise to replace > with lockdep_assert_held(). > > Signed-off-by: Sanjeev Sharma > --- > Changes in v3: > incorporated review comment suggested by Greg Thanks, this is still: Acked-by: Hans de Goede Regards, Hans > > drivers/usb/storage/uas.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c > index 3f42785..05b2d8e 100644 > --- a/drivers/usb/storage/uas.c > +++ b/drivers/usb/storage/uas.c > @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info > *devinfo, > struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); > > uas_log_cmd_state(cmnd, caller); > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); > cmdinfo->state |= COMMAND_ABORTED; > cmdinfo->state &= ~IS_IN_WORK_LIST; > @@ -181,7 +181,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) > struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); > struct uas_dev_info *devinfo = cmnd->device->hostdata; > > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > cmdinfo->state |= IS_IN_WORK_LIST; > schedule_work(&devinfo->work); > } > @@ -283,7 +283,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const > char *caller) > struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; > struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; > > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > if (cmdinfo->state & (COMMAND_INFLIGHT | > DATA_IN_URB_INFLIGHT | > DATA_OUT_URB_INFLIGHT | > @@ -622,7 +622,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, > struct urb *urb; > int err; > > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > if (cmdinfo->state & SUBMIT_STATUS_URB) { > urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo->stream); > if (!urb) > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
Yes I have incorporated review comment from Greg. Regards Sanjeev Sharma -Original Message- From: Hans de Goede [mailto:hdego...@redhat.com] Sent: Tuesday, August 12, 2014 11:59 AM To: Sharma, Sanjeev Cc: gre...@linuxfoundation.org; kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-s...@vger.kernel.org Subject: Re: [PATCH v3] uas: replace WARN_ON_ONCE() with lockdep_assert_held() Hi, On 08/12/2014 08:40 AM, Sanjeev Sharma wrote: > on some architecture spin_is_locked() always return false in > uniprocessor configuration and therefore it would be advise to replace > with lockdep_assert_held(). > > Signed-off-by: Sanjeev Sharma > --- > Changes in v3: > incorporated review comment suggested by Greg Thanks, this is still: Acked-by: Hans de Goede Regards, Hans > > drivers/usb/storage/uas.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c > index 3f42785..05b2d8e 100644 > --- a/drivers/usb/storage/uas.c > +++ b/drivers/usb/storage/uas.c > @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info > *devinfo, > struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); > > uas_log_cmd_state(cmnd, caller); > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); > cmdinfo->state |= COMMAND_ABORTED; > cmdinfo->state &= ~IS_IN_WORK_LIST; > @@ -181,7 +181,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) > struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); > struct uas_dev_info *devinfo = cmnd->device->hostdata; > > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > cmdinfo->state |= IS_IN_WORK_LIST; > schedule_work(&devinfo->work); > } > @@ -283,7 +283,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const > char *caller) > struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; > struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; > > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > if (cmdinfo->state & (COMMAND_INFLIGHT | > DATA_IN_URB_INFLIGHT | > DATA_OUT_URB_INFLIGHT | > @@ -622,7 +622,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, > struct urb *urb; > int err; > > - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); > + lockdep_assert_held(&devinfo->lock); > if (cmdinfo->state & SUBMIT_STATUS_URB) { > urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo->stream); > if (!urb) > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
Done ! Thanks Sanjeev Sharma -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Tuesday, August 12, 2014 11:32 AM To: Sharma, Sanjeev Cc: hdego...@redhat.com; kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-s...@vger.kernel.org Subject: Re: [PATCH v2] uas: replace WARN_ON_ONCE() with lockdep_assert_held() On Tue, Aug 12, 2014 at 11:38:37AM +0530, Sanjeev Sharma wrote: > spin_is_locked() always return false in uniprocessor configuration and > therefore it would be advise to replace with lockdep_assert_held(). Add "on some architectures" in here somewhere, as it's not broken on the large majority of UP cpus :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] smsc: replace WARN_ON() with WARN_ON_SMP()
Thanks ! -Original Message- From: David Miller [mailto:da...@davemloft.net] Sent: Tuesday, August 12, 2014 3:09 AM To: Sharma, Sanjeev Cc: gre...@linuxfoundation.org; steve.glendinn...@shawell.net; net...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] smsc: replace WARN_ON() with WARN_ON_SMP() From: Sanjeev Sharma Date: Mon, 11 Aug 2014 13:46:23 +0530 > spin_is_locked() always return false in uniprocessor configuration and > therefore it would be advise to repalce with WARN_ON_SMP(). > > Signed-off-by: Sanjeev Sharma This looks fine, applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] smsc: replace WARN_ON() with WARN_ON_SMP()
Ok Greg probably Steve will have a look. Regards Sanjeev Sharma -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Monday, August 11, 2014 1:50 PM To: Sharma, Sanjeev Cc: steve.glendinn...@shawell.net; net...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] smsc: replace WARN_ON() with WARN_ON_SMP() On Mon, Aug 11, 2014 at 01:46:23PM +0530, Sanjeev Sharma wrote: > spin_is_locked() always return false in uniprocessor configuration and > therefore it would be advise to repalce with WARN_ON_SMP(). > > Signed-off-by: Sanjeev Sharma > --- > drivers/net/ethernet/smsc/smsc911x.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Hint, I have nothing to do with this file, no need to send a patch for it to me, I can't do anything with it... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] uas: replace WARN_ON_ONCE() with assert_spin_locked().
Hi Greg, Please find my comment in line. -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Monday, August 11, 2014 1:58 PM To: Sharma, Sanjeev Cc: hdego...@redhat.com; kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-s...@vger.kernel.org Subject: Re: [PATCH] uas: replace WARN_ON_ONCE() with assert_spin_locked(). On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: > spin_is_locked() always return false in uniprocessor configuration Really? On what processor type? I don't see that being the case on a few processors, but I didn't check them all, or I might be totally confused with all of the arch_spin_is_locked() definitions in the tree. specially in case of mips and powerpc but we can configure our kernel to uniprocessor configuration for specific processor and in that case we should be extra cautious. Please check spinlock_up.h > and therefore it > would be advise to repalce with assert_spin_locked(). This implies that all uses of this function is wrong and should be replaced and removed, right? Yes and there are only few places in driver for which I am submitting patches which need to be change and at other places it has been already taken care. Please have an look into the Old discussion for more details. http://linux-kernel.2935.n7.nabble.com/Is-spin-is-locked-safe-to-use-with-BUG-ON-WARN-ON-td654800.html#a921802 thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] staging:r819xU: coding style: Fixed commenting style
Opps, I forget. Let me correct and send V2 patch. Regards Sanjeev Sharma -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Thursday, August 07, 2014 9:33 PM To: Sharma, Sanjeev Cc: de...@driverdev.osuosl.org; oor...@gmail.com; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:r819xU: coding style: Fixed commenting style On Thu, Aug 07, 2014 at 12:15:57PM +0530, Sanjeev Sharma wrote: > This is a patch to the r819xU_phyreg.h file that fixes commenting > style warning > > Signed-off-by: Sanjeev Sharma > --- > drivers/staging/rtl8192u/r819xU_phyreg.h | 188 > --- > 1 file changed, 97 insertions(+), 91 deletions(-) > > diff --git a/drivers/staging/rtl8192u/r819xU_phyreg.h > b/drivers/staging/rtl8192u/r819xU_phyreg.h > index 64285d6..f07d2f1 100644 > --- a/drivers/staging/rtl8192u/r819xU_phyreg.h > +++ b/drivers/staging/rtl8192u/r819xU_phyreg.h > @@ -2,10 +2,10 @@ > #define _R819XU_PHYREG_H > > > -#define RF_DATA0x1d4 > // FW will write RF data in the register. > +#define RF_DATA0x1d4 > /* FW will write RF data in the register.*/ > > -//Register //duplicate register due to connection: RF_Mode, TRxRN, NumOf > L-STF > -//page 1 > +/* Register //duplicate register due to connection: RF_Mode, TRxRN, NumOf > L-STF */ Does that line look correct? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 3/3] staging:r8190: coding style: Fixed checkpatch reported Error
Hello All, I have submitted few patches last week and also get reply from Greg that patches will show up in linux-next tree and in parallel I need to submit new patches and now Looks like I need to Sync my tree with linux-next tree before start working on New set of change and as soon as I am pulling change I always get conflicts. Anyone has idea who to overcome this problem ? Regards Sanjeev Sharma -Original Message- From: Sanjeev Sharma [mailto:sanjeev_sha...@mentor.com] Sent: Thursday, July 31, 2014 11:14 AM To: gre...@linuxfoundation.org Cc: oor...@gmail.com; de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Sharma, Sanjeev; Sharma, Sanjeev Subject: [PATCH v2 3/3] staging:r8190: coding style: Fixed checkpatch reported Error This is a patch to the r8190_rtl8256.c file that fixes checkpatch reported space & coding style issues. Signed-off-by: Sanjeev Sharma --- Changes in v2: - Added space character in the signed-off-by field. drivers/staging/rtl8192u/r8190_rtl8256.c | 169 +++ 1 file changed, 79 insertions(+), 90 deletions(-) diff --git a/drivers/staging/rtl8192u/r8190_rtl8256.c b/drivers/staging/rtl8192u/r8190_rtl8256.c index 08e1bc9..43ed768 100644 --- a/drivers/staging/rtl8192u/r8190_rtl8256.c +++ b/drivers/staging/rtl8192u/r8190_rtl8256.c @@ -23,62 +23,64 @@ * Return: NONE * Note: 8226 support both 20M and 40 MHz *---*/ -void PHY_SetRF8256Bandwidth(struct net_device *dev , HT_CHANNEL_WIDTH Bandwidth) //20M or 40M +void PHY_SetRF8256Bandwidth(struct net_device *dev , HT_CHANNEL_WIDTH +Bandwidth) { u8 eRFPath; struct r8192_priv *priv = ieee80211_priv(dev); - //for(eRFPath = RF90_PATH_A; eRFPath NumTotalRFPath; eRFPath++) - for(eRFPath = 0; eRFPath NumTotalRFPath; +* eRFPath++) +*/ + for (eRFPath = 0; eRFPath < RF90_PATH_MAX; eRFPath++) { if (!rtl8192_phy_CheckIsLegalRFPath(dev, eRFPath)) continue; - switch (Bandwidth) - { - case HT_CHANNEL_WIDTH_20: - if(priv->card_8192_version == VERSION_819xU_A || priv->card_8192_version == VERSION_819xU_B)// 8256 D-cut, E-cut, xiong: consider it later! - { - rtl8192_phy_SetRFReg(dev, (RF90_RADIO_PATH_E)eRFPath, 0x0b, bMask12Bits, 0x100); //phy para:1ba - rtl8192_phy_SetRFReg(dev, (RF90_RADIO_PATH_E)eRFPath, 0x2c, bMask12Bits, 0x3d7); - rtl8192_phy_SetRFReg(dev, (RF90_RADIO_PATH_E)eRFPath, 0x0e, bMask12Bits, 0x021); - - //cosa add for sd3's request 01/23/2008 - rtl8192_phy_SetRFReg(dev, (RF90_RADIO_PATH_E)eRFPath, 0x14, bMask12Bits, 0x5ab); - } - else - { + switch (Bandwidth) { + case HT_CHANNEL_WIDTH_20: + if (priv->card_8192_version == VERSION_819xU_A + || priv->card_8192_version + == VERSION_819xU_B) { /* 8256 D-cut, E-cut, xiong: consider it later! */ + rtl8192_phy_SetRFReg(dev, + (RF90_RADIO_PATH_E)eRFPath, + 0x0b, bMask12Bits, 0x100); /* phy para:1ba */ + rtl8192_phy_SetRFReg(dev, + (RF90_RADIO_PATH_E)eRFPath, + 0x2c, bMask12Bits, 0x3d7); + rtl8192_phy_SetRFReg(dev, + (RF90_RADIO_PATH_E)eRFPath, + 0x0e, bMask12Bits, 0x021); + + /* cosa add for sd3's request 01/23/2008 +*/ + rtl8192_phy_SetRFReg(dev, + (RF90_RADIO_PATH_E)eRFPath, + 0x14, bMask12Bits, 0x5ab); + } else { RT_TRACE(COMP_ERR, "PHY_SetRF8256Bandwidth(): unknown hardware version\n"); - } - + } break; - case HT_CHANNEL_WIDTH_20_40: - if(priv->card_8192_version == VERSION_819xU_A ||priv->card_8192_version == VERSION_819xU_B)//
RE: [PATCH] staging:r8190: coding style: Fixed checkpatch reported Error
Hi Greg, I have resent all the patches in order. Please review. Regards Sanjeev Sharma -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Thursday, July 31, 2014 5:51 AM To: Sharma, Sanjeev Cc: de...@driverdev.osuosl.org; oor...@gmail.com; linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging:r8190: coding style: Fixed checkpatch reported Error On Tue, Jul 29, 2014 at 04:41:53PM +0530, Sanjeev Sharma wrote: > This is a patch to the r8190_rtl8256.c file that fixes checkpatch > reported space & coding style issues. > > Signed-off-by: Sanjeev Sharma Please use a ' ' character... Please resend all of your patches, they don't have signed-off-by lines, and I don't know what order to apply them in, even if I could do so. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] staging: android: Fixed missing blank line
Hello Greg, I didn't received automated email. Regards Sanjeev Sharma -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Wednesday, July 23, 2014 8:55 PM To: Sharma, Sanjeev Cc: de...@driverdev.osuosl.org; swetl...@google.com; way...@gmail.com; linux-kernel@vger.kernel.org; dan...@ffwll.ch Subject: Re: [PATCH] staging: android: Fixed missing blank line On Wed, Jul 23, 2014 at 10:08:44AM +0000, Sharma, Sanjeev wrote: > Thanks, so this is also available in next kernel release version. I don't understand the question. You should have gotten an automated email when the patch was applied that explained where the patch was now located, and when it would be merged into Linus's tree. Did you not get that? greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] staging: android: Fixed missing blank line
Thanks, so this is also available in next kernel release version. Regards Sanjeev Sharma -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Saturday, July 19, 2014 4:46 AM To: Sharma, Sanjeev Cc: de...@driverdev.osuosl.org; way...@gmail.com; swetl...@google.com; linux-kernel@vger.kernel.org; dan...@ffwll.ch Subject: Re: [PATCH] staging: android: Fixed missing blank line On Fri, Jul 18, 2014 at 10:17:54AM +0530, Sanjeev Sharma wrote: > This patch will add an blank line after declaration reported by > checkpatch.pl script. > > Signed-off-by: Sanjeev Sharma > --- > drivers/staging/android/sw_sync.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/staging/android/sw_sync.c > b/drivers/staging/android/sw_sync.c > index a76db3f..863d4b1 100644 > --- a/drivers/staging/android/sw_sync.c > +++ b/drivers/staging/android/sw_sync.c > @@ -97,6 +97,7 @@ static void sw_sync_pt_value_str(struct sync_pt *sync_pt, > char *str, int size) > { > struct sw_sync_pt *pt = (struct sw_sync_pt *)sync_pt; > + > snprintf(str, size, "%d", pt->value); } > > @@ -156,6 +157,7 @@ static int sw_sync_open(struct inode *inode, > struct file *file) static int sw_sync_release(struct inode *inode, > struct file *file) { > struct sw_sync_timeline *obj = file->private_data; > + > sync_timeline_destroy(&obj->obj); > return 0; > } I already applied a previous version of this patch, with your gmail address :( -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] staging: android: Fixed missing blank line
Done ! ,Please review now. Regards Sanjeev Sharma -Original Message- From: Dan Carpenter [mailto:dan.carpen...@oracle.com] Sent: Thursday, July 17, 2014 2:41 PM To: Sharma, Sanjeev Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; way...@gmail.com; swetl...@google.com; linux-kernel@vger.kernel.org; dan...@ffwll.ch Subject: Re: [PATCH v2] staging: android: Fixed missing blank line On Thu, Jul 17, 2014 at 02:43:27PM +0530, sanjeev sharma wrote: > From: sanjeevs1 Only use this if you are sending on behalf of someone else. > > This patch will add an blank line after declaration reported by > checkpatch.pl script. > > Signed-off-by: Sanjeev Sharma > --- > Changes in v2: > - Fixed frm header Send these to yourself to test. Also look at your email. Bad - From: sanjeev sharma Good - From: Sanjeev Sharma regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 1/4] staging: rtl8192u: Remove useless return statements.
Thanks Greg for review. Change has been just pushed now with version 4. Regards Sanjeev Sharma -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Thursday, July 10, 2014 12:55 AM To: sanjeev sharma Cc: de...@driverdev.osuosl.org; peter.se...@gmail.com; linux-kernel@vger.kernel.org; rmf...@gmail.com; teobal...@gmail.com; Sharma, Sanjeev Subject: Re: [PATCH v3 1/4] staging: rtl8192u: Remove useless return statements. On Wed, Jul 09, 2014 at 05:49:39PM +0530, sanjeev sharma wrote: > From: sanjeev sharma No "S" characters? And your 0/4 Subject: was really odd, please fix up and resend all of these. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 4/4] staging: rtl8192u: Fixed too long lines
Hi Joe, I have incorporated your review comment in version v4. Regards Sanjeev Sharma -Original Message- From: Joe Perches [mailto:j...@perches.com] Sent: Wednesday, July 09, 2014 6:27 PM To: sanjeev sharma Cc: gre...@linuxfoundation.org; rmf...@gmail.com; peter.se...@gmail.com; teobal...@gmail.com; de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Sharma, Sanjeev Subject: Re: [PATCH v3 4/4] staging: rtl8192u: Fixed too long lines On Wed, 2014-07-09 at 17:49 +0530, sanjeev sharma wrote: > This patch will fix too long lines warning reported by checkpatch.pl. Hi Sanjeev. > diff --git a/drivers/staging/rtl8192u/r819xU_phy.c > b/drivers/staging/rtl8192u/r819xU_phy.c [] > @@ -1786,7 +1788,9 @@ void InitialGainOperateWorkItemCallBack(struct > work_struct *work) > RT_TRACE(COMP_SCAN, "Scan BBInitialGainRestore 0xa0a is %x\n", >priv->initgain_backup.cca); > > - rtl8192_phy_setTxPower(dev, > priv->ieee80211->current_network.channel); > + rtl8192_phy_setTxPower(dev, > + priv->ieee80211->current_network.channel > +); This one is a little off + rtl8192_phy_setTxPower(dev, + priv->ieee80211->current_network.channel + ); Please align multiline statements to the appropriate open parenthesis using a mix of initial leading tabs followed by the minimal number of spaces required to put the first non-whitespace char at the column immediately after the preceding open paren. Here it's 4 tabs, 7 spaces, not 5 tabs: + rtl8192_phy_setTxPower(dev, + priv->ieee80211->current_network.channel); And please don't use a line that's just a close parenthesis and semicolon. I think it's better to ignore the 80 column limit for these and place them at the end of the preceding line. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2 1/4] staging: rtl8192u: Remove useless return statement in r819xU_phy.c
Thanks Dan, -Original Message- From: Dan Carpenter [mailto:dan.carpen...@oracle.com] Sent: Wednesday, July 09, 2014 1:40 PM To: sanjeev sharma Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; peter.se...@gmail.com; linux-kernel@vger.kernel.org; rmf...@gmail.com; teobal...@gmail.com; Sharma, Sanjeev Subject: Re: [PATCH v2 1/4] staging: rtl8192u: Remove useless return statement in r819xU_phy.c These patches look ok now. The only thing is that the changelogs are a little off. On Wed, Jul 09, 2014 at 11:34:17AM +0530, sanjeev sharma wrote: > From: sanjeev sharma Don't use the From header unless you are sending patches on behalf of someone else. It's better if you can configure your email client so you can send the patches from your @mentor.com address. It would be better if the names were capitalized. I will take care for future patches and configure email client by using mentor address. > > This is a patch to the r819xU_phy.c file that remove unneeded return > statements in code. Please line wrap the changelog at 72 characters (the same as email). Otherwise Greg sometimes fixes them manually... But the patches themselves are nice now. @Greg.,@Dan: Is it Ok if this can be taken care in future. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/