Re: [PATCH] remove return statement
Hi surenderpolsani, [auto build test ERROR on v4.9-rc8] [also build test ERROR on next-20170413] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/surenderpolsani/remove-return-statement/20170415-130917 config: i386-allmodconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/staging/rtl8188eu/hal/rtl8188e_dm.c: In function 'rtw_hal_dm_watchdog': >> drivers/staging/rtl8188eu/hal/rtl8188e_dm.c:165:1: error: label at end of >> compound statement skip_dm: ^~~ vim +165 drivers/staging/rtl8188eu/hal/rtl8188e_dm.c 7ef8ded0 Larry Finger 2013-08-21 159 if (check_fwstate(pmlmepriv, _FW_LINKED)) 7ef8ded0 Larry Finger 2013-08-21 160 bLinked = true; 7ef8ded0 Larry Finger 2013-08-21 161 } 7ef8ded0 Larry Finger 2013-08-21 162 177aa53a Ivan Safonov 2016-09-19 163 Adapter->HalData->odmpriv.bLinked = bLinked; 177aa53a Ivan Safonov 2016-09-19 164 ODM_DMWatchdog(&Adapter->HalData->odmpriv); 7ef8ded0 Larry Finger 2013-08-21 @165 skip_dm: 7ef8ded0 Larry Finger 2013-08-21 166 /* Check GPIO to determine current RF on/off and Pbc status. */ 7ef8ded0 Larry Finger 2013-08-21 167 /* Check Hardware Radio ON/OFF or not */ 7ef8ded0 Larry Finger 2013-08-21 168 } :: The code at line 165 was first introduced by commit :: 7ef8ded0cfdb690e37581af85eea35fa67cdb38d staging: r8188eu: Add files for new driver - part 13 :: TO: Larry Finger :: CC: Greg Kroah-Hartman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging : rtl8188eu : remove void function return
kernel coding style doesn't allow the return statement in void function. Signed-off-by: surenderpolsani --- Changes for v2: corrected subject line as suggested --- drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c index d04b7fb..6db0e19 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c @@ -165,7 +165,6 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter) skip_dm: /* Check GPIO to determine current RF on/off and Pbc status. */ /* Check Hardware Radio ON/OFF or not */ - return; } void rtw_hal_dm_init(struct adapter *Adapter) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] remove return statement
On Sat, 2017-04-15 at 10:35 +0530, surenderpolsani wrote: > staging : rtl8188e : remove return in void function Your patch subject isn't correct. It should be something like: Subject: [PATCH] staging: rtl8188e: Remove void function return > kernel coding style doesn't allow the return statement > in void function. [] > diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c > b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c [] > @@ -165,7 +165,6 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter) > skip_dm: > /* Check GPIO to determine current RF on/off and Pbc status. */ > /* Check Hardware Radio ON/OFF or not */ > - return; And the comments? Are those supposed to be reminders of code to write? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] remove return statement
staging : rtl8188e : remove return in void function kernel coding style doesn't allow the return statement in void function. Signed-off-by: surenderpolsani --- drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c index d04b7fb..6db0e19 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c @@ -165,7 +165,6 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter) skip_dm: /* Check GPIO to determine current RF on/off and Pbc status. */ /* Check Hardware Radio ON/OFF or not */ - return; } void rtw_hal_dm_init(struct adapter *Adapter) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/22] crypto: chcr: Make use of the new sg_map helper function
On Fri, Apr 14, 2017 at 3:35 AM, Logan Gunthorpe wrote:
> The get_page in this area looks *highly* suspect due to there being no
> corresponding put_page. However, I've left that as is to avoid breaking
> things.
chcr driver will post the request to LLD driver cxgb4 and put_page is
implemented there. it will no harm. Any how
we have removed the below code from driver.
http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg24561.html
After this merge we can ignore your patch. Thanks
>
> I've also removed the KMAP_ATOMIC_ARGS check as it appears to be dead
> code that dates back to when it was first committed...
>
> Signed-off-by: Logan Gunthorpe
> ---
> drivers/crypto/chelsio/chcr_algo.c | 28 +++-
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/crypto/chelsio/chcr_algo.c
> b/drivers/crypto/chelsio/chcr_algo.c
> index 41bc7f4..a993d1d 100644
> --- a/drivers/crypto/chelsio/chcr_algo.c
> +++ b/drivers/crypto/chelsio/chcr_algo.c
> @@ -1489,22 +1489,21 @@ static struct sk_buff *create_authenc_wr(struct
> aead_request *req,
> return ERR_PTR(-EINVAL);
> }
>
> -static void aes_gcm_empty_pld_pad(struct scatterlist *sg,
> - unsigned short offset)
> +static int aes_gcm_empty_pld_pad(struct scatterlist *sg,
> +unsigned short offset)
> {
> - struct page *spage;
> unsigned char *addr;
>
> - spage = sg_page(sg);
> - get_page(spage); /* so that it is not freed by NIC */
> -#ifdef KMAP_ATOMIC_ARGS
> - addr = kmap_atomic(spage, KM_SOFTIRQ0);
> -#else
> - addr = kmap_atomic(spage);
> -#endif
> - memset(addr + sg->offset, 0, offset + 1);
> + get_page(sg_page(sg)); /* so that it is not freed by NIC */
> +
> + addr = sg_map(sg, SG_KMAP_ATOMIC);
> + if (IS_ERR(addr))
> + return PTR_ERR(addr);
> +
> + memset(addr, 0, offset + 1);
> + sg_unmap(sg, addr, SG_KMAP_ATOMIC);
>
> - kunmap_atomic(addr);
> + return 0;
> }
>
> static int set_msg_len(u8 *block, unsigned int msglen, int csize)
> @@ -1940,7 +1939,10 @@ static struct sk_buff *create_gcm_wr(struct
> aead_request *req,
> if (req->cryptlen) {
> write_sg_to_skb(skb, &frags, src, req->cryptlen);
> } else {
> - aes_gcm_empty_pld_pad(req->dst, authsize - 1);
> + err = aes_gcm_empty_pld_pad(req->dst, authsize - 1);
> + if (err)
> + goto dstmap_fail;
> +
> write_sg_to_skb(skb, &frags, reqctx->dst, crypt_len);
>
> }
> --
> 2.1.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/media: make atomisp vlv2_plat_clock explicitly non-modular
> I'm pretty sure we want this code to be built as a module, so maybe a > Kconfig change would resolve the issue instead? > > Alan, any thoughts? It's a tiny chunk of platform helper code. It probably ultimately belongs in arch/x86 somewhere or folded into the driver. At the moment it won't build modular. I'm fine with the change, it strips out more pointless code so helps see what tiny bits of code in there are actually used for anything real. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 17/39] platform: add video-multiplexer subdevice driver
Hi!
> > The MUX framework is already in linux-next. Could you use that instead of
> > adding new driver + bindings that are not compliant with the MUX framework?
> > I don't think it'd be much of a change in terms of code, using the MUX
> > framework appears quite simple.
>
> It is not quite clear to me how to design the DT bindings for this. Just
> splitting the video-multiplexer driver from the mux-mmio / mux-gpio
> would make it necessary to keep the video-multiplexer node to describe
> the of-graph bindings. But then we have two different nodes in the DT
> that describe the same hardware:
>
> mux: mux {
> compatible = "mux-gpio";
> mux-gpios = <&gpio 0>, <&gpio 1>;
> #mux-control-cells = <0>;
> }
>
> video-multiplexer {
> compatible = "video-multiplexer"
> mux-controls = <&mux>;
>
> ports {
> /* ... */
> }
> }
>
> It would feel more natural to have the ports in the mux node, but then
> how would the video-multiplexer driver be instanciated, and how would it
> get to the of-graph nodes?
Device tree representation and code used to implement the muxing
driver should be pretty independend, no? Yes, one piece of hardware
should have one entry in the device tree, so it should be something
like:
video-multiplexer {
compatible = "video-multiplexer-gpio"
mux-gpios = <&gpio 0>, <&gpio 1>;
#mux-control-cells = <0>;
mux-controls = <&mux>;
ports {
/* ... */
}
}
You should be able to use code in drivers/mux as a library...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/22] staging: unisys: visorbus: Make use of the new sg_map helper function
Great, thanks!
Logan
On 14/04/17 10:07 AM, Kershner, David A wrote:
> Can you add Acked-by for this patch?
>
> Acked-by: David Kershner
>
> Tested on s-Par and no problems.
>
> Thanks,
> David Kershner
>
>> ---
>> drivers/staging/unisys/visorhba/visorhba_main.c | 12 +++-
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c
>> b/drivers/staging/unisys/visorhba/visorhba_main.c
>> index 0ce92c8..2d8c8bc 100644
>> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
>> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
>> @@ -842,7 +842,6 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp, struct
>> scsi_cmnd *scsicmd)
>> struct scatterlist *sg;
>> unsigned int i;
>> char *this_page;
>> -char *this_page_orig;
>> int bufind = 0;
>> struct visordisk_info *vdisk;
>> struct visorhba_devdata *devdata;
>> @@ -869,11 +868,14 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp,
>> struct scsi_cmnd *scsicmd)
>>
>> sg = scsi_sglist(scsicmd);
>> for (i = 0; i < scsi_sg_count(scsicmd); i++) {
>> -this_page_orig = kmap_atomic(sg_page(sg + i));
>> -this_page = (void *)((unsigned long)this_page_orig |
>> - sg[i].offset);
>> +this_page = sg_map(sg + i, SG_KMAP_ATOMIC);
>> +if (IS_ERR(this_page)) {
>> +scsicmd->result = DID_ERROR << 16;
>> +return;
>> +}
>> +
>> memcpy(this_page, buf + bufind, sg[i].length);
>> -kunmap_atomic(this_page_orig);
>> +sg_unmap(sg + i, this_page, SG_KMAP_ATOMIC);
>> }
>> } else {
>> devdata = (struct visorhba_devdata *)scsidev->host-
>>> hostdata;
>> --
>> 2.1.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)
Thanks Julia. I missed that and I'll fix it in my series.
Logan
On 14/04/17 09:19 AM, Julia Lawall wrote:
> It looks like &udev->cmdr_lock should be released at line 512 if it has
> not been released otherwise. The lock was taken at line 438.
>
> julia
>
> -- Forwarded message --
> Date: Fri, 14 Apr 2017 22:21:44 +0800
> From: kbuild test robot
> To: kbu...@01.org
> Cc: Julia Lawall
> Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16
> call sites
>
> Hi Logan,
>
> [auto build test WARNING on scsi/for-next]
> [also build test WARNING on v4.11-rc6]
> [cannot apply to next-20170413]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
> :: branch date: 8 hours ago
> :: commit date: 8 hours ago
>
>>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438
>drivers/target/target_core_user.c:512:2-8: preceding lock on line 471
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 78082134e7afdc59d744eb8d2def5c588e89c378
> vim +512 drivers/target/target_core_user.c
>
> 7c9e7a6f Andy Grover 2014-10-01 432
> sizeof(struct tcmu_cmd_entry));
> 7c9e7a6f Andy Grover 2014-10-01 433 command_size = base_command_size
> 7c9e7a6f Andy Grover 2014-10-01 434 +
> round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
> 7c9e7a6f Andy Grover 2014-10-01 435
> 7c9e7a6f Andy Grover 2014-10-01 436 WARN_ON(command_size &
> (TCMU_OP_ALIGN_SIZE-1));
> 7c9e7a6f Andy Grover 2014-10-01 437
> 7c9e7a6f Andy Grover 2014-10-01 @438 spin_lock_irq(&udev->cmdr_lock);
> 7c9e7a6f Andy Grover 2014-10-01 439
> 7c9e7a6f Andy Grover 2014-10-01 440 mb = udev->mb_addr;
> 7c9e7a6f Andy Grover 2014-10-01 441 cmd_head = mb->cmd_head %
> udev->cmdr_size; /* UAM */
> 26418649 Sheng Yang 2016-02-26 442 data_length =
> se_cmd->data_length;
> 26418649 Sheng Yang 2016-02-26 443 if (se_cmd->se_cmd_flags &
> SCF_BIDI) {
> 26418649 Sheng Yang 2016-02-26 444
> BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
> 26418649 Sheng Yang 2016-02-26 445 data_length +=
> se_cmd->t_bidi_data_sg->length;
> 26418649 Sheng Yang 2016-02-26 446 }
> 554617b2 Andy Grover 2016-08-25 447 if ((command_size >
> (udev->cmdr_size / 2)) ||
> 554617b2 Andy Grover 2016-08-25 448 data_length >
> udev->data_size) {
> 554617b2 Andy Grover 2016-08-25 449 pr_warn("TCMU: Request
> of size %zu/%zu is too big for %u/%zu "
> 3d9b9555 Andy Grover 2016-08-25 450 "cmd ring/data
> area\n", command_size, data_length,
> 7c9e7a6f Andy Grover 2014-10-01 451
> udev->cmdr_size, udev->data_size);
> 554617b2 Andy Grover 2016-08-25 452
> spin_unlock_irq(&udev->cmdr_lock);
> 554617b2 Andy Grover 2016-08-25 453 return
> TCM_INVALID_CDB_FIELD;
> 554617b2 Andy Grover 2016-08-25 454 }
> 7c9e7a6f Andy Grover 2014-10-01 455
> 26418649 Sheng Yang 2016-02-26 456 while
> (!is_ring_space_avail(udev, command_size, data_length)) {
> 7c9e7a6f Andy Grover 2014-10-01 457 int ret;
> 7c9e7a6f Andy Grover 2014-10-01 458 DEFINE_WAIT(__wait);
> 7c9e7a6f Andy Grover 2014-10-01 459
> 7c9e7a6f Andy Grover 2014-10-01 460
> prepare_to_wait(&udev->wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
> 7c9e7a6f Andy Grover 2014-10-01 461
> 7c9e7a6f Andy Grover 2014-10-01 462 pr_debug("sleeping for
> ring space\n");
> 7c9e7a6f Andy Grover 2014-10-01 463
> spin_unlock_irq(&udev->cmdr_lock);
> 7c9e7a6f Andy Grover 2014-10-01 464 ret =
> schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT));
> 7c9e7a6f Andy Grover 2014-10-01 465
> finish_wait(&udev->wait_cmdr, &__wait);
> 7c9e7a6f Andy Grover 2014-10-01 466 if (!ret) {
> 7c9e7a6f Andy Grover 2014-10-01 467 pr_warn("tcmu:
> command timed out\n");
> 02eb924f Andy Grover 2016-10-06 468
RE: [PATCH 10/22] staging: unisys: visorbus: Make use of the new sg_map helper function
> -Original Message-
> From: Logan Gunthorpe [mailto:log...@deltatee.com]
...
> Subject: [PATCH 10/22] staging: unisys: visorbus: Make use of the new
> sg_map helper function
>
> Straightforward conversion to the new function.
>
> Signed-off-by: Logan Gunthorpe
Can you add Acked-by for this patch?
Acked-by: David Kershner
Tested on s-Par and no problems.
Thanks,
David Kershner
> ---
> drivers/staging/unisys/visorhba/visorhba_main.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c
> b/drivers/staging/unisys/visorhba/visorhba_main.c
> index 0ce92c8..2d8c8bc 100644
> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
> @@ -842,7 +842,6 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp, struct
> scsi_cmnd *scsicmd)
> struct scatterlist *sg;
> unsigned int i;
> char *this_page;
> - char *this_page_orig;
> int bufind = 0;
> struct visordisk_info *vdisk;
> struct visorhba_devdata *devdata;
> @@ -869,11 +868,14 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp,
> struct scsi_cmnd *scsicmd)
>
> sg = scsi_sglist(scsicmd);
> for (i = 0; i < scsi_sg_count(scsicmd); i++) {
> - this_page_orig = kmap_atomic(sg_page(sg + i));
> - this_page = (void *)((unsigned long)this_page_orig |
> - sg[i].offset);
> + this_page = sg_map(sg + i, SG_KMAP_ATOMIC);
> + if (IS_ERR(this_page)) {
> + scsicmd->result = DID_ERROR << 16;
> + return;
> + }
> +
> memcpy(this_page, buf + bufind, sg[i].length);
> - kunmap_atomic(this_page_orig);
> + sg_unmap(sg + i, this_page, SG_KMAP_ATOMIC);
> }
> } else {
> devdata = (struct visorhba_devdata *)scsidev->host-
> >hostdata;
> --
> 2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
On 14/04/17 02:39 AM, Christoph Hellwig wrote: > On Thu, Apr 13, 2017 at 04:05:22PM -0600, Logan Gunthorpe wrote: >> Very straightforward conversion to the new function in all four spots. > > I think the right fix here is to switch dm-crypt to the ahash API > that takes a scatterlist. Hmm, well I'm not sure I understand the code enough to make that conversion. But I was looking at it. One tricky bit seems to be that crypt_iv_lmk_one adds a seed, skips the first 16 bytes in the page and then hashes another 16 bytes from other data. What would you do construct a new sgl for it and pass it to the ahash api? The other thing is crypt_iv_lmk_post also seems to modify the page after the hash with a crypto_xor so you'd still need at least one kmap in there. Logan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/media: make atomisp vlv2_plat_clock explicitly non-modular
[Re: [PATCH] staging/media: make atomisp vlv2_plat_clock explicitly non-modular] On 14/04/2017 (Fri 10:12) Greg Kroah-Hartman wrote: > On Wed, Apr 12, 2017 at 09:57:55PM -0400, Paul Gortmaker wrote: > > The Makefile / Kconfig currently controlling compilation of this code is: > > > > clock/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += vlv2_plat_clock.o > > > > atomisp/Kconfig:menuconfig INTEL_ATOMISP > > atomisp/Kconfig:bool "Enable support to Intel MIPI camera drivers" > > > > ...meaning that it currently is not being built as a module by anyone. [...] > I'm pretty sure we want this code to be built as a module, so maybe a > Kconfig change would resolve the issue instead? As always, I'm good with things being moved to tristate if there is a use case for it. I will note that in this case however, that the above Kconfig option is not specific to this file/driver. It is controlling the inclusion of several dirs/files, and so a more fine grained Kconfig may be required if some are to be built-in and some are to be tristate... P. ~/git/linux-head/drivers/staging/media/atomisp$ git grep 'obj.*INTEL_ATOMISP' Makefile:obj-$(CONFIG_INTEL_ATOMISP) += pci/ Makefile:obj-$(CONFIG_INTEL_ATOMISP) += i2c/ Makefile:obj-$(CONFIG_INTEL_ATOMISP) += platform/ platform/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += clock/ platform/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += intel-mid/ platform/clock/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += vlv2_plat_clock.o platform/clock/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += platform_vlv2_plat_clk.o platform/intel-mid/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += intel_mid_pcihelpers.o platform/intel-mid/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += atomisp_gmin_platform.o > > Alan, any thoughts? > > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/22] libiscsi: Make use of new the sg_map helper function
On 14/04/17 02:36 AM, Christoph Hellwig wrote: > On Thu, Apr 13, 2017 at 04:05:16PM -0600, Logan Gunthorpe wrote: >> Convert the kmap and kmap_atomic uses to the sg_map function. We now >> store the flags for the kmap instead of a boolean to indicate >> atomicitiy. We also propogate a possible kmap error down and create >> a new ISCSI_TCP_INTERNAL_ERR error type for this. > > Can you split out the new error handling into a separate prep patch > which should go to the iscsi maintainers ASAP? > Yes, I can do that. I'd just have thought they'd want to see the use case for the new error before accepting a patch like that... Logan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/22] scatterlist: Introduce sg_map helper functions
On 14/04/17 02:35 AM, Christoph Hellwig wrote:
>> +
>> static inline int is_dma_buf_file(struct file *);
>>
>> struct dma_buf_list {
>
> I think the right fix here is to rename the operation to unmap_atomic
> and send out a little patch for that ASAP.
Ok, I can do that next week.
> I'd rather have separate functions for kmap vs kmap_atomic instead of
> the flags parameter. And while you're at it just always pass the 0
> offset parameter instead of adding a wrapper..
>
> Otherwise this looks good to me.
I settled on the flags because I thought the interface could be expanded
to do more things like automatically copy iomem to a bounce buffer (with
a flag). It'd also be possible to add things like vmap and
physical_address to the interface which would cover even more sg_page
users. All the implementations would then share the common offset
calculations, and switching between them becomes a matter of changing a
couple flags.
If you're still not convinced by the above arguments then I'll change
it but I did have reasons for choosing to do it this way.
I am fine with removing the offset versions. I will make that change.
Thanks,
Logan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)
It looks like &udev->cmdr_lock should be released at line 512 if it has
not been released otherwise. The lock was taken at line 438.
julia
-- Forwarded message --
Date: Fri, 14 Apr 2017 22:21:44 +0800
From: kbuild test robot
To: kbu...@01.org
Cc: Julia Lawall
Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16
call sites
Hi Logan,
[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.11-rc6]
[cannot apply to next-20170413]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 8 hours ago
:: commit date: 8 hours ago
>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438
drivers/target/target_core_user.c:512:2-8: preceding lock on line 471
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 78082134e7afdc59d744eb8d2def5c588e89c378
vim +512 drivers/target/target_core_user.c
7c9e7a6f Andy Grover 2014-10-01 432
sizeof(struct tcmu_cmd_entry));
7c9e7a6f Andy Grover 2014-10-01 433 command_size = base_command_size
7c9e7a6f Andy Grover 2014-10-01 434 +
round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
7c9e7a6f Andy Grover 2014-10-01 435
7c9e7a6f Andy Grover 2014-10-01 436 WARN_ON(command_size &
(TCMU_OP_ALIGN_SIZE-1));
7c9e7a6f Andy Grover 2014-10-01 437
7c9e7a6f Andy Grover 2014-10-01 @438 spin_lock_irq(&udev->cmdr_lock);
7c9e7a6f Andy Grover 2014-10-01 439
7c9e7a6f Andy Grover 2014-10-01 440 mb = udev->mb_addr;
7c9e7a6f Andy Grover 2014-10-01 441 cmd_head = mb->cmd_head %
udev->cmdr_size; /* UAM */
26418649 Sheng Yang 2016-02-26 442 data_length =
se_cmd->data_length;
26418649 Sheng Yang 2016-02-26 443 if (se_cmd->se_cmd_flags &
SCF_BIDI) {
26418649 Sheng Yang 2016-02-26 444
BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
26418649 Sheng Yang 2016-02-26 445 data_length +=
se_cmd->t_bidi_data_sg->length;
26418649 Sheng Yang 2016-02-26 446 }
554617b2 Andy Grover 2016-08-25 447 if ((command_size >
(udev->cmdr_size / 2)) ||
554617b2 Andy Grover 2016-08-25 448 data_length >
udev->data_size) {
554617b2 Andy Grover 2016-08-25 449 pr_warn("TCMU: Request
of size %zu/%zu is too big for %u/%zu "
3d9b9555 Andy Grover 2016-08-25 450 "cmd ring/data
area\n", command_size, data_length,
7c9e7a6f Andy Grover 2014-10-01 451
udev->cmdr_size, udev->data_size);
554617b2 Andy Grover 2016-08-25 452
spin_unlock_irq(&udev->cmdr_lock);
554617b2 Andy Grover 2016-08-25 453 return
TCM_INVALID_CDB_FIELD;
554617b2 Andy Grover 2016-08-25 454 }
7c9e7a6f Andy Grover 2014-10-01 455
26418649 Sheng Yang 2016-02-26 456 while
(!is_ring_space_avail(udev, command_size, data_length)) {
7c9e7a6f Andy Grover 2014-10-01 457 int ret;
7c9e7a6f Andy Grover 2014-10-01 458 DEFINE_WAIT(__wait);
7c9e7a6f Andy Grover 2014-10-01 459
7c9e7a6f Andy Grover 2014-10-01 460
prepare_to_wait(&udev->wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
7c9e7a6f Andy Grover 2014-10-01 461
7c9e7a6f Andy Grover 2014-10-01 462 pr_debug("sleeping for
ring space\n");
7c9e7a6f Andy Grover 2014-10-01 463
spin_unlock_irq(&udev->cmdr_lock);
7c9e7a6f Andy Grover 2014-10-01 464 ret =
schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT));
7c9e7a6f Andy Grover 2014-10-01 465
finish_wait(&udev->wait_cmdr, &__wait);
7c9e7a6f Andy Grover 2014-10-01 466 if (!ret) {
7c9e7a6f Andy Grover 2014-10-01 467 pr_warn("tcmu:
command timed out\n");
02eb924f Andy Grover 2016-10-06 468 return
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
7c9e7a6f Andy Grover 2014-10-01 469 }
7c9e7a6f Andy Grover 2014-10-01 470
7c9e7a6f Andy Grover 2014-10-01 471
spin_lock_irq(&udev->cmdr_lock);
7c9e7a6f Andy Grover 2014-10-01 472
7c9e7a6f Andy Grover 2014-10-01 473 /* We dropped
cmdr_lock, cmd_head is stale */
7c9e7a6f Andy Grover 2014-10-01 474 cmd_head = mb->cmd_head
% udev->cmdr_size; /* UAM */
7c9e7a6
[PATCH] staging: media: atomisp: fix range checking on clk_num
From: Colin Ian King
The range checking on clk_num is incorrect; fix these so that invalid
clk_num values are detected correctly.
Detected by static analysis with by PVS-Studio
Signed-off-by: Colin Ian King
---
drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
index 25e939c50aef..9efdf5790f90 100644
--- a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
+++ b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
@@ -67,7 +67,7 @@ int vlv2_plat_set_clock_freq(int clk_num, int freq_type)
{
void __iomem *addr;
- if (clk_num < 0 && clk_num > MAX_CLK_COUNT) {
+ if (clk_num < 0 || clk_num >= MAX_CLK_COUNT) {
pr_err("Clock number out of range (%d)\n", clk_num);
return -EINVAL;
}
@@ -103,7 +103,7 @@ int vlv2_plat_get_clock_freq(int clk_num)
{
u32 ret;
- if (clk_num < 0 && clk_num > MAX_CLK_COUNT) {
+ if (clk_num < 0 || clk_num >= MAX_CLK_COUNT) {
pr_err("Clock number out of range (%d)\n", clk_num);
return -EINVAL;
}
@@ -133,7 +133,7 @@ int vlv2_plat_configure_clock(int clk_num, u32 conf)
{
void __iomem *addr;
- if (clk_num < 0 && clk_num > MAX_CLK_COUNT) {
+ if (clk_num < 0 || clk_num >= MAX_CLK_COUNT) {
pr_err("Clock number out of range (%d)\n", clk_num);
return -EINVAL;
}
@@ -169,7 +169,7 @@ int vlv2_plat_get_clock_status(int clk_num)
{
int ret;
- if (clk_num < 0 && clk_num > MAX_CLK_COUNT) {
+ if (clk_num < 0 || clk_num >= MAX_CLK_COUNT) {
pr_err("Clock number out of range (%d)\n", clk_num);
return -EINVAL;
}
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Make ANDROID a menuconfig to ease disabling it all
On Fri, Apr 14, 2017 at 2:32 PM, Greg KH wrote: > That's fine, but you aren't actually changing the functionality of any > of the build options here. You are just adding a 'menu' and showing > things a bit differently. Yes exactly, I did not intend to change functionality, only ease disabling options, by not having to enter the menu. I.e. nothing much, especially for this one where the new now-unconfigurable menu will only have a single config entry inside (in fact I assumed there would be more coming) That's why I let it stay inside a menu, and not straight removing the menu and moved the config option one level up... > You aren't changing any dependancies (which > is what dictates what is and is not built), which does not make it > easier, or harder, to disable/enable anything here. I think I don't understand what you're telling here, I added a dep to ANDROID for the ANDROID_BINDER_IPC config entry. > I'm not against this, but you need to explain it a lot better as to what > you are doing and why. The "why" isn't covered by the "this will make > the kernel build smaller", as that's just not true :) This is not intended to make the kernel build smaller, but to ease the tedious process of going through "make menuconfig" and disabling all the options you don't need. The quantity of options has greatly increased, and when I could do a minimal kernel config in a few minutes years ago, I now have to take tens of minutes going through all. This work is a step trying to make this step quicker. Is that better ? Thanks -- Vincent Legoll ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Make ANDROID a menuconfig to ease disabling it all
On Fri, Apr 14, 2017 at 11:46:07AM +0200, Vincent Legoll wrote: > Hello, > > >> No need to get into the submenu to disable all ANDROID-related config > >> entries > > > > I don't understand this, what exactly do you mean? > > This is intended for people using make menuconfig to tailor > their kernel config to their need by disabling all options they > don't need. In order to have a smaller kernel, for example, > but also to get smaller build times. That's fine, but you aren't actually changing the functionality of any of the build options here. You are just adding a 'menu' and showing things a bit differently. You aren't changing any dependancies (which is what dictates what is and is not built), which does not make it easier, or harder, to disable/enable anything here. I'm not against this, but you need to explain it a lot better as to what you are doing and why. The "why" isn't covered by the "this will make the kernel build smaller", as that's just not true :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: remove redundant comparisons of unsigned ints with >= 0
Hi,
On 13-04-17 16:13, Colin King wrote:
From: Colin Ian King
The comparison of mode >= 0 is redundant as mode is a u32 and this
is always true. Remove this redundant code.
Detected with CoverityScan ("Unsigned compared against 0")
Signed-off-by: Colin Ian King
Patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
---
drivers/staging/rtl8723bs/core/rtw_debug.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c
b/drivers/staging/rtl8723bs/core/rtw_debug.c
index 51cef55d3f76..fc6b94d59c37 100644
--- a/drivers/staging/rtl8723bs/core/rtw_debug.c
+++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
@@ -1031,7 +1031,7 @@ ssize_t proc_set_ht_enable(struct file *file, const char
__user *buffer, size_t
if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
sscanf(tmp, "%d ", &mode);
- if (pregpriv && mode >= 0 && mode < 2) {
+ if (pregpriv && mode < 2) {
pregpriv->ht_enable = mode;
printk("ht_enable =%d\n", pregpriv->ht_enable);
}
@@ -1150,7 +1150,7 @@ ssize_t proc_set_rx_ampdu(struct file *file, const char
__user *buffer, size_t c
sscanf(tmp, "%d ", &mode);
- if (pregpriv && mode >= 0 && mode < 2) {
+ if (pregpriv && mode < 2) {
pmlmeinfo->bAcceptAddbaReq = mode;
DBG_871X("pmlmeinfo->bAcceptAddbaReq =%d\n",
pmlmeinfo->bAcceptAddbaReq);
if (mode == 0) {
@@ -1191,7 +1191,7 @@ ssize_t proc_set_en_fwps(struct file *file, const char
__user *buffer, size_t co
if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
sscanf(tmp, "%d ", &mode);
- if (pregpriv && mode >= 0 && mode < 2) {
+ if (pregpriv && mode < 2) {
pregpriv->check_fw_ps = mode;
DBG_871X("pregpriv->check_fw_ps =%d\n",
pregpriv->check_fw_ps);
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: clean up identical code on an if statement
Hi,
On 13-04-17 17:46, Colin King wrote:
From: Colin Ian King
The two different paths for an if statement are identical and hence
we can just replace it with the single statement.
Detected by CoverityScan, CID#1428443 ("Identical code for
different branches")
Signed-off-by: Colin Ian King
Patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
---
drivers/staging/rtl8723bs/core/rtw_mlme.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c
b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 53755e5b97a6..9e355734f0c0 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -1093,11 +1093,7 @@ void rtw_free_assoc_resources(struct adapter *adapter,
int lock_scanned_queue)
rtw_init_bcmc_stainfo(adapter);
}
- if (lock_scanned_queue) {
- find_network(adapter);
- } else {
- find_network(adapter);
- }
+ find_network(adapter);
if (lock_scanned_queue)
adapter->securitypriv.key_mask = 0;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Make ANDROID a menuconfig to ease disabling it all
Hello, >> No need to get into the submenu to disable all ANDROID-related config entries > > I don't understand this, what exactly do you mean? This is intended for people using make menuconfig to tailor their kernel config to their need by disabling all options they don't need. In order to have a smaller kernel, for example, but also to get smaller build times. So what this patch (and the others I also sent) is doing: There was a "menu", under which you have some config entries. In order to disable any or all of those config entries, you have to enter the submenu to then select each one and disable it. With a menuentry (and config options depending on it), you get a way to disable all those config entries at once without having to enter the menu. Does that make sense ? Is there a better way to achieve the disabling-easiness goal ? Any input appreciated. Thanks -- Vincent Legoll ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Make ANDROID a menuconfig to ease disabling it all
On Fri, Apr 14, 2017 at 11:04:21AM +0200, Vincent Legoll wrote: > No need to get into the submenu to disable all ANDROID-related config entries I don't understand this, what exactly do you mean? > > Signed-off-by: Vincent Legoll > --- > drivers/android/Kconfig | 12 ++-- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig > index a82fc02..c2b6c37 100644 > --- a/drivers/android/Kconfig > +++ b/drivers/android/Kconfig > @@ -1,15 +1,11 @@ > -menu "Android" > - > -config ANDROID > +menuconfig ANDROID > bool "Android Drivers" > ---help--- > Enable support for various drivers needed on the Android platform > > -if ANDROID > - > config ANDROID_BINDER_IPC > bool "Android Binder IPC Driver" > - depends on MMU > + depends on ANDROID && MMU > default n > ---help--- > Binder is used in Android for both communication between processes, > @@ -43,7 +39,3 @@ config ANDROID_BINDER_IPC_32BIT > earlier). > > Note that enabling this will break newer Android user-space. > - > -endif # if ANDROID > - > -endmenu There are other ANDROID config options in the kernel other than right here, so having a "menu" is a bit odd, right? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Using ion memory for direct-io
Hi Currently, the ion mapped to userspace will be forced with VM_IO and VM_PFNMAP flags. When I use the ion memory to do the direct-io, it will fail when reaching the get_user_pages, Back to the VM_IO and VM_PFNMAP flag, there two flags are introduced by the remap_pfn_range called by the ion_heap_mmap_user. >From my point of view, all ion memory(cma/vmalloc/system heap) are managed by >linux vm, it is not reasonable to have the VM_IO and VM_PFNMAP flag, but I don't any suitable function to replace the remap_pfn_range, any suggestions? Thanks && Regards Zengtao ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Make ANDROID a menuconfig to ease disabling it all
No need to get into the submenu to disable all ANDROID-related config entries Signed-off-by: Vincent Legoll --- drivers/android/Kconfig | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig index a82fc02..c2b6c37 100644 --- a/drivers/android/Kconfig +++ b/drivers/android/Kconfig @@ -1,15 +1,11 @@ -menu "Android" - -config ANDROID +menuconfig ANDROID bool "Android Drivers" ---help--- Enable support for various drivers needed on the Android platform -if ANDROID - config ANDROID_BINDER_IPC bool "Android Binder IPC Driver" - depends on MMU + depends on ANDROID && MMU default n ---help--- Binder is used in Android for both communication between processes, @@ -43,7 +39,3 @@ config ANDROID_BINDER_IPC_32BIT earlier). Note that enabling this will break newer Android user-space. - -endif # if ANDROID - -endmenu -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
On Thu, Apr 13, 2017 at 04:05:22PM -0600, Logan Gunthorpe wrote: > Very straightforward conversion to the new function in all four spots. I think the right fix here is to switch dm-crypt to the ahash API that takes a scatterlist. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/22] libiscsi: Make use of new the sg_map helper function
On Thu, Apr 13, 2017 at 04:05:16PM -0600, Logan Gunthorpe wrote: > Convert the kmap and kmap_atomic uses to the sg_map function. We now > store the flags for the kmap instead of a boolean to indicate > atomicitiy. We also propogate a possible kmap error down and create > a new ISCSI_TCP_INTERNAL_ERR error type for this. Can you split out the new error handling into a separate prep patch which should go to the iscsi maintainers ASAP? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/22] scatterlist: Introduce sg_map helper functions
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 0007b79..b95934b 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -37,6 +37,9 @@
>
> #include
>
> +/* Prevent the highmem.h macro from aliasing ops->kunmap_atomic */
> +#undef kunmap_atomic
> +
> static inline int is_dma_buf_file(struct file *);
>
> struct dma_buf_list {
I think the right fix here is to rename the operation to unmap_atomic
and send out a little patch for that ASAP.
> + * Flags can be any of:
> + * * SG_KMAP- Use kmap to create the mapping
> + * * SG_KMAP_ATOMIC - Use kmap_atomic to map the page atommically.
> + * Thus, the rules of that function apply: the cpu
> + * may not sleep until it is unmaped.
> + *
> + * Also, consider carefully whether this function is appropriate. It is
> + * largely not recommended for new code and if the sgl came from another
> + * subsystem and you don't know what kind of memory might be in the list
> + * then you definitely should not call it. Non-mappable memory may be in
> + * the sgl and thus this function may fail unexpectedly.
> + **/
> +static inline void *sg_map_offset(struct scatterlist *sg, size_t offset,
> +int flags)
I'd rather have separate functions for kmap vs kmap_atomic instead of
the flags parameter. And while you're at it just always pass the 0
offset parameter instead of adding a wrapper..
Otherwise this looks good to me.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Bug] VCHIQ functional test broken
Hi Rabin,
> Rabin Vincent hat am 14. April 2017 um 09:41 geschrieben:
>
>
> On Thu, Apr 13, 2017 at 11:29:15PM +0100, Russell King - ARM Linux wrote:
> > > 00a19f3e25c0c40e0ec77f52d4841d23ad269169 is the first bad commit
> > > commit 00a19f3e25c0c40e0ec77f52d4841d23ad269169
> > > Author: Rabin Vincent
> > > Date: Tue Nov 8 09:21:19 2016 +0100
> > >
> > > ARM: 8627/1: avoid cache flushing in flush_dcache_page()
> > >
> > > When the data cache is PIPT or VIPT non-aliasing, and cache operations
> > > are broadcast by the hardware, we can always postpone the flush in
> > > flush_dcache_page(). A similar change was done for ARM64 in commit
> > > b5b6c9e9149d ("arm64: Avoid cache flushing in flush_dcache_page()").
> > >
> > > Reviewed-by: Catalin Marinas
> > > Signed-off-by: Rabin Vincent
> > > Signed-off-by: Russell King
> > >
> > > It seems that staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm
> > > relies on the behavior of flush_dcache_page before this patch get
> > > applied. Any advices to fix this issues are appreciated.
> >
> > Any ideas why this causes a problem for this driver? From what I can see,
> > it doesn't make use of flush_dcache_page().
>
> The driver's create_pagelist() uses get_free_pages(), and
> get_free_pages() calls flush_dcache_page().
i think you mean get_user_pages().
>
> The problem is that the driver fails to flush the pages which it
> acquires via get_free_pages(). It's clear that the driver needs to do
> it, since there is a flush in the is_vmalloc_addr() path in the same
> function. The driver probably worked earlier because of the unecessary
> flush in flush_dcache_page() which existed before this patch, but the
> purpose of that flush was not DMA coherency and it was never guaranteed
> that it would flush all the way to the point that devices could see the
> data.
>
> See radeon_ttm_tt_pin_userptr() in drivers/gpu/drm/radeon/radeon_ttm.c
> for an example of how a driver can ensure cache coherency using the DMA
> mapping API if it intends to DMA from/to pages acquired by
> get_free_pages().
Thanks for your explanation and the example, but i can't see the relevant
difference. So i think i'm not the right person to fix this issue, but i will
test any patches regarding to this issue.
>
> The rest of the driver should also be converted to the DMA mapping API
> instead of abusing the API's private functions (dmac_map_area etc.)
I will add this to the TODO file of the driver.
Regards
Stefan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/18] staging: ks7010: clean up SDIO header comments
On Wed, Apr 12, 2017 at 09:56:52AM +1000, Tobin C. Harding wrote:
> SDIO header file does not use kernel doc format struct
> comments. Adding them aids readability and enables documentation to be
> built from the source code. Other comments may be tidied up as we do this.
>
> Add kernel format struct comments. Tidy up comments.
>
> Signed-off-by: Tobin C. Harding
> ---
> drivers/staging/ks7010/ks7010_sdio.h | 56
> +++-
> 1 file changed, 42 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.h
> b/drivers/staging/ks7010/ks7010_sdio.h
> index b9d5a41..63fbd2d 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.h
> +++ b/drivers/staging/ks7010/ks7010_sdio.h
> @@ -82,8 +82,6 @@ enum gen_com_reg_b {
> };
>
> /* Wakeup Register */
> -/* #define WAKEUP0x008104 */
> -/* #define WAKEUP_REQ0x00 */
> #define WAKEUP 0x008018
> #define WAKEUP_REQ 0x5a
>
> @@ -93,9 +91,6 @@ enum gen_com_reg_b {
>
> #define KS7010_IRAM_ADDRESS 0x0600
>
> -/*
> - * struct define
> - */
> struct hw_info_t {
> struct ks_sdio_card *sdio_card;
> struct workqueue_struct *ks7010sdio_wq;
> @@ -108,38 +103,71 @@ struct ks_sdio_card {
> struct ks_wlan_private *priv;
> };
>
> -/* Tx Device struct */
> +/*
> + * Tx Queue
> + */
> +
Why a blank line?
Anyway, patch doesn't apply as I didn't take earlier patches. I've
stopped here, please fix up and resend the series.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/18] staging: ks7010: remove argument identifiers
On Wed, Apr 12, 2017 at 09:56:51AM +1000, Tobin C. Harding wrote: > When declaring a function with a function pointer as parameter, having > the parameters to the function pointer prototype with explicit > identifiers does not add that much extra meaning to the code. In this > case the identifiers are 'arg1' and 'arg2', these definitely do not > add extra meaning to the code. Removing them makes the code easier to > read. No, put the arguments in there. If they aren't used, then perhaps the whole function pointer needs to be changed instead? void * use in a driver is usually a sign that something is wrong and can be fixed up. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/18] staging: ks7010: replace defines with enums
On Wed, Apr 12, 2017 at 09:56:46AM +1000, Tobin C. Harding wrote:
> Header has multiple constants defined using preprocessor
> directive. In the cases where these are an integer progression an
> enumeration type can be used. Doing so adds documentation to the code
> and makes the usage explicit.
>
> Replace (integer progression) preprocessor constants with enumeration type.
>
> Signed-off-by: Tobin C. Harding
> ---
> drivers/staging/ks7010/ks7010_sdio.h | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.h
> b/drivers/staging/ks7010/ks7010_sdio.h
> index a1c7551..43b9990 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.h
> +++ b/drivers/staging/ks7010/ks7010_sdio.h
> @@ -24,8 +24,10 @@
>
> /* Read Status Register */
> #define READ_STATUS 0x00
> -#define READ_STATUS_BUSY 0
> -#define READ_STATUS_IDLE 1
> +enum read_status_type {
> + READ_STATUS_BUSY,
> + READ_STATUS_IDLE
> +};
Is this even used? Why not just delete these unused defines?
I don't see READ_STATUS_BUSY used anywhere...
> /* Read Index Register */
> #define READ_INDEX 0x04
> @@ -35,8 +37,10 @@
>
> /* Write Status Register */
> #define WRITE_STATUS 0x0C
> -#define WRITE_STATUS_BUSY0
> -#define WRITE_STATUS_IDLE1
> +enum write_status_type {
> + WRITE_STATUS_BUSY,
> + WRITE_STATUS_IDLE
> +};
WRITE_STATUS_IDLE doesn't seem to be used either, implying that
something is odd here.
Please fix this up correctly please.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers/staging/iio: braces {} are not necessary for single statement blocks
On Mon, Apr 10, 2017 at 10:08:07AM +0530, Pushkar Jambhlekar wrote: > Handling checkpatch.pl warning for if block. For single if statement block, > braces are not neccessary. Making code consistent with linux kernel coding > guidelines. Pleasse wrap your changelog comments at 72 columns like git asked you to do. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: lustre: replace simple_strtoul with kstrtoint
On Tue, Mar 21, 2017 at 01:46:09PM +0100, Marcin Ciupak wrote:
> Replace simple_strtoul with kstrtoint.
> simple_strtoul is marked for obsoletion as reported by checkpatch.pl
>
> Signed-off-by: Marcin Ciupak
> ---
> v2:
> -improving kstrtoint error handling
> -updating commit message
>
> drivers/staging/lustre/lustre/obdclass/obd_mount.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> index 8e0d4b1d86dc..42858ee5b444 100644
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -924,12 +924,20 @@ static int lmd_parse(char *options, struct
> lustre_mount_data *lmd)
> lmd->lmd_flags |= LMD_FLG_ABORT_RECOV;
> clear++;
> } else if (strncmp(s1, "recovery_time_soft=", 19) == 0) {
> - lmd->lmd_recovery_time_soft = max_t(int,
> - simple_strtoul(s1 + 19, NULL, 10), time_min);
> + int res;
> +
> + rc = kstrtoint(s1 + 19, 10, &res);
> + if (rc)
> + goto invalid;
> + lmd->lmd_recovery_time_soft = max_t(int, res, time_min);
Are you sure max_t is still needed here?
And have you tested this change?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: lustre: Fix sparse endianness warnings cast to restricted __le64 and __le32
On Thu, Apr 13, 2017 at 12:09:23AM -0700, skanda.kash...@gmail.com wrote: > From: Skanda Guruanand > > Modified struct lu_dirpage in lustre_idl.h file to remove the sparse > warnings where cast to restricted types are made. > > Following warnings are removed by this fix. > > drivers/staging/lustre/lustre/mdc/mdc_request.c:958:42: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:959:42: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:962:42: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:963:42: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:985:50: warning: cast to > restricted __le32 > drivers/staging/lustre/lustre/mdc/mdc_request.c:1193:24: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:1328:25: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:1329:23: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:1332:25: warning: cast to > restricted __le64 > drivers/staging/lustre/lustre/mdc/mdc_request.c:1333:23: warning: cast to > restricted __le64 > > Signed-off-by: Skanda Guruanand > --- > Isn't the below reason good enough? > Since the structure elements are always converted from little endian to > processor native format > in mdc_request.c, struct lu_dirpage element types is modified. Why isn't this above in the changelog text? And I need an ack from a lustre maintainer before I can take this... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RESEND v2] Staging: lustre cleanup macros in libcfs_private.h
On Thu, Apr 13, 2017 at 10:24:41AM +0100, Craig Inches wrote:
> This resolves a checkpatch warning that "Single statement macros should
> not use a do {} while (0) loop" by removing the loop and adjusting line
> length accordingly.
>
> Signed-off-by: Craig Inches
> ---
> Changes in v2:
> - Kept statements together
> - Kept operator on previous line
Why RESEND?
>
> .../lustre/include/linux/libcfs/libcfs_private.h | 51
> +++---
> 1 file changed, 15 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> index 2dae857..e774c75 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h
> @@ -87,12 +87,9 @@ do {
> \
> #define LIBCFS_VMALLOC_SIZE (2 << PAGE_SHIFT) /* 2 pages */
> #endif
>
> -#define LIBCFS_ALLOC_PRE(size, mask) \
> -do { \
> - LASSERT(!in_interrupt() || \
> - ((size) <= LIBCFS_VMALLOC_SIZE && \
> - !gfpflags_allow_blocking(mask))); \
> -} while (0)
> +#define LIBCFS_ALLOC_PRE(size, mask) \
> + LASSERT(!in_interrupt() || ((size) <= LIBCFS_VMALLOC_SIZE &&\
> + !gfpflags_allow_blocking(mask)))
>
> #define LIBCFS_ALLOC_POST(ptr, size) \
> do { \
> @@ -187,46 +184,28 @@ void cfs_array_free(void *vars);
> #if LASSERT_ATOMIC_ENABLED
>
> /** assert value of @a is equal to @v */
> -#define LASSERT_ATOMIC_EQ(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) == v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_EQ(a, v) \
> + LASSERTF(atomic_read(a) == v, "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is unequal to @v */
> -#define LASSERT_ATOMIC_NE(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) != v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_NE(a, v) \
> + LASSERTF(atomic_read(a) != v, "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is little than @v */
> -#define LASSERT_ATOMIC_LT(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) < v,\
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_LT(a, v) \
> + LASSERTF(atomic_read(a) < v, "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is little/equal to @v */
> -#define LASSERT_ATOMIC_LE(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) <= v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_LE(a, v) \
> + LASSERTF(atomic_read(a) <= v, "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is great than @v */
> -#define LASSERT_ATOMIC_GT(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) > v,\
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_GT(a, v) \
> + LASSERTF(atomic_read(a) > v, "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is great/equal to @v */
> -#define LASSERT_ATOMIC_GE(a, v) \
> -do { \
> - LASSERTF(atomic_read(a) >= v, \
> - "value: %d\n", atomic_read((a)));\
> -} while (0)
> +#define LASSERT_ATOMIC_GE(a, v) \
> + LASSERTF(atomic_read(a) >= v, "value: %d\n", atomic_read((a)))
>
> /** assert value of @a is great than @v1 and little than @v2 */
> #define LASSERT_ATOMIC_GT_LT(a, v1, v2) \
I need a lustre maintainer to ack this one before I can take it.
Perhaps there was a good reasaon do { } while is used here...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/media: make atomisp vlv2_plat_clock explicitly non-modular
On Wed, Apr 12, 2017 at 09:57:55PM -0400, Paul Gortmaker wrote: > The Makefile / Kconfig currently controlling compilation of this code is: > > clock/Makefile:obj-$(CONFIG_INTEL_ATOMISP) += vlv2_plat_clock.o > > atomisp/Kconfig:menuconfig INTEL_ATOMISP > atomisp/Kconfig:bool "Enable support to Intel MIPI camera drivers" > > ...meaning that it currently is not being built as a module by anyone. > > Lets remove the modular code that is essentially orphaned, so that > when reading the driver there is no doubt it is builtin-only. > > Since module_init was already not in use by this driver, the init > ordering remains unchanged with this commit. > > Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code. > > We also delete the MODULE_LICENSE tag etc. since all that information > is already contained at the top of the file in the comments. > > Cc: Mauro Carvalho Chehab > Cc: Greg Kroah-Hartman > Cc: Alan Cox > Cc: linux-me...@vger.kernel.org > Cc: de...@driverdev.osuosl.org > Signed-off-by: Paul Gortmaker I'm pretty sure we want this code to be built as a module, so maybe a Kconfig change would resolve the issue instead? Alan, any thoughts? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Bug] VCHIQ functional test broken
On Thu, Apr 13, 2017 at 11:29:15PM +0100, Russell King - ARM Linux wrote:
> > 00a19f3e25c0c40e0ec77f52d4841d23ad269169 is the first bad commit
> > commit 00a19f3e25c0c40e0ec77f52d4841d23ad269169
> > Author: Rabin Vincent
> > Date: Tue Nov 8 09:21:19 2016 +0100
> >
> > ARM: 8627/1: avoid cache flushing in flush_dcache_page()
> >
> > When the data cache is PIPT or VIPT non-aliasing, and cache operations
> > are broadcast by the hardware, we can always postpone the flush in
> > flush_dcache_page(). A similar change was done for ARM64 in commit
> > b5b6c9e9149d ("arm64: Avoid cache flushing in flush_dcache_page()").
> >
> > Reviewed-by: Catalin Marinas
> > Signed-off-by: Rabin Vincent
> > Signed-off-by: Russell King
> >
> > It seems that staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm
> > relies on the behavior of flush_dcache_page before this patch get
> > applied. Any advices to fix this issues are appreciated.
>
> Any ideas why this causes a problem for this driver? From what I can see,
> it doesn't make use of flush_dcache_page().
The driver's create_pagelist() uses get_free_pages(), and
get_free_pages() calls flush_dcache_page().
The problem is that the driver fails to flush the pages which it
acquires via get_free_pages(). It's clear that the driver needs to do
it, since there is a flush in the is_vmalloc_addr() path in the same
function. The driver probably worked earlier because of the unecessary
flush in flush_dcache_page() which existed before this patch, but the
purpose of that flush was not DMA coherency and it was never guaranteed
that it would flush all the way to the point that devices could see the
data.
See radeon_ttm_tt_pin_userptr() in drivers/gpu/drm/radeon/radeon_ttm.c
for an example of how a driver can ensure cache coherency using the DMA
mapping API if it intends to DMA from/to pages acquired by
get_free_pages().
The rest of the driver should also be converted to the DMA mapping API
instead of abusing the API's private functions (dmac_map_area etc.)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
