+https://source.codeaurora.org/quic/la/platform/vendor/qcom-opensource/\
+dataservices/tree/rmnetctl
Don't split URL better to have long line.
diff --git a/drivers/net/rmnet/Kconfig b/drivers/net/rmnet/Kconfig
Since this is Qualcomm and Ethernet specific, maybe better to put
in drivers/net/ethernet/qualcom/rmnet
+config RMNET_DEBUG
Please use network device standard debug mechanism.
netif_msg_XXX
+ buffloc += snprintf(&buffer[buffloc], sizeof(buffer) - buffloc,
+ " %02x", skb->data[i]);
If you really have to do this. Use hex_dump_bytes API.
+ skb->mac_header = 0;
+ skb->mac_len = 0;
+}
Why not use sbk_set_mac_header(skb, 0)?
+static inline struct rmnet_phys_ep_conf_s *_rmnet_get_phys_ep_config
+ (struct net_device *dev)
awkward line break.
dev could be const
+ config = (struct rmnet_phys_ep_conf_s *)
+ rcu_dereference(dev->rx_handler_data);
Please don't directly reference rx_handler. There is already functions
like netdev_is_rx_handler_busy() to abstract that API.
+ * @epconfig: endpoint configuration structure to set
+ */
You are using docbook format here, but this is not a docbook comment.
ie.
/**
* function - This is a docbook comment
* @dev: this is a param
*/
Plus these are static functions so there is no point in documentating
internal API with docbook.
+ ASSERT_RTNL();
+
+ if (!dev || config_id < RMNET_LOCAL_LOGICAL_ENDPOINT ||
+ config_id >= RMNET_MAX_LOGICAL_EP)
+ return -EINVAL;
For internal API's you should validate parmeters at the external
entry point not in each call. Otherwise you have a multitude of
impossible error checks and dead code paths.
+ .next = 0,
+ .priority = 0
+};
Don't initialize fields that are not used or should be zero.
Hi Stephen
I'll make these changes.
Could just be:
static inline int _rmnet_is_physical_endpoint_associated(const struct
net_device *dev)
{
rx_handler_func_t *rx_handler
= rcu_dereference(dev->rx_handler);
return rx_handler == rmet_rx_handler;
}
But standard practice is to use ndo_ops to identify self in network
drivers.
I.e
return dev->netdev_ops == &rmnet_device_ops;
The netdevice which is associated is the physical (real_dev). This
device
can vary based on hardware and will have its own netdev_ops associated
with it.
rmnet_device_ops applies to the rmnet devices only. I'll use the first
option you have suggested.
+ if (!data[IFLA_RMNET_MUX_ID])
+ return -EINVAL;
So you are inventing private link netlink attributes.
Why? Why can't you use device switch, bridge, or other master/slave
model.
I would like to eventually add more configuration options for the
ingress & egress data formats as well as the endpoint configuration
(VND mode vs BRIDGE mode). I was able to re-use IFLA_VLAN_ID for
IFLA_RMNET_MUX_ID and test but it wasn't sufficient for the additional
parameters. I'll check the bridge attributes and try to reuse it.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
