Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
On Wed, Mar 07, 2018 at 08:50:30PM +, Jonathan Cameron wrote: > On Mon, 5 Mar 2018 13:19:29 +0530 > Himanshu Jha wrote: > > > Clarify the conversion and formation of resultant data in the > > adis16201_read_raw() with sufficient comments. > > > > Signed-off-by: Himanshu Jha > This is fine but it needs to be in the original comment changing patch > rather than removing the comments first then a few patches later putting > back a different version. > > So good change but in the wrong place in the series. Learning to reorder > a series and merge down multiple patches into one is very useful when > working with git. I will send v2 with all the updates! What about sign_extend32 & reverse christmas ordering patch ? Will you pick that up or shall I send them again ? -- Thanks Himanshu Jha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
,Your urgent confirmation
Attn: Beneficiary, We have contacted the Federal Ministry of Finance on your Behalf and they have brought a solution to your problem by coordinating your payment in total (10,000,000.00) Ten Million Dollars in an atm card which you can use to withdraw money from any ATM MACHINE CENTER anywhere in the world with a maximum of 1 Dollars daily. You now have the lawful right to claim your fund in an atm card. Since the Federal Bureau of Investigation is involved in this transaction, you have to be rest assured for this is 100% risk free it is our duty to protect the American Citizens, European Citizens, Asian Citizen. All I want you to do is to contact the atm card CENTER Via email or call the office telephone number one of the Consultant will assist you for their requirements to proceed and procure your Approval Slip on your behalf. CONTACT INFORMATION NAME: James Williams EMAIL: paymasterofficed...@gmail.com Do contact us with your details: Full name// Address// Age// Telephone Numbers// Occupation// Your Country// Bank Details// So your files would be updated after which the Delivery of your atm card will be affected to your designated home Address without any further delay and the bank will transfer your funds in total (10,000,000.00) Ten Million Dollars to your Bank account. We will reply you with the secret code (1600 atm card). We advice you get back to the payment office after you have contacted the ATM SWIFT CARD CENTER and we do await your response so we can move on with our Investigation and make sure your ATM SWIFT CARD gets to you. Best Regards James Williams Paymaster General Federal Republic Of Nigeri ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ACKNOWLEDGE RECEIPT 3/7/2018
Good Day, This is a letter of Intent for Investment Project. See attached for full details. Indicate your interest to my personal email: batud...@yandex.com Yours sincerely. Mr.Dave Batu ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] android: ion: How to properly clean caches for uncached allocations
On 02/28/2018 09:18 PM, Liam Mark wrote: The issue: Currently in ION if you allocate uncached memory it is possible that there are still dirty lines in the cache. And often these dirty lines in the cache are the zeros which were meant to clear out any sensitive kernel data. What this means is that if you allocate uncached memory from ION, and then subsequently write to that buffer (using the uncached mapping you are provided by ION) then the data you have written could be corrupted at some point in the future if a dirty line is evicted from the cache. Also this means there is a potential security issue. If an un-privileged userspace user allocated uncached memory (for example from the system heap) and then if they were to read from that buffer (through the un-cached mapping they are provided by ION), and if some of the zeros which were written to that memory are still in the cache then this un-privileged userspace user could read potentially sensitive kernel data. For the use case you are describing we don't actually need the memory to be non-cached until it comes time to do the dma mapping. Here's a proposal to shoot holes in: - Before any dma_buf attach happens, all mmap mappings are cached - At the time attach happens, we shoot down any existing userspace mappings, do the dma_map with appropriate flags to clean the pages and then allow remapping to userspace as uncached. Really this looks like a variation on the old Ion faulting code which I removed except it's for uncached buffers instead of cached buffers. Potential problems: - I'm not 100% about the behavior here if the attaching device is already dma_coherent. I also consider uncached mappings enough of a device specific optimization that you shouldn't do them unless you know it's needed. - The locking/sequencing with userspace could be tricky since userspace may not like us ripping mappings out from underneath if it's trying to access. Thoughts? Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ashmem: Remove deadlock
On Wed, Mar 07, 2018 at 02:02:13PM -0800, Paul Lawrence wrote: > Great! We need to make sure this gets backported to 4.4 and 4.9, and to > 3.18 with the original dependency, please. That will happen when it lands in Linus's tree, which should be later this week if all goes well. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fsl-mc/dpio: Add missing argument identifier
When running checkpatch over the DPIO code the following warning is reported: WARNING: function definition argument 'struct dpaa2_io_notification_ctx *' should also have an identifier name Add the missing identifier. Signed-off-by: Roy Pledge --- drivers/staging/fsl-mc/include/dpaa2-io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fsl-mc/include/dpaa2-io.h b/drivers/staging/fsl-mc/include/dpaa2-io.h index 9cb1eec..f71227d 100644 --- a/drivers/staging/fsl-mc/include/dpaa2-io.h +++ b/drivers/staging/fsl-mc/include/dpaa2-io.h @@ -80,7 +80,7 @@ struct dpaa2_io *dpaa2_io_service_select(int cpu); * Used when a FQDAN/CDAN registration is made by drivers. */ struct dpaa2_io_notification_ctx { - void (*cb)(struct dpaa2_io_notification_ctx *); + void (*cb)(struct dpaa2_io_notification_ctx *ctx); int is_cdan; u32 id; int desired_cpu; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH PATCH net v2 3/4] hv_netvsc: fix locking for rx_mode
The rx_mode operation handler is different than other callbacks in that is not always called with rtnl held. Therefore use RCU to ensure that references are valid. Fixes: bee9d41b37ea ("hv_netvsc: propagate rx filters to VF") Signed-off-by: Stephen Hemminger --- drivers/net/hyperv/netvsc_drv.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index cdb78eefab67..48d9fa7a66c2 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -89,15 +89,20 @@ static void netvsc_change_rx_flags(struct net_device *net, int change) static void netvsc_set_rx_mode(struct net_device *net) { struct net_device_context *ndev_ctx = netdev_priv(net); - struct net_device *vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev); - struct netvsc_device *nvdev = rtnl_dereference(ndev_ctx->nvdev); + struct net_device *vf_netdev; + struct netvsc_device *nvdev; + rcu_read_lock(); + vf_netdev = rcu_dereference(ndev_ctx->vf_netdev); if (vf_netdev) { dev_uc_sync(vf_netdev, net); dev_mc_sync(vf_netdev, net); } - rndis_filter_update(nvdev); + nvdev = rcu_dereference(ndev_ctx->nvdev); + if (nvdev) + rndis_filter_update(nvdev); + rcu_read_unlock(); } static int netvsc_open(struct net_device *net) -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH PATCH net v2 4/4] hv_netvsc: fix locking during VF setup
The dev_uc/mc_sync calls need to have the device address list locked. This was spotted by running with lockdep enabled. Fixes: bee9d41b37ea ("hv_netvsc: propagate rx filters to VF") Signed-off-by: Stephen Hemminger --- drivers/net/hyperv/netvsc_drv.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 48d9fa7a66c2..faea0be18924 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -1851,8 +1851,12 @@ static void __netvsc_vf_setup(struct net_device *ndev, /* set multicast etc flags on VF */ dev_change_flags(vf_netdev, ndev->flags | IFF_SLAVE); + + /* sync address list from ndev to VF */ + netif_addr_lock_bh(ndev); dev_uc_sync(vf_netdev, ndev); dev_mc_sync(vf_netdev, ndev); + netif_addr_unlock_bh(ndev); if (netif_running(ndev)) { ret = dev_open(vf_netdev); -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH PATCH net v2 2/4] hv_netvsc: avoid repeated updates of packet filter
The netvsc driver can get repeated calls to netvsc_rx_mode during network setup; each of these calls ends up scheduling the lower layers to update tha packet filter. This update requires an request/response to the host. So avoid doing this if we already know that the correct packet filter value is set. Signed-off-by: Stephen Hemminger --- drivers/net/hyperv/hyperv_net.h | 1 + drivers/net/hyperv/rndis_filter.c | 8 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index 0db3bd1ea06f..cd538d5a7986 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -173,6 +173,7 @@ struct rndis_device { struct list_head req_list; struct work_struct mcast_work; + u32 filter; bool link_state;/* 0 - link up, 1 - link down */ diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index 411a3aee39b2..00ec80c23fe5 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -825,13 +825,15 @@ static int rndis_filter_set_packet_filter(struct rndis_device *dev, struct rndis_set_request *set; int ret; + if (dev->filter == new_filter) + return 0; + request = get_rndis_request(dev, RNDIS_MSG_SET, RNDIS_MESSAGE_SIZE(struct rndis_set_request) + sizeof(u32)); if (!request) return -ENOMEM; - /* Setup the rndis set */ set = &request->request_msg.msg.set_req; set->oid = RNDIS_OID_GEN_CURRENT_PACKET_FILTER; @@ -842,8 +844,10 @@ static int rndis_filter_set_packet_filter(struct rndis_device *dev, &new_filter, sizeof(u32)); ret = rndis_filter_send_request(dev, request); - if (ret == 0) + if (ret == 0) { wait_for_completion(&request->wait_event); + dev->filter = new_filter; + } put_rndis_request(dev, request); -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH PATCH net v2 1/4] hv_netvsc: fix filter flags
The recent change to nto always enable all multicast and broadcast was broken; meant to set filter, not change flags. Fixes: 009f766ca238 ("hv_netvsc: filter multicast/broadcast") Signed-off-by: Stephen Hemminger --- drivers/net/hyperv/rndis_filter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index 8927c483c217..411a3aee39b2 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -861,9 +861,9 @@ static void rndis_set_multicast(struct work_struct *w) filter = NDIS_PACKET_TYPE_PROMISCUOUS; } else { if (flags & IFF_ALLMULTI) - flags |= NDIS_PACKET_TYPE_ALL_MULTICAST; + filter |= NDIS_PACKET_TYPE_ALL_MULTICAST; if (flags & IFF_BROADCAST) - flags |= NDIS_PACKET_TYPE_BROADCAST; + filter |= NDIS_PACKET_TYPE_BROADCAST; } rndis_filter_set_packet_filter(rdev, filter); -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ashmem: Remove deadlock
On Wed, Mar 07, 2018 at 01:40:30PM -0800, Paul Lawrence wrote: > Regression introduced in commit ce8a3a9e76d0193e2e8d74a06d275b3c324ca652 > ("staging: android: ashmem: Fix a race condition in pin ioctls") > causing deadlock. > > No need to hold ashmem_mutex while copying from user > > Stacks are: > > ashmem_mmap+0x53/0x400 drivers/staging/android/ashmem.c:379 > mmap_region+0x7dd/0xfd0 mm/mmap.c:1694 > do_mmap+0x57b/0xbe0 mm/mmap.c:1473 > > and > > lock_acquire+0x12e/0x410 kernel/locking/lockdep.c:3756 > __might_fault+0x14a/0x1d0 mm/memory.c:4014 > copy_from_user arch/x86/include/asm/uaccess.h:705 [inline] > ashmem_pin_unpin drivers/staging/android/ashmem.c:719 [inline] > > Signed-off-by: Paul Lawrence > Cc: # 4.9.x > Cc: # 4.4.x > Cc: # 3.18.x: ce8a3a9e76d01 > Cc: # 3.18.x > Cc: Ben Hutchings > --- > drivers/staging/android/ashmem.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/android/ashmem.c > b/drivers/staging/android/ashmem.c > index 6dbba5aff191..8c55706c2cfa 100644 > --- a/drivers/staging/android/ashmem.c > +++ b/drivers/staging/android/ashmem.c > @@ -702,16 +702,14 @@ static int ashmem_pin_unpin(struct ashmem_area *asma, > unsigned long cmd, > size_t pgstart, pgend; > int ret = -EINVAL; > > + if (unlikely(copy_from_user(&pin, p, sizeof(pin > + return -EFAULT; > + > mutex_lock(&ashmem_mutex); > > if (unlikely(!asma->file)) > goto out_unlock; > > - if (unlikely(copy_from_user(&pin, p, sizeof(pin { > - ret = -EFAULT; > - goto out_unlock; > - } > - > /* per custom, you can pass zero for len to mean "everything onward" */ > if (!pin.len) > pin.len = PAGE_ALIGN(asma->size) - pin.offset; > -- > 2.16.2.395.g2e18187dfd-goog > Hey Paul, Looks like this same patch is already in Greg's tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id=740a5759bf222332fbb5eda42f89aa25ba38f9b2 Cheers! Nathan Chancellor ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH PATCH net v2 0/4] hv_netvsc: fix multicast flags and sync
This set of patches deals with the handling of multicast flags and addresses in transparent VF mode. The recent set of patches (in linux-net) had a couple of bugs. Stephen Hemminger (4): hv_netvsc: fix filter flags hv_netvsc: avoid repeated updates of packet filter hv_netvsc: fix locking for rx_mode hv_netvsc: fix locking during VF setup drivers/net/hyperv/hyperv_net.h | 1 + drivers/net/hyperv/netvsc_drv.c | 15 --- drivers/net/hyperv/rndis_filter.c | 12 3 files changed, 21 insertions(+), 7 deletions(-) -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ashmem: Remove deadlock
Regression introduced in commit ce8a3a9e76d0193e2e8d74a06d275b3c324ca652 ("staging: android: ashmem: Fix a race condition in pin ioctls") causing deadlock. No need to hold ashmem_mutex while copying from user Stacks are: ashmem_mmap+0x53/0x400 drivers/staging/android/ashmem.c:379 mmap_region+0x7dd/0xfd0 mm/mmap.c:1694 do_mmap+0x57b/0xbe0 mm/mmap.c:1473 and lock_acquire+0x12e/0x410 kernel/locking/lockdep.c:3756 __might_fault+0x14a/0x1d0 mm/memory.c:4014 copy_from_user arch/x86/include/asm/uaccess.h:705 [inline] ashmem_pin_unpin drivers/staging/android/ashmem.c:719 [inline] Signed-off-by: Paul Lawrence Cc: # 4.9.x Cc: # 4.4.x Cc: # 3.18.x: ce8a3a9e76d01 Cc: # 3.18.x Cc: Ben Hutchings --- drivers/staging/android/ashmem.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c index 6dbba5aff191..8c55706c2cfa 100644 --- a/drivers/staging/android/ashmem.c +++ b/drivers/staging/android/ashmem.c @@ -702,16 +702,14 @@ static int ashmem_pin_unpin(struct ashmem_area *asma, unsigned long cmd, size_t pgstart, pgend; int ret = -EINVAL; + if (unlikely(copy_from_user(&pin, p, sizeof(pin + return -EFAULT; + mutex_lock(&ashmem_mutex); if (unlikely(!asma->file)) goto out_unlock; - if (unlikely(copy_from_user(&pin, p, sizeof(pin { - ret = -EFAULT; - goto out_unlock; - } - /* per custom, you can pass zero for len to mean "everything onward" */ if (!pin.len) pin.len = PAGE_ALIGN(asma->size) - pin.offset; -- 2.16.2.395.g2e18187dfd-goog ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg()
> From: Lorenzo Pieralisi > Sent: Wednesday, March 7, 2018 04:35 > On Tue, Mar 06, 2018 at 06:21:56PM +, Dexuan Cui wrote: > > 1. With the patch "x86/vector/msi: Switch to global reservation mode" > > (4900be8360), the recent v4.15 and newer kernels always hang for 1-vCPU > > Hyper-V VM with SR-IOV. This is because when we reach > hv_compose_msi_msg() > > by request_irq() -> request_threaded_irq() -> __setup_irq()->irq_startup() > > -> __irq_startup() -> irq_domain_activate_irq() -> ... -> > > msi_domain_activate() -> ... -> hv_compose_msi_msg(), local irq is > > disabled in __setup_irq(). > > > > Fix this by polling the channel. > > > > 2. If the host is ejecting the VF device before we reach > > hv_compose_msi_msg(), in a UP VM, we can hang in hv_compose_msi_msg() > > forever, because at this time the host doesn't respond to the > > CREATE_INTERRUPT request. This issue also happens to old kernels like > > v4.14, v4.13, etc. > > If you are fixing a problem you should report what commit you are fixing > with a Fixes: tag and add a CC: sta...@vger.kernel.org to the commit log > to send it to stable kernels to which it should be applied; mentioning > kernel versions in the commit log is useless and should be omitted. Hi Lorenzo, Thanks for your comments! This patch does have a "Cc: sta...@vger.kernel.org" in the sign-off area. :-) Here the patch is made to resolve 2 issues: #1 is triggered by the x86 global reservation mode (4900be8360) patch. 4900be8360 in itself is good. It's just that drivers/pci/host/pci-hyperv.c should be fixed. #2 is a longstanding issue since the first day the pci-hyperv driver was accepted into the kernel. So IMO actually we don't really need to add a Fixes: tag, which is usually used to specify a specific commit that introduces a bug that is being fixed. > Side note: you should not have sta...@vger.kernel.org in the email > addresses CC list you are sending the patches to (you mark patches for > stable by adding an appropriate CC tag in the commit log). Sorry, I didn't know this, but actually I didn't add sta...@vger.kernel.org manually. Instead I used "git send-email" to send this patchset, and it told me "The Cc list above has been expanded by additional addresses found in the patch commit message." I didn't find a way to disable this behavior of "git send-email" by checking its manual and googling it. This is strange. > Here: > > git.kernel.org/.../Documentation/process/stable-kernel-rules.rst > > Last but not least, most of the patches in this series do not justify > sending them to stable kernels at all so you should remove the > corresponding tag from the patches. I hope at least these 2 patches can go into the stable kernels: [PATCH v3 3/6] PCI: hv: serialize the present/eject work items [PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg() Especially the second one, which fixes a real hang issue for UP virtual machines running v4.15 and newer. And, IMO the patches are small enough (<100 lines) , but definitely the maintainers make the final call. > > Thanks, > Lorenzo > > > Fix this by polling the channel for the PCI_EJECT message and > > hpdev->state, and by checking the PCI vendor ID. > > > > Note: actually the above issues also happen to a SMP VM, if > > "hbus->hdev->channel->target_cpu == smp_processor_id()" is true. > > > > Signed-off-by: Dexuan Cui > > Tested-by: Adrian Suhov > > Tested-by: Chris Valean > > Cc: sta...@vger.kernel.org > > Cc: Stephen Hemminger > > Cc: K. Y. Srinivasan > > Cc: Vitaly Kuznetsov > > Cc: Jack Morgenstein > > --- > > drivers/pci/host/pci-hyperv.c | 58 Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 4/4] Staging: iio: adis16209: Use sign_extend32 function
On Sun, 4 Mar 2018 18:15:06 +0530 Shreeya Patel wrote: > Use sign_extend32 function instead of manually coding > it. > > Signed-off-by: Shreeya Patel Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > > Changes in v3 > -After split patch. > > drivers/staging/iio/accel/adis16209.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index 9cb1ce0..a8453bf 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -212,9 +212,8 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, > ret = adis_read_reg_16(st, addr, &val16); > if (ret) > return ret; > - val16 &= (1 << bits) - 1; > - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits); > - *val = val16; > + > + *val = sign_extend32(val16, bits - 1); > return IIO_VAL_INT; > } > return -EINVAL; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 3/4] Staging: iio: adis16209: Adjust a switch statement
On Sun, 4 Mar 2018 18:13:12 +0530 Shreeya Patel wrote: > Adjust a switch block to explicitly match channels and > return -EINVAL as default case which makes the code > semantically more clear. > > Signed-off-by: Shreeya Patel Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan. > --- > > Changes in v3 > -After split patch. > > drivers/staging/iio/accel/adis16209.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index eb5c878..9cb1ce0 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -155,10 +155,16 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, > switch (chan->type) { > case IIO_VOLTAGE: > *val = 0; > - if (chan->channel == 0) > + switch (chan->channel) { > + case 0: > *val2 = 305180; /* 0.30518 mV */ > - else > + break; > + case 1: > *val2 = 610500; /* 0.6105 mV */ > + break; > + default: > + return -EINVAL; > + } > return IIO_VAL_INT_PLUS_MICRO; > case IIO_TEMP: > *val = -470; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/4] Staging: iio: adis16209: Change some macro names
On Sun, 4 Mar 2018 18:11:17 +0530 Shreeya Patel wrote: > Make some of the macro names according to the names > given in the datasheet of the adis16209 driver. > > Signed-off-by: Shreeya Patel A small comment inline which we should clear up in a follow up patch. > --- > > Changes in v3 > -Introduce this new patch for v3 of the series. > > drivers/staging/iio/accel/adis16209.c | 48 > +-- > 1 file changed, 24 insertions(+), 24 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c > b/drivers/staging/iio/accel/adis16209.c > index d8aef9c..eb5c878 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -68,21 +68,21 @@ > #define ADIS16209_MSC_CTRL_ACTIVE_HIGH BIT(1) > #define ADIS16209_MSC_CTRL_DATA_RDY_DIO2BIT(0) > > -#define ADIS16209_DIAG_STAT_REG 0x3C > -#define ADIS16209_DIAG_STAT_ALARM2 BIT(9) > -#define ADIS16209_DIAG_STAT_ALARM1 BIT(8) > -#define ADIS16209_DIAG_STAT_SELFTEST_FAIL_BIT5 > -#define ADIS16209_DIAG_STAT_SPI_FAIL_BIT 3 > -#define ADIS16209_DIAG_STAT_FLASH_UPT_BIT2 > +#define ADIS16209_STAT_REG 0x3C > +#define ADIS16209_STAT_ALARM2 BIT(9) > +#define ADIS16209_STAT_ALARM1 BIT(8) > +#define ADIS16209_STAT_SELFTEST_FAIL_BIT 5 > +#define ADIS16209_STAT_SPI_FAIL_BIT 3 > +#define ADIS16209_STAT_FLASH_UPT_FAIL_BIT2 Given these are also fields of the STAT_REG, be it defined in a different fashion I think they should also have the small additional indent. > /* Power supply above 3.625 V */ > -#define ADIS16209_DIAG_STAT_POWER_HIGH_BIT 1 > +#define ADIS16209_STAT_POWER_HIGH_BIT1 > /* Power supply below 3.15 V */ > -#define ADIS16209_DIAG_STAT_POWER_LOW_BIT0 > +#define ADIS16209_STAT_POWER_LOW_BIT 0 > > -#define ADIS16209_GLOB_CMD_REG 0x3E > -#define ADIS16209_GLOB_CMD_SW_RESET BIT(7) > -#define ADIS16209_GLOB_CMD_CLEAR_STAT BIT(4) > -#define ADIS16209_GLOB_CMD_FACTORY_CAL BIT(1) > +#define ADIS16209_CMD_REG0x3E > +#define ADIS16209_CMD_SW_RESET BIT(7) > +#define ADIS16209_CMD_CLEAR_STATBIT(4) > +#define ADIS16209_CMD_FACTORY_CAL BIT(1) > > #define ADIS16209_ERROR_ACTIVE BIT(14) > > @@ -238,29 +238,29 @@ static const struct iio_info adis16209_info = { > }; > > static const char * const adis16209_status_error_msgs[] = { > - [ADIS16209_DIAG_STAT_SELFTEST_FAIL_BIT] = "Self test failure", > - [ADIS16209_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure", > - [ADIS16209_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed", > - [ADIS16209_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V", > - [ADIS16209_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V", > + [ADIS16209_STAT_SELFTEST_FAIL_BIT] = "Self test failure", > + [ADIS16209_STAT_SPI_FAIL_BIT] = "SPI failure", > + [ADIS16209_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed", > + [ADIS16209_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V", > + [ADIS16209_STAT_POWER_LOW_BIT] = "Power supply below 3.15V", > }; > > static const struct adis_data adis16209_data = { > .read_delay = 30, > .msc_ctrl_reg = ADIS16209_MSC_CTRL_REG, > - .glob_cmd_reg = ADIS16209_GLOB_CMD_REG, > - .diag_stat_reg = ADIS16209_DIAG_STAT_REG, > + .glob_cmd_reg = ADIS16209_CMD_REG, > + .diag_stat_reg = ADIS16209_STAT_REG, > > .self_test_mask = ADIS16209_MSC_CTRL_SELF_TEST_EN, > .self_test_no_autoclear = true, > .startup_delay = ADIS16209_STARTUP_DELAY_MS, > > .status_error_msgs = adis16209_status_error_msgs, > - .status_error_mask = BIT(ADIS16209_DIAG_STAT_SELFTEST_FAIL_BIT) | > - BIT(ADIS16209_DIAG_STAT_SPI_FAIL_BIT) | > - BIT(ADIS16209_DIAG_STAT_FLASH_UPT_BIT) | > - BIT(ADIS16209_DIAG_STAT_POWER_HIGH_BIT) | > - BIT(ADIS16209_DIAG_STAT_POWER_LOW_BIT), > + .status_error_mask = BIT(ADIS16209_STAT_SELFTEST_FAIL_BIT) | > + BIT(ADIS16209_STAT_SPI_FAIL_BIT) | > + BIT(ADIS16209_STAT_FLASH_UPT_FAIL_BIT) | > + BIT(ADIS16209_STAT_POWER_HIGH_BIT) | > + BIT(ADIS16209_STAT_POWER_LOW_BIT), > }; > > static int adis16209_probe(struct spi_device *spi) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/4] Staging: iio: adis16209: Remove and add some comments and group the definitions
On Sun, 04 Mar 2018 18:37:04 +0530 Shreeya Patel wrote: > On Sun, 2018-03-04 at 18:26 +0530, Himanshu Jha wrote: > > Hi Shreeya, > > > > On Sun, Mar 04, 2018 at 06:06:22PM +0530, Shreeya Patel wrote: > > > > > > Remove some unnecessay comments and group the control > > > register and register field macros together. > > > Some of the register names does not make it's puporse > > > very clear and hence, add some comments for more > > > information. > > > Also there are certain unit based comments which are not > > > providing sufficient information, so expand those comments. > > > > > > Signed-off-by: Shreeya Patel > > > --- > > > > > > Changes in v3 > > > -This patch is the combination of two patches from the > > > previous series. Also add some more comments. > > > > > > > > > drivers/staging/iio/accel/adis16209.c | 132 ++-- > > > -- > > > 1 file changed, 39 insertions(+), 93 deletions(-) > > > > > > diff --git a/drivers/staging/iio/accel/adis16209.c > > > b/drivers/staging/iio/accel/adis16209.c > > > index 151120f..d8aef9c 100644 > > > --- a/drivers/staging/iio/accel/adis16209.c > > > +++ b/drivers/staging/iio/accel/adis16209.c > > > @@ -21,135 +21,70 @@ > > > #include > > > > > > #define ADIS16209_STARTUP_DELAY_MS 220 > > > - > > > -/* Flash memory write count */ > > > #define ADIS16209_FLASH_CNT_REG 0x00 > > > > > > -/* Output, power supply */ > > > +/* Data Output Register Definitions */ > > > #define ADIS16209_SUPPLY_OUT_REG 0x02 > > > - > > > -/* Output, x-axis accelerometer */ > > > #define ADIS16209_XACCL_OUT_REG 0x04 > > > - > > > -/* Output, y-axis accelerometer */ > > > #define ADIS16209_YACCL_OUT_REG 0x06 > > > - > > > /* Output, auxiliary ADC input */ > > > #define ADIS16209_AUX_ADC_REG0x08 > > > - > > > /* Output, temperature */ > > > #define ADIS16209_TEMP_OUT_REG 0x0A > > > - > > > -/* Output, x-axis inclination */ > > > +/* Output, +/- 90 degrees X-axis inclination */ > > > #define ADIS16209_XINCL_OUT_REG 0x0C > > > - > > > -/* Output, y-axis inclination */ > > > #define ADIS16209_YINCL_OUT_REG 0x0E > > > - > > > /* Output, +/-180 vertical rotational position */ > > > #define ADIS16209_ROT_OUT_REG0x10 > > > > > > -/* Calibration, x-axis acceleration offset null */ > > > +/* > > > + * Calibration Register Definitions. > > > + * Acceleration, inclination or rotation offset null. > > > + */ > > > #define ADIS16209_XACCL_NULL_REG 0x12 > > > - > > > -/* Calibration, y-axis acceleration offset null */ > > > #define ADIS16209_YACCL_NULL_REG 0x14 > > > - > > > -/* Calibration, x-axis inclination offset null */ > > > #define ADIS16209_XINCL_NULL_REG 0x16 > > > - > > > -/* Calibration, y-axis inclination offset null */ > > > #define ADIS16209_YINCL_NULL_REG 0x18 > > > - > > > -/* Calibration, vertical rotation offset null */ > > > #define ADIS16209_ROT_NULL_REG 0x1A > > > > > > -/* Alarm 1 amplitude threshold */ > > > +/* Alarm Register Definitions */ > > > #define ADIS16209_ALM_MAG1_REG 0x20 > > > - > > > -/* Alarm 2 amplitude threshold */ > > > #define ADIS16209_ALM_MAG2_REG 0x22 > > > - > > > -/* Alarm 1, sample period */ > > > #define ADIS16209_ALM_SMPL1_REG 0x24 > > > - > > > -/* Alarm 2, sample period */ > > > #define ADIS16209_ALM_SMPL2_REG 0x26 > > > - > > > -/* Alarm control */ > > > #define ADIS16209_ALM_CTRL_REG 0x28 > > > > > > -/* Auxiliary DAC data */ > > > #define ADIS16209_AUX_DAC_REG0x30 > > > - > > > -/* General-purpose digital input/output control */ > > > #define ADIS16209_GPIO_CTRL_REG 0x32 > > > - > > > -/* Miscellaneous control */ > > > -#define ADIS16209_MSC_CTRL_REG 0x34 > > > - > > > -/* Internal sample period (rate) control */ > > > #define ADIS16209_SMPL_PRD_REG 0x36 > > > - > > > -/* Operation, filter configuration */ > > > #define ADIS16209_AVG_CNT_REG0x38 > > > - > > > -/* Operation, sleep mode control */ > > > #define ADIS16209_SLP_CNT_REG0x3A > > > > > > -/* Diagnostics, system status register */ > > > -#define ADIS16209_DIAG_STAT_REG 0x3C > > > - > > > -/* Operation, system command register */ > > > -#define ADIS16209_GLOB_CMD_REG 0x3E > > > - > > > -/* MSC_CTRL */ > > > - > > > -/* Self-test at power-on: 1 = disabled, 0 = enabled */ > > > -#define ADIS16209_MSC_CTRL_PWRUP_SELF_TEST BIT(10) > > > - > > > -/* Self-test enable */ > > > -#define ADIS16209_MSC_CTRL_SELF_TEST_EN BIT(8) > > > - > > > -/* Data-ready enable: 1 = enabled, 0 = disabled */ > > > -#define ADIS16209_MSC_CTRL_DATA_RDY_EN BIT(2) > > > - > > > +#define ADIS16209_MSC_CTRL_REG 0x34 > > > +#define ADIS16209_MSC_CTRL_PWRUP_SELF_TEST BIT(10) > > > +#define ADIS16209_MSC_CTRL_SELF_TEST_EN BIT(8) > > > +#define ADIS16209_MSC_CTRL_DATA_RDY_EN
Re: [PATCH 11/11] Staging: iio: accel: Move adis16201 driver out of staging subsystem
On Mon, 5 Mar 2018 13:19:30 +0530 Himanshu Jha wrote: > Move the adis16201 driver out of staging directory and merge to the > mainline IIO subsystem. > > Signed-off-by: Himanshu Jha One comment inline (that I should have noticed in one of your earlier patches but missed). Michael, do you have the time to look through these at the moment? The changes are not likely to break anything so if you are too busy I'm happy to move them out of staging if you are fine with that. Thanks, Jonathan > --- > drivers/iio/accel/Kconfig | 12 ++ > drivers/iio/accel/Makefile| 1 + > drivers/iio/accel/adis16201.c | 323 > ++ > drivers/staging/iio/accel/Kconfig | 12 -- > drivers/staging/iio/accel/Makefile| 1 - > drivers/staging/iio/accel/adis16201.c | 323 > -- > 6 files changed, 336 insertions(+), 336 deletions(-) > create mode 100644 drivers/iio/accel/adis16201.c > delete mode 100644 drivers/staging/iio/accel/adis16201.c > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > index c6d9517..9416c6f 100644 > --- a/drivers/iio/accel/Kconfig > +++ b/drivers/iio/accel/Kconfig > @@ -5,6 +5,18 @@ > > menu "Accelerometers" > > +config ADIS16201 > +tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer > and Accelerometer" > +depends on SPI > +select IIO_ADIS_LIB > +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER > +help > + Say Y here to build support for Analog Devices adis16201 dual-axis > + digital inclinometer and accelerometer. > + > + To compile this driver as a module, say M here: the module will > + be called adis16201. > + > config ADXL345 > tristate > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > index 368aedb..7832ec9 100644 > --- a/drivers/iio/accel/Makefile > +++ b/drivers/iio/accel/Makefile > @@ -4,6 +4,7 @@ > # > > # When adding new entries keep the list in alphabetical order > +obj-$(CONFIG_ADIS16201) += adis16201.o > obj-$(CONFIG_ADXL345) += adxl345_core.o > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c > new file mode 100644 > index 000..946c7b1 > --- /dev/null > +++ b/drivers/iio/accel/adis16201.c > @@ -0,0 +1,323 @@ > +/* > + * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer > + * > + * Copyright 2010 Analog Devices Inc. > + * > + * Licensed under the GPL-2 or later. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#define ADIS16201_STARTUP_DELAY_MS 220 > +#define ADIS16201_FLASH_CNT 0x00 > + > +/* Data Output Register Information */ > +#define ADIS16201_SUPPLY_OUT_REG 0x02 > +#define ADIS16201_XACCL_OUT_REG 0x04 > +#define ADIS16201_YACCL_OUT_REG 0x06 > +#define ADIS16201_AUX_ADC_REG0x08 > +#define ADIS16201_TEMP_OUT_REG 0x0A > +#define ADIS16201_XINCL_OUT_REG 0x0C > +#define ADIS16201_YINCL_OUT_REG 0x0E > + > +/* Calibration Register Definition */ > +#define ADIS16201_XACCL_OFFS_REG 0x10 > +#define ADIS16201_YACCL_OFFS_REG 0x12 > +#define ADIS16201_XACCL_SCALE_REG0x14 > +#define ADIS16201_YACCL_SCALE_REG0x16 > +#define ADIS16201_XINCL_OFFS_REG 0x18 > +#define ADIS16201_YINCL_OFFS_REG 0x1A > +#define ADIS16201_XINCL_SCALE_REG0x1C > +#define ADIS16201_YINCL_SCALE_REG0x1E > + > +/* Alarm Register Definition */ > +#define ADIS16201_ALM_MAG1_REG 0x20 > +#define ADIS16201_ALM_MAG2_REG 0x22 > +#define ADIS16201_ALM_SMPL1_REG 0x24 > +#define ADIS16201_ALM_SMPL2_REG 0x26 > +#define ADIS16201_ALM_CTRL_REG 0x28 > + > +#define ADIS16201_AUX_DAC_REG0x30 > +#define ADIS16201_GPIO_CTRL_REG 0x32 > +#define ADIS16201_MSC_CTRL_REG 0x34 > +#define ADIS16201_SMPL_PRD_REG 0x36 > +#define ADIS16201_AVG_CNT_REG0x38 > +#define ADIS16201_SLP_CNT_REG0x3A > + > +/* Misce
[PATCH v2] staging: lustre: Remove VLA usage
The kernel would like to have all stack VLA usage removed[1]. This switches to a simple kasprintf() instead, and in the process fixes an off-by-one between the allocation and the sprintf (allocation did not include NULL byte in calculation). [1] https://lkml.org/lkml/2018/3/7/621 Signed-off-by: Kees Cook Reviewed-by: Rasmus Villemoes --- drivers/staging/lustre/lustre/llite/xattr.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c index 532384c91447..ff6fe81a4ddb 100644 --- a/drivers/staging/lustre/lustre/llite/xattr.c +++ b/drivers/staging/lustre/lustre/llite/xattr.c @@ -87,10 +87,10 @@ ll_xattr_set_common(const struct xattr_handler *handler, const char *name, const void *value, size_t size, int flags) { - char fullname[strlen(handler->prefix) + strlen(name) + 1]; struct ll_sb_info *sbi = ll_i2sbi(inode); struct ptlrpc_request *req = NULL; const char *pv = value; + char *fullname; __u64 valid; int rc; @@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler, return -EPERM; } - sprintf(fullname, "%s%s\n", handler->prefix, name); + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name); + if (!fullname) + return -ENOMEM; rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode), valid, fullname, pv, size, 0, flags, ll_i2suppgid(inode), &req); + kfree(fullname); if (rc) { if (rc == -EOPNOTSUPP && handler->flags == XATTR_USER_T) { LCONSOLE_INFO("Disabling user_xattr feature because it is not supported on the server\n"); @@ -364,11 +367,11 @@ static int ll_xattr_get_common(const struct xattr_handler *handler, struct dentry *dentry, struct inode *inode, const char *name, void *buffer, size_t size) { - char fullname[strlen(handler->prefix) + strlen(name) + 1]; struct ll_sb_info *sbi = ll_i2sbi(inode); #ifdef CONFIG_FS_POSIX_ACL struct ll_inode_info *lli = ll_i2info(inode); #endif + char *fullname; int rc; CDEBUG(D_VFSTRACE, "VFS Op:inode=" DFID "(%p)\n", @@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct xattr_handler *handler, if (handler->flags == XATTR_ACL_DEFAULT_T && !S_ISDIR(inode->i_mode)) return -ENODATA; #endif - sprintf(fullname, "%s%s\n", handler->prefix, name); - return ll_xattr_list(inode, fullname, handler->flags, buffer, size, -OBD_MD_FLXATTR); + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name); + if (!fullname) + return -ENOMEM; + rc = ll_xattr_list(inode, fullname, handler->flags, buffer, size, + OBD_MD_FLXATTR); + kfree(fullname); + return rc; } static ssize_t ll_getxattr_lov(struct inode *inode, void *buf, size_t buf_size) -- 2.7.4 -- Kees Cook Pixel Security ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
On Mon, 5 Mar 2018 13:19:29 +0530 Himanshu Jha wrote: > Clarify the conversion and formation of resultant data in the > adis16201_read_raw() with sufficient comments. > > Signed-off-by: Himanshu Jha This is fine but it needs to be in the original comment changing patch rather than removing the comments first then a few patches later putting back a different version. So good change but in the wrong place in the series. Learning to reorder a series and merge down multiple patches into one is very useful when working with git. Thanks, Jonathan > --- > drivers/staging/iio/accel/adis16201.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/staging/iio/accel/adis16201.c > b/drivers/staging/iio/accel/adis16201.c > index 8d795e2..946c7b1 100644 > --- a/drivers/staging/iio/accel/adis16201.c > +++ b/drivers/staging/iio/accel/adis16201.c > @@ -130,6 +130,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev, > *val2 = 0; > return IIO_VAL_INT_PLUS_MICRO; > case IIO_ACCEL: > +/* > + * IIO base unit for sensitivity of accelerometer > + * is milli g. > + * 1 LSB represents 0.244 mg. > + */ > *val = 0; > *val2 = IIO_G_TO_M_S_2(462400); > return IIO_VAL_INT_PLUS_NANO; > @@ -142,6 +147,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev, > } > break; > case IIO_CHAN_INFO_OFFSET: > +/* > + * The raw ADC value is 0x4FE when the temperature > + * is 25 degrees and the scale factor per milli > + * degree celcius is -470. > + */ > *val = 25000 / -470 - 1278; > return IIO_VAL_INT; > case IIO_CHAN_INFO_CALIBBIAS: ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: Remove VLA usage
On Wed, Mar 07 2018, Kees Cook wrote: > On Wed, Mar 7, 2018 at 5:10 AM, Rasmus Villemoes > wrote: >> On 2018-03-07 06:46, Kees Cook wrote: >>> The kernel would like to remove all VLA usage. This switches to a >>> simple kasprintf() instead. >>> >> >> It's probably worth pointing out that this actually fixes an >> unconditional buffer overflow: fullname only has room for the two >> strings and the '\n', but vsnprintf() is told that the buffer has >> infinite size (well, INT_MAX), so there should be plenty of room to >> append the '\0' after the '\n'. >> > > Oh yes, hah. I didn't even see the \n in the string. :P > > So, both a VLA fix and a buffer over-run fix. Can I add your "Reviewed-by"? :) Sure, Reviewed-by: Rasmus Villemoes A nit, if you're resending anyway: can you move the "char *fullname" declarations down a bit, to between pv,valid, and lli,rc, respectively? That keeps the initialized and uninitialized variables nicely together and ends up looking better. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/11] Staging: iio: accel: Use switch statement than if-else
On Mon, 5 Mar 2018 13:19:27 +0530 Himanshu Jha wrote: > Use switch statement instead of if-else pair to explicitly match > the only two channels present. > > Signed-off-by: Himanshu Jha I think this is going to generate some warnings in the various static analysers as they will point out there are lots of values channel can take that aren't handled by the switch statement.. You should have a default. (This is what made Dan less than convinced of whether this was a good change when I originally suggested it) I still think it's a marginal improvement in making it explicit that we only have two valid choices though - and Dan didn't care strongly about it. Jonathan > --- > drivers/staging/iio/accel/adis16201.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16201.c > b/drivers/staging/iio/accel/adis16201.c > index 1737708..307d4ab 100644 > --- a/drivers/staging/iio/accel/adis16201.c > +++ b/drivers/staging/iio/accel/adis16201.c > @@ -114,12 +114,15 @@ static int adis16201_read_raw(struct iio_dev *indio_dev, > case IIO_CHAN_INFO_SCALE: > switch (chan->type) { > case IIO_VOLTAGE: > - if (chan->channel == 0) { > + switch (chan->channel) { > + case 0: > *val = 1; > *val2 = 22; > - } else { > + break; > + case 1: > *val = 0; > *val2 = 61; > + break; > } > return IIO_VAL_INT_PLUS_MICRO; > case IIO_TEMP: ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/11] Staging: iio: accel: Add _REG suffix to registers
On Mon, 5 Mar 2018 13:19:24 +0530 Himanshu Jha wrote: > Addition of _REG suffix to the register definitions allows a distinction > between registers and register fields. The various registers and its field > bits are grouped together to improve readability and easy indentification. > > Signed-off-by: Himanshu Jha This one looks good. I will pick it up once you've tweaked the predecessor patches. Thanks, Jonathan > --- > drivers/staging/iio/accel/adis16201.c | 133 > -- > 1 file changed, 61 insertions(+), 72 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16201.c > b/drivers/staging/iio/accel/adis16201.c > index 445cb56..476b1c3 100644 > --- a/drivers/staging/iio/accel/adis16201.c > +++ b/drivers/staging/iio/accel/adis16201.c > @@ -20,75 +20,64 @@ > #include > #include > > -#define ADIS16201_STARTUP_DELAY_MS 220 > -#define ADIS16201_FLASH_CNT 0x00 > +#define ADIS16201_STARTUP_DELAY_MS 220 > +#define ADIS16201_FLASH_CNT 0x00 > > /* Data Output Register Information */ > -#define ADIS16201_SUPPLY_OUT 0x02 > -#define ADIS16201_XACCL_OUT 0x04 > -#define ADIS16201_YACCL_OUT 0x06 > -#define ADIS16201_AUX_ADC0x08 > -#define ADIS16201_TEMP_OUT 0x0A > -#define ADIS16201_XINCL_OUT 0x0C > -#define ADIS16201_YINCL_OUT 0x0E > +#define ADIS16201_SUPPLY_OUT_REG 0x02 > +#define ADIS16201_XACCL_OUT_REG 0x04 > +#define ADIS16201_YACCL_OUT_REG 0x06 > +#define ADIS16201_AUX_ADC_REG0x08 > +#define ADIS16201_TEMP_OUT_REG 0x0A > +#define ADIS16201_XINCL_OUT_REG 0x0C > +#define ADIS16201_YINCL_OUT_REG 0x0E > > /* Calibration Register Definition */ > -#define ADIS16201_XACCL_OFFS 0x10 > -#define ADIS16201_YACCL_OFFS 0x12 > -#define ADIS16201_XACCL_SCALE0x14 > -#define ADIS16201_YACCL_SCALE0x16 > -#define ADIS16201_XINCL_OFFS 0x18 > -#define ADIS16201_YINCL_OFFS 0x1A > -#define ADIS16201_XINCL_SCALE0x1C > -#define ADIS16201_YINCL_SCALE0x1E > +#define ADIS16201_XACCL_OFFS_REG 0x10 > +#define ADIS16201_YACCL_OFFS_REG 0x12 > +#define ADIS16201_XACCL_SCALE_REG0x14 > +#define ADIS16201_YACCL_SCALE_REG0x16 > +#define ADIS16201_XINCL_OFFS_REG 0x18 > +#define ADIS16201_YINCL_OFFS_REG 0x1A > +#define ADIS16201_XINCL_SCALE_REG0x1C > +#define ADIS16201_YINCL_SCALE_REG0x1E > > /* Alarm Register Definition */ > -#define ADIS16201_ALM_MAG1 0x20 > -#define ADIS16201_ALM_MAG2 0x22 > -#define ADIS16201_ALM_SMPL1 0x24 > -#define ADIS16201_ALM_SMPL2 0x26 > -#define ADIS16201_ALM_CTRL 0x28 > - > -#define ADIS16201_AUX_DAC0x30 > -#define ADIS16201_GPIO_CTRL 0x32 > -#define ADIS16201_MSC_CTRL 0x34 > - > -#define ADIS16201_SMPL_PRD 0x36 > -#define ADIS16201_AVG_CNT0x38 > -#define ADIS16201_SLP_CNT0x3A > +#define ADIS16201_ALM_MAG1_REG 0x20 > +#define ADIS16201_ALM_MAG2_REG 0x22 > +#define ADIS16201_ALM_SMPL1_REG 0x24 > +#define ADIS16201_ALM_SMPL2_REG 0x26 > +#define ADIS16201_ALM_CTRL_REG 0x28 > + > +#define ADIS16201_AUX_DAC_REG0x30 > +#define ADIS16201_GPIO_CTRL_REG 0x32 > +#define ADIS16201_MSC_CTRL_REG 0x34 > +#define ADIS16201_SMPL_PRD_REG 0x36 > +#define ADIS16201_AVG_CNT_REG0x38 > +#define ADIS16201_SLP_CNT_REG0x3A > + > +/* Miscellaneous Control Register Definition */ > +#define ADIS16201_MSC_CTRL_REG 0x34 > +#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8) > +#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2) > +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1) > +#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1BIT(0) > > /* Diagnostics, system status register */ > -#define ADIS16201_DIAG_STAT 0x3C > +#define ADIS16201_DIAG_STAT_REG 0x3C > +#define ADIS16201_DIAG_STAT_ALARM2 BIT(9) > +#define ADIS16201_DIAG_STAT_ALARM1 BIT(8) > +#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT
Re: [PATCH 04/11] Staging: iio: accel: Rename few macro definitions
On Mon, 5 Mar 2018 13:19:23 +0530 Himanshu Jha wrote: > Rename few macros with appropriate names specifying their usage/function. Most of these are obviously good, but for one I didn't understand your reasoning. Given there are only a few of them, I'd put some more description here to explain why you changed each one. Thanks, Jonathan > > Signed-off-by: Himanshu Jha > --- > drivers/staging/iio/accel/adis16201.c | 17 - > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16201.c > b/drivers/staging/iio/accel/adis16201.c > index 59c1166..445cb56 100644 > --- a/drivers/staging/iio/accel/adis16201.c > +++ b/drivers/staging/iio/accel/adis16201.c > @@ -20,9 +20,8 @@ > #include > #include > > -#define ADIS16201_STARTUP_DELAY 220 > - > -#define ADIS16201_FLASH_CNT 0x00 > +#define ADIS16201_STARTUP_DELAY_MS 220 > +#define ADIS16201_FLASH_CNT 0x00 > > /* Data Output Register Information */ > #define ADIS16201_SUPPLY_OUT 0x02 > @@ -69,7 +68,7 @@ > > #define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2) > > -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1) > +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1) > > #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0) > > @@ -80,14 +79,14 @@ > > #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3 > > -#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2 > +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2 > > #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1 > > #define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0 > > #define ADIS16201_GLOB_CMD_SW_RESET BIT(7) > -#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1) > +#define ADIS16201_GLOB_CMD_FACTORY BIT(1) What was the logic behind this particular change? > > #define ADIS16201_ERROR_ACTIVE BIT(14) > > @@ -231,7 +230,7 @@ static const struct iio_info adis16201_info = { > > static const char * const adis16201_status_error_msgs[] = { > [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure", > - [ADIS16201_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed", > + [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed", > [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V", > [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V", > }; > @@ -244,11 +243,11 @@ static const struct adis_data adis16201_data = { > > .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN, > .self_test_no_autoclear = true, > - .startup_delay = ADIS16201_STARTUP_DELAY, > + .startup_delay = ADIS16201_STARTUP_DELAY_MS, > > .status_error_msgs = adis16201_status_error_msgs, > .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) | > - BIT(ADIS16201_DIAG_STAT_FLASH_UPT_BIT) | > + BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) | > BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) | > BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT), > }; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments
On Mon, 5 Mar 2018 13:19:22 +0530 Himanshu Jha wrote: > Remove unnecessary comments since the definitions are pretty clear > with their macro names. > > Signed-off-by: Himanshu Jha Hi, The art of commenting (and indeed removing comments) is never a clear one. I agree with the vast majority of what you have here, but as with other drivers in this set, I think there are a couple of places comments are useful and even in one case where the comment should be expanded to become useful. I'd also like to see the patches reordered so the renames occur before this one. That will make some of the changes you have made in here a lot more obviously good! Thanks, Jonathan. > --- > drivers/staging/iio/accel/adis16201.c | 82 > +-- > 1 file changed, 10 insertions(+), 72 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16201.c > b/drivers/staging/iio/accel/adis16201.c > index 0fae8aa..59c1166 100644 > --- a/drivers/staging/iio/accel/adis16201.c > +++ b/drivers/staging/iio/accel/adis16201.c > @@ -20,87 +20,42 @@ > #include > #include > > -#define ADIS16201_STARTUP_DELAY 220 /* ms */ > +#define ADIS16201_STARTUP_DELAY 220 As below, this would have made a lot more sense if you had the rename of the macro patch (I assume the unit will be added to the name) before this one. > > -/* Flash memory write count */ > #define ADIS16201_FLASH_CNT 0x00 > > -/* Output, power supply */ > +/* Data Output Register Information */ > #define ADIS16201_SUPPLY_OUT 0x02 > - > -/* Output, x-axis accelerometer */ > #define ADIS16201_XACCL_OUT 0x04 > - > -/* Output, y-axis accelerometer */ > #define ADIS16201_YACCL_OUT 0x06 > - > -/* Output, auxiliary ADC input */ > #define ADIS16201_AUX_ADC0x08 > - > -/* Output, temperature */ > #define ADIS16201_TEMP_OUT 0x0A > - > -/* Output, x-axis inclination */ > #define ADIS16201_XINCL_OUT 0x0C > - > -/* Output, y-axis inclination */ > #define ADIS16201_YINCL_OUT 0x0E > > -/* Calibration, x-axis acceleration offset */ > +/* Calibration Register Definition */ > #define ADIS16201_XACCL_OFFS 0x10 > - > -/* Calibration, y-axis acceleration offset */ > #define ADIS16201_YACCL_OFFS 0x12 > - > -/* x-axis acceleration scale factor */ > #define ADIS16201_XACCL_SCALE0x14 > - > -/* y-axis acceleration scale factor */ > #define ADIS16201_YACCL_SCALE0x16 > - > -/* Calibration, x-axis inclination offset */ > #define ADIS16201_XINCL_OFFS 0x18 > - > -/* Calibration, y-axis inclination offset */ > #define ADIS16201_YINCL_OFFS 0x1A > - > -/* x-axis inclination scale factor */ > #define ADIS16201_XINCL_SCALE0x1C > - > -/* y-axis inclination scale factor */ > #define ADIS16201_YINCL_SCALE0x1E > > -/* Alarm 1 amplitude threshold */ > +/* Alarm Register Definition */ > #define ADIS16201_ALM_MAG1 0x20 > - > -/* Alarm 2 amplitude threshold */ > #define ADIS16201_ALM_MAG2 0x22 > - > -/* Alarm 1, sample period */ > #define ADIS16201_ALM_SMPL1 0x24 > - > -/* Alarm 2, sample period */ > #define ADIS16201_ALM_SMPL2 0x26 > - > -/* Alarm control */ > #define ADIS16201_ALM_CTRL 0x28 > > -/* Auxiliary DAC data */ > #define ADIS16201_AUX_DAC0x30 > - > -/* General-purpose digital input/output control */ > #define ADIS16201_GPIO_CTRL 0x32 > - > -/* Miscellaneous control */ > #define ADIS16201_MSC_CTRL 0x34 > > -/* Internal sample period (rate) control */ > #define ADIS16201_SMPL_PRD 0x36 > - > -/* Operation, filter configuration */ > #define ADIS16201_AVG_CNT0x38 > - > -/* Operation, sleep mode control */ > #define ADIS16201_SLP_CNT0x3A > > /* Diagnostics, system status register */ > @@ -109,42 +64,28 @@ > /* Operation, system command register */ > #define ADIS16201_GLOB_CMD 0x3E > > -/* MSC_CTRL */ > > -/* Self-test enable */ > #define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8) > > -/* Data-ready enable: 1 = enabled, 0 = disabled */ > #define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2) > > -/* Data-ready polarity: 1 = active high, 0 = active low */ > #define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1) > > -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */ > #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0) > > -/* DIAG_STAT */ > > -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */ > #define ADIS16201_DIAG_STAT_ALARM2BIT(9) > > -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */ > #define ADIS16201_DIAG_STAT_ALARM1BIT(8) > > -/* SPI communications failure */ > #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3 > > -/* Flash update failure */ > #define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2 I would have preferred this and the next patch to be in the opposite order as then it would have been obvious when reading this patch on why the comment here is no longer needed. > > -/* Power supply ab
Re: [PATCH 02/11] Staging: iio: accel: Add a blank space before returns
On Mon, 5 Mar 2018 13:19:21 +0530 Himanshu Jha wrote: > Adding a blank space before/after some returns improves readability. > > Signed-off-by: Himanshu Jha Applied - patch title adjusted. Thanks, Jonathan > --- > drivers/staging/iio/accel/adis16201.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/staging/iio/accel/adis16201.c > b/drivers/staging/iio/accel/adis16201.c > index 0f6a204..0fae8aa 100644 > --- a/drivers/staging/iio/accel/adis16201.c > +++ b/drivers/staging/iio/accel/adis16201.c > @@ -232,6 +232,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev, > *val = val16; > return IIO_VAL_INT; > } > + > return -EINVAL; > } > > @@ -262,6 +263,7 @@ static int adis16201_write_raw(struct iio_dev *indio_dev, > addr = adis16201_addresses[chan->scan_index]; > return adis_write_reg_16(st, addr, val16); > } > + > return -EINVAL; > } > > @@ -336,6 +338,7 @@ static int adis16201_probe(struct spi_device *spi) > ret = adis_init(st, indio_dev, spi, &adis16201_data); > if (ret) > return ret; > + > ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL); > if (ret) > return ret; > @@ -348,6 +351,7 @@ static int adis16201_probe(struct spi_device *spi) > ret = iio_device_register(indio_dev); > if (ret < 0) > goto error_cleanup_buffer_trigger; > + > return 0; > > error_cleanup_buffer_trigger: ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/11] Staging: iio: accel: Prefer alphabetical sequence of header files
On Mon, 5 Mar 2018 13:19:20 +0530 Himanshu Jha wrote: > Arrange header files in alphabetical sequence to improve readability. > > Signed-off-by: Himanshu Jha One general comment - when naming a patch it will form the description in the git short log or on the various web interfaces etc. It's really useful to actually say which driver it is. I've fixed that up by adding adis16201: in the appropriate place. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/staging/iio/accel/adis16201.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16201.c > b/drivers/staging/iio/accel/adis16201.c > index 2ebd275..0f6a204 100644 > --- a/drivers/staging/iio/accel/adis16201.c > +++ b/drivers/staging/iio/accel/adis16201.c > @@ -7,13 +7,13 @@ > */ > > #include > -#include > #include > #include > -#include > +#include > +#include > #include > +#include > #include > -#include > > #include > #include ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/3] staging:iio:meter: Aligns open parenthesis
On Tue, 6 Mar 2018 21:44:26 -0300 Rodrigo Siqueira wrote: > This patch fixes the checkpatch.pl checks: > > staging/iio/meter/ade7854-spi.c:19: CHECK: Alignment should match open > parenthesis > staging/iio/meter/ade7854-spi.c:44: CHECK: Alignment should match open > parenthesis > staging/iio/meter/ade7854-spi.c:70: CHECK: Alignment should match open > parenthesis > staging/iio/meter/ade7854-spi.c:97: CHECK: Alignment should match open > parenthesis > staging/iio/meter/ade7854-spi.c:125: CHECK: Alignment should match open > parenthesis > > ... > > Signed-off-by: Rodrigo Siqueira Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/staging/iio/meter/ade7758_trigger.c | 6 ++--- > drivers/staging/iio/meter/ade7854-spi.c | 40 > ++--- > 2 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/drivers/staging/iio/meter/ade7758_trigger.c > b/drivers/staging/iio/meter/ade7758_trigger.c > index 1f0d1a0cf889..3314aa298d99 100644 > --- a/drivers/staging/iio/meter/ade7758_trigger.c > +++ b/drivers/staging/iio/meter/ade7758_trigger.c > @@ -30,7 +30,7 @@ static irqreturn_t ade7758_data_rdy_trig_poll(int irq, void > *private) > * ade7758_data_rdy_trigger_set_state() set datardy interrupt state > **/ > static int ade7758_data_rdy_trigger_set_state(struct iio_trigger *trig, > - bool state) > + bool state) > { > struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); > > @@ -63,8 +63,8 @@ int ade7758_probe_trigger(struct iio_dev *indio_dev) > int ret; > > st->trig = iio_trigger_alloc("%s-dev%d", > - spi_get_device_id(st->us)->name, > - indio_dev->id); > + spi_get_device_id(st->us)->name, > + indio_dev->id); > if (!st->trig) { > ret = -ENOMEM; > goto error_ret; > diff --git a/drivers/staging/iio/meter/ade7854-spi.c > b/drivers/staging/iio/meter/ade7854-spi.c > index 72eddfec21f7..4419b8f06197 100644 > --- a/drivers/staging/iio/meter/ade7854-spi.c > +++ b/drivers/staging/iio/meter/ade7854-spi.c > @@ -16,8 +16,8 @@ > #include "ade7854.h" > > static int ade7854_spi_write_reg_8(struct device *dev, > - u16 reg_address, > - u8 val) > +u16 reg_address, > +u8 val) > { > int ret; > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > @@ -41,8 +41,8 @@ static int ade7854_spi_write_reg_8(struct device *dev, > } > > static int ade7854_spi_write_reg_16(struct device *dev, > - u16 reg_address, > - u16 val) > + u16 reg_address, > + u16 val) > { > int ret; > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > @@ -67,8 +67,8 @@ static int ade7854_spi_write_reg_16(struct device *dev, > } > > static int ade7854_spi_write_reg_24(struct device *dev, > - u16 reg_address, > - u32 val) > + u16 reg_address, > + u32 val) > { > int ret; > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > @@ -94,8 +94,8 @@ static int ade7854_spi_write_reg_24(struct device *dev, > } > > static int ade7854_spi_write_reg_32(struct device *dev, > - u16 reg_address, > - u32 val) > + u16 reg_address, > + u32 val) > { > int ret; > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > @@ -122,8 +122,8 @@ static int ade7854_spi_write_reg_32(struct device *dev, > } > > static int ade7854_spi_read_reg_8(struct device *dev, > - u16 reg_address, > - u8 *val) > + u16 reg_address, > + u8 *val) > { > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > struct ade7854_state *st = iio_priv(indio_dev); > @@ -149,7 +149,7 @@ static int ade7854_spi_read_reg_8(struct device *dev, > ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); > if (ret) { > dev_err(&st->spi->dev, "problem when reading 8 bit register > 0x%02X", > - reg_address); > + reg_address); > goto error_ret; > } > *val = st->rx[0]; > @@ -160,8 +160,8 @@ static int ade7854_spi_read_reg_8(struct device *dev, > } > > static int ade7854_spi_read_reg_16(struct device *dev, > - u16 reg_address, > - u16 *val) > +u16 reg_address, > +u16 *val) > { > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > str
Re: [PATCH v2 2/3] staging:iio:meter: Remove unused macro IIO_DEV_ATTR_CH_OFF
On Tue, 6 Mar 2018 21:44:07 -0300 Rodrigo Siqueira wrote: > This patch removes the macro IIO_DEV_ATTR_CH_OFF. The macro > IIO_DEV_ATTR_CH_OFF is not required, due to the replace of it by the > direct use of IIO_DEVICE_ATTR in files staging/iio/meter/ade7759.c and > staging/iio/meter/ade7753.c. > > Signed-off-by: Rodrigo Siqueira Doh! I should have read to the next patch. Personally I wouldn't have split this in to, but it is a small thing so doesn't matter. Applied to the togreg branch of iio.git (having reverted my tweak to the previous patch) and pushed out as testing. Thanks, Jonathan > --- > drivers/staging/iio/meter/meter.h | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/staging/iio/meter/meter.h > b/drivers/staging/iio/meter/meter.h > index edf26302fa57..5ed59bf30a25 100644 > --- a/drivers/staging/iio/meter/meter.h > +++ b/drivers/staging/iio/meter/meter.h > @@ -348,9 +348,6 @@ > #define IIO_DEV_ATTR_VPERIOD(_mode, _show, _store, _addr)\ > IIO_DEVICE_ATTR(vperiod, _mode, _show, _store, _addr) > > -#define IIO_DEV_ATTR_CH_OFF(_num, _mode, _show, _store, _addr) > \ > - IIO_DEVICE_ATTR(choff_##_num, _mode, _show, _store, _addr) > - > /* active energy register, AENERGY, is more than half full */ > #define IIO_EVENT_ATTR_AENERGY_HALF_FULL(_evlist, _show, _store, _mask) \ > IIO_EVENT_ATTR_SH(aenergy_half_full, _evlist, _show, _store, _mask) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR
On Tue, 6 Mar 2018 21:43:47 -0300 Rodrigo Siqueira wrote: > The macro IIO_DEV_ATTR_CH_OFF is a wrapper for IIO_DEVICE_ATTR, with a > tiny change in the name definition. This extra macro does not improve > the readability and also creates some checkpatch errors. > > This patch fixes the checkpatch.pl errors: > > staging/iio/meter/ade7753.c:391: ERROR: Use 4 digit octal (0777) not > decimal permissions > staging/iio/meter/ade7753.c:395: ERROR: Use 4 digit octal (0777) not > decimal permissions > staging/iio/meter/ade7759.c:331: ERROR: Use 4 digit octal (0777) not > decimal permissions > staging/iio/meter/ade7759.c:335: ERROR: Use 4 digit octal (0777) not > decimal permissions > > Signed-off-by: Rodrigo Siqueira Hmm. I wondered a bit about this one. It's a correct patch in of itself but the interface in question doesn't even vaguely conform to any of defined IIO ABI. Anyhow, it's still and improvement so I'll take it. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. I also added the removal of the header define which is no longer used. Please note, following discussions with Michael, I am going to send an email announcing an intent to drop these meter drivers next cycle unless someone can provide testing for any attempt to move them out of staging. I'm still taking patches on the basis that 'might' happen - but I wouldn't focus on these until we have some certainty on whether they will be around long term! Jonathan > --- > drivers/staging/iio/meter/ade7753.c | 18 ++ > drivers/staging/iio/meter/ade7759.c | 18 ++ > 2 files changed, 20 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/iio/meter/ade7753.c > b/drivers/staging/iio/meter/ade7753.c > index c44eb577dc35..275e8dfff836 100644 > --- a/drivers/staging/iio/meter/ade7753.c > +++ b/drivers/staging/iio/meter/ade7753.c > @@ -388,14 +388,16 @@ static IIO_DEV_ATTR_VPERIOD(0444, > ade7753_read_16bit, > NULL, > ADE7753_PERIOD); > -static IIO_DEV_ATTR_CH_OFF(1, 0644, > - ade7753_read_8bit, > - ade7753_write_8bit, > - ADE7753_CH1OS); > -static IIO_DEV_ATTR_CH_OFF(2, 0644, > - ade7753_read_8bit, > - ade7753_write_8bit, > - ADE7753_CH2OS); > + > +static IIO_DEVICE_ATTR(choff_1, 0644, > + ade7753_read_8bit, > + ade7753_write_8bit, > + ADE7753_CH1OS); > + > +static IIO_DEVICE_ATTR(choff_2, 0644, > + ade7753_read_8bit, > + ade7753_write_8bit, > + ADE7753_CH2OS); > > static int ade7753_set_irq(struct device *dev, bool enable) > { > diff --git a/drivers/staging/iio/meter/ade7759.c > b/drivers/staging/iio/meter/ade7759.c > index 1decb2b8afab..c078b770fa53 100644 > --- a/drivers/staging/iio/meter/ade7759.c > +++ b/drivers/staging/iio/meter/ade7759.c > @@ -328,14 +328,16 @@ static IIO_DEV_ATTR_ACTIVE_POWER_GAIN(0644, > ade7759_read_16bit, > ade7759_write_16bit, > ADE7759_APGAIN); > -static IIO_DEV_ATTR_CH_OFF(1, 0644, > - ade7759_read_8bit, > - ade7759_write_8bit, > - ADE7759_CH1OS); > -static IIO_DEV_ATTR_CH_OFF(2, 0644, > - ade7759_read_8bit, > - ade7759_write_8bit, > - ADE7759_CH2OS); > + > +static IIO_DEVICE_ATTR(choff_1, 0644, > + ade7759_read_8bit, > + ade7759_write_8bit, > + ADE7759_CH1OS); > + > +static IIO_DEVICE_ATTR(choff_2, 0644, > + ade7759_read_8bit, > + ade7759_write_8bit, > + ADE7759_CH2OS); > > static int ade7759_set_irq(struct device *dev, bool enable) > { ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: meter: Remove reduntant __func__ from debug print
On Wed, 7 Mar 2018 11:08:04 +0530 hariprasath.ela...@gmail.com wrote: > From: HariPrasath Elango > > dev_dbg includes the function name & line number by default when dynamic > debugging is enabled. Hence__func__ is reduntant here and removed. > > Signed-off-by: HariPrasath Elango Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Please note that, after discussions with Michael, I'm planning to send out an email stating these meter drivers will be dropped next cycle unless we have someone come forward who can test them. We can't realistically do the work needed to move them out of testing without a high risk of breaking them and they are 'not suitable for new designs'. Ah well, we'll see - someone might care and be able to help Thanks Jonathan > --- > drivers/staging/iio/meter/ade7758_trigger.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/meter/ade7758_trigger.c > b/drivers/staging/iio/meter/ade7758_trigger.c > index 1f0d1a0..da489ae 100644 > --- a/drivers/staging/iio/meter/ade7758_trigger.c > +++ b/drivers/staging/iio/meter/ade7758_trigger.c > @@ -34,7 +34,7 @@ static int ade7758_data_rdy_trigger_set_state(struct > iio_trigger *trig, > { > struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); > > - dev_dbg(&indio_dev->dev, "%s (%d)\n", __func__, state); > + dev_dbg(&indio_dev->dev, "(%d)\n", state); > return ade7758_set_irq(&indio_dev->dev, state); > } > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: Remove unnecessary cast on void pointer
On Wed, 7 Mar 2018 18:47:17 +0530 Arushi Singhal wrote: > The following Coccinelle script was used to detect this: > @r@ > expression x; > void* e; > type T; > identifier f; > @@ > ( > *((T *)e) > | > ((T *)x)[...] > | > ((T*)x)->f > | > > - (T*) > e > ) > > Signed-off-by: Arushi Singhal Applied to the togreg branch of iio.git and pushed out as testing.git for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/staging/iio/adc/ad7816.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/adc/ad7816.c > b/drivers/staging/iio/adc/ad7816.c > index bfe180a..bf76a86 100644 > --- a/drivers/staging/iio/adc/ad7816.c > +++ b/drivers/staging/iio/adc/ad7816.c > @@ -254,7 +254,7 @@ static const struct attribute_group > ad7816_attribute_group = { > static irqreturn_t ad7816_event_handler(int irq, void *private) > { > iio_push_event(private, IIO_EVENT_CODE_AD7816_OTI, > -iio_get_time_ns((struct iio_dev *)private)); > +iio_get_time_ns(private)); > return IRQ_HANDLED; > } > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: Remove VLA usage
On Wed, Mar 7, 2018 at 5:10 AM, Rasmus Villemoes wrote: > On 2018-03-07 06:46, Kees Cook wrote: >> The kernel would like to remove all VLA usage. This switches to a >> simple kasprintf() instead. >> >> Signed-off-by: Kees Cook >> --- >> drivers/staging/lustre/lustre/llite/xattr.c | 19 +-- >> 1 file changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c >> b/drivers/staging/lustre/lustre/llite/xattr.c >> index 532384c91447..aab4eab64289 100644 >> --- a/drivers/staging/lustre/lustre/llite/xattr.c >> +++ b/drivers/staging/lustre/lustre/llite/xattr.c >> @@ -87,7 +87,7 @@ ll_xattr_set_common(const struct xattr_handler *handler, >> const char *name, const void *value, size_t size, >> int flags) >> { >> - char fullname[strlen(handler->prefix) + strlen(name) + 1]; >> + char *fullname; >> struct ll_sb_info *sbi = ll_i2sbi(inode); >> struct ptlrpc_request *req = NULL; >> const char *pv = value; >> @@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler >> *handler, >> return -EPERM; >> } >> >> - sprintf(fullname, "%s%s\n", handler->prefix, name); > > It's probably worth pointing out that this actually fixes an > unconditional buffer overflow: fullname only has room for the two > strings and the '\n', but vsnprintf() is told that the buffer has > infinite size (well, INT_MAX), so there should be plenty of room to > append the '\0' after the '\n'. > >> + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name); >> + if (!fullname) >> + return -ENOMEM; >> rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode), >>valid, fullname, pv, size, 0, flags, >>ll_i2suppgid(inode), &req); >> + kfree(fullname); >> if (rc) { >> if (rc == -EOPNOTSUPP && handler->flags == XATTR_USER_T) { >> LCONSOLE_INFO("Disabling user_xattr feature because it >> is not supported on the server\n"); >> @@ -364,7 +367,7 @@ static int ll_xattr_get_common(const struct >> xattr_handler *handler, >> struct dentry *dentry, struct inode *inode, >> const char *name, void *buffer, size_t size) >> { >> - char fullname[strlen(handler->prefix) + strlen(name) + 1]; >> + char *fullname; >> struct ll_sb_info *sbi = ll_i2sbi(inode); >> #ifdef CONFIG_FS_POSIX_ACL >> struct ll_inode_info *lli = ll_i2info(inode); >> @@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct >> xattr_handler *handler, >> if (handler->flags == XATTR_ACL_DEFAULT_T && !S_ISDIR(inode->i_mode)) >> return -ENODATA; >> #endif >> - sprintf(fullname, "%s%s\n", handler->prefix, name); > > Same here. > > I'm a little surprised this hasn't been caugt by static analysis, I > thought gcc/coverity/smatch/whatnot had gotten pretty good at computing > the size of the output generated by a given format string with "known" > arguments and comparing to the size of the output buffer. Though of > course it does require the tool to be able to do symbolic manipulations, > in this case realizing that > > outsize == strlen(x)+strlen(y)+1+1 > bufsize == strlen(x)+strlen(y)+1 > > Rasmus Oh yes, hah. I didn't even see the \n in the string. :P So, both a VLA fix and a buffer over-run fix. Can I add your "Reviewed-by"? :) -Kees -- Kees Cook Pixel Security ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: Remove VLA usage
On 2018-03-07 06:46, Kees Cook wrote: > The kernel would like to remove all VLA usage. This switches to a > simple kasprintf() instead. > > Signed-off-by: Kees Cook > --- > drivers/staging/lustre/lustre/llite/xattr.c | 19 +-- > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/llite/xattr.c > b/drivers/staging/lustre/lustre/llite/xattr.c > index 532384c91447..aab4eab64289 100644 > --- a/drivers/staging/lustre/lustre/llite/xattr.c > +++ b/drivers/staging/lustre/lustre/llite/xattr.c > @@ -87,7 +87,7 @@ ll_xattr_set_common(const struct xattr_handler *handler, > const char *name, const void *value, size_t size, > int flags) > { > - char fullname[strlen(handler->prefix) + strlen(name) + 1]; > + char *fullname; > struct ll_sb_info *sbi = ll_i2sbi(inode); > struct ptlrpc_request *req = NULL; > const char *pv = value; > @@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler, > return -EPERM; > } > > - sprintf(fullname, "%s%s\n", handler->prefix, name); It's probably worth pointing out that this actually fixes an unconditional buffer overflow: fullname only has room for the two strings and the '\n', but vsnprintf() is told that the buffer has infinite size (well, INT_MAX), so there should be plenty of room to append the '\0' after the '\n'. > + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name); > + if (!fullname) > + return -ENOMEM; > rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode), >valid, fullname, pv, size, 0, flags, >ll_i2suppgid(inode), &req); > + kfree(fullname); > if (rc) { > if (rc == -EOPNOTSUPP && handler->flags == XATTR_USER_T) { > LCONSOLE_INFO("Disabling user_xattr feature because it > is not supported on the server\n"); > @@ -364,7 +367,7 @@ static int ll_xattr_get_common(const struct xattr_handler > *handler, > struct dentry *dentry, struct inode *inode, > const char *name, void *buffer, size_t size) > { > - char fullname[strlen(handler->prefix) + strlen(name) + 1]; > + char *fullname; > struct ll_sb_info *sbi = ll_i2sbi(inode); > #ifdef CONFIG_FS_POSIX_ACL > struct ll_inode_info *lli = ll_i2info(inode); > @@ -411,9 +414,13 @@ static int ll_xattr_get_common(const struct > xattr_handler *handler, > if (handler->flags == XATTR_ACL_DEFAULT_T && !S_ISDIR(inode->i_mode)) > return -ENODATA; > #endif > - sprintf(fullname, "%s%s\n", handler->prefix, name); Same here. I'm a little surprised this hasn't been caugt by static analysis, I thought gcc/coverity/smatch/whatnot had gotten pretty good at computing the size of the output generated by a given format string with "known" arguments and comparing to the size of the output buffer. Though of course it does require the tool to be able to do symbolic manipulations, in this case realizing that outsize == strlen(x)+strlen(y)+1+1 > bufsize == strlen(x)+strlen(y)+1 Rasmus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: Remove unnecessary cast on void pointer
The following Coccinelle script was used to detect this: @r@ expression x; void* e; type T; identifier f; @@ ( *((T *)e) | ((T *)x)[...] | ((T*)x)->f | - (T*) e ) Signed-off-by: Arushi Singhal --- drivers/staging/iio/adc/ad7816.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c index bfe180a..bf76a86 100644 --- a/drivers/staging/iio/adc/ad7816.c +++ b/drivers/staging/iio/adc/ad7816.c @@ -254,7 +254,7 @@ static const struct attribute_group ad7816_attribute_group = { static irqreturn_t ad7816_event_handler(int irq, void *private) { iio_push_event(private, IIO_EVENT_CODE_AD7816_OTI, - iio_get_time_ns((struct iio_dev *)private)); + iio_get_time_ns(private)); return IRQ_HANDLED; } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 6/6] PCI: hv: fix 2 hang issues in hv_compose_msi_msg()
On Tue, Mar 06, 2018 at 06:21:56PM +, Dexuan Cui wrote: > 1. With the patch "x86/vector/msi: Switch to global reservation mode" > (4900be8360), the recent v4.15 and newer kernels always hang for 1-vCPU > Hyper-V VM with SR-IOV. This is because when we reach hv_compose_msi_msg() > by request_irq() -> request_threaded_irq() -> __setup_irq()->irq_startup() > -> __irq_startup() -> irq_domain_activate_irq() -> ... -> > msi_domain_activate() -> ... -> hv_compose_msi_msg(), local irq is > disabled in __setup_irq(). > > Fix this by polling the channel. > > 2. If the host is ejecting the VF device before we reach > hv_compose_msi_msg(), in a UP VM, we can hang in hv_compose_msi_msg() > forever, because at this time the host doesn't respond to the > CREATE_INTERRUPT request. This issue also happens to old kernels like > v4.14, v4.13, etc. If you are fixing a problem you should report what commit you are fixing with a Fixes: tag and add a CC: sta...@vger.kernel.org to the commit log to send it to stable kernels to which it should be applied; mentioning kernel versions in the commit log is useless and should be omitted. Side note: you should not have sta...@vger.kernel.org in the email addresses CC list you are sending the patches to (you mark patches for stable by adding an appropriate CC tag in the commit log). Here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v4.16-rc4 Last but not least, most of the patches in this series do not justify sending them to stable kernels at all so you should remove the corresponding tag from the patches. Thanks, Lorenzo > Fix this by polling the channel for the PCI_EJECT message and > hpdev->state, and by checking the PCI vendor ID. > > Note: actually the above issues also happen to a SMP VM, if > "hbus->hdev->channel->target_cpu == smp_processor_id()" is true. > > Signed-off-by: Dexuan Cui > Tested-by: Adrian Suhov > Tested-by: Chris Valean > Cc: sta...@vger.kernel.org > Cc: Stephen Hemminger > Cc: K. Y. Srinivasan > Cc: Vitaly Kuznetsov > Cc: Jack Morgenstein > --- > drivers/pci/host/pci-hyperv.c | 58 > ++- > 1 file changed, 57 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 265ba11e53e2..50cdefe3f6d3 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -521,6 +521,8 @@ struct hv_pci_compl { > s32 completion_status; > }; > > +static void hv_pci_onchannelcallback(void *context); > + > /** > * hv_pci_generic_compl() - Invoked for a completion packet > * @context: Set up by the sender of the packet. > @@ -665,6 +667,31 @@ static void _hv_pcifront_read_config(struct hv_pci_dev > *hpdev, int where, > } > } > > +static u16 hv_pcifront_get_vendor_id(struct hv_pci_dev *hpdev) > +{ > + u16 ret; > + unsigned long flags; > + void __iomem *addr = hpdev->hbus->cfg_addr + CFG_PAGE_OFFSET + > + PCI_VENDOR_ID; > + > + spin_lock_irqsave(&hpdev->hbus->config_lock, flags); > + > + /* Choose the function to be read. (See comment above) */ > + writel(hpdev->desc.win_slot.slot, hpdev->hbus->cfg_addr); > + /* Make sure the function was chosen before we start reading. */ > + mb(); > + /* Read from that function's config space. */ > + ret = readw(addr); > + /* > + * mb() is not required here, because the spin_unlock_irqrestore() > + * is a barrier. > + */ > + > + spin_unlock_irqrestore(&hpdev->hbus->config_lock, flags); > + > + return ret; > +} > + > /** > * _hv_pcifront_write_config() - Internal PCI config write > * @hpdev: The PCI driver's representation of the device > @@ -1107,8 +1134,37 @@ static void hv_compose_msi_msg(struct irq_data *data, > struct msi_msg *msg) >* Since this function is called with IRQ locks held, can't >* do normal wait for completion; instead poll. >*/ > - while (!try_wait_for_completion(&comp.comp_pkt.host_event)) > + while (!try_wait_for_completion(&comp.comp_pkt.host_event)) { > + /* 0x means an invalid PCI VENDOR ID. */ > + if (hv_pcifront_get_vendor_id(hpdev) == 0x) { > + dev_err_once(&hbus->hdev->device, > + "the device has gone\n"); > + goto free_int_desc; > + } > + > + /* > + * When the higher level interrupt code calls us with > + * interrupt disabled, we must poll the channel by calling > + * the channel callback directly when channel->target_cpu is > + * the current CPU. When the higher level interrupt code > + * calls us with interrupt enabled, let's add the > + * local_bh_disable()/enable() to avoid race. > + */ > + local_bh_disable();
Re: [Outreachy kernel] Re: [PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.
On Wed, Mar 07, 2018 at 11:52:16AM +0100, Julia Lawall wrote: > > > On Wed, 7 Mar 2018, Dan Carpenter wrote: > > > On Wed, Mar 07, 2018 at 04:09:09PM +0530, Arushi Singhal wrote: > > > @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff > > > *skb, int hdr_len, void *priv) > > > keyidx = pos[3]; > > > if (!(keyidx & (1 << 5))) { > > > if (net_ratelimit()) { > > > - printk(KERN_DEBUG "CCMP: received packet without ExtIV > > > flag from %pM\n", > > > - hdr->addr2); > > > + netdev_dbg(skb->dev, "CCMP: received packet without > > > ExtIV flag from %pM\n", > > > +hdr->addr2); > > > } > > > > I'm sorry. I really hate to do this to you... But the right thing here > > is to use net_dbg_ratelimited() but get rid of the if (net_ratelimit()) > > check. So it looks like this: > > > > if (!(keyidx & (1 << 5))) { > > net_dbg_ratelimited(skb->dev, > > "CCMP: received packet without ExtIV flag > > from %pM\n", > > hdr->addr2); > > } > > > > Sorry again, when I looked at it before I just replied what my fast > > thinking lizard brain said instead of taking time to look at what I was > > saying. > > I don't think that net_dbg_ratelimited wants a skb->dev argument. The > various definitions in include/linux/net.h just have fmt as the first > argument. > Ah That's fine then. Don't worry about it. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] Re: [PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.
On Wed, 7 Mar 2018, Dan Carpenter wrote: > On Wed, Mar 07, 2018 at 04:09:09PM +0530, Arushi Singhal wrote: > > @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff > > *skb, int hdr_len, void *priv) > > keyidx = pos[3]; > > if (!(keyidx & (1 << 5))) { > > if (net_ratelimit()) { > > - printk(KERN_DEBUG "CCMP: received packet without ExtIV > > flag from %pM\n", > > - hdr->addr2); > > + netdev_dbg(skb->dev, "CCMP: received packet without > > ExtIV flag from %pM\n", > > + hdr->addr2); > > } > > I'm sorry. I really hate to do this to you... But the right thing here > is to use net_dbg_ratelimited() but get rid of the if (net_ratelimit()) > check. So it looks like this: > > if (!(keyidx & (1 << 5))) { > net_dbg_ratelimited(skb->dev, > "CCMP: received packet without ExtIV flag > from %pM\n", > hdr->addr2); > } > > Sorry again, when I looked at it before I just replied what my fast > thinking lizard brain said instead of taking time to look at what I was > saying. I don't think that net_dbg_ratelimited wants a skb->dev argument. The various definitions in include/linux/net.h just have fmt as the first argument. julia > > > regards, > dan carpenter > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/20180307104741.mdy2wa7hplfwxmb3%40mwanda. > For more options, visit https://groups.google.com/d/optout. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.
On Wed, Mar 07, 2018 at 04:09:09PM +0530, Arushi Singhal wrote: > @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, > int hdr_len, void *priv) > keyidx = pos[3]; > if (!(keyidx & (1 << 5))) { > if (net_ratelimit()) { > - printk(KERN_DEBUG "CCMP: received packet without ExtIV > flag from %pM\n", > - hdr->addr2); > + netdev_dbg(skb->dev, "CCMP: received packet without > ExtIV flag from %pM\n", > +hdr->addr2); > } I'm sorry. I really hate to do this to you... But the right thing here is to use net_dbg_ratelimited() but get rid of the if (net_ratelimit()) check. So it looks like this: if (!(keyidx & (1 << 5))) { net_dbg_ratelimited(skb->dev, "CCMP: received packet without ExtIV flag from %pM\n", hdr->addr2); } Sorry again, when I looked at it before I just replied what my fast thinking lizard brain said instead of taking time to look at what I was saying. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: rtl8192u: Replace printk() with more standardize output format.
printk() is the raw way to print output and should be avoided. For drivers with defined "struct device object", dev_*macro() is prefer and for "struct netdevice object", netdev_*macro() is prefer over dev_*macro() to standardize the output format within the subsystem. If no "struct device object" is defined prefer pr_*macro() over printk(). This patch Replace printk having a log level with the appropriate output format according to the order of preference. Signed-off-by: Arushi Singhal --- Changes in v3 *In version2 net_*macro_ratelimited was used which is not helping in standardizing the output format. Changes in v2 *change the subject line, driver name was wrong. .../rtl8192u/ieee80211/ieee80211_crypt_ccmp.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c index e6648f7..a4b4042 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c @@ -73,7 +73,7 @@ static void *ieee80211_ccmp_init(int key_idx) priv->tfm = (void *)crypto_alloc_cipher("aes", 0, CRYPTO_ALG_ASYNC); if (IS_ERR(priv->tfm)) { - printk(KERN_DEBUG "ieee80211_crypt_ccmp: could not allocate crypto API aes\n"); + pr_debug("ieee80211_crypt_ccmp: could not allocate crypto API aes\n"); priv->tfm = NULL; goto fail; } @@ -276,22 +276,22 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, int hdr_len, void *priv) keyidx = pos[3]; if (!(keyidx & (1 << 5))) { if (net_ratelimit()) { - printk(KERN_DEBUG "CCMP: received packet without ExtIV flag from %pM\n", - hdr->addr2); + netdev_dbg(skb->dev, "CCMP: received packet without ExtIV flag from %pM\n", + hdr->addr2); } key->dot11RSNAStatsCCMPFormatErrors++; return -2; } keyidx >>= 6; if (key->key_idx != keyidx) { - printk(KERN_DEBUG "CCMP: RX tkey->key_idx=%d frame keyidx=%d priv=%p\n", - key->key_idx, keyidx, priv); + netdev_dbg(skb->dev, "CCMP: RX tkey->key_idx=%d frame keyidx=%d priv=%p\n", + key->key_idx, keyidx, priv); return -6; } if (!key->key_set) { if (net_ratelimit()) { - printk(KERN_DEBUG "CCMP: received packet from %pM with keyid=%d that does not have a configured key\n", - hdr->addr2, keyidx); + netdev_dbg(skb->dev, "CCMP: received packet from %pM with keyid=%d that does not have a configured key\n", + hdr->addr2, keyidx); } return -3; } @@ -306,8 +306,8 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, int hdr_len, void *priv) if (memcmp(pn, key->rx_pn, CCMP_PN_LEN) <= 0) { if (net_ratelimit()) { - printk(KERN_DEBUG "CCMP: replay detected: STA=%pM previous PN %pm received PN %pm\n", - hdr->addr2, key->rx_pn, pn); + netdev_dbg(skb->dev, "CCMP: replay detected: STA=%pM previous PN %pm received PN %pm\n", + hdr->addr2, key->rx_pn, pn); } key->dot11RSNAStatsCCMPReplays++; return -4; @@ -341,8 +341,8 @@ static int ieee80211_ccmp_decrypt(struct sk_buff *skb, int hdr_len, void *priv) if (memcmp(mic, a, CCMP_MIC_LEN) != 0) { if (net_ratelimit()) { - printk(KERN_DEBUG "CCMP: decrypt failed: STA=%pM\n", - hdr->addr2); + netdev_dbg(skb->dev, "CCMP: decrypt failed: STA=%pM\n", + hdr->addr2); } key->dot11RSNAStatsCCMPDecryptErrors++; return -5; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: Remove VLA usage
On Tue, Mar 06, 2018 at 09:46:08PM -0800, Kees Cook wrote: > The kernel would like to remove all VLA usage. This switches to a > simple kasprintf() instead. > > Signed-off-by: Kees Cook > --- > drivers/staging/lustre/lustre/llite/xattr.c | 19 +-- > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/llite/xattr.c > b/drivers/staging/lustre/lustre/llite/xattr.c > index 532384c91447..aab4eab64289 100644 > --- a/drivers/staging/lustre/lustre/llite/xattr.c > +++ b/drivers/staging/lustre/lustre/llite/xattr.c > @@ -87,7 +87,7 @@ ll_xattr_set_common(const struct xattr_handler *handler, > const char *name, const void *value, size_t size, > int flags) > { > - char fullname[strlen(handler->prefix) + strlen(name) + 1]; > + char *fullname; > struct ll_sb_info *sbi = ll_i2sbi(inode); > struct ptlrpc_request *req = NULL; > const char *pv = value; > @@ -141,10 +141,13 @@ ll_xattr_set_common(const struct xattr_handler *handler, > return -EPERM; > } > > - sprintf(fullname, "%s%s\n", handler->prefix, name); > + fullname = kasprintf(GFP_KERNEL, "%s%s\n", handler->prefix, name); > + if (!fullname) > + return -ENOMEM; > rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode), >valid, fullname, pv, size, 0, flags, >ll_i2suppgid(inode), &req); > + kfree(fullname); This is cool. We've had kasprintf() since 2007, who knew?! thanks, Tobin. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: most: Remove unnecessary usage of BUG_ON().
On 07.03.2018 02:31, Quytelda Kahja wrote: There is no need for the calls to BUG_ON() in this driver, which are used to check if mbo or mbo->context are NULL; mbo is never NULL, and if mbo->context is NULL it would have already been dereferenced and oopsed before reaching the BUG_ON(). Signed-off-by: Quytelda Kahja Acked-by: Christian Gromm --- drivers/staging/most/core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c index 0ab2de5ecf18..3afc25a61643 100644 --- a/drivers/staging/most/core.c +++ b/drivers/staging/most/core.c @@ -915,7 +915,6 @@ static void arm_mbo(struct mbo *mbo) unsigned long flags; struct most_channel *c; - BUG_ON((!mbo) || (!mbo->context)); c = mbo->context; if (c->is_poisoned) { @@ -1018,8 +1017,6 @@ static void most_write_completion(struct mbo *mbo) { struct most_channel *c; - BUG_ON((!mbo) || (!mbo->context)); - c = mbo->context; if (mbo->status == MBO_E_INVAL) pr_info("WARN: Tx MBO status: invalid\n"); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel