| Message ID | 20190719081005.4598-1-hslester96@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32 | expand |
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 <hslester96@gmail.com> > --- > 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(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET); > - le32_to_cpus(&header); > + 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
<Ajay.Kathat@microchip.com> 于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 <hslester96@gmail.com> > > --- > > 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(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET); > > - le32_to_cpus(&header); > > + 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
On 7/19/2019 5:16 PM, Chuhong Yuan wrote: > > <Ajay.Kathat@microchip.com> 于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 <hslester96@gmail.com> >>> --- >>> 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(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET); >>> - le32_to_cpus(&header); >>> + 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
On Fri, Jul 19, 2019 at 12:05:07PM +0000, Ajay.Kathat@microchip.com wrote: > > On 7/19/2019 5:16 PM, Chuhong Yuan wrote: > > > > <Ajay.Kathat@microchip.com> 于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 <hslester96@gmail.com> > >>> --- > >>> 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(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET); > >>> - le32_to_cpus(&header); > >>> + 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
On Fri, Jul 19, 2019 at 05:01:33PM +0300, Dan Carpenter wrote: > On Fri, Jul 19, 2019 at 12:05:07PM +0000, Ajay.Kathat@microchip.com wrote: > > > > On 7/19/2019 5:16 PM, Chuhong Yuan wrote: > > > > > > <Ajay.Kathat@microchip.com> 于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 <hslester96@gmail.com> > > >>> --- > > >>> 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(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET); > > >>> - le32_to_cpus(&header); > > >>> + 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
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(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET); - le32_to_cpus(&header); + header = get_unaligned_le32(buff - HOST_HDR_OFFSET); pkt_offset = GET_PKT_OFFSET(header); if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
Merge the combo use of memcpy and le32_to_cpus. Use get_unaligned_le32 instead. This simplifies the code. Signed-off-by: Chuhong Yuan <hslester96@gmail.com> --- drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)