* Re: [RFC bpf-next 0/5] Convert iproute2 to use libbpf (WIP)
From: Toke Høiland-Jørgensen @ 2019-08-23 11:29 UTC (permalink / raw)
To: Andrii Nakryiko, Daniel Borkmann
Cc: Stephen Hemminger, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
Yonghong Song, David Miller, Jesper Dangaard Brouer, Networking,
bpf
In-Reply-To: <CAEf4Bzab_w0AXy5P9mG14mcyJVgUCzuuNda5FpU5wSEwUciGfg@mail.gmail.com>
[ ... snip ...]
> E.g., today's API is essentially three steps:
>
> 1. open and parse ELF: collect relos, programs, map definitions
> 2. load: create maps from collected defs, do program/global data/CO-RE
> relocs, load and verify BPF programs
> 3. attach programs one by one.
>
> Between step 1 and 2 user has flexibility to create more maps, set up
> map-in-map, etc. Between 2 and 3 you can fill in global data, fill in
> tail call maps, etc. That's already pretty flexible. But we can tune
> and break apart those steps even further, if necessary.
Today, steps 1 and 2 can be collapsed into a single call to
bpf_prog_load_xattr(). As Jesper's mail explains, for XDP we don't
generally want to do all the fancy rewriting stuff, we just want a
simple way to load a program and get reusable pinning of maps.
Preferably in a way that is compatible with the iproute2 loader.
So I really think we need two things:
(1) a flexible API that splits up all the various steps in a way that
allows programs to inject their own map definitions before
relocations and loading
(2) a simple convenience wrapper that loads an object file, does
something sensible with pinning and map-in-map definitions, and loads
everything into the kernel.
I'd go so far as to say that (2) should even support system-wide
configuration, similar to the /etc/iproute2/bpf_pinning file. E.g., an
/etc/libbpf/pinning.conf file that sets the default pinning directory,
and makes it possible to set up pin-value-to-subdir mappings like what
iproute2 does today.
Having (2) makes it more likely that all the different custom loaders
will be compatible with each other, while still allowing people to do
their own custom thing with (1). And of course, (2) could be implemented
in terms of (1) internally in libbpf.
In my ideal world, (2) would just use the definition format already in
iproute2 (this is basically what I implemented already), but if you guys
don't want to put this into libbpf, I can probably live with the default
format being BTF-based instead. Which would mean that iproute2 I would
end up with a flow like this:
- When given an elf file, try to run it through the "standard loader"
(2). If this works, great, proceed to program attach.
- If using (2) fails because it doesn't understand the map definition,
fall back to a compatibility loader that parses the legacy iproute2
map definition format and uses (1) to load that.
Does the above make sense? :)
-Toke
^ permalink raw reply
* Re: [PATCH] wimax/i2400m: fix calculation of index, remove sizeof
From: Colin Ian King @ 2019-08-23 11:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190823112337.GB23408@kadam>
On 23/08/2019 12:23, Dan Carpenter wrote:
> On Fri, Aug 23, 2019 at 09:52:30AM +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The subtraction of the two pointers is automatically scaled by the
>> size of the size of the object the pointers point to, so the division
>> by sizeof(*i2400m->barker) is incorrect. Fix this by removing the
>> division. Also make index an unsigned int to clean up a checkpatch
>> warning.
>>
>> Addresses-Coverity: ("Extra sizeof expression")
>> Fixes: aba3792ac2d7 ("wimax/i2400m: rework bootrom initialization to be more flexible")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/wimax/i2400m/fw.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
>> index 489cba9b284d..599a703af6eb 100644
>> --- a/drivers/net/wimax/i2400m/fw.c
>> +++ b/drivers/net/wimax/i2400m/fw.c
>> @@ -399,8 +399,7 @@ int i2400m_is_boot_barker(struct i2400m *i2400m,
>> * associated with the device. */
>> if (i2400m->barker
>> && !memcmp(buf, i2400m->barker, sizeof(i2400m->barker->data))) {
>> - unsigned index = (i2400m->barker - i2400m_barker_db)
>> - / sizeof(*i2400m->barker);
>> + unsigned int index = i2400m->barker - i2400m_barker_db;
>> d_printf(2, dev, "boot barker cache-confirmed #%u/%08x\n",
>> index, le32_to_cpu(i2400m->barker->data[0]));
>
> It's only used for this debug output. You may as well just delete it.
>
>> return 0;
Deleting wrong debug code vs fixing debug code? I'd rather go for the
latter.
>
> regards,
> dan carpenter
>
^ permalink raw reply
* Re: [PATCH] wimax/i2400m: fix calculation of index, remove sizeof
From: Dan Carpenter @ 2019-08-23 11:23 UTC (permalink / raw)
To: Colin King
Cc: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190823085230.6225-1-colin.king@canonical.com>
On Fri, Aug 23, 2019 at 09:52:30AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The subtraction of the two pointers is automatically scaled by the
> size of the size of the object the pointers point to, so the division
> by sizeof(*i2400m->barker) is incorrect. Fix this by removing the
> division. Also make index an unsigned int to clean up a checkpatch
> warning.
>
> Addresses-Coverity: ("Extra sizeof expression")
> Fixes: aba3792ac2d7 ("wimax/i2400m: rework bootrom initialization to be more flexible")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/wimax/i2400m/fw.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
> index 489cba9b284d..599a703af6eb 100644
> --- a/drivers/net/wimax/i2400m/fw.c
> +++ b/drivers/net/wimax/i2400m/fw.c
> @@ -399,8 +399,7 @@ int i2400m_is_boot_barker(struct i2400m *i2400m,
> * associated with the device. */
> if (i2400m->barker
> && !memcmp(buf, i2400m->barker, sizeof(i2400m->barker->data))) {
> - unsigned index = (i2400m->barker - i2400m_barker_db)
> - / sizeof(*i2400m->barker);
> + unsigned int index = i2400m->barker - i2400m_barker_db;
> d_printf(2, dev, "boot barker cache-confirmed #%u/%08x\n",
> index, le32_to_cpu(i2400m->barker->data[0]));
It's only used for this debug output. You may as well just delete it.
> return 0;
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH][next] mac80211: minstrel_ht: fix infinite loop because supported is not being shifted
From: Felix Fietkau @ 2019-08-23 11:07 UTC (permalink / raw)
To: Colin King, Johannes Berg, David S . Miller, linux-wireless,
netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20190822122034.28664-1-colin.king@canonical.com>
On 2019-08-22 14:20, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the for-loop will spin forever if variable supported is
> non-zero because supported is never changed. Fix this by adding in
> the missing right shift of supported.
>
> Addresses-Coverity: ("Infinite loop")
> Fixes: 48cb39522a9d ("mac80211: minstrel_ht: improve rate probing for devices with static fallback")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Felix Fietkau <nbd@nbd.name>
Thanks,
- Felix
^ permalink raw reply
* Applied "spi: spi-fsl-dspi: Reduce indentation level in dspi_interrupt" to the spi tree
From: Mark Brown @ 2019-08-23 11:02 UTC (permalink / raw)
To: Vladimir Oltean
Cc: broonie, devicetree, linux-kernel, linux-spi, Mark Brown, netdev
In-Reply-To: <20190822211514.19288-2-olteanv@gmail.com>
The patch
spi: spi-fsl-dspi: Reduce indentation level in dspi_interrupt
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 12fb61a973935c63f2580b3b053017cc14b51f42 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 23 Aug 2019 00:15:10 +0300
Subject: [PATCH] spi: spi-fsl-dspi: Reduce indentation level in dspi_interrupt
If the entire function depends on the SPI status register having the
interrupt bits asserted, then just check it and exit early if those bits
aren't set (such as in the case of the shared IRQ being triggered for
the other peripheral). Cosmetic patch.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Link: https://lore.kernel.org/r/20190822211514.19288-2-olteanv@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-dspi.c | 79 +++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 39 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 790cb02fc181..c90db7db4121 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -658,47 +658,48 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
regmap_read(dspi->regmap, SPI_SR, &spi_sr);
regmap_write(dspi->regmap, SPI_SR, spi_sr);
+ if (!(spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)))
+ return IRQ_HANDLED;
+
+ /* Get transfer counter (in number of SPI transfers). It was
+ * reset to 0 when transfer(s) were started.
+ */
+ regmap_read(dspi->regmap, SPI_TCR, &spi_tcr);
+ spi_tcnt = SPI_TCR_GET_TCNT(spi_tcr);
+ /* Update total number of bytes that were transferred */
+ msg->actual_length += spi_tcnt * dspi->bytes_per_word;
+
+ trans_mode = dspi->devtype_data->trans_mode;
+ switch (trans_mode) {
+ case DSPI_EOQ_MODE:
+ dspi_eoq_read(dspi);
+ break;
+ case DSPI_TCFQ_MODE:
+ dspi_tcfq_read(dspi);
+ break;
+ default:
+ dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
+ trans_mode);
+ return IRQ_HANDLED;
+ }
- if (spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)) {
- /* Get transfer counter (in number of SPI transfers). It was
- * reset to 0 when transfer(s) were started.
- */
- regmap_read(dspi->regmap, SPI_TCR, &spi_tcr);
- spi_tcnt = SPI_TCR_GET_TCNT(spi_tcr);
- /* Update total number of bytes that were transferred */
- msg->actual_length += spi_tcnt * dspi->bytes_per_word;
-
- trans_mode = dspi->devtype_data->trans_mode;
- switch (trans_mode) {
- case DSPI_EOQ_MODE:
- dspi_eoq_read(dspi);
- break;
- case DSPI_TCFQ_MODE:
- dspi_tcfq_read(dspi);
- break;
- default:
- dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
- trans_mode);
- return IRQ_HANDLED;
- }
+ if (!dspi->len) {
+ dspi->waitflags = 1;
+ wake_up_interruptible(&dspi->waitq);
+ return IRQ_HANDLED;
+ }
- if (!dspi->len) {
- dspi->waitflags = 1;
- wake_up_interruptible(&dspi->waitq);
- } else {
- switch (trans_mode) {
- case DSPI_EOQ_MODE:
- dspi_eoq_write(dspi);
- break;
- case DSPI_TCFQ_MODE:
- dspi_tcfq_write(dspi);
- break;
- default:
- dev_err(&dspi->pdev->dev,
- "unsupported trans_mode %u\n",
- trans_mode);
- }
- }
+ switch (trans_mode) {
+ case DSPI_EOQ_MODE:
+ dspi_eoq_write(dspi);
+ break;
+ case DSPI_TCFQ_MODE:
+ dspi_tcfq_write(dspi);
+ break;
+ default:
+ dev_err(&dspi->pdev->dev,
+ "unsupported trans_mode %u\n",
+ trans_mode);
}
return IRQ_HANDLED;
--
2.20.1
^ permalink raw reply related
* Applied "spi: spi-fsl-dspi: Remove impossible to reach error check" to the spi tree
From: Mark Brown @ 2019-08-23 11:02 UTC (permalink / raw)
To: Vladimir Oltean
Cc: broonie, devicetree, linux-kernel, linux-spi, Mark Brown, netdev
In-Reply-To: <20190822211514.19288-4-olteanv@gmail.com>
The patch
spi: spi-fsl-dspi: Remove impossible to reach error check
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 1eaeba70738e723be1e5787bdfd9a30f7471d730 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 23 Aug 2019 00:15:12 +0300
Subject: [PATCH] spi: spi-fsl-dspi: Remove impossible to reach error check
dspi->devtype_data is under the total control of the driver. Therefore,
a bad value is a driver bug and checking it at runtime (and during an
ISR, at that!) is pointless.
The second "else if" check is only for clarity (instead of a broader
"else") in case other transfer modes are added in the future. But the
printing is dead code and can be removed.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Link: https://lore.kernel.org/r/20190822211514.19288-4-olteanv@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-dspi.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 6ef2279a3699..6d2c7984ab0e 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -670,18 +670,10 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
msg->actual_length += spi_tcnt * dspi->bytes_per_word;
trans_mode = dspi->devtype_data->trans_mode;
- switch (trans_mode) {
- case DSPI_EOQ_MODE:
+ if (trans_mode == DSPI_EOQ_MODE)
dspi_eoq_read(dspi);
- break;
- case DSPI_TCFQ_MODE:
+ else if (trans_mode == DSPI_TCFQ_MODE)
dspi_tcfq_read(dspi);
- break;
- default:
- dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
- trans_mode);
- return IRQ_HANDLED;
- }
if (!dspi->len) {
dspi->waitflags = 1;
@@ -689,18 +681,10 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
- switch (trans_mode) {
- case DSPI_EOQ_MODE:
+ if (trans_mode == DSPI_EOQ_MODE)
dspi_eoq_write(dspi);
- break;
- case DSPI_TCFQ_MODE:
+ else if (trans_mode == DSPI_TCFQ_MODE)
dspi_tcfq_write(dspi);
- break;
- default:
- dev_err(&dspi->pdev->dev,
- "unsupported trans_mode %u\n",
- trans_mode);
- }
return IRQ_HANDLED;
}
--
2.20.1
^ permalink raw reply related
* Applied "spi: spi-fsl-dspi: Use poll mode in case the platform IRQ is missing" to the spi tree
From: Mark Brown @ 2019-08-23 11:02 UTC (permalink / raw)
To: Vladimir Oltean
Cc: broonie, devicetree, linux-kernel, linux-spi, Mark Brown, netdev
In-Reply-To: <20190822211514.19288-5-olteanv@gmail.com>
The patch
spi: spi-fsl-dspi: Use poll mode in case the platform IRQ is missing
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From c55be305915974db160ce6472722ff74f45b8d4e Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 23 Aug 2019 00:15:13 +0300
Subject: [PATCH] spi: spi-fsl-dspi: Use poll mode in case the platform IRQ is
missing
On platforms like LS1021A which use TCFQ mode, an interrupt needs to be
processed after each byte is TXed/RXed. I tried to make the DSPI
implementation on this SoC operate in other, more efficient modes (EOQ,
DMA) but it looks like it simply isn't possible.
Therefore allow the driver to operate in poll mode, to ease a bit of
this absurd amount of IRQ load generated in TCFQ mode. Doing so reduces
both the net time it takes to transmit a SPI message, as well as the
inter-frame jitter that occurs while doing so.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Link: https://lore.kernel.org/r/20190822211514.19288-5-olteanv@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-dspi.c | 87 ++++++++++++++++++++++++++++----------
1 file changed, 64 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 6d2c7984ab0e..77db43f1290f 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -647,19 +647,12 @@ static void dspi_eoq_read(struct fsl_dspi *dspi)
dspi_push_rx(dspi, fifo_read(dspi));
}
-static irqreturn_t dspi_interrupt(int irq, void *dev_id)
+static int dspi_rxtx(struct fsl_dspi *dspi)
{
- struct fsl_dspi *dspi = (struct fsl_dspi *)dev_id;
struct spi_message *msg = dspi->cur_msg;
enum dspi_trans_mode trans_mode;
- u32 spi_sr, spi_tcr;
u16 spi_tcnt;
-
- regmap_read(dspi->regmap, SPI_SR, &spi_sr);
- regmap_write(dspi->regmap, SPI_SR, spi_sr);
-
- if (!(spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)))
- return IRQ_NONE;
+ u32 spi_tcr;
/* Get transfer counter (in number of SPI transfers). It was
* reset to 0 when transfer(s) were started.
@@ -675,17 +668,55 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
else if (trans_mode == DSPI_TCFQ_MODE)
dspi_tcfq_read(dspi);
- if (!dspi->len) {
- dspi->waitflags = 1;
- wake_up_interruptible(&dspi->waitq);
- return IRQ_HANDLED;
- }
+ if (!dspi->len)
+ /* Success! */
+ return 0;
if (trans_mode == DSPI_EOQ_MODE)
dspi_eoq_write(dspi);
else if (trans_mode == DSPI_TCFQ_MODE)
dspi_tcfq_write(dspi);
+ return -EINPROGRESS;
+}
+
+static int dspi_poll(struct fsl_dspi *dspi)
+{
+ int tries = 1000;
+ u32 spi_sr;
+
+ do {
+ regmap_read(dspi->regmap, SPI_SR, &spi_sr);
+ regmap_write(dspi->regmap, SPI_SR, spi_sr);
+
+ if (spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF))
+ break;
+ } while (--tries);
+
+ if (!tries)
+ return -ETIMEDOUT;
+
+ return dspi_rxtx(dspi);
+}
+
+static irqreturn_t dspi_interrupt(int irq, void *dev_id)
+{
+ struct fsl_dspi *dspi = (struct fsl_dspi *)dev_id;
+ u32 spi_sr;
+
+ regmap_read(dspi->regmap, SPI_SR, &spi_sr);
+ regmap_write(dspi->regmap, SPI_SR, spi_sr);
+
+ if (!(spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)))
+ return IRQ_NONE;
+
+ dspi_rxtx(dspi);
+
+ if (!dspi->len) {
+ dspi->waitflags = 1;
+ wake_up_interruptible(&dspi->waitq);
+ }
+
return IRQ_HANDLED;
}
@@ -773,13 +804,18 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
goto out;
}
- if (trans_mode != DSPI_DMA_MODE) {
- if (wait_event_interruptible(dspi->waitq,
- dspi->waitflags))
- dev_err(&dspi->pdev->dev,
- "wait transfer complete fail!\n");
+ if (!dspi->irq) {
+ do {
+ status = dspi_poll(dspi);
+ } while (status == -EINPROGRESS);
+ } else if (trans_mode != DSPI_DMA_MODE) {
+ status = wait_event_interruptible(dspi->waitq,
+ dspi->waitflags);
dspi->waitflags = 0;
}
+ if (status)
+ dev_err(&dspi->pdev->dev,
+ "Waiting for transfer to complete failed!\n");
if (transfer->delay_usecs)
udelay(transfer->delay_usecs);
@@ -1079,10 +1115,13 @@ static int dspi_probe(struct platform_device *pdev)
goto out_ctlr_put;
dspi_init(dspi);
+
dspi->irq = platform_get_irq(pdev, 0);
- if (dspi->irq < 0) {
- ret = dspi->irq;
- goto out_clk_put;
+ if (dspi->irq <= 0) {
+ dev_info(&pdev->dev,
+ "can't get platform irq, using poll mode\n");
+ dspi->irq = 0;
+ goto poll_mode;
}
ret = devm_request_irq(&pdev->dev, dspi->irq, dspi_interrupt,
@@ -1092,6 +1131,9 @@ static int dspi_probe(struct platform_device *pdev)
goto out_clk_put;
}
+ init_waitqueue_head(&dspi->waitq);
+
+poll_mode:
if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
ret = dspi_request_dma(dspi, res->start);
if (ret < 0) {
@@ -1103,7 +1145,6 @@ static int dspi_probe(struct platform_device *pdev)
ctlr->max_speed_hz =
clk_get_rate(dspi->clk) / dspi->devtype_data->max_clock_factor;
- init_waitqueue_head(&dspi->waitq);
platform_set_drvdata(pdev, ctlr);
ret = spi_register_controller(ctlr);
--
2.20.1
^ permalink raw reply related
* Applied "spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not ours" to the spi tree
From: Mark Brown @ 2019-08-23 11:02 UTC (permalink / raw)
To: Vladimir Oltean
Cc: broonie, devicetree, linux-kernel, linux-spi, Mark Brown, netdev
In-Reply-To: <20190822211514.19288-3-olteanv@gmail.com>
The patch
spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not ours
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 37b4100180641968056cb4e034cebc38338e8652 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 23 Aug 2019 00:15:11 +0300
Subject: [PATCH] spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not
ours
The DSPI interrupt can be shared between two controllers at least on the
LX2160A. In that case, the driver for one controller might misbehave and
consume the other's interrupt. Fix this by actually checking if any of
the bits in the status register have been asserted.
Fixes: 13aed2392741 ("spi: spi-fsl-dspi: use IRQF_SHARED mode to request IRQ")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Link: https://lore.kernel.org/r/20190822211514.19288-3-olteanv@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-dspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index c90db7db4121..6ef2279a3699 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -659,7 +659,7 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
regmap_write(dspi->regmap, SPI_SR, spi_sr);
if (!(spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)))
- return IRQ_HANDLED;
+ return IRQ_NONE;
/* Get transfer counter (in number of SPI transfers). It was
* reset to 0 when transfer(s) were started.
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 2/5] spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not ours
From: Mark Brown @ 2019-08-23 10:59 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: linux-spi, lkml, devicetree, netdev
In-Reply-To: <20190823105044.GO23391@sirena.co.uk>
[-- Attachment #1: Type: text/plain, Size: 786 bytes --]
On Fri, Aug 23, 2019 at 11:50:44AM +0100, Mark Brown wrote:
> On Fri, Aug 23, 2019 at 01:30:27PM +0300, Vladimir Oltean wrote:
> > Did you see this?
> > https://lkml.org/lkml/2019/8/22/1542
> I'm not online enough to readily follow that link right now, I
> did apply another patch for a similar issue though. If that's
> a different version of the same change please don't do that,
> sending multiple conflicting versions of the same thing creates
> conflicts and makes everything harder to work with.
Oh, I guess this was due to there being an existing refactoring
in -next that meant the fix wouldn't apply directly. I sorted
that out now I think, but in general the same thing applies -
it's better to put fixes before anything else in the series,
it'll flag up when reviewing.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/5] spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not ours
From: Mark Brown @ 2019-08-23 10:50 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: linux-spi, lkml, devicetree, netdev
In-Reply-To: <CA+h21hoUfbW8Gpyfa+a-vqVp_qARYoq1_eyFfZFh-5USNGNE2g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]
On Fri, Aug 23, 2019 at 01:30:27PM +0300, Vladimir Oltean wrote:
> On Fri, 23 Aug 2019 at 13:28, Mark Brown <broonie@kernel.org> wrote:
> > It would be better to have done this as the first patch before
> > the restructuring, that way we could send this as a fix - the
> > refactoring while good doesn't really fit with stable.
> Did you see this?
> https://lkml.org/lkml/2019/8/22/1542
I'm not online enough to readily follow that link right now, I
did apply another patch for a similar issue though. If that's
a different version of the same change please don't do that,
sending multiple conflicting versions of the same thing creates
conflicts and makes everything harder to work with.
Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 06/10] net: sched: conditionally obtain rtnl lock in cls hw offloads API
From: Vlad Buslov @ 2019-08-23 10:42 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Vlad Buslov, netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
pablo@netfilter.org, Jiri Pirko
In-Reply-To: <20190822160328.46f7fab7@cakuba.netronome.com>
On Fri 23 Aug 2019 at 02:03, Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Thu, 22 Aug 2019 15:43:49 +0300, Vlad Buslov wrote:
>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> index 02a547aa77c0..bda42f1b5514 100644
>> --- a/net/sched/cls_api.c
>> +++ b/net/sched/cls_api.c
>> @@ -3076,11 +3076,28 @@ __tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
>> int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
>> void *type_data, bool err_stop, bool rtnl_held)
>> {
>> + bool take_rtnl = false;
>
> Should we perhaps speculatively:
>
> bool take_rtnl = READ_ONCE(block->lockeddevcnt);
>
> here? It shouldn't hurt, really, and otherwise every offload that
> requires rtnl will have to re-lock cb_lock, every single time..
Great idea! This looks like a straightforward opportunity for
optimization.
>
>> int ok_count;
>>
>> +retry:
>> + if (take_rtnl)
>> + rtnl_lock();
>> down_read(&block->cb_lock);
>> + /* Need to obtain rtnl lock if block is bound to devs that require it.
>> + * In block bind code cb_lock is obtained while holding rtnl, so we must
>> + * obtain the locks in same order here.
>> + */
>> + if (!rtnl_held && !take_rtnl && block->lockeddevcnt) {
>> + up_read(&block->cb_lock);
>> + take_rtnl = true;
>> + goto retry;
>> + }
>> +
>> ok_count = __tc_setup_cb_call(block, type, type_data, err_stop);
>> +
>> up_read(&block->cb_lock);
>> + if (take_rtnl)
>> + rtnl_unlock();
>> return ok_count;
>> }
>> EXPORT_SYMBOL(tc_setup_cb_call);
^ permalink raw reply
* Re: [PATCH net-next 03/10] net: sched: refactor block offloads counter usage
From: Vlad Buslov @ 2019-08-23 10:39 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Vlad Buslov, netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
pablo@netfilter.org, Jiri Pirko
In-Reply-To: <20190822155358.0171852c@cakuba.netronome.com>
On Fri 23 Aug 2019 at 01:53, Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Thu, 22 Aug 2019 15:43:46 +0300, Vlad Buslov wrote:
>> Without rtnl lock protection filters can no longer safely manage block
>> offloads counter themselves. Refactor cls API to protect block offloadcnt
>> with tcf_block->cb_lock that is already used to protect driver callback
>> list and nooffloaddevcnt counter. The counter can be modified by concurrent
>> tasks by new functions that execute block callbacks (which is safe with
>> previous patch that changed its type to atomic_t), however, block
>> bind/unbind code that checks the counter value takes cb_lock in write mode
>> to exclude any concurrent modifications. This approach prevents race
>> conditions between bind/unbind and callback execution code but allows for
>> concurrency for tc rule update path.
>>
>> Move block offload counter, filter in hardware counter and filter flags
>> management from classifiers into cls hardware offloads API. Make functions
>> tcf_block_offload_inc() and tcf_block_offload_dec() to be cls API private.
>> Implement following new cls API to be used instead:
>>
>> tc_setup_cb_add() - non-destructive filter add. If filter that wasn't
>> already in hardware is successfully offloaded, increment block offloads
>> counter, set filter in hardware counter and flag. On failure, previously
>> offloaded filter is considered to be intact and offloads counter is not
>> decremented.
>>
>> tc_setup_cb_replace() - destructive filter replace. Release existing
>> filter block offload counter and reset its in hardware counter and flag.
>> Set new filter in hardware counter and flag. On failure, previously
>> offloaded filter is considered to be destroyed and offload counter is
>> decremented.
>>
>> tc_setup_cb_destroy() - filter destroy. Unconditionally decrement block
>> offloads counter.
>>
>> Refactor all offload-capable classifiers to atomically offload filters to
>> hardware, change block offload counter, and set filter in hardware counter
>> and flag by means of the new cls API functions.
>>
>> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>
> Looks good, minor nits
>
>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> index 8502bd006b37..4215c849f4a3 100644
>> --- a/net/sched/cls_api.c
>> +++ b/net/sched/cls_api.c
>> @@ -3000,13 +3000,97 @@ int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts)
>> }
>> EXPORT_SYMBOL(tcf_exts_dump_stats);
>>
>> -int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
>> - void *type_data, bool err_stop)
>> +static void tcf_block_offload_inc(struct tcf_block *block, u32 *flags)
>> +{
>> + if (*flags & TCA_CLS_FLAGS_IN_HW)
>> + return;
>> + *flags |= TCA_CLS_FLAGS_IN_HW;
>> + atomic_inc(&block->offloadcnt);
>> +}
>> +
>> +static void tcf_block_offload_dec(struct tcf_block *block, u32 *flags)
>> +{
>> + if (!(*flags & TCA_CLS_FLAGS_IN_HW))
>> + return;
>> + *flags &= ~TCA_CLS_FLAGS_IN_HW;
>> + atomic_dec(&block->offloadcnt);
>> +}
>> +
>> +void tc_cls_offload_cnt_update(struct tcf_block *block, struct tcf_proto *tp,
>> + u32 *cnt, u32 *flags, u32 diff, bool add)
>> +{
>> + lockdep_assert_held(&block->cb_lock);
>> +
>> + spin_lock(&tp->lock);
>> + if (add) {
>> + if (!*cnt)
>> + tcf_block_offload_inc(block, flags);
>> + (*cnt) += diff;
>
> brackets unnecessary
>
>> + } else {
>> + (*cnt) -= diff;
>> + if (!*cnt)
>> + tcf_block_offload_dec(block, flags);
>> + }
>> + spin_unlock(&tp->lock);
>> +}
>> +EXPORT_SYMBOL(tc_cls_offload_cnt_update);
>> +
>> +static void
>> +tc_cls_offload_cnt_reset(struct tcf_block *block, struct tcf_proto *tp,
>> + u32 *cnt, u32 *flags)
>> +{
>> + lockdep_assert_held(&block->cb_lock);
>> +
>> + spin_lock(&tp->lock);
>> + tcf_block_offload_dec(block, flags);
>> + (*cnt) = 0;
>
> ditto
>
>> + spin_unlock(&tp->lock);
>> +}
>> +
>> +static int
>> +__tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
>> + void *type_data, bool err_stop)
>> {
>> struct flow_block_cb *block_cb;
>> int ok_count = 0;
>> int err;
>>
>> + list_for_each_entry(block_cb, &block->flow_block.cb_list, list) {
>> + err = block_cb->cb(type, type_data, block_cb->cb_priv);
>> + if (err) {
>> + if (err_stop)
>> + return err;
>> + } else {
>> + ok_count++;
>> + }
>> + }
>> + return ok_count;
>> +}
>> +
>> +int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
>> + void *type_data, bool err_stop, bool rtnl_held)
>> +{
>> + int ok_count;
>> +
>> + down_read(&block->cb_lock);
>> + ok_count = __tc_setup_cb_call(block, type, type_data, err_stop);
>> + up_read(&block->cb_lock);
>> + return ok_count;
>> +}
>> +EXPORT_SYMBOL(tc_setup_cb_call);
>> +
>> +/* Non-destructive filter add. If filter that wasn't already in hardware is
>> + * successfully offloaded, increment block offloads counter. On failure,
>> + * previously offloaded filter is considered to be intact and offloads counter
>> + * is not decremented.
>> + */
>> +
>
> Spurious new line here?
>
>> +int tc_setup_cb_add(struct tcf_block *block, struct tcf_proto *tp,
>> + enum tc_setup_type type, void *type_data, bool err_stop,
>> + u32 *flags, unsigned int *in_hw_count, bool rtnl_held)
>> +{
>> + int ok_count;
>> +
>> down_read(&block->cb_lock);
>> /* Make sure all netdevs sharing this block are offload-capable. */
>> if (block->nooffloaddevcnt && err_stop) {
>> @@ -3014,22 +3098,67 @@ int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
>> goto errout;
>> }
>>
>> - list_for_each_entry(block_cb, &block->flow_block.cb_list, list) {
>> - err = block_cb->cb(type, type_data, block_cb->cb_priv);
>> - if (err) {
>> - if (err_stop) {
>> - ok_count = err;
>> - goto errout;
>> - }
>> - } else {
>> - ok_count++;
>> - }
>> + ok_count = __tc_setup_cb_call(block, type, type_data, err_stop);
>> + if (ok_count > 0)
>> + tc_cls_offload_cnt_update(block, tp, in_hw_count, flags,
>> + ok_count, true);
>> +errout:
>
> and the labels again
>
>> + up_read(&block->cb_lock);
>> + return ok_count;
>> +}
>> +EXPORT_SYMBOL(tc_setup_cb_add);
>> +
>> +/* Destructive filter replace. If filter that wasn't already in hardware is
>> + * successfully offloaded, increment block offload counter. On failure,
>> + * previously offloaded filter is considered to be destroyed and offload counter
>> + * is decremented.
>> + */
>> +
>
> spurious new line?
>
>> +int tc_setup_cb_replace(struct tcf_block *block, struct tcf_proto *tp,
>> + enum tc_setup_type type, void *type_data, bool err_stop,
>> + u32 *old_flags, unsigned int *old_in_hw_count,
>> + u32 *new_flags, unsigned int *new_in_hw_count,
>> + bool rtnl_held)
>> +{
>> + int ok_count;
>> +
>> + down_read(&block->cb_lock);
>> + /* Make sure all netdevs sharing this block are offload-capable. */
>> + if (block->nooffloaddevcnt && err_stop) {
>> + ok_count = -EOPNOTSUPP;
>> + goto errout;
>> }
>> +
>> + tc_cls_offload_cnt_reset(block, tp, old_in_hw_count, old_flags);
>> +
>> + ok_count = __tc_setup_cb_call(block, type, type_data, err_stop);
>> + if (ok_count > 0)
>> + tc_cls_offload_cnt_update(block, tp, new_in_hw_count, new_flags,
>> + ok_count, true);
>> errout:
>> up_read(&block->cb_lock);
>> return ok_count;
>> }
>> -EXPORT_SYMBOL(tc_setup_cb_call);
>> +EXPORT_SYMBOL(tc_setup_cb_replace);
>> +
>> +/* Destroy filter and decrement block offload counter, if filter was previously
>> + * offloaded.
>> + */
>> +
>
> hm.. is this gap between comment and function it pertains to
> intentional?
Majority of function comments in cls_api.c have newline after them (not
all of them though). I don't have any strong opinions regarding this.
You suggest it is better not to have blank lines after function
comments?
>
>> +int tc_setup_cb_destroy(struct tcf_block *block, struct tcf_proto *tp,
>> + enum tc_setup_type type, void *type_data, bool err_stop,
>> + u32 *flags, unsigned int *in_hw_count, bool rtnl_held)
>> +{
>> + int ok_count;
>> +
>> + down_read(&block->cb_lock);
>> + ok_count = __tc_setup_cb_call(block, type, type_data, err_stop);
>> +
>> + tc_cls_offload_cnt_reset(block, tp, in_hw_count, flags);
>> + up_read(&block->cb_lock);
>> + return ok_count;
>> +}
>> +EXPORT_SYMBOL(tc_setup_cb_destroy);
>>
>> int tc_setup_flow_action(struct flow_action *flow_action,
>> const struct tcf_exts *exts)
>> diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
>> index 3f7a9c02b70c..7f304db7e697 100644
>> --- a/net/sched/cls_bpf.c
>> +++ b/net/sched/cls_bpf.c
>> @@ -162,17 +162,21 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
>> cls_bpf.name = obj->bpf_name;
>> cls_bpf.exts_integrated = obj->exts_integrated;
>>
>> - if (oldprog)
>> - tcf_block_offload_dec(block, &oldprog->gen_flags);
>> + if (cls_bpf.oldprog)
>
> why the change from oldprog to cls_bpf.oldprog?
No reason. Looks like a mistake I made when rewriting the conditional
for new tc_setup_cb_*() API.
>
>> + err = tc_setup_cb_replace(block, tp, TC_SETUP_CLSBPF, &cls_bpf,
>> + skip_sw, &oldprog->gen_flags,
>> + &oldprog->in_hw_count,
>> + &prog->gen_flags, &prog->in_hw_count,
>> + true);
>> + else
>> + err = tc_setup_cb_add(block, tp, TC_SETUP_CLSBPF, &cls_bpf,
>> + skip_sw, &prog->gen_flags,
>> + &prog->in_hw_count, true);
>>
>> - err = tc_setup_cb_call(block, TC_SETUP_CLSBPF, &cls_bpf, skip_sw);
>> if (prog) {
>> if (err < 0) {
>> cls_bpf_offload_cmd(tp, oldprog, prog, extack);
>> return err;
>> - } else if (err > 0) {
>> - prog->in_hw_count = err;
>> - tcf_block_offload_inc(block, &prog->gen_flags);
>> }
>> }
>>
>> @@ -230,7 +234,7 @@ static void cls_bpf_offload_update_stats(struct tcf_proto *tp,
>> cls_bpf.name = prog->bpf_name;
>> cls_bpf.exts_integrated = prog->exts_integrated;
>>
>> - tc_setup_cb_call(block, TC_SETUP_CLSBPF, &cls_bpf, false);
>> + tc_setup_cb_call(block, TC_SETUP_CLSBPF, &cls_bpf, false, true);
>> }
>>
>> static int cls_bpf_init(struct tcf_proto *tp)
>> @@ -680,8 +684,8 @@ static int cls_bpf_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb
>> continue;
>> }
>>
>> - tc_cls_offload_cnt_update(block, &prog->in_hw_count,
>> - &prog->gen_flags, add);
>> + tc_cls_offload_cnt_update(block, tp, &prog->in_hw_count,
>> + &prog->gen_flags, 1, add);
>
> Since we're adding those higher level add/replace/destroy helpers,
> would it also be possible to have a helper which takes care of
> reoffload? tc_cls_offload_cnt_update() is kind of low level now, it'd
> be cool to also hide it in the core.
Agree. I'll try to come up with something more elegant.
>
>> }
>>
>> return 0;
>> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>> index 054123742e32..0001a933d48b 100644
>> --- a/net/sched/cls_flower.c
>> +++ b/net/sched/cls_flower.c
>> @@ -419,10 +419,10 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f,
>> cls_flower.command = FLOW_CLS_DESTROY;
>> cls_flower.cookie = (unsigned long) f;
>>
>> - tc_setup_cb_call(block, TC_SETUP_CLSFLOWER, &cls_flower, false);
>> + tc_setup_cb_destroy(block, tp, TC_SETUP_CLSFLOWER, &cls_flower, false,
>> + &f->flags, &f->in_hw_count, true);
>> spin_lock(&tp->lock);
>> list_del_init(&f->hw_list);
>> - tcf_block_offload_dec(block, &f->flags);
>> spin_unlock(&tp->lock);
>>
>> if (!rtnl_held)
>> @@ -466,18 +466,15 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
>> goto errout;
>> }
>>
>> - err = tc_setup_cb_call(block, TC_SETUP_CLSFLOWER, &cls_flower, skip_sw);
>> + err = tc_setup_cb_add(block, tp, TC_SETUP_CLSFLOWER, &cls_flower,
>> + skip_sw, &f->flags, &f->in_hw_count, true);
>> kfree(cls_flower.rule);
>>
>> if (err < 0) {
>> fl_hw_destroy_filter(tp, f, true, NULL);
>> goto errout;
>> } else if (err > 0) {
>> - f->in_hw_count = err;
>> err = 0;
>
> Why does the tc_setup_cb* API still return the positive values, the
> callers should no longer care, right?
Yep. I'll refactor this for V2 and simplify related conditionals in
classifiers.
>
>> - spin_lock(&tp->lock);
>> - tcf_block_offload_inc(block, &f->flags);
>> - spin_unlock(&tp->lock);
>> }
>>
>> if (skip_sw && !(f->flags & TCA_CLS_FLAGS_IN_HW)) {
^ permalink raw reply
* [RESEND PATCH 1/5] dt-bindings: net: Add compatible for BCM4345C5 bluetooth device
From: megous @ 2019-08-23 10:31 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Marcel Holtmann,
Johan Hedberg
Cc: Mark Rutland, David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth, Ondrej Jirman
In-Reply-To: <20190823103139.17687-1-megous@megous.com>
From: Ondrej Jirman <megous@megous.com>
This is present in the AP6526 WiFi/Bluetooth 5.0 module.
Signed-off-by: Ondrej Jirman <megous@megous.com>
---
Documentation/devicetree/bindings/net/broadcom-bluetooth.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
index c26f4e11037c..4fa00e2eafcf 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
@@ -13,6 +13,7 @@ Required properties:
* "brcm,bcm20702a1"
* "brcm,bcm4330-bt"
* "brcm,bcm43438-bt"
+ * "brcm,bcm4345c5"
Optional properties:
--
2.23.0
^ permalink raw reply related
* [RESEND PATCH 2/5] bluetooth: bcm: Add support for loading firmware for BCM4345C5
From: megous @ 2019-08-23 10:31 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Marcel Holtmann,
Johan Hedberg
Cc: Mark Rutland, David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth, Ondrej Jirman
In-Reply-To: <20190823103139.17687-1-megous@megous.com>
From: Ondrej Jirman <megous@megous.com>
Detect BCM4345C5 and load a corresponding firmware file.
Signed-off-by: Ondrej Jirman <megous@megous.com>
---
Checkpatch reports a spurious error.
drivers/bluetooth/btbcm.c | 3 +++
drivers/bluetooth/hci_bcm.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 124ef0a3e1dd..2d2e6d862068 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -23,6 +23,7 @@
#define BDADDR_BCM43430A0 (&(bdaddr_t) {{0xac, 0x1f, 0x12, 0xa0, 0x43, 0x43}})
#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43}})
#define BDADDR_BCM4330B1 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb1, 0x30, 0x43}})
+#define BDADDR_BCM4345C5 (&(bdaddr_t) {{0xac, 0x1f, 0x00, 0xc5, 0x45, 0x43}})
#define BDADDR_BCM43341B (&(bdaddr_t) {{0xac, 0x1f, 0x00, 0x1b, 0x34, 0x43}})
int btbcm_check_bdaddr(struct hci_dev *hdev)
@@ -73,6 +74,7 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
!bacmp(&bda->bdaddr, BDADDR_BCM2076B1) ||
!bacmp(&bda->bdaddr, BDADDR_BCM4324B3) ||
!bacmp(&bda->bdaddr, BDADDR_BCM4330B1) ||
+ !bacmp(&bda->bdaddr, BDADDR_BCM4345C5) ||
!bacmp(&bda->bdaddr, BDADDR_BCM43430A0) ||
!bacmp(&bda->bdaddr, BDADDR_BCM43341B)) {
bt_dev_info(hdev, "BCM: Using default device address (%pMR)",
@@ -332,6 +334,7 @@ static const struct bcm_subver_table bcm_uart_subver_table[] = {
{ 0x2122, "BCM4343A0" }, /* 001.001.034 */
{ 0x2209, "BCM43430A1" }, /* 001.002.009 */
{ 0x6119, "BCM4345C0" }, /* 003.001.025 */
+ { 0x6606, "BCM4345C5" }, /* 003.006.006 */
{ 0x230f, "BCM4356A2" }, /* 001.003.015 */
{ 0x220e, "BCM20702A1" }, /* 001.002.014 */
{ 0x4217, "BCM4329B1" }, /* 002.002.023 */
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 4d9de20bab7b..95c312ae94cf 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -1419,6 +1419,7 @@ static void bcm_serdev_remove(struct serdev_device *serdev)
#ifdef CONFIG_OF
static const struct of_device_id bcm_bluetooth_of_match[] = {
{ .compatible = "brcm,bcm20702a1" },
+ { .compatible = "brcm,bcm4345c5" },
{ .compatible = "brcm,bcm4330-bt" },
{ .compatible = "brcm,bcm43438-bt" },
{ },
--
2.23.0
^ permalink raw reply related
* [RESEND PATCH 0/5] Add bluetooth support for Orange Pi 3
From: megous @ 2019-08-23 10:31 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Marcel Holtmann,
Johan Hedberg
Cc: Mark Rutland, David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth, Ondrej Jirman
From: Ondrej Jirman <megous@megous.com>
(Resend to add missing lists, sorry for the noise.)
This series implements bluetooth support for Xunlong Orange Pi 3 board.
The board uses AP6256 WiFi/BT 5.0 chip.
Summary of changes:
- add more delay to let initialize the chip
- let the kernel detect firmware file path
- add new compatible and update dt-bindings
- update Orange Pi 3 / H6 DTS
Please take a look.
thank you and regards,
Ondrej Jirman
Ondrej Jirman (5):
dt-bindings: net: Add compatible for BCM4345C5 bluetooth device
bluetooth: bcm: Add support for loading firmware for BCM4345C5
bluetooth: hci_bcm: Give more time to come out of reset
arm64: dts: allwinner: h6: Add pin configs for uart1
arm64: dts: allwinner: orange-pi-3: Enable UART1 / Bluetooth
.../bindings/net/broadcom-bluetooth.txt | 1 +
.../dts/allwinner/sun50i-h6-orangepi-3.dts | 19 +++++++++++++++++++
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
drivers/bluetooth/btbcm.c | 3 +++
drivers/bluetooth/hci_bcm.c | 3 ++-
5 files changed, 35 insertions(+), 1 deletion(-)
--
2.23.0
^ permalink raw reply
* [RESEND PATCH 4/5] arm64: dts: allwinner: h6: Add pin configs for uart1
From: megous @ 2019-08-23 10:31 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Marcel Holtmann,
Johan Hedberg
Cc: Mark Rutland, David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth, Ondrej Jirman
In-Reply-To: <20190823103139.17687-1-megous@megous.com>
From: Ondrej Jirman <megous@megous.com>
Orange Pi 3 uses UART1 for bluetooth. Add pinconfigs so that we can use
them.
Signed-off-by: Ondrej Jirman <megous@megous.com>
---
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 67f920e0fc33..7657e816096b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -298,6 +298,16 @@
pins = "PH0", "PH1";
function = "uart0";
};
+
+ uart1_pins: uart1-pins {
+ pins = "PG6", "PG7";
+ function = "uart1";
+ };
+
+ uart1_rts_cts_pins: uart1-rts-cts-pins {
+ pins = "PG8", "PG9";
+ function = "uart1";
+ };
};
gic: interrupt-controller@3021000 {
--
2.23.0
^ permalink raw reply related
* [RESEND PATCH 5/5] arm64: dts: allwinner: orange-pi-3: Enable UART1 / Bluetooth
From: megous @ 2019-08-23 10:31 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Marcel Holtmann,
Johan Hedberg
Cc: Mark Rutland, David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth, Ondrej Jirman
In-Reply-To: <20190823103139.17687-1-megous@megous.com>
From: Ondrej Jirman <megous@megous.com>
The board contains AP6256 WiFi/BT module that has its bluetooth part
connected to SoC's UART1 port. Enable this port, and add node for the
bluetooth device.
Bluetooth part is named bcm4345c5.
You'll need a BCM4345C5.hcd firmware file that can be found in the
Xulongs's repository for H6:
https://github.com/orangepi-xunlong/OrangePiH6_external/tree/master/ap6256
The driver expects the firmware at the following path relative to the
firmware directory:
brcm/BCM4345C5.hcd
Signed-off-by: Ondrej Jirman <megous@megous.com>
---
.../dts/allwinner/sun50i-h6-orangepi-3.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
index 49d954369087..a9e776446c35 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
@@ -15,6 +15,7 @@
aliases {
serial0 = &uart0;
+ serial1 = &uart1;
};
chosen {
@@ -271,6 +272,24 @@
status = "okay";
};
+/* There's the BT part of the AP6256 connected to that UART */
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+ uart-has-rtscts;
+ status = "okay";
+
+ bluetooth {
+ compatible = "brcm,bcm4345c5";
+ clocks = <&rtc 1>;
+ clock-names = "lpo";
+ device-wakeup-gpios = <&r_pio 1 2 GPIO_ACTIVE_HIGH>; /* PM2 */
+ host-wakeup-gpios = <&r_pio 1 1 GPIO_ACTIVE_HIGH>; /* PM1 */
+ shutdown-gpios = <&r_pio 1 4 GPIO_ACTIVE_HIGH>; /* PM4 */
+ max-speed = <1500000>;
+ };
+};
+
&usb2otg {
/*
* This board doesn't have a controllable VBUS even though it
--
2.23.0
^ permalink raw reply related
* [RESEND PATCH 3/5] bluetooth: hci_bcm: Give more time to come out of reset
From: megous @ 2019-08-23 10:31 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Marcel Holtmann,
Johan Hedberg
Cc: Mark Rutland, David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth, Ondrej Jirman
In-Reply-To: <20190823103139.17687-1-megous@megous.com>
From: Ondrej Jirman <megous@megous.com>
Some supported devices need more time to come out of reset (eg.
BCM4345C5 in AP6256).
I don't have/found a datasheet, so the value was arrive at
experimentally with the Oprange Pi 3 board. Without increased delay,
I got intermittent failures during probe. This is a Bluetooth 5.0
device, so maybe that's why it takes longer to initialize than the
others.
Signed-off-by: Ondrej Jirman <megous@megous.com>
---
drivers/bluetooth/hci_bcm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 95c312ae94cf..7646636f2d18 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -260,7 +260,7 @@ static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
}
/* wait for device to power on and come out of reset */
- usleep_range(10000, 20000);
+ usleep_range(100000, 120000);
dev->res_enabled = powered;
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v2 2/5] spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not ours
From: Vladimir Oltean @ 2019-08-23 10:30 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-spi, lkml, devicetree, netdev
In-Reply-To: <20190823102816.GN23391@sirena.co.uk>
Hi Mark,
On Fri, 23 Aug 2019 at 13:28, Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Aug 23, 2019 at 12:15:11AM +0300, Vladimir Oltean wrote:
> > The DSPI interrupt can be shared between two controllers at least on the
> > LX2160A. In that case, the driver for one controller might misbehave and
> > consume the other's interrupt. Fix this by actually checking if any of
> > the bits in the status register have been asserted.
>
> It would be better to have done this as the first patch before
> the restructuring, that way we could send this as a fix - the
> refactoring while good doesn't really fit with stable.
Did you see this?
https://lkml.org/lkml/2019/8/22/1542
Regards,
-Vladimir
^ permalink raw reply
* Re: [PATCH v2 2/5] spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not ours
From: Mark Brown @ 2019-08-23 10:28 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: linux-spi, linux-kernel, devicetree, netdev
In-Reply-To: <20190822211514.19288-3-olteanv@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 523 bytes --]
On Fri, Aug 23, 2019 at 12:15:11AM +0300, Vladimir Oltean wrote:
> The DSPI interrupt can be shared between two controllers at least on the
> LX2160A. In that case, the driver for one controller might misbehave and
> consume the other's interrupt. Fix this by actually checking if any of
> the bits in the status register have been asserted.
It would be better to have done this as the first patch before
the restructuring, that way we could send this as a fix - the
refactoring while good doesn't really fit with stable.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC bpf-next 0/5] Convert iproute2 to use libbpf (WIP)
From: Jesper Dangaard Brouer @ 2019-08-23 10:27 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Toke Høiland-Jørgensen, Stephen Hemminger,
Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
Yonghong Song, David Miller, Networking, bpf, brouer,
Anton Protopopov, Stanislav Fomichev, Yoel Caspersen
In-Reply-To: <CAEf4BzZxb7qZabw6aDVaTqnhr3AGtwEo+DbuBR9U9tJr+qVuyg@mail.gmail.com>
On Wed, 21 Aug 2019 13:30:09 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> On Tue, Aug 20, 2019 at 4:47 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > iproute2 uses its own bpf loader to load eBPF programs, which has
> > evolved separately from libbpf. Since we are now standardising on
> > libbpf, this becomes a problem as iproute2 is slowly accumulating
> > feature incompatibilities with libbpf-based loaders. In particular,
> > iproute2 has its own (expanded) version of the map definition struct,
> > which makes it difficult to write programs that can be loaded with both
> > custom loaders and iproute2.
> >
> > This series seeks to address this by converting iproute2 to using libbpf
> > for all its bpf needs. This version is an early proof-of-concept RFC, to
> > get some feedback on whether people think this is the right direction.
> >
> > What this series does is the following:
> >
> > - Updates the libbpf map definition struct to match that of iproute2
> > (patch 1).
>
>
> Hi Toke,
>
> Thanks for taking a stab at unifying libbpf and iproute2 loaders. I'm
> totally in support of making iproute2 use libbpf to load/initialize
> BPF programs. But I'm against adding iproute2-specific fields to
> libbpf's bpf_map_def definitions to support this.
>
> I've proposed the plan of extending libbpf's supported features so
> that it can be used to load iproute2-style BPF programs earlier,
> please see discussions in [0] and [1]. I think instead of emulating
> iproute2 way of matching everything based on user-specified internal
> IDs, which doesn't provide good user experience and is quite easy to
> get wrong, we should support same scenarios with better declarative
> syntax and in a less error-prone way. I believe we can do that by
> relying on BTF more heavily (again, please check some of my proposals
> in [0], [1], and discussion with Daniel in those threads). It will
> feel more natural and be more straightforward to follow. It would be
> great if you can lend a hand in implementing pieces of that plan!
>
> I'm currently on vacation, so my availability is very sparse, but I'd
> be happy to discuss this further, if need be.
>
> [0] https://lore.kernel.org/bpf/CAEf4BzbfdG2ub7gCi0OYqBrUoChVHWsmOntWAkJt47=FE+km+A@mail.gmail.com/
> [1] https://www.spinics.net/lists/bpf/msg03976.html
>
> > - Adds functionality to libbpf to support automatic pinning of maps when
> > loading an eBPF program, while re-using pinned maps if they already
> > exist (patches 2-3).
For production use-cases, libbpf really need an easier higher-level API
for re-using pinned maps, for establishing shared maps between
programs. The existing libbpf API bpf_object__pin_maps() and
bpf_object__unpin_maps(), which don't re-use pinned maps, are not
really usable, because they pin/unpin ALL maps in the ELF file.
What users really need is an easy way to specify, on a per map basis,
what kind of pinning and reuse/sharing they want. E.g. like iproute2
have, "global", "object-scope", and "no-pinning". ("ifindex-scope" would
be nice for XDP).
Today users have to split/reimplement bpf_prog_load_xattr(), and
use/add bpf_map__reuse_fd(). Which is that I ended doing for
xdp-cpumap-tc[2] (used in production at ISP) resulting in 142 lines of
extra code[3] that should have been hidden inside libbpf. And worse,
in this solution[4] the maps for reuse-pinning is specified in the code
by name. Thus, they cannot use a generic loader. That I why, I want
to mark the maps via a pinning member, like iproute2.
I really hope this moves in a practical direction, as I have the next
production request lined up (also from an ISP), and I hate to have to
advice them to choose the same route as [3].
[2] https://github.com/xdp-project/xdp-cpumap-tc/
[3] https://github.com/xdp-project/xdp-cpumap-tc/blob/master/src/xdp_iphash_to_cpu_user.c#L262-L403
[4] https://github.com/xdp-project/xdp-cpumap-tc/blob/master/src/xdp_iphash_to_cpu_user.c#L431-L441
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: kernel BUG at include/linux/skbuff.h:LINE! (2)
From: Xin Long @ 2019-08-23 10:26 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: syzbot, davem, LKML, linux-sctp, Marcelo Ricardo Leitner,
network dev, Neil Horman, syzkaller-bugs, Vlad Yasevich
In-Reply-To: <CADvbK_c7BXurbHyAqjX+0h2ZYtmJ0802zxmQB3qv8=GLv2ig9g@mail.gmail.com>
On Mon, Aug 19, 2019 at 10:44 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Sun, Aug 18, 2019 at 10:13 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > On Sun, Aug 18, 2019 at 7:07 AM Xin Long <lucien.xin@gmail.com> wrote:
> > >
> > > On Sat, Aug 17, 2019 at 2:38 AM syzbot
> > > <syzbot+eb349eeee854e389c36d@syzkaller.appspotmail.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > > > HEAD commit: 459c5fb4 Merge branch 'mscc-PTP-support'
> > > > git tree: net-next
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=13f2d33c600000
> > > > kernel config: https://syzkaller.appspot.com/x/.config?x=d4cf1ffb87d590d7
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=eb349eeee854e389c36d
> > > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=111849e2600000
> > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1442c25a600000
> > > >
> > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > Reported-by: syzbot+eb349eeee854e389c36d@syzkaller.appspotmail.com
> > > >
> > > > ------------[ cut here ]------------
> > > > kernel BUG at include/linux/skbuff.h:2225!
> > > > invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> > > > CPU: 0 PID: 9030 Comm: syz-executor649 Not tainted 5.3.0-rc3+ #134
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > Google 01/01/2011
> > > > RIP: 0010:__skb_pull include/linux/skbuff.h:2225 [inline]
> > > > RIP: 0010:__skb_pull include/linux/skbuff.h:2222 [inline]
> > > > RIP: 0010:skb_pull_inline include/linux/skbuff.h:2231 [inline]
> > > > RIP: 0010:skb_pull+0xea/0x110 net/core/skbuff.c:1902
> > > > Code: 9d c8 00 00 00 49 89 dc 49 89 9d c8 00 00 00 e8 9c e5 dd fb 4c 89 e0
> > > > 5b 41 5c 41 5d 41 5e 5d c3 45 31 e4 eb ea e8 86 e5 dd fb <0f> 0b e8 df 13
> > > > 18 fc e9 44 ff ff ff e8 d5 13 18 fc eb 8a e8 ee 13
> > > > RSP: 0018:ffff88808ac96e10 EFLAGS: 00010293
> > > > RAX: ffff88809c546000 RBX: 0000000000000004 RCX: ffffffff8594a3a6
> > > > RDX: 0000000000000000 RSI: ffffffff8594a3fa RDI: 0000000000000004
> > > > RBP: ffff88808ac96e30 R08: ffff88809c546000 R09: fffffbfff14a8f4f
> > > > R10: fffffbfff14a8f4e R11: ffffffff8a547a77 R12: 0000000095e28bcc
> > > > R13: ffff88808ac97478 R14: 00000000ffff8880 R15: ffff88808ac97478
> > > > FS: 0000555556549880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > CR2: 0000000020000100 CR3: 0000000089c3c000 CR4: 00000000001406f0
> > > > Call Trace:
> > > > sctp_inq_pop+0x2f1/0xd80 net/sctp/inqueue.c:202
> > > > sctp_endpoint_bh_rcv+0x184/0x8d0 net/sctp/endpointola.c:385
> > > > sctp_inq_push+0x1e4/0x280 net/sctp/inqueue.c:80
> > > > sctp_rcv+0x2807/0x3590 net/sctp/input.c:256
> > > > sctp6_rcv+0x17/0x30 net/sctp/ipv6.c:1049
> > > > ip6_protocol_deliver_rcu+0x2fe/0x1660 net/ipv6/ip6_input.c:397
> > > > ip6_input_finish+0x84/0x170 net/ipv6/ip6_input.c:438
> > > > NF_HOOK include/linux/netfilter.h:305 [inline]
> > > > NF_HOOK include/linux/netfilter.h:299 [inline]
> > > > ip6_input+0xe4/0x3f0 net/ipv6/ip6_input.c:447
> > > > dst_input include/net/dst.h:442 [inline]
> > > > ip6_sublist_rcv_finish+0x98/0x1e0 net/ipv6/ip6_input.c:84
> > > Looks skb_list_del_init() should be called in ip6_sublist_rcv_finish,
> > > as does in ip_sublist_rcv_finish().
> >
> > This was recently introduced, right? Only in net-next and linux-next.
> > Otherwise, is it a remote DoS? If so and if it's present in any
> > releases, may need a CVE.
> I need to reproduce and confirm it, will let you know.
The panic could be triggered since the listified RX support for
GRO_NORMAL skbs:
https://patchwork.ozlabs.org/cover/1142808/
(it's only in net-next now, I will post a fix soon)
But the bug itself is not really related with the patch series above.
the issue here is pretty much like what this patch fixed:
https://patchwork.ozlabs.org/patch/942541/
I didn't see a CVE for it, maybe because it was only on net-next too.
^ permalink raw reply
* Re: [PATCH net-next 01/10] net: sched: protect block offload-related fields with rw_semaphore
From: Vlad Buslov @ 2019-08-23 10:25 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Vlad Buslov, netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
pablo@netfilter.org, Jiri Pirko
In-Reply-To: <20190822151530.09f7ca04@cakuba.netronome.com>
On Fri 23 Aug 2019 at 01:15, Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Thu, 22 Aug 2019 15:43:44 +0300, Vlad Buslov wrote:
>> @@ -2987,19 +3007,26 @@ int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
>> int ok_count = 0;
>> int err;
>>
>> + down_read(&block->cb_lock);
>> /* Make sure all netdevs sharing this block are offload-capable. */
>> - if (block->nooffloaddevcnt && err_stop)
>> - return -EOPNOTSUPP;
>> + if (block->nooffloaddevcnt && err_stop) {
>> + ok_count = -EOPNOTSUPP;
>> + goto errout;
>> + }
>>
>> list_for_each_entry(block_cb, &block->flow_block.cb_list, list) {
>> err = block_cb->cb(type, type_data, block_cb->cb_priv);
>> if (err) {
>> - if (err_stop)
>> - return err;
>> + if (err_stop) {
>> + ok_count = err;
>> + goto errout;
>> + }
>> } else {
>> ok_count++;
>> }
>> }
>> +errout:
>
> Please name the labels with the first action they perform. Here:
>
> err_unlock:
Thanks for reviewing. Will fix in V2.
>
>> + up_read(&block->cb_lock);
>> return ok_count;
^ permalink raw reply
* Re: [PATCH v2 11/11] vsock_test: wait for the remote to close the connection
From: Stefan Hajnoczi @ 2019-08-23 10:09 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, kvm, Dexuan Cui, virtualization, David S. Miller,
Jorgen Hansen, linux-kernel
In-Reply-To: <20190822091546.qcns2kot6tzju7yv@steredhat>
[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]
On Thu, Aug 22, 2019 at 11:15:46AM +0200, Stefano Garzarella wrote:
> On Tue, Aug 20, 2019 at 09:28:28AM +0100, Stefan Hajnoczi wrote:
> > On Thu, Aug 01, 2019 at 05:25:41PM +0200, Stefano Garzarella wrote:
> > > +/* Wait for the remote to close the connection */
> > > +void vsock_wait_remote_close(int fd)
> > > +{
> > > + struct epoll_event ev;
> > > + int epollfd, nfds;
> > > +
> > > + epollfd = epoll_create1(0);
> > > + if (epollfd == -1) {
> > > + perror("epoll_create1");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + ev.events = EPOLLRDHUP | EPOLLHUP;
> > > + ev.data.fd = fd;
> > > + if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
> > > + perror("epoll_ctl");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + nfds = epoll_wait(epollfd, &ev, 1, TIMEOUT * 1000);
> > > + if (nfds == -1) {
> > > + perror("epoll_wait");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + if (nfds == 0) {
> > > + fprintf(stderr, "epoll_wait timed out\n");
> > > + exit(EXIT_FAILURE);
> > > + }
> > > +
> > > + assert(nfds == 1);
> > > + assert(ev.events & (EPOLLRDHUP | EPOLLHUP));
> > > + assert(ev.data.fd == fd);
> > > +
> > > + close(epollfd);
> > > +}
> >
> > Please use timeout_begin()/timeout_end() so that the test cannot hang.
> >
>
> I used the TIMEOUT macro in the epoll_wait() to avoid the hang.
> Do you think is better to use the timeout_begin()/timeout_end()?
> In this case, should I remove the timeout in the epoll_wait()?
Oops, you're right. There are no other blocking calls in this function
so the existing patch is fine.
Thanks,
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v6 5/7] nfc: pn533: add UART phy driver
From: Lars Poeschel @ 2019-08-23 10:06 UTC (permalink / raw)
To: Claudiu.Beznea
Cc: gregkh, tglx, swinslow, allison, opensource, kstewart,
linux-kernel, netdev, johan
In-Reply-To: <909777a0-a70e-2174-4455-4afa0591a462@microchip.com>
On Thu, Aug 22, 2019 at 10:09:09AM +0000, Claudiu.Beznea@microchip.com wrote:
> Hi Lars,
>
> On 20.08.2019 15:03, Lars Poeschel wrote:
> > This adds the UART phy interface for the pn533 driver.
> > The pn533 driver can be used through UART interface this way.
> > It is implemented as a serdev device.
> >
> > Cc: Johan Hovold <johan@kernel.org>
> > Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
> > ---
> > Changes in v6:
> > - Rebased the patch series on v5.3-rc5
> >
> > Changes in v5:
> > - Use the splitted pn53x_common_init and pn53x_register_nfc
> > and pn53x_common_clean and pn53x_unregister_nfc alike
> >
> > Changes in v4:
> > - SPDX-License-Identifier: GPL-2.0+
> > - Source code comments above refering items
> > - Error check for serdev_device_write's
> > - Change if (xxx == NULL) to if (!xxx)
> > - Remove device name from a dev_err
> > - move pn533_register in _probe a bit towards the end of _probe
> > - make use of newly added dev_up / dev_down phy_ops
> > - control send_wakeup variable from dev_up / dev_down
> >
> > Changes in v3:
> > - depend on SERIAL_DEV_BUS in Kconfig
> >
> > Changes in v2:
> > - switched from tty line discipline to serdev, resulting in many
> > simplifications
> > - SPDX License Identifier
> >
> > drivers/nfc/pn533/Kconfig | 11 ++
> > drivers/nfc/pn533/Makefile | 2 +
> > drivers/nfc/pn533/pn533.h | 8 +
> > drivers/nfc/pn533/uart.c | 316 +++++++++++++++++++++++++++++++++++++
> > 4 files changed, 337 insertions(+)
> > create mode 100644 drivers/nfc/pn533/uart.c
> >
> > diff --git a/drivers/nfc/pn533/Kconfig b/drivers/nfc/pn533/Kconfig
> > index f6d6b345ba0d..7fe1bbe26568 100644
> > --- a/drivers/nfc/pn533/Kconfig
> > +++ b/drivers/nfc/pn533/Kconfig
> > @@ -26,3 +26,14 @@ config NFC_PN533_I2C
> >
> > If you choose to build a module, it'll be called pn533_i2c.
> > Say N if unsure.
> > +
> > +config NFC_PN532_UART
> > + tristate "NFC PN532 device support (UART)"
> > + depends on SERIAL_DEV_BUS
> > + select NFC_PN533
> > + ---help---
> > + This module adds support for the NXP pn532 UART interface.
> > + Select this if your platform is using the UART bus.
> > +
> > + If you choose to build a module, it'll be called pn532_uart.
> > + Say N if unsure.
> > diff --git a/drivers/nfc/pn533/Makefile b/drivers/nfc/pn533/Makefile
> > index 43c25b4f9466..b9648337576f 100644
> > --- a/drivers/nfc/pn533/Makefile
> > +++ b/drivers/nfc/pn533/Makefile
> > @@ -4,7 +4,9 @@
> > #
> > pn533_usb-objs = usb.o
> > pn533_i2c-objs = i2c.o
> > +pn532_uart-objs = uart.o
> >
> > obj-$(CONFIG_NFC_PN533) += pn533.o
> > obj-$(CONFIG_NFC_PN533_USB) += pn533_usb.o
> > obj-$(CONFIG_NFC_PN533_I2C) += pn533_i2c.o
> > +obj-$(CONFIG_NFC_PN532_UART) += pn532_uart.o
> > diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
> > index 510ddebbd896..6541088fad73 100644
> > --- a/drivers/nfc/pn533/pn533.h
> > +++ b/drivers/nfc/pn533/pn533.h
> > @@ -43,6 +43,11 @@
> >
> > /* Preamble (1), SoPC (2), ACK Code (2), Postamble (1) */
> > #define PN533_STD_FRAME_ACK_SIZE 6
> > +/*
> > + * Preamble (1), SoPC (2), Packet Length (1), Packet Length Checksum (1),
> > + * Specific Application Level Error Code (1) , Postamble (1)
> > + */
> > +#define PN533_STD_ERROR_FRAME_SIZE 8
> > #define PN533_STD_FRAME_CHECKSUM(f) (f->data[f->datalen])
> > #define PN533_STD_FRAME_POSTAMBLE(f) (f->data[f->datalen + 1])
> > /* Half start code (3), LEN (4) should be 0xffff for extended frame */
> > @@ -84,6 +89,9 @@
> > #define PN533_CMD_MI_MASK 0x40
> > #define PN533_CMD_RET_SUCCESS 0x00
> >
> > +#define PN533_FRAME_DATALEN_ACK 0x00
> > +#define PN533_FRAME_DATALEN_ERROR 0x01
> > +#define PN533_FRAME_DATALEN_EXTENDED 0xFF
> >
> > enum pn533_protocol_type {
> > PN533_PROTO_REQ_ACK_RESP = 0,
> > diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
> > new file mode 100644
> > index 000000000000..f1cc2354a4fd
> > --- /dev/null
> > +++ b/drivers/nfc/pn533/uart.c
> > @@ -0,0 +1,316 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Driver for NXP PN532 NFC Chip - UART transport layer
> > + *
> > + * Copyright (C) 2018 Lemonage Software GmbH
> > + * Author: Lars Pöschel <poeschel@lemonage.de>
> > + * All rights reserved.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/nfc.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/of.h>
> > +#include <linux/serdev.h>
> > +#include "pn533.h"
> > +
> > +#define PN532_UART_SKB_BUFF_LEN (PN533_CMD_DATAEXCH_DATA_MAXLEN * 2)
> > +
> > +enum send_wakeup {
> > + PN532_SEND_NO_WAKEUP = 0,
> > + PN532_SEND_WAKEUP,
> > + PN532_SEND_LAST_WAKEUP,
> > +};
> > +
> > +
> > +struct pn532_uart_phy {
> > + struct serdev_device *serdev;
> > + struct sk_buff *recv_skb;
> > + struct pn533 *priv;
> > + enum send_wakeup send_wakeup;
>
> Could there be any concurrency issues w/ regards to accessing this
> variable? I see it is accessed in pn532_uart_send_frame(), pn532_dev_up(),
> pn532_dev_down() which may be called from the following wq:
>
> INIT_WORK(&priv->mi_tm_rx_work, pn533_wq_tm_mi_recv);
>
> INIT_WORK(&priv->mi_tm_tx_work, pn533_wq_tm_mi_send);
>
> INIT_DELAYED_WORK(&priv->poll_work, pn533_wq_poll);
>
>
> and from net/nfc/core.c via dev_up()/dev_down().
Well, I spend some minutes thinking about this. There should be no real
problem. The code in pn533.c ensures, that commands are transmitted
sequencially. And it always is command - response. So if a command is
send, the driver waits for a response from the chip.
So pn532_uart_send_frame should not be called multiple times without
reaching at least serdev_device_write, but at this point the race is
already over.
There is one exception, this is the abort command. This command can be
sent without receiving a previous response. So there is the possibility
of a successful race.
The send_wakeup variable is used to control if we need to send a
wakeup request to the pn532 chip prior to the actual command we would
like to send.
Worst thing that I see could happen - if the race succeeds - is that we
send a wakeup to the chip that is propably not needed as it is already
awake. But this does not hurt as a wakeup send to the pn532 is
essentially a no-op if the chip is awake already. I could have
implemented it so, that a wakeup is sent in front of every command
without thinking and the driver would work.
The same is with pn532_dev_up. It could be that there is one wakeup sent
to much, but it does not hurt.
pn532_dev_down is not problematic I think.
To sum it up: There is maybe a very little probability, but it does
nothing bad. Question is now: Is it worth mutex'ing the send_wakeup
variable or can we leave it as-is ?
Thank you for your review, Claudiu.
Regards,
Lars
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox