* [PATCH 04/10] NFC: st95hf: remove logging from spi functions
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>
The callers of these functions already log errors, so there's no need to do
it from two places.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/nfc/st95hf/spi.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/nfc/st95hf/spi.c b/drivers/nfc/st95hf/spi.c
index e2d3bbcc8c34..d5894d4546b1 100644
--- a/drivers/nfc/st95hf/spi.c
+++ b/drivers/nfc/st95hf/spi.c
@@ -47,8 +47,6 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
result = spi_sync(spidev, &m);
if (result) {
- dev_err(&spidev->dev, "error: sending cmd to st95hf using SPI = %d\n",
- result);
mutex_unlock(&spicontext->spi_lock);
return result;
}
@@ -62,12 +60,10 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
result = wait_for_completion_timeout(&spicontext->done,
msecs_to_jiffies(1000));
/* check for timeout or success */
- if (!result) {
- dev_err(&spidev->dev, "error: response not ready timeout\n");
+ if (!result)
result = -ETIMEDOUT;
- } else {
+ else
result = 0;
- }
mutex_unlock(&spicontext->spi_lock);
@@ -79,7 +75,7 @@ EXPORT_SYMBOL_GPL(st95hf_spi_send);
int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
unsigned char *receivebuff)
{
- int len = 0;
+ int ret, len;
struct spi_transfer tx_takedata;
struct spi_message m;
struct spi_device *spidev = spicontext->spidev;
@@ -89,8 +85,6 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
{.rx_buf = receivebuff, .len = 2, .cs_change = 1,},
};
- int ret = 0;
-
memset(&tx_takedata, 0x0, sizeof(struct spi_transfer));
mutex_lock(&spicontext->spi_lock);
@@ -102,8 +96,6 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
ret = spi_sync(spidev, &m);
if (ret) {
- dev_err(&spidev->dev, "spi_recv_resp, data length error = %d\n",
- ret);
mutex_unlock(&spicontext->spi_lock);
return ret;
}
@@ -127,11 +119,8 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
ret = spi_sync(spidev, &m);
mutex_unlock(&spicontext->spi_lock);
- if (ret) {
- dev_err(&spidev->dev, "spi_recv_resp, data read error = %d\n",
- ret);
+ if (ret)
return ret;
- }
return len;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/4] iio: adc: xilinx: XADC driver Enhancements and bug fixes
From: Manish Narani @ 2018-07-23 15:01 UTC (permalink / raw)
To: linux-arm-kernel
This patch series resolves code style problems as reported by code analysis
tools.
Changes in v2:
- From the first version of patches, 2 patches are dropped in this
series.
- In the v1 patch set, 1/4 was not required as the issue is in
checkpatch.pl script and not in the code.
- In the v1 patch set 2/4 was applied on togreg branch of iio.git
- for 3/4 patch in v1 patch set, the patch is broken up in 2 patches
in this series as per review comments. They are now 1/4 and 2/4 in
this series.
- 4/4 patch in the v1 patch set was basically to resolve coverity
warning for platform_get_irq() in xadc_remove function, but that was
causing potential race condition. Fixed that in this series (3/4).
- A new patch is added in this series for moving request_irq() before
enabling interrupts.
Manish Narani (4):
iio: adc: xilinx: Check for return values in clk related functions
iio: adc: xilinx: limit pcap clock frequency value
iio: adc: xilinx: Remove platform_get_irq from xadc_remove function
iio: adc: xilinx: Move request_irq before enabling interrupts
drivers/iio/adc/xilinx-xadc-core.c | 40 +++++++++++++++++++++++++++++---------
drivers/iio/adc/xilinx-xadc.h | 1 +
2 files changed, 32 insertions(+), 9 deletions(-)
--
2.1.1
^ permalink raw reply
* [PATCH v2 1/4] iio: adc: xilinx: Check for return values in clk related functions
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
To: lakshmi.sai.krishna.potthuri, manish.narani, michal.simek, pmeerw,
lars, knaack.h, jic23
Cc: anirudh, sgoud, linux-kernel, linux-arm-kernel, linux-iio
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>
This patch adds check for return values from clock related functions.
This was reported by static code analysis tool.
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
drivers/iio/adc/xilinx-xadc-core.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 0127e85..23395fc 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -340,6 +340,8 @@ static int xadc_zynq_setup(struct platform_device *pdev,
xadc->zynq_intmask = ~0;
pcap_rate = clk_get_rate(xadc->clk);
+ if (!pcap_rate)
+ return -EINVAL;
if (tck_rate > pcap_rate / 2) {
div = 2;
@@ -887,6 +889,9 @@ static int xadc_write_raw(struct iio_dev *indio_dev,
unsigned long clk_rate = xadc_get_dclk_rate(xadc);
unsigned int div;
+ if (!clk_rate)
+ return -EINVAL;
+
if (info != IIO_CHAN_INFO_SAMP_FREQ)
return -EINVAL;
@@ -1237,8 +1242,10 @@ static int xadc_probe(struct platform_device *pdev)
goto err_free_irq;
/* Disable all alarms */
- xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
- XADC_CONF1_ALARM_MASK);
+ ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
+ XADC_CONF1_ALARM_MASK);
+ if (ret)
+ goto err_free_irq;
/* Set thresholds to min/max */
for (i = 0; i < 16; i++) {
--
2.1.1
^ permalink raw reply related
* [PATCH 07/10] NFC: st95hf: re-order command defines
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>
Just a small cleanup to bring the command defines in a numerical order.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/nfc/st95hf/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 99f84ddfdfef..e7ecc57dde8f 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -45,10 +45,10 @@
/* Command Send Interface */
/* ST95HF_COMMAND_SEND CMD Ids */
-#define ECHO_CMD 0x55
-#define WRITE_REGISTER_CMD 0x9
#define PROTOCOL_SELECT_CMD 0x2
#define SEND_RECEIVE_CMD 0x4
+#define WRITE_REGISTER_CMD 0x9
+#define ECHO_CMD 0x55
/* Select protocol codes */
#define ISO15693_PROTOCOL_CODE 0x1
--
2.17.1
^ permalink raw reply related
* [PATCH 06/10] NFC: st95hf: move skb allocation to ISR
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>
The driver currently assumes that interrupts can only occur between the
time when when a command has been sent to the device and the response
to it.
This assumption, however, is wrong. The antenna of the chip is likely to
catch a lot of environmental noise, and once in a while, the device will
latch the interrupt to signal a protocol error, and keep it latched until
the response bytes are read from the chip.
As the code currently stands, skbs for responses are only prepared when
a command is sent, and the ISR bails out early if that wasn't the case.
Hence, the interrupt remains latched, and no further communication with
device is possible.
To fix this, move the call to nfc_alloc_recv_skb() from
st95hf_in_send_cmd() to st95hf_irq_thread_handler() so we can always read
back the interrupt reason.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/nfc/st95hf/core.c | 36 ++++++------------------------------
1 file changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 6761ab90f68d..99f84ddfdfef 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -196,7 +196,6 @@ static const struct cmd cmd_array[] = {
/* st95_digital_cmd_complete_arg stores client context */
struct st95_digital_cmd_complete_arg {
- struct sk_buff *skb_resp;
nfc_digital_cmd_complete_t complete_cb;
void *cb_usrarg;
bool rats;
@@ -783,12 +782,10 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
spidevice = &stcontext->spicontext.spidev->dev;
cb_arg = &stcontext->complete_cb_arg;
- skb_resp = cb_arg->skb_resp;
- if (unlikely(!skb_resp)) {
- WARN(1, "unknown context in ST95HF ISR");
+ skb_resp = nfc_alloc_recv_skb(MAX_RESPONSE_BUFFER_SIZE, GFP_KERNEL);
+ if (WARN_ON(!skb_resp))
return IRQ_NONE;
- }
mutex_lock(&stcontext->rm_lock);
res_len = st95hf_spi_recv_response(&stcontext->spicontext,
@@ -838,12 +835,6 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
/* call digital layer callback */
cb_arg->complete_cb(stcontext->ddev, cb_arg->cb_usrarg, skb_resp);
- /*
- * This skb is now owned by the core layer.
- * Make sure not to use it again.
- */
- cb_arg->skb_resp = NULL;
-
/* up the semaphore before returning */
mutex_unlock(&stcontext->rm_lock);
@@ -913,15 +904,7 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
void *arg)
{
struct st95hf_context *stcontext = nfc_digital_get_drvdata(ddev);
- int rc;
- struct sk_buff *skb_resp;
- int len_data_to_tag = 0;
-
- skb_resp = nfc_alloc_recv_skb(MAX_RESPONSE_BUFFER_SIZE, GFP_KERNEL);
- if (!skb_resp) {
- rc = -ENOMEM;
- goto error;
- }
+ int rc, len_data_to_tag = 0;
switch (stcontext->current_rf_tech) {
case NFC_DIGITAL_RF_TECH_106A:
@@ -933,8 +916,7 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
len_data_to_tag = skb->len;
break;
default:
- rc = -EINVAL;
- goto free_skb_resp;
+ return -EINVAL;
}
skb_push(skb, 3);
@@ -942,7 +924,6 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
skb->data[1] = SEND_RECEIVE_CMD;
skb->data[2] = len_data_to_tag;
- stcontext->complete_cb_arg.skb_resp = skb_resp;
stcontext->complete_cb_arg.cb_usrarg = arg;
stcontext->complete_cb_arg.complete_cb = cb;
@@ -956,17 +937,12 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
if (rc) {
dev_err(&stcontext->nfcdev->dev,
"Error %d trying to perform data_exchange", rc);
- goto free_skb_resp;
+ return rc;
}
kfree_skb(skb);
- return rc;
-
-free_skb_resp:
- kfree_skb(skb_resp);
-error:
- return rc;
+ return 0;
}
/* p2p will be supported in a later release ! */
--
2.17.1
^ permalink raw reply related
* [PATCH 05/10] NFC: st95hf: remove exchange_lock
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>
This patch removes the exchange_lock sempahore. Its intended function
was two-fold:
a) Lock the remove() callback of the driver against the ISR, so that
the resources only go away after the ISR has finished. This is
unnecessary though, because `rm_lock' does that already, in
combination with the nullification of `scontext->ddev'.
b) Indicate whether a command was sent previously. If the semaphore
is found unused in the threaded ISR, an error is reported.
This case can be handled much nicer by checking whether `skb_resp'
is present in the context. For this, nullify the `skb_resp' pointer
in the callback context.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/nfc/st95hf/core.c | 52 +++++++--------------------------------
1 file changed, 9 insertions(+), 43 deletions(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index d857197ec7b2..6761ab90f68d 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -214,8 +214,6 @@ struct st95_digital_cmd_complete_arg {
* @st95hf_supply: regulator "consumer" for NFC device.
* @sendrcv_trflag: last byte of frame send by sendrecv command
* of st95hf. This byte contains transmission flag info.
- * @exchange_lock: semaphore used for signaling the st95hf_remove
- * function that the last outstanding async nfc request is finished.
* @rm_lock: mutex for ensuring safe access of nfc digital object
* from threaded ISR. Usage of this mutex avoids any race between
* deletion of the object from st95hf_remove() and its access from
@@ -233,7 +231,6 @@ struct st95hf_context {
struct st95_digital_cmd_complete_arg complete_cb_arg;
struct regulator *st95hf_supply;
unsigned char sendrcv_trflag;
- struct semaphore exchange_lock;
struct mutex rm_lock;
u8 current_protocol;
u8 current_rf_tech;
@@ -785,29 +782,14 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
struct st95_digital_cmd_complete_arg *cb_arg;
spidevice = &stcontext->spicontext.spidev->dev;
+ cb_arg = &stcontext->complete_cb_arg;
+ skb_resp = cb_arg->skb_resp;
- /*
- * check semaphore, if not down() already, then we don't
- * know in which context the ISR is called and surely it
- * will be a bug. Note that down() of the semaphore is done
- * in the corresponding st95hf_in_send_cmd() and then
- * only this ISR should be called. ISR will up() the
- * semaphore before leaving. Hence when the ISR is called
- * the correct behaviour is down_trylock() should always
- * return 1 (indicating semaphore cant be taken and hence no
- * change in semaphore count).
- * If not, then we up() the semaphore and crash on
- * a BUG() !
- */
- if (!down_trylock(&stcontext->exchange_lock)) {
- up(&stcontext->exchange_lock);
+ if (unlikely(!skb_resp)) {
WARN(1, "unknown context in ST95HF ISR");
return IRQ_NONE;
}
- cb_arg = &stcontext->complete_cb_arg;
- skb_resp = cb_arg->skb_resp;
-
mutex_lock(&stcontext->rm_lock);
res_len = st95hf_spi_recv_response(&stcontext->spicontext,
skb_resp->data);
@@ -856,8 +838,13 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
/* call digital layer callback */
cb_arg->complete_cb(stcontext->ddev, cb_arg->cb_usrarg, skb_resp);
+ /*
+ * This skb is now owned by the core layer.
+ * Make sure not to use it again.
+ */
+ cb_arg->skb_resp = NULL;
+
/* up the semaphore before returning */
- up(&stcontext->exchange_lock);
mutex_unlock(&stcontext->rm_lock);
return IRQ_HANDLED;
@@ -868,8 +855,6 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
skb_resp = ERR_PTR(result);
/* call of callback with error */
cb_arg->complete_cb(stcontext->ddev, cb_arg->cb_usrarg, skb_resp);
- /* up the semaphore before returning */
- up(&stcontext->exchange_lock);
mutex_unlock(&stcontext->rm_lock);
return IRQ_HANDLED;
}
@@ -965,25 +950,12 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
ddev->curr_protocol == NFC_PROTO_ISO14443)
stcontext->complete_cb_arg.rats = true;
- /*
- * down the semaphore to indicate to remove func that an
- * ISR is pending, note that it will not block here in any case.
- * If found blocked, it is a BUG!
- */
- rc = down_killable(&stcontext->exchange_lock);
- if (rc) {
- WARN(1, "Semaphore is not found up in st95hf_in_send_cmd\n");
- return rc;
- }
-
rc = st95hf_spi_send(&stcontext->spicontext, skb->data,
skb->len,
ASYNC);
if (rc) {
dev_err(&stcontext->nfcdev->dev,
"Error %d trying to perform data_exchange", rc);
- /* up the semaphore since ISR will never come in this case */
- up(&stcontext->exchange_lock);
goto free_skb_resp;
}
@@ -1104,7 +1076,6 @@ static int st95hf_probe(struct spi_device *nfc_spi_dev)
}
}
- sema_init(&st95context->exchange_lock, 1);
init_completion(&spicontext->done);
mutex_init(&spicontext->spi_lock);
mutex_init(&st95context->rm_lock);
@@ -1220,11 +1191,6 @@ static int st95hf_remove(struct spi_device *nfc_spi_dev)
mutex_unlock(&stcontext->rm_lock);
- /* if last in_send_cmd's ISR is pending, wait for it to finish */
- result = down_killable(&stcontext->exchange_lock);
- if (result == -EINTR)
- dev_err(&spictx->spidev->dev, "sleep for semaphore interrupted by signal\n");
-
/* next reset the ST95HF controller */
result = st95hf_spi_send(&stcontext->spicontext,
&reset_cmd,
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/4] iio: adc: xilinx: Check for return values in clk related functions
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>
This patch adds check for return values from clock related functions.
This was reported by static code analysis tool.
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
drivers/iio/adc/xilinx-xadc-core.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 0127e85..23395fc 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -340,6 +340,8 @@ static int xadc_zynq_setup(struct platform_device *pdev,
xadc->zynq_intmask = ~0;
pcap_rate = clk_get_rate(xadc->clk);
+ if (!pcap_rate)
+ return -EINVAL;
if (tck_rate > pcap_rate / 2) {
div = 2;
@@ -887,6 +889,9 @@ static int xadc_write_raw(struct iio_dev *indio_dev,
unsigned long clk_rate = xadc_get_dclk_rate(xadc);
unsigned int div;
+ if (!clk_rate)
+ return -EINVAL;
+
if (info != IIO_CHAN_INFO_SAMP_FREQ)
return -EINVAL;
@@ -1237,8 +1242,10 @@ static int xadc_probe(struct platform_device *pdev)
goto err_free_irq;
/* Disable all alarms */
- xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
- XADC_CONF1_ALARM_MASK);
+ ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
+ XADC_CONF1_ALARM_MASK);
+ if (ret)
+ goto err_free_irq;
/* Set thresholds to min/max */
for (i = 0; i < 16; i++) {
--
2.1.1
^ permalink raw reply related
* [PATCH v2 2/4] iio: adc: xilinx: limit pcap clock frequency value
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>
This patch limits the xadc pcap clock frequency value to be less than
200MHz. This fixes the issue when zynq is booted at higher frequency
values, pcap crosses the maximum limit of 200MHz(Fmax) as it is derived
from IOPLL.
If this limit is crossed it is required to alter the WEDGE and REDGE
bits of XADC_CFG register to make timings better in the interface. So to
avoid alteration of these bits every time, the pcap value should not
cross the Fmax limit.
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
drivers/iio/adc/xilinx-xadc-core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 23395fc..0dd306d 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -322,6 +322,7 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
#define XADC_ZYNQ_TCK_RATE_MAX 50000000
#define XADC_ZYNQ_IGAP_DEFAULT 20
+#define XADC_ZYNQ_PCAP_RATE_MAX 200000000
static int xadc_zynq_setup(struct platform_device *pdev,
struct iio_dev *indio_dev, int irq)
@@ -332,6 +333,7 @@ static int xadc_zynq_setup(struct platform_device *pdev,
unsigned int div;
unsigned int igap;
unsigned int tck_rate;
+ int ret;
/* TODO: Figure out how to make igap and tck_rate configurable */
igap = XADC_ZYNQ_IGAP_DEFAULT;
@@ -343,6 +345,13 @@ static int xadc_zynq_setup(struct platform_device *pdev,
if (!pcap_rate)
return -EINVAL;
+ if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
+ ret = clk_set_rate(xadc->clk,
+ (unsigned long)XADC_ZYNQ_PCAP_RATE_MAX);
+ if (ret)
+ return ret;
+ }
+
if (tck_rate > pcap_rate / 2) {
div = 2;
} else {
@@ -368,6 +377,12 @@ static int xadc_zynq_setup(struct platform_device *pdev,
XADC_ZYNQ_CFG_REDGE | XADC_ZYNQ_CFG_WEDGE |
tck_div | XADC_ZYNQ_CFG_IGAP(igap));
+ if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
+ ret = clk_set_rate(xadc->clk, pcap_rate);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
--
2.1.1
^ permalink raw reply related
* [PATCH 08/10] NFC: st95hf: unify sync/async flags
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>
Keep the information whether a command is asynchronous in a boolean flag
everywhere in the code. This way, the enum can go away.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/nfc/st95hf/core.c | 38 ++++++++++++++++----------------------
drivers/nfc/st95hf/spi.c | 12 +++++-------
drivers/nfc/st95hf/spi.h | 8 +-------
3 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index e7ecc57dde8f..835a1e61c817 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -101,7 +101,7 @@ struct cmd {
unsigned char cmd_id;
unsigned char no_cmd_params;
unsigned char cmd_params[MAX_CMD_PARAMS];
- enum req_type req;
+ bool is_sync;
};
struct param_list {
@@ -134,63 +134,62 @@ static const struct cmd cmd_array[] = {
.cmd_len = 0x2,
.cmd_id = ECHO_CMD,
.no_cmd_params = 0,
- .req = SYNC,
+ .is_sync = true,
},
[CMD_ISO14443A_CONFIG] = {
.cmd_len = 0x7,
.cmd_id = WRITE_REGISTER_CMD,
.no_cmd_params = 0x4,
.cmd_params = {0x3A, 0x00, 0x5A, 0x04},
- .req = SYNC,
+ .is_sync = true,
},
[CMD_ISO14443A_DEMOGAIN] = {
.cmd_len = 0x7,
.cmd_id = WRITE_REGISTER_CMD,
.no_cmd_params = 0x4,
.cmd_params = {0x68, 0x01, 0x01, 0xDF},
- .req = SYNC,
+ .is_sync = true,
},
[CMD_ISO14443B_DEMOGAIN] = {
.cmd_len = 0x7,
.cmd_id = WRITE_REGISTER_CMD,
.no_cmd_params = 0x4,
.cmd_params = {0x68, 0x01, 0x01, 0x51},
- .req = SYNC,
+ .is_sync = true,
},
[CMD_ISO14443A_PROTOCOL_SELECT] = {
.cmd_len = 0x7,
.cmd_id = PROTOCOL_SELECT_CMD,
.no_cmd_params = 0x4,
.cmd_params = {ISO14443A_PROTOCOL_CODE, 0x00, 0x01, 0xA0},
- .req = SYNC,
+ .is_sync = true,
},
[CMD_ISO14443B_PROTOCOL_SELECT] = {
.cmd_len = 0x7,
.cmd_id = PROTOCOL_SELECT_CMD,
.no_cmd_params = 0x4,
.cmd_params = {ISO14443B_PROTOCOL_CODE, 0x01, 0x03, 0xFF},
- .req = SYNC,
+ .is_sync = true,
},
[CMD_WTX_RESPONSE] = {
.cmd_len = 0x6,
.cmd_id = SEND_RECEIVE_CMD,
.no_cmd_params = 0x3,
.cmd_params = {0xF2, 0x00, TRFLAG_NFCA_STD_FRAME_CRC},
- .req = ASYNC,
},
[CMD_FIELD_OFF] = {
.cmd_len = 0x5,
.cmd_id = PROTOCOL_SELECT_CMD,
.no_cmd_params = 0x2,
.cmd_params = {0x0, 0x0},
- .req = SYNC,
+ .is_sync = true,
},
[CMD_ISO15693_PROTOCOL_SELECT] = {
.cmd_len = 0x5,
.cmd_id = PROTOCOL_SELECT_CMD,
.no_cmd_params = 0x2,
.cmd_params = {ISO15693_PROTOCOL_CODE, 0x0D},
- .req = SYNC,
+ .is_sync = true,
},
};
@@ -279,13 +278,13 @@ static int st95hf_send_recv_cmd(struct st95hf_context *st95context,
ret = st95hf_spi_send(&st95context->spicontext,
spi_cmd_buffer,
cmd_array[cmd].cmd_len,
- cmd_array[cmd].req);
+ cmd_array[cmd].is_sync);
if (ret) {
dev_err(dev, "st95hf_spi_send failed with error %d\n", ret);
return ret;
}
- if (cmd_array[cmd].req == SYNC && recv_res) {
+ if (cmd_array[cmd].is_sync && recv_res) {
unsigned char st95hf_response_arr[2];
ret = st95hf_spi_recv_response(&st95context->spicontext,
@@ -480,10 +479,8 @@ static int st95hf_send_spi_reset_sequence(struct st95hf_context *st95context)
int result = 0;
unsigned char reset_cmd = ST95HF_COMMAND_RESET;
- result = st95hf_spi_send(&st95context->spicontext,
- &reset_cmd,
- ST95HF_RESET_CMD_LEN,
- ASYNC);
+ result = st95hf_spi_send(&st95context->spicontext, &reset_cmd,
+ ST95HF_RESET_CMD_LEN, false);
if (result) {
dev_err(&st95context->spicontext.spidev->dev,
"spi reset sequence cmd error = %d", result);
@@ -932,8 +929,7 @@ static int st95hf_in_send_cmd(struct nfc_digital_dev *ddev,
stcontext->complete_cb_arg.rats = true;
rc = st95hf_spi_send(&stcontext->spicontext, skb->data,
- skb->len,
- ASYNC);
+ skb->len, false);
if (rc) {
dev_err(&stcontext->nfcdev->dev,
"Error %d trying to perform data_exchange", rc);
@@ -1168,10 +1164,8 @@ static int st95hf_remove(struct spi_device *nfc_spi_dev)
mutex_unlock(&stcontext->rm_lock);
/* next reset the ST95HF controller */
- result = st95hf_spi_send(&stcontext->spicontext,
- &reset_cmd,
- ST95HF_RESET_CMD_LEN,
- ASYNC);
+ result = st95hf_spi_send(&stcontext->spicontext, &reset_cmd,
+ ST95HF_RESET_CMD_LEN, false);
if (result) {
dev_err(&spictx->spidev->dev,
"ST95HF reset failed in remove() err = %d\n", result);
diff --git a/drivers/nfc/st95hf/spi.c b/drivers/nfc/st95hf/spi.c
index d5894d4546b1..ef12ed67343f 100644
--- a/drivers/nfc/st95hf/spi.c
+++ b/drivers/nfc/st95hf/spi.c
@@ -23,7 +23,7 @@
int st95hf_spi_send(struct st95hf_spi_context *spicontext,
unsigned char *buffertx,
int datalen,
- enum req_type reqtype)
+ bool is_sync)
{
struct spi_message m;
int result = 0;
@@ -35,12 +35,10 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
mutex_lock(&spicontext->spi_lock);
- if (reqtype == SYNC) {
- spicontext->req_issync = true;
+ spicontext->req_issync = is_sync;
+
+ if (is_sync)
reinit_completion(&spicontext->done);
- } else {
- spicontext->req_issync = false;
- }
spi_message_init(&m);
spi_message_add_tail(&tx_transfer, &m);
@@ -52,7 +50,7 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
}
/* return for asynchronous or no-wait case */
- if (reqtype == ASYNC) {
+ if (!is_sync) {
mutex_unlock(&spicontext->spi_lock);
return 0;
}
diff --git a/drivers/nfc/st95hf/spi.h b/drivers/nfc/st95hf/spi.h
index 552d220747cd..ede17eef6ab1 100644
--- a/drivers/nfc/st95hf/spi.h
+++ b/drivers/nfc/st95hf/spi.h
@@ -44,16 +44,10 @@ struct st95hf_spi_context {
struct mutex spi_lock;
};
-/* flag to differentiate synchronous & asynchronous spi request */
-enum req_type {
- SYNC,
- ASYNC,
-};
-
int st95hf_spi_send(struct st95hf_spi_context *spicontext,
unsigned char *buffertx,
int datalen,
- enum req_type reqtype);
+ bool is_sync);
int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
unsigned char *receivebuff);
--
2.17.1
^ permalink raw reply related
* [PATCH 09/10] NFC: st95hf: two small style nits
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>
Address two minor style issues that I came across while working on the
driver.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/nfc/st95hf/core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 835a1e61c817..4db3c020921c 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -662,7 +662,8 @@ static int st95hf_error_handling(struct st95hf_context *stcontext,
result = -ETIMEDOUT;
else
result = -EIO;
- return result;
+
+ return result;
}
/* Check for CRC err only if CRC is present in the tag response */
@@ -844,6 +845,7 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
/* call of callback with error */
cb_arg->complete_cb(stcontext->ddev, cb_arg->cb_usrarg, skb_resp);
mutex_unlock(&stcontext->rm_lock);
+
return IRQ_HANDLED;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 3/4] iio: adc: xilinx: Remove platform_get_irq from xadc_remove function
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>
This patch avoids getting irq number in xadc_remove function. Instead
store 'irq' in xadc struct and use xadc->irq wherever needed.
This patch also resolves a warning reported by coverity where it asks to
check return value of platform_get_irq() for any errors in xadc_remove.
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
drivers/iio/adc/xilinx-xadc-core.c | 10 +++++-----
drivers/iio/adc/xilinx-xadc.h | 1 +
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 0dd306d..44a2519 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1175,6 +1175,7 @@ static int xadc_probe(struct platform_device *pdev)
xadc = iio_priv(indio_dev);
xadc->ops = id->data;
+ xadc->irq = irq;
init_completion(&xadc->completion);
mutex_init(&xadc->mutex);
spin_lock_init(&xadc->lock);
@@ -1225,11 +1226,11 @@ static int xadc_probe(struct platform_device *pdev)
if (ret)
goto err_free_samplerate_trigger;
- ret = xadc->ops->setup(pdev, indio_dev, irq);
+ ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
if (ret)
goto err_clk_disable_unprepare;
- ret = request_irq(irq, xadc->ops->interrupt_handler, 0,
+ ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
dev_name(&pdev->dev), indio_dev);
if (ret)
goto err_clk_disable_unprepare;
@@ -1288,7 +1289,7 @@ static int xadc_probe(struct platform_device *pdev)
return 0;
err_free_irq:
- free_irq(irq, indio_dev);
+ free_irq(xadc->irq, indio_dev);
err_clk_disable_unprepare:
clk_disable_unprepare(xadc->clk);
err_free_samplerate_trigger:
@@ -1310,7 +1311,6 @@ static int xadc_remove(struct platform_device *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct xadc *xadc = iio_priv(indio_dev);
- int irq = platform_get_irq(pdev, 0);
iio_device_unregister(indio_dev);
if (xadc->ops->flags & XADC_FLAGS_BUFFERED) {
@@ -1318,7 +1318,7 @@ static int xadc_remove(struct platform_device *pdev)
iio_trigger_free(xadc->convst_trigger);
iio_triggered_buffer_cleanup(indio_dev);
}
- free_irq(irq, indio_dev);
+ free_irq(xadc->irq, indio_dev);
clk_disable_unprepare(xadc->clk);
cancel_delayed_work(&xadc->zynq_unmask_work);
kfree(xadc->data);
diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index 62edbda..8c00095 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -68,6 +68,7 @@ struct xadc {
spinlock_t lock;
struct completion completion;
+ int irq;
};
struct xadc_ops {
--
2.1.1
^ permalink raw reply related
* [PATCH v2 4/4] iio: adc: xilinx: Move request_irq before enabling interrupts
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
To: lakshmi.sai.krishna.potthuri, manish.narani, michal.simek, pmeerw,
lars, knaack.h, jic23
Cc: anirudh, sgoud, linux-kernel, linux-arm-kernel, linux-iio
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>
Enabling the Interrupts before registering the irq handler is a bad
idea. This patch corrects the same for XADC driver.
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
drivers/iio/adc/xilinx-xadc-core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 44a2519..3f6be5a 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1226,15 +1226,15 @@ static int xadc_probe(struct platform_device *pdev)
if (ret)
goto err_free_samplerate_trigger;
- ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
- if (ret)
- goto err_clk_disable_unprepare;
-
ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
dev_name(&pdev->dev), indio_dev);
if (ret)
goto err_clk_disable_unprepare;
+ ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
+ if (ret)
+ goto err_free_irq;
+
for (i = 0; i < 16; i++)
xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i),
&xadc->threshold[i]);
--
2.1.1
^ permalink raw reply related
* [PATCH v2 4/4] iio: adc: xilinx: Move request_irq before enabling interrupts
From: Manish Narani @ 2018-07-23 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1532358123-23485-1-git-send-email-manish.narani@xilinx.com>
Enabling the Interrupts before registering the irq handler is a bad
idea. This patch corrects the same for XADC driver.
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
drivers/iio/adc/xilinx-xadc-core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 44a2519..3f6be5a 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1226,15 +1226,15 @@ static int xadc_probe(struct platform_device *pdev)
if (ret)
goto err_free_samplerate_trigger;
- ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
- if (ret)
- goto err_clk_disable_unprepare;
-
ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
dev_name(&pdev->dev), indio_dev);
if (ret)
goto err_clk_disable_unprepare;
+ ret = xadc->ops->setup(pdev, indio_dev, xadc->irq);
+ if (ret)
+ goto err_free_irq;
+
for (i = 0; i < 16; i++)
xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i),
&xadc->threshold[i]);
--
2.1.1
^ permalink raw reply related
* [PATCH 10/10] NFC: st95hf: add of match table
From: Daniel Mack @ 2018-07-23 14:00 UTC (permalink / raw)
To: sameo; +Cc: linux-wireless, colin.king, shikha.singh, Daniel Mack, devicetree
In-Reply-To: <20180723140015.11663-1-daniel@zonque.org>
Add a match table for device tree compatible strings. Interestingly, a
document describing the bindings already exists since a while, but users
currently reply on the implicit matching in the drivers core.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Cc: devicetree@vger.kernel.org
---
drivers/nfc/st95hf/core.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 4db3c020921c..5a01b454fbc4 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -1012,6 +1012,12 @@ static const struct spi_device_id st95hf_id[] = {
};
MODULE_DEVICE_TABLE(spi, st95hf_id);
+static const struct of_device_id st95hf_of_match[] = {
+ { .compatible = "st,st95hf", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, st95hf_of_match);
+
static int st95hf_probe(struct spi_device *nfc_spi_dev)
{
int ret;
@@ -1189,6 +1195,7 @@ static struct spi_driver st95hf_driver = {
.driver = {
.name = "st95hf",
.owner = THIS_MODULE,
+ .of_match_table = st95hf_of_match,
},
.id_table = st95hf_id,
.probe = st95hf_probe,
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 3/4] perf/x86/intel/ds: Handle PEBS overflow for fixed counters
From: Peter Zijlstra @ 2018-07-23 15:02 UTC (permalink / raw)
To: kan.liang
Cc: tglx, mingo, linux-kernel, acme, alexander.shishkin,
vincent.weaver, jolsa, ak
In-Reply-To: <20180723145944.GB2458@hirez.programming.kicks-ass.net>
On Mon, Jul 23, 2018 at 04:59:44PM +0200, Peter Zijlstra wrote:
> On Thu, Mar 08, 2018 at 06:15:41PM -0800, kan.liang@linux.intel.com wrote:
> > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> > index ef47a418d819..86149b87cce8 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
> > * counters from the GLOBAL_STATUS mask and we always process PEBS
> > * events via drain_pebs().
> > */
> > - status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
> > + if (x86_pmu.flags & PMU_FL_PEBS_ALL)
> > + status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
> > + else
> > + status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
> >
> > /*
> > * PEBS overflow sets bit 62 in the global status register
>
> Doesn't this re-introduce the problem fixed in commit fd583ad1563be,
> where pebs_enabled:32-34 are PEBS Load Latency, instead of fixed
> counters?
*GROAN* the MSR definitions now differ between Core and Atom :-(
^ permalink raw reply
* Re: [PATCH 5/7] btrfs: warn for num_devices below 0
From: David Sterba @ 2018-07-23 14:01 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
In-Reply-To: <20180716145812.20836-6-anand.jain@oracle.com>
On Mon, Jul 16, 2018 at 10:58:10PM +0800, Anand Jain wrote:
> In preparation to de-duplicate a section of code where we deduce the
> num_devices, use warn instead of bug.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> fs/btrfs/volumes.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 7f4973fc2b52..0f4c512aa6b4 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3726,7 +3726,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
> num_devices = fs_info->fs_devices->num_devices;
> btrfs_dev_replace_read_lock(&fs_info->dev_replace);
> if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
> - BUG_ON(num_devices < 1);
> + WARN_ON(num_devices < 1);
I wonder if there any valid cases when there are 0 devices when balance
is started, ie. before num_devices gets decremented.
The WARN_ON is either redundant or should be turned to a proper sanity
check.
> num_devices--;
^ permalink raw reply
* Re: [PATCH v2 07/12] sched/core: uclamp: enforce last task UCLAMP_MAX
From: Patrick Bellasi @ 2018-07-23 15:02 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: linux-kernel, linux-pm, Ingo Molnar, Peter Zijlstra, Tejun Heo,
Rafael J . Wysocki, Viresh Kumar, Vincent Guittot, Paul Turner,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpH6CsDVG35Z3NV87mUgJW9TmoCuOs8GeW2TEqkP0K0f9g@mail.gmail.com>
On 20-Jul 18:23, Suren Baghdasaryan wrote:
> Hi Patrick,
Hi Sure,
thank!
> On Mon, Jul 16, 2018 at 1:29 AM, Patrick Bellasi
> <patrick.bellasi@arm.com> wrote:
[...]
> > @@ -977,13 +991,21 @@ static inline void uclamp_cpu_get_id(struct task_struct *p,
> > uc_grp = &rq->uclamp.group[clamp_id][0];
> > uc_grp[group_id].tasks += 1;
> >
> > + /* Force clamp update on idle exit */
> > + uc_cpu = &rq->uclamp;
> > + clamp_value = p->uclamp[clamp_id].value;
> > + if (unlikely(uc_cpu->flags & UCLAMP_FLAG_IDLE)) {
>
> The condition below is not needed because UCLAMP_FLAG_IDLE is set only
> for UCLAMP_MAX clamp_id, therefore the above condition already covers
> the one below.
Not really, this function is called two times, the first time to
update UCLAMP_MIN and a second time to update UCLAMP_MAX.
For both clamp_id we want to force update uc_cpu->value[clamp_id],
thus the UCLAMP_FLAG_IDLE flag has to be cleared only the second time.
Maybe I can had the following comment to better explain the reason of
the check:
/*
* This function is called for both UCLAMP_MIN (before) and
* UCLAMP_MAX (after). Let's reset the flag only the when
* we know that UCLAMP_MIN has been already updated.
*/
> > + if (clamp_id == UCLAMP_MAX)
> > + uc_cpu->flags &= ~UCLAMP_FLAG_IDLE;
> > + uc_cpu->value[clamp_id] = clamp_value;
> > + return;
> > + }
[...]
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: wireguardnl: Go package for interacting with WireGuard via generic netlink
From: Jason A. Donenfeld @ 2018-07-23 15:12 UTC (permalink / raw)
To: mdlayher; +Cc: WireGuard mailing list
In-Reply-To: <3c167a80-6459-7c0e-8935-a98e226fa023@gmail.com>
Hi Matt,
> This is super interesting and I actually did not discover it until after
> I pushed the first few commits to my package. I could see it making
> sense to refactor my current package layout to something like three
> packages:
>
> - wireguardnl: netlink-based communication
> - wireguardcfg: text-based userspace configuration protocol communication
> - wireguard: wrapper for both that detects the module in use and
> seamlessly presents a unified interface
No, that's really not a good approach at all. First of all, do not
take the raw name "wireguard". That's going to cause a lot of
confusion. It's really not appropriate.
But more importantly, you shouldn't expose either the netlink or the
xplatform API distinction to users ever. They should be given one
interface, not three, and that one interface should be able to select
the right thing in 100% of cases.
Jason
^ permalink raw reply
* Re: [PATCH 2/2] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits
From: Boris Ostrovsky @ 2018-07-23 15:04 UTC (permalink / raw)
To: M. Vefa Bicakci, linux-kernel
Cc: Juergen Gross, x86, stable, Ingo Molnar, Andy Lutomirski,
H. Peter Anvin, xen-devel, Thomas Gleixner, Kirill A. Shutemov
In-Reply-To: <13b774d7-bc8c-039a-fbc0-52318f535883@runbox.com>
On 07/22/2018 11:57 AM, M. Vefa Bicakci wrote:
> On 07/21/2018 07:17 PM, M. Vefa Bicakci wrote:
>> On 07/21/2018 05:25 PM, Boris Ostrovsky wrote:
>>> On 07/21/2018 03:49 PM, M. Vefa Bicakci wrote:
>>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>>>> index 439a94bf89ad..87afb000142a 100644
>>>> --- a/arch/x86/xen/enlighten_pv.c
>>>> +++ b/arch/x86/xen/enlighten_pv.c
>>>> @@ -1257,6 +1257,7 @@ asmlinkage __visible void __init
>>>> xen_start_kernel(void)
>>>> /* Work out if we support NX */
>>>> get_cpu_cap(&boot_cpu_data);
>>>> + get_cpu_address_sizes(&boot_cpu_data);
>>>> x86_configure_nx();
>>>
>>>
>>> Have you observed any problems without this call? get_cpu_cap() is only
>>> called here to set X86_FEATURE_NX, and is then called again, together
>>> with get_cpu_address_sizes(), from early_identify_cpu().
>>
>> Thank you for the reviews! Without the call to get_cpu_address_sizes,
>> paravirtualized virtual machines do not boot up kernels with versions
>> 4.17 and up at all; this includes dom0 and domU. No domU logs are
>> generated in dom0's /var/log/xen/console/ directory either, despite
>> having earlyprintk=xen on the kernel command line for my test domU.
>
> Hello Boris,
>
> I debugged this further with a debugging version of Xen (so that I can
> get early kernel print-outs via the "xen_raw_console_write" function),
> and I found the root cause of the boot up failure.
>
> In summary, the issue is due to the following call path in version
> 4.17 (and higher, I assume), which the kernel goes through /only/ when
> CONFIG_DEBUG_VIRTUAL is enabled:
>
> enlighten_pv.c::xen_start_kernel
> mmu_pv.c::xen_reserve_special_pages
> page.h::__pa
> physaddr.c::__phys_addr
> physaddr.h::phys_addr_valid // uses boot_cpu_data.x86_phys_bits
>
> The return value of phys_addr_valid is used with the VIRTUAL_BUG_ON
> macro,
> which evaluates to BUG_ON in case CONFIG_DEBUG_VIRTUAL is enabled.
Ah, that's why it hasn't been detected.
>
> It looks like the call to get_cpu_address_size is required in the
> xen_start_kernel function. Perhaps there is a more elegant way to
> resolve this issue as well.
>
> Another approach could be to check in the phys_addr_valid function
> whether
> boot_cpu_data.x86_phys_bits has been initialized or not, I think, but
> I am
> not sure about the correctness of this approach.
No, I think your patch is good. The only thing I'd suggest is to move
the call a few lines down. The way it is placed now may create
impression that we are calling get_cpu_address_sizes() to figure out NX
support.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* Re: [PATCH 2/2] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits
From: Boris Ostrovsky @ 2018-07-23 15:04 UTC (permalink / raw)
To: M. Vefa Bicakci, linux-kernel
Cc: Kirill A. Shutemov, Andy Lutomirski, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, Juergen Gross, xen-devel, x86, stable
In-Reply-To: <13b774d7-bc8c-039a-fbc0-52318f535883@runbox.com>
On 07/22/2018 11:57 AM, M. Vefa Bicakci wrote:
> On 07/21/2018 07:17 PM, M. Vefa Bicakci wrote:
>> On 07/21/2018 05:25 PM, Boris Ostrovsky wrote:
>>> On 07/21/2018 03:49 PM, M. Vefa Bicakci wrote:
>>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>>>> index 439a94bf89ad..87afb000142a 100644
>>>> --- a/arch/x86/xen/enlighten_pv.c
>>>> +++ b/arch/x86/xen/enlighten_pv.c
>>>> @@ -1257,6 +1257,7 @@ asmlinkage __visible void __init
>>>> xen_start_kernel(void)
>>>> /* Work out if we support NX */
>>>> get_cpu_cap(&boot_cpu_data);
>>>> + get_cpu_address_sizes(&boot_cpu_data);
>>>> x86_configure_nx();
>>>
>>>
>>> Have you observed any problems without this call? get_cpu_cap() is only
>>> called here to set X86_FEATURE_NX, and is then called again, together
>>> with get_cpu_address_sizes(), from early_identify_cpu().
>>
>> Thank you for the reviews! Without the call to get_cpu_address_sizes,
>> paravirtualized virtual machines do not boot up kernels with versions
>> 4.17 and up at all; this includes dom0 and domU. No domU logs are
>> generated in dom0's /var/log/xen/console/ directory either, despite
>> having earlyprintk=xen on the kernel command line for my test domU.
>
> Hello Boris,
>
> I debugged this further with a debugging version of Xen (so that I can
> get early kernel print-outs via the "xen_raw_console_write" function),
> and I found the root cause of the boot up failure.
>
> In summary, the issue is due to the following call path in version
> 4.17 (and higher, I assume), which the kernel goes through /only/ when
> CONFIG_DEBUG_VIRTUAL is enabled:
>
> enlighten_pv.c::xen_start_kernel
> mmu_pv.c::xen_reserve_special_pages
> page.h::__pa
> physaddr.c::__phys_addr
> physaddr.h::phys_addr_valid // uses boot_cpu_data.x86_phys_bits
>
> The return value of phys_addr_valid is used with the VIRTUAL_BUG_ON
> macro,
> which evaluates to BUG_ON in case CONFIG_DEBUG_VIRTUAL is enabled.
Ah, that's why it hasn't been detected.
>
> It looks like the call to get_cpu_address_size is required in the
> xen_start_kernel function. Perhaps there is a more elegant way to
> resolve this issue as well.
>
> Another approach could be to check in the phys_addr_valid function
> whether
> boot_cpu_data.x86_phys_bits has been initialized or not, I think, but
> I am
> not sure about the correctness of this approach.
No, I think your patch is good. The only thing I'd suggest is to move
the call a few lines down. The way it is placed now may create
impression that we are calling get_cpu_address_sizes() to figure out NX
support.
-boris
^ permalink raw reply
* Re: [stable-4.14 00/23] block/scsi multiqueue performance enhancement and
From: Jens Axboe @ 2018-07-23 15:05 UTC (permalink / raw)
To: Jack Wang, Greg Kroah-Hartman, linux-scsi, linux-block
Cc: Wang Jinpu, stable, Jinpu Wang
In-Reply-To: <CA+res+TE5-mgnQys67zQ8cqRyv+dgNPpTFLOEq81t2izpq1dsw@mail.gmail.com>
On 7/23/18 9:00 AM, Jack Wang wrote:
> Hi Greg,
>
> Thanks for quick reply. Please see reply inline.
>
>
>
> Greg KH <gregkh@linuxfoundation.org> 于2018年7月23日周一 下午3:34写道：
>>
>> On Mon, Jul 23, 2018 at 03:24:22PM +0200, Jack Wang wrote:
>>> Hi Greg,
>>>
>>> Please consider this patchset, which include block/scsi multiqueue performance
>>> enhancement and bugfix.
>>
>> What exactly is the performance enhancement? How can you measure it?
>> How did you measure it?
> I'm testing on SRP/IBNBD using fio, I've seen +10% with mix IO load,
> and 50% improvement on small IO (bs=512.) with the patchset.
Big nak on backporting this huge series, it's a lot of core changes.
It's way beyond the scope of a stable fix backport.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 6/6] x86/shadow: a little bit of style cleanup
From: Tim Deegan @ 2018-07-23 15:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: George Dunlap, xen-devel, Andrew Cooper
In-Reply-To: <5B506D1702000078001D5D8D@prv1-mh.provo.novell.com>
At 04:51 -0600 on 19 Jul (1531975863), Jan Beulich wrote:
> Correct indentation of a piece of code, adjusting comment style at the
> same time. Constify gl3e pointers and drop a bogus (and useless once
> corrected) cast.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tim Deegan <tim@xen.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* Re: How to tell bitbake my prefered terminal in for devshell and access bitbake datastore?
From: Rudolf J Streif @ 2018-07-23 15:05 UTC (permalink / raw)
To: MOHAMMAD RASIM, yocto
In-Reply-To: <1e741f3f-f20e-4e38-d9ec-643e22b34e2d@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1136 bytes --]
You can use the variable OE_TERMINAL to choose your preferred terminal.
By default it's set to auto and Bitbake figures out what terminal is
available on your system. Please refer to the manual at:
https://www.yoctoproject.org/docs/2.5/mega-manual/mega-manual.html#var-OE_TERMINAL.
> Also, is it possible to access bitbake datastore from inside the
devshell(or devpyshell) ?
You cannot access d if you mean that, however, all the variables are
exported to the shell.
:rjs
On 07/23/2018 02:49 AM, MOHAMMAD RASIM wrote:
> Hi,
>
> when running bitbake with a recipe and giving the devshell as a
> command bitbake runs the devshell inside xterm, on my other machine it
> runs tmux.
>
> looking at the openembedded source code here
> https://github.com/openembedded/openembedded-core/blob/master/meta/lib/oe/terminal.py
>
> I can see there are multiple classes for terminals that bitbake loop
> through, but how can I tell bitbake that I prefer a certain terminal?
>
> Also, is it possible to access bitbake datastore from inside the
> devshell(or devpyshell) ?
>
> Regards
>
--
Rudolf J Streif
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 499 bytes --]
^ permalink raw reply
* [PATCH net-next] net: phy: prevent PHYs w/o Clause 22 regs from calling genphy_config_aneg
From: Camelia Groza @ 2018-07-23 15:06 UTC (permalink / raw)
To: andrew, f.fainelli, rmk+kernel, davem; +Cc: netdev, linux-kernel, Camelia Groza
genphy_config_aneg() should be called only by PHYs that implement
the Clause 22 register set. Prevent Clause 45 PHYs that don't implement
the register set from calling the genphy function.
Signed-off-by: Camelia Groza <camelia.groza@nxp.com>
---
drivers/net/phy/phy.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index efa0a3c..6049652 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -471,8 +471,14 @@ static int phy_config_aneg(struct phy_device *phydev)
{
if (phydev->drv->config_aneg)
return phydev->drv->config_aneg(phydev);
- else
- return genphy_config_aneg(phydev);
+
+ /* Clause 45 PHYs that don't implement Clause 22 registers are not
+ * allowed to call genphy_config_aneg()
+ */
+ if (phydev->is_c45 && !(phydev->c45_ids.devices_in_package & BIT(0)))
+ return -EOPNOTSUPP;
+
+ return genphy_config_aneg(phydev);
}
/**
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 1/2] powerpc/pseries: Avoid blocking rtas polling handling multiple PRRN events
From: John Allen @ 2018-07-23 15:05 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, nfont
In-Reply-To: <87muuihyjn.fsf@concordia.ellerman.id.au>
On Mon, Jul 23, 2018 at 11:27:56PM +1000, Michael Ellerman wrote:
>Hi John,
>
>I'm a bit puzzled by this one.
>
>John Allen <jallen@linux.ibm.com> writes:
>> When a PRRN event is being handled and another PRRN event comes in, the
>> second event will block rtas polling waiting on the first to complete,
>> preventing any further rtas events from being handled. This can be
>> especially problematic in case that PRRN events are continuously being
>> queued in which case rtas polling gets indefinitely blocked completely.
>>
>> This patch introduces a mutex that prevents any subsequent PRRN events from
>> running while there is a prrn event being handled, allowing rtas polling to
>> continue normally.
>>
>> Signed-off-by: John Allen <jallen@linux.ibm.com>
>> ---
>> v2:
>> -Unlock prrn_lock when PRRN operations are complete, not after handler is
>> scheduled.
>> -Remove call to flush_work, the previous broken method of serializing
>> PRRN events.
>> ---
>> arch/powerpc/kernel/rtasd.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
>> index 44d66c33d59d..845fc5aec178 100644
>> --- a/arch/powerpc/kernel/rtasd.c
>> +++ b/arch/powerpc/kernel/rtasd.c
>> @@ -284,15 +286,17 @@ static void prrn_work_fn(struct work_struct *work)
>> */
>> pseries_devicetree_update(-prrn_update_scope);
>> numa_update_cpu_topology(false);
>> + mutex_unlock(&prrn_lock);
>> }
>>
>> static DECLARE_WORK(prrn_work, prrn_work_fn);
>>
>> static void prrn_schedule_update(u32 scope)
>> {
>> - flush_work(&prrn_work);
>
>This seems like it's actually the core of the change. Previously we were
>basically blocking on the flush before continuing.
The idea here is to replace the blocking flush_work with a non-blocking
mutex. So rather than waiting on the running PRRN event to complete, we
bail out since a PRRN event is already running. The situation this is
meant to address is flooding the workqueue with PRRN events, which like
the situation in patch 2/2, these can be queued up faster than they can
actually be handled.
>
>> - prrn_update_scope = scope;
>
>I don't really understand the scope. With the old code we always ran the
>work function once for call, now we potentially throw away the scope
>value (if the try lock fails).
So anytime we actually want to run with the scope (in the event the
trylock succeeds), we schedule the work with the scope value set
accordingly as seen in the code below. In the case that we actually
don't want to run a PRRN event (if one is already running) we do throw
away the scope and ignore the request entirely.
>
>> - schedule_work(&prrn_work);
>> + if (mutex_trylock(&prrn_lock)) {
>> + prrn_update_scope = scope;
>> + schedule_work(&prrn_work);
>> + }
>
>Ignoring the scope, the addition of the mutex should not actually make
>any difference. If you see the doco for schedule_work() it says:
>
> * This puts a job in the kernel-global workqueue if it was not already
> * queued and leaves it in the same position on the kernel-global
> * workqueue otherwise.
>
>
>So the mutex basically implements that existing behaviour. But maybe the
>scope is the issue? Like I said I don't really understand the scope
>value.
>
>
>So I guess I'm wondering if we just need to drop the flush_work() and
>the rest is not required?
To sum up the above, the behavior without the mutex is not the same as
with the mutex. Without the mutex, that means that anytime we get a PRRN
event, it will get queued on the workqueue which can get flooded if PRRN
events are queued continuously. With the mutex, only one PRRN event can
be queued for handling at once.
Hope that clears things up!
-John
>
>cheers
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.