Re: [PATCH 1/2] staging: gdm7240: adding LTE USB driver
On Tue, Jul 23, 2013 at 04:40:29PM +0900, Won Kang wrote: > Thank you so much for the review. I will try to fix them soon. > Should I continue to send patches until reviewers agrees to accept > them? I have no experience with submitting codes, and just not sure > how to follow up. it's first time for me to do this kind of job. > Always CC the mailing list in case someone has a similar question later. Yes. That's basically the process. Greg would probably have taken it as-is, if I hadn't said anything. Maybe he still will if you want. But going through staging is not mandatory if your driver is ok from the start. I don't think staging is going to help you much. I've already commented on most of the style issues. You need reviews from networking people. Otherwise for other maintainers you just fix every complaint or send an email saying why the complaint is wrong. Resend the patch with a [patch 1/2 v2] staging: gdm7240: adding LTE USB driver After the signed off by line put: Signed-off-by: You --- v2: cleanup and minor fixes The same for version 3 and so on. This is a network driver so you really need to CC netdev and linux-wireless. You want to make the linux wireless maintainer, John Linville, happy by Aug 9 so he can merge it in time for v3.12. Aug 9 is not much time, so maybe it's a good idea to sit in staging for one kernel release? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: bcm: Qos: Fix some coding style issues
>From a2f8b299baee0e075d548d2bbf77619373035446 Mon Sep 17 00:00:00 2001 From: Lilis Iskandar Date: Tue, 23 Jul 2013 17:08:37 +0800 Subject: [PATCH] Staging: bcm: Qos: Fix some coding style issues Fix some coding style issues. Signed-off-by: Lilis Iskandar --- drivers/staging/bcm/Qos.c | 600 ++ 1 file changed, 285 insertions(+), 315 deletions(-) diff --git a/drivers/staging/bcm/Qos.c b/drivers/staging/bcm/Qos.c index 8d142a5..a1fc892 100644 --- a/drivers/staging/bcm/Qos.c +++ b/drivers/staging/bcm/Qos.c @@ -4,11 +4,15 @@ This file contains the routines related to Quality of Service. */ #include "headers.h" -static void EThCSGetPktInfo(struct bcm_mini_adapter *Adapter,PVOID pvEthPayload, struct bcm_eth_packet_info *pstEthCsPktInfo); -static BOOLEAN EThCSClassifyPkt(struct bcm_mini_adapter *Adapter,struct sk_buff* skb, struct bcm_eth_packet_info *pstEthCsPktInfo,struct bcm_classifier_rule *pstClassifierRule, B_UINT8 EthCSCupport); +static void EThCSGetPktInfo(struct bcm_mini_adapter *Adapter, PVOID pvEthPayload, + struct bcm_eth_packet_info *pstEthCsPktInfo); +static BOOLEAN EThCSClassifyPkt(struct bcm_mini_adapter *Adapter, struct sk_buff *skb, + struct bcm_eth_packet_info *pstEthCsPktInfo, + struct bcm_classifier_rule *pstClassifierRule, + B_UINT8 EthCSCupport); static USHORT IpVersion4(struct bcm_mini_adapter *Adapter, struct iphdr *iphd, - struct bcm_classifier_rule *pstClassifierRule ); + struct bcm_classifier_rule *pstClassifierRule); static VOID PruneQueue(struct bcm_mini_adapter *Adapter, INT iIndex); @@ -20,30 +24,31 @@ static VOID PruneQueue(struct bcm_mini_adapter *Adapter, INT iIndex); * matches with that of Queue. * * Parameters - pstClassifierRule: Pointer to the packet info structure. -*- ulSrcIP : Source IP address from the packet. +*- ulSrcIP : Source IP address from the packet. * * Returns - TRUE(If address matches) else FAIL . */ -BOOLEAN MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule,ULONG ulSrcIP) +BOOLEAN MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule, ULONG ulSrcIP) { -UCHAR ucLoopIndex=0; - -struct bcm_mini_adapter *Adapter = GET_BCM_ADAPTER(gblpnetdev); - -ulSrcIP=ntohl(ulSrcIP); -if(0 == pstClassifierRule->ucIPSourceAddressLength) - return TRUE; -for(ucLoopIndex=0; ucLoopIndex < (pstClassifierRule->ucIPSourceAddressLength);ucLoopIndex++) -{ - BCM_DEBUG_PRINT(Adapter,DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, "Src Ip Address Mask:0x%x PacketIp:0x%x and Classification:0x%x", (UINT)pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex], (UINT)ulSrcIP, (UINT)pstClassifierRule->stSrcIpAddress.ulIpv6Addr[ucLoopIndex]); - if((pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex] & ulSrcIP)== - (pstClassifierRule->stSrcIpAddress.ulIpv4Addr[ucLoopIndex] & pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex] )) - { + UCHAR ucLoopIndex = 0; + struct bcm_mini_adapter *Adapter = GET_BCM_ADAPTER(gblpnetdev); + + ulSrcIP = ntohl(ulSrcIP); + if (0 == pstClassifierRule->ucIPSourceAddressLength) return TRUE; - } -} -BCM_DEBUG_PRINT(Adapter,DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, "Src Ip Address Not Matched"); - return FALSE; + for (ucLoopIndex = 0; ucLoopIndex < (pstClassifierRule->ucIPSourceAddressLength); ucLoopIndex++) { + BCM_DEBUG_PRINT(Adapter,DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, + "Src Ip Address Mask:0x%x PacketIp:0x%x and Classification:0x%x", + (UINT)pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex], + (UINT)ulSrcIP, (UINT)pstClassifierRule->stSrcIpAddress.ulIpv6Addr[ucLoopIndex]); + if ((pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex] & ulSrcIP) == + (pstClassifierRule->stSrcIpAddress.ulIpv4Addr[ucLoopIndex] & pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex])) + { + return TRUE; + } + } + BCM_DEBUG_PRINT(Adapter,DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, "Src Ip Address Not Matched"); + return FALSE; } @@ -60,24 +65,26 @@ BOOLEAN MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule,ULONG ul */ BOOLEAN MatchDestIpAddress(struct bcm_classifier_rule *pstClassifierRule,ULONG ulDestIP) { - UCHAR ucLoopIndex=0; + U
Re: [PATCH] Staging: bcm: Qos: Fix some coding style issues
Hi hi, Welcome first time submitter newbie. :) The tradition is to reject first time code submissions in the meanest and grumpiest way possible. On Tue, Jul 23, 2013 at 05:20:29PM +0800, Lilis Iskandar wrote: > >From a2f8b299baee0e075d548d2bbf77619373035446 Mon Sep 17 00:00:00 2001 > From: Lilis Iskandar > Date: Tue, 23 Jul 2013 17:08:37 +0800 > Subject: [PATCH] Staging: bcm: Qos: Fix some coding style issues This header stuff is not needed. See if you can figure out how to get rid of it. You are fixing too many kinds of problems at once so the patch is impossible to review. Please break it up into one kind of issue per patch. > - if(FALSE == (bClassificationSucceed = > + if (FALSE == (bClassificationSucceed = > MatchSrcPort(pstClassifierRule, > - ntohs((iphd->protocol == UDP)? > - xprt_hdr->uhdr.source:xprt_hdr->thdr.source > + ntohs((iphd->protocol == UDP)? > + xprt_hdr->uhdr.source : xprt_hdr->thdr.source > + { > break; > + } I don't know why you are adding curly braces here but they don't belong. Braces are explained in CodingStyle. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: Qos: Fix some coding style issues
Hi Dan, I have already accepted my fate when I submitted the patch. Though it's really great that I receive a response so fast :) Regarding the braces, I thought it would be more readable that way because of the long code wrapping inside the "if" parameter. I'll fix it and send the patches again in pieces. Should I send a new mail or reply to this thread? Lilis On Tue, 2013-07-23 at 12:54 +0300, Dan Carpenter wrote: > Hi hi, > > Welcome first time submitter newbie. :) The tradition is to reject > first time code submissions in the meanest and grumpiest way > possible. > > On Tue, Jul 23, 2013 at 05:20:29PM +0800, Lilis Iskandar wrote: > > >From a2f8b299baee0e075d548d2bbf77619373035446 Mon Sep 17 00:00:00 2001 > > From: Lilis Iskandar > > Date: Tue, 23 Jul 2013 17:08:37 +0800 > > Subject: [PATCH] Staging: bcm: Qos: Fix some coding style issues > > This header stuff is not needed. See if you can figure out how to > get rid of it. > > You are fixing too many kinds of problems at once so the patch is > impossible to review. Please break it up into one kind of issue > per patch. > > > - if(FALSE == (bClassificationSucceed = > > + if (FALSE == (bClassificationSucceed = > > MatchSrcPort(pstClassifierRule, > > - ntohs((iphd->protocol == UDP)? > > - xprt_hdr->uhdr.source:xprt_hdr->thdr.source > > + ntohs((iphd->protocol == UDP)? > > + xprt_hdr->uhdr.source : xprt_hdr->thdr.source > > + { > > break; > > + } > > I don't know why you are adding curly braces here but they don't > belong. Braces are explained in CodingStyle. > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: Qos: Fix some coding style issues
On Tue, Jul 23, 2013 at 06:10:41PM +0800, Lilis Iskandar wrote: > Hi Dan, > > I have already accepted my fate when I submitted the patch. Though it's > really great that I receive a response so fast :) > > Regarding the braces, I thought it would be more readable that way > because of the long code wrapping inside the "if" parameter. The way to do that is to use spaces to align the stuff in the if statement. if (long_ blah blah blah blah blah blah == this part lines up with the 'l' in "long_") frob(); Read CodingStyle. One other thing is that in staging we prefer if multi-line indents have braces around them even if it's not needed. Ugly: for (i = 0; i < 100; i++) if (x) { frob(); frob(); frob(); } Nice to look at: for (i = 0; i < 100; i++) { if (x) { frob(); frob(); frob(); } } > I'll fix it and send the patches again in pieces. Should I send a > new mail or reply to this thread? This thread, but it doesn't matter. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: tidspbridge: replace strict_strtol() with kstrtol()
The usage of strict_strtol() is not preferred, because strict_strtol() is obsolete. Thus, kstrtol() should be used. Signed-off-by: Jingoo Han --- drivers/staging/tidspbridge/pmgr/dbll.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/tidspbridge/pmgr/dbll.c b/drivers/staging/tidspbridge/pmgr/dbll.c index c191ae2..82de57a 100644 --- a/drivers/staging/tidspbridge/pmgr/dbll.c +++ b/drivers/staging/tidspbridge/pmgr/dbll.c @@ -1120,8 +1120,10 @@ static int dbll_rmm_alloc(struct dynamic_loader_allocate *this, or DYN_EXTERNAL, then mem granularity information is present within the section name - only process if there are at least three tokens within the section name (just a minor optimization) */ - if (count >= 3) - strict_strtol(sz_last_token, 10, (long *)&req); + if (count >= 3) { + if (kstrtol(sz_last_token, 10, (long *)&req)) + dev_dbg(bridge, "%s: kstrtol failed\n", __func__); + } if ((req == 0) || (req == 1)) { if (strcmp(sz_sec_last_token, "DYN_DARAM") == 0) { -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: iio: replace strict_strto*() with kstrto*()
The usage of strict_strto*() is not preferred, because strict_strto*() is obsolete. Thus, kstrto*() should be used. Signed-off-by: Jingoo Han --- drivers/staging/iio/accel/sca3000_core.c |4 ++-- drivers/staging/iio/accel/sca3000_ring.c |2 +- drivers/staging/iio/addac/adt7316.c| 20 ++-- drivers/staging/iio/frequency/ad9832.c |2 +- drivers/staging/iio/frequency/ad9834.c |2 +- drivers/staging/iio/gyro/adis16260_core.c |2 +- drivers/staging/iio/impedance-analyzer/ad5933.c|4 ++-- drivers/staging/iio/light/isl29018.c |6 +++--- drivers/staging/iio/light/tsl2583.c| 12 ++-- drivers/staging/iio/meter/ade7753.c|6 +++--- drivers/staging/iio/meter/ade7754.c|6 +++--- drivers/staging/iio/meter/ade7758_core.c |6 +++--- drivers/staging/iio/meter/ade7759.c|6 +++--- drivers/staging/iio/meter/ade7854.c|8 drivers/staging/iio/resolver/ad2s1210.c| 10 +- drivers/staging/iio/trigger/iio-trig-bfin-timer.c |2 +- .../staging/iio/trigger/iio-trig-periodic-rtc.c|2 +- 17 files changed, 50 insertions(+), 50 deletions(-) diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c index 32950ad..1dfa1de 100644 --- a/drivers/staging/iio/accel/sca3000_core.c +++ b/drivers/staging/iio/accel/sca3000_core.c @@ -626,7 +626,7 @@ static ssize_t sca3000_set_frequency(struct device *dev, int ctrlval; long val; - ret = strict_strtol(buf, 10, &val); + ret = kstrtol(buf, 10, &val); if (ret) return ret; @@ -936,7 +936,7 @@ static ssize_t sca3000_set_free_fall_mode(struct device *dev, u8 protect_mask = SCA3000_FREE_FALL_DETECT; mutex_lock(&st->lock); - ret = strict_strtol(buf, 10, &val); + ret = kstrtol(buf, 10, &val); if (ret) goto error_ret; diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c index 3e5e860..393100b 100644 --- a/drivers/staging/iio/accel/sca3000_ring.c +++ b/drivers/staging/iio/accel/sca3000_ring.c @@ -181,7 +181,7 @@ static ssize_t sca3000_set_ring_int(struct device *dev, int ret; mutex_lock(&st->lock); - ret = strict_strtol(buf, 10, &val); + ret = kstrtol(buf, 10, &val); if (ret) goto error_ret; ret = sca3000_read_data_short(st, SCA3000_REG_ADDR_INT_MASK, 1); diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c index 506b5a7..a0e8149 100644 --- a/drivers/staging/iio/addac/adt7316.c +++ b/drivers/staging/iio/addac/adt7316.c @@ -418,7 +418,7 @@ static ssize_t adt7316_store_ad_channel(struct device *dev, if (!(chip->config2 & ADT7316_AD_SINGLE_CH_MODE)) return -EPERM; - ret = strict_strtoul(buf, 10, &data); + ret = kstrtoul(buf, 10, &data); if (ret) return -EINVAL; @@ -851,7 +851,7 @@ static ssize_t adt7316_store_DAC_2Vref_ch_mask(struct device *dev, unsigned long data = 0; int ret; - ret = strict_strtoul(buf, 16, &data); + ret = kstrtoul(buf, 16, &data); if (ret || data > ADT7316_DA_2VREF_CH_MASK) return -EINVAL; @@ -909,7 +909,7 @@ static ssize_t adt7316_store_DAC_update_mode(struct device *dev, if (!(chip->config3 & ADT7316_DA_EN_VIA_DAC_LDCA)) return -EPERM; - ret = strict_strtoul(buf, 10, &data); + ret = kstrtoul(buf, 10, &data); if (ret || data > ADT7316_DA_EN_MODE_MASK) return -EINVAL; @@ -966,7 +966,7 @@ static ssize_t adt7316_store_update_DAC(struct device *dev, ADT7316_DA_EN_MODE_LDAC) return -EPERM; - ret = strict_strtoul(buf, 16, &data); + ret = kstrtoul(buf, 16, &data); if (ret || data > ADT7316_LDAC_EN_DA_MASK) return -EINVAL; @@ -1108,7 +1108,7 @@ static ssize_t adt7316_store_DAC_internal_Vref(struct device *dev, int ret; if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX) { - ret = strict_strtoul(buf, 16, &data); + ret = kstrtoul(buf, 16, &data); if (ret || data > 3) return -EINVAL; @@ -1118,7 +1118,7 @@ static ssize_t adt7316_store_DAC_internal_Vref(struct device *dev, else if (data & 0x2) ldac_config |= ADT7516_DAC_CD_IN_VREF; } else { - ret = strict_strtoul(buf, 16, &data); + ret = kstrtoul(buf, 16, &data); if (ret) return -EINVAL; @@ -1310,7 +1310,7 @@ static ssize_t adt7316_store_temp_offset(struct adt7316_chip_info
Re: [PATCH 1/2] staging: tidspbridge: replace strict_strtol() with kstrtol()
On Tue, Jul 23, 2013 at 07:16:21PM +0900, Jingoo Han wrote: > The usage of strict_strtol() is not preferred, because > strict_strtol() is obsolete. Thus, kstrtol() should be > used. > > Signed-off-by: Jingoo Han > --- > drivers/staging/tidspbridge/pmgr/dbll.c |6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/tidspbridge/pmgr/dbll.c > b/drivers/staging/tidspbridge/pmgr/dbll.c > index c191ae2..82de57a 100644 > --- a/drivers/staging/tidspbridge/pmgr/dbll.c > +++ b/drivers/staging/tidspbridge/pmgr/dbll.c > @@ -1120,8 +1120,10 @@ static int dbll_rmm_alloc(struct > dynamic_loader_allocate *this, > or DYN_EXTERNAL, then mem granularity information is present > within the section name - only process if there are at least three > tokens within the section name (just a minor optimization) */ > - if (count >= 3) > - strict_strtol(sz_last_token, 10, (long *)&req); > + if (count >= 3) { > + if (kstrtol(sz_last_token, 10, (long *)&req)) This bug is in the original code as well but you're passing a 32 bit pointer to kstrtol() which sets 64 bits on x86_64. So it will corrupt memory. Also the error handling is pretty bogus. I know you didn't introduce this, but could fix and resend anyway. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: tidspbridge: replace strict_strtol() with kstrtol()
On Tuesday, July 23, 2013 7:34 PM, Dan Carpenter wrote: > On Tue, Jul 23, 2013 at 07:16:21PM +0900, Jingoo Han wrote: > > The usage of strict_strtol() is not preferred, because > > strict_strtol() is obsolete. Thus, kstrtol() should be > > used. > > > > Signed-off-by: Jingoo Han > > --- > > drivers/staging/tidspbridge/pmgr/dbll.c |6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/tidspbridge/pmgr/dbll.c > > b/drivers/staging/tidspbridge/pmgr/dbll.c > > index c191ae2..82de57a 100644 > > --- a/drivers/staging/tidspbridge/pmgr/dbll.c > > +++ b/drivers/staging/tidspbridge/pmgr/dbll.c > > @@ -1120,8 +1120,10 @@ static int dbll_rmm_alloc(struct > > dynamic_loader_allocate *this, > >or DYN_EXTERNAL, then mem granularity information is present > >within the section name - only process if there are at least three > >tokens within the section name (just a minor optimization) */ > > - if (count >= 3) > > - strict_strtol(sz_last_token, 10, (long *)&req); > > + if (count >= 3) { > > + if (kstrtol(sz_last_token, 10, (long *)&req)) > > This bug is in the original code as well but you're passing a 32 bit > pointer to kstrtol() which sets 64 bits on x86_64. So it will > corrupt memory. Do you mean '(long *)&req'? Sorry, but I cannot understand fully what you mean. :( Could you let me know how to solve this? > > Also the error handling is pretty bogus. I know you didn't > introduce this, but could fix and resend anyway. If there is no error handling, it will make build warning. Because, __must_check is added to kstrtol() as below: static inline int __must_check kstrtol(const char *s, unsigned int base, long *res) How about adding this to commit message? Best regards Jingoo Han ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: iio: replace strict_strto*() with kstrto*()
On Tue, Jul 23, 2013 at 07:19:03PM +0900, Jingoo Han wrote: > The usage of strict_strto*() is not preferred, because > strict_strto*() is obsolete. Thus, kstrto*() should be > used. In olden times there was just strict_strtol() and strict_strtoul(). These days we have kstrtou8() and kstrtoint() and a bunch of others. So when you do these patches you have to take more time to consider what type we should be using and update the surrounding code. I haven't reviewed all of these so please check everything again. > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c > b/drivers/staging/iio/impedance-analyzer/ad5933.c > index 6330af6..afe191a 100644 > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c > @@ -326,7 +326,7 @@ static ssize_t ad5933_store_frequency(struct device *dev, > long val; > int ret; > > - ret = strict_strtoul(buf, 10, &val); > + ret = kstrtoul(buf, 10, &val); This bug is in the original code as well. "val" is long but it should be unsigned long. Later we check that: if (val > AD5933_MAX_OUTPUT_FREQ_Hz) { Forgetting that "val" could be a negative number. > diff --git a/drivers/staging/iio/meter/ade7753.c > b/drivers/staging/iio/meter/ade7753.c > index e5943e2..fbb230a 100644 > --- a/drivers/staging/iio/meter/ade7753.c > +++ b/drivers/staging/iio/meter/ade7753.c > @@ -188,7 +188,7 @@ static ssize_t ade7753_write_8bit(struct device *dev, > int ret; > long val; > > - ret = strict_strtol(buf, 10, &val); > + ret = kstrtol(buf, 10, &val); For these we should be using the new kstrtou8() functions. > if (ret) > goto error_ret; > ret = ade7753_spi_write_reg_8(dev, this_attr->address, val); > @@ -206,7 +206,7 @@ static ssize_t ade7753_write_16bit(struct device *dev, > int ret; > long val; > > - ret = strict_strtol(buf, 10, &val); > + ret = kstrtol(buf, 10, &val); Same. > if (ret) > goto error_ret; > ret = ade7753_spi_write_reg_16(dev, this_attr->address, val); > @@ -418,7 +418,7 @@ static ssize_t ade7753_write_frequency(struct device *dev, > int ret; > u16 reg, t; > > - ret = strict_strtol(buf, 10, &val); > + ret = kstrtol(buf, 10, &val); > if (ret) > return ret; > if (val == 0) > diff --git a/drivers/staging/iio/meter/ade7754.c > b/drivers/staging/iio/meter/ade7754.c > index 7b6503b..ea337c4 100644 > --- a/drivers/staging/iio/meter/ade7754.c > +++ b/drivers/staging/iio/meter/ade7754.c > @@ -188,7 +188,7 @@ static ssize_t ade7754_write_8bit(struct device *dev, > int ret; > long val; > > - ret = strict_strtol(buf, 10, &val); > + ret = kstrtol(buf, 10, &val); Same. > if (ret) > goto error_ret; > ret = ade7754_spi_write_reg_8(dev, this_attr->address, val); > @@ -206,7 +206,7 @@ static ssize_t ade7754_write_16bit(struct device *dev, > int ret; > long val; > > - ret = strict_strtol(buf, 10, &val); > + ret = kstrtol(buf, 10, &val); Same. > if (ret) > goto error_ret; > ret = ade7754_spi_write_reg_16(dev, this_attr->address, val); > --- a/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c > +++ b/drivers/staging/iio/trigger/iio-trig-periodic-rtc.c > @@ -56,7 +56,7 @@ static ssize_t iio_trig_periodic_write_freq(struct device > *dev, > unsigned long val; > int ret; > > - ret = strict_strtoul(buf, 10, &val); > + ret = kstrtoul(buf, 10, &val); For this one we should make "val" an int. > if (ret) > goto error_ret; > regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: tidspbridge: replace strict_strtol() with kstrtol()
The following test program illustrates the memory corruption. You would hope foo.b would be 42 but it is corrupted to -1. #include #include struct foo { int a, b; }; void kstrtol(long *x) { *x = -1; } int main(void) { struct foo foo; foo.b = 42; kstrtol((long *)&foo.a); printf("%d %d\n", foo.a, foo.b); return 0; } The error handling should return an error, it shouldn't just print something. It shouldn't print anything actually, it should just handle the error without printing or complaining. :P regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: iio: replace strict_strto*() with kstrto*()
On Tue, Jul 23, 2013 at 07:19:03PM +0900, Jingoo Han wrote: > diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c > b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c > index 38a158b..03766bb 100644 > --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c > +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c > @@ -87,7 +87,7 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device > *dev, > bool enabled; > int ret; > > - ret = strict_strtoul(buf, 10, &val); > + ret = kstrtoul(buf, 10, &val); > if (ret) > goto error_ret; > Btw, this function is not beautiful. drivers/staging/iio/trigger/iio-trig-bfin-timer.c 81 static ssize_t iio_bfin_tmr_frequency_store(struct device *dev, 82 struct device_attribute *attr, const char *buf, size_t count) 83 { 84 struct iio_trigger *trig = to_iio_trigger(dev); 85 struct bfin_tmr_state *st = iio_trigger_get_drvdata(trig); 86 unsigned long val; 87 bool enabled; 88 int ret; 89 90 ret = strict_strtoul(buf, 10, &val); 91 if (ret) 92 goto error_ret; I have updated CodingStyle to reflect that we are encouraged to return directly here instead of doing bogus gotos which imply error handly but in fact do nothing at all. 93 94 if (val > 10) { 95 ret = -EINVAL; 96 goto error_ret; 97 } 98 99 enabled = get_enabled_gptimers() & st->t->bit; 100 101 if (enabled) 102 disable_gptimers(st->t->bit); 103 104 if (!val) 105 goto error_ret; So at this point we have disabled disable_gptimers(). The goto is called "error_ret" which says "error" but actually we are returning success. It's easy to imagine that "val == 0" is invalid because that would cause a divide by zero. But on the other hand maybe zero has a special meaning which is that we should disable gptimers and return success? If the code said: if (enabled) disable_gptimers(st->t->bit); if (val == 0) return count; [---blank line---] val = get_sclk() / val; That would be totally clear what was intended. 106 107 val = get_sclk() / val; 108 if (val <= 4 || val <= st->duty) { 109 ret = -EINVAL; 110 goto error_ret; I'm pretty sure this is a bug. We want to enable gptimers if "val" is totally invalid. 111 } 112 113 set_gptimer_period(st->t->id, val); 114 set_gptimer_pwidth(st->t->id, val - st->duty); 115 116 if (enabled) 117 enable_gptimers(st->t->bit); 118 119 error_ret: 120 return ret ? ret : count; 121 } regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: iio: replace strict_strto*() with kstrto*()
On 07/23/2013 01:26 PM, Dan Carpenter wrote: > On Tue, Jul 23, 2013 at 07:19:03PM +0900, Jingoo Han wrote: >> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c >> b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c >> index 38a158b..03766bb 100644 >> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c >> +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c >> @@ -87,7 +87,7 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device >> *dev, >> bool enabled; >> int ret; >> >> -ret = strict_strtoul(buf, 10, &val); >> +ret = kstrtoul(buf, 10, &val); >> if (ret) >> goto error_ret; >> > > Btw, this function is not beautiful. The whole driver is not beautiful ;) It will eventually be replaced with something more generic, I wouldn't put too much effort into cleaning it up. - Lars ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: Qos: Fix some coding style issues
Fixed spacing/tabs issues that were found using checkpatch. Signed-off-by: Lilis Iskandar --- drivers/staging/bcm/Qos.c | 538 +++--- 1 file changed, 269 insertions(+), 269 deletions(-) diff --git a/drivers/staging/bcm/Qos.c b/drivers/staging/bcm/Qos.c index 8d142a5..2d4a77c 100644 --- a/drivers/staging/bcm/Qos.c +++ b/drivers/staging/bcm/Qos.c @@ -4,11 +4,11 @@ This file contains the routines related to Quality of Service. */ #include "headers.h" -static void EThCSGetPktInfo(struct bcm_mini_adapter *Adapter,PVOID pvEthPayload, struct bcm_eth_packet_info *pstEthCsPktInfo); -static BOOLEAN EThCSClassifyPkt(struct bcm_mini_adapter *Adapter,struct sk_buff* skb, struct bcm_eth_packet_info *pstEthCsPktInfo,struct bcm_classifier_rule *pstClassifierRule, B_UINT8 EthCSCupport); +static void EThCSGetPktInfo(struct bcm_mini_adapter *Adapter, PVOID pvEthPayload, struct bcm_eth_packet_info *pstEthCsPktInfo); +static BOOLEAN EThCSClassifyPkt(struct bcm_mini_adapter *Adapter, struct sk_buff* skb, struct bcm_eth_packet_info *pstEthCsPktInfo, struct bcm_classifier_rule *pstClassifierRule, B_UINT8 EthCSCupport); static USHORT IpVersion4(struct bcm_mini_adapter *Adapter, struct iphdr *iphd, - struct bcm_classifier_rule *pstClassifierRule ); + struct bcm_classifier_rule *pstClassifierRule); static VOID PruneQueue(struct bcm_mini_adapter *Adapter, INT iIndex); @@ -20,30 +20,30 @@ static VOID PruneQueue(struct bcm_mini_adapter *Adapter, INT iIndex); * matches with that of Queue. * * Parameters - pstClassifierRule: Pointer to the packet info structure. -*- ulSrcIP : Source IP address from the packet. +* - ulSrcIP : Source IP address from the packet. * * Returns - TRUE(If address matches) else FAIL . */ -BOOLEAN MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule,ULONG ulSrcIP) +BOOLEAN MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule, ULONG ulSrcIP) { -UCHAR ucLoopIndex=0; - -struct bcm_mini_adapter *Adapter = GET_BCM_ADAPTER(gblpnetdev); - -ulSrcIP=ntohl(ulSrcIP); -if(0 == pstClassifierRule->ucIPSourceAddressLength) - return TRUE; -for(ucLoopIndex=0; ucLoopIndex < (pstClassifierRule->ucIPSourceAddressLength);ucLoopIndex++) -{ - BCM_DEBUG_PRINT(Adapter,DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, "Src Ip Address Mask:0x%x PacketIp:0x%x and Classification:0x%x", (UINT)pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex], (UINT)ulSrcIP, (UINT)pstClassifierRule->stSrcIpAddress.ulIpv6Addr[ucLoopIndex]); - if((pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex] & ulSrcIP)== - (pstClassifierRule->stSrcIpAddress.ulIpv4Addr[ucLoopIndex] & pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex] )) - { - return TRUE; - } -} -BCM_DEBUG_PRINT(Adapter,DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, "Src Ip Address Not Matched"); - return FALSE; + UCHAR ucLoopIndex = 0; + + struct bcm_mini_adapter *Adapter = GET_BCM_ADAPTER(gblpnetdev); + + ulSrcIP = ntohl(ulSrcIP); + if (0 == pstClassifierRule->ucIPSourceAddressLength) + return TRUE; + for (ucLoopIndex = 0; ucLoopIndex < (pstClassifierRule->ucIPSourceAddressLength); ucLoopIndex++) + { + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, "Src Ip Address Mask:0x%x PacketIp:0x%x and Classification:0x%x", (UINT)pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex], (UINT)ulSrcIP, (UINT)pstClassifierRule->stSrcIpAddress.ulIpv6Addr[ucLoopIndex]); + if ((pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex] & ulSrcIP) == + (pstClassifierRule->stSrcIpAddress.ulIpv4Addr[ucLoopIndex] & pstClassifierRule->stSrcIpAddress.ulIpv4Mask[ucLoopIndex])) + { + return TRUE; + } + } + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, "Src Ip Address Not Matched"); + return FALSE; } @@ -54,30 +54,30 @@ BOOLEAN MatchSrcIpAddress(struct bcm_classifier_rule *pstClassifierRule,ULONG ul * matches with that of Queue. * * Parameters - pstClassifierRule: Pointer to the packet info structure. -*- ulDestIP: Destination IP address from the packet. +* - ulDestIP: Destination IP address from the packet. * * Returns - TRUE(If address matches) else FAIL . */ -BOOLEAN MatchDestIpAddress(struct bcm_classifier_rule *pstClassifierRule,ULONG ulDestIP) +BOOLEAN MatchDestIpAddress(struct bcm_classifier_ru
[PATCH v3 0/5] staging: ozwpan: Replace debug macro
This is v3 of this original patch series here:- http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2013-June/039280.html v3 adds commit log for each patch as suggested by Joe here:- http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2013-July/039361.html Joe Perches (5): staging: ozwpan: Remove extra debug logs. staging: ozwpan: Replace oz_trace with oz_dbg staging: ozwpan: Remove old debug macro. staging: ozwpan: Convert macro to function. staging: ozwpan: Rename Kbuild to Makefile drivers/staging/ozwpan/Kbuild | 18 -- drivers/staging/ozwpan/Makefile| 16 ++ drivers/staging/ozwpan/ozcdev.c| 52 +++--- drivers/staging/ozwpan/ozconfig.h | 26 --- drivers/staging/ozwpan/ozdbg.h | 54 ++ drivers/staging/ozwpan/ozeltbuf.c | 32 ++-- drivers/staging/ozwpan/ozhcd.c | 296 +++- drivers/staging/ozwpan/ozmain.c|7 +- drivers/staging/ozwpan/ozpd.c | 67 drivers/staging/ozwpan/ozproto.c | 60 +++ drivers/staging/ozwpan/ozproto.h |2 +- drivers/staging/ozwpan/oztrace.c | 36 drivers/staging/ozwpan/oztrace.h | 35 drivers/staging/ozwpan/ozurbparanoia.c | 14 +- drivers/staging/ozwpan/ozurbparanoia.h |4 +- drivers/staging/ozwpan/ozusbsvc.c | 25 +-- drivers/staging/ozwpan/ozusbsvc1.c | 19 +- 17 files changed, 348 insertions(+), 415 deletions(-) delete mode 100644 drivers/staging/ozwpan/Kbuild create mode 100644 drivers/staging/ozwpan/Makefile delete mode 100644 drivers/staging/ozwpan/ozconfig.h create mode 100644 drivers/staging/ozwpan/ozdbg.h delete mode 100644 drivers/staging/ozwpan/oztrace.c delete mode 100644 drivers/staging/ozwpan/oztrace.h -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/5] staging: ozwpan: Remove old debug macro.
From: Joe Perches Remove old oz_trace & oz_trace2 macro & related header files. Signed-off-by: Joe Perches Signed-off-by: Rupesh Gujare --- drivers/staging/ozwpan/Kbuild |6 ++ drivers/staging/ozwpan/ozcdev.c|2 -- drivers/staging/ozwpan/ozconfig.h | 26 --- drivers/staging/ozwpan/ozeltbuf.c |2 -- drivers/staging/ozwpan/ozhcd.c |7 +-- drivers/staging/ozwpan/ozmain.c|7 +-- drivers/staging/ozwpan/ozpd.c |6 ++ drivers/staging/ozwpan/ozproto.c |4 ++-- drivers/staging/ozwpan/oztrace.c | 36 drivers/staging/ozwpan/oztrace.h | 35 --- drivers/staging/ozwpan/ozurbparanoia.c |5 +++-- drivers/staging/ozwpan/ozusbsvc.c |4 ++-- drivers/staging/ozwpan/ozusbsvc1.c |2 -- 13 files changed, 17 insertions(+), 125 deletions(-) delete mode 100644 drivers/staging/ozwpan/ozconfig.h delete mode 100644 drivers/staging/ozwpan/oztrace.c delete mode 100644 drivers/staging/ozwpan/oztrace.h diff --git a/drivers/staging/ozwpan/Kbuild b/drivers/staging/ozwpan/Kbuild index 1766a26..29529c1 100644 --- a/drivers/staging/ozwpan/Kbuild +++ b/drivers/staging/ozwpan/Kbuild @@ -2,6 +2,7 @@ # Copyright (c) 2011 Ozmo Inc # Released under the GNU General Public License Version 2 (GPLv2). # - + obj-$(CONFIG_USB_WPAN_HCD) += ozwpan.o ozwpan-y := \ ozmain.o \ @@ -12,7 +13,4 @@ ozwpan-y := \ ozeltbuf.o \ ozproto.o \ ozcdev.o \ - ozurbparanoia.o \ - oztrace.o - - + ozurbparanoia.o diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c index 87c3131..3e29760 100644 --- a/drivers/staging/ozwpan/ozcdev.c +++ b/drivers/staging/ozwpan/ozcdev.c @@ -11,10 +11,8 @@ #include #include #include -#include "ozconfig.h" #include "ozdbg.h" #include "ozprotocol.h" -#include "oztrace.h" #include "ozappif.h" #include "ozeltbuf.h" #include "ozpd.h" diff --git a/drivers/staging/ozwpan/ozconfig.h b/drivers/staging/ozwpan/ozconfig.h deleted file mode 100644 index 087c322..000 --- a/drivers/staging/ozwpan/ozconfig.h +++ /dev/null @@ -1,26 +0,0 @@ -/* - - * Copyright (c) 2011 Ozmo Inc - * Released under the GNU General Public License Version 2 (GPLv2). - * ---*/ -#ifndef _OZCONFIG_H -#define _OZCONFIG_H - -/* #define WANT_TRACE */ -#ifdef WANT_TRACE -#define WANT_VERBOSE_TRACE -#endif /* #ifdef WANT_TRACE */ -/* #define WANT_URB_PARANOIA */ - -/* #define WANT_PRE_2_6_39 */ - -/* These defines determine what verbose trace is displayed. */ -#ifdef WANT_VERBOSE_TRACE -/* #define WANT_TRACE_STREAM */ -/* #define WANT_TRACE_URB */ -/* #define WANT_TRACE_CTRL_DETAIL */ -#define WANT_TRACE_HUB -/* #define WANT_TRACE_RX_FRAMES */ -/* #define WANT_TRACE_TX_FRAMES */ -#endif /* WANT_VERBOSE_TRACE */ - -#endif /* _OZCONFIG_H */ diff --git a/drivers/staging/ozwpan/ozeltbuf.c b/drivers/staging/ozwpan/ozeltbuf.c index bf280aa..5e98aeb 100644 --- a/drivers/staging/ozwpan/ozeltbuf.c +++ b/drivers/staging/ozwpan/ozeltbuf.c @@ -6,12 +6,10 @@ #include #include #include -#include "ozconfig.h" #include "ozdbg.h" #include "ozprotocol.h" #include "ozeltbuf.h" #include "ozpd.h" -#include "oztrace.h" /*-- */ diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c index fece2da..6b16bfc 100644 --- a/drivers/staging/ozwpan/ozhcd.c +++ b/drivers/staging/ozwpan/ozhcd.c @@ -32,9 +32,7 @@ #include "linux/usb/hcd.h" #include #include "ozdbg.h" -#include "ozconfig.h" #include "ozusbif.h" -#include "oztrace.h" #include "ozurbparanoia.h" #include "ozhcd.h" /*-- @@ -797,7 +795,6 @@ void oz_hcd_get_desc_cnf(void *hport, u8 req_id, int status, const u8 *desc, /*-- * Context: softirq */ -#ifdef WANT_TRACE static void oz_display_conf_type(u8 t) { switch (t) { @@ -836,9 +833,7 @@ static void oz_display_conf_type(u8 t) break; } } -#else -#define oz_display_conf_type(__x) -#endif /* WANT_TRACE */ + /*-- * Context: softirq */ diff --git a/drivers/staging/ozwpan/ozmain.c b/drivers/staging/ozwpan/ozmain.c index 51fe9e9..e26d6be 100644 --- a/drivers/staging/ozwpan/ozmain.c +++ b/drivers/staging/ozwpan/ozmain.c @@ -3,6 +3,7 @@ * Released under the GNU General Public License Version 2 (GPLv2). * - */
[PATCH v3 1/5] staging: ozwpan: Remove extra debug logs.
From: Joe Perches Remove unnecessary debug logs. Most of these logs print function name at the start of function, which are not really required. Signed-off-by: Joe Perches Signed-off-by: Rupesh Gujare --- drivers/staging/ozwpan/ozcdev.c |2 -- drivers/staging/ozwpan/ozhcd.c | 28 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c index 374fdc3..284c26d 100644 --- a/drivers/staging/ozwpan/ozcdev.c +++ b/drivers/staging/ozwpan/ozcdev.c @@ -73,7 +73,6 @@ static void oz_cdev_release_ctx(struct oz_serial_ctx *ctx) static int oz_cdev_open(struct inode *inode, struct file *filp) { struct oz_cdev *dev; - oz_trace("oz_cdev_open()\n"); oz_trace("major = %d minor = %d\n", imajor(inode), iminor(inode)); dev = container_of(inode->i_cdev, struct oz_cdev, cdev); filp->private_data = dev; @@ -84,7 +83,6 @@ static int oz_cdev_open(struct inode *inode, struct file *filp) */ static int oz_cdev_release(struct inode *inode, struct file *filp) { - oz_trace("oz_cdev_release()\n"); return 0; } /*-- diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c index d68d63a..9d1bd44 100644 --- a/drivers/staging/ozwpan/ozhcd.c +++ b/drivers/staging/ozwpan/ozhcd.c @@ -394,7 +394,6 @@ static void oz_complete_urb(struct usb_hcd *hcd, struct urb *urb, */ static void oz_ep_free(struct oz_port *port, struct oz_endpoint *ep) { - oz_trace("oz_ep_free()\n"); if (port) { struct list_head list; struct oz_hcd *ozhcd = port->ozhcd; @@ -631,7 +630,6 @@ void *oz_hcd_pd_arrived(void *hpd) void *hport = NULL; struct oz_hcd *ozhcd = NULL; struct oz_endpoint *ep; - oz_trace("oz_hcd_pd_arrived()\n"); ozhcd = oz_hcd_claim(); if (ozhcd == NULL) return NULL; @@ -691,7 +689,6 @@ void oz_hcd_pd_departed(void *hport) void *hpd; struct oz_endpoint *ep = NULL; - oz_trace("oz_hcd_pd_departed()\n"); if (port == NULL) { oz_trace("oz_hcd_pd_departed() port = 0\n"); return; @@ -1696,7 +1693,6 @@ static void oz_hcd_clear_orphanage(struct oz_hcd *ozhcd, int status) */ static int oz_hcd_start(struct usb_hcd *hcd) { - oz_trace("oz_hcd_start()\n"); hcd->power_budget = 200; hcd->state = HC_STATE_RUNNING; hcd->uses_new_polling = 1; @@ -1707,14 +1703,12 @@ static int oz_hcd_start(struct usb_hcd *hcd) */ static void oz_hcd_stop(struct usb_hcd *hcd) { - oz_trace("oz_hcd_stop()\n"); } /*-- * Context: unknown */ static void oz_hcd_shutdown(struct usb_hcd *hcd) { - oz_trace("oz_hcd_shutdown()\n"); } /*-- * Called to queue an urb for the device. @@ -1844,7 +1838,6 @@ static int oz_hcd_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) static void oz_hcd_endpoint_disable(struct usb_hcd *hcd, struct usb_host_endpoint *ep) { - oz_trace("oz_hcd_endpoint_disable\n"); } /*-- * Context: unknown @@ -1852,7 +1845,6 @@ static void oz_hcd_endpoint_disable(struct usb_hcd *hcd, static void oz_hcd_endpoint_reset(struct usb_hcd *hcd, struct usb_host_endpoint *ep) { - oz_trace("oz_hcd_endpoint_reset\n"); } /*-- * Context: unknown @@ -1872,7 +1864,6 @@ static int oz_hcd_hub_status_data(struct usb_hcd *hcd, char *buf) struct oz_hcd *ozhcd = oz_hcd_private(hcd); int i; - oz_trace2(OZ_TRACE_HUB, "oz_hcd_hub_status_data()\n"); buf[0] = 0; spin_lock_bh(&ozhcd->hcd_lock); @@ -1892,7 +1883,6 @@ static int oz_hcd_hub_status_data(struct usb_hcd *hcd, char *buf) static void oz_get_hub_descriptor(struct usb_hcd *hcd, struct usb_hub_descriptor *desc) { - oz_trace2(OZ_TRACE_HUB, "GetHubDescriptor\n"); memset(desc, 0, sizeof(*desc)); desc->bDescriptorType = 0x29; desc->bDescLength = 9; @@ -1911,7 +1901,7 @@ static int oz_set_port_feature(struct usb_hcd *hcd, u16 wvalue, u16 windex) struct oz_hcd *ozhcd = oz_hcd_private(hcd); unsigned set_bits = 0; unsigned clear_bits = 0; - oz_trace2(OZ_TRACE_HUB, "SetPortFeature\n"); + if ((port_id < 1) || (port_id > OZ_NB_PORTS)) return -EPIPE; port = &ozhcd->ports[port_id-1]; @@ -1986,7 +1976,7 @@ static int oz_clear_port_feature(struct usb_hcd *hcd, u16 wvalue, u16 windex) u8 port_id = (u8)
[PATCH v3 4/5] staging: ozwpan: Convert macro to function.
From: Joe Perches Replace macro with inline function. Signed-off-by: Joe Perches Signed-off-by: Rupesh Gujare --- drivers/staging/ozwpan/ozurbparanoia.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ozwpan/ozurbparanoia.h b/drivers/staging/ozwpan/ozurbparanoia.h index 00f5a3a..5080ea7 100644 --- a/drivers/staging/ozwpan/ozurbparanoia.h +++ b/drivers/staging/ozwpan/ozurbparanoia.h @@ -10,8 +10,8 @@ void oz_remember_urb(struct urb *urb); int oz_forget_urb(struct urb *urb); #else -#define oz_remember_urb(__x) -#define oz_forget_urb(__x) 0 +static inline void oz_remember_urb(struct urb *urb) {} +static inline int oz_forget_urb(struct urb *urb) { return 0; } #endif /* WANT_URB_PARANOIA */ -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 5/5] staging: ozwpan: Rename Kbuild to Makefile
From: Joe Perches Rename Kbuild to usual Makefile, consistent with Kernel build structure. Signed-off-by: Joe Perches Signed-off-by: Rupesh Gujare --- drivers/staging/ozwpan/Kbuild | 16 drivers/staging/ozwpan/Makefile | 16 2 files changed, 16 insertions(+), 16 deletions(-) delete mode 100644 drivers/staging/ozwpan/Kbuild create mode 100644 drivers/staging/ozwpan/Makefile diff --git a/drivers/staging/ozwpan/Kbuild b/drivers/staging/ozwpan/Kbuild deleted file mode 100644 index 29529c1..000 --- a/drivers/staging/ozwpan/Kbuild +++ /dev/null @@ -1,16 +0,0 @@ -# - -# Copyright (c) 2011 Ozmo Inc -# Released under the GNU General Public License Version 2 (GPLv2). -# - - -obj-$(CONFIG_USB_WPAN_HCD) += ozwpan.o -ozwpan-y := \ - ozmain.o \ - ozpd.o \ - ozusbsvc.o \ - ozusbsvc1.o \ - ozhcd.o \ - ozeltbuf.o \ - ozproto.o \ - ozcdev.o \ - ozurbparanoia.o diff --git a/drivers/staging/ozwpan/Makefile b/drivers/staging/ozwpan/Makefile new file mode 100644 index 000..29529c1 --- /dev/null +++ b/drivers/staging/ozwpan/Makefile @@ -0,0 +1,16 @@ +# - +# Copyright (c) 2011 Ozmo Inc +# Released under the GNU General Public License Version 2 (GPLv2). +# - + +obj-$(CONFIG_USB_WPAN_HCD) += ozwpan.o +ozwpan-y := \ + ozmain.o \ + ozpd.o \ + ozusbsvc.o \ + ozusbsvc1.o \ + ozhcd.o \ + ozeltbuf.o \ + ozproto.o \ + ozcdev.o \ + ozurbparanoia.o -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
> -Original Message- > From: Michal Hocko [mailto:mho...@suse.cz] > Sent: Monday, July 22, 2013 8:37 AM > To: KY Srinivasan > Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org; > kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com; > jasow...@redhat.com; k...@vrfy.org > Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining > memory blocks > > On Fri 19-07-13 12:23:05, K. Y. Srinivasan wrote: > > The current machinery for hot-adding memory requires having udev > > rules to bring the memory segments online. Export the necessary > > functionality > > to to bring the memory segment online without involving user space code. > > Why? Who is going to use it and for what purpose? > If you need to do it from the kernel cannot you use usermod helper > thread? > > Besides that this is far from being complete. memory_block_change_state > seems to depend on device_hotplug_lock and find_memory_block is > currently called with mem_sysfs_mutex held. None of them is exported > AFAICS. You are right; not all of the required symbols are exported (yet). Let me answer your other questions first: The Hyper-V balloon driver can use this functionality. I have prototyped the in-kernel "onlining" of hot added memory without requiring any help from user level code that performs significantly better than having user level code involved in the hot add process. With this change, I am able to successfully hot-add and online the hot-added memory even under extreme memory pressure which is what you would want given that we are hot-adding memory to alleviate memory pressure. The current scheme of involving user level code to close this loop obviously does not perform well under high memory pressure. I can, if you prefer export all of the necessary functionality in one patch. Regards, K. Y ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
On Tue 23-07-13 14:52:36, KY Srinivasan wrote: > > > > -Original Message- > > From: Michal Hocko [mailto:mho...@suse.cz] > > Sent: Monday, July 22, 2013 8:37 AM > > To: KY Srinivasan > > Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > > a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org; > > kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com; > > jasow...@redhat.com; k...@vrfy.org > > Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining > > memory blocks > > > > On Fri 19-07-13 12:23:05, K. Y. Srinivasan wrote: > > > The current machinery for hot-adding memory requires having udev > > > rules to bring the memory segments online. Export the necessary > > > functionality > > > to to bring the memory segment online without involving user space code. > > > > Why? Who is going to use it and for what purpose? > > If you need to do it from the kernel cannot you use usermod helper > > thread? > > > > Besides that this is far from being complete. memory_block_change_state > > seems to depend on device_hotplug_lock and find_memory_block is > > currently called with mem_sysfs_mutex held. None of them is exported > > AFAICS. > > You are right; not all of the required symbols are exported (yet). Let > me answer your other questions first: > > The Hyper-V balloon driver can use this functionality. I have > prototyped the in-kernel "onlining" of hot added memory without > requiring any help from user level code that performs significantly > better than having user level code involved in the hot add process. What does significantly better mean here? > With this change, I am able to successfully hot-add and online the > hot-added memory even under extreme memory pressure which is what you > would want given that we are hot-adding memory to alleviate memory > pressure. The current scheme of involving user level code to close > this loop obviously does not perform well under high memory pressure. Hmm, this is really unexpected. Why the high memory pressure matters here? Userspace only need to access sysfs file and echo a simple string into a file. The reset is same regardless you do it from the userspace. > I can, if you prefer export all of the necessary functionality in one > patch. If this turns out really a valid use case then I would prefer exporting a high level function which would hide all the locking and direct manipulation with mem blocks. -- Michal Hocko SUSE Labs ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: Qos: Fix some coding style issues
On Tue, Jul 23, 2013 at 07:51:02PM +0800, Lilis Iskandar wrote: > Fixed spacing/tabs issues that were found using checkpatch. > Looks good. Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
On 07/23/2013 07:52 AM, KY Srinivasan wrote: > The current scheme of involving user > level code to close this loop obviously does not perform well under high > memory pressure. Adding memory usually requires allocating some large, contiguous areas of memory for use as mem_map[] and other VM structures. That's really hard to do under heavy memory pressure. How are you accomplishing this? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
> -Original Message- > From: Michal Hocko [mailto:mho...@suse.cz] > Sent: Tuesday, July 23, 2013 11:10 AM > To: KY Srinivasan > Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org; > kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com; > jasow...@redhat.com; k...@vrfy.org > Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining > memory blocks > > On Tue 23-07-13 14:52:36, KY Srinivasan wrote: > > > > > > > -Original Message- > > > From: Michal Hocko [mailto:mho...@suse.cz] > > > Sent: Monday, July 22, 2013 8:37 AM > > > To: KY Srinivasan > > > Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > > > a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org; > > > kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; > ying...@google.com; > > > jasow...@redhat.com; k...@vrfy.org > > > Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for > > > onlining > > > memory blocks > > > > > > On Fri 19-07-13 12:23:05, K. Y. Srinivasan wrote: > > > > The current machinery for hot-adding memory requires having udev > > > > rules to bring the memory segments online. Export the necessary > functionality > > > > to to bring the memory segment online without involving user space code. > > > > > > Why? Who is going to use it and for what purpose? > > > If you need to do it from the kernel cannot you use usermod helper > > > thread? > > > > > > Besides that this is far from being complete. memory_block_change_state > > > seems to depend on device_hotplug_lock and find_memory_block is > > > currently called with mem_sysfs_mutex held. None of them is exported > > > AFAICS. > > > > You are right; not all of the required symbols are exported (yet). Let > > me answer your other questions first: > > > > The Hyper-V balloon driver can use this functionality. I have > > prototyped the in-kernel "onlining" of hot added memory without > > requiring any help from user level code that performs significantly > > better than having user level code involved in the hot add process. > > What does significantly better mean here? Less failures than before. > > > With this change, I am able to successfully hot-add and online the > > hot-added memory even under extreme memory pressure which is what you > > would want given that we are hot-adding memory to alleviate memory > > pressure. The current scheme of involving user level code to close > > this loop obviously does not perform well under high memory pressure. > > Hmm, this is really unexpected. Why the high memory pressure matters > here? Userspace only need to access sysfs file and echo a simple string > into a file. The reset is same regardless you do it from the userspace. Could it be that we could fail to launch the user-space thread. The host presents a large chunk of memory for "hot adding". Within the guest, I break this up and hot-add 128MB chunks; as I loop through this process, I wait for the onlining to occur before proceeding with the next hotadd operation. With user space code involved in the onlining process, I would frequently timeout waiting for onlining to complete (under high memory load). After I switched over to not involving the user space code, this problem does not exist since onlining is done "in context". > > > I can, if you prefer export all of the necessary functionality in one > > patch. > > If this turns out really a valid use case then I would prefer exporting > a high level function which would hide all the locking and direct > manipulation with mem blocks. I will take a crack at defining wrappers to hide some of the details. I will also post the Hyper-V balloon driver patch that uses this functionality. Regards, K. Y ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
> -Original Message- > From: Dave Hansen [mailto:dave.han...@intel.com] > Sent: Tuesday, July 23, 2013 11:28 AM > To: KY Srinivasan > Cc: Michal Hocko; gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org; > kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com; > jasow...@redhat.com; k...@vrfy.org > Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining > memory blocks > > On 07/23/2013 07:52 AM, KY Srinivasan wrote: > > The current scheme of involving user > > level code to close this loop obviously does not perform well under high > memory pressure. > > Adding memory usually requires allocating some large, contiguous areas > of memory for use as mem_map[] and other VM structures. That's really > hard to do under heavy memory pressure. How are you accomplishing this? I cannot avoid failures because of lack of memory. In this case I notify the host of the failure and also tag the failure as transient. Host retries the operation after some delay. There is no guarantee it will succeed though. K. Y > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
On Fri, Jul 19, 2013 at 12:23:05PM -0700, K. Y. Srinivasan wrote: > The current machinery for hot-adding memory requires having udev > rules to bring the memory segments online. Export the necessary functionality > to to bring the memory segment online without involving user space code. > > Signed-off-by: K. Y. Srinivasan > --- > drivers/base/memory.c |5 - > include/linux/memory.h |4 > 2 files changed, 8 insertions(+), 1 deletions(-) > > diff --git a/drivers/base/memory.c b/drivers/base/memory.c > index 2b7813e..a8204ac 100644 > --- a/drivers/base/memory.c > +++ b/drivers/base/memory.c > @@ -328,7 +328,7 @@ static int __memory_block_change_state_uevent(struct > memory_block *mem, > return ret; > } > > -static int memory_block_change_state(struct memory_block *mem, > +int memory_block_change_state(struct memory_block *mem, > unsigned long to_state, unsigned long from_state_req, > int online_type) > { > @@ -341,6 +341,8 @@ static int memory_block_change_state(struct memory_block > *mem, > > return ret; > } > +EXPORT_SYMBOL(memory_block_change_state); EXPORT_SYMBOL_GPL() for all of these please. And as others have pointed out, I can't export symbols without a user of those symbols going into the tree at the same time. So I'll drop this patch for now and wait for your consumer of these symbols to be submitted. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
On 07/23/2013 08:54 AM, KY Srinivasan wrote: >> > Adding memory usually requires allocating some large, contiguous areas >> > of memory for use as mem_map[] and other VM structures. That's really >> > hard to do under heavy memory pressure. How are you accomplishing this? > I cannot avoid failures because of lack of memory. In this case I notify the > host of > the failure and also tag the failure as transient. Host retries the operation > after some > delay. There is no guarantee it will succeed though. You didn't really answer the question. You have allocated some large, physically contiguous areas of memory under heavy pressure. But you also contend that there is too much memory pressure to run a small userspace helper. Under heavy memory pressure, I'd expect large, kernel allocations to fail much more often than running a small userspace helper. It _sounds_ like you really want to be able to have the host retry the operation if it fails, and you return success/failure from inside the kernel. It's hard for you to tell if running the userspace helper failed, so your solution is to move what what previously done in userspace in to the kernel so that you can more easily tell if it failed or succeeded. Is that right? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: Qos: Fix some coding style issues
On Tue, 2013-07-23 at 19:51 +0800, Lilis Iskandar wrote: > Fixed spacing/tabs issues that were found using checkpatch. Hi Lilis. It'd be better to change the email subject to "[PATCH v2] etc..." to show that's it's a new version of the patch. It's also useful to add a note or two like "git diff -w shows no differences" and "no differences in objects" to make review and acceptance a bit easier. Good luck with continued cleanups of bcm. There's a lot to clean in those files. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Tuesday, July 23, 2013 12:02 PM > To: KY Srinivasan > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; > a...@canonical.com; a...@firstfloor.org; a...@linux-foundation.org; linux- > m...@kvack.org; kamezawa.hiroy...@gmail.com; mho...@suse.cz; > han...@cmpxchg.org; ying...@google.com; jasow...@redhat.com; > k...@vrfy.org > Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining > memory blocks > > On Fri, Jul 19, 2013 at 12:23:05PM -0700, K. Y. Srinivasan wrote: > > The current machinery for hot-adding memory requires having udev > > rules to bring the memory segments online. Export the necessary > > functionality > > to to bring the memory segment online without involving user space code. > > > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/base/memory.c |5 - > > include/linux/memory.h |4 > > 2 files changed, 8 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/base/memory.c b/drivers/base/memory.c > > index 2b7813e..a8204ac 100644 > > --- a/drivers/base/memory.c > > +++ b/drivers/base/memory.c > > @@ -328,7 +328,7 @@ static int > __memory_block_change_state_uevent(struct memory_block *mem, > > return ret; > > } > > > > -static int memory_block_change_state(struct memory_block *mem, > > +int memory_block_change_state(struct memory_block *mem, > > unsigned long to_state, unsigned long from_state_req, > > int online_type) > > { > > @@ -341,6 +341,8 @@ static int memory_block_change_state(struct > memory_block *mem, > > > > return ret; > > } > > +EXPORT_SYMBOL(memory_block_change_state); > > EXPORT_SYMBOL_GPL() for all of these please. Will do. > > And as others have pointed out, I can't export symbols without a user of > those symbols going into the tree at the same time. So I'll drop this > patch for now and wait for your consumer of these symbols to be > submitted. I will submit the consumer as well. Thanks, K. Y > greg k-h > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining memory blocks
> -Original Message- > From: Dave Hansen [mailto:dave.han...@intel.com] > Sent: Tuesday, July 23, 2013 12:01 PM > To: KY Srinivasan > Cc: Michal Hocko; gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > a...@firstfloor.org; a...@linux-foundation.org; linux...@kvack.org; > kamezawa.hiroy...@gmail.com; han...@cmpxchg.org; ying...@google.com; > jasow...@redhat.com; k...@vrfy.org > Subject: Re: [PATCH 1/1] Drivers: base: memory: Export symbols for onlining > memory blocks > > On 07/23/2013 08:54 AM, KY Srinivasan wrote: > >> > Adding memory usually requires allocating some large, contiguous areas > >> > of memory for use as mem_map[] and other VM structures. That's really > >> > hard to do under heavy memory pressure. How are you accomplishing this? > > I cannot avoid failures because of lack of memory. In this case I notify > > the host > of > > the failure and also tag the failure as transient. Host retries the > > operation after > some > > delay. There is no guarantee it will succeed though. > > You didn't really answer the question. > > You have allocated some large, physically contiguous areas of memory > under heavy pressure. But you also contend that there is too much > memory pressure to run a small userspace helper. Under heavy memory > pressure, I'd expect large, kernel allocations to fail much more often > than running a small userspace helper. I am only reporting what I am seeing. Broadly, I have two main failure conditions to deal with: (a) resource related failure (add_memory() returning -ENOMEM) and (b) not being able to online a segment that has been successfully hot-added. I have seen both these failures under high memory pressure. By supporting "in context" onlining, we can eliminate one failure case. Our inability to online is not a recoverable failure from the host's point of view - the memory is committed to the guest (since hot add succeeded) but is not usable since it is not onlined. > > It _sounds_ like you really want to be able to have the host retry the > operation if it fails, and you return success/failure from inside the > kernel. It's hard for you to tell if running the userspace helper > failed, so your solution is to move what what previously done in > userspace in to the kernel so that you can more easily tell if it failed > or succeeded. > > Is that right? No; I am able to get the proper error code for recoverable failures (hot add failures because of lack of memory). By doing what I am proposing here, we can avoid one class of failures completely and I think this is what resulted in a better "hot add" experience in the guest. K. Y > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: Qos: Fix some coding style issues
On Tue, 2013-07-23 at 09:42 -0700, Joe Perches wrote: > On Tue, 2013-07-23 at 19:51 +0800, Lilis Iskandar wrote: > > Fixed spacing/tabs issues that were found using checkpatch. > > Hi Lilis. > > It'd be better to change the email subject to > "[PATCH v2] etc..." to show that's it's a new > version of the patch. > > It's also useful to add a note or two like > "git diff -w shows no differences" and > "no differences in objects" to make review > and acceptance a bit easier. > > Good luck with continued cleanups of bcm. > There's a lot to clean in those files. Thanks for the advice! I'll keep that in mind for the following patches. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 18/27] drivers/staging/imx-drm: don't check resource with devm_ioremap_resource
devm_ioremap_resource does sanity checks on the given resource. No need to duplicate this in the driver. Signed-off-by: Wolfram Sang --- Please apply via the subsystem-tree. drivers/staging/imx-drm/imx-tve.c |5 - 1 file changed, 5 deletions(-) diff --git a/drivers/staging/imx-drm/imx-tve.c b/drivers/staging/imx-drm/imx-tve.c index a56797d..8232cea 100644 --- a/drivers/staging/imx-drm/imx-tve.c +++ b/drivers/staging/imx-drm/imx-tve.c @@ -623,11 +623,6 @@ static int imx_tve_probe(struct platform_device *pdev) } res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) { - dev_err(&pdev->dev, "failed to get memory region\n"); - return -ENOENT; - } - base = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(base)) return PTR_ERR(base); -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/27] devm cleanup, part #1, take #3
Here is another bit of cleaning up the devm usage. It is again removing the resource check with devm_ioremap_resource, because a) new drivers came in and b) coccinelle had a bug and missed to find a couple of occasions. Unlike last time, I think it is better if these patches go in via the subsystem trees to reduce merge conflicts. And there is one driver which I fixed manually because the original code needed some bigger update. All is based on v3.11-rc2 and the branch can be found at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git devm_no_resource_check Other things which happened: I wanted to get rid of devm_request_and_ioremap, luckily other people are already working on it. Hooray! I already sent a patch series picking another low hanging fruit, i.e. drivers can skip devm_pinctrl handling if they are using only the default pin setup. And not devm related, there is still my proposal to rename INIT_COMPLETION to reinit_completion, that probably needs some more persistence... Yay, so much to clean up \o/ Regards, Wolfram Wolfram Sang (27): arch/mips/lantiq/xway: don't check resource with devm_ioremap_resource drivers/amba: don't check resource with devm_ioremap_resource drivers/cpuidle: don't check resource with devm_ioremap_resource drivers/dma: don't check resource with devm_ioremap_resource drivers/gpu/host1x/drm: don't check resource with devm_ioremap_resource drivers/i2c/busses: don't check resource with devm_ioremap_resource drivers/input/serio: don't check resource with devm_ioremap_resource drivers/iommu: don't check resource with devm_ioremap_resource drivers/media/platform: don't check resource with devm_ioremap_resource drivers/memory: don't check resource with devm_ioremap_resource drivers/mtd/nand: don't check resource with devm_ioremap_resource drivers/net/ethernet/stmicro/stmmac: don't check resource with devm_ioremap_resource drivers/pci/host: don't check resource with devm_ioremap_resource drivers/pinctrl: don't check resource with devm_ioremap_resource drivers/pwm: don't check resource with devm_ioremap_resource drivers/scsi/ufs: don't check resource with devm_ioremap_resource drivers/spi: don't check resource with devm_ioremap_resource drivers/staging/imx-drm: don't check resource with devm_ioremap_resource drivers/usb/phy: don't check resource with devm_ioremap_resource drivers/watchdog: don't check resource with devm_ioremap_resource sound/soc/au1x: don't check resource with devm_ioremap_resource sound/soc/cirrus: don't check resource with devm_ioremap_resource sound/soc/nuc900: don't check resource with devm_ioremap_resource sound/soc/pxa: don't check resource with devm_ioremap_resource sound/soc/tegra: don't check resource with devm_ioremap_resource sound/soc/txx9: don't check resource with devm_ioremap_resource thermal: ti-bandgap: cleanup resource allocation arch/mips/lantiq/xway/dma.c|4 drivers/amba/tegra-ahb.c |2 -- drivers/cpuidle/cpuidle-kirkwood.c |3 --- drivers/dma/mmp_pdma.c |3 --- drivers/dma/mmp_tdma.c |3 --- drivers/gpu/host1x/drm/hdmi.c |3 --- drivers/i2c/busses/i2c-stu300.c|3 --- drivers/input/serio/olpc_apsp.c|3 --- drivers/iommu/tegra-smmu.c |2 -- drivers/media/platform/coda.c |5 - drivers/memory/tegra20-mc.c|2 -- drivers/memory/tegra30-mc.c|2 -- drivers/mtd/nand/mxc_nand.c|5 - .../net/ethernet/stmicro/stmmac/stmmac_platform.c |3 --- drivers/pci/host/pcie-designware.c | 12 drivers/pinctrl/pinctrl-imx.c |3 --- drivers/pinctrl/pinctrl-rockchip.c |5 - drivers/pinctrl/pinctrl-u300.c |3 --- drivers/pwm/pwm-lpc32xx.c |3 --- drivers/pwm/pwm-renesas-tpu.c |5 - drivers/scsi/ufs/ufshcd-pltfrm.c |6 -- drivers/spi/spi-bcm2835.c |6 -- drivers/staging/imx-drm/imx-tve.c |5 - drivers/thermal/ti-soc-thermal/ti-bandgap.c| 20 drivers/usb/phy/phy-rcar-usb.c |5 - drivers/watchdog/nuc900_wdt.c |5 - drivers/watchdog/ts72xx_wdt.c | 10 -- sound/soc/au1x/psc-ac97.c |3 --- sound/soc/cirrus/ep93xx-ac97.c |3 --- sound/soc/cirrus/ep93xx-i2s.c |3 --- sound/soc/nuc900/nuc900-ac97.c |3 --- sound/soc/pxa/mmp-sspa.c |3 ---
Re: [PATCH 1/1] staging/speakup/kobjects.c: Code improvement.
On Fri, Jun 21, 2013 at 10:54:40PM -0300, Raphael S. Carvalho wrote: > Well, there is no need to use strcmp since we can make a test of similar > semantic by using the var_id field of param. > I moved the test into the VAR_NUM:VAR_TIME case since VAR_STRING will > never be "voice". > > spk_xlate isn't used anymore (in line 608), then there is no difference > between using cp and buf in VAR_STRING case. > Besides, buf is a const char and those changes remove one uneeded line. > > I created the function spk_reset_default_value because it clarifies the > code and allows code reusing. > > Signed-off-by: Raphael S. Carvalho > Acked-by: Samuel Thibault > --- > drivers/staging/speakup/kobjects.c | 73 +++ > 1 files changed, 40 insertions(+), 33 deletions(-) This patch no longer applies to my tree, can you please refresh it against linux-next and resend, keeping Samuel's ack on it? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().
On Thu, Jun 27, 2013 at 03:34:52PM +0530, navin patidar wrote: > dev_warn() is preferred over pr_warning(). > > container_of() is used to get usb_driver pointer from usbip_device container > (stub_device or vhci_device), to get device structure required for dev_warn(). > > Signed-off-by: navin patidar > --- > drivers/staging/usbip/usbip_event.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/usbip/usbip_event.c > b/drivers/staging/usbip/usbip_event.c > index 82123be..1f3a571 100644 > --- a/drivers/staging/usbip/usbip_event.c > +++ b/drivers/staging/usbip/usbip_event.c > @@ -21,6 +21,8 @@ > #include > > #include "usbip_common.h" > +#include "stub.h" > +#include "vhci.h" > > static int event_handler(struct usbip_device *ud) > { > @@ -85,7 +87,14 @@ int usbip_start_eh(struct usbip_device *ud) > > ud->eh = kthread_run(event_handler_loop, ud, "usbip_eh"); > if (IS_ERR(ud->eh)) { > - pr_warning("Unable to start control thread\n"); > + struct device *dev; > + > + if (ud->side == USBIP_STUB) > + dev = &container_of(ud, struct stub_device, > ud)->udev->dev; > + else > + dev = &container_of(ud, struct vhci_device, > ud)->udev->dev; Putting '&' in front of container_of seems odd, are you sure it's needed here? If ud is a pointer, everything else should "just work" properly without that. Can you please fix this up? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: cxt1e1: ossiRelease.c: fixed a coding style issue.
On Mon, Jul 08, 2013 at 07:33:48PM +0300, Aldo Iljazi wrote: > Fixed a coding style issue, specifically checkpatch.pl complain: > ossiRelease.c:27: WARNING: line over 80 characters > > Signed-off-by: Aldo Iljazi > --- > drivers/staging/cxt1e1/ossiRelease.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/cxt1e1/ossiRelease.c > b/drivers/staging/cxt1e1/ossiRelease.c > index f17a902..35575bf 100644 > --- a/drivers/staging/cxt1e1/ossiRelease.c > +++ b/drivers/staging/cxt1e1/ossiRelease.c > @@ -24,6 +24,7 @@ > > *- > */ > > -char pmcc4_OSSI_release[] = "$Release: PMCC4_3_1B, Copyright (c) 2008 One > Stop Systems$"; > +char pmcc4_OSSI_release[] = "$Release: PMCC4_3_1B, " > +"Copyright (c) 2008 One Stop Systems$"; No, please fix this correctly, by just removing it entirely, it's not needed. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: return MAC in standard form
On Thu, Jul 11, 2013 at 10:23:57AM +0300, Andy Shevchenko wrote: > > Do we have typo in the MAINTAINERS file? I have got mail delivery > rejection for the de...@driverdev.osuosl.org. No, the list was down, it should be back up now... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: dwc2: add microframe scheduler from downstream Pi kernel
On Wed, Jul 17, 2013 at 12:35:46PM -0700, Paul Zimmerman wrote: > The transfer scheduler in the dwc2 driver is pretty basic, not to > mention buggy. It works fairly well with just a couple of devices > plugged in, but if you add, say, multiple devices with periodic > endpoints, the scheduler breaks down and can't even enumerate all > the devices. > > To fix this, import the "microframe scheduler" patch from the > driver in the downstream Raspberry Pi kernel, which is based on > the Synopsys vendor driver. That patch was done by the guys from > raspberrypi.org, including at least Gordon Hollingsworth and > "popcornmix". > > I have added a driver parameter for this, enabled by default, in > case anyone has problems with it and needs to disable it. I don't > think we should add a DT binding for that, though, since I plan to > remove the option once any bugs are fixed. > > Signed-off-by: Paul Zimmerman I'm guessing there will be a new version of this, so I'll drop this one from my queue. Please feel free to resend an updated version. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: gdm7240: adding LTE USB driver
On Tue, Jul 23, 2013 at 11:43:52AM +0300, Dan Carpenter wrote: > On Tue, Jul 23, 2013 at 04:40:29PM +0900, Won Kang wrote: > > Thank you so much for the review. I will try to fix them soon. > > Should I continue to send patches until reviewers agrees to accept > > them? I have no experience with submitting codes, and just not sure > > how to follow up. it's first time for me to do this kind of job. > > > > Always CC the mailing list in case someone has a similar question > later. > > Yes. That's basically the process. > > Greg would probably have taken it as-is, if I hadn't said anything. > Maybe he still will if you want. But going through staging is not > mandatory if your driver is ok from the start. I don't think > staging is going to help you much. I've already commented on most > of the style issues. You need reviews from networking people. Yes, I will take it as-is, unless the authors think they can clean it up in the next month or so to get it merged properly through the networking tree. Personally, I'd recommend doing the work now, and get it merged through networking, as you will have to do that work anyway, might as well do it now. Won, let me know what you want to do. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: tidspbridge: replace strict_strtol() with kstrtol()
On Tuesday, July 23, 2013 8:03 PM, Dan Carpenter wrote: > > The following test program illustrates the memory corruption. You > would hope foo.b would be 42 but it is corrupted to -1. > > #include > #include > > struct foo { > int a, b; > }; > > void kstrtol(long *x) > { > *x = -1; > } > > int main(void) > { > struct foo foo; > > foo.b = 42; > kstrtol((long *)&foo.a); > printf("%d %d\n", foo.a, foo.b); > return 0; > } > > The error handling should return an error, it shouldn't just print > something. It shouldn't print anything actually, it should just > handle the error without printing or complaining. :P Oh, thank you very much. Your comment is very kind. :) I will use kstrtos32() instead of kstrtol() as below: - if (count >= 3) - strict_strtol(sz_last_token, 10, (long *)&req); + if (count >= 3) { + status = kstrtos32(sz_last_token, 10, &req); + if (status) + goto func_cont; + } I really appreciate your comment. Best regards, Jingoo Han ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2 1/2] staging: tidspbridge: replace strict_strtol() with kstrtos32()
The usage of strict_strtol() is not preferred, because strict_strtol() is obsolete. Thus, kstrtos32() should be used in order to convert a string to s32. Also, error handling is added to get rid of a __must_check warning. Signed-off-by: Jingoo Han --- drivers/staging/tidspbridge/pmgr/dbll.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/staging/tidspbridge/pmgr/dbll.c b/drivers/staging/tidspbridge/pmgr/dbll.c index c191ae2..41e88ab 100644 --- a/drivers/staging/tidspbridge/pmgr/dbll.c +++ b/drivers/staging/tidspbridge/pmgr/dbll.c @@ -1120,8 +1120,11 @@ static int dbll_rmm_alloc(struct dynamic_loader_allocate *this, or DYN_EXTERNAL, then mem granularity information is present within the section name - only process if there are at least three tokens within the section name (just a minor optimization) */ - if (count >= 3) - strict_strtol(sz_last_token, 10, (long *)&req); + if (count >= 3) { + status = kstrtos32(sz_last_token, 10, &req); + if (status) + goto func_cont; + } if ((req == 0) || (req == 1)) { if (strcmp(sz_sec_last_token, "DYN_DARAM") == 0) { -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2 2/2] staging: iio: replace strict_strto*() with kstrto*()
The usage of strict_strto*() is not preferred, because strict_strto*() is obsolete. Thus, kstrto*() should be used. Previously, there were only strict_strtol(), strict_strtoul(), strict_strtoull(), and strict_strtoll(). Thus, when converting to the variables, only long, unsigned long, unsigned long long, and long long can be used. However, kstrto*() provides various functions handling all types of variables. Therefore, the types of variables can be changed properly. Signed-off-by: Jingoo Han --- drivers/staging/iio/accel/sca3000_core.c |8 ++--- drivers/staging/iio/accel/sca3000_ring.c |4 +-- drivers/staging/iio/addac/adt7316.c| 38 ++-- drivers/staging/iio/frequency/ad9832.c |4 +-- drivers/staging/iio/frequency/ad9834.c |4 +-- drivers/staging/iio/gyro/adis16260_core.c |4 +-- drivers/staging/iio/impedance-analyzer/ad5933.c| 12 +++ drivers/staging/iio/light/isl29018.c | 12 +++ drivers/staging/iio/light/tsl2583.c| 24 ++--- drivers/staging/iio/meter/ade7753.c| 12 +++ drivers/staging/iio/meter/ade7754.c| 12 +++ drivers/staging/iio/meter/ade7758_core.c | 12 +++ drivers/staging/iio/meter/ade7759.c| 12 +++ drivers/staging/iio/meter/ade7854.c| 16 - drivers/staging/iio/resolver/ad2s1210.c| 20 +-- drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 23 +--- .../staging/iio/trigger/iio-trig-periodic-rtc.c|4 +-- 17 files changed, 108 insertions(+), 113 deletions(-) diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c index 32950ad..5e99975 100644 --- a/drivers/staging/iio/accel/sca3000_core.c +++ b/drivers/staging/iio/accel/sca3000_core.c @@ -624,9 +624,9 @@ static ssize_t sca3000_set_frequency(struct device *dev, struct sca3000_state *st = iio_priv(indio_dev); int ret, base_freq = 0; int ctrlval; - long val; + int val; - ret = strict_strtol(buf, 10, &val); + ret = kstrtoint(buf, 10, &val); if (ret) return ret; @@ -931,12 +931,12 @@ static ssize_t sca3000_set_free_fall_mode(struct device *dev, { struct iio_dev *indio_dev = dev_to_iio_dev(dev); struct sca3000_state *st = iio_priv(indio_dev); - long val; + u8 val; int ret; u8 protect_mask = SCA3000_FREE_FALL_DETECT; mutex_lock(&st->lock); - ret = strict_strtol(buf, 10, &val); + ret = kstrtou8(buf, 10, &val); if (ret) goto error_ret; diff --git a/drivers/staging/iio/accel/sca3000_ring.c b/drivers/staging/iio/accel/sca3000_ring.c index 3e5e860..0f2ee33 100644 --- a/drivers/staging/iio/accel/sca3000_ring.c +++ b/drivers/staging/iio/accel/sca3000_ring.c @@ -177,11 +177,11 @@ static ssize_t sca3000_set_ring_int(struct device *dev, struct iio_dev *indio_dev = dev_to_iio_dev(dev); struct sca3000_state *st = iio_priv(indio_dev); struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); - long val; + u8 val; int ret; mutex_lock(&st->lock); - ret = strict_strtol(buf, 10, &val); + ret = kstrtou8(buf, 10, &val); if (ret) goto error_ret; ret = sca3000_read_data_short(st, SCA3000_REG_ADDR_INT_MASK, 1); diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c index 506b5a7..dfc4256 100644 --- a/drivers/staging/iio/addac/adt7316.c +++ b/drivers/staging/iio/addac/adt7316.c @@ -412,13 +412,13 @@ static ssize_t adt7316_store_ad_channel(struct device *dev, struct iio_dev *dev_info = dev_to_iio_dev(dev); struct adt7316_chip_info *chip = iio_priv(dev_info); u8 config2; - unsigned long data = 0; + u8 data; int ret; if (!(chip->config2 & ADT7316_AD_SINGLE_CH_MODE)) return -EPERM; - ret = strict_strtoul(buf, 10, &data); + ret = kstrtou8(buf, 10, &data); if (ret) return -EINVAL; @@ -848,10 +848,10 @@ static ssize_t adt7316_store_DAC_2Vref_ch_mask(struct device *dev, struct iio_dev *dev_info = dev_to_iio_dev(dev); struct adt7316_chip_info *chip = iio_priv(dev_info); u8 dac_config; - unsigned long data = 0; + u8 data; int ret; - ret = strict_strtoul(buf, 16, &data); + ret = kstrtou8(buf, 16, &data); if (ret || data > ADT7316_DA_2VREF_CH_MASK) return -EINVAL; @@ -903,13 +903,13 @@ static ssize_t adt7316_store_DAC_update_mode(struct device *dev, struct iio_dev *dev_info = dev_to_iio_dev(dev); struct adt7316_chip_info *chip = iio_priv(dev_info); u8 dac_config; - unsigned long data; + u8 data; int ret;
[PATCH 13/30] staging: lustre: Remove version.h header inclusion in dir.c
version.h header inclusion is not necessary as detected by versioncheck. Signed-off-by: Sachin Kamat --- drivers/staging/lustre/lustre/llite/dir.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/llite/dir.c b/drivers/staging/lustre/lustre/llite/dir.c index a94e463..002b374 100644 --- a/drivers/staging/lustre/lustre/llite/dir.c +++ b/drivers/staging/lustre/lustre/llite/dir.c @@ -41,7 +41,6 @@ #include #include #include -#include #include #include// for wait_on_buffer #include -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 19/30] staging: lustre: Remove version.h header inclusion in super25.c
version.h header inclusion is not necessary as detected by versioncheck. Signed-off-by: Sachin Kamat --- drivers/staging/lustre/lustre/llite/super25.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c index ea06f1a..0beaf4e 100644 --- a/drivers/staging/lustre/lustre/llite/super25.c +++ b/drivers/staging/lustre/lustre/llite/super25.c @@ -38,7 +38,6 @@ #include #include -#include #include #include #include -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/30] staging: lustre: Remove version.h header inclusion in socklnd_lib-linux.h
version.h header inclusion is not necessary as detected by versioncheck. Signed-off-by: Sachin Kamat --- .../lustre/lnet/klnds/socklnd/socklnd_lib-linux.h |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h index 3c13578..384eb7c 100644 --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.h @@ -41,7 +41,6 @@ #include #include -#include #include #include #include -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 12/30] staging: lustre: Remove version.h header inclusion in linux-debug.c
version.h header inclusion is not necessary as detected by versioncheck. Signed-off-by: Sachin Kamat --- .../lustre/lustre/libcfs/linux/linux-debug.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c b/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c index 9b5fa91..0069b5e 100644 --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-debug.c @@ -55,7 +55,6 @@ #include #include #include -#include # define DEBUG_SUBSYSTEM S_LNET -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel