Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Kalle,

On 06/22/2016 02:17 AM, Kalle Valo wrote:
> Javier Martinez Canillas  writes:
> 
 Patch 3/3 applies cleanly even after dropping patch 2/3.
 Is that Ok for you or do you want me to re-resend a v3
 with only patches 1/3 and 3/3?
>>>
>>> I can drop patch 2, no need to resend. Thanks.
>>>
>>
>> I saw that you sent your pull request for v4.8 but the
>> patches from this series were not included:
>>
>> https://lkml.org/lkml/2016/6/21/400
> 
> I had forgotten to change the state of patches 1 and 3 from 'Changes
> Requested' back to 'New' and that's why I missed them. I did that now so
> they are back in my queue:
> 
> https://patchwork.kernel.org/patch/9158855/
> https://patchwork.kernel.org/patch/9158851/
> 
> Thanks for checking.
> 

Ok, thanks for the information.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Kalle,

On 06/22/2016 02:17 AM, Kalle Valo wrote:
> Javier Martinez Canillas  writes:
> 
 Patch 3/3 applies cleanly even after dropping patch 2/3.
 Is that Ok for you or do you want me to re-resend a v3
 with only patches 1/3 and 3/3?
>>>
>>> I can drop patch 2, no need to resend. Thanks.
>>>
>>
>> I saw that you sent your pull request for v4.8 but the
>> patches from this series were not included:
>>
>> https://lkml.org/lkml/2016/6/21/400
> 
> I had forgotten to change the state of patches 1 and 3 from 'Changes
> Requested' back to 'New' and that's why I missed them. I did that now so
> they are back in my queue:
> 
> https://patchwork.kernel.org/patch/9158855/
> https://patchwork.kernel.org/patch/9158851/
> 
> Thanks for checking.
> 

Ok, thanks for the information.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Kalle Valo]

Javier Martinez Canillas  writes:

>>> Patch 3/3 applies cleanly even after dropping patch 2/3.
>>> Is that Ok for you or do you want me to re-resend a v3
>>> with only patches 1/3 and 3/3?
>> 
>> I can drop patch 2, no need to resend. Thanks.
>> 
>
> I saw that you sent your pull request for v4.8 but the
> patches from this series were not included:
>
> https://lkml.org/lkml/2016/6/21/400

I had forgotten to change the state of patches 1 and 3 from 'Changes
Requested' back to 'New' and that's why I missed them. I did that now so
they are back in my queue:

https://patchwork.kernel.org/patch/9158855/
https://patchwork.kernel.org/patch/9158851/

Thanks for checking.

-- 
Kalle Valo

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Kalle Valo]

Javier Martinez Canillas  writes:

>>> Patch 3/3 applies cleanly even after dropping patch 2/3.
>>> Is that Ok for you or do you want me to re-resend a v3
>>> with only patches 1/3 and 3/3?
>> 
>> I can drop patch 2, no need to resend. Thanks.
>> 
>
> I saw that you sent your pull request for v4.8 but the
> patches from this series were not included:
>
> https://lkml.org/lkml/2016/6/21/400

I had forgotten to change the state of patches 1 and 3 from 'Changes
Requested' back to 'New' and that's why I missed them. I did that now so
they are back in my queue:

https://patchwork.kernel.org/patch/9158855/
https://patchwork.kernel.org/patch/9158851/

Thanks for checking.

-- 
Kalle Valo

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Kalle,

On 06/10/2016 03:54 PM, Kalle Valo wrote:
> Javier Martinez Canillas  writes:
> 
>>> This patch (2/3) is only for code rearrangement and adds an
>>> unnecessary wrapper function. We can simply drop the patch.
>>
>> Agreed.
>>
>> Kalle,
>>
>> Patch 3/3 applies cleanly even after dropping patch 2/3.
>> Is that Ok for you or do you want me to re-resend a v3
>> with only patches 1/3 and 3/3?
> 
> I can drop patch 2, no need to resend. Thanks.
> 

I saw that you sent your pull request for v4.8 but the
patches from this series were not included:

https://lkml.org/lkml/2016/6/21/400

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Kalle,

On 06/10/2016 03:54 PM, Kalle Valo wrote:
> Javier Martinez Canillas  writes:
> 
>>> This patch (2/3) is only for code rearrangement and adds an
>>> unnecessary wrapper function. We can simply drop the patch.
>>
>> Agreed.
>>
>> Kalle,
>>
>> Patch 3/3 applies cleanly even after dropping patch 2/3.
>> Is that Ok for you or do you want me to re-resend a v3
>> with only patches 1/3 and 3/3?
> 
> I can drop patch 2, no need to resend. Thanks.
> 

I saw that you sent your pull request for v4.8 but the
patches from this series were not included:

https://lkml.org/lkml/2016/6/21/400

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Kalle Valo]

Javier Martinez Canillas  writes:

>> This patch (2/3) is only for code rearrangement and adds an
>> unnecessary wrapper function. We can simply drop the patch.
>
> Agreed.
>
> Kalle,
>
> Patch 3/3 applies cleanly even after dropping patch 2/3.
> Is that Ok for you or do you want me to re-resend a v3
> with only patches 1/3 and 3/3?

I can drop patch 2, no need to resend. Thanks.

-- 
Kalle Valo

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Kalle Valo]

Javier Martinez Canillas  writes:

>> This patch (2/3) is only for code rearrangement and adds an
>> unnecessary wrapper function. We can simply drop the patch.
>
> Agreed.
>
> Kalle,
>
> Patch 3/3 applies cleanly even after dropping patch 2/3.
> Is that Ok for you or do you want me to re-resend a v3
> with only patches 1/3 and 3/3?

I can drop patch 2, no need to resend. Thanks.

-- 
Kalle Valo

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Amitkumar,

On 06/10/2016 12:26 PM, Amitkumar Karwar wrote:
> Hi Kalle/Javier,
> 
>> From: Javier Martinez Canillas [mailto:jav...@osg.samsung.com]
>> Sent: Friday, June 10, 2016 8:07 PM
>> To: Kalle Valo
>> Cc: linux-kernel@vger.kernel.org; Julian Calaby; Shengzhen Li; Enric
>> Balletbo i Serra; Amitkumar Karwar; net...@vger.kernel.org; linux-
>> wirel...@vger.kernel.org; Nishant Sarmukadam
>> Subject: Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station
>> ioctl file
>>
>> Hello Kalle,
>>
>> On 06/10/2016 10:30 AM, Kalle Valo wrote:
>>> Javier Martinez Canillas <jav...@osg.samsung.com> writes:
>>>
>>>> From: Shengzhen Li <s...@marvell.com>
>>>>
>>>> Most cfg80211 operations are just a wrappers to functions defined in
>>>> the sta_ioctl.c file, so for consistency move the .get_tx_power logic
>> there.
>>>>
>>>> Signed-off-by: Shengzhen Li <s...@marvell.com>
>>>> Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
>>>> [javier: update the subject line and commit message]
>>>> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
>>>
>>> [...]
>>>
>>>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>>> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy
>> *wiphy,
>>>>  int *dbm)
>>>>  {
>>>>struct mwifiex_adapter *adapter =
>> mwifiex_cfg80211_get_adapter(wiphy);
>>>> -  struct mwifiex_private *priv = mwifiex_get_priv(adapter,
>>>> -  MWIFIEX_BSS_ROLE_ANY);
>>>> -  int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
>>>> - HostCmd_ACT_GEN_GET, 0, NULL, true);
>>>> -
>>>> -  if (ret < 0)
>>>> -  return ret;
>>>> -
>>>> -  /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler
>> */
>>>> -  *dbm = priv->tx_power_level;
>>>> +  struct mwifiex_private *priv;
>>>>
>>>> -  return 0;
>>>> +  priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>>>> +  return mwifiex_get_tx_power(priv, dbm);
>>>>  }
>>>
>>> So in patch 1 you added the patch and in patch 2 you move it to a
>>> different location? That doesn't make any sense, can't you just fold
>>> the two patches into one so that the function is added only once.
>>>
>>
>> I posted this patch in v1 but then Amitkumar shared his patch with me
>> that was very similar to mine, only that the logic was in a different
>> location.
>>
>> So I included his delta as a separate patch to try keeping attribution
>> as best as possible.
>>
> 
> This patch (2/3) is only for code rearrangement and adds an unnecessary 
> wrapper function. We can simply drop the patch.
>

Agreed.

Kalle,

Patch 3/3 applies cleanly even after dropping patch 2/3.
Is that Ok for you or do you want me to re-resend a v3
with only patches 1/3 and 3/3?

> Regards,
> Amitkumar
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Amitkumar,

On 06/10/2016 12:26 PM, Amitkumar Karwar wrote:
> Hi Kalle/Javier,
> 
>> From: Javier Martinez Canillas [mailto:jav...@osg.samsung.com]
>> Sent: Friday, June 10, 2016 8:07 PM
>> To: Kalle Valo
>> Cc: linux-kernel@vger.kernel.org; Julian Calaby; Shengzhen Li; Enric
>> Balletbo i Serra; Amitkumar Karwar; net...@vger.kernel.org; linux-
>> wirel...@vger.kernel.org; Nishant Sarmukadam
>> Subject: Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station
>> ioctl file
>>
>> Hello Kalle,
>>
>> On 06/10/2016 10:30 AM, Kalle Valo wrote:
>>> Javier Martinez Canillas  writes:
>>>
>>>> From: Shengzhen Li 
>>>>
>>>> Most cfg80211 operations are just a wrappers to functions defined in
>>>> the sta_ioctl.c file, so for consistency move the .get_tx_power logic
>> there.
>>>>
>>>> Signed-off-by: Shengzhen Li 
>>>> Signed-off-by: Amitkumar Karwar 
>>>> [javier: update the subject line and commit message]
>>>> Signed-off-by: Javier Martinez Canillas 
>>>
>>> [...]
>>>
>>>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>>> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy
>> *wiphy,
>>>>  int *dbm)
>>>>  {
>>>>struct mwifiex_adapter *adapter =
>> mwifiex_cfg80211_get_adapter(wiphy);
>>>> -  struct mwifiex_private *priv = mwifiex_get_priv(adapter,
>>>> -  MWIFIEX_BSS_ROLE_ANY);
>>>> -  int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
>>>> - HostCmd_ACT_GEN_GET, 0, NULL, true);
>>>> -
>>>> -  if (ret < 0)
>>>> -  return ret;
>>>> -
>>>> -  /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler
>> */
>>>> -  *dbm = priv->tx_power_level;
>>>> +  struct mwifiex_private *priv;
>>>>
>>>> -  return 0;
>>>> +  priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>>>> +  return mwifiex_get_tx_power(priv, dbm);
>>>>  }
>>>
>>> So in patch 1 you added the patch and in patch 2 you move it to a
>>> different location? That doesn't make any sense, can't you just fold
>>> the two patches into one so that the function is added only once.
>>>
>>
>> I posted this patch in v1 but then Amitkumar shared his patch with me
>> that was very similar to mine, only that the logic was in a different
>> location.
>>
>> So I included his delta as a separate patch to try keeping attribution
>> as best as possible.
>>
> 
> This patch (2/3) is only for code rearrangement and adds an unnecessary 
> wrapper function. We can simply drop the patch.
>

Agreed.

Kalle,

Patch 3/3 applies cleanly even after dropping patch 2/3.
Is that Ok for you or do you want me to re-resend a v3
with only patches 1/3 and 3/3?

> Regards,
> Amitkumar
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

RE: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Amitkumar Karwar]

Hi Kalle/Javier,

> From: Javier Martinez Canillas [mailto:jav...@osg.samsung.com]
> Sent: Friday, June 10, 2016 8:07 PM
> To: Kalle Valo
> Cc: linux-kernel@vger.kernel.org; Julian Calaby; Shengzhen Li; Enric
> Balletbo i Serra; Amitkumar Karwar; net...@vger.kernel.org; linux-
> wirel...@vger.kernel.org; Nishant Sarmukadam
> Subject: Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station
> ioctl file
> 
> Hello Kalle,
> 
> On 06/10/2016 10:30 AM, Kalle Valo wrote:
> > Javier Martinez Canillas <jav...@osg.samsung.com> writes:
> >
> >> From: Shengzhen Li <s...@marvell.com>
> >>
> >> Most cfg80211 operations are just a wrappers to functions defined in
> >> the sta_ioctl.c file, so for consistency move the .get_tx_power logic
> there.
> >>
> >> Signed-off-by: Shengzhen Li <s...@marvell.com>
> >> Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
> >> [javier: update the subject line and commit message]
> >> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
> >
> > [...]
> >
> >> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> >> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> >> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy
> *wiphy,
> >>  int *dbm)
> >>  {
> >>struct mwifiex_adapter *adapter =
> mwifiex_cfg80211_get_adapter(wiphy);
> >> -  struct mwifiex_private *priv = mwifiex_get_priv(adapter,
> >> -  MWIFIEX_BSS_ROLE_ANY);
> >> -  int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
> >> - HostCmd_ACT_GEN_GET, 0, NULL, true);
> >> -
> >> -  if (ret < 0)
> >> -  return ret;
> >> -
> >> -  /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler
> */
> >> -  *dbm = priv->tx_power_level;
> >> +  struct mwifiex_private *priv;
> >>
> >> -  return 0;
> >> +  priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
> >> +  return mwifiex_get_tx_power(priv, dbm);
> >>  }
> >
> > So in patch 1 you added the patch and in patch 2 you move it to a
> > different location? That doesn't make any sense, can't you just fold
> > the two patches into one so that the function is added only once.
> >
> 
> I posted this patch in v1 but then Amitkumar shared his patch with me
> that was very similar to mine, only that the logic was in a different
> location.
> 
> So I included his delta as a separate patch to try keeping attribution
> as best as possible.
> 

This patch (2/3) is only for code rearrangement and adds an unnecessary wrapper 
function. We can simply drop the patch.

Regards,
Amitkumar

RE: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Amitkumar Karwar]

Hi Kalle/Javier,

> From: Javier Martinez Canillas [mailto:jav...@osg.samsung.com]
> Sent: Friday, June 10, 2016 8:07 PM
> To: Kalle Valo
> Cc: linux-kernel@vger.kernel.org; Julian Calaby; Shengzhen Li; Enric
> Balletbo i Serra; Amitkumar Karwar; net...@vger.kernel.org; linux-
> wirel...@vger.kernel.org; Nishant Sarmukadam
> Subject: Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station
> ioctl file
> 
> Hello Kalle,
> 
> On 06/10/2016 10:30 AM, Kalle Valo wrote:
> > Javier Martinez Canillas  writes:
> >
> >> From: Shengzhen Li 
> >>
> >> Most cfg80211 operations are just a wrappers to functions defined in
> >> the sta_ioctl.c file, so for consistency move the .get_tx_power logic
> there.
> >>
> >> Signed-off-by: Shengzhen Li 
> >> Signed-off-by: Amitkumar Karwar 
> >> [javier: update the subject line and commit message]
> >> Signed-off-by: Javier Martinez Canillas 
> >
> > [...]
> >
> >> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> >> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> >> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy
> *wiphy,
> >>  int *dbm)
> >>  {
> >>struct mwifiex_adapter *adapter =
> mwifiex_cfg80211_get_adapter(wiphy);
> >> -  struct mwifiex_private *priv = mwifiex_get_priv(adapter,
> >> -  MWIFIEX_BSS_ROLE_ANY);
> >> -  int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
> >> - HostCmd_ACT_GEN_GET, 0, NULL, true);
> >> -
> >> -  if (ret < 0)
> >> -  return ret;
> >> -
> >> -  /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler
> */
> >> -  *dbm = priv->tx_power_level;
> >> +  struct mwifiex_private *priv;
> >>
> >> -  return 0;
> >> +  priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
> >> +  return mwifiex_get_tx_power(priv, dbm);
> >>  }
> >
> > So in patch 1 you added the patch and in patch 2 you move it to a
> > different location? That doesn't make any sense, can't you just fold
> > the two patches into one so that the function is added only once.
> >
> 
> I posted this patch in v1 but then Amitkumar shared his patch with me
> that was very similar to mine, only that the logic was in a different
> location.
> 
> So I included his delta as a separate patch to try keeping attribution
> as best as possible.
> 

This patch (2/3) is only for code rearrangement and adds an unnecessary wrapper 
function. We can simply drop the patch.

Regards,
Amitkumar

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Kalle,

On 06/10/2016 10:30 AM, Kalle Valo wrote:
> Javier Martinez Canillas  writes:
> 
>> From: Shengzhen Li 
>>
>> Most cfg80211 operations are just a wrappers to functions defined in the
>> sta_ioctl.c file, so for consistency move the .get_tx_power logic there.
>>
>> Signed-off-by: Shengzhen Li 
>> Signed-off-by: Amitkumar Karwar 
>> [javier: update the subject line and commit message]
>> Signed-off-by: Javier Martinez Canillas 
> 
> [...]
> 
>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
>>int *dbm)
>>  {
>>  struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
>> -struct mwifiex_private *priv = mwifiex_get_priv(adapter,
>> -MWIFIEX_BSS_ROLE_ANY);
>> -int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
>> -   HostCmd_ACT_GEN_GET, 0, NULL, true);
>> -
>> -if (ret < 0)
>> -return ret;
>> -
>> -/* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
>> -*dbm = priv->tx_power_level;
>> +struct mwifiex_private *priv;
>>  
>> -return 0;
>> +priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>> +return mwifiex_get_tx_power(priv, dbm);
>>  }
> 
> So in patch 1 you added the patch and in patch 2 you move it to a
> different location? That doesn't make any sense, can't you just fold the
> two patches into one so that the function is added only once.
>

I posted this patch in v1 but then Amitkumar shared his patch with me that
was very similar to mine, only that the logic was in a different location.

So I included his delta as a separate patch to try keeping attribution as
best as possible.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Javier Martinez Canillas]

Hello Kalle,

On 06/10/2016 10:30 AM, Kalle Valo wrote:
> Javier Martinez Canillas  writes:
> 
>> From: Shengzhen Li 
>>
>> Most cfg80211 operations are just a wrappers to functions defined in the
>> sta_ioctl.c file, so for consistency move the .get_tx_power logic there.
>>
>> Signed-off-by: Shengzhen Li 
>> Signed-off-by: Amitkumar Karwar 
>> [javier: update the subject line and commit message]
>> Signed-off-by: Javier Martinez Canillas 
> 
> [...]
> 
>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
>>int *dbm)
>>  {
>>  struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
>> -struct mwifiex_private *priv = mwifiex_get_priv(adapter,
>> -MWIFIEX_BSS_ROLE_ANY);
>> -int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
>> -   HostCmd_ACT_GEN_GET, 0, NULL, true);
>> -
>> -if (ret < 0)
>> -return ret;
>> -
>> -/* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
>> -*dbm = priv->tx_power_level;
>> +struct mwifiex_private *priv;
>>  
>> -return 0;
>> +priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>> +return mwifiex_get_tx_power(priv, dbm);
>>  }
> 
> So in patch 1 you added the patch and in patch 2 you move it to a
> different location? That doesn't make any sense, can't you just fold the
> two patches into one so that the function is added only once.
>

I posted this patch in v1 but then Amitkumar shared his patch with me that
was very similar to mine, only that the logic was in a different location.

So I included his delta as a separate patch to try keeping attribution as
best as possible.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Kalle Valo]

Javier Martinez Canillas  writes:

> From: Shengzhen Li 
>
> Most cfg80211 operations are just a wrappers to functions defined in the
> sta_ioctl.c file, so for consistency move the .get_tx_power logic there.
>
> Signed-off-by: Shengzhen Li 
> Signed-off-by: Amitkumar Karwar 
> [javier: update the subject line and commit message]
> Signed-off-by: Javier Martinez Canillas 

[...]

> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
> int *dbm)
>  {
>   struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
> - struct mwifiex_private *priv = mwifiex_get_priv(adapter,
> - MWIFIEX_BSS_ROLE_ANY);
> - int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
> -HostCmd_ACT_GEN_GET, 0, NULL, true);
> -
> - if (ret < 0)
> - return ret;
> -
> - /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
> - *dbm = priv->tx_power_level;
> + struct mwifiex_private *priv;
>  
> - return 0;
> + priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
> + return mwifiex_get_tx_power(priv, dbm);
>  }

So in patch 1 you added the patch and in patch 2 you move it to a
different location? That doesn't make any sense, can't you just fold the
two patches into one so that the function is added only once.

-- 
Kalle Valo

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Kalle Valo]

Javier Martinez Canillas  writes:

> From: Shengzhen Li 
>
> Most cfg80211 operations are just a wrappers to functions defined in the
> sta_ioctl.c file, so for consistency move the .get_tx_power logic there.
>
> Signed-off-by: Shengzhen Li 
> Signed-off-by: Amitkumar Karwar 
> [javier: update the subject line and commit message]
> Signed-off-by: Javier Martinez Canillas 

[...]

> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
> int *dbm)
>  {
>   struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
> - struct mwifiex_private *priv = mwifiex_get_priv(adapter,
> - MWIFIEX_BSS_ROLE_ANY);
> - int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
> -HostCmd_ACT_GEN_GET, 0, NULL, true);
> -
> - if (ret < 0)
> - return ret;
> -
> - /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
> - *dbm = priv->tx_power_level;
> + struct mwifiex_private *priv;
>  
> - return 0;
> + priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
> + return mwifiex_get_tx_power(priv, dbm);
>  }

So in patch 1 you added the patch and in patch 2 you move it to a
different location? That doesn't make any sense, can't you just fold the
two patches into one so that the function is added only once.

-- 
Kalle Valo

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Enric Balletbo Serra]

2016-06-06 19:02 GMT+02:00 Javier Martinez Canillas :
> From: Shengzhen Li 
>
> Most cfg80211 operations are just a wrappers to functions defined in the
> sta_ioctl.c file, so for consistency move the .get_tx_power logic there.
>
> Signed-off-by: Shengzhen Li 
> Signed-off-by: Amitkumar Karwar 
> [javier: update the subject line and commit message]
> Signed-off-by: Javier Martinez Canillas 
>
> ---
>
>  drivers/net/wireless/marvell/mwifiex/cfg80211.c  | 14 +++---
>  drivers/net/wireless/marvell/mwifiex/main.h  |  2 ++
>  drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 18 ++
>  3 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
> b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index b17f3d09a2c7..ff3f63ed95e1 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
>   int *dbm)
>  {
> struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
> -   struct mwifiex_private *priv = mwifiex_get_priv(adapter,
> -   MWIFIEX_BSS_ROLE_ANY);
> -   int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
> -  HostCmd_ACT_GEN_GET, 0, NULL, true);
> -
> -   if (ret < 0)
> -   return ret;
> -
> -   /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
> -   *dbm = priv->tx_power_level;
> +   struct mwifiex_private *priv;
>
> -   return 0;
> +   priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
> +   return mwifiex_get_tx_power(priv, dbm);
>  }
>
>  /*
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
> b/drivers/net/wireless/marvell/mwifiex/main.h
> index 0207af00be42..79c28cfb7780 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -1464,6 +1464,8 @@ int mwifiex_drv_get_driver_version(struct 
> mwifiex_adapter *adapter,
>  int mwifiex_set_tx_power(struct mwifiex_private *priv,
>  struct mwifiex_power_cfg *power_cfg);
>
> +int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm);
> +
>  int mwifiex_main_process(struct mwifiex_adapter *);
>
>  int mwifiex_queue_tx_pkt(struct mwifiex_private *priv, struct sk_buff *skb);
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c 
> b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> index 8e0862657122..70ff9b805b5b 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> @@ -775,6 +775,24 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
>  }
>
>  /*
> + * IOCTL request handler to get tx power configuration.
> + *
> + * This function prepares the correct firmware command and
> + * issues it.
> + */
> +int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm)
> +{
> +   int ret;
> +
> +   ret = mwifiex_send_cmd(priv, HostCmd_CMD_TXPWR_CFG,
> +  HostCmd_ACT_GEN_GET, 0, NULL, true);
> +
> +   *dbm = priv->tx_power_level;
> +
> +   return ret;
> +}
> +
> +/*
>   * IOCTL request handler to get power save mode.
>   *
>   * This function prepares the correct firmware command and
> --
> 2.5.5
>

Tested-by: Enric Balletbo i Serra 

Re: [PATCH v2 2/3] mwifiex: move .get_tx_power logic to station ioctl file

[Enric Balletbo Serra]

2016-06-06 19:02 GMT+02:00 Javier Martinez Canillas :
> From: Shengzhen Li 
>
> Most cfg80211 operations are just a wrappers to functions defined in the
> sta_ioctl.c file, so for consistency move the .get_tx_power logic there.
>
> Signed-off-by: Shengzhen Li 
> Signed-off-by: Amitkumar Karwar 
> [javier: update the subject line and commit message]
> Signed-off-by: Javier Martinez Canillas 
>
> ---
>
>  drivers/net/wireless/marvell/mwifiex/cfg80211.c  | 14 +++---
>  drivers/net/wireless/marvell/mwifiex/main.h  |  2 ++
>  drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 18 ++
>  3 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
> b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index b17f3d09a2c7..ff3f63ed95e1 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -385,18 +385,10 @@ mwifiex_cfg80211_get_tx_power(struct wiphy *wiphy,
>   int *dbm)
>  {
> struct mwifiex_adapter *adapter = mwifiex_cfg80211_get_adapter(wiphy);
> -   struct mwifiex_private *priv = mwifiex_get_priv(adapter,
> -   MWIFIEX_BSS_ROLE_ANY);
> -   int ret = mwifiex_send_cmd(priv, HostCmd_CMD_RF_TX_PWR,
> -  HostCmd_ACT_GEN_GET, 0, NULL, true);
> -
> -   if (ret < 0)
> -   return ret;
> -
> -   /* tx_power_level is set in HostCmd_CMD_RF_TX_PWR command handler */
> -   *dbm = priv->tx_power_level;
> +   struct mwifiex_private *priv;
>
> -   return 0;
> +   priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
> +   return mwifiex_get_tx_power(priv, dbm);
>  }
>
>  /*
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
> b/drivers/net/wireless/marvell/mwifiex/main.h
> index 0207af00be42..79c28cfb7780 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -1464,6 +1464,8 @@ int mwifiex_drv_get_driver_version(struct 
> mwifiex_adapter *adapter,
>  int mwifiex_set_tx_power(struct mwifiex_private *priv,
>  struct mwifiex_power_cfg *power_cfg);
>
> +int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm);
> +
>  int mwifiex_main_process(struct mwifiex_adapter *);
>
>  int mwifiex_queue_tx_pkt(struct mwifiex_private *priv, struct sk_buff *skb);
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c 
> b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> index 8e0862657122..70ff9b805b5b 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> @@ -775,6 +775,24 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
>  }
>
>  /*
> + * IOCTL request handler to get tx power configuration.
> + *
> + * This function prepares the correct firmware command and
> + * issues it.
> + */
> +int mwifiex_get_tx_power(struct mwifiex_private *priv, int *dbm)
> +{
> +   int ret;
> +
> +   ret = mwifiex_send_cmd(priv, HostCmd_CMD_TXPWR_CFG,
> +  HostCmd_ACT_GEN_GET, 0, NULL, true);
> +
> +   *dbm = priv->tx_power_level;
> +
> +   return ret;
> +}
> +
> +/*
>   * IOCTL request handler to get power save mode.
>   *
>   * This function prepares the correct firmware command and
> --
> 2.5.5
>

Tested-by: Enric Balletbo i Serra