* usbnet private-data accessor functions
@ 2018-10-12 9:16 Ben Dooks
2018-10-12 9:16 ` [PATCH 1/7] net: smsc75xx: add usbnet -> priv function Ben Dooks
` (7 more replies)
0 siblings, 8 replies; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn, steve.glendinning
I have been looking at the usbnet drivers and the possibility of some
code cleanups. One of the things I've found is that changing the way
the drivers use the private data with the usbnet structure is often
hand-coded each time is needed.
An easy cleanp (and making it easier in the future to change the
access method) would be to add an inline conversion function to
each driver so that it is done in one place.
Future work would be to look at the usbnet.data and see if it could
be changed (such as moving to a union, allocation of the data after
the usbnet structure, or similar).
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/7] net: smsc75xx: add usbnet -> priv function
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
@ 2018-10-12 9:16 ` Ben Dooks
2018-10-12 10:19 ` Greg KH
2018-10-12 9:16 ` [PATCH 2/7] net: asix: " Ben Dooks
` (6 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn,
steve.glendinning, Ben Dooks
There are a number of places in the smsc75xx driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
drivers/net/usb/smsc75xx.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 05553d252446..6d12fcd9b4b0 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -73,6 +73,11 @@ struct smsc75xx_priv {
u8 suspend_flags;
};
+static inline struct smsc75xx_priv *usbnet_to_smsc(struct usbnet *dev)
+{
+ return (struct smsc75xx_priv *)dev->data[0];
+}
+
struct usb_context {
struct usb_ctrlrequest req;
struct usbnet *dev;
@@ -469,7 +474,7 @@ static int smsc75xx_dataport_wait_not_busy(struct usbnet *dev)
static int smsc75xx_dataport_write(struct usbnet *dev, u32 ram_select, u32 addr,
u32 length, u32 *buf)
{
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 dp_sel;
int i, ret;
@@ -553,7 +558,7 @@ static void smsc75xx_deferred_multicast_write(struct work_struct *param)
static void smsc75xx_set_multicast(struct net_device *netdev)
{
struct usbnet *dev = netdev_priv(netdev);
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int i;
@@ -718,7 +723,7 @@ static void smsc75xx_ethtool_get_wol(struct net_device *net,
struct ethtool_wolinfo *wolinfo)
{
struct usbnet *dev = netdev_priv(net);
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
wolinfo->supported = SUPPORTED_WAKE;
wolinfo->wolopts = pdata->wolopts;
@@ -728,7 +733,7 @@ static int smsc75xx_ethtool_set_wol(struct net_device *net,
struct ethtool_wolinfo *wolinfo)
{
struct usbnet *dev = netdev_priv(net);
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
int ret;
pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
@@ -945,7 +950,7 @@ static int smsc75xx_set_features(struct net_device *netdev,
netdev_features_t features)
{
struct usbnet *dev = netdev_priv(netdev);
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int ret;
@@ -1051,7 +1056,7 @@ static int smsc75xx_phy_gig_workaround(struct usbnet *dev)
static int smsc75xx_reset(struct usbnet *dev)
{
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 buf;
int ret = 0, timeout;
@@ -1515,7 +1520,7 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
static void smsc75xx_unbind(struct usbnet *dev, struct usb_interface *intf)
{
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
if (pdata) {
netif_dbg(dev, ifdown, dev->net, "free pdata\n");
kfree(pdata);
@@ -1571,7 +1576,7 @@ static int smsc75xx_write_wuff(struct usbnet *dev, int filter, u32 wuf_cfg,
static int smsc75xx_enter_suspend0(struct usbnet *dev)
{
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
@@ -1597,7 +1602,7 @@ static int smsc75xx_enter_suspend0(struct usbnet *dev)
static int smsc75xx_enter_suspend1(struct usbnet *dev)
{
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
@@ -1633,7 +1638,7 @@ static int smsc75xx_enter_suspend1(struct usbnet *dev)
static int smsc75xx_enter_suspend2(struct usbnet *dev)
{
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
@@ -1659,7 +1664,7 @@ static int smsc75xx_enter_suspend2(struct usbnet *dev)
static int smsc75xx_enter_suspend3(struct usbnet *dev)
{
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val;
int ret;
@@ -1794,7 +1799,7 @@ static int smsc75xx_autosuspend(struct usbnet *dev, u32 link_up)
static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
{
struct usbnet *dev = usb_get_intfdata(intf);
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u32 val, link_up;
int ret;
@@ -2095,7 +2100,7 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
static int smsc75xx_resume(struct usb_interface *intf)
{
struct usbnet *dev = usb_get_intfdata(intf);
- struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+ struct smsc75xx_priv *pdata = usbnet_to_smsc(dev);
u8 suspend_flags = pdata->suspend_flags;
int ret;
u32 val;
--
2.19.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/7] net: asix: add usbnet -> priv function
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
2018-10-12 9:16 ` [PATCH 1/7] net: smsc75xx: add usbnet -> priv function Ben Dooks
@ 2018-10-12 9:16 ` Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 3/7] net: usb: asix88179_178a: " Ben Dooks
` (5 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn,
steve.glendinning, Ben Dooks
There are a number of places in the asix driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
drivers/net/usb/asix.h | 5 +++++
drivers/net/usb/asix_common.c | 4 ++--
drivers/net/usb/asix_devices.c | 16 ++++++++--------
drivers/net/usb/ax88172a.c | 2 +-
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 9a4171b90947..4bbb52669ac4 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -197,6 +197,11 @@ extern const struct driver_info ax88172a_info;
/* ASIX specific flags */
#define FLAG_EEPROM_MAC (1UL << 0) /* init device MAC from eeprom */
+static inline struct asix_data *usbnet_to_asix(struct usbnet *usb)
+{
+ return (struct asix_data *)&usb->data;
+}
+
int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data, int in_pm);
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index e95dd12edec4..1fd650faca5b 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -418,7 +418,7 @@ int asix_write_gpio(struct usbnet *dev, u16 value, int sleep, int in_pm)
void asix_set_multicast(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
u16 rx_ctl = AX_DEFAULT_RX_CTL;
if (net->flags & IFF_PROMISC) {
@@ -751,7 +751,7 @@ void asix_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
int asix_set_mac_address(struct net_device *net, void *p)
{
struct usbnet *dev = netdev_priv(net);
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
struct sockaddr *addr = p;
if (netif_running(net))
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index b654f05b2ccd..8e387f06dccf 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -144,7 +144,7 @@ static const struct ethtool_ops ax88172_ethtool_ops = {
static void ax88172_set_multicast(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
u8 rx_ctl = 0x8c;
if (net->flags & IFF_PROMISC) {
@@ -332,7 +332,7 @@ static int ax88772_link_reset(struct usbnet *dev)
static int ax88772_reset(struct usbnet *dev)
{
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
int ret;
/* Rewrite MAC address */
@@ -359,7 +359,7 @@ static int ax88772_reset(struct usbnet *dev)
static int ax88772_hw_reset(struct usbnet *dev, int in_pm)
{
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
int ret, embd_phy;
u16 rx_ctl;
@@ -454,7 +454,7 @@ static int ax88772_hw_reset(struct usbnet *dev, int in_pm)
static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
{
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
int ret, embd_phy;
u16 rx_ctl, phy14h, phy15h, phy16h;
u8 chipcode = 0;
@@ -795,7 +795,7 @@ static const struct ethtool_ops ax88178_ethtool_ops = {
static int marvell_phy_init(struct usbnet *dev)
{
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
u16 reg;
netdev_dbg(dev->net, "marvell_phy_init()\n");
@@ -827,7 +827,7 @@ static int marvell_phy_init(struct usbnet *dev)
static int rtl8211cl_phy_init(struct usbnet *dev)
{
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
netdev_dbg(dev->net, "rtl8211cl_phy_init()\n");
@@ -874,7 +874,7 @@ static int marvell_led_status(struct usbnet *dev, u16 speed)
static int ax88178_reset(struct usbnet *dev)
{
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
int ret;
__le16 eeprom;
u8 status;
@@ -962,7 +962,7 @@ static int ax88178_link_reset(struct usbnet *dev)
{
u16 mode;
struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
u32 speed;
netdev_dbg(dev->net, "ax88178_link_reset()\n");
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index 501576f53854..08f33c89f002 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -289,7 +289,7 @@ static void ax88172a_unbind(struct usbnet *dev, struct usb_interface *intf)
static int ax88172a_reset(struct usbnet *dev)
{
- struct asix_data *data = (struct asix_data *)&dev->data;
+ struct asix_data *data = usbnet_to_asix(dev);
struct ax88172a_private *priv = dev->driver_priv;
int ret;
u16 rx_ctl;
--
2.19.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/7] net: usb: asix88179_178a: add usbnet -> priv function
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
2018-10-12 9:16 ` [PATCH 1/7] net: smsc75xx: add usbnet -> priv function Ben Dooks
2018-10-12 9:16 ` [PATCH 2/7] net: asix: " Ben Dooks
@ 2018-10-12 9:16 ` Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 4/7] net: qmi_wwan: " Ben Dooks
` (4 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn,
steve.glendinning, Ben Dooks
There are a number of places in the asix88179_178a driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
drivers/net/usb/ax88179_178a.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 9e8ad372f419..f4b64e7e7706 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -196,6 +196,11 @@ static const struct {
{7, 0xcc, 0x4c, 0x18, 8},
};
+static inline struct ax88179_data *usbnet_to_ax(struct usbnet *usb)
+{
+ return (struct ax88179_data *)usb->data;
+}
+
static int __ax88179_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data, int in_pm)
{
@@ -678,7 +683,7 @@ ax88179_ethtool_set_eee(struct usbnet *dev, struct ethtool_eee *data)
static int ax88179_chk_eee(struct usbnet *dev)
{
struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
- struct ax88179_data *priv = (struct ax88179_data *)dev->data;
+ struct ax88179_data *priv = usbnet_to_ax(dev);
mii_ethtool_gset(&dev->mii, &ecmd);
@@ -781,7 +786,7 @@ static void ax88179_enable_eee(struct usbnet *dev)
static int ax88179_get_eee(struct net_device *net, struct ethtool_eee *edata)
{
struct usbnet *dev = netdev_priv(net);
- struct ax88179_data *priv = (struct ax88179_data *)dev->data;
+ struct ax88179_data *priv = usbnet_to_ax(dev);
edata->eee_enabled = priv->eee_enabled;
edata->eee_active = priv->eee_active;
@@ -792,7 +797,7 @@ static int ax88179_get_eee(struct net_device *net, struct ethtool_eee *edata)
static int ax88179_set_eee(struct net_device *net, struct ethtool_eee *edata)
{
struct usbnet *dev = netdev_priv(net);
- struct ax88179_data *priv = (struct ax88179_data *)dev->data;
+ struct ax88179_data *priv = usbnet_to_ax(dev);
int ret = -EOPNOTSUPP;
priv->eee_enabled = edata->eee_enabled;
@@ -841,7 +846,7 @@ static const struct ethtool_ops ax88179_ethtool_ops = {
static void ax88179_set_multicast(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- struct ax88179_data *data = (struct ax88179_data *)dev->data;
+ struct ax88179_data *data = usbnet_to_ax(dev);
u8 *m_filter = ((u8 *)dev->data) + 12;
data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB | AX_RX_CTL_IPE);
@@ -1228,7 +1233,7 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
u8 buf[5];
u16 *tmp16;
u8 *tmp;
- struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
+ struct ax88179_data *ax179_data = usbnet_to_ax(dev);
struct ethtool_eee eee_data;
usbnet_get_endpoints(dev, intf);
@@ -1458,7 +1463,7 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
static int ax88179_link_reset(struct usbnet *dev)
{
- struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
+ struct ax88179_data *ax179_data = usbnet_to_ax(dev);
u8 tmp[5], link_sts;
u16 mode, tmp16, delay = HZ / 10;
u32 tmp32 = 0x40000000;
@@ -1533,7 +1538,7 @@ static int ax88179_reset(struct usbnet *dev)
u8 buf[5];
u16 *tmp16;
u8 *tmp;
- struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
+ struct ax88179_data *ax179_data = usbnet_to_ax(dev);
struct ethtool_eee eee_data;
tmp16 = (u16 *)buf;
--
2.19.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
` (2 preceding siblings ...)
2018-10-12 9:16 ` [PATCH 3/7] net: usb: asix88179_178a: " Ben Dooks
@ 2018-10-12 9:16 ` Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 10:44 ` Bjørn Mork
2018-10-12 9:16 ` [PATCH 5/7] net: cdc_ether: " Ben Dooks
` (3 subsequent siblings)
7 siblings, 2 replies; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn,
steve.glendinning, Ben Dooks
There are a number of places in the qmi_wwan driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
drivers/net/usb/qmi_wwan.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 533b6fb8d923..45930758a945 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -76,6 +76,11 @@ struct qmimux_priv {
u8 mux_id;
};
+static inline struct qmi_wwan_state *usbnet_to_qmi(struct usbnet *usb)
+{
+ return (void *) &usb->data;
+}
+
static int qmimux_open(struct net_device *dev)
{
struct qmimux_priv *priv = netdev_priv(dev);
@@ -253,7 +258,7 @@ static void qmimux_unregister_device(struct net_device *dev)
static void qmi_wwan_netdev_setup(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
if (info->flags & QMI_WWAN_FLAG_RAWIP) {
net->header_ops = NULL; /* No header */
@@ -276,7 +281,7 @@ static void qmi_wwan_netdev_setup(struct net_device *net)
static ssize_t raw_ip_show(struct device *d, struct device_attribute *attr, char *buf)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
return sprintf(buf, "%c\n", info->flags & QMI_WWAN_FLAG_RAWIP ? 'Y' : 'N');
}
@@ -284,7 +289,7 @@ static ssize_t raw_ip_show(struct device *d, struct device_attribute *attr, char
static ssize_t raw_ip_store(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
bool enable;
int ret;
@@ -346,7 +351,7 @@ static ssize_t add_mux_show(struct device *d, struct device_attribute *attr, cha
static ssize_t add_mux_store(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
u8 mux_id;
int ret;
@@ -391,7 +396,7 @@ static ssize_t del_mux_show(struct device *d, struct device_attribute *attr, cha
static ssize_t del_mux_store(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
struct net_device *del_dev;
u8 mux_id;
int ret = 0;
@@ -468,7 +473,7 @@ static const u8 buggy_fw_addr[ETH_ALEN] = {0x00, 0xa0, 0xc6, 0x00, 0x00, 0x00};
*/
static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
bool rawip = info->flags & QMI_WWAN_FLAG_RAWIP;
__be16 proto;
@@ -555,7 +560,7 @@ static const struct net_device_ops qmi_wwan_netdev_ops = {
*/
static int qmi_wwan_manage_power(struct usbnet *dev, int on)
{
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
int rv;
dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__,
@@ -589,7 +594,7 @@ static int qmi_wwan_register_subdriver(struct usbnet *dev)
{
int rv;
struct usb_driver *subdriver = NULL;
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
/* collect bulk endpoints */
rv = usbnet_get_endpoints(dev, info->data);
@@ -651,7 +656,7 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
struct usb_cdc_union_desc *cdc_union;
struct usb_cdc_ether_desc *cdc_ether;
struct usb_driver *driver = driver_of(intf);
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
struct usb_cdc_parsed_header hdr;
BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) <
@@ -746,7 +751,7 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
{
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
struct usb_driver *driver = driver_of(intf);
struct usb_interface *other;
@@ -785,7 +790,7 @@ static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
{
struct usbnet *dev = usb_get_intfdata(intf);
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
int ret;
/* Both usbnet_suspend() and subdriver->suspend() MUST return 0
@@ -808,7 +813,7 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
static int qmi_wwan_resume(struct usb_interface *intf)
{
struct usbnet *dev = usb_get_intfdata(intf);
- struct qmi_wwan_state *info = (void *)&dev->data;
+ struct qmi_wwan_state *info = usbnet_to_qmi(dev);
int ret = 0;
bool callsub = (intf == info->control && info->subdriver &&
info->subdriver->resume);
@@ -1406,7 +1411,7 @@ static void qmi_wwan_disconnect(struct usb_interface *intf)
/* called twice if separate control and data intf */
if (!dev)
return;
- info = (void *)&dev->data;
+ info = usbnet_to_qmi(dev);
if (info->flags & QMI_WWAN_FLAG_MUX) {
if (!rtnl_trylock()) {
restart_syscall();
--
2.19.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/7] net: cdc_ether: add usbnet -> priv function
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
` (3 preceding siblings ...)
2018-10-12 9:16 ` [PATCH 4/7] net: qmi_wwan: " Ben Dooks
@ 2018-10-12 9:16 ` Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 6/7] net: huawei_cdc_ncm: " Ben Dooks
` (2 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn,
steve.glendinning, Ben Dooks
There are a number of places in the cdc drivers where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
drivers/net/usb/cdc_ether.c | 8 ++---
drivers/net/usb/cdc_mbim.c | 23 ++++++++------
drivers/net/usb/cdc_ncm.c | 61 +++++++++++++++++++-----------------
drivers/net/usb/rndis_host.c | 6 ++--
include/linux/usb/usbnet.h | 5 +++
5 files changed, 59 insertions(+), 44 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 5c42cf81a08b..7fee0ebc1943 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -77,7 +77,7 @@ static const u8 mbm_guid[16] = {
static void usbnet_cdc_update_filter(struct usbnet *dev)
{
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = usbnet_to_cdc(dev);
struct usb_interface *intf = info->control;
struct net_device *net = dev->net;
@@ -115,7 +115,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
u8 *buf = intf->cur_altsetting->extra;
int len = intf->cur_altsetting->extralen;
struct usb_interface_descriptor *d;
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = usbnet_to_cdc(dev);
int status;
int rndis;
bool android_rndis_quirk = false;
@@ -353,7 +353,7 @@ EXPORT_SYMBOL_GPL(usbnet_ether_cdc_bind);
void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
{
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = usbnet_to_cdc(dev);
struct usb_driver *driver = driver_of(intf);
/* combined interface - nothing to do */
@@ -438,7 +438,7 @@ EXPORT_SYMBOL_GPL(usbnet_cdc_status);
int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
{
int status;
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = usbnet_to_cdc(dev);
BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data)
< sizeof(struct cdc_state)));
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 0362acd5cdca..aec8f8eb21a7 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -36,6 +36,11 @@ struct cdc_mbim_state {
unsigned long flags;
};
+static inline struct cdc_mbim_state *usbnet_to_mbim(struct usbnet *usb)
+{
+ return (void *)&usb->data;
+}
+
/* flags for the cdc_mbim_state.flags field */
enum cdc_mbim_flags {
FLAG_IPS0_VLAN = 1 << 0, /* IP session 0 is tagged */
@@ -44,7 +49,7 @@ enum cdc_mbim_flags {
/* using a counter to merge subdriver requests with our own into a combined state */
static int cdc_mbim_manage_power(struct usbnet *dev, int on)
{
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
int rv = 0;
dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
@@ -73,7 +78,7 @@ static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
{
struct usbnet *dev = netdev_priv(netdev);
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
/* creation of this VLAN is a request to tag IP session 0 */
if (vid == MBIM_IPS0_VID)
@@ -87,7 +92,7 @@ static int cdc_mbim_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
static int cdc_mbim_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 vid)
{
struct usbnet *dev = netdev_priv(netdev);
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
/* this is a request for an untagged IP session 0 */
if (vid == MBIM_IPS0_VID)
@@ -144,7 +149,7 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
struct usb_driver *subdriver = ERR_PTR(-ENODEV);
int ret = -ENODEV;
u8 data_altsetting = 1;
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
/* should we change control altsetting on a NCM/MBIM function? */
if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
@@ -195,7 +200,7 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
static void cdc_mbim_unbind(struct usbnet *dev, struct usb_interface *intf)
{
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
struct cdc_ncm_ctx *ctx = info->ctx;
/* disconnect subdriver from control interface */
@@ -221,7 +226,7 @@ static bool is_ip_proto(__be16 proto)
static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
struct sk_buff *skb_out;
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
struct cdc_ncm_ctx *ctx = info->ctx;
__le32 sign = cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN);
u16 tci = 0;
@@ -411,7 +416,7 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_
static int cdc_mbim_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
{
struct sk_buff *skb;
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
struct cdc_ncm_ctx *ctx = info->ctx;
int len;
int nframes;
@@ -506,7 +511,7 @@ static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
{
int ret = -ENODEV;
struct usbnet *dev = usb_get_intfdata(intf);
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
struct cdc_ncm_ctx *ctx = info->ctx;
if (!ctx)
@@ -534,7 +539,7 @@ static int cdc_mbim_resume(struct usb_interface *intf)
{
int ret = 0;
struct usbnet *dev = usb_get_intfdata(intf);
- struct cdc_mbim_state *info = (void *)&dev->data;
+ struct cdc_mbim_state *info = usbnet_to_mbim(dev);
struct cdc_ncm_ctx *ctx = info->ctx;
bool callsub = (intf == ctx->control && info->subdriver && info->subdriver->resume);
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 1eaec648bd1f..0d722b326e1b 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -91,6 +91,11 @@ static const struct cdc_ncm_stats cdc_ncm_gstrings_stats[] = {
#define CDC_NCM_LOW_MEM_MAX_CNT 10
+static inline struct cdc_ncm_ctx *usbnet_to_ncm(struct usbnet *net)
+{
+ return (struct cdc_ncm_ctx *)net->data[0];
+}
+
static int cdc_ncm_get_sset_count(struct net_device __always_unused *netdev, int sset)
{
switch (sset) {
@@ -106,7 +111,7 @@ static void cdc_ncm_get_ethtool_stats(struct net_device *netdev,
u64 *data)
{
struct usbnet *dev = netdev_priv(netdev);
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
int i;
char *p = NULL;
@@ -148,7 +153,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
u32 val, max, min;
/* clamp new_rx to sane values */
@@ -171,7 +176,7 @@ static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx)
static u32 cdc_ncm_check_tx_max(struct usbnet *dev, u32 new_tx)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
u32 val, max, min;
/* clamp new_tx to sane values */
@@ -191,7 +196,7 @@ static u32 cdc_ncm_check_tx_max(struct usbnet *dev, u32 new_tx)
static ssize_t cdc_ncm_show_min_tx_pkt(struct device *d, struct device_attribute *attr, char *buf)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
return sprintf(buf, "%u\n", ctx->min_tx_pkt);
}
@@ -199,7 +204,7 @@ static ssize_t cdc_ncm_show_min_tx_pkt(struct device *d, struct device_attribute
static ssize_t cdc_ncm_show_rx_max(struct device *d, struct device_attribute *attr, char *buf)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
return sprintf(buf, "%u\n", ctx->rx_max);
}
@@ -207,7 +212,7 @@ static ssize_t cdc_ncm_show_rx_max(struct device *d, struct device_attribute *at
static ssize_t cdc_ncm_show_tx_max(struct device *d, struct device_attribute *attr, char *buf)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
return sprintf(buf, "%u\n", ctx->tx_max);
}
@@ -215,7 +220,7 @@ static ssize_t cdc_ncm_show_tx_max(struct device *d, struct device_attribute *at
static ssize_t cdc_ncm_show_tx_timer_usecs(struct device *d, struct device_attribute *attr, char *buf)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
return sprintf(buf, "%u\n", ctx->timer_interval / (u32)NSEC_PER_USEC);
}
@@ -223,7 +228,7 @@ static ssize_t cdc_ncm_show_tx_timer_usecs(struct device *d, struct device_attri
static ssize_t cdc_ncm_store_min_tx_pkt(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
unsigned long val;
/* no need to restrict values - anything from 0 to infinity is OK */
@@ -237,7 +242,7 @@ static ssize_t cdc_ncm_store_min_tx_pkt(struct device *d, struct device_attribu
static ssize_t cdc_ncm_store_rx_max(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
unsigned long val;
if (kstrtoul(buf, 0, &val) || cdc_ncm_check_rx_max(dev, val) != val)
@@ -250,7 +255,7 @@ static ssize_t cdc_ncm_store_rx_max(struct device *d, struct device_attribute *
static ssize_t cdc_ncm_store_tx_max(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
unsigned long val;
if (kstrtoul(buf, 0, &val) || cdc_ncm_check_tx_max(dev, val) != val)
@@ -263,7 +268,7 @@ static ssize_t cdc_ncm_store_tx_max(struct device *d, struct device_attribute *
static ssize_t cdc_ncm_store_tx_timer_usecs(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
ssize_t ret;
unsigned long val;
@@ -289,7 +294,7 @@ static DEVICE_ATTR(tx_timer_usecs, 0644, cdc_ncm_show_tx_timer_usecs, cdc_ncm_st
static ssize_t ndp_to_end_show(struct device *d, struct device_attribute *attr, char *buf)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
return sprintf(buf, "%c\n", ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END ? 'Y' : 'N');
}
@@ -297,7 +302,7 @@ static ssize_t ndp_to_end_show(struct device *d, struct device_attribute *attr,
static ssize_t ndp_to_end_store(struct device *d, struct device_attribute *attr, const char *buf, size_t len)
{
struct usbnet *dev = netdev_priv(to_net_dev(d));
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
bool enable;
if (strtobool(buf, &enable))
@@ -332,7 +337,7 @@ static DEVICE_ATTR_RW(ndp_to_end);
static ssize_t cdc_ncm_show_##name(struct device *d, struct device_attribute *attr, char *buf) \
{ \
struct usbnet *dev = netdev_priv(to_net_dev(d)); \
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; \
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev); \
return sprintf(buf, format "\n", tocpu(ctx->ncm_parm.name)); \
} \
static DEVICE_ATTR(name, 0444, cdc_ncm_show_##name, NULL)
@@ -375,7 +380,7 @@ static const struct attribute_group cdc_ncm_sysfs_attr_group = {
/* handle rx_max and tx_max changes */
static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
u32 val;
@@ -447,7 +452,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
/* helpers for NCM and MBIM differences */
static u8 cdc_ncm_flags(struct usbnet *dev)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
if (cdc_ncm_comm_intf_is_mbim(dev->intf->cur_altsetting) && ctx->mbim_desc)
return ctx->mbim_desc->bmNetworkCapabilities;
@@ -472,7 +477,7 @@ static u32 cdc_ncm_min_dgram_size(struct usbnet *dev)
static u32 cdc_ncm_max_dgram_size(struct usbnet *dev)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
if (cdc_ncm_comm_intf_is_mbim(dev->intf->cur_altsetting) && ctx->mbim_desc)
return le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
@@ -486,7 +491,7 @@ static u32 cdc_ncm_max_dgram_size(struct usbnet *dev)
*/
static int cdc_ncm_init(struct usbnet *dev)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
int err;
@@ -560,7 +565,7 @@ static int cdc_ncm_init(struct usbnet *dev)
/* set a new max datagram size */
static void cdc_ncm_set_dgram_size(struct usbnet *dev, int new_size)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
__le16 max_datagram_size;
u16 mbim_mtu;
@@ -608,7 +613,7 @@ static void cdc_ncm_set_dgram_size(struct usbnet *dev, int new_size)
static void cdc_ncm_fix_modulus(struct usbnet *dev)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
u32 val;
/*
@@ -652,7 +657,7 @@ static void cdc_ncm_fix_modulus(struct usbnet *dev)
static int cdc_ncm_setup(struct usbnet *dev)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
u32 def_rx, def_tx;
/* be conservative when selecting intial buffer size to
@@ -950,7 +955,7 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
if (ctx->data != ctx->control)
usb_driver_release_interface(driver, ctx->data);
error:
- cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
+ cdc_ncm_free(usbnet_to_ncm(dev));
dev->data[0] = 0;
dev_info(&intf->dev, "bind() failure\n");
return -ENODEV;
@@ -959,7 +964,7 @@ EXPORT_SYMBOL_GPL(cdc_ncm_bind_common);
void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
struct usb_driver *driver = driver_of(intf);
if (ctx == NULL)
@@ -1110,7 +1115,7 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_
struct sk_buff *
cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
struct usb_cdc_ncm_nth16 *nth16;
struct usb_cdc_ncm_ndp16 *ndp16;
struct sk_buff *skb_out;
@@ -1360,7 +1365,7 @@ static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
static void cdc_ncm_txpath_bh(unsigned long param)
{
struct usbnet *dev = (struct usbnet *)param;
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
spin_lock_bh(&ctx->mtx);
if (ctx->tx_timer_pending != 0) {
@@ -1382,7 +1387,7 @@ struct sk_buff *
cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
struct sk_buff *skb_out;
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
/*
* The Ethernet API we are using does not support transmitting
@@ -1495,7 +1500,7 @@ EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
{
struct sk_buff *skb;
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct cdc_ncm_ctx *ctx = usbnet_to_ncm(dev);
int len;
int nframes;
int x;
@@ -1604,7 +1609,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
struct cdc_ncm_ctx *ctx;
struct usb_cdc_notification *event;
- ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ ctx = usbnet_to_ncm(dev);
if (urb->actual_length < sizeof(*event))
return;
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index b807c91abe1d..a24af05a74fb 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -69,7 +69,7 @@ EXPORT_SYMBOL_GPL(rndis_status);
static void rndis_msg_indicate(struct usbnet *dev, struct rndis_indicate *msg,
int buflen)
{
- struct cdc_state *info = (void *)&dev->data;
+ struct cdc_state *info = usbnet_to_cdc(dev);
struct device *udev = &info->control->dev;
if (dev->driver_info->indication) {
@@ -102,7 +102,7 @@ static void rndis_msg_indicate(struct usbnet *dev, struct rndis_indicate *msg,
*/
int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen)
{
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = usbnet_to_cdc(dev);
struct usb_cdc_notification notification;
int master_ifnum;
int retval;
@@ -301,7 +301,7 @@ generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
{
int retval;
struct net_device *net = dev->net;
- struct cdc_state *info = (void *) &dev->data;
+ struct cdc_state *info = usbnet_to_cdc(dev);
union {
void *buf;
struct rndis_msg_hdr *header;
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index e2ec3582e549..cdb54dd3a4b4 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -207,6 +207,11 @@ struct cdc_state {
struct usb_interface *data;
};
+static inline struct cdc_state *usbnet_to_cdc(struct usbnet *net)
+{
+ return (void *) &net->data;
+}
+
extern int usbnet_generic_cdc_bind(struct usbnet *, struct usb_interface *);
extern int usbnet_ether_cdc_bind(struct usbnet *dev, struct usb_interface *intf);
extern int usbnet_cdc_bind(struct usbnet *, struct usb_interface *);
--
2.19.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/7] net: huawei_cdc_ncm: add usbnet -> priv function
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
` (4 preceding siblings ...)
2018-10-12 9:16 ` [PATCH 5/7] net: cdc_ether: " Ben Dooks
@ 2018-10-12 9:16 ` Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 7/7] net: usb: sr9800: " Ben Dooks
2018-10-12 15:38 ` usbnet private-data accessor functions Oliver Neukum
7 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn,
steve.glendinning, Ben Dooks
There are a number of places in the huawei driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
---
drivers/net/usb/huawei_cdc_ncm.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index 63f28908afda..d290b8c318be 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -38,9 +38,14 @@ struct huawei_cdc_ncm_state {
struct usb_interface *data;
};
+static inline struct huawei_cdc_ncm_state *usbnet_to_state(struct usbnet *usb)
+{
+ return (struct huawei_cdc_ncm_state *)&usb->data;
+}
+
static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
{
- struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
int rv;
if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) ||
@@ -72,7 +77,7 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
struct cdc_ncm_ctx *ctx;
struct usb_driver *subdriver = ERR_PTR(-ENODEV);
int ret = -ENODEV;
- struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
int drvflags = 0;
/* altsetting should always be 1 for NCM devices - so we hard-coded
@@ -119,7 +124,7 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev,
struct usb_interface *intf)
{
- struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
struct cdc_ncm_ctx *ctx = drvstate->ctx;
if (drvstate->subdriver && drvstate->subdriver->disconnect)
@@ -134,7 +139,7 @@ static int huawei_cdc_ncm_suspend(struct usb_interface *intf,
{
int ret = 0;
struct usbnet *usbnet_dev = usb_get_intfdata(intf);
- struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
struct cdc_ncm_ctx *ctx = drvstate->ctx;
if (ctx == NULL) {
@@ -161,7 +166,7 @@ static int huawei_cdc_ncm_resume(struct usb_interface *intf)
{
int ret = 0;
struct usbnet *usbnet_dev = usb_get_intfdata(intf);
- struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ struct huawei_cdc_ncm_state *drvstate = usbnet_to_state(usbnet_dev);
bool callsub;
struct cdc_ncm_ctx *ctx = drvstate->ctx;
--
2.19.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 7/7] net: usb: sr9800: add usbnet -> priv function
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
` (5 preceding siblings ...)
2018-10-12 9:16 ` [PATCH 6/7] net: huawei_cdc_ncm: " Ben Dooks
@ 2018-10-12 9:16 ` Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 15:38 ` usbnet private-data accessor functions Oliver Neukum
7 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 9:16 UTC (permalink / raw)
To: davem, netdev
Cc: linux-usb, linux-kernel, linux-kernel, gregkh, bjorn,
steve.glendinning, Ben Dooks
There are a number of places in the sr8900 driver where it gets the
private-data from the usbnet passed in. It would be sensible to have
one inline function to convert it and change all points in the driver
to use that.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
drivers/net/usb/sr9800.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/sr9800.c b/drivers/net/usb/sr9800.c
index 9277a0f228df..2093ecfff5a5 100644
--- a/drivers/net/usb/sr9800.c
+++ b/drivers/net/usb/sr9800.c
@@ -25,6 +25,11 @@
#include "sr9800.h"
+static inline struct sr_data *usbnet_to_sr(struct usbnet *usb)
+{
+ return (struct sr_data *)&usb->data;
+}
+
static int sr_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data)
{
@@ -296,7 +301,7 @@ static int sr_write_gpio(struct usbnet *dev, u16 value, int sleep)
static void sr_set_multicast(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- struct sr_data *data = (struct sr_data *)&dev->data;
+ struct sr_data *data = usbnet_to_sr(dev);
u16 rx_ctl = SR_DEFAULT_RX_CTL;
if (net->flags & IFF_PROMISC) {
@@ -436,7 +441,7 @@ sr_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
static int sr_get_eeprom_len(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- struct sr_data *data = (struct sr_data *)&dev->data;
+ struct sr_data *data = usbnet_to_sr(dev);
return data->eeprom_len;
}
@@ -493,7 +498,7 @@ static int sr_ioctl(struct net_device *net, struct ifreq *rq, int cmd)
static int sr_set_mac_address(struct net_device *net, void *p)
{
struct usbnet *dev = netdev_priv(net);
- struct sr_data *data = (struct sr_data *)&dev->data;
+ struct sr_data *data = usbnet_to_sr(dev);
struct sockaddr *addr = p;
if (netif_running(net))
@@ -595,7 +600,7 @@ static int sr9800_set_default_mode(struct usbnet *dev)
static int sr9800_reset(struct usbnet *dev)
{
- struct sr_data *data = (struct sr_data *)&dev->data;
+ struct sr_data *data = usbnet_to_sr(dev);
int ret, embd_phy;
u16 rx_ctl;
@@ -726,7 +731,7 @@ static int sr9800_phy_powerup(struct usbnet *dev)
static int sr9800_bind(struct usbnet *dev, struct usb_interface *intf)
{
- struct sr_data *data = (struct sr_data *)&dev->data;
+ struct sr_data *data = usbnet_to_sr(dev);
u16 led01_mux, led23_mux;
int ret, embd_phy;
u32 phyid;
--
2.19.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/7] net: smsc75xx: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 1/7] net: smsc75xx: add usbnet -> priv function Ben Dooks
@ 2018-10-12 10:19 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-10-12 10:19 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, bjorn,
steve.glendinning
On Fri, Oct 12, 2018 at 10:16:36AM +0100, Ben Dooks wrote:
> There are a number of places in the smsc75xx driver where it gets the
> private-data from the usbnet passed in. It would be sensible to have
> one inline function to convert it and change all points in the driver
> to use that.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 4/7] net: qmi_wwan: " Ben Dooks
@ 2018-10-12 10:20 ` Greg KH
2018-10-12 10:44 ` Bjørn Mork
1 sibling, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-10-12 10:20 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, bjorn,
steve.glendinning
On Fri, Oct 12, 2018 at 10:16:39AM +0100, Ben Dooks wrote:
> There are a number of places in the qmi_wwan driver where it gets the
> private-data from the usbnet passed in. It would be sensible to have
> one inline function to convert it and change all points in the driver
> to use that.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/7] net: asix: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 2/7] net: asix: " Ben Dooks
@ 2018-10-12 10:20 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-10-12 10:20 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, bjorn,
steve.glendinning
On Fri, Oct 12, 2018 at 10:16:37AM +0100, Ben Dooks wrote:
> There are a number of places in the asix driver where it gets the
> private-data from the usbnet passed in. It would be sensible to have
> one inline function to convert it and change all points in the driver
> to use that.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/7] net: usb: asix88179_178a: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 3/7] net: usb: asix88179_178a: " Ben Dooks
@ 2018-10-12 10:20 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-10-12 10:20 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, bjorn,
steve.glendinning
On Fri, Oct 12, 2018 at 10:16:38AM +0100, Ben Dooks wrote:
> There are a number of places in the asix88179_178a driver where it gets the
> private-data from the usbnet passed in. It would be sensible to have
> one inline function to convert it and change all points in the driver
> to use that.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] net: cdc_ether: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 5/7] net: cdc_ether: " Ben Dooks
@ 2018-10-12 10:20 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-10-12 10:20 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, bjorn,
steve.glendinning
On Fri, Oct 12, 2018 at 10:16:40AM +0100, Ben Dooks wrote:
> There are a number of places in the cdc drivers where it gets the
> private-data from the usbnet passed in. It would be sensible to have
> one inline function to convert it and change all points in the driver
> to use that.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] net: huawei_cdc_ncm: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 6/7] net: huawei_cdc_ncm: " Ben Dooks
@ 2018-10-12 10:20 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-10-12 10:20 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, bjorn,
steve.glendinning
On Fri, Oct 12, 2018 at 10:16:41AM +0100, Ben Dooks wrote:
> There are a number of places in the huawei driver where it gets the
> private-data from the usbnet passed in. It would be sensible to have
> one inline function to convert it and change all points in the driver
> to use that.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 7/7] net: usb: sr9800: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 7/7] net: usb: sr9800: " Ben Dooks
@ 2018-10-12 10:20 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-10-12 10:20 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, bjorn,
steve.glendinning
On Fri, Oct 12, 2018 at 10:16:42AM +0100, Ben Dooks wrote:
> There are a number of places in the sr8900 driver where it gets the
> private-data from the usbnet passed in. It would be sensible to have
> one inline function to convert it and change all points in the driver
> to use that.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
2018-10-12 9:16 ` [PATCH 4/7] net: qmi_wwan: " Ben Dooks
2018-10-12 10:20 ` Greg KH
@ 2018-10-12 10:44 ` Bjørn Mork
2018-10-12 13:22 ` Ben Dooks
1 sibling, 1 reply; 18+ messages in thread
From: Bjørn Mork @ 2018-10-12 10:44 UTC (permalink / raw)
To: Ben Dooks
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, gregkh,
steve.glendinning
Ben Dooks <ben.dooks@codethink.co.uk> writes:
> +static inline struct qmi_wwan_state *usbnet_to_qmi(struct usbnet *usb)
> +{
> + return (void *) &usb->data;
> +}
checkpatch doesn't like this, and it is also inconsistent with the
coding style elsewhere in the driver.
CHECK: No space is necessary after a cast
#30: FILE: drivers/net/usb/qmi_wwan.c:81:
+ return (void *) &usb->data;
total: 0 errors, 0 warnings, 1 checks, 115 lines checked
And as for consistency: I realize that "dev" is a very generic name,
but it's used consistendly for all struct usbnet pointers in the driver:
bjorn@miraculix:/usr/local/src/git/linux$ git grep 'struct usbnet' drivers/net/usb/qmi_wwan.c
drivers/net/usb/qmi_wwan.c:static struct net_device *qmimux_find_dev(struct usbnet *dev, u8 mux_id)
drivers/net/usb/qmi_wwan.c:static bool qmimux_has_slaves(struct usbnet *dev)
drivers/net/usb/qmi_wwan.c:static int qmimux_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(net);
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_manage_power(struct usbnet *dev, int on)
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_register_subdriver(struct usbnet *dev)
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_change_dtr(struct usbnet *dev, bool on)
drivers/net/usb/qmi_wwan.c:static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
drivers/net/usb/qmi_wwan.c: BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) <
drivers/net/usb/qmi_wwan.c:static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
The name was chosen to be consistent with the cdc_ether usage. I'd
prefer it if this function could use the "dev" name, to avoid confusing
things unnecessarly with yet another generic name like "usb". Which
isn't used anywhere else in the driver, and could just as easily refer
to a struct usb_device or struct usb_interface.
(yes, I know there are a couple of "struct usb_device *dev" cases. But
I'd rather see those renamed TBH)
I also don't see why this function should be qmi_wwan specific. No need
to duplicate the same function in every minidriver, when you can do with
two generic helpers (maybe with more meaningful names):
static inline void *usbnet_priv(const struct usbnet *dev)
{
return (void *)&dev->data;
}
static inline void *usbnet_priv0(const struct usbnet * *dev)
{
return (void *)dev->data[0];
}
Please fix the checkpatch warning and consider the other comments.
Personally I don't really think it makes the code any easier to read.
But if you want it, then I'm fine this. I guess I've grown too used to
seeing those casts ;-)
Bjørn
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
2018-10-12 10:44 ` Bjørn Mork
@ 2018-10-12 13:22 ` Ben Dooks
0 siblings, 0 replies; 18+ messages in thread
From: Ben Dooks @ 2018-10-12 13:22 UTC (permalink / raw)
To: Bjørn Mork
Cc: davem, netdev, linux-usb, linux-kernel, linux-kernel, gregkh,
steve.glendinning
On 12/10/18 11:44, Bjørn Mork wrote:
> Ben Dooks <ben.dooks@codethink.co.uk> writes:
>
>> +static inline struct qmi_wwan_state *usbnet_to_qmi(struct usbnet *usb)
>> +{
>> + return (void *) &usb->data;
>> +}
>
>
> checkpatch doesn't like this, and it is also inconsistent with the
> coding style elsewhere in the driver.
Thank you for pointing this out, will fix in v2.
> CHECK: No space is necessary after a cast
> #30: FILE: drivers/net/usb/qmi_wwan.c:81:
> + return (void *) &usb->data;
>
> total: 0 errors, 0 warnings, 1 checks, 115 lines checked
>
>
> And as for consistency: I realize that "dev" is a very generic name,
> but it's used consistendly for all struct usbnet pointers in the driver:
Ok, I'll change it.
>
> bjorn@miraculix:/usr/local/src/git/linux$ git grep 'struct usbnet' drivers/net/usb/qmi_wwan.c
> drivers/net/usb/qmi_wwan.c:static struct net_device *qmimux_find_dev(struct usbnet *dev, u8 mux_id)
> drivers/net/usb/qmi_wwan.c:static bool qmimux_has_slaves(struct usbnet *dev)
> drivers/net/usb/qmi_wwan.c:static int qmimux_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(net);
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = netdev_priv(to_net_dev(d));
> drivers/net/usb/qmi_wwan.c:static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> drivers/net/usb/qmi_wwan.c:static int qmi_wwan_manage_power(struct usbnet *dev, int on)
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
> drivers/net/usb/qmi_wwan.c:static int qmi_wwan_register_subdriver(struct usbnet *dev)
> drivers/net/usb/qmi_wwan.c:static int qmi_wwan_change_dtr(struct usbnet *dev, bool on)
> drivers/net/usb/qmi_wwan.c:static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
> drivers/net/usb/qmi_wwan.c: BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) <
> drivers/net/usb/qmi_wwan.c:static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
> drivers/net/usb/qmi_wwan.c: struct usbnet *dev = usb_get_intfdata(intf);
>
> The name was chosen to be consistent with the cdc_ether usage. I'd
> prefer it if this function could use the "dev" name, to avoid confusing
> things unnecessarly with yet another generic name like "usb". Which
> isn't used anywhere else in the driver, and could just as easily refer
> to a struct usb_device or struct usb_interface.
Ok, should be fairly easy to change this.
> (yes, I know there are a couple of "struct usb_device *dev" cases. But
> I'd rather see those renamed TBH)
I'd rather not touch that at the moment, this is already gaining complexity.
> I also don't see why this function should be qmi_wwan specific. No need
> to duplicate the same function in every minidriver, when you can do with
> two generic helpers (maybe with more meaningful names):
I personally prefer the return type to be specifically the private
data type for the driver. It makes it a bit easier to spot when
you don't get the cast right.
The functions below are the idea I am working towards for the
usbnet driver, I was trying to avoid doing everything in one
go. Making the accessor functions means the next round of changes
can be much smaller, touching 1-2 areas per driver.
> static inline void *usbnet_priv(const struct usbnet *dev)
> {
> return (void *)&dev->data;
> }
>
> static inline void *usbnet_priv0(const struct usbnet * *dev)
> {
> return (void *)dev->data[0];
> }
This is probably the end-game of the patch series, just need to
look at a couple of issues and see if there's anything better
than can be done.
I might also add a usbnet_set_privdata(dev, data) or something
like usbnet_alloc_privdata(dev, sizeof(data).
Note, there's also the fun here of the usbnet having private data
for itself, and these sub-drivers also having their own private
data... this probably needs to be named carefully.
> Please fix the checkpatch warning and consider the other comments.
>
> Personally I don't really think it makes the code any easier to read.
> But if you want it, then I'm fine this. I guess I've grown too used to
> seeing those casts ;-)
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: usbnet private-data accessor functions
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
` (6 preceding siblings ...)
2018-10-12 9:16 ` [PATCH 7/7] net: usb: sr9800: " Ben Dooks
@ 2018-10-12 15:38 ` Oliver Neukum
7 siblings, 0 replies; 18+ messages in thread
From: Oliver Neukum @ 2018-10-12 15:38 UTC (permalink / raw)
To: Ben Dooks, davem, netdev
Cc: gregkh, linux-kernel, bjorn, steve.glendinning, linux-kernel, linux-usb
On Fr, 2018-10-12 at 10:16 +0100, Ben Dooks wrote:
> I have been looking at the usbnet drivers and the possibility of some
> code cleanups. One of the things I've found is that changing the way
> the drivers use the private data with the usbnet structure is often
> hand-coded each time is needed.
Where is the improvement? You are just hiding how the data is passed.
It may look more pleasant to you, but that is a subjective impression.
I would suggest that if you want a nicely named way to get at private
data, add a field to the appropriate data structure.
Regards
Oliver
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-10-12 15:47 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12 9:16 usbnet private-data accessor functions Ben Dooks
2018-10-12 9:16 ` [PATCH 1/7] net: smsc75xx: add usbnet -> priv function Ben Dooks
2018-10-12 10:19 ` Greg KH
2018-10-12 9:16 ` [PATCH 2/7] net: asix: " Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 3/7] net: usb: asix88179_178a: " Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 4/7] net: qmi_wwan: " Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 10:44 ` Bjørn Mork
2018-10-12 13:22 ` Ben Dooks
2018-10-12 9:16 ` [PATCH 5/7] net: cdc_ether: " Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 6/7] net: huawei_cdc_ncm: " Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 9:16 ` [PATCH 7/7] net: usb: sr9800: " Ben Dooks
2018-10-12 10:20 ` Greg KH
2018-10-12 15:38 ` usbnet private-data accessor functions Oliver Neukum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).