Re: [PATCH v3] staging: kpc2000: Convert put_page to put_user_page*()
On 7/19/19 1:02 PM, Bharath Vedartham wrote:
> There have been issues with coordination of various subsystems using
> get_user_pages. These issues are better described in [1].
>
> An implementation of tracking get_user_pages is currently underway
> The implementation requires the use put_user_page*() variants to release
> a reference rather than put_page(). The commit that introduced
> put_user_pages, Commit fc1d8e7cca2daa18d2fe56b94874848adf89d7f5 ("mm:
> introduce
> put_user_page*(), placeholder version").
>
> The implementation currently simply calls put_page() within
> put_user_page(). But in the future, it is to change to add a mechanism
> to keep track of get_user_pages. Once a tracking mechanism is
> implemented, we can make attempts to work on improving on coordination
> between various subsystems using get_user_pages.
>
> [1] https://lwn.net/Articles/753027/
Optional: I've been fussing about how to keep the change log reasonable,
and finally came up with the following recommended template for these
conversion patches. This would replace the text you have above, because the
put_user_page placeholder commit has all the documentation (and then some)
that we need:
For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page().
This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").
For the change itself, you will need to rebase it onto the latest
linux.git, as it doesn't quite apply there.
Testing is good if we can get it, but as far as I can tell this is
correct, so you can also add:
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
>
> Cc: Ira Weiny
> Cc: John Hubbard
> Cc: Jérôme Glisse
> Cc: Greg Kroah-Hartman
> Cc: Matt Sickler
> Cc: de...@driverdev.osuosl.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux...@kvack.org
> Signed-off-by: Bharath Vedartham
> ---
> Changes since v1
> - Improved changelog by John's suggestion.
> - Moved logic to dirty pages below sg_dma_unmap
> and removed PageReserved check.
> Changes since v2
> - Added back PageResevered check as suggested by John Hubbard.
>
> The PageReserved check needs a closer look and is not worth messing
> around with for now.
>
> Matt, Could you give any suggestions for testing this patch?
>
> If in-case, you are willing to pick this up to test. Could you
> apply this patch to this tree
> https://github.com/johnhubbard/linux/tree/gup_dma_core
> and test it with your devices?
>
> ---
> drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
> b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 6166587..75ad263 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -198,9 +198,7 @@ int kpc_dma_transfer(struct dev_private_data *priv,
> struct kiocb *kcb, unsigned
> sg_free_table(>sgt);
> err_dma_map_sg:
> err_alloc_sg_table:
> - for (i = 0 ; i < acd->page_count ; i++){
> - put_page(acd->user_pages[i]);
> - }
> + put_user_pages(acd->user_pages, acd->page_count);
> err_get_user_pages:
> kfree(acd->user_pages);
> err_alloc_userpages:
> @@ -221,16 +219,13 @@ void transfer_complete_cb(struct aio_cb_data *acd,
> size_t xfr_count, u32 flags)
>
> dev_dbg(>ldev->pldev->dev, "transfer_complete_cb(acd = [%p])\n",
> acd);
>
> - for (i = 0 ; i < acd->page_count ; i++){
> - if (!PageReserved(acd->user_pages[i])){
> - set_page_dirty(acd->user_pages[i]);
> - }
> - }
> -
> dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
> acd->ldev->dir);
>
> - for (i = 0 ; i < acd->page_count ; i++){
> - put_page(acd->user_pages[i]);
> + for (i = 0; i < acd->page_count; i++) {
> + if (!PageReserved(acd->user_pages[i]))
> + put_user_pages_dirty(>user_pages[i], 1);
> + else
> + put_user_page(acd->user_pages[i]);
> }
>
> sg_free_table(>sgt);
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3] staging: kpc2000: Convert put_page to put_user_page*()
>From: Bharath Vedartham >Changes since v2 >- Added back PageResevered check as suggested by John Hubbard. > >The PageReserved check needs a closer look and is not worth messing >around with for now. > >Matt, Could you give any suggestions for testing this patch? Myself or someone else from Daktronics would have to do the testing since the hardware isn't really commercially available. I've been toying with the idea of asking for a volunteer from the mailing list to help me out with this - I'd send them some hardware and they'd do all the development and testing. :) I still have to run that idea by Management though. >If in-case, you are willing to pick this up to test. Could you >apply this patch to this tree and test it with your devices? I've been meaning to get to testing the changes to the drivers since upstreaming them, but I've been swamped with other development. I'm keeping an eye on the mailing lists, so I'm at least aware of what is coming down the pipe. I'm not too worried about this specific change, even though I don't really know if the reserved check and the dirtying are even necessary. It sounded like John's suggestion was to not do the PageReserved() check and just use put_user_pges_dirty() all the time. John, is that incorrect? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: kpc2000: Convert put_page to put_user_page*()
On 7/19/19 1:59 PM, Matt Sickler wrote: >> From: Bharath Vedartham >> Changes since v2 >>- Added back PageResevered check as suggested by John Hubbard. >> >> The PageReserved check needs a closer look and is not worth messing >> around with for now. >> >> Matt, Could you give any suggestions for testing this patch? > > Myself or someone else from Daktronics would have to do the testing since the > hardware isn't really commercially available. I've been toying with the idea > of asking for a volunteer from the mailing list to help me out with this - I'd > send them some hardware and they'd do all the development and testing. :) > I still have to run that idea by Management though. > >> If in-case, you are willing to pick this up to test. Could you >> apply this patch to this tree and test it with your devices? > > I've been meaning to get to testing the changes to the drivers since > upstreaming > them, but I've been swamped with other development. I'm keeping an eye on the > mailing lists, so I'm at least aware of what is coming down the pipe. > I'm not too worried about this specific change, even though I don't really > know > if the reserved check and the dirtying are even necessary. > It sounded like John's suggestion was to not do the PageReserved() check and > just > use put_user_pges_dirty() all the time. John, is that incorrect? > That's what I suggested at first. But then I saw at least one other place where this pattern is being used, and it shook my confidence. I don't clearly see what the PageReserved check is protecting against here, but it's better to be safe, and do things in two steps: step 1 is *only* convert from put_page() to put_user_page(), and step 2 is to maybe remove the PageReserved() check, once fully understood. thanks, -- John Hubbard NVIDIA ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: kpc2000: Convert put_page to put_user_page*()
There have been issues with coordination of various subsystems using
get_user_pages. These issues are better described in [1].
An implementation of tracking get_user_pages is currently underway
The implementation requires the use put_user_page*() variants to release
a reference rather than put_page(). The commit that introduced
put_user_pages, Commit fc1d8e7cca2daa18d2fe56b94874848adf89d7f5 ("mm: introduce
put_user_page*(), placeholder version").
The implementation currently simply calls put_page() within
put_user_page(). But in the future, it is to change to add a mechanism
to keep track of get_user_pages. Once a tracking mechanism is
implemented, we can make attempts to work on improving on coordination
between various subsystems using get_user_pages.
[1] https://lwn.net/Articles/753027/
Cc: Ira Weiny
Cc: John Hubbard
Cc: Jérôme Glisse
Cc: Greg Kroah-Hartman
Cc: Matt Sickler
Cc: de...@driverdev.osuosl.org
Cc: linux-ker...@vger.kernel.org
Cc: linux...@kvack.org
Signed-off-by: Bharath Vedartham
---
Changes since v1
- Improved changelog by John's suggestion.
- Moved logic to dirty pages below sg_dma_unmap
and removed PageReserved check.
Changes since v2
- Added back PageResevered check as suggested by John Hubbard.
The PageReserved check needs a closer look and is not worth messing
around with for now.
Matt, Could you give any suggestions for testing this patch?
If in-case, you are willing to pick this up to test. Could you
apply this patch to this tree
https://github.com/johnhubbard/linux/tree/gup_dma_core
and test it with your devices?
---
drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 6166587..75ad263 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -198,9 +198,7 @@ int kpc_dma_transfer(struct dev_private_data *priv, struct
kiocb *kcb, unsigned
sg_free_table(>sgt);
err_dma_map_sg:
err_alloc_sg_table:
- for (i = 0 ; i < acd->page_count ; i++){
- put_page(acd->user_pages[i]);
- }
+ put_user_pages(acd->user_pages, acd->page_count);
err_get_user_pages:
kfree(acd->user_pages);
err_alloc_userpages:
@@ -221,16 +219,13 @@ void transfer_complete_cb(struct aio_cb_data *acd,
size_t xfr_count, u32 flags)
dev_dbg(>ldev->pldev->dev, "transfer_complete_cb(acd = [%p])\n",
acd);
- for (i = 0 ; i < acd->page_count ; i++){
- if (!PageReserved(acd->user_pages[i])){
- set_page_dirty(acd->user_pages[i]);
- }
- }
-
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
acd->ldev->dir);
- for (i = 0 ; i < acd->page_count ; i++){
- put_page(acd->user_pages[i]);
+ for (i = 0; i < acd->page_count; i++) {
+ if (!PageReserved(acd->user_pages[i]))
+ put_user_pages_dirty(>user_pages[i], 1);
+ else
+ put_user_page(acd->user_pages[i]);
}
sg_free_table(>sgt);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
On Fri, Jul 19, 2019 at 05:01:33PM +0300, Dan Carpenter wrote:
> On Fri, Jul 19, 2019 at 12:05:07PM +, ajay.kat...@microchip.com wrote:
> >
> > On 7/19/2019 5:16 PM, Chuhong Yuan wrote:
> > >
> > > 于2019年7月19日周五 下午7:34写道：
> > >>
> > >> On 7/19/2019 1:40 PM, Chuhong Yuan wrote:
> > >>>
> > >>> Merge the combo use of memcpy and le32_to_cpus.
> > >>> Use get_unaligned_le32 instead.
> > >>> This simplifies the code.
> > >>>
> > >>> Signed-off-by: Chuhong Yuan
> > >>> ---
> > >>> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
> > >>> 1 file changed, 1 insertion(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > >>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > >>> index d72fdd333050..12fb4add05ec 100644
> > >>> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > >>> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > >>> @@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8
> > >>> *buff, u32 size)
> > >>> s32 freq;
> > >>> __le16 fc;
> > >>>
> > >>> - memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> > >>> - le32_to_cpus();
> > >>> + header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
> > >>> pkt_offset = GET_PKT_OFFSET(header);
> > >>>
> > >>> if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
> > >>>
> > >>
> > >> Thanks for sending the patches.
> > >>
> > >> The code change looks okay to me. Just a minor comment, avoid the use of
> > >> same subject line for different patches.
> > >
> > > These two patches are in the same subsystem and solve the same problem.
> > > I splitted them into two patches by mistake since I did not notice the
> > > problems
> > > in the second patch when I sent the first one.
> > > Should I merge the two patches and resend?
> > >
> >
> > Yes, please go ahead, merge the patches and send the updated version.
> >
>
> This is wrong advice. Don't merge the patches because they are for
> different drivers. The original subjects are fine because the
> subsystems are different so that's okay.
>
Oh... My bad... I was looking at the wrong patches. :P You are
100% correct Ajay. Merge the two patches and always make sure to not
send multiple patches with the same subject.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
On Fri, Jul 19, 2019 at 12:05:07PM +, ajay.kat...@microchip.com wrote:
>
> On 7/19/2019 5:16 PM, Chuhong Yuan wrote:
> >
> > 于2019年7月19日周五 下午7:34写道：
> >>
> >> On 7/19/2019 1:40 PM, Chuhong Yuan wrote:
> >>>
> >>> Merge the combo use of memcpy and le32_to_cpus.
> >>> Use get_unaligned_le32 instead.
> >>> This simplifies the code.
> >>>
> >>> Signed-off-by: Chuhong Yuan
> >>> ---
> >>> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
> >>> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> >>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> >>> index d72fdd333050..12fb4add05ec 100644
> >>> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> >>> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> >>> @@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8
> >>> *buff, u32 size)
> >>> s32 freq;
> >>> __le16 fc;
> >>>
> >>> - memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> >>> - le32_to_cpus();
> >>> + header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
> >>> pkt_offset = GET_PKT_OFFSET(header);
> >>>
> >>> if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
> >>>
> >>
> >> Thanks for sending the patches.
> >>
> >> The code change looks okay to me. Just a minor comment, avoid the use of
> >> same subject line for different patches.
> >
> > These two patches are in the same subsystem and solve the same problem.
> > I splitted them into two patches by mistake since I did not notice the
> > problems
> > in the second patch when I sent the first one.
> > Should I merge the two patches and resend?
> >
>
> Yes, please go ahead, merge the patches and send the updated version.
>
This is wrong advice. Don't merge the patches because they are for
different drivers. The original subjects are fine because the
subsystems are different so that's okay.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:debugfs_cleanup 21/55] drivers/gpu/drm/i915/i915_debugfs.c:4444:6: error: conflicting types for 'i915_debugfs_register'
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/driver-core.git debugfs_cleanup head: 88ff756ea1e770a5a0dab006b06b969c4cda7a24 commit: 9ec380374a43c5e6fcf1c6c1a429379b208a7905 [21/55] drm: make .debugfs_init and drm_debugfs_create_files() return void config: x86_64-rhel (attached as .config) compiler: gcc-7 (Debian 7.4.0-10) 7.4.0 reproduce: git checkout 9ec380374a43c5e6fcf1c6c1a429379b208a7905 # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): >> drivers/gpu/drm/i915/i915_debugfs.c::6: error: conflicting types for >> 'i915_debugfs_register' void i915_debugfs_register(struct drm_i915_private *dev_priv) ^ In file included from drivers/gpu/drm/i915/i915_debugfs.c:44:0: drivers/gpu/drm/i915/i915_debugfs.h:13:5: note: previous declaration of 'i915_debugfs_register' was here int i915_debugfs_register(struct drm_i915_private *dev_priv); ^ vim +/i915_debugfs_register + drivers/gpu/drm/i915/i915_debugfs.c --- 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 v2] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
Merge the combo use of memcpy and le32_to_cpus.
Use get_unaligned_le32 instead.
This simplifies the code.
Signed-off-by: Chuhong Yuan
---
Changes in v2:
- Merge the two patches with the same
subject line.
drivers/staging/wilc1000/wilc_mon.c | 3 +--
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
drivers/staging/wilc1000/wilc_wlan.c | 9 +++--
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_mon.c
b/drivers/staging/wilc1000/wilc_mon.c
index 7d7933d40924..d6f14f69ad64 100644
--- a/drivers/staging/wilc1000/wilc_mon.c
+++ b/drivers/staging/wilc1000/wilc_mon.c
@@ -35,8 +35,7 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8
*buff, u32 size)
return;
/* Get WILC header */
- memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
- le32_to_cpus();
+ header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
/*
* The packet offset field contain info about what type of management
* the frame we are dealing with and ack status
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index d72fdd333050..12fb4add05ec 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32
size)
s32 freq;
__le16 fc;
- memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
- le32_to_cpus();
+ header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
pkt_offset = GET_PKT_OFFSET(header);
if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
diff --git a/drivers/staging/wilc1000/wilc_wlan.c
b/drivers/staging/wilc1000/wilc_wlan.c
index d46876edcfeb..7d438ae90c3e 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -703,8 +703,7 @@ static void wilc_wlan_handle_rx_buff(struct wilc *wilc, u8
*buffer, int size)
do {
buff_ptr = buffer + offset;
- memcpy(, buff_ptr, 4);
- le32_to_cpus();
+ header = get_unaligned_le32(buff_ptr);
is_cfg_packet = (header >> 31) & 0x1;
pkt_offset = (header >> 22) & 0x1ff;
@@ -874,10 +873,8 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const
u8 *buffer,
offset = 0;
do {
- memcpy(, [offset], 4);
- memcpy(, [offset + 4], 4);
- le32_to_cpus();
- le32_to_cpus();
+ addr = get_unaligned_le32([offset]);
+ size = get_unaligned_le32([offset + 4]);
acquire_bus(wilc, WILC_BUS_ACQUIRE_ONLY);
offset += 8;
while (((int)size) && (offset < buffer_size)) {
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
On 7/19/2019 5:16 PM, Chuhong Yuan wrote:
>
> 于2019年7月19日周五 下午7:34写道：
>>
>> On 7/19/2019 1:40 PM, Chuhong Yuan wrote:
>>>
>>> Merge the combo use of memcpy and le32_to_cpus.
>>> Use get_unaligned_le32 instead.
>>> This simplifies the code.
>>>
>>> Signed-off-by: Chuhong Yuan
>>> ---
>>> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
>>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
>>> index d72fdd333050..12fb4add05ec 100644
>>> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
>>> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
>>> @@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff,
>>> u32 size)
>>> s32 freq;
>>> __le16 fc;
>>>
>>> - memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
>>> - le32_to_cpus();
>>> + header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
>>> pkt_offset = GET_PKT_OFFSET(header);
>>>
>>> if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
>>>
>>
>> Thanks for sending the patches.
>>
>> The code change looks okay to me. Just a minor comment, avoid the use of
>> same subject line for different patches.
>
> These two patches are in the same subsystem and solve the same problem.
> I splitted them into two patches by mistake since I did not notice the
> problems
> in the second patch when I sent the first one.
> Should I merge the two patches and resend?
>
Yes, please go ahead, merge the patches and send the updated version.
Regards,
Ajay
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
于2019年7月19日周五 下午7:34写道：
>
> On 7/19/2019 1:40 PM, Chuhong Yuan wrote:
> >
> > Merge the combo use of memcpy and le32_to_cpus.
> > Use get_unaligned_le32 instead.
> > This simplifies the code.
> >
> > Signed-off-by: Chuhong Yuan
> > ---
> > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > index d72fdd333050..12fb4add05ec 100644
> > --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > @@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff,
> > u32 size)
> > s32 freq;
> > __le16 fc;
> >
> > - memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> > - le32_to_cpus();
> > + header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
> > pkt_offset = GET_PKT_OFFSET(header);
> >
> > if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
> >
>
> Thanks for sending the patches.
>
> The code change looks okay to me. Just a minor comment, avoid the use of
> same subject line for different patches.
These two patches are in the same subsystem and solve the same problem.
I splitted them into two patches by mistake since I did not notice the problems
in the second patch when I sent the first one.
Should I merge the two patches and resend?
>
> Regards,
> Ajay
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
On 7/19/2019 1:40 PM, Chuhong Yuan wrote:
>
> Merge the combo use of memcpy and le32_to_cpus.
> Use get_unaligned_le32 instead.
> This simplifies the code.
>
> Signed-off-by: Chuhong Yuan
> ---
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index d72fdd333050..12fb4add05ec 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff,
> u32 size)
> s32 freq;
> __le16 fc;
>
> - memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> - le32_to_cpus();
> + header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
> pkt_offset = GET_PKT_OFFSET(header);
>
> if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
>
Thanks for sending the patches.
The code change looks okay to me. Just a minor comment, avoid the use of
same subject line for different patches.
Regards,
Ajay
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [OSSNA] Intro to kernel hacking tutorial
On Fri, Jul 05, 2019 at 12:50:55PM +1000, Tobin C. Harding wrote: > Outcome will (hopefully) be a small patch set into drivers/staging/. > (Don't worry Greg only one group got to this stage last time, you > won't get flooded with patches :) We're good at reviewing floods of patches. Send away. In the end what we want is people who will take over a driver and understand it completely and become the maintainer. We've had a few people that did appointed themselves to become the maintainer of a random driver and move it out of staging. But even if people don't make it all the way to become a maintainer, it's nice when they start down that path by focusing on one driver and trying to understand it as much as possible. Most of the time when you look at a new staging driver, then you do want to clean up the white space just because it's hard to look at non-standard code. So that's the first step. But then maybe start at the probe and release functions and clean it up. Keep your eyes open to any other mistakes or bugs you see. Write them down. Then the ioctls. Etc. Look at the TODO too. The other thing I wish people knew was about the relationship with maintainers. When you start out, you're virtually anonymous for the first couple patchsets. We get so many and they blend together so we don't remember your name. So don't think that we mean anything personally if we don't apply your patch. We have forgotten about the patch as soon as we reply to it. Don't panic and resend quickly. You will be too stressed. Wait until the next day. In staging we really want to apply patches (unless it's in staging because we're going to remove the code). I get annoyed with other staging reviewers who NAK patches because "I don't like churn" or whatever. On the other hand, patches just "silencing checkpatch.pl" is not a valid justification for sending a patch. Patches should make the code more readable. Anyway, maintainers are not monsters. Very few people have made me annoyed to the point where I refuse to review their code. And everyone else is in my good books so that's fine. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
Merge the combo use of memcpy and le32_to_cpus.
Use get_unaligned_le32 instead.
This simplifies the code.
Signed-off-by: Chuhong Yuan
---
drivers/staging/wilc1000/wilc_mon.c | 3 +--
drivers/staging/wilc1000/wilc_wlan.c | 9 +++--
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_mon.c
b/drivers/staging/wilc1000/wilc_mon.c
index 7d7933d40924..d6f14f69ad64 100644
--- a/drivers/staging/wilc1000/wilc_mon.c
+++ b/drivers/staging/wilc1000/wilc_mon.c
@@ -35,8 +35,7 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8
*buff, u32 size)
return;
/* Get WILC header */
- memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
- le32_to_cpus();
+ header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
/*
* The packet offset field contain info about what type of management
* the frame we are dealing with and ack status
diff --git a/drivers/staging/wilc1000/wilc_wlan.c
b/drivers/staging/wilc1000/wilc_wlan.c
index d46876edcfeb..7d438ae90c3e 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -703,8 +703,7 @@ static void wilc_wlan_handle_rx_buff(struct wilc *wilc, u8
*buffer, int size)
do {
buff_ptr = buffer + offset;
- memcpy(, buff_ptr, 4);
- le32_to_cpus();
+ header = get_unaligned_le32(buff_ptr);
is_cfg_packet = (header >> 31) & 0x1;
pkt_offset = (header >> 22) & 0x1ff;
@@ -874,10 +873,8 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const
u8 *buffer,
offset = 0;
do {
- memcpy(, [offset], 4);
- memcpy(, [offset + 4], 4);
- le32_to_cpus();
- le32_to_cpus();
+ addr = get_unaligned_le32([offset]);
+ size = get_unaligned_le32([offset + 4]);
acquire_bus(wilc, WILC_BUS_ACQUIRE_ONLY);
offset += 8;
while (((int)size) && (offset < buffer_size)) {
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
Merge the combo use of memcpy and le32_to_cpus.
Use get_unaligned_le32 instead.
This simplifies the code.
Signed-off-by: Chuhong Yuan
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index d72fdd333050..12fb4add05ec 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32
size)
s32 freq;
__le16 fc;
- memcpy(, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
- le32_to_cpus();
+ header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
pkt_offset = GET_PKT_OFFSET(header);
if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8712: Merge memcpy + be16_to_cpus to get_unaligned_be16
Merge the combo of memcpy and be16_to_cpus.
Use get_unaligned_be16 instead.
This simplifies the code.
Signed-off-by: Chuhong Yuan
---
drivers/staging/rtl8712/rtl871x_recv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_recv.c
b/drivers/staging/rtl8712/rtl871x_recv.c
index 5298fe603437..9969e5265a40 100644
--- a/drivers/staging/rtl8712/rtl871x_recv.c
+++ b/drivers/staging/rtl8712/rtl871x_recv.c
@@ -245,8 +245,7 @@ union recv_frame *r8712_portctrl(struct _adapter *adapter,
if (auth_alg == 2) {
/* get ether_type */
ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
- memcpy(_type, ptr, 2);
- be16_to_cpus(_type);
+ ether_type = get_unaligned_be16(ptr);
if ((psta != NULL) && (psta->ieee8021x_blocked)) {
/* blocked
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
