staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
diff mbox series

Message ID 20190719081005.4598-1-hslester96@gmail.com
State Not Applicable
Delegated to: Johannes Berg
Headers show
Series
  • staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
Related show

Commit Message

Chuhong Yuan July 19, 2019, 8:10 a.m. UTC
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(-)

Comments

Ajay Singh July 19, 2019, 11:34 a.m. UTC | #1
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
Chuhong Yuan July 19, 2019, 11:46 a.m. UTC | #2
<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
Ajay Singh July 19, 2019, 12:05 p.m. UTC | #3
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
Dan Carpenter July 19, 2019, 2:01 p.m. UTC | #4
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
Dan Carpenter July 19, 2019, 2:04 p.m. UTC | #5
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

Patch
diff mbox series

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) {