URGENT RESPONSE NEEDED
Hello my dear. Did you receive my email message to you? Please, get back to me ASAP as the matter is becoming late. Expecting your urgent response. Sean.
RE: [PATCH v13 0/2] Add support for USB Type-C interface on latest NVIDIA GPU
Hi Heikki and Wolfram Do you have any comments on these changes? Regards, Ajay --nvpublic > Hi Heikki and Wolfram, > > These two changes add support for USB Type-C interface on latest NVIDIA GPU > card. > The Type-C controller used is Cypress CCGx and is over I2C interface. > > I2C host controller has known limitation of sending STOP after every read. > Since each read can be of 4 byte maximum length so there is a limit of 4 byte > read. > This is mentioned in adapter quirks as "max_read_len = 4" > > I2C host controller is mainly used for "write-then-read" or "write" messages > so added the flag I2C_AQ_COMB_WRITE_THEN_READ in adapter quirks. > > I think the patches should through usb tree because the main functionality is > usb Type-C. > > The changes have been reviewed by Andy Shevchenko and Heikki Krogerus. > Peter Rosin also helped review the patches and provided valuable comments. > Thanks to all reviewers. > > Thanks > Ajay > > Ajay Gupta (2): > i2c: buses: add i2c bus driver for NVIDIA GPU > usb: typec: ucsi: add support for Cypress CCGx > > Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ > MAINTAINERS | 7 + > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-nvidia-gpu.c | 369 > > drivers/usb/typec/ucsi/Kconfig | 10 + > drivers/usb/typec/ucsi/Makefile | 2 + > drivers/usb/typec/ucsi/ucsi_ccg.c | 305 ++ > 8 files changed, 721 insertions(+) > create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu > create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c > create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c > > -- > 2.7.4
[PATCH 4/8] usbnet: smsc95xx: check for csum being in last four bytes
The manual states that the checksum cannot lie in the last DWORD of the
transmission, so add a basic check for this and fall back to software
checksumming the packet.
This only seems to trigger for ACK packets with no options or data to
return to the other end, and the use of the tx-alignment option makes
it more likely to happen.
Signed-off-by: Ben Dooks
---
Fixes for v2:
- Fix spelling of check at Sergei's suggestion
- Move skb->len check into smsc95xx_can_tx_checksum()
- Change name of smsc95xx_can_checksum to explicitly say it is tx-csum
---
drivers/net/usb/smsc95xx.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 0c083d1b7f34..19e71fe15f6c 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2000,6 +2000,23 @@ static u32 smsc95xx_calc_csum_preamble(struct sk_buff
*skb)
return (high_16 << 16) | low_16;
}
+/* The TX CSUM won't work if the checksum lies in the last 4 bytes of the
+ * transmission. This is fairly unlikely, only seems to trigger with some
+ * short TCP ACK packets sent.
+ *
+ * Note, this calculation should probably check for the alignment of the
+ * data as well, but a straight check for csum being in the last four bytes
+ * of the packet should be ok for now.
+*/
+static bool smsc95xx_can_tx_checksum(struct sk_buff *skb)
+{
+ unsigned int len = skb->len - skb_checksum_start_offset(skb);
+
+ if (skb->len <= 45)
+ return false;
+ return skb->csum_offset < (len - (4 + 1));
+}
+
static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
struct sk_buff *skb, gfp_t flags)
{
@@ -2024,7 +2041,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet
*dev,
tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
if (csum) {
- if (skb->len <= 45) {
+ if (!smsc95xx_can_tx_checksum(skb)) {
/* workaround - hardware tx checksum does not work
* properly with extremely small packets */
long csstart = skb_checksum_start_offset(skb);
--
2.19.1
[PATCH 1/8] usbnet: smsc95xx: fix rx packet alignment
The smsc95xx driver already takes into account the NET_IP_ALIGN parameter when setting up the receive packet data, which means we do not need to worry about aligning the packets in the usbnet driver. Adding the EVENT_NO_IP_ALIGN means that the IPv4 header is now passed to the ip_rcv() routine with the start on an aligned address. Tested on Raspberry Pi B3. Signed-off-by: Ben Dooks --- drivers/net/usb/smsc95xx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 06b4d290784d..401ec9feb495 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -1292,6 +1292,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf) dev->net->features |= NETIF_F_RXCSUM; dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM; + set_bit(EVENT_NO_IP_ALIGN, &dev->flags); smsc95xx_init_mac_address(dev); -- 2.19.1
[PATCH 7/8] usbnet: smsc95xx: add usbnet -> priv function
There are a number of places in the smsc95xx 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
---
drivers/net/usb/smsc95xx.c | 47 +-
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 03c3c02b569c..1eb0795ec90f 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -78,6 +78,11 @@ struct smsc95xx_priv {
struct usbnet *dev;
};
+static inline struct smsc95xx_priv *usbnet_to_smsc(struct usbnet *dev)
+{
+ return (struct smsc95xx_priv *)dev->data[0];
+}
+
static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
module_param(turbo_mode, bool, 0644);
MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -467,7 +472,7 @@ static unsigned int smsc95xx_hash(char addr[ETH_ALEN])
static void smsc95xx_set_multicast(struct net_device *netdev)
{
struct usbnet *dev = netdev_priv(netdev);
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int ret;
@@ -562,7 +567,7 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet
*dev, u8 duplex,
static int smsc95xx_link_reset(struct usbnet *dev)
{
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
struct mii_if_info *mii = &dev->mii;
struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
unsigned long flags;
@@ -630,7 +635,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb
*urb)
static void set_carrier(struct usbnet *dev, bool link)
{
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
if (pdata->link_ok == link)
return;
@@ -759,7 +764,7 @@ static void smsc95xx_ethtool_get_wol(struct net_device *net,
struct ethtool_wolinfo *wolinfo)
{
struct usbnet *dev = netdev_priv(net);
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
wolinfo->supported = SUPPORTED_WAKE;
wolinfo->wolopts = pdata->wolopts;
@@ -769,7 +774,7 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
struct ethtool_wolinfo *wolinfo)
{
struct usbnet *dev = netdev_priv(net);
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int ret;
pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
@@ -805,7 +810,7 @@ static int get_mdix_status(struct net_device *net)
static void set_mdix_status(struct net_device *net, __u8 mdix_ctrl)
{
struct usbnet *dev = netdev_priv(net);
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int buf;
if ((pdata->chip_id == ID_REV_CHIP_ID_9500A_) ||
@@ -854,7 +859,7 @@ static int smsc95xx_get_link_ksettings(struct net_device
*net,
struct ethtool_link_ksettings *cmd)
{
struct usbnet *dev = netdev_priv(net);
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int retval;
retval = usbnet_get_link_ksettings(net, cmd);
@@ -869,7 +874,7 @@ static int smsc95xx_set_link_ksettings(struct net_device
*net,
const struct ethtool_link_ksettings *cmd)
{
struct usbnet *dev = netdev_priv(net);
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
int retval;
if (pdata->mdix_ctrl != cmd->base.eth_tp_mdix_ctrl)
@@ -951,7 +956,7 @@ static int smsc95xx_set_mac_address(struct usbnet *dev)
/* starts the TX path */
static int smsc95xx_start_tx_path(struct usbnet *dev)
{
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
int ret;
@@ -971,7 +976,7 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
/* Starts the Receive path */
static int smsc95xx_start_rx_path(struct usbnet *dev, int in_pm)
{
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
unsigned long flags;
spin_lock_irqsave(&pdata->mac_cr_lock, flags);
@@ -1028,7 +1033,7 @@ static int smsc95xx_phy_initialize(struct us
[PATCH 2/8] usbnet: smsc95xx: add kconfig for turbo mode
Add a configuration option for the default state of turbo mode
on the smsc95xx networking driver. Some systems it is better
to default this to off as it causes significant increases in
soft-irq load.
Signed-off-by: Ben Dooks
---
drivers/net/usb/Kconfig| 13 +
drivers/net/usb/smsc95xx.c | 2 +-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 418b0904cecb..c3ebc43a6582 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -351,6 +351,19 @@ config USB_NET_SMSC95XX
This option adds support for SMSC LAN95XX based USB 2.0
10/100 Ethernet adapters.
+config USB_NET_SMSC95XX_TURBO
+ bool "Use turbo receive mode by default"
+ depends on USB_NET_SMSC95XX
+ default y
+ help
+ This options sets the default turbo mode settings for the
+ driver's receive path. These can also be altered by the
+ turbo_mode module parameter.
+
+ There are some systems where the turbo mode causes higher
+ soft-irq load, thus making it useful to default the option
+ off for these.
+
config USB_NET_GL620A
tristate "GeneSys GL620USB-A based cables"
depends on USB_USBNET
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 401ec9feb495..cb19aea139d3 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -78,7 +78,7 @@ struct smsc95xx_priv {
struct usbnet *dev;
};
-static bool turbo_mode = true;
+static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
module_param(turbo_mode, bool, 0644);
MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
--
2.19.1
[PATCH 5/8] usbnet: smsc95xx: align tx-buffer to word
The tegra EHCI driver requires alignment of the buffer, so try and
make this better by pushing the buffer start back to an word
aligned address. At the worst this makes memcpy() easier as
it is word aligned, at best it makes sure the usb can directly
map the buffer.
Signed-off-by: Ben Dooks
---
Changes since v1:
- Removed the module parameter
- Reworked the tx code to ensure retry if alignment changed
- Explicitly mention the EHCI in the tegra
- Deal with new simpler tx code
---
drivers/net/usb/Kconfig| 12
drivers/net/usb/smsc95xx.c | 22 +++---
2 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index c3ebc43a6582..1af6fb0cadb1 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -364,6 +364,18 @@ config USB_NET_SMSC95XX_TURBO
soft-irq load, thus making it useful to default the option
off for these.
+config USB_NET_SMSC95XX_TXALIGN
+ bool "Add bytes to align transmit buffers"
+ depends on USB_NET_SMSC95XX
+ default n
+ help
+ This option makes the tx buffers 32 bit aligned which might
+ help with systems that want tx data aligned to a 32 bit
+ boundary.
+
+ Using this option will mean there may be up to 3 bytes of
+ data per packet sent.
+
config USB_NET_GL620A
tristate "GeneSys GL620USB-A based cables"
depends on USB_USBNET
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 19e71fe15f6c..8ce190da8be0 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2017,28 +2017,43 @@ static bool smsc95xx_can_tx_checksum(struct sk_buff
*skb)
return skb->csum_offset < (len - (4 + 1));
}
+static inline u32 tx_align(struct sk_buff *skb)
+{
+#ifdef CONFIG_USB_NET_SMSC95XX_TXALIGN
+ return ((u32)skb->data) & 3;
+#else
+ return 0;
+#endif
+}
+
static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
struct sk_buff *skb, gfp_t flags)
{
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+ u32 align;
void *ptr;
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
+retry_align:
+ align = tx_align(skb);
+
/* Make writable and expand header space by overhead if required */
- if (skb_cow_head(skb, overhead)) {
+ if (skb_cow_head(skb, overhead + align)) {
/* Must deallocate here as returning NULL to indicate error
* means the skb won't be deallocated in the caller.
*/
dev_kfree_skb_any(skb);
return NULL;
- }
+ } else if (tx_align(skb) != align)
+ goto retry_align;
tx_cmd_b = (u32)skb->len;
tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+ tx_cmd_a |= align << 16;
if (csum) {
if (!smsc95xx_can_tx_checksum(skb)) {
@@ -2062,7 +2077,8 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet
*dev,
}
}
- ptr = skb_push(skb, 8);
+ /* TX command format is in section 5.4 of SMSC95XX datasbook */
+ ptr = skb_push(skb, 8 + align);
put_unaligned_le32(tx_cmd_a, ptr);
put_unaligned_le32(tx_cmd_b, ptr+4);
--
2.19.1
[PATCH 6/8] usbnet: smsc95xx: fix memcpy for accessing rx-data
Change the RX code to use get_unaligned_le32() instead of the combo of memcpy and cpu_to_le32s(&var). Signed-off-by: Ben Dooks --- drivers/net/usb/smsc95xx.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 8ce190da8be0..03c3c02b569c 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -618,9 +618,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb) return; } - memcpy(&intdata, urb->transfer_buffer, 4); - le32_to_cpus(&intdata); - + intdata = get_unaligned_le32(urb->transfer_buffer); netif_dbg(dev, link, dev->net, "intdata: 0x%08X\n", intdata); if (intdata & INT_ENP_PHY_INT_) @@ -1922,8 +1920,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb) unsigned char *packet; u16 size; - memcpy(&header, skb->data, sizeof(header)); - le32_to_cpus(&header); + header = get_unaligned_le32(skb->data); skb_pull(skb, 4 + NET_IP_ALIGN); packet = skb->data; -- 2.19.1
[PATCH 8/8] usbnet: smsc95xx: add rx_turbo attribute
Add attribute for the rx_turbo mode to allow run-time configuration of
this feature.
Note, this requires a restart of the device to take effect.
Signed-off-by: Ben Dooks
---
drivers/net/usb/smsc95xx.c | 41 +-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 1eb0795ec90f..330c3cf5d6f6 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -73,6 +73,7 @@ struct smsc95xx_priv {
u8 features;
u8 suspend_flags;
u8 mdix_ctrl;
+ bool rx_turbo;
bool link_ok;
struct delayed_work carrier_check;
struct usbnet *dev;
@@ -1103,7 +1104,7 @@ static int smsc95xx_reset(struct usbnet *dev)
"Read Value from HW_CFG after writing HW_CFG_BIR_: 0x%08x\n",
read_buf);
- if (!turbo_mode) {
+ if (!pdata->rx_turbo) {
burst_cap = 0;
dev->rx_urb_size = MAX_SINGLE_PACKET_SIZE;
} else if (dev->udev->speed == USB_SPEED_HIGH) {
@@ -1259,6 +1260,37 @@ static const struct net_device_ops smsc95xx_netdev_ops =
{
.ndo_set_features = smsc95xx_set_features,
};
+static inline struct smsc95xx_priv *dev_to_priv(struct device *dev)
+{
+ struct usb_interface *ui = container_of(dev, struct usb_interface, dev);
+ return usbnet_to_smsc(usb_get_intfdata(ui));
+}
+
+/* Currently rx_turbo will not take effect until next close/open */
+static ssize_t rx_turbo_show(struct device *adev,
+struct device_attribute *attr,
+char *buf)
+{
+ struct smsc95xx_priv *priv = dev_to_priv(adev);
+ return snprintf(buf, PAGE_SIZE, "%d", priv->rx_turbo);
+}
+
+static ssize_t rx_turbo_store(struct device *adev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct smsc95xx_priv *priv = dev_to_priv(adev);
+ bool res;
+
+ if (kstrtobool(buf, &res))
+ return -EINVAL;
+
+ priv->rx_turbo = res;
+ return count;
+}
+
+static DEVICE_ATTR_RW(rx_turbo);
+
static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
{
struct smsc95xx_priv *pdata = NULL;
@@ -1280,6 +1312,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct
usb_interface *intf)
if (!pdata)
return -ENOMEM;
+ pdata->rx_turbo = turbo_mode;
spin_lock_init(&pdata->mac_cr_lock);
/* LAN95xx devices do not alter the computed checksum of 0 to 0x.
@@ -1328,6 +1361,11 @@ static int smsc95xx_bind(struct usbnet *dev, struct
usb_interface *intf)
INIT_DELAYED_WORK(&pdata->carrier_check, check_carrier);
schedule_delayed_work(&pdata->carrier_check, CARRIER_CHECK_DELAY);
+ ret = device_create_file(&dev->udev->dev, &dev_attr_rx_turbo);
+ if (ret)
+ netdev_warn(dev->net,
+ "failed to create rx_turbo attribute: %d\n", ret);
+
return 0;
}
@@ -1336,6 +1374,7 @@ static void smsc95xx_unbind(struct usbnet *dev, struct
usb_interface *intf)
struct smsc95xx_priv *pdata = usbnet_to_smsc(dev);
if (pdata) {
+ device_remove_file(&dev->udev->dev, &dev_attr_rx_turbo);
cancel_delayed_work(&pdata->carrier_check);
netif_dbg(dev, ifdown, dev->net, "free pdata\n");
kfree(pdata);
--
2.19.1
SMSC95XX updates for packet alignment and turbo mode (V2)
This is the new seris of SMSC95XX patches to deal with the issues we
found when working on this driver for a client. The new series has been
tested on an Raspberry Pi3 B.
Changes since v1:
- Change memcpy to use {get,put}_unaligned_le32() calls
- Merge tx fixups
- Added rx_turbo attribute
[PATCH 3/8] usbnet: smsc95xx: simplify tx_fixup code
The smsc95xx_tx_fixup is doing multiple calls to skb_push() to
put an 8-byte command header onto the packet. It would be easier
to do one skb_push() and then copy the data in once the push is
done.
We also make the code smaller by using proper unaligned puts for
the header. This merges in the CPU to LE32 conversion as well and
makes the whole sequence easier to understand hopefully.
Signed-off-by: Ben Dooks
---
Since v1:
- Add alignment changes using put_unaligned_le32()
---
drivers/net/usb/smsc95xx.c | 28 +---
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index cb19aea139d3..0c083d1b7f34 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2006,6 +2006,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet
*dev,
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+ void *ptr;
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
@@ -2019,6 +2020,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet
*dev,
return NULL;
}
+ tx_cmd_b = (u32)skb->len;
+ tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+
if (csum) {
if (skb->len <= 45) {
/* workaround - hardware tx checksum does not work
@@ -2032,24 +2036,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet
*dev,
csum = false;
} else {
u32 csum_preamble = smsc95xx_calc_csum_preamble(skb);
- skb_push(skb, 4);
- cpu_to_le32s(&csum_preamble);
- memcpy(skb->data, &csum_preamble, 4);
+ ptr = skb_push(skb, 4);
+ put_unaligned_le32(csum_preamble, ptr);
+
+ tx_cmd_a += 4;
+ tx_cmd_b += 4;
+ tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
}
}
- skb_push(skb, 4);
- tx_cmd_b = (u32)(skb->len - 4);
- if (csum)
- tx_cmd_b |= TX_CMD_B_CSUM_ENABLE;
- cpu_to_le32s(&tx_cmd_b);
- memcpy(skb->data, &tx_cmd_b, 4);
-
- skb_push(skb, 4);
- tx_cmd_a = (u32)(skb->len - 8) | TX_CMD_A_FIRST_SEG_ |
- TX_CMD_A_LAST_SEG_;
- cpu_to_le32s(&tx_cmd_a);
- memcpy(skb->data, &tx_cmd_a, 4);
+ ptr = skb_push(skb, 8);
+ put_unaligned_le32(tx_cmd_a, ptr);
+ put_unaligned_le32(tx_cmd_b, ptr+4);
return skb;
}
--
2.19.1
Re: [PATCH 6/8] usbnet: smsc95xx: fix memcpy for accessing rx-data
Hello! On 12.10.2018 11:34, Ben Dooks wrote: Change the RX code to use get_unaligned_le32() instead of the combo of memcpy and cpu_to_le32s(&var). le32_to_cpus(), actually. Signed-off-by: Ben Dooks --- drivers/net/usb/smsc95xx.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 8ce190da8be0..03c3c02b569c 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -618,9 +618,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb) return; } - memcpy(&intdata, urb->transfer_buffer, 4); - le32_to_cpus(&intdata); - + intdata = get_unaligned_le32(urb->transfer_buffer); netif_dbg(dev, link, dev->net, "intdata: 0x%08X\n", intdata); if (intdata & INT_ENP_PHY_INT_) @@ -1922,8 +1920,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb) unsigned char *packet; u16 size; - memcpy(&header, skb->data, sizeof(header)); - le32_to_cpus(&header); + header = get_unaligned_le32(skb->data); skb_pull(skb, 4 + NET_IP_ALIGN); packet = skb->data; MBR, Sergei
[PATCH 5/7] net: cdc_ether: add usbnet -> priv function
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
---
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;
boolandroid_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)
{
-
usbnet private-data accessor functions
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).
[PATCH 3/7] net: usb: asix88179_178a: add usbnet -> priv function
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
---
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 = 0x4000;
@@ -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
[PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
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
---
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
us
[PATCH 2/7] net: asix: add usbnet -> priv function
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
---
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
[PATCH 6/7] net: huawei_cdc_ncm: add usbnet -> priv function
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
---
---
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
[PATCH 1/7] net: smsc75xx: add usbnet -> priv function
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
---
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_
[PATCH 7/7] net: usb: sr9800: add usbnet -> priv function
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
---
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
Re: [PATCH 2/7] net: asix: add usbnet -> priv function
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 Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 6/7] net: huawei_cdc_ncm: add usbnet -> priv function
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 Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 5/7] net: cdc_ether: add usbnet -> priv function
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 Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 3/7] net: usb: asix88179_178a: add usbnet -> priv function
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 Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 7/7] net: usb: sr9800: add usbnet -> priv function
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 Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 1/7] net: smsc75xx: add usbnet -> priv function
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 Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
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 Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
Ben Dooks 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
Re: [PATCH 4/7] net: qmi_wwan: add usbnet -> priv function
On 12/10/18 11:44, Bjørn Mork wrote:
Ben Dooks 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
Re: [PATCH v3 3/3] usb: typec: tcpm: charge current handling for sink during hard reset
On Mon, 1 Oct 2018 12:45:01 -0700, Badhri Jagan Sridharan wrote: > During the initial connect to a non-pd port, sink would hard reset > twice before deeming that the port partner is non-pd. TCPM sets the > the charge path to false during the hard reset. This causes unnecessary > connects/disconnects of charge path and makes port take longer to > charge from the non-pd ports. Avoid this by not setting the charge path > to false unless the partner has already identified to be pd capable. > > When partner is a pd port, set the charge path to false in > SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based > of CC pull up and resume the charge path when port enters > SNK_HARD_RESET_SINK_ON. > > Signed-off-by: Badhri Jagan Sridharan > > > Changes in V3: > Rebase on top of usb-next > > Changes in V2: > Based on feedback of ja...@codeaurora.org > - vsafe_5v_hard_reset flag from tcpc_config is removed > - Patch only differentiates between pd port partner and non-pd port > partner > > V1 version of the patch is here: > https://lkml.org/lkml/2018/9/14/11 > --- > drivers/usb/typec/tcpm/tcpm.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > Reviewed-by: Rob Herring
Re: usbnet private-data accessor functions
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
Re: [PATCH net-next] net: cdc_ncm: remove set but not used variable 'ctx'
From: YueHaibing
Date: Fri, 12 Oct 2018 01:49:13 +
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/usb/cdc_ncm.c: In function 'cdc_ncm_status':
> drivers/net/usb/cdc_ncm.c:1603:22: warning:
> variable 'ctx' set but not used [-Wunused-but-set-variable]
> struct cdc_ncm_ctx *ctx;
>
> It not used any more after
> commit fa83dbeee558 ("net: cdc_ncm: remove redundant "disconnected" flag")
>
> Signed-off-by: YueHaibing
Applied.
WARNING in usb_submit_urb (3)
Hello, syzbot found the following crash on: HEAD commit:9dcd936c5312 Merge tag 'for-4.19/dm-fixes-4' of git://git... git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=123b8da140 kernel config: https://syzkaller.appspot.com/x/.config?x=88e9a8a39dc0be2d dashboard link: https://syzkaller.appspot.com/bug?extid=24a30223a4b609bb802e compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1388899140 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1476e5e640 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+24a30223a4b609bb8...@syzkaller.appspotmail.com IPVS: ftp: loaded support on port[0] = 21 [ cut here ] usb usb7: BOGUS urb flags, 1 --> 0 WARNING: CPU: 0 PID: 5828 at drivers/usb/core/urb.c:503 usb_submit_urb+0x717/0x14e0 drivers/usb/core/urb.c:502 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 5828 Comm: syz-executor149 Not tainted 4.19.0-rc7+ #278 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113 panic+0x238/0x4e7 kernel/panic.c:184 __warn.cold.8+0x163/0x1ba kernel/panic.c:536 report_bug+0x254/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296 do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 RIP: 0010:usb_submit_urb+0x717/0x14e0 drivers/usb/core/urb.c:502 Code: 83 fc 48 8b 45 d0 48 8d b8 a0 00 00 00 e8 d1 be 44 ff 45 89 e0 44 89 e9 4c 89 fa 48 89 c6 48 c7 c7 00 72 71 88 e8 09 b3 4d fc <0f> 0b e8 12 e0 83 fc 48 c7 c6 00 73 71 88 4c 89 f7 e8 53 e1 83 fc RSP: 0018:8801bb42f268 EFLAGS: 00010286 RAX: RBX: 8801d754d300 RCX: RDX: RSI: 81650405 RDI: 0005 RBP: 8801bb42f2d8 R08: 8801d8fa6200 R09: fbfff12720fc R10: fbfff12720fc R11: 893907e3 R12: R13: 0001 R14: R15: 8801cdc41bc0 proc_do_submiturb+0x1b7d/0x4020 drivers/usb/core/devio.c:1781 proc_submiturb_compat+0x544/0x800 drivers/usb/core/devio.c:2015 usbdev_do_ioctl+0x19a2/0x3b50 drivers/usb/core/devio.c:2492 usbdev_ioctl+0x25/0x30 drivers/usb/core/devio.c:2569 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:501 [inline] do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 __do_sys_ioctl fs/ioctl.c:709 [inline] __se_sys_ioctl fs/ioctl.c:707 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x444769 Code: 25 02 00 85 c0 b8 00 00 00 00 48 0f 44 c3 5b c3 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 2b d7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:007eff78 EFLAGS: 0213 ORIG_RAX: 0010 RAX: ffda RBX: 7ffc1a2515c0 RCX: 00444769 RDX: 2080 RSI: 802c550a RDI: 0003 RBP: R08: 000120080522 R09: 000120080522 R10: 000f R11: 0213 R12: 00402320 R13: 004023b0 R14: R15: Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
