[PATCH 4/6] staging: ks7010: Remove unnecessary braces.
Braces aren't required for a single line if statement. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 00b97e8e9b4f..2de4dbbcd9de 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1387,9 +1387,8 @@ static __le16 ks_wlan_cap(struct ks_wlan_private *priv) { u16 capability = 0x; - if (priv->reg.preamble == SHORT_PREAMBLE) { + if (priv->reg.preamble == SHORT_PREAMBLE) capability |= WLAN_CAPABILITY_SHORT_PREAMBLE; - } capability &= ~(WLAN_CAPABILITY_PBCC); /* pbcc not support */ -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: ks7010: Fix line over 80 characters.
There is no reason for comment describing the BSSID check for loop to be spaced so far to the right. Move it above the for loop. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 2de4dbbcd9de..6fc2c3647908 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -844,7 +844,8 @@ void hostif_scan_indication(struct ks_wlan_private *priv) ap_info = (struct ap_info_t *)(priv->rxp); if (priv->scan_ind_count) { - for (i = 0; i < priv->aplist.size; i++) { /* bssid check */ + /* bssid check */ + for (i = 0; i < priv->aplist.size; i++) { if (memcmp(ap_info->bssid, priv->aplist.ap[i].bssid, ETH_ALEN) != 0) continue; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: ks7010: Factor out code into helper methods.
Some cases in the switch statement in get_ap_information() are indented as much as five levels, which makes the code difficult to read because of all the wrapping. Factor them out into helper methods. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 46 +- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index a946ce76f899..948d45280d18 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -238,6 +238,30 @@ static u8 read_ie(unsigned char *bp, u8 max, u8 *body, char *name) return size; } +static void read_ie_ext_supp_rates(struct local_ap_t *ap, unsigned char *bp) +{ + if ((*(bp + 1) + ap->rate_set.size) <= RATE_SET_MAX_SIZE) { + memcpy(&ap->rate_set.body[ap->rate_set.size], + bp + 2, *(bp + 1)); + ap->rate_set.size += *(bp + 1); + } else { + DPRINTK(1, "size over :: rate size=%d\n", + (*(bp + 1) + ap->rate_set.size)); + memcpy(&ap->rate_set.body[ap->rate_set.size], bp + 2, + RATE_SET_MAX_SIZE - ap->rate_set.size); + ap->rate_set.size += (RATE_SET_MAX_SIZE - ap->rate_set.size); + } +} + +static void read_ie_wpa(struct local_ap_t *ap, unsigned char *bp) +{ + if (memcmp(bp + 2, CIPHER_ID_WPA_WEP40, 4) == 0) { /* WPA OUI check */ + ap->wpa_ie.id = *bp; + ap->wpa_ie.size = read_ie(bp, RSN_IE_BODY_MAX, + ap->wpa_ie.body, "wpa"); + } +} + static int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, struct local_ap_t *ap) @@ -273,20 +297,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, break; case WLAN_EID_SUPP_RATES: case WLAN_EID_EXT_SUPP_RATES: - if ((*(bp + 1) + ap->rate_set.size) <= - RATE_SET_MAX_SIZE) { - memcpy(&ap->rate_set.body[ap->rate_set.size], - bp + 2, *(bp + 1)); - ap->rate_set.size += *(bp + 1); - } else { - DPRINTK(1, "size over :: rate size=%d\n", - (*(bp + 1) + ap->rate_set.size)); - memcpy(&ap->rate_set.body[ap->rate_set.size], - bp + 2, - RATE_SET_MAX_SIZE - ap->rate_set.size); - ap->rate_set.size += - (RATE_SET_MAX_SIZE - ap->rate_set.size); - } + read_ie_ext_supp_rates(ap, bp); break; case WLAN_EID_DS_PARAMS: break; @@ -296,12 +307,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, ap->rsn_ie.body, "rsn"); break; case WLAN_EID_VENDOR_SPECIFIC: /* WPA */ - if (memcmp(bp + 2, CIPHER_ID_WPA_WEP40, 4) == 0) { /* WPA OUI check */ - ap->wpa_ie.id = *bp; - ap->wpa_ie.size = read_ie(bp, RSN_IE_BODY_MAX, - ap->wpa_ie.body, - "wpa"); - } + read_ie_wpa(ap, bp); break; case WLAN_EID_FH_PARAMS: -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: ks7010: Factor out repeated request initialization code.
The code to initialize various different types of request structs is repeated multiple times. Factor this code out into a macro called INIT_REQUEST. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 55 +++--- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 6fc2c3647908..3e5016aad029 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -40,6 +40,17 @@ static inline unsigned int cnt_smeqbody(struct ks_wlan_private *priv) #define KS_WLAN_MEM_FLAG (GFP_ATOMIC) +#define INIT_REQUEST(pp, priv) +{( + pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); + pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); + pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); + pp->rate_set.size = priv->reg.rate_set.size; + pp->capability = ks_wlan_cap(priv); + memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], + priv->reg.rate_set.size); +)} + static inline u8 get_BYTE(struct ks_wlan_private *priv) { @@ -1412,14 +1423,7 @@ void hostif_ps_adhoc_set_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - pp->channel = cpu_to_le16((uint16_t)(priv->reg.channel)); - pp->rate_set.size = priv->reg.rate_set.size; - pp->capability = ks_wlan_cap(priv); - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); /* send to device request */ ps_confirm_wait_inc(priv); @@ -1437,16 +1441,9 @@ void hostif_infrastructure_set_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); pp->ssid.size = priv->reg.ssid.size; memcpy(&pp->ssid.body[0], &priv->reg.ssid.body[0], priv->reg.ssid.size); - pp->capability = ks_wlan_cap(priv); pp->beacon_lost_count = cpu_to_le16((uint16_t)(priv->reg.beacon_lost_count)); pp->auth_type = cpu_to_le16((uint16_t)(priv->reg.authenticate_type)); @@ -1486,16 +1483,9 @@ static void hostif_infrastructure_set2_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); pp->ssid.size = priv->reg.ssid.size; memcpy(&pp->ssid.body[0], &priv->reg.ssid.body[0], priv->reg.ssid.size); - pp->capability = ks_wlan_cap(priv); pp->beacon_lost_count = cpu_to_le16((uint16_t)(priv->reg.beacon_lost_count)); pp->auth_type = cpu_to_le16((uint16_t)(priv->reg.authenticate_type)); @@ -1538,16 +1528,9 @@ void hostif_adhoc_set_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - pp->channel = cpu_to_le16((uint16_t)(priv->reg.channel)); - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); + INIT_REQUEST(pp, priv); pp->ssid.size = priv->reg.ssid.size; memcpy(&pp->ssid.body[0], &priv->reg.ssid.body[0], priv->reg.ssid.size); - pp->capability = ks_wlan_cap(priv); /* send to device request */ ps_confirm_wait_inc(priv); @@ -1565,15 +1548,9 @@ void hostif_adhoc_set2_request(struct ks_wlan_private *priv) if (!pp) return; - pp->phy_type = cpu_to_le16((uint16_t)(priv->reg.phy_type)); - pp->cts_mode = cpu_to_le16((uint16_t)(priv->reg.cts_mode)); - pp->scan_type = cpu_to_le16((uint16_t)(priv->reg.scan_type)); - pp->rate_set.size = priv->reg.rate_set.size; - memcpy(&pp->rate_set.body[0], &priv->reg.rate_set.body[0], - priv->reg.rate_set.size); +
[PATCH 3/6] staging: ks7010: Remove unnecessary parentheses.
Remove unnecessary parentheses highlighted by checkpatch. Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 948d45280d18..00b97e8e9b4f 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -45,7 +45,7 @@ inline u8 get_BYTE(struct ks_wlan_private *priv) { u8 data; - data = *(priv->rxp)++; + data = *priv->rxp++; /* length check in advance ! */ --(priv->rx_size); return data; @@ -860,7 +860,7 @@ void hostif_scan_indication(struct ks_wlan_private *priv) DPRINTK(4, " scan_ind_count=%d :: aplist.size=%d\n", priv->scan_ind_count, priv->aplist.size); get_ap_information(priv, (struct ap_info_t *)(priv->rxp), - &(priv->aplist.ap[priv->scan_ind_count - 1])); + &priv->aplist.ap[priv->scan_ind_count - 1]); priv->aplist.size = priv->scan_ind_count; } else { DPRINTK(4, " count over :: scan_ind_count=%d\n", -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging: ks7010: Factor out repeated code.
Some of the code for reading IEs is replicated multiple times in the switch statement for get_ap_information(). Factor that code out into read_ie(). Signed-off-by: Quytelda Kahja --- drivers/staging/ks7010/ks_hostif.c | 48 +- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 05f7be4638fe..a946ce76f899 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -223,6 +223,21 @@ int get_current_ap(struct ks_wlan_private *priv, struct link_ap_info_t *ap_info) return 0; } +static u8 read_ie(unsigned char *bp, u8 max, u8 *body, char *name) +{ + u8 size; + + if (*(bp + 1) <= max) { + size = *(bp + 1); + } else { + DPRINTK(1, "size over :: %s size=%d\n", name, *(bp + 1)); + size = max; + } + + memcpy(body, bp + 2, size); + return size; +} + static int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, struct local_ap_t *ap) @@ -253,14 +268,8 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, while (bsize > offset) { switch (*bp) { /* Information Element ID */ case WLAN_EID_SSID: - if (*(bp + 1) <= IEEE80211_MAX_SSID_LEN) { - ap->ssid.size = *(bp + 1); - } else { - DPRINTK(1, "size over :: ssid size=%d\n", - *(bp + 1)); - ap->ssid.size = IEEE80211_MAX_SSID_LEN; - } - memcpy(ap->ssid.body, bp + 2, ap->ssid.size); + ap->ssid.size = read_ie(bp, IEEE80211_MAX_SSID_LEN, + ap->ssid.body, "ssid"); break; case WLAN_EID_SUPP_RATES: case WLAN_EID_EXT_SUPP_RATES: @@ -283,28 +292,15 @@ int get_ap_information(struct ks_wlan_private *priv, struct ap_info_t *ap_info, break; case WLAN_EID_RSN: ap->rsn_ie.id = *bp; - if (*(bp + 1) <= RSN_IE_BODY_MAX) { - ap->rsn_ie.size = *(bp + 1); - } else { - DPRINTK(1, "size over :: rsn size=%d\n", - *(bp + 1)); - ap->rsn_ie.size = RSN_IE_BODY_MAX; - } - memcpy(ap->rsn_ie.body, bp + 2, ap->rsn_ie.size); + ap->rsn_ie.size = read_ie(bp, RSN_IE_BODY_MAX, + ap->rsn_ie.body, "rsn"); break; case WLAN_EID_VENDOR_SPECIFIC: /* WPA */ if (memcmp(bp + 2, CIPHER_ID_WPA_WEP40, 4) == 0) { /* WPA OUI check */ ap->wpa_ie.id = *bp; - if (*(bp + 1) <= RSN_IE_BODY_MAX) { - ap->wpa_ie.size = *(bp + 1); - } else { - DPRINTK(1, - "size over :: wpa size=%d\n", - *(bp + 1)); - ap->wpa_ie.size = RSN_IE_BODY_MAX; - } - memcpy(ap->wpa_ie.body, bp + 2, - ap->wpa_ie.size); + ap->wpa_ie.size = read_ie(bp, RSN_IE_BODY_MAX, + ap->wpa_ie.body, + "wpa"); } break; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: media: davinci_vpfe: add error handling on kmalloc failure
There is no failure checking on the param value which will be allocated memory by kmalloc. Add a null pointer checking statement. Then goto error: and return -ENOMEM error code when kmalloc is failed. Signed-off-by: Ji-Hun Kim --- drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c index 6a3434c..55a922c 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c @@ -1280,6 +1280,10 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) params = kmalloc(sizeof(struct ipipe_module_params), GFP_KERNEL); + if (!params) { + rval = -ENOMEM; + goto error; + } to = (void *)params + module_if->param_offset; size = module_if->param_size; @@ -1323,6 +1327,10 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) params = kmalloc(sizeof(struct ipipe_module_params), GFP_KERNEL); + if (!params) { + rval = -ENOMEM; + goto error; + } from = (void *)params + module_if->param_offset; size = module_if->param_size; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] storvsc: Set up correct queue depth values for IDE devices
On Thu, 15 Mar 2018 16:52:23 -0700 Long Li wrote: > From: Long Li > > Unlike SCSI and FC, we don't use multiple channels for IDE. So set queue depth > correctly for IDE. > > Also set the correct cmd_per_lun for all devices. > > Signed-off-by: Long Li Looks correct. Acked-by: Stephen Hemminger ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/7] staging:iio:ade7854: Add proper error handling condition
On 03/15, Dan Carpenter wrote: > On Wed, Mar 14, 2018 at 03:12:18PM -0300, Rodrigo Siqueira wrote: > > There is some improper error handling for IRQ and device register. This > > patch adds a proper verification. The IRQ correction was extracted from > > John Syne patches. > > > > Signed-off-by: Rodrigo Siqueira > > Signed-off-by: John Syne > > --- > > drivers/staging/iio/meter/ade7854.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/iio/meter/ade7854.c > > b/drivers/staging/iio/meter/ade7854.c > > index 09fd8c067738..49cbe365e43d 100644 > > --- a/drivers/staging/iio/meter/ade7854.c > > +++ b/drivers/staging/iio/meter/ade7854.c > > @@ -436,7 +436,7 @@ static int ade7854_initial_setup(struct iio_dev > > *indio_dev) > > > > /* Disable IRQ */ > > ret = ade7854_set_irq(dev, false); > > - if (ret) { > > + if (ret < 0) { > > dev_err(dev, "disable irq failed"); > > goto err_ret; > > } > > Why is the original wrong? It seems fine. Hi, If you look at "drivers/staging/iio/meter/ade7854-(i2c|spi).c", you will find a line like this: " st->write_reg = ade7854_(i2c|spi)_write_reg;". Than, if you go through the "ade7854_i2c_write_reg" (focus only on i2c for while) you will see "i2c_master_send(st->i2c, st->tx, count)", which can returns a positive number that does not necessary means an error. If you find the same for the spi case, you will end up in the function "spi_sync_transfer" that only return 0 for success or negative for failure. So, handling the negative case better fit the i2c and spi case. Thanks > regards, > dan carpenter > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] staging:iio:ade7854: Rework I2C write function
Hi, I will fixes all of these things here and in the other patches and send a v2. Thanks for the review. On 03/15, Dan Carpenter wrote: > On Wed, Mar 14, 2018 at 03:10:18PM -0300, Rodrigo Siqueira wrote: > > The write operation using I2C has many code duplications and four > > different interfaces per data size. This patch introduces a single > > function that centralizes the main tasks. > > > > The central function inserted by this patch can easily replace all the > > four functions related to the data size. However, this patch does not > > remove any code signature for keeping the meter module work and make > > easier to review this patch. > > > > Signed-off-by: Rodrigo Siqueira > > --- > > drivers/staging/iio/meter/ade7854-i2c.c | 89 > > +++-- > > drivers/staging/iio/meter/ade7854.h | 7 +++ > > 2 files changed, 58 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/staging/iio/meter/ade7854-i2c.c > > b/drivers/staging/iio/meter/ade7854-i2c.c > > index 317e4f0d8176..03133a05eae4 100644 > > --- a/drivers/staging/iio/meter/ade7854-i2c.c > > +++ b/drivers/staging/iio/meter/ade7854-i2c.c > > @@ -15,41 +15,74 @@ > > #include > > #include "ade7854.h" > > > > -static int ade7854_i2c_write_reg_8(struct device *dev, > > - u16 reg_address, > > - u8 val) > > +static int ade7854_i2c_write_reg(struct device *dev, > > +u16 reg_address, > > +u32 val, > > +enum data_size type) > > > The data size should just be the number of bytes and not an enum. > 1 means 1 byte / 8 bits. > 2 means 2 bytes / 16 bits. > 3 means 3 bytes / 24 bits. > etc. > > > { > > int ret; > > + int count; > > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > > struct ade7854_state *st = iio_priv(indio_dev); > > > > mutex_lock(&st->buf_lock); > > st->tx[0] = (reg_address >> 8) & 0xFF; > > st->tx[1] = reg_address & 0xFF; > > - st->tx[2] = val; > > > > - ret = i2c_master_send(st->i2c, st->tx, 3); > > + switch (type) { > > + case DATA_SIZE_8_BITS: > > + st->tx[2] = val & 0xFF; > > + count = 3; > > + break; > > + case DATA_SIZE_16_BITS: > > + st->tx[2] = (val >> 8) & 0xFF; > > + st->tx[3] = val & 0xFF; > > + count = 4; > > + break; > > + case DATA_SIZE_24_BITS: > > + st->tx[2] = (val >> 16) & 0xFF; > > + st->tx[3] = (val >> 8) & 0xFF; > > + st->tx[4] = val & 0xFF; > > + count = 5; > > + break; > > + case DATA_SIZE_32_BITS: > > + st->tx[2] = (val >> 24) & 0xFF; > > + st->tx[3] = (val >> 16) & 0xFF; > > + st->tx[4] = (val >> 8) & 0xFF; > > + st->tx[5] = val & 0xFF; > > + count = 6; > > + break; > > + default: > > + ret = -EINVAL; > > + goto error_i2c_write_unlock; > > + } > > + > > + ret = i2c_master_send(st->i2c, st->tx, count); > > + > > +error_i2c_write_unlock: > > These labels are sort of long. And what does the "i2c_write" really > mean? It should be obvious that we're not jumping to a different > function. > > Just "unlock:" is OK as a label name. > > > mutex_unlock(&st->buf_lock); > > > > return ret; > > } > > > > +static int ade7854_i2c_write_reg_8(struct device *dev, > > + u16 reg_address, > > + u8 val) > > +{ > > + int ret; > > + > > + ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS); > > + > > + return ret; > > +} > > Just do it like this: > > static int ade7854_i2c_write_reg_8(struct device *dev, u16 reg_address, u8 > val) > { > return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS); > } > > > > > + > > static int ade7854_i2c_write_reg_16(struct device *dev, > > u16 reg_address, > > u16 val) > > { > > int ret; > > - struct iio_dev *indio_dev = dev_to_iio_dev(dev); > > - struct ade7854_state *st = iio_priv(indio_dev); > > > > - mutex_lock(&st->buf_lock); > > - st->tx[0] = (reg_address >> 8) & 0xFF; > > - st->tx[1] = reg_address & 0xFF; > > - st->tx[2] = (val >> 8) & 0xFF; > > - st->tx[3] = val & 0xFF; > > - > > - ret = i2c_master_send(st->i2c, st->tx, 4); > > - mutex_unlock(&st->buf_lock); > > + ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS); > > > > return ret; > > Again: > > return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS); > > > > > } > > @@ -59,18 +92,8 @@ static int ade7854_i2c_write_reg_24(struct device *dev, > > u32 val) > > { > > int ret; > > - struct iio_dev *indio_dev = dev_to_iio_dev(dev); > > - struct ade7854_state *st = iio_priv(indio_dev); > > > > - mutex_lock(&st
Re: [PATCH v6 0/6] staging: Introduce DPAA2 Ethernet Switch driver
On Thu, Mar 15, 2018 at 01:56:42PM +0300, Dan Carpenter wrote: > On Thu, Mar 15, 2018 at 12:44:37AM +0100, Andrew Lunn wrote: > > On Wed, Mar 14, 2018 at 10:55:52AM -0500, Razvan Stefanescu wrote: > > > This patchset introduces the Ethernet Switch Driver for Freescale/NXP SoCs > > > with DPAA2 (DataPath Acceleration Architecture v2). The driver manages > > > switch objects discovered on the fsl-mc bus. A description of the driver > > > can be found in the associated README file. > > > > Hi Greg > > > > This code has much better quality than the usual stuff in staging. I > > see no reason not to merge it. > > Yeah. It seems pretty decent. Stuart, Laurentiu, care to comment? > > Meanwhile, netdev and DaveM aren't even on the CC list and they're the > ones to ultimately decide. The patches are for staging, so it is GregKH who decides at this point, not really DaveM. Andrew ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] storvsc: Set up correct queue depth values for IDE devices
From: Long Li Unlike SCSI and FC, we don't use multiple channels for IDE. So set queue depth correctly for IDE. Also set the correct cmd_per_lun for all devices. Signed-off-by: Long Li --- drivers/scsi/storvsc_drv.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 8c51d628b52e..fba170640e9c 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1722,15 +1722,19 @@ static int storvsc_probe(struct hv_device *device, max_targets = STORVSC_MAX_TARGETS; max_channels = STORVSC_MAX_CHANNELS; /* -* On Windows8 and above, we support sub-channels for storage. +* On Windows8 and above, we support sub-channels for storage +* on SCSI and FC controllers. * The number of sub-channels offerred is based on the number of * VCPUs in the guest. */ - max_sub_channels = (num_cpus / storvsc_vcpus_per_sub_channel); + if (!dev_is_ide) + max_sub_channels = + num_cpus / storvsc_vcpus_per_sub_channel; } scsi_driver.can_queue = (max_outstanding_req_per_channel * (max_sub_channels + 1)); + scsi_driver.cmd_per_lun = scsi_driver.can_queue; host = scsi_host_alloc(&scsi_driver, sizeof(struct hv_host_device)); -- 2.14.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On Thu, Mar 15 2018, John Crispin wrote: > On 15/03/18 21:12, NeilBrown wrote: >> On Thu, Mar 15 2018, John Crispin wrote: >> >>> On 15/03/18 11:48, Dan Carpenter wrote: This all seems fine. Generally the requirements for staging are that it has a TODO, someone to work on it, and it doesn't break the build. But some of the patches don't have commit message and those are required and some of the commit messages are just the changes you have made not don't describe the actual code... John Crispin's email is j...@phrozen.org. regards, dan carpenter >>> Hi All, >>> >>> looks like i was CC'ed on the openwrt addr, which no longer exists. This >>> series makes no sense. None of the stuff posted is anywhere near ready >>> to be upstreamed. >>> >>> * we dont need a dedicated pinctrl driver, pinctrl-single will work fine >>> on these SoCs >>> * the DMA/sdhci driver is a hacked up version of the SDK driver. >>> * drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable >>> to mt7621, same goes for the gsw driver. >> Hi John, >> I think it makes sense in that, with the patches, the hardware works, and >> without the patches (at least the first) you cannot even build with >> CONFIG_SOC_MT7621=y as pcibios_map_irq() is undefined. Having >> working code is a great starting point for further development. >> It certainly isn't ready for upstream, which is why it is heading for >> drivers/staging. This is explicitly for code that isn't yet ready. >> By putting the code there it should be safe from bit-rot, and can be >> worked on by multiple people. It gets increased visibility so people >> can say how bad it is (as you have done - thanks). This feed back is a >> valuable part of improving the code and getting it out of staging. >> >> I'll add notes to various TODO files based on your comments. If you >> have anything else to add, it would be most welcome. Thank you for >> making these patches available in the first place, so that my hardware >> can work! >> >> Thanks, >> NeilBrown > > Hi Neil, > > I understand your reasoning, however ... > only the pcie driver is worth merging. all other drivers are already > inside the kernel for mt7623 and can be easily adapted to work on > mt7621. having duplicate drivers is a certain no-go. > cleaning up the pci driver is a matter of a few days work. merging a > shitty pci driver just to postpone doing the 3-5 days work involved to > polish it seems a rally bad trade-off. > i strongly oppose having any of this code merged into the kernel, even > if it is only the staging area. Hi John, I don't understand why you would want to deny people easy access to code which makes their hardware work. I'm sure that isn't your intention, but it could well be the effect. If you think you can make it work in 3-5 days, then please go right ahead. I will happily test anything you submit. I suspect the most likely outcome, however, is that I'll end up doing all the work, and I'm sure it will take a lot more than 3-5 days and will involve a lot of learning. I'll be happy if I can get all of this back out of staging in 6 months. That is an extra 6 months (at least) that people will be able to use a mainline kernel on their gnubee. With respect to your suggestion that "having duplicate drivers is a certain no-go", I don't think that is correct. As an approximate counter-point, I'm in the process of cleaning up the drivers/staging/lustre filesystem with the hope of eventually moving it out of staging. It had duplicate wait_event() code, duplicate PRNG, duplicate workqueues, duplicate resizeable hashtable, duplicate tracing infrastructure, and more that I haven't had a chance to look closely at yet. This duplication certainly kept it out of linux/fs/, but has no bearing on whether it belong in linux/drivers/staging/. Thanks, NeilBrown signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: o2iblnd: Enable Multiple OPA Endpoints between Nodes
OPA driver optimizations are based on the MPI model where it is expected to have multiple endpoints between two given nodes. To enable this optimization for Lustre, we need to make it possible, via an LND-specific tuneable, to create multiple endpoints and to balance the traffic over them. Both sides of a connection must have this patch for it to work. Only the active side of the connection (usually the client) needs to have the new tuneable set > 1. Signed-off-by: Doug Oucharek Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8943 Reviewed-on: https://review.whamcloud.com/25168 Reviewed-by: Amir Shehata Reviewed-by: Dmitry Eremin Reviewed-by: James Simmons Reviewed-by: Oleg Drokin Signed-off-by: Doug Oucharek --- .../lustre/include/uapi/linux/lnet/lnet-dlc.h | 3 ++- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h| 17 --- .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 25 +++--- .../lustre/lnet/klnds/o2iblnd/o2iblnd_modparams.c | 9 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h b/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h index e45d828..c1619f4 100644 --- a/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h +++ b/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h @@ -53,7 +53,8 @@ struct lnet_ioctl_config_o2iblnd_tunables { __u32 lnd_fmr_pool_size; __u32 lnd_fmr_flush_trigger; __u32 lnd_fmr_cache; - __u32 pad; + __u16 lnd_conns_per_peer; + __u16 pad; }; struct lnet_ioctl_config_lnd_tunables { diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h index b18911d..af5f573 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h @@ -568,6 +568,8 @@ struct kib_peer { lnet_nid_t ibp_nid; /* who's on the other end(s) */ struct lnet_ni *ibp_ni; /* LNet interface */ struct list_head ibp_conns; /* all active connections */ + struct kib_conn *ibp_next_conn; /* next connection to send on for + round robin */ struct list_head ibp_tx_queue;/* msgs waiting for a conn */ __u64ibp_incarnation; /* incarnation of peer */ /* when (in jiffies) I was last alive */ @@ -581,7 +583,7 @@ struct kib_peer { /* current active connection attempts */ unsigned short ibp_connecting; /* reconnect this peer later */ - unsigned short ibp_reconnecting:1; + unsigned char ibp_reconnecting; /* counter of how many times we triggered a conn race */ unsigned char ibp_races; /* # consecutive reconnection attempts to this peer */ @@ -744,10 +746,19 @@ struct kib_peer { static inline struct kib_conn * kiblnd_get_conn_locked(struct kib_peer *peer) { + struct list_head *next; + LASSERT(!list_empty(&peer->ibp_conns)); - /* just return the first connection */ - return list_entry(peer->ibp_conns.next, struct kib_conn, ibc_list); + /* Advance to next connection, be sure to skip the head node */ + if (!peer->ibp_next_conn || + peer->ibp_next_conn->ibc_list.next == &peer->ibp_conns) + next = peer->ibp_conns.next; + else + next = peer->ibp_next_conn->ibc_list.next; + peer->ibp_next_conn = list_entry(next, struct kib_conn, ibc_list); + + return peer->ibp_next_conn; } static inline int diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c index 6690a6c..3cb1f720 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c @@ -1250,7 +1250,6 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid, LASSERT(net); LASSERT(peer->ibp_connecting > 0); - LASSERT(!peer->ibp_reconnecting); cmid = kiblnd_rdma_create_id(kiblnd_cm_callback, peer, RDMA_PS_TCP, IB_QPT_RC); @@ -1332,7 +1331,7 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid, LASSERT(!peer->ibp_accepting && !peer->ibp_connecting && list_empty(&peer->ibp_conns)); - peer->ibp_reconnecting = 0; + peer->ibp_reconnecting--; if (!kiblnd_peer_active(peer)) { list_splice_init(&peer->ibp_tx_queue, &txs); @@ -1365,6 +1364,8 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid, rwlock_t *g_lock = &kiblnd_data.kib_global_lock; unsigned long flags; int rc; + inti; + struct lnet_ioctl_config_o2iblnd_tunables *tunables; /* * If I get here, I've committed to send, so I complete the tx with @@ -1461,7 +1462,8 @
Re: [PATCH v2] Staging: iio: adis16209: Move adis16209 driver out of staging
On 16 March 2018 00:31:53 GMT+05:30, Shreeya Patel wrote: >On Sat, 2018-03-10 at 15:57 +, Jonathan Cameron wrote: > >Hi Jonathan, > >> On Sat, 10 Mar 2018 15:50:23 +0530 >> Shreeya Patel wrote: >> >> > >> > Move the adis16209 driver out of staging directory and merge to the >> > mainline IIO subsystem. >> > >> > Signed-off-by: Shreeya Patel >> As this has a clear dependency on the previous patch, please put them >> in the same series for the next version. That way I won't miss one! >> >> This also doesn't actually seem to have all the patches in place. >> The sign extend one is definitely missing for some reason. >> >> One question on the ABI choice of X for the rotation axis. >> I think the logical choice is actually Z but would like to know what >> you and others think. >> >> All existing users of IIO_ROT (outside staging) have been >> magnetometer >> where we don't have an axis, but rather a magnetic reference frame or >> a quaternion output which includes all the axes. >> >> Jonathan >> >> > >> > --- >> > >> > Changes in v2 >> > -Re-send the patch after having some cleanups in the >> > file included in this patch. >> > >> > drivers/iio/accel/Kconfig | 12 ++ >> > drivers/iio/accel/Makefile| 1 + >> > drivers/iio/accel/adis16209.c | 329 >> > ++ >> > drivers/staging/iio/accel/Kconfig | 12 -- >> > drivers/staging/iio/accel/Makefile| 1 - >> > drivers/staging/iio/accel/adis16209.c | 329 -- >> > >> > 6 files changed, 342 insertions(+), 342 deletions(-) >> > create mode 100644 drivers/iio/accel/adis16209.c >> > delete mode 100644 drivers/staging/iio/accel/adis16209.c >> > >> > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig >> > index c6d9517..f95f43c 100644 >> > --- a/drivers/iio/accel/Kconfig >> > +++ b/drivers/iio/accel/Kconfig >> > @@ -5,6 +5,18 @@ >> > >> > menu "Accelerometers" >> > >> > +config ADIS16209 >> > +tristate "Analog Devices ADIS16209 Dual-Axis Digital >> > Inclinometer and Accelerometer" >> > +depends on SPI >> > +select IIO_ADIS_LIB >> > +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER >> > +help >> > + Say Y here to build support for Analog Devices adis16209 >> > dual-axis digital inclinometer >> > + and accelerometer. >> > + >> > + To compile this driver as a module, say M here: the >> > module will be >> > + called adis16209. >> > + >> > config ADXL345 >> > tristate >> > >> > diff --git a/drivers/iio/accel/Makefile >> > b/drivers/iio/accel/Makefile >> > index 368aedb..40861b9 100644 >> > --- a/drivers/iio/accel/Makefile >> > +++ b/drivers/iio/accel/Makefile >> > @@ -4,6 +4,7 @@ >> > # >> > >> > # When adding new entries keep the list in alphabetical order >> > +obj-$(CONFIG_ADIS16209) += adis16209.o >> > obj-$(CONFIG_ADXL345) += adxl345_core.o >> > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o >> > obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o >> > diff --git a/drivers/iio/accel/adis16209.c >> > b/drivers/iio/accel/adis16209.c >> > new file mode 100644 >> > index 000..ed2e89f >> > --- /dev/null >> > +++ b/drivers/iio/accel/adis16209.c >> > @@ -0,0 +1,329 @@ >> > +/* >> > + * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer >> > + * >> > + * Copyright 2010 Analog Devices Inc. >> > + * >> > + * Licensed under the GPL-2 or later. >> > + */ >> > + >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > + >> > +#include >> > +#include >> > +#include >> > +#include >> > + >> > +#define ADIS16209_STARTUP_DELAY_MS220 >> > +#define ADIS16209_FLASH_CNT_REG 0x00 >> > + >> > +/* Data Output Register Definitions */ >> > +#define ADIS16209_SUPPLY_OUT_REG 0x02 >> > +#define ADIS16209_XACCL_OUT_REG 0x04 >> > +#define ADIS16209_YACCL_OUT_REG 0x06 >> > +/* Output, auxiliary ADC input */ >> > +#define ADIS16209_AUX_ADC_REG 0x08 >> > +/* Output, temperature */ >> > +#define ADIS16209_TEMP_OUT_REG0x0A >> > +/* Output, +/- 90 degrees X-axis inclination */ >> > +#define ADIS16209_XINCL_OUT_REG 0x0C >> > +#define ADIS16209_YINCL_OUT_REG 0x0E >> > +/* Output, +/-180 vertical rotational position */ >> > +#define ADIS16209_ROT_OUT_REG 0x10 >> > + >> > +/* >> > + * Calibration Register Definitions. >> > + * Acceleration, inclination or rotation offset null. >> > + */ >> > +#define ADIS16209_XACCL_NULL_REG 0x12 >> > +#define ADIS16209_YACCL_NULL_REG 0x14 >> > +#define ADIS16209_XINCL_NULL_REG 0x16 >> > +#define ADIS16209_YINCL_NULL_REG 0x18 >> > +#define ADIS16209_ROT_NULL_REG0x1A >> > + >> > +/* Alarm Register Definitions */ >> > +#define ADIS16209_ALM_MAG1_REG0x20 >> > +#define ADIS16209_ALM_MAG2_REG0x22 >> > +#define ADIS16209_ALM_SMPL1_REG
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On 15/03/18 21:12, NeilBrown wrote: On Thu, Mar 15 2018, John Crispin wrote: On 15/03/18 11:48, Dan Carpenter wrote: This all seems fine. Generally the requirements for staging are that it has a TODO, someone to work on it, and it doesn't break the build. But some of the patches don't have commit message and those are required and some of the commit messages are just the changes you have made not don't describe the actual code... John Crispin's email is j...@phrozen.org. regards, dan carpenter Hi All, looks like i was CC'ed on the openwrt addr, which no longer exists. This series makes no sense. None of the stuff posted is anywhere near ready to be upstreamed. * we dont need a dedicated pinctrl driver, pinctrl-single will work fine on these SoCs * the DMA/sdhci driver is a hacked up version of the SDK driver. * drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable to mt7621, same goes for the gsw driver. Hi John, I think it makes sense in that, with the patches, the hardware works, and without the patches (at least the first) you cannot even build with CONFIG_SOC_MT7621=y as pcibios_map_irq() is undefined. Having working code is a great starting point for further development. It certainly isn't ready for upstream, which is why it is heading for drivers/staging. This is explicitly for code that isn't yet ready. By putting the code there it should be safe from bit-rot, and can be worked on by multiple people. It gets increased visibility so people can say how bad it is (as you have done - thanks). This feed back is a valuable part of improving the code and getting it out of staging. I'll add notes to various TODO files based on your comments. If you have anything else to add, it would be most welcome. Thank you for making these patches available in the first place, so that my hardware can work! Thanks, NeilBrown Hi Neil, I understand your reasoning, however ... only the pcie driver is worth merging. all other drivers are already inside the kernel for mt7623 and can be easily adapted to work on mt7621. having duplicate drivers is a certain no-go. cleaning up the pci driver is a matter of a few days work. merging a shitty pci driver just to postpone doing the 3-5 days work involved to polish it seems a rally bad trade-off. i strongly oppose having any of this code merged into the kernel, even if it is only the staging area. John ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On Thu, Mar 15 2018, John Crispin wrote: > On 15/03/18 11:48, Dan Carpenter wrote: >> This all seems fine. Generally the requirements for staging are that it >> has a TODO, someone to work on it, and it doesn't break the build. But >> some of the patches don't have commit message and those are required and >> some of the commit messages are just the changes you have made not don't >> describe the actual code... >> >> John Crispin's email is j...@phrozen.org. >> >> regards, >> dan carpenter >> > Hi All, > > looks like i was CC'ed on the openwrt addr, which no longer exists. This > series makes no sense. None of the stuff posted is anywhere near ready > to be upstreamed. > > * we dont need a dedicated pinctrl driver, pinctrl-single will work fine > on these SoCs > * the DMA/sdhci driver is a hacked up version of the SDK driver. > * drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable > to mt7621, same goes for the gsw driver. Hi John, I think it makes sense in that, with the patches, the hardware works, and without the patches (at least the first) you cannot even build with CONFIG_SOC_MT7621=y as pcibios_map_irq() is undefined. Having working code is a great starting point for further development. It certainly isn't ready for upstream, which is why it is heading for drivers/staging. This is explicitly for code that isn't yet ready. By putting the code there it should be safe from bit-rot, and can be worked on by multiple people. It gets increased visibility so people can say how bad it is (as you have done - thanks). This feed back is a valuable part of improving the code and getting it out of staging. I'll add notes to various TODO files based on your comments. If you have anything else to add, it would be most welcome. Thank you for making these patches available in the first place, so that my hardware can work! Thanks, NeilBrown signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On Thu, Mar 15 2018, Dan Carpenter wrote: > On Thu, Mar 15, 2018 at 10:04:33PM +1100, NeilBrown wrote: >> On Thu, Mar 15 2018, Dan Carpenter wrote: >> >> > This all seems fine. Generally the requirements for staging are that it >> > has a TODO, someone to work on it, and it doesn't break the build. But >> > some of the patches don't have commit message and those are required and >> > some of the commit messages are just the changes you have made not don't >> > describe the actual code... >> >> Thanks for having a look. >> It seems odd to require detailed commit messages, when we don't require >> the same level of quality in the code. >> Naturally when the driver is moved out of staging a properly detailed >> commit message should be added, but is that needed on the way in to >> staging? At this stage I don't know much more than is already there. >> After I've cleaned up the code I probably will. >> >> For patch 01/13 you asked "what kind of device this is". The subject >> line makes it clear that it is a "pcie driver". What extra detail did >> you want? Would it be sufficient to just copy the subject line so that >> it appears twice in the commit message? >> > > Ah... Sorry. It's literally a pcie driver. For some reason I thought > it was a device that ran over pcie. > > We don't require a detailed changelog, but you have to put something... > Probably just restating the subject and adding that it's for the gnubee1 > is fine. I'll resend sometime next week with more words. However could you please clarify a couple of things for me? 1/ Why do you (sometimes) call the commit message a "change log". When I see the term "change log" in the context of a patch, my first thought is that it it means a log of changes that have been made to the patch - typically through the review cycle. But that isn't what you mean. This has confused me a couple of times. 2/ Why don't you consider the first line of the commit message to be part of the commit message? Why is duplication required? (You said "some of the patches don't have commit message[s]", which isn't true, though some of the messages are only one line). Maybe the requirements on the commit message (including this duplication) could be included in the "Staging trees" section of Documentation/process/2.process.rst. That file only lists the TODO and "doesn't break the build" requirements. It doesn't metion the "someone to work on it" requirement. That might seem obvious, but it doesn't hurt to be explicit. Thanks, NeilBrown signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue
Resending the email because it was sent only to Greg. Context: In my previous patch, I had removed an extra newline at the end of the code. My Reply: It was unintentional, but does it violate any coding or other standard? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] drivers:staging:android:ashmem: Changing return type from int to loff_t
Changing return type from int to loff_t. Actual return type of the function (vfs_llseek) is loff_t (long long). Here due to implicit converion from long long to int, result will be implementation defined. Signed-off-by: Rohit Kumar --- drivers/staging/android/ashmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c index 86580b6..a1a0025 100644 --- a/drivers/staging/android/ashmem.c +++ b/drivers/staging/android/ashmem.c @@ -321,7 +321,7 @@ static ssize_t ashmem_read_iter(struct kiocb *iocb, struct iov_iter *iter) static loff_t ashmem_llseek(struct file *file, loff_t offset, int origin) { struct ashmem_area *asma = file->private_data; - int ret; + loff_t ret; mutex_lock(&ashmem_mutex); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[no subject]
de...@driverdev.osuosl.org Bcc: Subject: [PATCH v2] drivers:staging:android:ashmem: Changing return type from int to loff_t Reply-To: Changing return type from int to loff_t. Actual return type of the function (vfs_llseek) is loff_t (long long). Here due to implicit converion from long long to int, result will be implementation defined. Signed-off-by: Rohit Kumar --- drivers/staging/android/ashmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c index 86580b6..a1a0025 100644 --- a/drivers/staging/android/ashmem.c +++ b/drivers/staging/android/ashmem.c @@ -321,7 +321,7 @@ static ssize_t ashmem_read_iter(struct kiocb *iocb, struct iov_iter *iter) static loff_t ashmem_llseek(struct file *file, loff_t offset, int origin) { struct ashmem_area *asma = file->private_data; - int ret; + loff_t ret; mutex_lock(&ashmem_mutex); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 2/2] media: staging/imx: fill vb2_v4l2_buffer sequence entry
- enables gstreamer v4l2src lost frame detection, e.g: 0:00:08.685185668 348 0x54f520 WARN v4l2src gstv4l2src.c:970:gst_v4l2src_create: lost frames detected: count = 141 - ts: 0:00:08.330177332 - fixes v4l2-compliance test failure: Streaming ioctls: test read/write: OK (Not Supported) Video Capture: Buffer: 0 Sequence: 0 Field: None Timestamp: 92.991450s Buffer: 1 Sequence: 0 Field: None Timestamp: 93.008135s fail: v4l2-test-buffers.cpp(294): (int)g_sequence() < seq.last_seq + 1 fail: v4l2-test-buffers.cpp(707): buf.check(q, last_seq) Signed-off-by: Peter Seiderer --- Changes in v2: - fill vb2_v4l2_buffer sequence entry in imx-ic-prpencvf too (suggested by Steve Longerbeam) Changes in v3: - add changelog (suggested by Greg Kroah-Hartman, Fabio Estevam and Dan Carpenter) and patch history - use u32 (instead of __u32) (suggested by Dan Carpenter) - let sequence counter start with zero, keeping v4l2-compliance testing happy (needs additional setting of field to a valid value, patch will follow soon) Changes in v4: - add v4l2-compliance test failure to changelog - reorder frame_sequence increment and assignement to avoid -1 as start value (suggeted by Steve Longerbeam) --- drivers/staging/media/imx/imx-ic-prpencvf.c | 4 drivers/staging/media/imx/imx-media-csi.c | 4 2 files changed, 8 insertions(+) diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c index ffeb017c73b2..28f41caba05d 100644 --- a/drivers/staging/media/imx/imx-ic-prpencvf.c +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c @@ -103,6 +103,7 @@ struct prp_priv { int nfb4eof_irq; int stream_count; + u32 frame_sequence; /* frame sequence counter */ bool last_eof; /* waiting for last EOF at stream off */ bool nfb4eof;/* NFB4EOF encountered during streaming */ struct completion last_eof_comp; @@ -211,12 +212,14 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct ipuv3_channel *ch) done = priv->active_vb2_buf[priv->ipu_buf_num]; if (done) { done->vbuf.field = vdev->fmt.fmt.pix.field; + done->vbuf.sequence = priv->frame_sequence; vb = &done->vbuf.vb2_buf; vb->timestamp = ktime_get_ns(); vb2_buffer_done(vb, priv->nfb4eof ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); } + priv->frame_sequence++; priv->nfb4eof = false; /* get next queued buffer */ @@ -638,6 +641,7 @@ static int prp_start(struct prp_priv *priv) /* init EOF completion waitq */ init_completion(&priv->last_eof_comp); + priv->frame_sequence = 0; priv->last_eof = false; priv->nfb4eof = false; diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 5f69117b5811..3f2ce05848f3 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -111,6 +111,7 @@ struct csi_priv { struct v4l2_ctrl_handler ctrl_hdlr; int stream_count; /* streaming counter */ + u32 frame_sequence; /* frame sequence counter */ bool last_eof; /* waiting for last EOF at stream off */ bool nfb4eof;/* NFB4EOF encountered during streaming */ struct completion last_eof_comp; @@ -237,12 +238,14 @@ static void csi_vb2_buf_done(struct csi_priv *priv) done = priv->active_vb2_buf[priv->ipu_buf_num]; if (done) { done->vbuf.field = vdev->fmt.fmt.pix.field; + done->vbuf.sequence = priv->frame_sequence; vb = &done->vbuf.vb2_buf; vb->timestamp = ktime_get_ns(); vb2_buffer_done(vb, priv->nfb4eof ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); } + priv->frame_sequence++; priv->nfb4eof = false; /* get next queued buffer */ @@ -544,6 +547,7 @@ static int csi_idmac_start(struct csi_priv *priv) /* init EOF completion waitq */ init_completion(&priv->last_eof_comp); + priv->frame_sequence = 0; priv->last_eof = false; priv->nfb4eof = false; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 1/2] media: staging/imx: fill vb2_v4l2_buffer field entry
- fixes gstreamer v4l2src warning: 0:00:00.716640334 349 0x164f720 WARN v4l2bufferpool gstv4l2bufferpool.c:1195:gst_v4l2_buffer_pool_dqbuf: Driver should never set v4l2_buffer.field to ANY - fixes v4l2-compliance test failure: Streaming ioctls: test read/write: OK (Not Supported) Video Capture: Buffer: 0 Sequence: 0 Field: Any Timestamp: 58.383658s fail: v4l2-test-buffers.cpp(297): g_field() == V4L2_FIELD_ANY Signed-off-by: Peter Seiderer --- Changes in v4: - new patch (put first because patch is needed to advance with the v4l2-compliance test), thanks to Philipp Zabel for suggested solution for the right field value source --- drivers/staging/media/imx/imx-ic-prpencvf.c | 1 + drivers/staging/media/imx/imx-media-csi.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c index ae453fd422f0..ffeb017c73b2 100644 --- a/drivers/staging/media/imx/imx-ic-prpencvf.c +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c @@ -210,6 +210,7 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct ipuv3_channel *ch) done = priv->active_vb2_buf[priv->ipu_buf_num]; if (done) { + done->vbuf.field = vdev->fmt.fmt.pix.field; vb = &done->vbuf.vb2_buf; vb->timestamp = ktime_get_ns(); vb2_buffer_done(vb, priv->nfb4eof ? diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 5a195f80a24d..5f69117b5811 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -236,6 +236,7 @@ static void csi_vb2_buf_done(struct csi_priv *priv) done = priv->active_vb2_buf[priv->ipu_buf_num]; if (done) { + done->vbuf.field = vdev->fmt.fmt.pix.field; vb = &done->vbuf.vb2_buf; vb->timestamp = ktime_get_ns(); vb2_buffer_done(vb, priv->nfb4eof ? -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/4] staging: ks7010: remove unnecessary brackets in single statement block
This patch fix the following checkpatch warning: braces {} are not necessary for single statement blocks Signed-off-by: Sergio Paracuellos --- drivers/staging/ks7010/ks_hostif.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index a5ff033..d644a1d 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1297,9 +1297,8 @@ static __le16 ks_wlan_cap(struct ks_wlan_private *priv) { u16 capability = 0x; - if (priv->reg.preamble == SHORT_PREAMBLE) { + if (priv->reg.preamble == SHORT_PREAMBLE) capability |= WLAN_CAPABILITY_SHORT_PREAMBLE; - } capability &= ~(WLAN_CAPABILITY_PBCC); /* pbcc not support */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/4] staging: ks7010: some cleanups and replaces
The following patch series includes some cleanups of useless traces as well as some replaces in order to use preferred macros for debugging and others. v2: * Review deleted DPRINTK traces * Minor checkpatch warning cleanup Sergio Paracuellos (4): staging: ks7010: remove useless DPRINTK traces staging: ks7010: replace DPRINTK traces in favour of netdev_* staging: ks7010: replace KS_WLAN_DEBUG with DEBUG preprocessor directive staging: ks7010: remove unnecessary brackets in single statement block drivers/staging/ks7010/ks7010_sdio.c | 151 ++ drivers/staging/ks7010/ks_hostif.c | 288 ++- drivers/staging/ks7010/ks_wlan.h | 10 -- drivers/staging/ks7010/ks_wlan_net.c | 90 +++ 4 files changed, 145 insertions(+), 394 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/4] staging: ks7010: remove useless DPRINTK traces
This commit removes some useless traces in some source files Signed-off-by: Sergio Paracuellos --- drivers/staging/ks7010/ks7010_sdio.c | 58 -- drivers/staging/ks7010/ks_hostif.c | 147 +++ drivers/staging/ks7010/ks_wlan_net.c | 56 + 3 files changed, 10 insertions(+), 251 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 7de78d1..beb689b 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -112,8 +112,6 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv) { int ret; - DPRINTK(4, "\n"); - /* clear request */ atomic_set(&priv->sleepstatus.doze_request, 0); @@ -123,11 +121,8 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv) DPRINTK(1, " error : GCR_B\n"); goto set_sleep_mode; } - DPRINTK(3, "sleep_mode=SLP_SLEEP\n"); atomic_set(&priv->sleepstatus.status, 1); priv->last_doze = jiffies; - } else { - DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode); } set_sleep_mode: @@ -138,8 +133,6 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv) { int ret; - DPRINTK(4, "\n"); - /* clear request */ atomic_set(&priv->sleepstatus.wakeup_request, 0); @@ -149,12 +142,9 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv) DPRINTK(1, " error : WAKEUP\n"); goto set_sleep_mode; } - DPRINTK(4, "wake up : WAKEUP\n"); atomic_set(&priv->sleepstatus.status, 0); priv->last_wakeup = jiffies; ++priv->wakeup_count; - } else { - DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode); } set_sleep_mode: @@ -165,19 +155,13 @@ void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv) { int ret; - DPRINTK(4, "\n"); if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ); if (ret) DPRINTK(1, " error : WAKEUP\n"); - else - DPRINTK(4, "wake up : WAKEUP\n"); priv->last_wakeup = jiffies; ++priv->wakeup_count; - } else { - DPRINTK(1, "psstatus=%d\n", - atomic_read(&priv->psstatus.status)); } } @@ -228,7 +212,6 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv) goto queue_delayed_work; } atomic_set(&priv->psstatus.status, PS_SNOOZE); - DPRINTK(3, "psstatus.status=PS_SNOOZE\n"); return; @@ -288,7 +271,6 @@ static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer, hdr = (struct hostif_hdr *)buffer; - DPRINTK(4, "size=%d\n", hdr->size); if (le16_to_cpu(hdr->event) < HIF_DATA_REQ || le16_to_cpu(hdr->event) > HIF_REQ_MAX) { DPRINTK(1, "unknown event=%04X\n", hdr->event); @@ -315,7 +297,6 @@ static void tx_device_task(struct ks_wlan_private *priv) struct tx_device_buffer *sp; int ret; - DPRINTK(4, "\n"); if (cnt_txqbody(priv) <= 0 || atomic_read(&priv->psstatus.status) == PS_SNOOZE) return; @@ -358,7 +339,6 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size, priv->hostt.buff[priv->hostt.qtail] = le16_to_cpu(hdr->event); priv->hostt.qtail = (priv->hostt.qtail + 1) % SME_EVENT_BUFF_SIZE; - DPRINTK(4, "event=%04X\n", hdr->event); spin_lock(&priv->tx_dev.tx_dev_lock); result = enqueue_txdev(priv, p, size, complete_handler, skb); spin_unlock(&priv->tx_dev.tx_dev_lock); @@ -374,8 +354,6 @@ static void rx_event_task(unsigned long dev) struct ks_wlan_private *priv = (struct ks_wlan_private *)dev; struct rx_device_buffer *rp; - DPRINTK(4, "\n"); - if (cnt_rxqbody(priv) > 0 && priv->dev_state >= DEVICE_STATE_BOOT) { rp = &priv->rx_dev.rx_dev_buff[priv->rx_dev.qhead]; hostif_receive(priv, rp->data, rp->size); @@ -393,8 +371,6 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size) struct hostif_hdr *hdr; unsigned short event = 0; - DPRINTK(4, "\n"); - /* receive data */ if (cnt_rxqbody(priv) >= (RX_DEVICE_BUFF_SIZE - 1)) { DPRINTK(1, "rx buffer overflow\n"); @@ -450,8 +426,6 @@ static void ks7010_rw_function(struct work_struct *work) priv = container_of(work, struct ks_wlan_private, rw_dwork.work); - DPRINTK(4, "\n"); - /* wait after DOZE */ if (time_aft
[PATCH v2 2/4] staging: ks7010: replace DPRINTK traces in favour of netdev_*
This commit removes custom defined DPRINTK macro and replaces all the associated debug and other traces for preferred ones netdev_*. Signed-off-by: Sergio Paracuellos --- drivers/staging/ks7010/ks7010_sdio.c | 84 ++--- drivers/staging/ks7010/ks_hostif.c | 138 ++- drivers/staging/ks7010/ks_wlan.h | 10 --- drivers/staging/ks7010/ks_wlan_net.c | 34 - 4 files changed, 130 insertions(+), 136 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index beb689b..50a7a15 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -118,7 +118,7 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv) if (atomic_read(&priv->sleepstatus.status) == 0) { ret = ks7010_sdio_writeb(priv, GCR_B, GCR_B_DOZE); if (ret) { - DPRINTK(1, " error : GCR_B\n"); + netdev_err(priv->net_dev, " error : GCR_B\n"); goto set_sleep_mode; } atomic_set(&priv->sleepstatus.status, 1); @@ -139,7 +139,7 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv) if (atomic_read(&priv->sleepstatus.status) == 1) { ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ); if (ret) { - DPRINTK(1, " error : WAKEUP\n"); + netdev_err(priv->net_dev, " error : WAKEUP\n"); goto set_sleep_mode; } atomic_set(&priv->sleepstatus.status, 0); @@ -158,7 +158,7 @@ void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv) if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) { ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ); if (ret) - DPRINTK(1, " error : WAKEUP\n"); + netdev_err(priv->net_dev, " error : WAKEUP\n"); priv->last_wakeup = jiffies; ++priv->wakeup_count; @@ -185,11 +185,11 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv) if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) return; - DPRINTK(5, "\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n", - atomic_read(&priv->psstatus.status), - atomic_read(&priv->psstatus.confirm_wait), - atomic_read(&priv->psstatus.snooze_guard), - cnt_txqbody(priv)); + netdev_dbg(priv->net_dev, "\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n", + atomic_read(&priv->psstatus.status), + atomic_read(&priv->psstatus.confirm_wait), + atomic_read(&priv->psstatus.snooze_guard), + cnt_txqbody(priv)); if (atomic_read(&priv->psstatus.confirm_wait) || atomic_read(&priv->psstatus.snooze_guard) || @@ -200,7 +200,7 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv) ret = ks7010_sdio_readb(priv, INT_PENDING, &byte); if (ret) { - DPRINTK(1, " error : INT_PENDING\n"); + netdev_err(priv->net_dev, " error : INT_PENDING\n"); goto queue_delayed_work; } if (byte) @@ -208,7 +208,7 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv) ret = ks7010_sdio_writeb(priv, GCR_B, GCR_B_DOZE); if (ret) { - DPRINTK(1, " error : GCR_B\n"); + netdev_err(priv->net_dev, " error : GCR_B\n"); goto queue_delayed_work; } atomic_set(&priv->psstatus.status, PS_SNOOZE); @@ -240,7 +240,7 @@ static int enqueue_txdev(struct ks_wlan_private *priv, unsigned char *p, } if ((TX_DEVICE_BUFF_SIZE - 1) <= cnt_txqbody(priv)) { - DPRINTK(1, "tx buffer overflow\n"); + netdev_err(priv->net_dev, "tx buffer overflow\n"); ret = -EOVERFLOW; goto err_complete; } @@ -273,19 +273,19 @@ static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer, if (le16_to_cpu(hdr->event) < HIF_DATA_REQ || le16_to_cpu(hdr->event) > HIF_REQ_MAX) { - DPRINTK(1, "unknown event=%04X\n", hdr->event); + netdev_err(priv->net_dev, "unknown event=%04X\n", hdr->event); return 0; } ret = ks7010_sdio_write(priv, DATA_WINDOW, buffer, size); if (ret) { - DPRINTK(1, " write error : retval=%d\n", ret); + netdev_err(priv->net_dev, " write error : retval=%d\n", ret); return ret; } ret = ks7010_sdio_writeb(priv, WRITE_STATUS, REG_STATUS_BUSY); if (ret) { - DPRINTK(1, " error : WRITE_STATUS\n");
[PATCH v2 3/4] staging: ks7010: replace KS_WLAN_DEBUG with DEBUG preprocessor directive
This commit replaces custom KS_WLAN_DEBUG which is not being used anymore in favour of DEBUG which is the one included when debugging is enabled. Signed-off-by: Sergio Paracuellos --- drivers/staging/ks7010/ks7010_sdio.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index 50a7a15..b8f55a1 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -385,11 +385,10 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size) /* length check */ if (size > 2046 || size == 0) { -#ifdef KS_WLAN_DEBUG - if (KS_WLAN_DEBUG > 5) - print_hex_dump_bytes("INVALID DATA dump: ", -DUMP_PREFIX_OFFSET, -rx_buffer->data, 32); +#ifdef DEBUG + print_hex_dump_bytes("INVALID DATA dump: ", +DUMP_PREFIX_OFFSET, +rx_buffer->data, 32); #endif ret = ks7010_sdio_writeb(priv, READ_STATUS, REG_STATUS_IDLE); if (ret) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue
Fixed coding style issue. Signed-off-by: Pratik Jain --- drivers/staging/comedi/drivers/ni_atmio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio.c b/drivers/staging/comedi/drivers/ni_atmio.c index b9e9ab548c4b..4e27a2959b64 100644 --- a/drivers/staging/comedi/drivers/ni_atmio.c +++ b/drivers/staging/comedi/drivers/ni_atmio.c @@ -226,8 +226,8 @@ static int ni_isapnp_find_board(struct pnp_dev **dev) for (i = 0; i < ARRAY_SIZE(ni_boards); i++) { isapnp_dev = pnp_find_dev(NULL, ISAPNP_VENDOR('N', 'I', 'C'), - ISAPNP_FUNCTION(ni_boards[i]. - isapnp_id), NULL); + ISAPNP_FUNCTION(ni_boards[i].isapnp_id), + NULL); if (!isapnp_dev || !isapnp_dev->card) continue; -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4 1/2] PCI: hv: Serialize the present and eject work items
> From: Dexuan Cui > > From: Lorenzo Pieralisi > > I need to know either what commit you are fixing (ie Fixes: tag - which > > is preferrable) or you tell me which kernel versions we are targeting > > for the stable backport. > > Lorenzo > > Sorry. Here I was hesitant to add a "Fixes:" because the bug was there the > first > day > when the driver was introduced. > > Please use > Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft > Hyper-V VMs") > or > Cc: # v4.6+ BTW, the bug here is a race condtion which couldn't be easily hit in the past, probably because most of the time only one PCI device was only added into the VM once. But now it's becoming typical that a VM can have 4 GPU devices so we start to notice this bug. With 7 Mellanox VFs assigned to a VM, we can easily reproduce the bug by "hot-remove and hot-add VFs" test: general protection fault: [#1] SMP ... Workqueue: events hv_eject_device_work [pci_hyperv] task: 8800ed5e5400 ti: 8800ee674000 task.ti: 8800ee674000 RIP: 0010:[] ... hv_eject_device_work+0xbe/0x160 [pci_hyperv] ... Call Trace: [] ? __schedule+0x3b6/0xa30 [] process_one_work+0x165/0x480 [] worker_thread+0x4b/0x4c0 [] ? process_one_work+0x480/0x480 [] kthread+0xe5/0x100 [] ? kthread_create_on_node+0x1e0/0x1e0 [] ret_from_fork+0x3f/0x70 [] ? kthread_create_on_node+0x1e0/0x1e0 Code: ... RIP [] hv_eject_device_work+0xbe/0x160 [pci_hyperv] ... BUG: unable to handle kernel paging request at ffd8 IP: [] kthread_data+0x10/0x20 PGD 1e0d067 PUD 1e0f067 PMD 0 Oops: [#2] SMP Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
Apologies for not aligning my reply, I just realized after looking into the mail archive in LKML. I have aligned the replay now. On 3/15/18, 10:56 AM, "Sridhar Pitchai" wrote: Hi Lorenzo, Answering the question inline. Kindly let me know if it clarifies. I will send out another patch after we agree on the clarification. Thanks Sridhar Pitchai -Original Message- From: Lorenzo Pieralisi Sent: Thursday, March 15, 2018 5:05 AM To: Sridhar Pitchai Cc: Bjorn Helgaas ; Jake Oshins ; Haiyang Zhang ; Stephen Hemminger ; Dexuan Cui ; KY Srinivasan ; Michael Kelley (EOSG) ; de...@linuxdriverproject.org; linux-...@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption On Thu, Mar 15, 2018 at 12:03:07AM +, Sridhar Pitchai wrote: Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even "Whenever a PCI bus is added" Sridhar>> yes with that when a first device is added to the bus, it overrides bus domain ID with the device serial number. Sometime this can result in BUS ID not Define "Sometime". Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a unique ID for it. But, that unique BUS ID is replaced with device serial number. 0 is a valid device serial number, and if there exists a PCI bus with domain ID 0 (Gen 1 version of hyperV VM have this for para virtual devices), this will result in PCI bus id not being unique. being unique. In this case, when PCI_BUS and a device to bus is added, the first device overwrites the bus domain ID to the device serial number, which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI s/exsist/exist Sridhar>> yes bus addition fails. This patch make sure when a device is added to a bus, it never updated the bus domain ID. s/updated/updates Sridhar >> yes Since we have the transparent SRIOV mode now, the short VF device name is no longer needed. I still do not understand what this means and how it is related to the patch below, it may be clear to you, it is not to me, at all. Sridhar >> the patch below, was introduced to make the device name small, by taking only 16bits of the serial number. Since we are not going to have the serial number updated to the BUS id, this has to be removed. Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain") Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain") Sridhr >> yes I asked you an explicit question. Commit above was added for a reason I assume. This patch implies that kernel has been broken since v4.11 which is almost a year ago and nobody every noticed ? Or there are systems where commit above is _necessary_ and this patch would break them ? I want a detailed explanation that highlights *why* it is safe to apply this patch and send it to stable kernels, commit log above won't do. Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child device when it is added. This cannot produce a unique domain ID all the time. Here in the bug, we see the collision between the serial number and already existing PCI bus. The cleaner way is never touch the domain ID provided by hyperV during the PCI bus creation. As long as hyperV make sure it provides a unique domain ID for the PCI for a VM it will not break, and HyperV will guarantees that the domain for the PCI bus for a given VM will be always unique. The original patch was also intending to have a unique domain ID for the PCI bus, by taking the serial number of the device, but it is not sufficient, when the device serial number is number which is the domain ID of the existing PCI bus. With the current kernel we can repro this issue by adding a device with a serial number matching the existing PCI bus domain id. (in this case that happens to be zero). Thanks, Lorenzo Cc: sta...@vger.kernel.org Signed-off-by: Sridhar Pitchai --- Changes in v3: * fix the commit comment. [KY Srinivasan, Michael Kelley] --- drivers/pci/host/pci-hyperv.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 2faf38e..ac67e56 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus, get_pcichild(hpdev, hv_pcidev_ref_childlist); spin_lock_irqsave(&hbus->device_list_lock, flags); - /* -* When a device is being added to the bus, we set the PCI domain -* number to be the device serial number, which is non-zero and -* unique on the same VM. The serial numbers start with 1, and -* increase by 1 for each device. So device names including this -* can have shorter names than based on the bus instance UUID. -* Only the first device serial number is used for domain, so the -* domain number will not change after the first device is added. -*/
[PATCH] staging: android: ion: Update wording in drivers/staging/android/ion/Kconfig
Changes the usage of the word 'Chose' to 'Choose' in the ION Memory Manager Kconfig. Signed-off-by: Phillip Potter --- --- a/drivers/staging/android/ion/Kconfig +++ b/drivers/staging/android/ion/Kconfig @@ -4,7 +4,7 @@ menuconfig ION select GENERIC_ALLOCATOR select DMA_SHARED_BUFFER ---help--- - Chose this option to enable the ION Memory Manager, + Choose this option to enable the ION Memory Manager, used by Android to efficiently allocate buffers from userspace that can be shared between drivers. If you're not using Android its probably safe to ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
Hi Lorenzo, Answering the question inline. Kindly let me know if it clarifies. I will send out another patch after we agree on the clarification. Thanks Sridhar Pitchai -Original Message- From: Lorenzo Pieralisi Sent: Thursday, March 15, 2018 5:05 AM To: Sridhar Pitchai Cc: Bjorn Helgaas ; Jake Oshins ; Haiyang Zhang ; Stephen Hemminger ; Dexuan Cui ; KY Srinivasan ; Michael Kelley (EOSG) ; de...@linuxdriverproject.org; linux-...@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption On Thu, Mar 15, 2018 at 12:03:07AM +, Sridhar Pitchai wrote: > Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even "Whenever a PCI bus is added" Sridhar>> yes > with that when a first device is added to the bus, it overrides bus domain > ID with the device serial number. Sometime this can result in BUS ID not Define "Sometime". Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a unique ID for it. But, that unique BUS ID is replaced with device serial number. 0 is a valid device serial number, and if there exists a PCI bus with domain ID 0 (Gen 1 version of hyperV VM have this for para virtual devices), this will result in PCI bus id not being unique. > being unique. In this case, when PCI_BUS and a device to bus is added, the > first device overwrites the bus domain ID to the device serial number, > which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI s/exsist/exist Sridhar>> yes > bus addition fails. This patch make sure when a device is added to a bus, > it never updated the bus domain ID. s/updated/updates Sridhar >> yes > Since we have the transparent SRIOV mode now, the short VF device name > is no longer needed. I still do not understand what this means and how it is related to the patch below, it may be clear to you, it is not to me, at all. Sridhar >> the patch below, was introduced to make the device name small, by taking only 16bits of the serial number. Since we are not going to have the serial number updated to the BUS id, this has to be removed. > Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain") Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain") Sridhr >> yes I asked you an explicit question. Commit above was added for a reason I assume. This patch implies that kernel has been broken since v4.11 which is almost a year ago and nobody every noticed ? Or there are systems where commit above is _necessary_ and this patch would break them ? I want a detailed explanation that highlights *why* it is safe to apply this patch and send it to stable kernels, commit log above won't do. Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child device when it is added. This cannot produce a unique domain ID all the time. Here in the bug, we see the collision between the serial number and already existing PCI bus. The cleaner way is never touch the domain ID provided by hyperV during the PCI bus creation. As long as hyperV make sure it provides a unique domain ID for the PCI for a VM it will not break, and HyperV will guarantees that the domain for the PCI bus for a given VM will be always unique. The original patch was also intending to have a unique domain ID for the PCI bus, by taking the serial number of the device, but it is not sufficient, when the device serial number is number which is the domain ID of the existing PCI bus. With the current kernel we can repro this issue by adding a device with a serial number matching the existing PCI bus domain id. (in this case that happens to be zero). Thanks, Lorenzo > Cc: sta...@vger.kernel.org > Signed-off-by: Sridhar Pitchai > --- > > Changes in v3: > * fix the commit comment. [KY Srinivasan, Michael Kelley] > --- > drivers/pci/host/pci-hyperv.c | 11 --- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 2faf38e..ac67e56 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct > hv_pcibus_device *hbus, > get_pcichild(hpdev, hv_pcidev_ref_childlist); > spin_lock_irqsave(&hbus->device_list_lock, flags); > > - /* > - * When a device is being added to the bus, we set the PCI domain > - * number to be the device serial number, which is non-zero and > - * unique on the same VM. The serial numbers start with 1, and > - * increase by 1 for each device. So device names including this > - * can have shorter names than based on the bus instance UUID. > - * Only the first device serial number is used for domain, so the > - * domain number will not change after the first device is added. > - */ > - if (list_empty(&hbus->children)) > - hbus->sysdata.domain = desc->ser; > list_add_tail
RE: [PATCH v4 1/2] PCI: hv: Serialize the present and eject work items
> From: Lorenzo Pieralisi > Sent: Thursday, March 15, 2018 10:02 > On Thu, Mar 15, 2018 at 02:20:53PM +, Dexuan Cui wrote: > > When we hot-remove the device, we first receive a PCI_EJECT message and > > then receive a PCI_BUS_RELATIONS message with bus_rel->device_count == > 0. > > > > The first message is offloaded to (), and the second > > is offloaded to pci_devices_present_work(). Both the paths can be running > > list_del(&hpdev->list_entry), causing general protection fault, because > > system_wq can run them concurrently. > > > > The patch eliminates the race condition. > > > > Cc: sta...@vger.kernel.org > > I need to know either what commit you are fixing (ie Fixes: tag - which > is preferrable) or you tell me which kernel versions we are targeting > for the stable backport. > > Thanks, > Lorenzo Sorry. Here I was hesitant to add a "Fixes:" because the bug was there the first day when the driver was introduced. Please use Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs") or Cc: # v4.6+ Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send
On Thu, 15 Mar 2018 17:24:13 +0100 Mohammed Gamal wrote: > On Wed, 2018-03-14 at 10:22 +0100, Mohammed Gamal wrote: > > On Tue, 2018-03-13 at 12:35 -0700, Stephen Hemminger wrote: > > > On Tue, 13 Mar 2018 20:06:50 +0100 > > > Mohammed Gamal wrote: > > > > > > > Dring high network traffic changes to network interface > > > > parameters > > > > such as number of channels or MTU can cause a kernel panic with a > > > > NULL > > > > pointer dereference. This is due to netvsc_device_remove() being > > > > called and deallocating the channel ring buffers, which can then > > > > be > > > > accessed by netvsc_send_pkt() before they're allocated on calling > > > > netvsc_device_add() > > > > > > > > The patch fixes this problem by checking the channel state and > > > > returning > > > > ENODEV if not yet opened. We also move the call to > > > > hv_ringbuf_avail_percent() > > > > which may access the uninitialized ring buffer. > > > > > > > > Signed-off-by: Mohammed Gamal > > > > --- > > > > drivers/net/hyperv/netvsc.c | 5 +++-- > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/net/hyperv/netvsc.c > > > > b/drivers/net/hyperv/netvsc.c > > > > index 0265d70..44a8358 100644 > > > > --- a/drivers/net/hyperv/netvsc.c > > > > +++ b/drivers/net/hyperv/netvsc.c > > > > @@ -757,7 +757,7 @@ static inline int netvsc_send_pkt( > > > > struct netdev_queue *txq = netdev_get_tx_queue(ndev, > > > > packet->q_idx); > > > > u64 req_id; > > > > int ret; > > > > - u32 ring_avail = hv_ringbuf_avail_percent(&out_channel- > > > > > outbound); > > > > > > > > + u32 ring_avail; > > > > > > > > nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT; > > > > if (skb) > > > > @@ -773,7 +773,7 @@ static inline int netvsc_send_pkt( > > > > > > > > req_id = (ulong)skb; > > > > > > > > - if (out_channel->rescind) > > > > + if (out_channel->rescind || out_channel->state != > > > > CHANNEL_OPENED_STATE) > > > > return -ENODEV; > > > > > > > > if (packet->page_buf_cnt) { > > > > @@ -791,6 +791,7 @@ static inline int netvsc_send_pkt( > > > > VMBUS_DATA_PACKET_FLAG_CO > > > > MP > > > > LETION_REQUESTED); > > > > } > > > > > > > > + ring_avail = hv_ringbuf_avail_percent(&out_channel- > > > > > outbound); > > > > > > > > if (ret == 0) { > > > > atomic_inc_return(&nvchan->queue_sends); > > > > > > > > > > Thanks for your patch. Yes there are races with the current update > > > logic. The root cause goes higher up in the flow; the send queues > > > should > > > be stopped before netvsc_device_remove is called. Solving it where > > > you tried > > > to is racy and not going to work reliably. > > > > > > Network patches should go to net...@vger.kernel.org > > > > > > You can't move the ring_avail check until after the > > > vmbus_sendpacket > > > because > > > that will break the flow control logic. > > > > > > > Why? I don't see ring_avail being used before that point. > > Ah, stupid me. vmbus_sendpacket() will write to the ring buffer and > that means that ring_avail value will be different than the expected. > > > > > > Instead, you should just move the avail_read check until just after > > > the existing rescind > > > check. > > > > > > Also, you shouldn't need to check for OPENED_STATE, just rescind is > > > enough. > > > > That rarely mitigated the race. channel->rescind flag is set on vmbus > > exit - called on module unload - and when a rescind offer is received > > from the host, which AFAICT doesn't happen on every call to > > netvsc_device_remove, so it's quite possible that the ringbuffer is > > accessed before it's allocated again on channel open and hence the > > check for OPENED_STAT - which is only set after all vmbus data is > > initialized. > > > > Perhaps I haven't been clear enough. The NULL pointer dereference > happens in the call to hv_ringbuf_avail_percent() which is used to > calculate ring_avail. > > So we need to stop the queues before calling it if the channel's ring > buffers haven't been allocated yet, but OTOH we should only stop the > queues based upon the value of ring_avail, so this leads into a chicken > and egg situation. > > Is my observation here correct? Please correct me if I am wrong, > Stephen. This is a far more drastic work of the shutdown logic which I am still working on. The current netvsc driver is not doing a good job of handling concurrent send/receives during ring changes. From a22da18b41ad5024340dddcc989d918987836f6d Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Tue, 6 Feb 2018 15:05:19 -0800 Subject: [PATCH] hv_netvsc: common detach logic Make common function for detaching internals of device during changes to MTU and RSS. Make sure no more packets are transmitted and all packets have been received before doing device t
Re: [PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue
On Thu, Mar 15, 2018 at 11:00:40PM +0530, Pratik Jain wrote: > Fixed coding style issue. > > Signed-off-by: Pratik Jain > --- > drivers/staging/comedi/drivers/ni_atmio.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/comedi/drivers/ni_atmio.c > b/drivers/staging/comedi/drivers/ni_atmio.c > index b9e9ab548c4b..e82fbe987dd8 100644 > --- a/drivers/staging/comedi/drivers/ni_atmio.c > +++ b/drivers/staging/comedi/drivers/ni_atmio.c > @@ -226,8 +226,8 @@ static int ni_isapnp_find_board(struct pnp_dev **dev) > for (i = 0; i < ARRAY_SIZE(ni_boards); i++) { > isapnp_dev = pnp_find_dev(NULL, > ISAPNP_VENDOR('N', 'I', 'C'), > - ISAPNP_FUNCTION(ni_boards[i]. > - isapnp_id), NULL); > + > ISAPNP_FUNCTION(ni_boards[i].isapnp_id), > + NULL); > > if (!isapnp_dev || !isapnp_dev->card) > continue; > @@ -356,4 +356,3 @@ module_comedi_driver(ni_atmio_driver); > MODULE_AUTHOR("Comedi http://www.comedi.org";); > MODULE_DESCRIPTION("Comedi low-level driver"); > MODULE_LICENSE("GPL"); > - Why did you also delete this last line? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue
Fixed coding style issue. Signed-off-by: Pratik Jain --- drivers/staging/comedi/drivers/ni_atmio.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_atmio.c b/drivers/staging/comedi/drivers/ni_atmio.c index b9e9ab548c4b..e82fbe987dd8 100644 --- a/drivers/staging/comedi/drivers/ni_atmio.c +++ b/drivers/staging/comedi/drivers/ni_atmio.c @@ -226,8 +226,8 @@ static int ni_isapnp_find_board(struct pnp_dev **dev) for (i = 0; i < ARRAY_SIZE(ni_boards); i++) { isapnp_dev = pnp_find_dev(NULL, ISAPNP_VENDOR('N', 'I', 'C'), - ISAPNP_FUNCTION(ni_boards[i]. - isapnp_id), NULL); + ISAPNP_FUNCTION(ni_boards[i].isapnp_id), + NULL); if (!isapnp_dev || !isapnp_dev->card) continue; @@ -356,4 +356,3 @@ module_comedi_driver(ni_atmio_driver); MODULE_AUTHOR("Comedi http://www.comedi.org";); MODULE_DESCRIPTION("Comedi low-level driver"); MODULE_LICENSE("GPL"); - -- 2.16.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:mt29f_spinand: MT29F2G failing as only 16 bits used for addressing.
From: Palle Christensen For NAND flash chips with more than 1Gbit (e.g. MT29F2G) more than 16 bits are necessary to address the correct page. Signed-off-by: Palle Christensen --- drivers/staging/mt29f_spinand/mt29f_spinand.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c index 264ad36..d20284b 100644 --- a/drivers/staging/mt29f_spinand/mt29f_spinand.c +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c @@ -316,6 +316,7 @@ static int spinand_read_page_to_cache(struct spi_device *spi_nand, u16 page_id) row = page_id; cmd.cmd = CMD_READ; cmd.n_addr = 3; + cmd.addr[0] = (u8)((row & 0xff) >> 16); cmd.addr[1] = (u8)((row & 0xff00) >> 8); cmd.addr[2] = (u8)(row & 0x00ff); @@ -464,6 +465,7 @@ static int spinand_program_execute(struct spi_device *spi_nand, u16 page_id) row = page_id; cmd.cmd = CMD_PROG_PAGE_EXC; cmd.n_addr = 3; + cmd.addr[0] = (u8)((row & 0xff) >> 16); cmd.addr[1] = (u8)((row & 0xff00) >> 8); cmd.addr[2] = (u8)(row & 0x00ff); @@ -579,6 +581,7 @@ static int spinand_erase_block_erase(struct spi_device *spi_nand, u16 block_id) row = block_id; cmd.cmd = CMD_ERASE_BLK; cmd.n_addr = 3; + cmd.addr[0] = (u8)((row & 0xff) >> 16); cmd.addr[1] = (u8)((row & 0xff00) >> 8); cmd.addr[2] = (u8)(row & 0x00ff); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/2] PCI: hv: Serialize the present and eject work items
On Thu, Mar 15, 2018 at 02:20:53PM +, Dexuan Cui wrote: > When we hot-remove the device, we first receive a PCI_EJECT message and > then receive a PCI_BUS_RELATIONS message with bus_rel->device_count == 0. > > The first message is offloaded to hv_eject_device_work(), and the second > is offloaded to pci_devices_present_work(). Both the paths can be running > list_del(&hpdev->list_entry), causing general protection fault, because > system_wq can run them concurrently. > > The patch eliminates the race condition. > > Tested-by: Adrian Suhov > Tested-by: Chris Valean > Signed-off-by: Dexuan Cui > Reviewed-by: Michael Kelley > Acked-by: Haiyang Zhang > Cc: sta...@vger.kernel.org I need to know either what commit you are fixing (ie Fixes: tag - which is preferrable) or you tell me which kernel versions we are targeting for the stable backport. Thanks, Lorenzo > Cc: Vitaly Kuznetsov > Cc: Jack Morgenstein > Cc: Stephen Hemminger > Cc: K. Y. Srinivasan > --- > drivers/pci/host/pci-hyperv.c | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 2faf38eab785..5e67dff779a7 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -461,6 +461,8 @@ struct hv_pcibus_device { > struct retarget_msi_interrupt retarget_msi_interrupt_params; > > spinlock_t retarget_msi_interrupt_lock; > + > + struct workqueue_struct *wq; > }; > > /* > @@ -1770,7 +1772,7 @@ static void hv_pci_devices_present(struct > hv_pcibus_device *hbus, > spin_unlock_irqrestore(&hbus->device_list_lock, flags); > > get_hvpcibus(hbus); > - schedule_work(&dr_wrk->wrk); > + queue_work(hbus->wq, &dr_wrk->wrk); > } > > /** > @@ -1848,7 +1850,7 @@ static void hv_pci_eject_device(struct hv_pci_dev > *hpdev) > get_pcichild(hpdev, hv_pcidev_ref_pnp); > INIT_WORK(&hpdev->wrk, hv_eject_device_work); > get_hvpcibus(hpdev->hbus); > - schedule_work(&hpdev->wrk); > + queue_work(hpdev->hbus->wq, &hpdev->wrk); > } > > /** > @@ -2463,11 +2465,17 @@ static int hv_pci_probe(struct hv_device *hdev, > spin_lock_init(&hbus->retarget_msi_interrupt_lock); > sema_init(&hbus->enum_sem, 1); > init_completion(&hbus->remove_event); > + hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0, > +hbus->sysdata.domain); > + if (!hbus->wq) { > + ret = -ENOMEM; > + goto free_bus; > + } > > ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0, >hv_pci_onchannelcallback, hbus); > if (ret) > - goto free_bus; > + goto destroy_wq; > > hv_set_drvdata(hdev, hbus); > > @@ -2536,6 +2544,8 @@ static int hv_pci_probe(struct hv_device *hdev, > hv_free_config_window(hbus); > close: > vmbus_close(hdev->channel); > +destroy_wq: > + destroy_workqueue(hbus->wq); > free_bus: > free_page((unsigned long)hbus); > return ret; > @@ -2615,6 +2625,7 @@ static int hv_pci_remove(struct hv_device *hdev) > irq_domain_free_fwnode(hbus->sysdata.fwnode); > put_hvpcibus(hbus); > wait_for_completion(&hbus->remove_event); > + destroy_workqueue(hbus->wq); > free_page((unsigned long)hbus); > return 0; > } > -- > 2.7.4 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send
On Wed, 2018-03-14 at 10:22 +0100, Mohammed Gamal wrote: > On Tue, 2018-03-13 at 12:35 -0700, Stephen Hemminger wrote: > > On Tue, 13 Mar 2018 20:06:50 +0100 > > Mohammed Gamal wrote: > > > > > Dring high network traffic changes to network interface > > > parameters > > > such as number of channels or MTU can cause a kernel panic with a > > > NULL > > > pointer dereference. This is due to netvsc_device_remove() being > > > called and deallocating the channel ring buffers, which can then > > > be > > > accessed by netvsc_send_pkt() before they're allocated on calling > > > netvsc_device_add() > > > > > > The patch fixes this problem by checking the channel state and > > > returning > > > ENODEV if not yet opened. We also move the call to > > > hv_ringbuf_avail_percent() > > > which may access the uninitialized ring buffer. > > > > > > Signed-off-by: Mohammed Gamal > > > --- > > > drivers/net/hyperv/netvsc.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/net/hyperv/netvsc.c > > > b/drivers/net/hyperv/netvsc.c > > > index 0265d70..44a8358 100644 > > > --- a/drivers/net/hyperv/netvsc.c > > > +++ b/drivers/net/hyperv/netvsc.c > > > @@ -757,7 +757,7 @@ static inline int netvsc_send_pkt( > > > struct netdev_queue *txq = netdev_get_tx_queue(ndev, > > > packet->q_idx); > > > u64 req_id; > > > int ret; > > > - u32 ring_avail = hv_ringbuf_avail_percent(&out_channel- > > > > outbound); > > > > > > + u32 ring_avail; > > > > > > nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT; > > > if (skb) > > > @@ -773,7 +773,7 @@ static inline int netvsc_send_pkt( > > > > > > req_id = (ulong)skb; > > > > > > - if (out_channel->rescind) > > > + if (out_channel->rescind || out_channel->state != > > > CHANNEL_OPENED_STATE) > > > return -ENODEV; > > > > > > if (packet->page_buf_cnt) { > > > @@ -791,6 +791,7 @@ static inline int netvsc_send_pkt( > > > VMBUS_DATA_PACKET_FLAG_CO > > > MP > > > LETION_REQUESTED); > > > } > > > > > > + ring_avail = hv_ringbuf_avail_percent(&out_channel- > > > > outbound); > > > > > > if (ret == 0) { > > > atomic_inc_return(&nvchan->queue_sends); > > > > > > > Thanks for your patch. Yes there are races with the current update > > logic. The root cause goes higher up in the flow; the send queues > > should > > be stopped before netvsc_device_remove is called. Solving it where > > you tried > > to is racy and not going to work reliably. > > > > Network patches should go to net...@vger.kernel.org > > > > You can't move the ring_avail check until after the > > vmbus_sendpacket > > because > > that will break the flow control logic. > > > > Why? I don't see ring_avail being used before that point. Ah, stupid me. vmbus_sendpacket() will write to the ring buffer and that means that ring_avail value will be different than the expected. > > > Instead, you should just move the avail_read check until just after > > the existing rescind > > check. > > > > Also, you shouldn't need to check for OPENED_STATE, just rescind is > > enough. > > That rarely mitigated the race. channel->rescind flag is set on vmbus > exit - called on module unload - and when a rescind offer is received > from the host, which AFAICT doesn't happen on every call to > netvsc_device_remove, so it's quite possible that the ringbuffer is > accessed before it's allocated again on channel open and hence the > check for OPENED_STAT - which is only set after all vmbus data is > initialized. > Perhaps I haven't been clear enough. The NULL pointer dereference happens in the call to hv_ringbuf_avail_percent() which is used to calculate ring_avail. So we need to stop the queues before calling it if the channel's ring buffers haven't been allocated yet, but OTOH we should only stop the queues based upon the value of ring_avail, so this leads into a chicken and egg situation. Is my observation here correct? Please correct me if I am wrong, Stephen. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 0/6] staging: Introduce DPAA2 Ethernet Switch driver
On Thu, Mar 15, 2018 at 12:44:37AM +0100, Andrew Lunn wrote: > On Wed, Mar 14, 2018 at 10:55:52AM -0500, Razvan Stefanescu wrote: > > This patchset introduces the Ethernet Switch Driver for Freescale/NXP SoCs > > with DPAA2 (DataPath Acceleration Architecture v2). The driver manages > > switch objects discovered on the fsl-mc bus. A description of the driver > > can be found in the associated README file. > > Hi Greg > > This code has much better quality than the usual stuff in staging. I > see no reason not to merge it. Yeah. It seems pretty decent. Stuart, Laurentiu, care to comment? Meanwhile, netdev and DaveM aren't even on the CC list and they're the ones to ultimately decide. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 4/4] PCI: hv: Only queue a new work in hv_pci_devices_present() if necessary
If there is a pending work, we just need to add the new dr into the dr_list. Signed-off-by: Dexuan Cui Reviewed-by: Michael Kelley Acked-by: Haiyang Zhang Cc: Vitaly Kuznetsov Cc: Jack Morgenstein Cc: Stephen Hemminger Cc: K. Y. Srinivasan --- drivers/pci/host/pci-hyperv.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 0dc2ecdbbe45..50cdefe3f6d3 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1789,6 +1789,7 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus, struct hv_dr_state *dr; struct hv_dr_work *dr_wrk; unsigned long flags; + bool pending_dr; dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT); if (!dr_wrk) @@ -1812,11 +1813,21 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus, } spin_lock_irqsave(&hbus->device_list_lock, flags); + /* +* If pending_dr is true, we have already queued a work, +* which will see the new dr. Otherwise, we need to +* queue a new work. +*/ + pending_dr = !list_empty(&hbus->dr_list); list_add_tail(&dr->list_entry, &hbus->dr_list); spin_unlock_irqrestore(&hbus->device_list_lock, flags); - get_hvpcibus(hbus); - queue_work(hbus->wq, &dr_wrk->wrk); + if (pending_dr) { + kfree(dr_wrk); + } else { + get_hvpcibus(hbus); + queue_work(hbus->wq, &dr_wrk->wrk); + } } /** -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 3/4] PCI: hv: Remove hbus->enum_sem
Since we serialize the present/eject work items now, we don't need the semaphore any more. Signed-off-by: Dexuan Cui Reviewed-by: Michael Kelley Acked-by: Haiyang Zhang Cc: Vitaly Kuznetsov Cc: Jack Morgenstein Cc: Stephen Hemminger Cc: K. Y. Srinivasan --- drivers/pci/host/pci-hyperv.c | 17 ++--- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 1d2e1cb617f4..0dc2ecdbbe45 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -447,7 +447,6 @@ struct hv_pcibus_device { spinlock_t device_list_lock;/* Protect lists below */ void __iomem *cfg_addr; - struct semaphore enum_sem; struct list_head resources_for_children; struct list_head children; @@ -1648,12 +1647,8 @@ static struct hv_pci_dev *get_pcichild_wslot(struct hv_pcibus_device *hbus, * It must also treat the omission of a previously observed device as * notification that the device no longer exists. * - * Note that this function is a work item, and it may not be - * invoked in the order that it was queued. Back to back - * updates of the list of present devices may involve queuing - * multiple work items, and this one may run before ones that - * were sent later. As such, this function only does something - * if is the last one in the queue. + * Note that this function is serialized with hv_eject_device_work(), + * because both are pushed to the ordered workqueue hbus->wq. */ static void pci_devices_present_work(struct work_struct *work) { @@ -1674,11 +1669,6 @@ static void pci_devices_present_work(struct work_struct *work) INIT_LIST_HEAD(&removed); - if (down_interruptible(&hbus->enum_sem)) { - put_hvpcibus(hbus); - return; - } - /* Pull this off the queue and process it if it was the last one. */ spin_lock_irqsave(&hbus->device_list_lock, flags); while (!list_empty(&hbus->dr_list)) { @@ -1695,7 +1685,6 @@ static void pci_devices_present_work(struct work_struct *work) spin_unlock_irqrestore(&hbus->device_list_lock, flags); if (!dr) { - up(&hbus->enum_sem); put_hvpcibus(hbus); return; } @@ -1782,7 +1771,6 @@ static void pci_devices_present_work(struct work_struct *work) break; } - up(&hbus->enum_sem); put_hvpcibus(hbus); kfree(dr); } @@ -2516,7 +2504,6 @@ static int hv_pci_probe(struct hv_device *hdev, spin_lock_init(&hbus->config_lock); spin_lock_init(&hbus->device_list_lock); spin_lock_init(&hbus->retarget_msi_interrupt_lock); - sema_init(&hbus->enum_sem, 1); init_completion(&hbus->remove_event); hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0, hbus->sysdata.domain); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 2/4] PCI: hv: Remove the bogus test in hv_eject_device_work()
When we're in the function, hpdev->state must be hv_pcichild_ejecting: see hv_pci_eject_device(). Signed-off-by: Dexuan Cui Reviewed-by: Michael Kelley Acked-by: Haiyang Zhang Cc: Vitaly Kuznetsov Cc: Jack Morgenstein Cc: Stephen Hemminger Cc: K. Y. Srinivasan --- drivers/pci/host/pci-hyperv.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 28fce2be0a5f..1d2e1cb617f4 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1854,10 +1854,7 @@ static void hv_eject_device_work(struct work_struct *work) hpdev = container_of(work, struct hv_pci_dev, wrk); - if (hpdev->state != hv_pcichild_ejecting) { - put_pcichild(hpdev, hv_pcidev_ref_pnp); - return; - } + WARN_ON(hpdev->state != hv_pcichild_ejecting); /* * Ejection can come before or after the PCI bus has been set up, so -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 1/4] PCI: hv: Fix a comment typo in _hv_pcifront_read_config()
No functional change. Fixes: bdd74440d9e8 ("PCI: hv: Add explicit barriers to config space access") Signed-off-by: Dexuan Cui Acked-by: Haiyang Zhang Cc: Vitaly Kuznetsov Cc: Stephen Hemminger Cc: K. Y. Srinivasan --- drivers/pci/host/pci-hyperv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index acb1941423a9..28fce2be0a5f 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -657,7 +657,7 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where, break; } /* -* Make sure the write was done before we release the spinlock +* Make sure the read was done before we release the spinlock * allowing consecutive reads/writes. */ mb(); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 0/4] PCI: hv: Add 4 cosmetic patches
The 4 patches should go in v4.16. There is no code change since v3: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1628496.html I just split the original 6 patches to 2 series, and this is series 2. I fixed the order of Signed-off-by/Tested-by/Cc, etc., and fixed the other format issues in changelog (I hope I have fixed all of them). Dexuan Cui (4): PCI: hv: Fix a comment typo in _hv_pcifront_read_config() PCI: hv: Remove the bogus test in hv_eject_device_work() PCI: hv: Remove hbus->enum_sem PCI: hv: Only queue a new work in hv_pci_devices_present() if necessary drivers/pci/host/pci-hyperv.c | 39 +-- 1 file changed, 17 insertions(+), 22 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 2/2] PCI: hv: Fix 2 hang issues in hv_compose_msi_msg()
1. With the patch "x86/vector/msi: Switch to global reservation mode", the recent v4.15 and newer kernels always hang for 1-vCPU Hyper-V VM with SR-IOV. This is because when we reach hv_compose_msi_msg() by request_irq() -> request_threaded_irq() ->__setup_irq()->irq_startup() -> __irq_startup() -> irq_domain_activate_irq() -> ... -> msi_domain_activate() -> ... -> hv_compose_msi_msg(), local irq is disabled in __setup_irq(). Note: when we reach hv_compose_msi_msg() by another code path: pci_enable_msix_range() -> ... -> irq_domain_activate_irq() -> ... -> hv_compose_msi_msg(), local irq is not disabled. hv_compose_msi_msg() depends on an interrupt from the host. With interrupts disabled, a UP VM always hangs in the busy loop in the function, because the interrupt callback hv_pci_onchannelcallback() can not be called. We can do nothing but work it around by polling the channel. This is ugly, but we don't have any other choice. 2. If the host is ejecting the VF device before we reach hv_compose_msi_msg(), in a UP VM, we can hang in hv_compose_msi_msg() forever, because at this time the host doesn't respond to the CREATE_INTERRUPT request. This issue exists the first day the pci-hyperv driver appears in the kernel. Luckily, this can also by worked around by polling the channel for the PCI_EJECT message and hpdev->state, and by checking the PCI vendor ID. Note: actually the above 2 issues also happen to a SMP VM, if "hbus->hdev->channel->target_cpu == smp_processor_id()" is true. Fixes: 4900be83602b ("x86/vector/msi: Switch to global reservation mode") Tested-by: Adrian Suhov Tested-by: Chris Valean Signed-off-by: Dexuan Cui Reviewed-by: Michael Kelley Acked-by: Haiyang Zhang Cc: sta...@vger.kernel.org Cc: Stephen Hemminger Cc: K. Y. Srinivasan Cc: Vitaly Kuznetsov Cc: Jack Morgenstein --- drivers/pci/host/pci-hyperv.c | 58 ++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 5e67dff779a7..acb1941423a9 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -522,6 +522,8 @@ struct hv_pci_compl { s32 completion_status; }; +static void hv_pci_onchannelcallback(void *context); + /** * hv_pci_generic_compl() - Invoked for a completion packet * @context: Set up by the sender of the packet. @@ -666,6 +668,31 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where, } } +static u16 hv_pcifront_get_vendor_id(struct hv_pci_dev *hpdev) +{ + u16 ret; + unsigned long flags; + void __iomem *addr = hpdev->hbus->cfg_addr + CFG_PAGE_OFFSET + +PCI_VENDOR_ID; + + spin_lock_irqsave(&hpdev->hbus->config_lock, flags); + + /* Choose the function to be read. (See comment above) */ + writel(hpdev->desc.win_slot.slot, hpdev->hbus->cfg_addr); + /* Make sure the function was chosen before we start reading. */ + mb(); + /* Read from that function's config space. */ + ret = readw(addr); + /* +* mb() is not required here, because the spin_unlock_irqrestore() +* is a barrier. +*/ + + spin_unlock_irqrestore(&hpdev->hbus->config_lock, flags); + + return ret; +} + /** * _hv_pcifront_write_config() - Internal PCI config write * @hpdev: The PCI driver's representation of the device @@ -1108,8 +1135,37 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) * Since this function is called with IRQ locks held, can't * do normal wait for completion; instead poll. */ - while (!try_wait_for_completion(&comp.comp_pkt.host_event)) + while (!try_wait_for_completion(&comp.comp_pkt.host_event)) { + /* 0x means an invalid PCI VENDOR ID. */ + if (hv_pcifront_get_vendor_id(hpdev) == 0x) { + dev_err_once(&hbus->hdev->device, +"the device has gone\n"); + goto free_int_desc; + } + + /* +* When the higher level interrupt code calls us with +* interrupt disabled, we must poll the channel by calling +* the channel callback directly when channel->target_cpu is +* the current CPU. When the higher level interrupt code +* calls us with interrupt enabled, let's add the +* local_bh_disable()/enable() to avoid race. +*/ + local_bh_disable(); + + if (hbus->hdev->channel->target_cpu == smp_processor_id()) + hv_pci_onchannelcallback(hbus); + + local_bh_enable(); + + if (hpdev->state == hv_pcichild_ejecting) { + dev_err_once(&hbus->hdev->device, +"the device is being ejected
[PATCH v4 1/2] PCI: hv: Serialize the present and eject work items
When we hot-remove the device, we first receive a PCI_EJECT message and then receive a PCI_BUS_RELATIONS message with bus_rel->device_count == 0. The first message is offloaded to hv_eject_device_work(), and the second is offloaded to pci_devices_present_work(). Both the paths can be running list_del(&hpdev->list_entry), causing general protection fault, because system_wq can run them concurrently. The patch eliminates the race condition. Tested-by: Adrian Suhov Tested-by: Chris Valean Signed-off-by: Dexuan Cui Reviewed-by: Michael Kelley Acked-by: Haiyang Zhang Cc: sta...@vger.kernel.org Cc: Vitaly Kuznetsov Cc: Jack Morgenstein Cc: Stephen Hemminger Cc: K. Y. Srinivasan --- drivers/pci/host/pci-hyperv.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 2faf38eab785..5e67dff779a7 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -461,6 +461,8 @@ struct hv_pcibus_device { struct retarget_msi_interrupt retarget_msi_interrupt_params; spinlock_t retarget_msi_interrupt_lock; + + struct workqueue_struct *wq; }; /* @@ -1770,7 +1772,7 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus, spin_unlock_irqrestore(&hbus->device_list_lock, flags); get_hvpcibus(hbus); - schedule_work(&dr_wrk->wrk); + queue_work(hbus->wq, &dr_wrk->wrk); } /** @@ -1848,7 +1850,7 @@ static void hv_pci_eject_device(struct hv_pci_dev *hpdev) get_pcichild(hpdev, hv_pcidev_ref_pnp); INIT_WORK(&hpdev->wrk, hv_eject_device_work); get_hvpcibus(hpdev->hbus); - schedule_work(&hpdev->wrk); + queue_work(hpdev->hbus->wq, &hpdev->wrk); } /** @@ -2463,11 +2465,17 @@ static int hv_pci_probe(struct hv_device *hdev, spin_lock_init(&hbus->retarget_msi_interrupt_lock); sema_init(&hbus->enum_sem, 1); init_completion(&hbus->remove_event); + hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0, + hbus->sysdata.domain); + if (!hbus->wq) { + ret = -ENOMEM; + goto free_bus; + } ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0, hv_pci_onchannelcallback, hbus); if (ret) - goto free_bus; + goto destroy_wq; hv_set_drvdata(hdev, hbus); @@ -2536,6 +2544,8 @@ static int hv_pci_probe(struct hv_device *hdev, hv_free_config_window(hbus); close: vmbus_close(hdev->channel); +destroy_wq: + destroy_workqueue(hbus->wq); free_bus: free_page((unsigned long)hbus); return ret; @@ -2615,6 +2625,7 @@ static int hv_pci_remove(struct hv_device *hdev) irq_domain_free_fwnode(hbus->sysdata.fwnode); put_hvpcibus(hbus); wait_for_completion(&hbus->remove_event); + destroy_workqueue(hbus->wq); free_page((unsigned long)hbus); return 0; } -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 0/2] PCI: hv: Fix some real issues
The 2 patches should go in v4.16. They should go in old stable kernels too, especially this one should go in v4.15: PCI: hv: Fix 2 hang issues in hv_compose_msi_msg() Otherwise, a UP v4.15 VM can 100% hang when we pass through a VF to the VM that runs on Hyper-V. There is no code change since v3: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1628496.html I just split the original 6 patches to 2 series, and this is series 1. I fixed the order of Signed-off-by/Tested-by/Cc, etc., and added a Fixes: tag, and fixed the other format issues in changelog. I hope I didn't omit anything this time. :-) Dexuan Cui (2): PCI: hv: Serialize the present and eject work items PCI: hv: Fix 2 hang issues in hv_compose_msi_msg() drivers/pci/host/pci-hyperv.c | 75 --- 1 file changed, 71 insertions(+), 4 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/7] staging:iio:ade7854: Add proper error handling condition
On Wed, Mar 14, 2018 at 03:12:18PM -0300, Rodrigo Siqueira wrote: > There is some improper error handling for IRQ and device register. This > patch adds a proper verification. The IRQ correction was extracted from > John Syne patches. > > Signed-off-by: Rodrigo Siqueira > Signed-off-by: John Syne > --- > drivers/staging/iio/meter/ade7854.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/iio/meter/ade7854.c > b/drivers/staging/iio/meter/ade7854.c > index 09fd8c067738..49cbe365e43d 100644 > --- a/drivers/staging/iio/meter/ade7854.c > +++ b/drivers/staging/iio/meter/ade7854.c > @@ -436,7 +436,7 @@ static int ade7854_initial_setup(struct iio_dev > *indio_dev) > > /* Disable IRQ */ > ret = ade7854_set_irq(dev, false); > - if (ret) { > + if (ret < 0) { > dev_err(dev, "disable irq failed"); > goto err_ret; > } Why is the original wrong? It seems fine. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] staging:iio:ade7854: Rework I2C write function
On Wed, Mar 14, 2018 at 03:10:18PM -0300, Rodrigo Siqueira wrote: > The write operation using I2C has many code duplications and four > different interfaces per data size. This patch introduces a single > function that centralizes the main tasks. > > The central function inserted by this patch can easily replace all the > four functions related to the data size. However, this patch does not > remove any code signature for keeping the meter module work and make > easier to review this patch. > > Signed-off-by: Rodrigo Siqueira > --- > drivers/staging/iio/meter/ade7854-i2c.c | 89 > +++-- > drivers/staging/iio/meter/ade7854.h | 7 +++ > 2 files changed, 58 insertions(+), 38 deletions(-) > > diff --git a/drivers/staging/iio/meter/ade7854-i2c.c > b/drivers/staging/iio/meter/ade7854-i2c.c > index 317e4f0d8176..03133a05eae4 100644 > --- a/drivers/staging/iio/meter/ade7854-i2c.c > +++ b/drivers/staging/iio/meter/ade7854-i2c.c > @@ -15,41 +15,74 @@ > #include > #include "ade7854.h" > > -static int ade7854_i2c_write_reg_8(struct device *dev, > -u16 reg_address, > -u8 val) > +static int ade7854_i2c_write_reg(struct device *dev, > + u16 reg_address, > + u32 val, > + enum data_size type) The data size should just be the number of bytes and not an enum. 1 means 1 byte / 8 bits. 2 means 2 bytes / 16 bits. 3 means 3 bytes / 24 bits. etc. > { > int ret; > + int count; > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > struct ade7854_state *st = iio_priv(indio_dev); > > mutex_lock(&st->buf_lock); > st->tx[0] = (reg_address >> 8) & 0xFF; > st->tx[1] = reg_address & 0xFF; > - st->tx[2] = val; > > - ret = i2c_master_send(st->i2c, st->tx, 3); > + switch (type) { > + case DATA_SIZE_8_BITS: > + st->tx[2] = val & 0xFF; > + count = 3; > + break; > + case DATA_SIZE_16_BITS: > + st->tx[2] = (val >> 8) & 0xFF; > + st->tx[3] = val & 0xFF; > + count = 4; > + break; > + case DATA_SIZE_24_BITS: > + st->tx[2] = (val >> 16) & 0xFF; > + st->tx[3] = (val >> 8) & 0xFF; > + st->tx[4] = val & 0xFF; > + count = 5; > + break; > + case DATA_SIZE_32_BITS: > + st->tx[2] = (val >> 24) & 0xFF; > + st->tx[3] = (val >> 16) & 0xFF; > + st->tx[4] = (val >> 8) & 0xFF; > + st->tx[5] = val & 0xFF; > + count = 6; > + break; > + default: > + ret = -EINVAL; > + goto error_i2c_write_unlock; > + } > + > + ret = i2c_master_send(st->i2c, st->tx, count); > + > +error_i2c_write_unlock: These labels are sort of long. And what does the "i2c_write" really mean? It should be obvious that we're not jumping to a different function. Just "unlock:" is OK as a label name. > mutex_unlock(&st->buf_lock); > > return ret; > } > > +static int ade7854_i2c_write_reg_8(struct device *dev, > +u16 reg_address, > +u8 val) > +{ > + int ret; > + > + ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS); > + > + return ret; > +} Just do it like this: static int ade7854_i2c_write_reg_8(struct device *dev, u16 reg_address, u8 val) { return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS); } > + > static int ade7854_i2c_write_reg_16(struct device *dev, > u16 reg_address, > u16 val) > { > int ret; > - struct iio_dev *indio_dev = dev_to_iio_dev(dev); > - struct ade7854_state *st = iio_priv(indio_dev); > > - mutex_lock(&st->buf_lock); > - st->tx[0] = (reg_address >> 8) & 0xFF; > - st->tx[1] = reg_address & 0xFF; > - st->tx[2] = (val >> 8) & 0xFF; > - st->tx[3] = val & 0xFF; > - > - ret = i2c_master_send(st->i2c, st->tx, 4); > - mutex_unlock(&st->buf_lock); > + ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS); > > return ret; Again: return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS); > } > @@ -59,18 +92,8 @@ static int ade7854_i2c_write_reg_24(struct device *dev, > u32 val) > { > int ret; > - struct iio_dev *indio_dev = dev_to_iio_dev(dev); > - struct ade7854_state *st = iio_priv(indio_dev); > > - mutex_lock(&st->buf_lock); > - st->tx[0] = (reg_address >> 8) & 0xFF; > - st->tx[1] = reg_address & 0xFF; > - st->tx[2] = (val >> 16) & 0xFF; > - st->tx[3] = (val >> 8) & 0xFF; > - st->tx[4] = val & 0xFF; > - > - ret = i2c_master_send(st->i2c, st->tx, 5); > - mutex_unlock(&st->buf_lock); > +
Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
On Thu, Mar 15, 2018 at 12:03:07AM +, Sridhar Pitchai wrote: > Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even "Whenever a PCI bus is added" > with that when a first device is added to the bus, it overrides bus domain > ID with the device serial number. Sometime this can result in BUS ID not Define "Sometime". > being unique. In this case, when PCI_BUS and a device to bus is added, the > first device overwrites the bus domain ID to the device serial number, > which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI s/exsist/exist > bus addition fails. This patch make sure when a device is added to a bus, > it never updated the bus domain ID. s/updated/updates > Since we have the transparent SRIOV mode now, the short VF device name > is no longer needed. I still do not understand what this means and how it is related to the patch below, it may be clear to you, it is not to me, at all. > Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain") Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain") I asked you an explicit question. Commit above was added for a reason I assume. This patch implies that kernel has been broken since v4.11 which is almost a year ago and nobody every noticed ? Or there are systems where commit above is _necessary_ and this patch would break them ? I want a detailed explanation that highlights *why* it is safe to apply this patch and send it to stable kernels, commit log above won't do. Thanks, Lorenzo > Cc: sta...@vger.kernel.org > Signed-off-by: Sridhar Pitchai > --- > > Changes in v3: > * fix the commit comment. [KY Srinivasan, Michael Kelley] > --- > drivers/pci/host/pci-hyperv.c | 11 --- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 2faf38e..ac67e56 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct > hv_pcibus_device *hbus, > get_pcichild(hpdev, hv_pcidev_ref_childlist); > spin_lock_irqsave(&hbus->device_list_lock, flags); > > - /* > - * When a device is being added to the bus, we set the PCI domain > - * number to be the device serial number, which is non-zero and > - * unique on the same VM. The serial numbers start with 1, and > - * increase by 1 for each device. So device names including this > - * can have shorter names than based on the bus instance UUID. > - * Only the first device serial number is used for domain, so the > - * domain number will not change after the first device is added. > - */ > - if (list_empty(&hbus->children)) > - hbus->sysdata.domain = desc->ser; > list_add_tail(&hpdev->list_entry, &hbus->children); > spin_unlock_irqrestore(&hbus->device_list_lock, flags); > return hpdev; > -- > 2.7.4 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On 15/03/18 11:48, Dan Carpenter wrote: This all seems fine. Generally the requirements for staging are that it has a TODO, someone to work on it, and it doesn't break the build. But some of the patches don't have commit message and those are required and some of the commit messages are just the changes you have made not don't describe the actual code... John Crispin's email is j...@phrozen.org. regards, dan carpenter Hi All, looks like i was CC'ed on the openwrt addr, which no longer exists. This series makes no sense. None of the stuff posted is anywhere near ready to be upstreamed. * we dont need a dedicated pinctrl driver, pinctrl-single will work fine on these SoCs * the DMA/sdhci driver is a hacked up version of the SDK driver. * drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable to mt7621, same goes for the gsw driver. John ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On Thu, Mar 15, 2018 at 12:07:26PM +0100, John Crispin wrote: > None of the stuff posted is anywhere near ready to be > upstreamed. Yeah. This is staging so we accept any quality... The rest of the information is very useful. Thanks! regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On Thu, Mar 15, 2018 at 10:04:33PM +1100, NeilBrown wrote: > On Thu, Mar 15 2018, Dan Carpenter wrote: > > > This all seems fine. Generally the requirements for staging are that it > > has a TODO, someone to work on it, and it doesn't break the build. But > > some of the patches don't have commit message and those are required and > > some of the commit messages are just the changes you have made not don't > > describe the actual code... > > Thanks for having a look. > It seems odd to require detailed commit messages, when we don't require > the same level of quality in the code. > Naturally when the driver is moved out of staging a properly detailed > commit message should be added, but is that needed on the way in to > staging? At this stage I don't know much more than is already there. > After I've cleaned up the code I probably will. > > For patch 01/13 you asked "what kind of device this is". The subject > line makes it clear that it is a "pcie driver". What extra detail did > you want? Would it be sufficient to just copy the subject line so that > it appears twice in the commit message? > Ah... Sorry. It's literally a pcie driver. For some reason I thought it was a device that ran over pcie. We don't require a detailed changelog, but you have to put something... Probably just restating the subject and adding that it's for the gnubee1 is fine. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
On Thu, Mar 15 2018, Dan Carpenter wrote: > This all seems fine. Generally the requirements for staging are that it > has a TODO, someone to work on it, and it doesn't break the build. But > some of the patches don't have commit message and those are required and > some of the commit messages are just the changes you have made not don't > describe the actual code... Thanks for having a look. It seems odd to require detailed commit messages, when we don't require the same level of quality in the code. Naturally when the driver is moved out of staging a properly detailed commit message should be added, but is that needed on the way in to staging? At this stage I don't know much more than is already there. After I've cleaned up the code I probably will. For patch 01/13 you asked "what kind of device this is". The subject line makes it clear that it is a "pcie driver". What extra detail did you want? Would it be sufficient to just copy the subject line so that it appears twice in the commit message? > > John Crispin's email is j...@phrozen.org. Thanks. NeilBrown > > regards, > dan carpenter signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1
This all seems fine. Generally the requirements for staging are that it has a TODO, someone to work on it, and it doesn't break the build. But some of the patches don't have commit message and those are required and some of the commit messages are just the changes you have made not don't describe the actual code... John Crispin's email is j...@phrozen.org. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/13] staging: mt7621-pci: MIPS/ralink: add MT7621 pcie driver
These patches need better changelogs. Some of them don't have a changelog at all. Like just say this what kind of device this is or something? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: ni_mio_common: ack ai fifo error interrupts.
From: Frank Mori Hess Ack ai fifo error interrupts in interrupt handler to clear interrupt after fifo overflow. It should prevent lock-ups after the ai fifo overflows. Cc: # v4.2+ Signed-off-by: Frank Mori Hess Signed-off-by: Ian Abbott --- I have not tested this patch on hardware. -- Frank Mori Hess Applies cleanly to v4.2+, but needs backporting for earlier stable kernels. -- Ian Abbott. --- drivers/staging/comedi/drivers/ni_mio_common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index d6eb55b41814..e40a2c0a9543 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -1275,6 +1275,8 @@ static void ack_a_interrupt(struct comedi_device *dev, unsigned short a_status) ack |= NISTC_INTA_ACK_AI_START; if (a_status & NISTC_AI_STATUS1_STOP) ack |= NISTC_INTA_ACK_AI_STOP; + if (a_status & NISTC_AI_STATUS1_OVER) + ack |= NISTC_INTA_ACK_AI_ERR; if (ack) ni_stc_writew(dev, ack, NISTC_INTA_ACK_REG); } -- 2.16.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Y2038] [PATCH v4 02/10] include: Move compat_timespec/ timeval to compat_time.h
On Thu, Mar 15, 2018 at 3:51 AM, Deepa Dinamani wrote: > On Wed, Mar 14, 2018 at 1:52 PM, Arnd Bergmann wrote: >> On Wed, Mar 14, 2018 at 4:50 AM, Deepa Dinamani >> wrote: >>> The file arch/arm64/kernel/process.c needs asm/compat.h also to be >>> included directly since this is included conditionally from >>> include/compat.h. This does seem to be typical of arm64 as I was not >>> completely able to get rid of asm/compat.h includes for arm64 in this >>> series. My plan is to have separate patches to get rid of asm/compat.h >>> includes for the architectures that are not straight forward to keep >>> this series simple. >>> I will fix this and update the series. >>> >> >> I ran across the same thing in two more files during randconfig testing on >> arm64 now, adding this fixup on top for the moment, but maybe there >> is a better way: > > I was looking at how Al tested his uaccess patches: > https://www.spinics.net/lists/linux-fsdevel/msg108752.html > > He seems to be running the kbuild bot tests on his own git. > Is it possible to verify it this way on the 2038 tree? Or, I could > host a tree also. The kbuild bot should generally pick up any branch on git.kernel.org, and the patches sent to the mailing list. It tests a lot of things configurations, but I tend to find some things that it doesn't find by doing lots of randconfig builds on fewer target architectures (I only build arm, arm64 and x86 regularly). I remember that there was some discussion about a method to get the bot to test other branches (besides asking Fengguang to add it manually), but I don't remember what came out of that. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel