[RFC bluetooth-next] mac802154: do not export ieee802154_rx()

[RFC bluetooth-next] mac802154: do not export ieee802154_rx()

[Varka Bhadram]

Right now there are no other users for ieee802154_rx()
in kernel. So lets remove EXPORT_SYMBOL() for this.

Signed-off-by: Varka Bhadram <varkabhadram@gmail.com>
---
 net/mac802154/rx.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index 5a258c1..7791c9b 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -290,7 +290,6 @@ void ieee802154_rx(struct ieee802154_hw *hw, struct sk_buff *skb)
 drop:
 	kfree_skb(skb);
 }
-EXPORT_SYMBOL(ieee802154_rx);
 
 void
 ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff *skb, u8 lqi)
-- 
1.7.9.5

Re: [RFC bluetooth-next] mac802154: do not export ieee802154_rx()

[Stefan Schmidt]

Hello.

On 29/06/15 11:31, Varka Bhadram wrote:
> Right now there are no other users for ieee802154_rx()
> in kernel. So lets remove EXPORT_SYMBOL() for this.
>
> Signed-off-by: Varka Bhadram <varkabhadram@gmail.com>
> ---
>   net/mac802154/rx.c |    1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
> index 5a258c1..7791c9b 100644
> --- a/net/mac802154/rx.c
> +++ b/net/mac802154/rx.c
> @@ -290,7 +290,6 @@ void ieee802154_rx(struct ieee802154_hw *hw, struct sk_buff *skb)
>   drop:
>   	kfree_skb(skb);
>   }
> -EXPORT_SYMBOL(ieee802154_rx);
>   
>   void
>   ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff *skb, u8 lqi)

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

Looks good to me. As long as we don't have any pending users this sounds 
like a good idea to not expose it until needed.

regards
Stefan Schmidt

Re: [RFC bluetooth-next] mac802154: do not export ieee802154_rx()

[Alexander Aring]

Hi,

On Mon, Jun 29, 2015 at 03:01:07PM +0530, Varka Bhadram wrote:
> Right now there are no other users for ieee802154_rx()
> in kernel. So lets remove EXPORT_SYMBOL() for this.
> 
> Signed-off-by: Varka Bhadram <varkabhadram@gmail.com>
> ---
>  net/mac802154/rx.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
> index 5a258c1..7791c9b 100644
> --- a/net/mac802154/rx.c
> +++ b/net/mac802154/rx.c
> @@ -290,7 +290,6 @@ void ieee802154_rx(struct ieee802154_hw *hw, struct sk_buff *skb)
>  drop:
>  	kfree_skb(skb);
>  }
> -EXPORT_SYMBOL(ieee802154_rx);
>  
>  void
>  ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff *skb, u8 lqi)

First I would ack that but please also move the function prototype from
mac802154.h to the internal header in mac802154 -> ieee802154_i.h.

Simple move [0] to [1]. Then we put it out of the drivers API.


And I have some global note about this function:

We have some use cases for drivers which, but they don't use this function.
These drivers are:

 - cc2520
 - mrf24j40

Also Alan notes this at [2] and I can confirm that the threaded irq
should be fine to use the _not_ irqsafe function. I think it slow down
the receive handling when it goes to first workqueue, then tasklet
again.

These drivers uses workqueues (or something other thing which is
preemptable and not irq context). In my opinion it should be possible
in these drivers to use ieee802154_rx instead ieee802154_rx_irqsafe.
(Didn't test it yet).


NOTE:
But even better would be that they don't use threaded/workqueues anymore
and use spi_async in the isr routines and use irqsafe then (in complete
handler of spi_async call).


The current behaviour isn't well yet, so driver which should use in
their current implementation can use ieee802154_rx but they don't use
it, but even better would be that these drivers use spi_async.


Another note would be the lqi paramater, currently this is garbage and
we should care about this paramater when we have some use case for
that. For now - leave the lqi parameter as it is. Means not in
ieee802154_rx and leave it in ieee802154_rx_irqsafe.

- Alex

[0] http://lxr.free-electrons.com/source/include/net/mac802154.h#L316
[1] http://lxr.free-electrons.com/source/net/mac802154/ieee802154_i.h
[2] http://lxr.free-electrons.com/source/drivers/net/ieee802154/mrf24j40.c#L563